[ovs-dev] [PATCH 5/9] odp-util: Return exact mask if netlink mask attribute is missing.

Daniele Di Proietto diproiettod at vmware.com
Thu Dec 10 02:27:38 UTC 2015


In the ODP context an empty mask netlink attribute usually means that
the flow should be an exact match.

odp_flow_key_to_mask{,_udpif}() instead return a struct flow_wildcards
with matches only on recirc_id and vlan_tci.

A more appropriate behavior is to handle a missing (zero length) netlink
mask specially (like we do in userspace and Linux datapath) and create
an exact match flow_wildcards from the original flow.

This fixes a bug in revalidate_ukey(): every flow created with
megaflows disabled would be revalidated away, because the mask would
seem too generic. (Another possible fix would be to handle the special
case of a missing mask in revalidate_ukey(), but this seems a more
generic solution).

Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
---
 lib/dpctl.c                   |  2 +-
 lib/dpif-netdev.c             | 46 ++++++++++++++++++++-----------------------
 lib/odp-util.c                | 35 ++++++++++++++++++++++++++------
 lib/odp-util.h                |  4 ++--
 ofproto/ofproto-dpif-upcall.c |  2 +-
 tests/test-odp.c              |  2 +-
 6 files changed, 55 insertions(+), 36 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index 438bfd3..26de23f 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -797,7 +797,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
 
             odp_flow_key_to_flow(f.key, f.key_len, &flow);
             odp_flow_key_to_mask(f.mask, f.mask_len, f.key, f.key_len,
-                                 &wc.masks, &flow);
+                                 &wc, &flow);
             match_init(&match, &flow, &wc);
 
             match_init(&match_filter, &flow_filter, &wc);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 3bf130d..8794ca0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1858,33 +1858,29 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
                               uint32_t mask_key_len, const struct flow *flow,
                               struct flow_wildcards *wc)
 {
-    if (mask_key_len) {
-        enum odp_key_fitness fitness;
-
-        fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
-                                             key_len, &wc->masks, flow);
-        if (fitness) {
-            /* This should not happen: it indicates that
-             * odp_flow_key_from_mask() and odp_flow_key_to_mask()
-             * disagree on the acceptable form of a mask.  Log the problem
-             * as an error, with enough details to enable debugging. */
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-            if (!VLOG_DROP_ERR(&rl)) {
-                struct ds s;
-
-                ds_init(&s);
-                odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
-                                true);
-                VLOG_ERR("internal error parsing flow mask %s (%s)",
-                         ds_cstr(&s), odp_key_fitness_to_string(fitness));
-                ds_destroy(&s);
-            }
+    enum odp_key_fitness fitness;
+
+    fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
+                                         key_len, wc, flow);
+    if (fitness) {
+        /* This should not happen: it indicates that
+         * odp_flow_key_from_mask() and odp_flow_key_to_mask()
+         * disagree on the acceptable form of a mask.  Log the problem
+         * as an error, with enough details to enable debugging. */
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+        if (!VLOG_DROP_ERR(&rl)) {
+            struct ds s;
 
-            return EINVAL;
+            ds_init(&s);
+            odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
+                            true);
+            VLOG_ERR("internal error parsing flow mask %s (%s)",
+                     ds_cstr(&s), odp_key_fitness_to_string(fitness));
+            ds_destroy(&s);
         }
-    } else {
-        flow_wildcards_init_for_packet(wc, flow);
+
+        return EINVAL;
     }
 
     return 0;
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4db371d..3717aee 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -5152,6 +5152,26 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
    return odp_flow_key_to_flow__(key, key_len, NULL, 0, flow, flow, false);
 }
 
