From patchwork Sat Aug 5 05:50:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Yang, Yi" X-Patchwork-Id: 798153 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xPXx94X3Bz9rxl for ; Sat, 5 Aug 2017 15:51:57 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id EC619A49; Sat, 5 Aug 2017 05:51:55 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 07E74481 for ; Sat, 5 Aug 2017 05:51:54 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 66148201 for ; Sat, 5 Aug 2017 05:50:40 +0000 (UTC) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Aug 2017 22:50:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,324,1498546800"; d="scan'208";a="136531362" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga006.fm.intel.com with ESMTP; 04 Aug 2017 22:50:15 -0700 Received: from fmsmsx154.amr.corp.intel.com (10.18.116.70) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 4 Aug 2017 22:50:15 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX154.amr.corp.intel.com (10.18.116.70) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 4 Aug 2017 22:50:14 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.116]) by shsmsx102.ccr.corp.intel.com ([169.254.2.183]) with mapi id 14.03.0319.002; Sat, 5 Aug 2017 13:50:12 +0800 From: "Yang, Yi Y" To: Ben Pfaff Thread-Topic: [PATCH v3 2/6] userspace: enable set_field support for nsh fields Thread-Index: AQHTC/Zy1xOSm4mL4ECF5L3tsGmiK6J0GhIAgAEq9XA= Date: Sat, 5 Aug 2017 05:50:12 +0000 Message-ID: <79BBBFE6CB6C9B488C1A45ACD284F51961C20EB1@SHSMSX103.ccr.corp.intel.com> References: <1501722900-7158-1-git-send-email-yi.y.yang@intel.com> <1501722900-7158-3-git-send-email-yi.y.yang@intel.com> <20170804195834.GS6175@ovn.org> In-Reply-To: <20170804195834.GS6175@ovn.org> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: "dev@openvswitch.org" Subject: Re: [ovs-dev] [PATCH v3 2/6] userspace: enable set_field support for nsh fields X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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:blp@ovn.org] Sent: Saturday, August 5, 2017 3:59 AM To: Yang, Yi Y Cc: dev@openvswitch.org; Jan Scheurich 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 > > Signed-off-by: Yi Yang > Signed-off-by: Jan Scheurich 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,#.*,, 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 types into C99 uint_t types, # and most Linux-specific __be into Open vSwitch ovs_be, 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" (aka "nshc"). * - * 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 experimenter 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 ONFOXM_ET_* code points defined by official - Open Networking Foundation extensions to OpenFlow 1.3 in e.g. [TCP Flags - Match Field Extension]. + as shown below.

@@ -717,6 +713,26 @@ tcp,tp_src=0x07c0/0xfff0

+ 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: +

+ +
+
0x4f4e4600 (ONFOXM_ET)
+
+ Used by official Open Networking Foundation extensions to OpenFlow 1.3 in + e.g. [TCP Flags Match Field Extension]. +
+ +
0x005ad650 (NXOXM_NSH)
+
+ Used by Open vSwitch for NSH extensions, in the absence of an official + ONF-assigned class. (This OUI is randomly generated.) +
+
+ +

Taken as a unit, class (or vendor), field, and experimenter (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; imd1.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; } }