Better abstract wildcards for Ethernet destination field.

I think that this makes nx-match.c a little easier to read.  The new
functions added here will have more users in an upcoming patch.

diff --git a/lib/classifier.c b/lib/classifier.c
index edaceb4..e89a6c8 100644 (file)
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -162,6 +162,7 @@ cls_rule_set_dl_src(struct cls_rule *rule, const uint8_t dl_src[ETH_ADDR_LEN])
     memcpy(rule->flow.dl_src, dl_src, ETH_ADDR_LEN);
 }
 
+/* Modifies 'rule' so that the Ethernet address must match 'dl_dst' exactly. */
 void
 cls_rule_set_dl_dst(struct cls_rule *rule, const uint8_t dl_dst[ETH_ADDR_LEN])
 {
@@ -169,6 +170,25 @@ cls_rule_set_dl_dst(struct cls_rule *rule, const uint8_t dl_dst[ETH_ADDR_LEN])
     memcpy(rule->flow.dl_dst, dl_dst, ETH_ADDR_LEN);
 }
 
+/* Modifies 'rule' so that the Ethernet address must match 'dl_dst' after each
+ * byte is ANDed with the appropriate byte in 'mask'.
+ *
+ * This function will assert-fail if 'mask' is invalid.  Only 'mask' values
+ * accepted by flow_wildcards_is_dl_dst_mask_valid() are allowed. */
+void
+cls_rule_set_dl_dst_masked(struct cls_rule *rule,
+                           const uint8_t dl_dst[ETH_ADDR_LEN],
+                           const uint8_t mask[ETH_ADDR_LEN])
+{
+    flow_wildcards_t *wc = &rule->wc.wildcards;
+    size_t i;
+
+    *wc = flow_wildcards_set_dl_dst_mask(*wc, mask);
+    for (i = 0; i < ETH_ADDR_LEN; i++) {
+        rule->flow.dl_dst[i] = dl_dst[i] & mask[i];
+    }
+}
+
 void
 cls_rule_set_dl_tci(struct cls_rule *rule, ovs_be16 tci)
 {

diff --git a/lib/classifier.h b/lib/classifier.h
index 08e2c0d..4227c98 100644 (file)
--- a/lib/classifier.h
+++ b/lib/classifier.h
@@ -84,6 +84,8 @@ void cls_rule_set_in_port(struct cls_rule *, uint16_t odp_port);
 void cls_rule_set_dl_type(struct cls_rule *, ovs_be16);
 void cls_rule_set_dl_src(struct cls_rule *, const uint8_t[6]);
 void cls_rule_set_dl_dst(struct cls_rule *, const uint8_t[6]);
+void cls_rule_set_dl_dst_masked(struct cls_rule *, const uint8_t dl_dst[6],
+                                const uint8_t mask[6]);
 void cls_rule_set_dl_tci(struct cls_rule *, ovs_be16 tci);
 void cls_rule_set_dl_tci_masked(struct cls_rule *,
                                 ovs_be16 tci, ovs_be16 mask);

diff --git a/lib/flow.c b/lib/flow.c
index 754c0de..85d7bfe 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -16,6 +16,7 @@
 #include <config.h>
 #include <sys/types.h>
 #include "flow.h"
+#include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <netinet/in.h>
@@ -726,6 +727,83 @@ flow_wildcards_set_reg_mask(struct flow_wildcards *wc, int idx, uint32_t mask)
     wc->reg_masks[idx] = mask;
 }
 
+/* Returns the wildcard bitmask for the Ethernet destination address
+ * that 'wc' specifies.  The bitmask has a 0 in each bit that is wildcarded
+ * and a 1 in each bit that must match.  */
+const uint8_t *
+flow_wildcards_to_dl_dst_mask(flow_wildcards_t wc)
+{
+    static const uint8_t    no_wild[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+    static const uint8_t  addr_wild[] = {0x01, 0x00, 0x00, 0x00, 0x00, 0x00};
+    static const uint8_t mcast_wild[] = {0xfe, 0xff, 0xff, 0xff, 0xff, 0xff};
+    static const uint8_t   all_wild[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+    switch (wc & (FWW_DL_DST | FWW_ETH_MCAST)) {
+    case 0:                             return no_wild;
+    case FWW_DL_DST:                    return addr_wild;
+    case FWW_ETH_MCAST:                 return mcast_wild;
+    case FWW_DL_DST | FWW_ETH_MCAST:    return all_wild;
+    }
+    NOT_REACHED();
+}
+
+/* Returns true if 'mask' is a valid wildcard bitmask for the Ethernet
+ * destination address.  Valid bitmasks are either all-bits-0 or all-bits-1,
+ * except that the multicast bit may differ from the rest of the bits.  So,
+ * there are four possible valid bitmasks:
+ *
+ *  - 00:00:00:00:00:00
+ *  - 01:00:00:00:00:00
+ *  - fe:ff:ff:ff:ff:ff
+ *  - ff:ff:ff:ff:ff:ff
+ *
+ * All other bitmasks are invalid. */
+bool
+flow_wildcards_is_dl_dst_mask_valid(const uint8_t mask[ETH_ADDR_LEN])
+{
+    switch (mask[0]) {
+    case 0x00:
+    case 0x01:
+        return (mask[1] | mask[2] | mask[3] | mask[4] | mask[5]) == 0x00;
+
+    case 0xfe:
+    case 0xff:
+        return (mask[1] & mask[2] & mask[3] & mask[4] & mask[5]) == 0xff;
+
+    default:
+        return false;
+    }
+}
+
+/* Returns 'wc' with the FWW_DL_DST and FWW_ETH_MCAST bits modified
+ * appropriately to match 'mask'.
+ *
+ * This function will assert-fail if 'mask' is invalid.  Only 'mask' values
+ * accepted by flow_wildcards_is_dl_dst_mask_valid() are allowed. */
+flow_wildcards_t
+flow_wildcards_set_dl_dst_mask(flow_wildcards_t wc,
+                               const uint8_t mask[ETH_ADDR_LEN])
+{
+    assert(flow_wildcards_is_dl_dst_mask_valid(mask));
+
+    switch (mask[0]) {
+    case 0x00:
+        return wc | FWW_DL_DST | FWW_ETH_MCAST;
+
+    case 0x01:
+        return (wc | FWW_DL_DST) & ~FWW_ETH_MCAST;
+
+    case 0xfe:
+        return (wc & ~FWW_DL_DST) | FWW_ETH_MCAST;
+
+    case 0xff:
+        return wc & ~(FWW_DL_DST | FWW_ETH_MCAST);
+
+    default:
+        NOT_REACHED();
+    }
+}
+
 /* Hashes 'flow' based on its L2 through L4 protocol information. */
 uint32_t
 flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis)

diff --git a/lib/flow.h b/lib/flow.h
index a83987b..2e617b0 100644 (file)
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -167,4 +167,9 @@ bool flow_wildcards_equal(const struct flow_wildcards *,
                           const struct flow_wildcards *);
 uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis);
 
