[ovs-dev] [PATCH v3 2/6] userspace: enable set_field support for nsh fields

Ben Pfaff blp at ovn.org
Fri Aug 4 19:58:34 UTC 2017


On Thu, Aug 03, 2017 at 09:14:56AM +0800, Yi Yang wrote:
> From: Jan Scheurich <jan.scheurich at ericsson.com>
> 
> Signed-off-by: Yi Yang <yi.y.yang at intel.com>
> Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>

Thanks for working on this.

I think that this should be folded into patch 1, since it fixes what is
essentially a bug in patch 1.

I noticed that struct nsh_hdr (introduced in the previous patch) isn't
suitable for 16-bit alignment, since it has 32-bit members.  Our
practice is to use appropriate type to ensure safety for 16-bit
alignment, like this:

/**
 * struct nsh_md1_ctx - Keeps track of NSH context data
 * @nshc<1-4>: NSH Contexts.
 */
struct nsh_md1_ctx {
    ovs_16aligned_be32 c[4];
};

struct nsh_md2_tlv {
    ovs_be16 md_class;
    uint8_t type;
    uint8_t length;
    uint8_t md_value[];
};

struct nsh_hdr {
    ovs_be16 ver_flags_len;
    uint8_t md_type;
    uint8_t next_proto;
    ovs_16aligned_be32 path_hdr;
    union {
        struct nsh_md1_ctx md1;
        struct nsh_md2_tlv md2[0];
    };
};

This makes it harder to forget to use the proper accessors to read
misaligned data, since the types more or less ensure it.

I noticed that it's easier to just use arrays of 4 elements (c[4]) than
separate members (c1, c2, c3, c4).

I'm appending an incremental that can be applied on top of patches 1 and
2 to achieve many of the suggestions I've made.  It includes the
incremental for patch 1.  I haven't tested anything, beyond compiling.

--8<--------------------------cut here-------------------------->8--

diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
index a509adb88f79..bc1cc35a7e9b 100755
--- a/build-aux/extract-odp-netlink-h
+++ b/build-aux/extract-odp-netlink-h
@@ -45,7 +45,11 @@ s,#.*<linux/if_ether\.h>,,
 s/__u8[[:space:]]*\([a-zA-Z0-9_]*\)[[:space:]]*\[[[:space:]]*ETH_ALEN[[:space:]]*\]/struct eth_addr \1/
 
 # Transform IPv6 addresses from an array to struct in6_addr
-s/__be32[[:space:]]*\([a-zA-Z0-9_]*\)[[:space:]]*\[[[:space:]]*4[[:space:]]*\]/struct in6_addr \1/
+#
+# As a very special case, only transform member names with more than
+# one character because struct ovs_key_nsh has a member "__be32 c[4];"
+# that is not an IPv6 address.
+s/__be32[[:space:]]*\([a-zA-Z0-9_]\{2,\}\)[[:space:]]*\[[[:space:]]*4[[:space:]]*\]/struct in6_addr \1/
 
 # Transform most Linux-specific __u<N> types into C99 uint<N>_t types,
 # and most Linux-specific __be<N> into Open vSwitch ovs_be<N>,
diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 7b6151f3384b..184b75e36bef 100755
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -67,9 +67,9 @@ PREREQS = {"none": "MFP_NONE",
 # If a name matches more than one prefix, the longest one is used.
 OXM_CLASSES = {"NXM_OF_":        (0,          0x0000),
                "NXM_NX_":        (0,          0x0001),
+               "NXOXM_NSH_":     (0x005ad650, 0xffff),
                "OXM_OF_":        (0,          0x8000),
                "OXM_OF_PKT_REG": (0,          0x8001),
-               "OXM_NSH_":       (0,          0x8004),
                "ONFOXM_ET_":     (0x4f4e4600, 0xffff),
 
                # This is the experimenter OXM class for Nicira, which is the
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 2381ed2c8c3c..5806aba93f73 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -360,9 +360,6 @@ enum ovs_key_attr {
 	OVS_KEY_ATTR_CT_LABELS,	/* 16-octet connection tracking labels */
 	OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4,   /* struct ovs_key_ct_tuple_ipv4 */
 	OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6,   /* struct ovs_key_ct_tuple_ipv6 */
-#ifndef __KERNEL__
-	OVS_KEY_ATTR_NSH,       /* struct ovs_key_nsh */
-#endif
 
 #ifdef __KERNEL__
 	/* Only used within kernel data path. */
@@ -372,6 +369,7 @@ enum ovs_key_attr {
 #ifndef __KERNEL__
 	/* Only used within userspace data path. */
 	OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
+	OVS_KEY_ATTR_NSH,	   /* struct ovs_key_nsh */
 #endif
 
 	__OVS_KEY_ATTR_MAX
@@ -500,10 +498,7 @@ struct ovs_key_nsh {
     __u8 np;
     __u8 pad;
     __be32 path_hdr;
-    __be32 c1;
-    __be32 c2;
-    __be32 c3;
-    __be32 c4;
+    __be32 c[4];
 };
 
 /* OVS_KEY_ATTR_CT_STATE flags */
diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index 6192898f76e3..1c4b56b3c051 100644
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -1745,7 +1745,7 @@ enum OVS_PACKED_ENUM mf_field_id {
 
     /* "nsh_flags".
      *
-     * flags field in NSH base header (8 bits).
+     * flags field in NSH base header.
      *
      * Type: u8.
      * Maskable: bitwise.
@@ -1753,13 +1753,13 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Prerequisites: NSH.
      * Access: read/write.
      * NXM: none.
-     * OXM: OXM_NSH_FLAGS(1) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_FLAGS(1) since OF1.3 and v2.8.
      */
     MFF_NSH_FLAGS,
 
     /* "nsh_mdtype".
      *
-     * mdtype field in NSH base header (8 bits).
+     * mdtype field in NSH base header.
      *
      * Type: u8.
      * Maskable: no.
@@ -1767,13 +1767,13 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Prerequisites: NSH.
      * Access: read-only.
      * NXM: none.
-     * OXM: OXM_NSH_MDTYPE(2) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_MDTYPE(2) since OF1.3 and v2.8.
      */
     MFF_NSH_MDTYPE,
 
     /* "nsh_np".
      *
-     * np (next protocol) field in NSH base header (8 bits)
+     * np (next protocol) field in NSH base header
      *
      * Type: u8.
      * Maskable: no.
@@ -1781,28 +1781,27 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Prerequisites: NSH.
      * Access: read-only.
      * NXM: none.
-     * OXM: OXM_NSH_NP(3) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_NP(3) since OF1.3 and v2.8.
      */
     MFF_NSH_NP,
 
     /* "nsh_spi" (aka "nsp").
      *
-     * spi (service path identifier) field in NSH base
-     * header (24 bits).
+     * spi (service path identifier) field in NSH base header.
      *
-     * Type: be32.
+     * Type: be32 (low 24 bits).
      * Maskable: no.
      * Formatting: hexadecimal.
      * Prerequisites: NSH.
      * Access: read/write.
      * NXM: none.
-     * OXM: OXM_NSH_SPI(4) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_SPI(4) since OF1.3 and v2.8.
      */
     MFF_NSH_SPI,
 
     /* "nsh_si" (aka "nsi").
      *
-     * si (service index) field in NSH base header (8 bits).
+     * si (service index) field in NSH base header.
      *
      * Type: u8.
      * Maskable: no.
@@ -1810,13 +1809,13 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Prerequisites: NSH.
      * Access: read/write.
      * NXM: none.
-     * OXM: OXM_NSH_SI(5) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_SI(5) since OF1.3 and v2.8.
      */
     MFF_NSH_SI,
 
-    /* "nsh_c1" (aka "nshc1").
+    /* "nsh_c<N>" (aka "nshc<N>").
      *
-     * c1 (Network Platform Context) field in NSH context header (32 bits)
+     * Network Platform Context field in NSH context header.
      *
      * Type: be32.
      * Maskable: bitwise.
@@ -1824,50 +1823,14 @@ enum OVS_PACKED_ENUM mf_field_id {
      * Prerequisites: NSH.
      * Access: read/write.
      * NXM: none.
-     * OXM: OXM_NSH_C1(6) since OF1.3 and v2.8.
+     * OXM: NXOXM_NSH_C1(6) since OF1.3 and v2.8 <1>.
+     * OXM: NXOXM_NSH_C2(7) since OF1.3 and v2.8 <2>.
+     * OXM: NXOXM_NSH_C3(8) since OF1.3 and v2.8 <3>.
+     * OXM: NXOXM_NSH_C4(9) since OF1.3 and v2.8 <4>.
      */
     MFF_NSH_C1,
-
-    /* "nsh_c2" (aka "nshc2").
-     *
-     * c2 (Network Shared Context) field in NSH context header (32 bits)
-     *
-     * Type: be32.
-     * Maskable: bitwise.
-     * Formatting: hexadecimal.
-     * Prerequisites: NSH.
-     * Access: read/write.
-     * NXM: none.
-     * OXM: OXM_NSH_C2(7) since OF1.3 and v2.8.
-     */
     MFF_NSH_C2,
-
-    /* "nsh_c3" (aka "nshc3").
-     *
-     * c3 (Service Platform Context) field in NSH context header (32 bits)
-     *
-     * Type: be32.
-     * Maskable: bitwise.
-     * Formatting: hexadecimal.
-     * Prerequisites: NSH.
-     * Access: read/write.
-     * NXM: none.
-     * OXM: OXM_NSH_C3(8) since OF1.3 and v2.8.
-     */
     MFF_NSH_C3,
-
-    /* "nsh_c4" (aka "nshc4").
-     *
-     * c4 (Service Shared Context) field in NSH context header (32 bits)
-     *
-     * Type: be32.
-     * Maskable: bitwise.
-     * Formatting: hexadecimal.
-     * Prerequisites: NSH.
-     * Access: read/write.
-     * NXM: none.
-     * OXM: OXM_NSH_C4(9) since OF1.3 and v2.8.
-     */
     MFF_NSH_C4,
 
     MFF_N_IDS
diff --git a/include/openvswitch/nsh.h b/include/openvswitch/nsh.h
index 1593da2f14d3..119070942856 100644
--- a/include/openvswitch/nsh.h
+++ b/include/openvswitch/nsh.h
@@ -51,10 +51,7 @@ extern "C" {
  * @nshc<1-4>: NSH Contexts.
  */
 struct nsh_md1_ctx {
-    ovs_be32 c1;
-    ovs_be32 c2;
-    ovs_be32 c3;
-    ovs_be32 c4;
+    ovs_16aligned_be32 c[4];
 };
 
 struct nsh_md2_tlv {
@@ -68,7 +65,7 @@ struct nsh_hdr {
     ovs_be16 ver_flags_len;
     uint8_t md_type;
     uint8_t next_proto;
-    ovs_be32 path_hdr;
+    ovs_16aligned_be32 path_hdr;
     union {
         struct nsh_md1_ctx md1;
         struct nsh_md2_tlv md2[0];
diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h
index 3f40ce55a094..be91e02daa60 100644
--- a/include/openvswitch/packets.h
+++ b/include/openvswitch/packets.h
@@ -84,10 +84,7 @@ struct flow_nsh {
     uint8_t np;
     uint8_t si;
     ovs_be32 spi;
-    ovs_be32 c1;
-    ovs_be32 c2;
-    ovs_be32 c3;
-    ovs_be32 c4;
+    ovs_be32 c[4];
 };
 
 /* NSH flags */
diff --git a/lib/flow.c b/lib/flow.c
index 333007303da5..04711636ce6f 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -552,7 +552,7 @@ parse_nsh(const void **datap, size_t *sizep, struct flow_nsh *key)
     key->mdtype = nsh->md_type;
     key->np = nsh->next_proto;
 
-    path_hdr = ntohl(nsh->path_hdr);
+    path_hdr = ntohl(get_16aligned_be32(&nsh->path_hdr));
     key->si = (path_hdr & NSH_SI_MASK) >> NSH_SI_SHIFT;
     key->spi = htonl((path_hdr & NSH_SPI_MASK) >> NSH_SPI_SHIFT);
 
@@ -561,10 +561,9 @@ parse_nsh(const void **datap, size_t *sizep, struct flow_nsh *key)
             if (length != 6) {
                 return -EINVAL;
             }
-            key->c1 = nsh->md1.c1;
-            key->c2 = nsh->md1.c2;
-            key->c3 = nsh->md1.c3;
-            key->c4 = nsh->md1.c4;
+            for (size_t i = 0; i < 4; i++) {
+                key->c[i] = get_16aligned_be32(&nsh->md1.c[i]);
+            }
             break;
         case NSH_M_TYPE2:
             /* Don't support MD type 2 yet, so return false */
@@ -1684,10 +1683,7 @@ flow_wildcards_init_for_packet(struct flow_wildcards *wc,
         WC_MASK_FIELD(wc, nsh.np);
         WC_MASK_FIELD(wc, nsh.spi);
         WC_MASK_FIELD(wc, nsh.si);
-        WC_MASK_FIELD(wc, nsh.c1);
-        WC_MASK_FIELD(wc, nsh.c2);
-        WC_MASK_FIELD(wc, nsh.c3);
-        WC_MASK_FIELD(wc, nsh.c4);
+        WC_MASK_FIELD(wc, nsh.c);
     } else {
         return; /* Unknown ethertype. */
     }
@@ -1821,10 +1817,7 @@ flow_wc_map(const struct flow *flow, struct flowmap *map)
         FLOWMAP_SET(map, nsh.np);
         FLOWMAP_SET(map, nsh.spi);
         FLOWMAP_SET(map, nsh.si);
-        FLOWMAP_SET(map, nsh.c1);
-        FLOWMAP_SET(map, nsh.c2);
-        FLOWMAP_SET(map, nsh.c3);
-        FLOWMAP_SET(map, nsh.c4);
+        FLOWMAP_SET(map, nsh.c);
     }
 }
 
diff --git a/lib/match.c b/lib/match.c
index f287df1be5d2..0ff4b90870e9 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1253,34 +1253,15 @@ format_ct_label_masked(struct ds *s, const ovs_u128 *key, const ovs_u128 *mask)
 static void
 format_nsh_masked(struct ds *s, const struct flow *f, const struct flow *m)
 {
-    if (m->nsh.flags) {
-        format_uint8_masked(s, "nsh_flags", f->nsh.flags, m->nsh.flags);
-    }
-    if (m->nsh.mdtype) {
-        format_uint8_masked(s, "nsh_mdtype", f->nsh.mdtype, m->nsh.mdtype);
-    }
-    if (m->nsh.np) {
-        format_uint8_masked(s, "nsh_np", f->nsh.np, m->nsh.np);
-    }
-    if (m->nsh.spi) {
-        format_be32_masked(s, "nsh_spi", f->nsh.spi, m->nsh.spi);
-    }
-    if (m->nsh.si) {
-        format_uint8_masked(s, "nsh_si", f->nsh.si, m->nsh.si);
-    }
-
-    if (m->nsh.c1) {
-        format_be32_masked_hex(s, "nsh_c1", f->nsh.c1, m->nsh.c1);
-    }
-    if (m->nsh.c2) {
-        format_be32_masked_hex(s, "nsh_c2", f->nsh.c2, m->nsh.c2);
-    }
-    if (m->nsh.c3) {
-        format_be32_masked_hex(s, "nsh_c3", f->nsh.c3, m->nsh.c3);
-    }
-    if (m->nsh.c4) {
-        format_be32_masked_hex(s, "nsh_c4", f->nsh.c4, m->nsh.c4);
-    }
+    format_uint8_masked(s, "nsh_flags", f->nsh.flags, m->nsh.flags);
+    format_uint8_masked(s, "nsh_mdtype", f->nsh.mdtype, m->nsh.mdtype);
+    format_uint8_masked(s, "nsh_np", f->nsh.np, m->nsh.np);
+    format_be32_masked(s, "nsh_spi", f->nsh.spi, m->nsh.spi);
+    format_uint8_masked(s, "nsh_si", f->nsh.si, m->nsh.si);
+    format_be32_masked_hex(s, "nsh_c1", f->nsh.c[0], m->nsh.c[0]);
+    format_be32_masked_hex(s, "nsh_c2", f->nsh.c[1], m->nsh.c[1]);
+    format_be32_masked_hex(s, "nsh_c3", f->nsh.c[2], m->nsh.c[2]);
+    format_be32_masked_hex(s, "nsh_c4", f->nsh.c[3], m->nsh.c[3]);
 }
 
 /* Appends a string representation of 'match' to 's'.  If 'priority' is
diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index 53c8ed0979e1..70f199e4106b 100644
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -370,13 +370,10 @@ mf_is_all_wild(const struct mf_field *mf, const struct flow_wildcards *wc)
     case MFF_NSH_SI:
         return !wc->masks.nsh.si;
     case MFF_NSH_C1:
-        return !wc->masks.nsh.c1;
     case MFF_NSH_C2:
-        return !wc->masks.nsh.c2;
     case MFF_NSH_C3:
-        return !wc->masks.nsh.c3;
     case MFF_NSH_C4:
-        return !wc->masks.nsh.c4;
+        return !wc->masks.nsh.c[mf->id - MFF_NSH_C1];
 
     case MFF_N_IDS:
     default:
@@ -911,16 +908,10 @@ mf_get_value(const struct mf_field *mf, const struct flow *flow,
         value->u8 = flow->nsh.si;
         break;
     case MFF_NSH_C1:
-        value->be32 = flow->nsh.c1;
-        break;
     case MFF_NSH_C2:
-        value->be32 = flow->nsh.c2;
-        break;
     case MFF_NSH_C3:
-        value->be32 = flow->nsh.c3;
-        break;
     case MFF_NSH_C4:
-        value->be32 = flow->nsh.c4;
+        value->be32 = flow->nsh.c[mf->id - MFF_NSH_C1];
         break;
 
     case MFF_N_IDS:
@@ -1232,16 +1223,10 @@ mf_set_value(const struct mf_field *mf,
         MATCH_SET_FIELD_UINT8(match, nsh.si, value->u8);
         break;
     case MFF_NSH_C1:
-        MATCH_SET_FIELD_BE32(match, nsh.c1, value->be32);
-        break;
     case MFF_NSH_C2:
-        MATCH_SET_FIELD_BE32(match, nsh.c2, value->be32);
-        break;
     case MFF_NSH_C3:
-        MATCH_SET_FIELD_BE32(match, nsh.c3, value->be32);
-        break;
     case MFF_NSH_C4:
-        MATCH_SET_FIELD_BE32(match, nsh.c4, value->be32);
+        MATCH_SET_FIELD_BE32(match, nsh.c[mf->id - MFF_NSH_C1], value->be32);
         break;
 
     case MFF_N_IDS:
@@ -1629,16 +1614,10 @@ mf_set_flow_value(const struct mf_field *mf,
         flow->nsh.si = value->u8;
         break;
     case MFF_NSH_C1:
-        flow->nsh.c1 = value->be32;
-        break;
     case MFF_NSH_C2:
-        flow->nsh.c2 = value->be32;
-        break;
     case MFF_NSH_C3:
-        flow->nsh.c3 = value->be32;
-        break;
     case MFF_NSH_C4:
-        flow->nsh.c4 = value->be32;
+        flow->nsh.c[mf->id - MFF_NSH_C1] = value->be32;
         break;
 
     case MFF_N_IDS:
@@ -2126,16 +2105,11 @@ mf_set_wild(const struct mf_field *mf, struct match *match, char **err_str)
         MATCH_SET_FIELD_MASKED(match, nsh.si, 0, 0);
         break;
     case MFF_NSH_C1:
-        MATCH_SET_FIELD_MASKED(match, nsh.c1, htonl(0), htonl(0));
-        break;
     case MFF_NSH_C2:
-        MATCH_SET_FIELD_MASKED(match, nsh.c2, htonl(0), htonl(0));
-        break;
     case MFF_NSH_C3:
-        MATCH_SET_FIELD_MASKED(match, nsh.c3, htonl(0), htonl(0));
-        break;
     case MFF_NSH_C4:
-        MATCH_SET_FIELD_MASKED(match, nsh.c4, htonl(0), htonl(0));
+        MATCH_SET_FIELD_MASKED(match, nsh.c[mf->id - MFF_NSH_C1],
+                               htonl(0), htonl(0));
         break;
 
     case MFF_N_IDS:
@@ -2391,16 +2365,11 @@ mf_set(const struct mf_field *mf,
         MATCH_SET_FIELD_MASKED(match, nsh.si, value->u8, mask->u8);
         break;
     case MFF_NSH_C1:
-        MATCH_SET_FIELD_MASKED(match, nsh.c1, value->be32, mask->be32);
-        break;
     case MFF_NSH_C2:
-        MATCH_SET_FIELD_MASKED(match, nsh.c2, value->be32, mask->be32);
-        break;
     case MFF_NSH_C3:
-        MATCH_SET_FIELD_MASKED(match, nsh.c3, value->be32, mask->be32);
-        break;
     case MFF_NSH_C4:
-        MATCH_SET_FIELD_MASKED(match, nsh.c4, value->be32, mask->be32);
+        MATCH_SET_FIELD_MASKED(match, nsh.c[mf->id - MFF_NSH_C1],
+                               value->be32, mask->be32);
         break;
 
     case MFF_N_IDS:
diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
index b634c517242b..83fe421f0c40 100644
--- a/lib/meta-flow.xml
+++ b/lib/meta-flow.xml
@@ -689,11 +689,7 @@ tcp,tp_src=0x07c0/0xfff0
     using the first 32 bits of the body as an <code>experimenter</code> field
     whose most significant byte is zero and whose remaining bytes are an
     Organizationally Unique Identifier (OUI) assigned by the IEEE [IEEE OUI],
-    as shown below.  OpenFlow says that support for experimenter fields is
-    optional.  Open vSwitch 2.4 and later does support them, primarily so that
-    it can support the <code>ONFOXM_ET_</code>* code points defined by official
-    Open Networking Foundation extensions to OpenFlow 1.3 in e.g. [TCP Flags
-    Match Field Extension].
+    as shown below.
   </p>
 
   <diagram>
@@ -717,6 +713,26 @@ tcp,tp_src=0x07c0/0xfff0
   </diagram>
 
   <p>
+    OpenFlow says that support for experimenter fields is optional.  Open
+    vSwitch 2.4 and later does support them, so that it can support the
+    following experimenter classes:
+  </p>
+
+  <dl>
+    <dt>0x4f4e4600 (<code>ONFOXM_ET</code>)</dt>
+    <dd>
+      Used by official Open Networking Foundation extensions to OpenFlow 1.3 in
+      e.g. [TCP Flags Match Field Extension].
+    </dd>
+
+    <dt>0x005ad650 (<code>NXOXM_NSH</code>)</dt>
+    <dd>
+      Used by Open vSwitch for NSH extensions, in the absence of an official
+      ONF-assigned class.  (This OUI is randomly generated.)
+    </dd>
+  </dl>
+
+  <p>
     Taken as a unit, <code>class</code> (or <code>vendor</code>),
     <code>field</code>, and <code>experimenter</code> (when present) uniquely
     identify a particular field.
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 8326f2ee6b3f..b782e8c5905f 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -1164,14 +1164,10 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,
     nxm_put_32m(&ctx, MFF_NSH_SPI, oxm, flow->nsh.spi,
                 match->wc.masks.nsh.spi);
     nxm_put_8m(&ctx, MFF_NSH_SI, oxm, flow->nsh.si, match->wc.masks.nsh.si);
-    nxm_put_32m(&ctx, MFF_NSH_C1, oxm, flow->nsh.c1,
-            match->wc.masks.nsh.c1);
-    nxm_put_32m(&ctx, MFF_NSH_C2, oxm, flow->nsh.c2,
-            match->wc.masks.nsh.c2);
-    nxm_put_32m(&ctx, MFF_NSH_C3, oxm, flow->nsh.c3,
-            match->wc.masks.nsh.c3);
-    nxm_put_32m(&ctx, MFF_NSH_C4, oxm, flow->nsh.c4,
-            match->wc.masks.nsh.c4);
+    for (int i = 0; i < 4; i++) {
+        nxm_put_32m(&ctx, MFF_NSH_C1 + i, oxm, flow->nsh.c[i],
+                    match->wc.masks.nsh.c[i]);
+    }
 
     /* Registers. */
     if (oxm < OFP15_VERSION) {
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 7c5d45921ea0..e631c6836797 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -276,24 +276,17 @@ static void
 odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key,
             const struct ovs_key_nsh *mask)
 {
-    struct nsh_hdr * nsh = (struct nsh_hdr *) dp_packet_l3(packet);
-    uint16_t *p, *k, *m;
-    size_t len;
-    ovs_be32 path_hdr;
-    uint8_t flags;
-    int i;
-
-    ovs_assert(nsh != NULL);
+    struct nsh_hdr *nsh = dp_packet_l3(packet);
 
     if (!mask) {
         nsh->ver_flags_len = htons(key->flags << NSH_FLAGS_SHIFT) |
                              (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK));
-        put_16aligned_be32((ovs_16aligned_be32 *) &nsh->path_hdr,
-                           key->path_hdr);
+        put_16aligned_be32(&nsh->path_hdr, key->path_hdr);
         switch (nsh->md_type) {
             case NSH_M_TYPE1:
-                /* Avoid the 16/32 bit alignment hassle. */
-                memcpy(&nsh->md1, &key->c1, sizeof(struct nsh_md1_ctx));
+                for (int i = 0; i < 4; i++) {
+                    put_16aligned_be32(&nsh->md1.c[i], key->c[i]);
+                }
                 break;
             case NSH_M_TYPE2:
                 /* TODO */
@@ -302,24 +295,22 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key,
                 OVS_NOT_REACHED();
         }
     } else {
-        flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >>
-                    NSH_FLAGS_SHIFT;
+        uint8_t flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >>
+                            NSH_FLAGS_SHIFT;
         flags = key->flags | (flags & ~mask->flags);
         nsh->ver_flags_len = htons(flags << NSH_FLAGS_SHIFT) |
                              (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK));
-        path_hdr = get_16aligned_be32((ovs_16aligned_be32 *) &nsh->path_hdr);
+
+        ovs_be32 path_hdr = get_16aligned_be32(&nsh->path_hdr);
         path_hdr = key->path_hdr | (path_hdr & ~mask->path_hdr);
-        put_16aligned_be32((ovs_16aligned_be32 *) &nsh->path_hdr,
-                           path_hdr);
+        put_16aligned_be32(&nsh->path_hdr, path_hdr);
         switch (nsh->md_type) {
             case NSH_M_TYPE1:
-                len = sizeof(struct nsh_md1_ctx) >> 1;
-                /* Avoid the 16/32 bit alignment hassle. */
-                p = (uint16_t *) &nsh->md1.c1;
-                k = (uint16_t *) &key->c1;
-                m = (uint16_t *) &mask->c1;
-                for (i=0; i<len; i++, p++, k++, m++) {
-                    *p = *k | (*p & ~*m);
+                for (int i = 0; i < 4; i++) {
+                    ovs_be32 p = get_16aligned_be32(&nsh->md1.c[i]);
+                    ovs_be32 k = key->c[i];
+                    ovs_be32 m = mask->c[i];
+                    put_16aligned_be32(&nsh->md1.c[i], k | (p & ~m));
                 }
                 break;
             case NSH_M_TYPE2:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 26c57453fdd3..909ba9980cf8 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -261,10 +261,9 @@ format_nsh_key(struct ds *ds, const struct ovs_key_nsh *key)
 
     switch (key->mdtype) {
         case NSH_M_TYPE1:
-            ds_put_format(ds, ",c1=0x%08x", ntohl(key->c1));
-            ds_put_format(ds, ",c2=0x%08x", ntohl(key->c2));
-            ds_put_format(ds, ",c3=0x%08x", ntohl(key->c3));
-            ds_put_format(ds, ",c4=0x%08x", ntohl(key->c4));
+            for (int i = 0; i < 4; i++) {
+                ds_put_format(ds, ",c%d=0x%08x", i + 1, ntohl(key->c[i]));
+            }
             break;
         case NSH_M_TYPE2:
             /* TODO */
@@ -334,10 +333,10 @@ format_nsh_key_mask(struct ds *ds, const struct ovs_key_nsh *key,
         format_uint8_masked(ds, &first, "np", key->np, mask->np);
         format_be32_masked(ds, &first, "spi", htonl(spi), htonl(spi_mask));
         format_uint8_masked(ds, &first, "si", si, si_mask);
-        format_be32_masked(ds, &first, "c1", key->c1, mask->c1);
-        format_be32_masked(ds, &first, "c2", key->c2, mask->c2);
-        format_be32_masked(ds, &first, "c3", key->c3, mask->c3);
-        format_be32_masked(ds, &first, "c4", key->c4, mask->c4);
+        format_be32_masked(ds, &first, "c1", key->c[0], mask->c[0]);
+        format_be32_masked(ds, &first, "c2", key->c[1], mask->c[1]);
+        format_be32_masked(ds, &first, "c3", key->c[2], mask->c[2]);
+        format_be32_masked(ds, &first, "c4", key->c[3], mask->c[3]);
     }
 }
 
@@ -4533,10 +4532,10 @@ parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
         SCAN_FIELD("mdtype=", u8, mdtype);
         SCAN_FIELD("np=", u8, np);
         SCAN_FIELD("path_hdr=", be32, path_hdr);
-        SCAN_FIELD("c1=", be32, c1);
-        SCAN_FIELD("c2=", be32, c2);
-        SCAN_FIELD("c3=", be32, c3);
-        SCAN_FIELD("c4=", be32, c4);
+        SCAN_FIELD("c1=", be32, c[0]);
+        SCAN_FIELD("c2=", be32, c[1]);
+        SCAN_FIELD("c3=", be32, c[2]);
+        SCAN_FIELD("c4=", be32, c[2]);
     } SCAN_END(OVS_KEY_ATTR_NSH);
 
     /* Encap open-coded. */
@@ -6453,17 +6452,15 @@ get_nsh_key(const struct flow *flow, struct ovs_key_nsh *nsh, bool is_mask)
     nsh->path_hdr = htonl((ntohl(flow->nsh.spi) << NSH_SPI_SHIFT) |
                           flow->nsh.si);
     if (is_mask) {
-        nsh->c1 = flow->nsh.c1;
-        nsh->c2 = flow->nsh.c2;
-        nsh->c3 = flow->nsh.c3;
-        nsh->c4 = flow->nsh.c4;
+        for (int i = 0; i < 4; i++) {
+            nsh->c[i] = flow->nsh.c[i];
+        }
     } else {
         switch (nsh->mdtype) {
         case NSH_M_TYPE1:
-            nsh->c1 = flow->nsh.c1;
-            nsh->c2 = flow->nsh.c2;
-            nsh->c3 = flow->nsh.c3;
-            nsh->c4 = flow->nsh.c4;
+            for (int i = 0; i < 4; i++) {
+                nsh->c[i] = flow->nsh.c[i];
+            }
             break;
         case NSH_M_TYPE2:
             /* TODO: MD type 2 */
@@ -6484,17 +6481,13 @@ put_nsh_key(const struct ovs_key_nsh *nsh, struct flow *flow,
     flow->nsh.si = (ntohl(nsh->path_hdr) & NSH_SI_MASK) >> NSH_SI_SHIFT;
     switch (nsh->mdtype) {
         case NSH_M_TYPE1:
-            flow->nsh.c1 = nsh->c1;
-            flow->nsh.c2 = nsh->c2;
-            flow->nsh.c3 = nsh->c3;
-            flow->nsh.c4 = nsh->c4;
+            for (int i = 0; i < 4; i++) {
+                flow->nsh.c[i] = nsh->c[i];
+            }
             break;
         case NSH_M_TYPE2:
             /* TODO: MD type 2 */
-            flow->nsh.c1 = 0;
-            flow->nsh.c2 = 0;
-            flow->nsh.c3 = 0;
-            flow->nsh.c4 = 0;
+            memset(flow->nsh.c, 0, sizeof flow->nsh.c);
             break;
     }
 }


More information about the dev mailing list