[ovs-dev] [PATCH v9 2/3] nsh: fix nested mask for OVS_KEY_ATTR_NSH

Yi Yang yi.y.yang at intel.com
Thu Jan 11 05:24:02 UTC 2018


NSH kernel implementation used nested mask for OVS_KEY_ATTR_NSH,
so NSH userspace must adapt to it, OVS hasn't used nested mask for
any key attribute so far, OVS_KEY_ATTR_NSH is the first use case.

Signed-off-by: Yi Yang <yi.y.yang at intel.com>
---
 lib/odp-execute.c |  56 +++++++++--------------
 lib/odp-util.c    | 133 +++++++++++++++++++++++++++++++++++++++++-------------
 lib/odp-util.h    |   3 +-
 3 files changed, 125 insertions(+), 67 deletions(-)

diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index c680364..ebaea31 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -274,20 +274,26 @@ odp_set_nd(struct dp_packet *packet, const struct ovs_key_nd *key,
 /* Set the NSH header. Assumes the NSH header is present and matches the
  * MD format of the key. The slow path must take case of that. */
 static void
-odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key,
-            const struct ovs_key_nsh *mask)
+odp_set_nsh(struct dp_packet *packet, const struct nlattr *a, bool has_mask)
 {
+    struct ovs_key_nsh key, mask;
     struct nsh_hdr *nsh = dp_packet_l3(packet);
     uint8_t mdtype = nsh_md_type(nsh);
     ovs_be32 path_hdr;
 
-    if (!mask) {
-        nsh_set_flags_and_ttl(nsh, key->flags, key->ttl);
-        put_16aligned_be32(&nsh->path_hdr, key->path_hdr);
+    if (has_mask) {
+        odp_nsh_key_from_attr(a, &key, &mask);
+    } else {
+        odp_nsh_key_from_attr(a, &key, NULL);
+    }
+
+    if (!has_mask) {
+        nsh_set_flags_and_ttl(nsh, key.flags, key.ttl);
+        put_16aligned_be32(&nsh->path_hdr, key.path_hdr);
         switch (mdtype) {
             case NSH_M_TYPE1:
                 for (int i = 0; i < 4; i++) {
-                    put_16aligned_be32(&nsh->md1.context[i], key->context[i]);
+                    put_16aligned_be32(&nsh->md1.context[i], key.context[i]);
                 }
                 break;
             case NSH_M_TYPE2:
@@ -299,19 +305,19 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key,
         uint8_t flags = nsh_get_flags(nsh);
         uint8_t ttl = nsh_get_ttl(nsh);
 
-        flags = key->flags | (flags & ~mask->flags);
-        ttl = key->ttl | (ttl & ~mask->ttl);
+        flags = key.flags | (flags & ~mask.flags);
+        ttl = key.ttl | (ttl & ~mask.ttl);
         nsh_set_flags_and_ttl(nsh, flags, ttl);
 
         uint32_t spi = ntohl(nsh_get_spi(nsh));
         uint8_t si = nsh_get_si(nsh);
-        uint32_t spi_mask = nsh_path_hdr_to_spi_uint32(mask->path_hdr);
-        uint8_t si_mask = nsh_path_hdr_to_si(mask->path_hdr);
+        uint32_t spi_mask = nsh_path_hdr_to_spi_uint32(mask.path_hdr);
+        uint8_t si_mask = nsh_path_hdr_to_si(mask.path_hdr);
         if (spi_mask == 0x00ffffff) {
             spi_mask = UINT32_MAX;
         }
-        spi = nsh_path_hdr_to_spi_uint32(key->path_hdr) | (spi & ~spi_mask);
-        si = nsh_path_hdr_to_si(key->path_hdr) | (si & ~si_mask);
+        spi = nsh_path_hdr_to_spi_uint32(key.path_hdr) | (spi & ~spi_mask);
+        si = nsh_path_hdr_to_si(key.path_hdr) | (si & ~si_mask);
         path_hdr = nsh_get_path_hdr(nsh);
         nsh_path_hdr_set_spi(&path_hdr, htonl(spi));
         nsh_path_hdr_set_si(&path_hdr, si);
@@ -320,8 +326,8 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key,
             case NSH_M_TYPE1:
                 for (int i = 0; i < 4; i++) {
                     ovs_be32 p = get_16aligned_be32(&nsh->md1.context[i]);
-                    ovs_be32 k = key->context[i];
-                    ovs_be32 m = mask->context[i];
+                    ovs_be32 k = key.context[i];
+                    ovs_be32 m = mask.context[i];
                     put_16aligned_be32(&nsh->md1.context[i], k | (p & ~m));
                 }
                 break;
@@ -359,9 +365,7 @@ odp_execute_set_action(struct dp_packet *packet, const struct nlattr *a)
         break;
 
     case OVS_KEY_ATTR_NSH: {
-        struct ovs_key_nsh nsh;
-        odp_nsh_key_from_attr(a, &nsh);
-        odp_set_nsh(packet, &nsh, NULL);
+        odp_set_nsh(packet, a, false);
         break;
     }
 
@@ -490,23 +494,7 @@ odp_execute_masked_set_action(struct dp_packet *packet,
         break;
 
     case OVS_KEY_ATTR_NSH: {
-        struct ovs_key_nsh nsh, nsh_mask;
-        struct {
-            struct nlattr nla;
-            uint8_t data[sizeof(struct ovs_nsh_key_base) + NSH_CTX_HDRS_MAX_LEN
-                         + 2 * NLA_HDRLEN];
-        } attr, mask;
-        size_t size = nl_attr_get_size(a) / 2;
-
-        mask.nla.nla_type = attr.nla.nla_type = nl_attr_type(a);
-        mask.nla.nla_len = attr.nla.nla_len = NLA_HDRLEN + size;
-        memcpy(attr.data, (char *)(a + 1), size);
-        memcpy(mask.data, (char *)(a + 1) + size, size);
-
-        odp_nsh_key_from_attr(&attr.nla, &nsh);
-        odp_nsh_key_from_attr(&mask.nla, &nsh_mask);
-        odp_set_nsh(packet, &nsh, &nsh_mask);
-
+        odp_set_nsh(packet, a, true);
         break;
     }
 
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 69c34ca..e059674 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -938,6 +938,68 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr)
     }
 }
 
+static const struct attr_len_tbl
+ovs_nsh_key_attr_lens[OVS_NSH_KEY_ATTR_MAX + 1] = {
+    [OVS_NSH_KEY_ATTR_BASE]     = { .len = 8 },
+    [OVS_NSH_KEY_ATTR_MD1]      = { .len = 16 },
+    [OVS_NSH_KEY_ATTR_MD2]      = { .len = ATTR_LEN_VARIABLE },
+};
+
+static void
+format_odp_set_nsh(struct ds *ds, const struct nlattr *attr)
+{
+    unsigned int left;
+    const struct nlattr *a;
+    struct ovs_key_nsh nsh;
+    struct ovs_key_nsh nsh_mask;
+
+    memset(&nsh, 0, sizeof nsh);
+    memset(&nsh_mask, 0xff, sizeof nsh_mask);
+
+    NL_NESTED_FOR_EACH (a, left, attr) {
+        enum ovs_nsh_key_attr type = nl_attr_type(a);
+        size_t len = nl_attr_get_size(a);
+
+        if (type >= OVS_NSH_KEY_ATTR_MAX) {
+            return;
+        }
+
+        int expected_len = ovs_nsh_key_attr_lens[type].len;
+        if ((expected_len != ATTR_LEN_VARIABLE) && (len != 2 * expected_len)) {
+            return;
+        }
+
+        switch (type) {
+        case OVS_NSH_KEY_ATTR_UNSPEC:
+            break;
+        case OVS_NSH_KEY_ATTR_BASE: {
+            const struct ovs_nsh_key_base *base = nl_attr_get(a);
+            const struct ovs_nsh_key_base *base_mask = base + 1;
+            memcpy(&nsh, base, sizeof(*base));
+            memcpy(&nsh_mask, base_mask, sizeof(*base_mask));
+            break;
+        }
+        case OVS_NSH_KEY_ATTR_MD1: {
+            const struct ovs_nsh_key_md1 *md1 = nl_attr_get(a);
+            const struct ovs_nsh_key_md1 *md1_mask = md1 + 1;
+            memcpy(&nsh.context, &md1->context, sizeof(*md1));
+            memcpy(&nsh_mask.context, &md1_mask->context, sizeof(*md1_mask));
+            break;
+        }
+        case OVS_NSH_KEY_ATTR_MD2:
+        case __OVS_NSH_KEY_ATTR_MAX:
+        default:
+            /* No support for matching other metadata formats yet. */
+            break;
+        }
+    }
+
+    ds_put_cstr(ds, "set(nsh(");
+    format_nsh_key_mask(ds, &nsh, &nsh_mask);
+    ds_put_cstr(ds, "))");
+}
+
+
 static void
 format_odp_action(struct ds *ds, const struct nlattr *a,
                   const struct hmap *portno_names)
@@ -988,6 +1050,11 @@ format_odp_action(struct ds *ds, const struct nlattr *a,
         break;
     case OVS_ACTION_ATTR_SET_MASKED:
         a = nl_attr_get(a);
+        /* OVS_KEY_ATTR_NSH is nested attribute, so it needs special process */
+        if (nl_attr_type(a) == OVS_KEY_ATTR_NSH) {
+            format_odp_set_nsh(ds, a);
+            break;
+        }
         size = nl_attr_get_size(a) / 2;
         ds_put_cstr(ds, "set(");
 
@@ -2258,13 +2325,6 @@ static const struct attr_len_tbl ovs_tun_key_attr_lens[OVS_TUNNEL_KEY_ATTR_MAX +
     [OVS_TUNNEL_KEY_ATTR_IPV6_DST]      = { .len = 16 },
 };
 
-static const struct attr_len_tbl
-ovs_nsh_key_attr_lens[OVS_NSH_KEY_ATTR_MAX + 1] = {
-    [OVS_NSH_KEY_ATTR_BASE]     = { .len = 8 },
-    [OVS_NSH_KEY_ATTR_MD1]      = { .len = 16 },
-    [OVS_NSH_KEY_ATTR_MD2]      = { .len = ATTR_LEN_VARIABLE },
-};
-
 const struct attr_len_tbl ovs_flow_key_attr_lens[OVS_KEY_ATTR_MAX + 1] = {
     [OVS_KEY_ATTR_ENCAP]     = { .len = ATTR_LEN_NESTED },
     [OVS_KEY_ATTR_PRIORITY]  = { .len = 4 },
@@ -2429,7 +2489,8 @@ odp_nsh_hdr_from_attr(const struct nlattr *attr,
 }
 
 enum odp_key_fitness
-odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh)
+odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
+                      struct ovs_key_nsh *nsh_mask)
 {
     unsigned int left;
     const struct nlattr *a;
@@ -2442,11 +2503,21 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh)
         int expected_len = odp_key_attr_len(ovs_nsh_key_attr_lens,
                                             OVS_NSH_KEY_ATTR_MAX, type);
 
-        if (len != expected_len && expected_len >= 0) {
+        /* the attribute can have mask, len is 2 * expected_len for that case.
+         */
+        if ((len != expected_len) && (len != 2 * expected_len) &&
+            (expected_len >= 0)) {
+            return ODP_FIT_ERROR;
+        }
+
+        if ((nsh_mask && (expected_len >= 0) && (len != 2 * expected_len)) ||
+            (!nsh_mask && (expected_len >= 0) && (len == 2 * expected_len))) {
             return ODP_FIT_ERROR;
         }
 
         switch (type) {
+        case OVS_NSH_KEY_ATTR_UNSPEC:
+            break;
         case OVS_NSH_KEY_ATTR_BASE: {
             const struct ovs_nsh_key_base *base = nl_attr_get(a);
             nsh->flags = base->flags;
@@ -2454,12 +2525,25 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh)
             nsh->mdtype = base->mdtype;
             nsh->np = base->np;
             nsh->path_hdr = base->path_hdr;
+            if (nsh_mask && (len == 2 * sizeof(*base))) {
+                const struct ovs_nsh_key_base *base_mask = base + 1;
+                nsh_mask->flags = base_mask->flags;
+                nsh_mask->ttl = base_mask->ttl;
+                nsh_mask->mdtype = base_mask->mdtype;
+                nsh_mask->np = base_mask->np;
+                nsh_mask->path_hdr = base_mask->path_hdr;
+            }
             break;
         }
         case OVS_NSH_KEY_ATTR_MD1: {
             const struct ovs_nsh_key_md1 *md1 = nl_attr_get(a);
             has_md1 = true;
             memcpy(nsh->context, md1->context, sizeof md1->context);
+            if (len == 2 * sizeof(*md1)) {
+                const struct ovs_nsh_key_md1 *md1_mask = md1 + 1;
+                memcpy(nsh_mask->context, md1_mask->context,
+                       sizeof(*md1_mask));
+            }
             break;
         }
         case OVS_NSH_KEY_ATTR_MD2:
@@ -5953,7 +6037,7 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
             expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_NSH;
         }
         if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_NSH)) {
-            odp_nsh_key_from_attr(attrs[OVS_KEY_ATTR_NSH], &flow->nsh);
+            odp_nsh_key_from_attr(attrs[OVS_KEY_ATTR_NSH], &flow->nsh, NULL);
             if (is_mask) {
                 check_start = nl_attr_get(attrs[OVS_KEY_ATTR_NSH]);
                 check_len = nl_attr_get_size(attrs[OVS_KEY_ATTR_NSH]);
@@ -7064,24 +7148,28 @@ commit_nsh(const struct ovs_key_nsh * flow_nsh, bool use_masked_set,
         /* OVS_KEY_ATTR_NSH keys */
         nsh_key_ofs = nl_msg_start_nested(odp_actions, OVS_KEY_ATTR_NSH);
 
+        /* put value and mask for OVS_NSH_KEY_ATTR_BASE */
         char *data = nl_msg_put_unspec_uninit(odp_actions,
                                               OVS_NSH_KEY_ATTR_BASE,
-                                              sizeof(nsh_base));
+                                              2 * sizeof(nsh_base));
         const char *lkey = (char *)&nsh_base, *lmask = (char *)&nsh_base_mask;
         size_t lkey_size = sizeof(nsh_base);
 
         while (lkey_size--) {
             *data++ = *lkey++ & *lmask++;
         }
+        lmask = (char *)&nsh_base_mask;
+        memcpy(data, lmask, sizeof(nsh_base_mask));
 
         switch (key->mdtype) {
         case NSH_M_TYPE1:
             memcpy(md1.context, key->context, sizeof key->context);
             memcpy(md1_mask.context, mask->context, sizeof mask->context);
 
+            /* put value and mask for OVS_NSH_KEY_ATTR_MD1 */
             data = nl_msg_put_unspec_uninit(odp_actions,
                                             OVS_NSH_KEY_ATTR_MD1,
-                                            sizeof(md1));
+                                            2 * sizeof(md1));
             lkey = (char *)&md1;
             lmask = (char *)&md1_mask;
             lkey_size = sizeof(md1);
@@ -7089,26 +7177,6 @@ commit_nsh(const struct ovs_key_nsh * flow_nsh, bool use_masked_set,
             while (lkey_size--) {
                 *data++ = *lkey++ & *lmask++;
             }
-            break;
-        case NSH_M_TYPE2:
-        default:
-            /* No match support for other MD formats yet. */
-            break;
-        }
-
-        /* OVS_KEY_ATTR_NSH masks */
-        data = nl_msg_put_unspec_uninit(odp_actions,
-                                        OVS_NSH_KEY_ATTR_BASE,
-                                        sizeof(nsh_base_mask));
-        lmask = (char *)&nsh_base_mask;
-
-        memcpy(data, lmask, sizeof(nsh_base_mask));
-
-        switch (key->mdtype) {
-        case NSH_M_TYPE1:
-            data = nl_msg_put_unspec_uninit(odp_actions,
-                                            OVS_NSH_KEY_ATTR_MD1,
-                                            sizeof(md1_mask));
             lmask = (char *)&md1_mask;
             memcpy(data, lmask, sizeof(md1_mask));
             break;
@@ -7117,6 +7185,7 @@ commit_nsh(const struct ovs_key_nsh * flow_nsh, bool use_masked_set,
             /* No match support for other MD formats yet. */
             break;
         }
+
         nl_msg_end_nested(odp_actions, nsh_key_ofs);
 
         nl_msg_end_nested(odp_actions, offset);
diff --git a/lib/odp-util.h b/lib/odp-util.h
index fafea62..d99727b 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -159,9 +159,10 @@ struct odputil_keybuf {
 enum odp_key_fitness odp_tun_key_from_attr(const struct nlattr *,
                                            struct flow_tnl *);
 enum odp_key_fitness odp_nsh_key_from_attr(const struct nlattr *,
+                                           struct ovs_key_nsh *,
                                            struct ovs_key_nsh *);
 enum odp_key_fitness odp_nsh_hdr_from_attr(const struct nlattr *,
-                                           struct nsh_hdr *, size_t size);
+                                           struct nsh_hdr *, size_t);
 
 int odp_ufid_from_string(const char *s_, ovs_u128 *ufid);
 void odp_format_ufid(const ovs_u128 *ufid, struct ds *);
-- 
2.1.0



More information about the dev mailing list