+static enum odp_key_fitness
+odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len,
+                       const struct nlattr *flow_key, size_t flow_key_len,
+                       struct flow_wildcards *mask,
+                       const struct flow *src_flow,
+                       bool udpif)
+{
+    if (mask_key_len) {
+        return odp_flow_key_to_flow__(mask_key, mask_key_len,
+                                      flow_key, flow_key_len,
+                                      &mask->masks, src_flow, udpif);
+
+    } else {
+        /* A missing mask means that the flow should be exact matched.
+         * Generate an appropriate exact wildcard for the flow. */
+        flow_wildcards_init_for_packet(mask, src_flow);
+
+        return ODP_FIT_PERFECT;
+    }
+}
 /* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key'
  * to a mask structure in 'mask'.  'flow' must be a previously translated flow
  * corresponding to 'mask' and similarly flow_key/flow_key_len must be the
@@ -5160,10 +5180,11 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
 enum odp_key_fitness
 odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len,
                      const struct nlattr *flow_key, size_t flow_key_len,
-                     struct flow *mask, const struct flow *flow)
+                     struct flow_wildcards *mask, const struct flow *flow)
 {
-   return odp_flow_key_to_flow__(mask_key, mask_key_len, flow_key, flow_key_len,
-                                 mask, flow, false);
+    return odp_flow_key_to_mask__(mask_key, mask_key_len,
+                                  flow_key, flow_key_len,
+                                  mask, flow, false);
 }
 
 /* These functions are similar to their non-"_udpif" variants but output a
@@ -5185,10 +5206,12 @@ odp_flow_key_to_flow_udpif(const struct nlattr *key, size_t key_len,
 enum odp_key_fitness
 odp_flow_key_to_mask_udpif(const struct nlattr *mask_key, size_t mask_key_len,
                            const struct nlattr *flow_key, size_t flow_key_len,
-                           struct flow *mask, const struct flow *flow)
+                           struct flow_wildcards *mask,
+                           const struct flow *flow)
 {
-   return odp_flow_key_to_flow__(mask_key, mask_key_len, flow_key, flow_key_len,
-                                 mask, flow, true);
+    return odp_flow_key_to_mask__(mask_key, mask_key_len,
+                                  flow_key, flow_key_len,
+                                  mask, flow, true);
 }
 
 /* Returns 'fitness' as a string, for use in debug messages. */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 35849ae..51cf5c3 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -240,7 +240,7 @@ enum odp_key_fitness odp_flow_key_to_mask(const struct nlattr *mask_key,
                                           size_t mask_key_len,
                                           const struct nlattr *flow_key,
                                           size_t flow_key_len,
-                                          struct flow *mask,
+                                          struct flow_wildcards *mask,
                                           const struct flow *flow);
 
 enum odp_key_fitness odp_flow_key_to_flow_udpif(const struct nlattr *, size_t,
@@ -249,7 +249,7 @@ enum odp_key_fitness odp_flow_key_to_mask_udpif(const struct nlattr *mask_key,
                                                 size_t mask_key_len,
                                                 const struct nlattr *flow_key,
                                                 size_t flow_key_len,
-                                                struct flow *mask,
+                                                struct flow_wildcards *mask,
                                                 const struct flow *flow);
 
 const char *odp_key_fitness_to_string(enum odp_key_fitness);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index c191927..cd8a9f0 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1878,7 +1878,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
     }
 
     if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, ukey->key,
-                             ukey->key_len, &dp_mask.masks, &flow)
+                             ukey->key_len, &dp_mask, &flow)
         == ODP_FIT_ERROR) {
         goto exit;
     }
diff --git a/tests/test-odp.c b/tests/test-odp.c
index cdc761f..55ee97e 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -197,7 +197,7 @@ parse_filter(char *filter_parse)
 
             odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
             odp_flow_key_to_mask(odp_mask.data, odp_mask.size, odp_key.data,
-                                 odp_key.size, &wc.masks, &flow);
+                                 odp_key.size, &wc, &flow);
             match_init(&match, &flow, &wc);
 
             match_init(&match_filter, &flow_filter, &wc);
-- 
2.1.4




More information about the dev mailing list