Ben, thank you so much for your comments and incremental patch, I have fixed your comments and merged your incremental patch and folded patch 2 to patch 1, new version v4 has been posted.
https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/336864.html -----Original Message----- From: Ben Pfaff [mailto:[email protected]] Sent: Saturday, August 5, 2017 3:59 AM To: Yang, Yi Y <[email protected]> Cc: [email protected]; Jan Scheurich <[email protected]> Subject: Re: [PATCH v3 2/6] userspace: enable set_field support for nsh fields On Thu, Aug 03, 2017 at 09:14:56AM +0800, Yi Yang wrote: > From: Jan Scheurich <[email protected]> > > Signed-off-by: Yi Yang <[email protected]> > Signed-off-by: Jan Scheurich <[email protected]> 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; } } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
