flow: Fully separate FWW_* from OFPFW10_*.
authorBen Pfaff <blp@nicira.com>
Mon, 18 Jun 2012 16:55:22 +0000 (09:55 -0700)
committerBen Pfaff <blp@nicira.com>
Tue, 4 Sep 2012 18:19:14 +0000 (11:19 -0700)
It might have been a useful optimization at one point to have FWW_*
correspond in OFPFW10_* where possible, but it doesn't seem worthwhile for
only 3 corresponding values.  It also makes the code somewhat more
confusing.

Signed-off-by: Ben Pfaff <blp@nicira.com>
lib/flow.h
lib/ofp-util.c

index 568e291..dac65e5 100644 (file)
@@ -141,14 +141,12 @@ flow_hash(const struct flow *flow, uint32_t basis)
 
 typedef unsigned int OVS_BITWISE flow_wildcards_t;
 
-/* Same values and meanings as corresponding OFPFW10_* bits. */
 #define FWW_IN_PORT     ((OVS_FORCE flow_wildcards_t) (1 << 0))
-#define FWW_DL_TYPE     ((OVS_FORCE flow_wildcards_t) (1 << 4))
-#define FWW_NW_PROTO    ((OVS_FORCE flow_wildcards_t) (1 << 5))
-/* No corresponding OFPFW10_* bits. */
-#define FWW_NW_DSCP     ((OVS_FORCE flow_wildcards_t) (1 << 1))
-#define FWW_NW_ECN      ((OVS_FORCE flow_wildcards_t) (1 << 2))
-#define FWW_NW_TTL      ((OVS_FORCE flow_wildcards_t) (1 << 3))
+#define FWW_DL_TYPE     ((OVS_FORCE flow_wildcards_t) (1 << 1))
+#define FWW_NW_PROTO    ((OVS_FORCE flow_wildcards_t) (1 << 2))
+#define FWW_NW_DSCP     ((OVS_FORCE flow_wildcards_t) (1 << 3))
+#define FWW_NW_ECN      ((OVS_FORCE flow_wildcards_t) (1 << 4))
+#define FWW_NW_TTL      ((OVS_FORCE flow_wildcards_t) (1 << 5))
 #define FWW_ALL         ((OVS_FORCE flow_wildcards_t) (((1 << 6)) - 1))
 
 /* Remember to update FLOW_WC_SEQ when adding or removing FWW_*. */
index ce9bb74..4cacadf 100644 (file)
@@ -77,27 +77,6 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
     return 32 - ip_count_cidr_bits(netmask);
 }
 
-/* A list of the FWW_* and OFPFW10_ bits that have the same value, meaning, and
- * name. */
-#define WC_INVARIANT_LIST \
-    WC_INVARIANT_BIT(IN_PORT) \
-    WC_INVARIANT_BIT(DL_TYPE) \
-    WC_INVARIANT_BIT(NW_PROTO)
-
-/* Verify that all of the invariant bits (as defined on WC_INVARIANT_LIST)
- * actually have the same names and values. */
-#define WC_INVARIANT_BIT(NAME) BUILD_ASSERT_DECL(FWW_##NAME == OFPFW10_##NAME);
-    WC_INVARIANT_LIST
-#undef WC_INVARIANT_BIT
-
-/* WC_INVARIANTS is the invariant bits (as defined on WC_INVARIANT_LIST) all
- * OR'd together. */
-static const flow_wildcards_t WC_INVARIANTS = 0
-#define WC_INVARIANT_BIT(NAME) | FWW_##NAME
-    WC_INVARIANT_LIST
-#undef WC_INVARIANT_BIT
-;
-
 /* Converts the OpenFlow 1.0 wildcards in 'ofpfw' (OFPFW10_*) into a
  * flow_wildcards in 'wc' for use in struct cls_rule.  It is the caller's
  * responsibility to handle the special case where the flow match's dl_vlan is
@@ -109,14 +88,20 @@ ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
 
     /* Initialize most of rule->wc. */
     flow_wildcards_init_catchall(wc);
-    wc->wildcards = (OVS_FORCE flow_wildcards_t) ofpfw & WC_INVARIANTS;
 
-    /* Wildcard fields that aren't defined by ofp10_match. */
-    wc->wildcards |= FWW_NW_ECN | FWW_NW_TTL;
+    /* Start with wildcard fields that aren't defined by ofp10_match. */
+    wc->wildcards = FWW_NW_ECN | FWW_NW_TTL;
 
+    if (ofpfw & OFPFW10_IN_PORT) {
+        wc->wildcards |= FWW_IN_PORT;
+    }
+    if (ofpfw & OFPFW10_DL_TYPE) {
+        wc->wildcards |= FWW_DL_TYPE;
+    }
+    if (ofpfw & OFPFW10_NW_PROTO) {
+        wc->wildcards |= FWW_NW_PROTO;
+    }
     if (ofpfw & OFPFW10_NW_TOS) {
-        /* OpenFlow 1.0 defines a TOS wildcard, but it's much later in
-         * the enum than we can use. */
         wc->wildcards |= FWW_NW_DSCP;
     }
 
@@ -205,7 +190,16 @@ ofputil_cls_rule_to_ofp10_match(const struct cls_rule *rule,
     uint32_t ofpfw;
 
     /* Figure out most OpenFlow wildcards. */
-    ofpfw = (OVS_FORCE uint32_t) (wc->wildcards & WC_INVARIANTS);
+    ofpfw = 0;
+    if (wc->wildcards & FWW_IN_PORT) {
+        ofpfw |= OFPFW10_IN_PORT;
+    }
+    if (wc->wildcards & FWW_DL_TYPE) {
+        ofpfw |= OFPFW10_DL_TYPE;
+    }
+    if (wc->wildcards & FWW_NW_PROTO) {
+        ofpfw |= OFPFW10_NW_PROTO;
+    }
     ofpfw |= (ofputil_netmask_to_wcbits(wc->nw_src_mask)
               << OFPFW10_NW_SRC_SHIFT);
     ofpfw |= (ofputil_netmask_to_wcbits(wc->nw_dst_mask)