+const uint8_t *flow_wildcards_to_dl_dst_mask(flow_wildcards_t);
+bool flow_wildcards_is_dl_dst_mask_valid(const uint8_t[6]);
+flow_wildcards_t flow_wildcards_set_dl_dst_mask(flow_wildcards_t,
+                                                const uint8_t mask[6]);
+
 #endif /* flow.h */

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 4a41136..f66c655 100644 (file)
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -78,16 +78,6 @@ static struct nxm_field nxm_fields[N_NXM_FIELDS] = {
 /* Hash table of 'nxm_fields'. */
 static struct hmap all_nxm_fields = HMAP_INITIALIZER(&all_nxm_fields);
 
-/* Possible masks for NXM_OF_ETH_DST_W. */
-static const uint8_t eth_all_0s[ETH_ADDR_LEN]
-    = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
-static const uint8_t eth_all_1s[ETH_ADDR_LEN]
-    = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
-static const uint8_t eth_mcast_1[ETH_ADDR_LEN]
-    = {0x01, 0x00, 0x00, 0x00, 0x00, 0x00};
-static const uint8_t eth_mcast_0[ETH_ADDR_LEN]
-    = {0xfe, 0xff, 0xff, 0xff, 0xff, 0xff};
-
 static void
 nxm_init(void)
 {
@@ -192,20 +182,8 @@ parse_nxm_entry(struct cls_rule *rule, const struct nxm_field *f,
         if ((wc->wildcards & (FWW_DL_DST | FWW_ETH_MCAST))
             != (FWW_DL_DST | FWW_ETH_MCAST)) {
             return NXM_DUP_TYPE;
-        } else if (eth_addr_equals(mask, eth_mcast_1)) {
-            wc->wildcards &= ~FWW_ETH_MCAST;
-            flow->dl_dst[0] = *(uint8_t *) value & 0x01;
-            return 0;
-        } else if (eth_addr_equals(mask, eth_mcast_0)) {
-            wc->wildcards &= ~FWW_DL_DST;
-            memcpy(flow->dl_dst, value, ETH_ADDR_LEN);
-            flow->dl_dst[0] &= 0xfe;
-            return 0;
-        } else if (eth_addr_equals(mask, eth_all_0s)) {
-            return 0;
-        } else if (eth_addr_equals(mask, eth_all_1s)) {
-            wc->wildcards &= ~(FWW_DL_DST | FWW_ETH_MCAST);
-            memcpy(flow->dl_dst, value, ETH_ADDR_LEN);
+        } else if (flow_wildcards_is_dl_dst_mask_valid(mask)) {
+            cls_rule_set_dl_dst_masked(rule, value, mask);
             return 0;
         } else {
             return NXM_BAD_MASK;
@@ -682,15 +660,10 @@ nxm_put_eth_dst(struct ofpbuf *b,
     switch (wc & (FWW_DL_DST | FWW_ETH_MCAST)) {
     case FWW_DL_DST | FWW_ETH_MCAST:
         break;
-    case FWW_DL_DST:
-        nxm_put_header(b, NXM_OF_ETH_DST_W);
-        ofpbuf_put(b, value, ETH_ADDR_LEN);
-        ofpbuf_put(b, eth_mcast_1, ETH_ADDR_LEN);
-        break;
-    case FWW_ETH_MCAST:
+    default:
         nxm_put_header(b, NXM_OF_ETH_DST_W);
         ofpbuf_put(b, value, ETH_ADDR_LEN);
-        ofpbuf_put(b, eth_mcast_0, ETH_ADDR_LEN);
+        ofpbuf_put(b, flow_wildcards_to_dl_dst_mask(wc), ETH_ADDR_LEN);
         break;
     case 0:
         nxm_put_eth(b, NXM_OF_ETH_DST, value);