[ovs-dev] [PATCH v2 15/26] meta-flow: Add mf_mask_field_masked().

Jarno Rajahalme jarno at ovn.org
Fri Jul 29 00:56:07 UTC 2016


Having a masked version allows generating better wildcarding.

Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
---
 include/openvswitch/meta-flow.h |  4 +++-
 include/openvswitch/ofp-parse.h |  4 ++--
 lib/dpctl.c                     |  4 ++--
 lib/meta-flow.c                 | 37 +++++++++++++++++++++++++++----------
 lib/ofp-parse.c                 | 31 ++++++++++++++++---------------
 ofproto/ofproto-dpif-xlate.c    |  2 +-
 tests/test-odp.c                |  2 +-
 7 files changed, 52 insertions(+), 32 deletions(-)

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 533a4e6..900cd6a 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -2090,7 +2090,9 @@ void mf_set_flow_value_masked(const struct mf_field *,
                               struct flow *);
 bool mf_is_tun_metadata(const struct mf_field *);
 bool mf_is_set(const struct mf_field *, const struct flow *);
-void mf_mask_field(const struct mf_field *, struct flow *);
+void mf_mask_field(const struct mf_field *, struct flow_wildcards *);
+void mf_mask_field_masked(const struct mf_field *, const union mf_value *mask,
+                          struct flow_wildcards *);
 int mf_field_len(const struct mf_field *, const union mf_value *value,
                  const union mf_value *mask, bool *is_masked);
 
diff --git a/include/openvswitch/ofp-parse.h b/include/openvswitch/ofp-parse.h
index 1ab5095..a460d8a 100644
--- a/include/openvswitch/ofp-parse.h
+++ b/include/openvswitch/ofp-parse.h
@@ -61,8 +61,8 @@ char *parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *,
                                        enum ofputil_protocol *usable_protocols)
     OVS_WARN_UNUSED_RESULT;
 
-char *parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
-                           const struct simap *portno_names);
+char *parse_ofp_exact_flow(struct flow *flow, struct flow_wildcards *wc,
+                           const char *s, const struct simap *portno_names);
 
 char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, const char *string,
                               int command,
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 76b701c..b470ab0 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -802,8 +802,8 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
     }
 
     if (filter) {
-        char *err = parse_ofp_exact_flow(&flow_filter, &wc_filter.masks,
-                                         filter, &names_portno);
+        char *err = parse_ofp_exact_flow(&flow_filter, &wc_filter, filter,
+                                         &names_portno);
         if (err) {
             dpctl_error(dpctl_p, 0, "Failed to parse filter (%s)", err);
             error = EINVAL;
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 503da0c..28079d7 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -1122,20 +1122,37 @@ mf_set_value(const struct mf_field *mf,
     }
 }
 
-/* Unwildcard 'mask' member field described by 'mf'.  The caller is
- * responsible for ensuring that 'mask' meets 'mf''s prerequisites. */
+/* Unwildcard the bits in 'mask' of the 'wc' member field described by 'mf'.
+ * The caller is responsible for ensuring that 'wc' meets 'mf''s
+ * prerequisites. */
 void
-mf_mask_field(const struct mf_field *mf, struct flow *mask)
+mf_mask_field_masked(const struct mf_field *mf, const union mf_value *mask,
+                     struct flow_wildcards *wc)
 {
-    /* For MFF_DL_VLAN, we cannot send a all 1's to flow_set_dl_vlan()
-     * as that will be considered as OFP10_VLAN_NONE. So consider it as a
-     * special case. For the rest, calling mf_set_flow_value() is good
-     * enough. */
+    union mf_value temp_mask;
+    /* For MFF_DL_VLAN, we cannot send a all 1's to flow_set_dl_vlan() as that
+     * will be considered as OFP10_VLAN_NONE. So make sure the mask only has
+     * valid bits in this case. */
     if (mf->id == MFF_DL_VLAN) {
-        flow_set_dl_vlan(mask, htons(VLAN_VID_MASK));
-    } else {
-        mf_set_flow_value(mf, &exact_match_mask, mask);
+        temp_mask.be16 = htons(VLAN_VID_MASK) & mask->be16;
+        mask = &temp_mask;
+    }
+
+    union mf_value mask_value;
+
+    mf_get_value(mf, &wc->masks, &mask_value);
+    for (size_t i = 0; i < mf->n_bytes; i++) {
+        mask_value.b[i] |= mask->b[i];
     }
+    mf_set_flow_value(mf, &mask_value, &wc->masks);
+}
+
+/* Unwildcard 'wc' member field described by 'mf'.  The caller is
+ * responsible for ensuring that 'mask' meets 'mf''s prerequisites. */
+void
+mf_mask_field(const struct mf_field *mf, struct flow_wildcards *wc)
+{
+    mf_mask_field_masked(mf, &exact_match_mask, wc);
 }
 
 static int
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 370e3e5..73072d6 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1115,23 +1115,23 @@ parse_ofp_flow_stats_request_str(struct ofputil_flow_stats_request *fsr,
 /* Parses a specification of a flow from 's' into 'flow'.  's' must take the
  * form FIELD=VALUE[,FIELD=VALUE]... where each FIELD is the name of a
  * mf_field.  Fields must be specified in a natural order for satisfying
- * prerequisites. If 'mask' is specified, fills the mask field for each of the
+ * prerequisites. If 'wc' is specified, masks the field in 'wc' for each of the
  * field specified in flow. If the map, 'names_portno' is specfied, converts
  * the in_port name into port no while setting the 'flow'.
  *
  * Returns NULL on success, otherwise a malloc()'d string that explains the
  * problem. */
 char *
-parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
-                     const struct simap *portno_names)
+parse_ofp_exact_flow(struct flow *flow, struct flow_wildcards *wc,
+                     const char *s, const struct simap *portno_names)
 {
     char *pos, *key, *value_s;
     char *error = NULL;
     char *copy;
 
     memset(flow, 0, sizeof *flow);
-    if (mask) {
-        memset(mask, 0, sizeof *mask);
+    if (wc) {
+        memset(wc, 0, sizeof *wc);
     }
 
     pos = copy = xstrdup(s);
@@ -1143,8 +1143,8 @@ parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
                 goto exit;
             }
             flow->dl_type = htons(p->dl_type);
-            if (mask) {
-                mask->dl_type = OVS_BE16_MAX;
+            if (wc) {
+                wc->masks.dl_type = OVS_BE16_MAX;
             }
 
             if (p->nw_proto) {
@@ -1154,8 +1154,8 @@ parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
                     goto exit;
                 }
                 flow->nw_proto = p->nw_proto;
-                if (mask) {
-                    mask->nw_proto = UINT8_MAX;
+                if (wc) {
+                    wc->masks.nw_proto = UINT8_MAX;
                 }
             }
         } else {
@@ -1185,8 +1185,9 @@ parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
                 && simap_contains(portno_names, value_s)) {
                 flow->in_port.ofp_port = u16_to_ofp(
                     simap_get(portno_names, value_s));
-                if (mask) {
-                    mask->in_port.ofp_port = u16_to_ofp(ntohs(OVS_BE16_MAX));
+                if (wc) {
+                    wc->masks.in_port.ofp_port
+                        = u16_to_ofp(ntohs(OVS_BE16_MAX));
                 }
             } else {
                 field_error = mf_parse_value(mf, value_s, &value);
@@ -1198,8 +1199,8 @@ parse_ofp_exact_flow(struct flow *flow, struct flow *mask, const char *s,
                 }
 
                 mf_set_flow_value(mf, &value, flow);
-                if (mask) {
-                    mf_mask_field(mf, mask);
+                if (wc) {
+                    mf_mask_field(mf, wc);
                 }
             }
         }
@@ -1214,8 +1215,8 @@ exit:
 
     if (error) {
         memset(flow, 0, sizeof *flow);
-        if (mask) {
-            memset(mask, 0, sizeof *mask);
+        if (wc) {
+            memset(wc, 0, sizeof *wc);
         }
     }
     return error;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a847557..c32d9bc 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3545,7 +3545,7 @@ xlate_hash_fields_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
                 basis = hash_boolean(mf_is_set(mf, &ctx->xin->flow), basis);
             }
 
-            mf_mask_field(mf, &ctx->wc->masks);
+            mf_mask_field(mf, ctx->wc);
         }
     }
 
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 8e4db09..dabfb85 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -163,7 +163,7 @@ parse_filter(char *filter_parse)
         memset(&flow_filter, 0, sizeof(flow_filter));
         memset(&wc_filter, 0, sizeof(wc_filter));
 
-        error = parse_ofp_exact_flow(&flow_filter, &wc_filter.masks, filter,
+        error = parse_ofp_exact_flow(&flow_filter, &wc_filter, filter,
                                      NULL);
         if (error) {
             ovs_fatal(0, "Failed to parse filter (%s)", error);
-- 
2.1.4




More information about the dev mailing list