On Mon, Mar 21, 2022 at 9:46 AM Ilya Maximets <[email protected]> wrote:
>
> On 3/18/22 22:34, Toms Atteka wrote:
> > IPv6 extension headers carry optional internet layer information
> > and are placed between the fixed header and the upper-layer
> > protocol header.
> >
> > This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and
> > packets can be filtered using ipv6_ext flag.
> >
> > Some spacing style issues fixed.
> >
> > Tested-at: https://github.com/TomCodeLV/ovs/actions/runs/504185214
> > Signed-off-by: Toms Atteka <[email protected]>
> > ---
>
> Hi. Thanks for the patch!
>
> I didn't try to run it, but there are few issues highlighted inline
> that we might need to act on sooner rather than later as they might
> need some kernel uAPI changes. Please, take a look.
> Some less urgent issues also inline.
>
> Best regards, Ilya Maximets.
>
> > build-aux/extract-odp-netlink-macros-h | 1 +
> > build-aux/extract-ofp-fields | 4 +-
> > datapath/flow.c | 192 +++++++++++++++---
> > datapath/flow.h | 14 ++
> > datapath/flow_netlink.c | 25 ++-
>
> Please, don't include code changes for the kernel datapath.
> OOT kernel module is deprecated at this point and will be
> completely removed soon. Only uAPI header and build fixes
> (if any) should be part of that patch.
At that point we would take kernel modules from linux direclty?
>
> > .../linux/compat/include/linux/openvswitch.h | 6 +
> > include/openvswitch/flow.h | 8 +-
> > include/openvswitch/match.h | 2 +
> > include/openvswitch/meta-flow.h | 18 ++
> > lib/dpif-netdev-extract-avx512.c | 2 +-
> > lib/flow.c | 188 ++++++++++++++++-
> > lib/flow.h | 6 +-
> > lib/match.c | 25 ++-
> > lib/meta-flow.c | 80 +++++++-
> > lib/meta-flow.xml | 14 ++
> > lib/nx-match.c | 13 +-
> > lib/odp-execute.c | 2 +
> > lib/odp-util.c | 111 +++++++++-
> > lib/odp-util.h | 11 +-
> > lib/ofp-match.c | 2 +-
> > lib/packets.c | 28 +++
> > lib/packets.h | 12 ++
> > ofproto/ofproto-dpif-rid.h | 2 +-
> > ofproto/ofproto-dpif-sflow.c | 1 +
> > ofproto/ofproto-dpif-xlate.c | 2 +-
> > ofproto/ofproto-dpif.c | 47 +++++
> > tests/ofproto.at | 4 +-
> > tests/system-traffic.at | 31 +++
> > 28 files changed, 789 insertions(+), 62 deletions(-)
> >
>
> <snip>
>
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index 8d9300091..b9e74543b 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -387,6 +387,7 @@ enum ovs_key_attr {
> > 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 */
> > OVS_KEY_ATTR_NSH, /* Nested set of ovs_nsh_key_* */
> > + OVS_KEY_ATTR_IPV6_EXTHDRS, /* struct ovs_key_ipv6_exthdr */
>
> In case you missed for some reason the latest kernel change,
> the uAPI header on net-next was flattened and looks differently
> now. Hence, the value of OVS_KEY_ATTR_IPV6_EXTHDRS is also
> different.
>
> I'll send a patch to partially backport the new header layout.
Which kernel change you are reffering? Could you provide commit id?
>
> >
> > #ifdef __KERNEL__
> > /* Only used within kernel data path. */
> > @@ -498,6 +499,11 @@ struct ovs_key_ipv6 {
> > __u8 ipv6_frag; /* One of OVS_FRAG_TYPE_*. */
> > };
> >
> > +/* separate structure to support backward compatibility with older user
> > space */
> > +struct ovs_key_ipv6_exthdrs {
> > + __u16 hdrs;
> > +};
> > +
> > struct ovs_key_tcp {
> > __be16 tcp_src;
> > __be16 tcp_dst;
> > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> > index 3054015d9..faa0fa412 100644
> > --- a/include/openvswitch/flow.h
> > +++ b/include/openvswitch/flow.h
> > @@ -27,7 +27,7 @@ extern "C" {
> > /* This sequence number should be incremented whenever anything involving
> > flows
> > * or the wildcarding of flows changes. This will cause build assertion
> > * failures in places which likely need to be updated. */
> > -#define FLOW_WC_SEQ 42
> > +#define FLOW_WC_SEQ 43
> >
> > /* Number of Open vSwitch extension 32-bit registers. */
> > #define FLOW_N_REGS 16
> > @@ -136,6 +136,8 @@ struct flow {
> > struct in6_addr ipv6_dst; /* IPv6 destination address. */
> > struct in6_addr ct_ipv6_src; /* CT orig tuple IPv6 source address. */
> > struct in6_addr ct_ipv6_dst; /* CT orig tuple IPv6 destination
> > address. */
> > + uint16_t ipv6_exthdr; /* IPv6 flow extension headers. */
> > + ovs_be16 pad4[3]; /* Pad to 64 bits. */
>
> I'm not sure if that's the most efficient way of packing the
> structure. Though I didn't actually dig into that enough to
> suggest anything better. Will take another look once other
> issues are sorted out.
So far I understand it has to be in some reasonable order, and padding
is mandatory.
>
> > ovs_be32 ipv6_label; /* IPv6 flow label. */
> > uint8_t nw_frag; /* FLOW_FRAG_* flags. */
> > uint8_t nw_tos; /* IP ToS (including DSCP and ECN). */
> > @@ -167,8 +169,8 @@ BUILD_ASSERT_DECL(sizeof(struct ovs_key_nsh) %
> > sizeof(uint64_t) == 0);
> >
> > /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
> > BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t)
> > - == sizeof(struct flow_tnl) + sizeof(struct ovs_key_nsh)
> > + 300
> > - && FLOW_WC_SEQ == 42);
> > + == sizeof(struct flow_tnl) + sizeof(struct ovs_key_nsh)
> > + 308
> > + && FLOW_WC_SEQ == 43);
> >
> > /* Incremental points at which flow classification may be performed in
> > * segments.
> > diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> > index 2e8812048..5ba129190 100644
> > --- a/include/openvswitch/match.h
> > +++ b/include/openvswitch/match.h
> > @@ -234,6 +234,8 @@ void match_set_ipv6_dst_masked(struct match *, const
> > struct in6_addr *,
> > const struct in6_addr *);
> > void match_set_ipv6_label(struct match *, ovs_be32);
> > void match_set_ipv6_label_masked(struct match *, ovs_be32, ovs_be32);
> > +void match_set_ipv6_exthdr(struct match *, uint16_t);
> > +void match_set_ipv6_exthdr_masked(struct match *, uint16_t, uint16_t);
> > void match_set_nd_target(struct match *, const struct in6_addr *);
> > void match_set_nd_target_masked(struct match *, const struct in6_addr *,
> > const struct in6_addr *);
> > diff --git a/include/openvswitch/meta-flow.h
> > b/include/openvswitch/meta-flow.h
> > index 045dce8f5..6f4d63b42 100644
> > --- a/include/openvswitch/meta-flow.h
> > +++ b/include/openvswitch/meta-flow.h
> > @@ -1392,6 +1392,22 @@ enum OVS_PACKED_ENUM mf_field_id {
> > */
> > MFF_IPV6_LABEL,
> >
> > + /* "ipv6_ext".
> > + *
> > + * IPV6 extension headers flags.
> > + *
> > + * Currently there are 9 IPV6 extension headers flags.
> > + *
> > + * Type: u16.
>
> This doesn't look right. OpenFlow spec defines the field as
> "16-bit integer with 7 most-significant bits forced to 0. Only the
> lower 9 bits have meaning". But that can not be achieved with the
> le field. In current implementation most significant bits will be
> set instead in most cases. So, that definition should be changed to
>
> Type: be16 (low 9 bits).
I agree that there is missing bits part. But as it a pseudo field and the
value does not come from network it shouldn't be big endian. As per
convention we use big endian for values coming from network devices.
So
Type: u16 (low 9 bits)
should be the way to go
>
> I also see that this field is defined as u16 in the kernel uAPI.
> That might be fine to have it that way, but we'll need to convert
> between le and be every time we're exchanging messages between
> ovs-vswitch and the datapath. That might be not very convenient
> and prone to error.
If I am not mistaken we don't exchange this value but generate it independently
on userspace and in kernel.
>
> One option here would be to change the kernel uAPI to have be16
> as well. But we have to do that before the next (5.18) kernel
> released. After the 5.18 release, we'll not be able to change the
> uAPI and will have to deal with translations.
>
> > + * Maskable: bitwise.
> > + * Formatting: IPV6 ext hdr.
> > + * Prerequisites: IPv6.
> > + * Access: read-only.
> > + * NXM: NXM_NX_IPV6_EXTHDR(126) since v2.11.
>
> Since v2.18.
>
> > + * OXM: OXM_OF_IPV6_EXTHDR(39) since OF1.2 and v2.11.
>
> There is no OXM_OF_IPV6_EXTHDR in the OpenFlow 1.2 spec, AFAICT.
> So, it should be OF1.3 and v2.18.
Yes, looks like a mistake, will update this.
>
> > + */
> > + MFF_IPV6_EXTHDR,
> > +
> > /* ## ----------------------- ## */
> > /* ## IPv4/IPv6 common fields ## */
> > /* ## ----------------------- ## */
> > @@ -2112,6 +2128,7 @@ enum OVS_PACKED_ENUM mf_string {
> > MFS_TNL_FLAGS, /* FLOW_TNL_F_* flags */
> > MFS_TCP_FLAGS, /* TCP_* flags */
> > MFS_PACKET_TYPE, /* "(NS,NS_TYPE)" */
> > + MFS_IPV6_EXTHDR,
>
> Comment here is needed.
Noted.
>
> > };
> >
> > struct mf_field {
> > @@ -2169,6 +2186,7 @@ union mf_value {
> > ovs_be64 be64;
> > ovs_be32 be32;
> > ovs_be16 be16;
> > + uint16_t u16;
> > uint8_t u8;
> > };
> > BUILD_ASSERT_DECL(sizeof(union mf_value) == 128);
> > diff --git a/lib/dpif-netdev-extract-avx512.c
> > b/lib/dpif-netdev-extract-avx512.c
> > index c1c1fefb6..3cbe04ca9 100644
> > --- a/lib/dpif-netdev-extract-avx512.c
> > +++ b/lib/dpif-netdev-extract-avx512.c
> > @@ -289,7 +289,7 @@ BUILD_ASSERT_DECL((OFFSETOFEND(struct dp_packet, l4_ofs)
> > BUILD_ASSERT_DECL(FLOWMAP_UNITS == 2);
> >
> > /* Ensure the miniflow-struct ABI is the expected version. */
> > -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> This is not enough, since AVX512 implementation heavily depends
> on the layout of the flow/miniflow and it changed.
>
> Not asking you to learn how that part works, just highlighting the
> missing parts. I'll ask Intel folks to assist with the change.
>
Yes, this is bit out of my scope. Thanks for pointing out.
> >
> > /* If the above build assert happens, this means that you might need to
> > make
> > * some modifications to the AVX512 miniflow extractor code. In general,
> > the
> > diff --git a/lib/flow.c b/lib/flow.c
> > index dd523c889..6a8f5d14c 100644
> > --- a/lib/flow.c
> > +++ b/lib/flow.c
> > @@ -30,6 +30,7 @@
> > #include "colors.h"
> > #include "coverage.h"
> > #include "csum.h"
> > +#include "openflow/openflow.h"
> > #include "openvswitch/dynamic-string.h"
> > #include "hash.h"
> > #include "jhash.h"
> > @@ -136,7 +137,7 @@ struct mf_ctx {
> > * away. Some GCC versions gave warnings on ALWAYS_INLINE, so these are
> > * defined as macros. */
> >
> > -#if (FLOW_WC_SEQ != 42)
> > +#if (FLOW_WC_SEQ != 43)
> > #define MINIFLOW_ASSERT(X) ovs_assert(X)
> > BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will have runtime "
> > "assertions enabled. Consider updating FLOW_WC_SEQ after "
> > @@ -572,6 +573,163 @@ parse_ipv6_ext_hdrs(const void **datap, size_t
> > *sizep, uint8_t *nw_proto,
> > frag_hdr);
> > }
> >
> > +uint16_t
> > +ipv6_ext_header_size__(uint8_t header_type, uint8_t len)
> > +{
> > + /*
> > + * Authentication header has different size calculation
> > + */
> > + if (header_type == IPPROTO_AH) {
> > + return (len + 2) << 2;
> > + } else {
> > + return (len + 1) << 3;
> > + }
> > +}
> > +
> > +/**
> > + * Parses packet and sets IPv6 extension header flags.
> > + *
> > + * datap pointer where extension header data starts in packet
> > + * nh ipv6 header
> > + * ext_hdrs flags are stored here
> > + *
> > + * OFPIEH12_UNREP is set to 1 if more than one of a given IPv6 extension
> > header
> > + * is unexpectedly encountered. (Two destination options headers may be
> > + * expected and would not cause this bit to be set.)
> > + *
> > + * OFPIEH12_UNSEQ is set to 1 if IPv6 extension headers were not in the
> > order
> > + * preferred (but not required) by RFC 2460:
> > + *
> > + * When more than one extension header is used in the same packet, it is
> > + * recommended that those headers appear in the following order:
> > + * IPv6 header
> > + * Hop-by-Hop Options header
> > + * Destination Options header
> > + * Routing header
> > + * Fragment header
> > + * Authentication header
> > + * Encapsulating Security Payload header
> > + * Destination Options header
> > + * upper-layer header
> > + */
> > +void
> > +get_ipv6_ext_hdrs(const void *datap, const struct ovs_16aligned_ip6_hdr
> > *nh,
> > + uint16_t *ext_hdrs)
> > +{
> > + uint8_t next_type = nh->ip6_nxt;
> > + size_t size = ntohs(nh->ip6_plen);
> > + int dest_options_header_count = 0;
> > +
> > + *ext_hdrs = 0;
> > +
> > + while (true) {
> > + /*
> > + * following switch code is identical to kernel datapath/flow.c
> > + * get_ipv6_ext_hdrs() switch code
> > + */
> > + switch (next_type) {
> > + case IPPROTO_NONE:
> > + *ext_hdrs |= OFPIEH12_NONEXT;
> > + /* stop parsing */
> > + return;
> > +
> > + case IPPROTO_ESP:
> > + if (*ext_hdrs & OFPIEH12_ESP) {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + if ((*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_DEST
> > + | OFPIEH12_ROUTER | IPPROTO_FRAGMENT
> > + | OFPIEH12_AUTH | OFPIEH12_UNREP))
> > + || dest_options_header_count >= 2) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + *ext_hdrs |= OFPIEH12_ESP;
> > + break;
> > +
> > + case IPPROTO_AH:
> > + if (*ext_hdrs & OFPIEH12_AUTH) {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + if ((*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_DEST
> > + | OFPIEH12_ROUTER | IPPROTO_FRAGMENT
> > + | OFPIEH12_UNREP))
> > + || dest_options_header_count >= 2) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + *ext_hdrs |= OFPIEH12_AUTH;
> > + break;
> > +
> > + case IPPROTO_DSTOPTS:
> > + if (dest_options_header_count == 0) {
> > + if (*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_UNREP)) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + *ext_hdrs |= OFPIEH12_DEST;
> > + } else if (dest_options_header_count == 1) {
> > + if (*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_DEST
> > + | OFPIEH12_ROUTER | OFPIEH12_FRAG
> > + | OFPIEH12_AUTH | OFPIEH12_ESP
> > + | OFPIEH12_UNREP)) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + } else {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + dest_options_header_count ++;
> > + break;
> > +
> > + case IPPROTO_FRAGMENT:
> > + if (*ext_hdrs & OFPIEH12_FRAG) {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + if ((*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_DEST
> > + | OFPIEH12_ROUTER | OFPIEH12_UNREP))
> > + || dest_options_header_count >= 2) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + *ext_hdrs |= OFPIEH12_FRAG;
> > + break;
> > +
> > + case IPPROTO_ROUTING:
> > + if (*ext_hdrs & OFPIEH12_ROUTER) {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + if ((*ext_hdrs & ~(OFPIEH12_HOP | OFPIEH12_DEST
> > + | OFPIEH12_UNREP))
> > + || dest_options_header_count >= 2) {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + *ext_hdrs |= OFPIEH12_ROUTER;
> > + break;
> > +
> > + case IPPROTO_HOPOPTS:
> > + if (*ext_hdrs & OFPIEH12_HOP) {
> > + *ext_hdrs |= OFPIEH12_UNREP;
> > + }
> > + /*
> > + * OFPIEH12_HOP is set to 1 if a hop-by-hop IPv6 extension
> > + * header is present as the first extension header in the
> > + * packet.
> > + */
> > + if (*ext_hdrs == 0) {
> > + *ext_hdrs |= OFPIEH12_HOP;
> > + } else {
> > + *ext_hdrs |= OFPIEH12_UNSEQ;
> > + }
> > + break;
> > +
> > + default:
> > + return;
> > + }
> > +
> > + const struct ip6_ext *ext_hdr = datap;
> > + data_try_pull(&datap, &size,
> > + ipv6_ext_header_size__(next_type,
> > ext_hdr->ip6e_len));
> > +
> > + next_type = ext_hdr->ip6e_nxt;
> > + }
> > +}
> > +
> > bool
> > parse_nsh(const void **datap, size_t *sizep, struct ovs_key_nsh *key)
> > {
> > @@ -761,7 +919,7 @@ void
> > miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
> > {
> > /* Add code to this function (or its callees) to extract new fields. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > const struct pkt_metadata *md = &packet->md;
> > const void *data = dp_packet_data(packet);
> > @@ -945,12 +1103,20 @@ miniflow_extract(struct dp_packet *packet, struct
> > miniflow *dst)
> > nw_ttl = nh->ip6_hlim;
> > nw_proto = nh->ip6_nxt;
> >
> > + uint16_t ext_hdrs;
> > +
> > + get_ipv6_ext_hdrs(data, nh, &ext_hdrs);
> > +
> > const struct ovs_16aligned_ip6_frag *frag_hdr;
> > if (!parse_ipv6_ext_hdrs__(&data, &size, &nw_proto, &nw_frag,
> > &frag_hdr)) {
>
> It looks like we're iterating over the same headers twice.
> Doesn't look very efficient. This can be important for the
> performance of the userspace datapath.
>
> Is there a way to combine get_ipv6_ext_hdrs() and parse_ipv6_ext_hdrs__() ?
I don't think its possible without extensive refactor as
parse_ipv6_ext_hdrs__() is used in multiple places.
While theorethicaly we could merge that code and use some flag it
would be unreadable.
The other reasoning is to keep userspace and kernel code as similar as possible,
as the kernel side code for parsing is implemented differently and
would be impossible to accomplish that.
>
> > goto out;
> > }
> >
> > + miniflow_pad_from_64(mf, ipv6_exthdr);
> > + miniflow_push_uint16(mf, ipv6_exthdr, ext_hdrs);
> > + miniflow_pad_to_64(mf, ipv6_exthdr);
> > +
> > /* This needs to be after the parse_ipv6_ext_hdrs__() call because
> > it
> > * leaves the nw_frag word uninitialized. */
> > ASSERT_SEQUENTIAL(ipv6_label, nw_frag);
> > @@ -1247,7 +1413,7 @@ flow_get_metadata(const struct flow *flow, struct
> > match *flow_metadata)
> > {
> > int i;
> >
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > match_init_catchall(flow_metadata);
> > if (flow->tunnel.tun_id != htonll(0)) {
> > @@ -1833,7 +1999,7 @@ flow_wildcards_init_for_packet(struct flow_wildcards
> > *wc,
> > memset(&wc->masks, 0x0, sizeof wc->masks);
> >
> > /* Update this function whenever struct flow changes. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > if (flow_tnl_dst_is_set(&flow->tunnel)) {
> > if (flow->tunnel.flags & FLOW_TNL_F_KEY) {
> > @@ -1913,6 +2079,7 @@ flow_wildcards_init_for_packet(struct flow_wildcards
> > *wc,
> > } else if (dl_type == htons(ETH_TYPE_IPV6)) {
> > WC_MASK_FIELD(wc, ipv6_src);
> > WC_MASK_FIELD(wc, ipv6_dst);
> > + WC_MASK_FIELD(wc, ipv6_exthdr);
> > WC_MASK_FIELD(wc, ipv6_label);
> > if (is_nd(flow, wc)) {
> > WC_MASK_FIELD(wc, arp_sha);
> > @@ -1986,7 +2153,7 @@ void
> > flow_wc_map(const struct flow *flow, struct flowmap *map)
> > {
> > /* Update this function whenever struct flow changes. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > flowmap_init(map);
> >
> > @@ -2043,6 +2210,7 @@ flow_wc_map(const struct flow *flow, struct flowmap
> > *map)
> > } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> > FLOWMAP_SET(map, ipv6_src);
> > FLOWMAP_SET(map, ipv6_dst);
> > + FLOWMAP_SET(map, ipv6_exthdr);
> > FLOWMAP_SET(map, ipv6_label);
> > FLOWMAP_SET(map, nw_proto);
> > FLOWMAP_SET(map, nw_frag);
> > @@ -2089,7 +2257,7 @@ void
> > flow_wildcards_clear_non_packet_fields(struct flow_wildcards *wc)
> > {
> > /* Update this function whenever struct flow changes. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
> > memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
> > @@ -2233,7 +2401,7 @@ flow_wildcards_set_xxreg_mask(struct flow_wildcards
> > *wc, int idx,
> > uint32_t
> > miniflow_hash_5tuple(const struct miniflow *flow, uint32_t basis)
> > {
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> > uint32_t hash = basis;
> >
> > if (flow) {
> > @@ -2280,7 +2448,7 @@ ASSERT_SEQUENTIAL(ipv6_src, ipv6_dst);
> > uint32_t
> > flow_hash_5tuple(const struct flow *flow, uint32_t basis)
> > {
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> > uint32_t hash = basis;
> >
> > if (flow) {
> > @@ -2958,7 +3126,7 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16
> > mpls_eth_type,
> >
> > if (clear_flow_L3) {
> > /* Clear all L3 and L4 fields and dp_hash. */
> > - BUILD_ASSERT(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT(FLOW_WC_SEQ == 43);
> > memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> > sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> > flow->dp_hash = 0;
> > @@ -3256,7 +3424,7 @@ flow_compose(struct dp_packet *p, const struct flow
> > *flow,
> > /* Add code to this function (or its callees) for emitting new fields
> > or
> > * protocols. (This isn't essential, so it can be skipped for initial
> > * testing.) */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > uint32_t pseudo_hdr_csum;
> > size_t l4_len;
> > diff --git a/lib/flow.h b/lib/flow.h
> > index c647ad83c..1251f2757 100644
> > --- a/lib/flow.h
> > +++ b/lib/flow.h
> > @@ -133,6 +133,10 @@ void packet_expand(struct dp_packet *, const struct
> > flow *, size_t size);
> > bool parse_ipv6_ext_hdrs(const void **datap, size_t *sizep, uint8_t
> > *nw_proto,
> > uint8_t *nw_frag,
> > const struct ovs_16aligned_ip6_frag **frag_hdr);
> > +uint16_t ipv6_ext_header_size__(uint8_t header_type, uint8_t len);
> > +void get_ipv6_ext_hdrs(const void *datap,
> > + const struct ovs_16aligned_ip6_hdr *nh,
> > + uint16_t *ext_hdrs);
> > bool parse_nsh(const void **datap, size_t *sizep, struct ovs_key_nsh *key);
> > uint16_t parse_tcp_flags(struct dp_packet *packet, ovs_be16 *dl_type_p,
> > uint8_t *nw_frag_p, ovs_be16 *first_vlan_tci_p);
> > @@ -965,7 +969,7 @@ static inline void
> > pkt_metadata_from_flow(struct pkt_metadata *md, const struct flow *flow)
> > {
> > /* Update this function whenever struct flow changes. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > md->recirc_id = flow->recirc_id;
> > md->dp_hash = flow->dp_hash;
> > diff --git a/lib/match.c b/lib/match.c
> > index 2ad03e044..9dc7025c8 100644
> > --- a/lib/match.c
> > +++ b/lib/match.c
> > @@ -1121,7 +1121,6 @@ match_set_ipv6_label(struct match *match, ovs_be32
> > ipv6_label)
> > match->flow.ipv6_label = ipv6_label;
> > }
> >
> > -
> > void
> > match_set_ipv6_label_masked(struct match *match, ovs_be32 ipv6_label,
> > ovs_be32 mask)
> > @@ -1130,6 +1129,21 @@ match_set_ipv6_label_masked(struct match *match,
> > ovs_be32 ipv6_label,
> > match->wc.masks.ipv6_label = mask;
> > }
> >
> > +void
> > +match_set_ipv6_exthdr(struct match *match, uint16_t ipv6_exthdr)
> > +{
> > + match->flow.ipv6_exthdr = ipv6_exthdr;
> > + match->wc.masks.ipv6_exthdr = UINT16_MAX;
> > +}
> > +
> > +void
> > +match_set_ipv6_exthdr_masked(struct match *match, uint16_t ipv6_exthdr,
> > + uint16_t mask)
> > +{
> > + match->flow.ipv6_exthdr = ipv6_exthdr & mask;
> > + match->wc.masks.ipv6_exthdr = mask;
> > +}
> > +
> > void
> > match_set_nd_target(struct match *match, const struct in6_addr *target)
> > {
> > @@ -1462,7 +1476,7 @@ match_format(const struct match *match,
> > bool is_megaflow = false;
> > int i;
> >
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > if (priority != OFP_DEFAULT_PRIORITY) {
> > ds_put_format(s, "%spriority=%s%d,",
> > @@ -1681,6 +1695,13 @@ match_format(const struct match *match,
> > ntohl(wc->masks.ipv6_label));
> > }
> > }
> > + if (f->ipv6_exthdr && wc->masks.ipv6_exthdr) {
> > + format_flags_masked(s, "ipv6_ext",
> > + packet_ipv6_exthdr_flag_to_string,
> > + f->ipv6_exthdr, wc->masks.ipv6_exthdr,
> > + UINT16_MAX);
> > + ds_put_char(s, ',');
> > + }
> > } else if (dl_type == htons(ETH_TYPE_ARP) ||
> > dl_type == htons(ETH_TYPE_RARP)) {
> > format_ip_netmask(s, "arp_spa", f->nw_src, wc->masks.nw_src);
> > diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> > index e03cd8d0c..9935f30bb 100644
> > --- a/lib/meta-flow.c
> > +++ b/lib/meta-flow.c
> > @@ -331,6 +331,9 @@ mf_is_all_wild(const struct mf_field *mf, const struct
> > flow_wildcards *wc)
> > case MFF_IPV6_LABEL:
> > return !wc->masks.ipv6_label;
> >
> > + case MFF_IPV6_EXTHDR:
> > + return !wc->masks.ipv6_exthdr;
> > +
> > case MFF_IP_PROTO:
> > return !wc->masks.nw_proto;
> > case MFF_IP_DSCP:
> > @@ -563,6 +566,7 @@ mf_is_value_valid(const struct mf_field *mf, const
> > union mf_value *value)
> > case MFF_IPV4_DST:
> > case MFF_IPV6_SRC:
> > case MFF_IPV6_DST:
> > + case MFF_IPV6_EXTHDR:
> > case MFF_IP_PROTO:
> > case MFF_IP_TTL:
> > case MFF_ARP_SPA:
> > @@ -869,6 +873,10 @@ mf_get_value(const struct mf_field *mf, const struct
> > flow *flow,
> > value->be32 = flow->ipv6_label;
> > break;
> >
> > + case MFF_IPV6_EXTHDR:
> > + value->u16 = flow->ipv6_exthdr;
> > + break;
> > +
> > case MFF_IP_PROTO:
> > value->u8 = flow->nw_proto;
> > break;
> > @@ -989,7 +997,7 @@ mf_get_value(const struct mf_field *mf, const struct
> > flow *flow,
> > * prerequisites.
> > *
> > * If non-NULL, 'err_str' returns a malloc'ed string describing any errors
> > - * with the request or NULL if there is no error. The caller is reponsible
> > + * with the request or NULL if there is no error. The caller is responsible
> > * for freeing the string. */
> > void
> > mf_set_value(const struct mf_field *mf,
> > @@ -1213,6 +1221,10 @@ mf_set_value(const struct mf_field *mf,
> > match_set_ipv6_label(match, value->be32);
> > break;
> >
> > + case MFF_IPV6_EXTHDR:
> > + match_set_ipv6_exthdr(match, value->u16);
> > + break;
> > +
> > case MFF_IP_PROTO:
> > match_set_nw_proto(match, value->u8);
> > break;
> > @@ -1633,6 +1645,10 @@ mf_set_flow_value(const struct mf_field *mf,
> > flow->ipv6_label = value->be32 & htonl(IPV6_LABEL_MASK);
> > break;
> >
> > + case MFF_IPV6_EXTHDR:
> > + flow->ipv6_exthdr = value->u16;
> > + break;
> > +
> > case MFF_IP_PROTO:
> > flow->nw_proto = value->u8;
> > break;
> > @@ -1865,6 +1881,7 @@ mf_is_pipeline_field(const struct mf_field *mf)
> > case MFF_IPV6_SRC:
> > case MFF_IPV6_DST:
> > case MFF_IPV6_LABEL:
> > + case MFF_IPV6_EXTHDR:
> > case MFF_IP_PROTO:
> > case MFF_IP_DSCP:
> > case MFF_IP_DSCP_SHIFTED:
> > @@ -1935,7 +1952,7 @@ mf_is_set(const struct mf_field *mf, const struct
> > flow *flow)
> > * prerequisites.
> > *
> > * If non-NULL, 'err_str' returns a malloc'ed string describing any errors
> > - * with the request or NULL if there is no error. The caller is reponsible
> > + * with the request or NULL if there is no error. The caller is responsible
> > * for freeing the string. */
> > void
> > mf_set_wild(const struct mf_field *mf, struct match *match, char **err_str)
> > @@ -2182,6 +2199,11 @@ mf_set_wild(const struct mf_field *mf, struct match
> > *match, char **err_str)
> > match->flow.ipv6_label = htonl(0);
> > break;
> >
> > + case MFF_IPV6_EXTHDR:
> > + match->wc.masks.ipv6_exthdr = 0;
> > + match->flow.ipv6_exthdr = 0;
> > + break;
> > +
> > case MFF_IP_PROTO:
> > match->wc.masks.nw_proto = 0;
> > match->flow.nw_proto = 0;
> > @@ -2307,7 +2329,7 @@ mf_set_wild(const struct mf_field *mf, struct match
> > *match, char **err_str)
> > * is responsible for ensuring that 'match' meets 'mf''s prerequisites.
> > *
> > * If non-NULL, 'err_str' returns a malloc'ed string describing any errors
> > - * with the request or NULL if there is no error. The caller is reponsible
> > + * with the request or NULL if there is no error. The caller is responsible
> > * for freeing the string.
> > *
> > * Return a set of enum ofputil_protocol bits (as an uint32_t to avoid
> > circular
> > @@ -2538,6 +2560,10 @@ mf_set(const struct mf_field *mf,
> > }
> > break;
> >
> > + case MFF_IPV6_EXTHDR:
> > + match_set_ipv6_exthdr_masked(match, value->u16, mask->u16);
> > + break;
> > +
> > case MFF_ND_TARGET:
> > match_set_nd_target_masked(match, &value->ipv6, &mask->ipv6);
> > break;
> > @@ -2960,6 +2986,29 @@ parse_mf_flags(const char *s, const char
> > *(*bit_to_string)(uint32_t),
> > return NULL;
> > }
> >
> > +static char *
> > +parse_mf_flags_le(const char *s, const char *(*bit_to_string)(uint32_t),
> > + const char *field_name, uint16_t *flagsp, uint16_t allowed,
> > + uint16_t *maskp)
> > +{
> > + int err;
> > + char *err_str;
> > + uint32_t flags, mask;
> > +
> > + err = parse_flags(s, bit_to_string, '\0', field_name, &err_str,
> > + &flags, allowed, maskp ? &mask : NULL);
> > + if (err < 0) {
> > + return err_str;
> > + }
> > +
> > + *flagsp = flags;
> > + if (maskp) {
> > + *maskp = mask;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > static char *
> > mf_from_tcp_flags_string(const char *s, ovs_be16 *flagsp, ovs_be16 *maskp)
> > {
> > @@ -2974,6 +3023,14 @@ mf_from_tun_flags_string(const char *s, ovs_be16
> > *flagsp, ovs_be16 *maskp)
> > htons(FLOW_TNL_PUB_F_MASK), maskp);
> > }
> >
> > +static char *
> > +mf_from_ipv6_exthdr_flags_string(const char *s, uint16_t *flagsp,
> > + uint16_t *maskp)
> > +{
> > + return parse_mf_flags_le(s, packet_ipv6_exthdr_flag_to_string,
> > "IPV6_EXT",
> > + flagsp, UINT16_MAX, maskp);
> > +}
> > +
> > static char *
> > mf_from_ct_state_string(const char *s, ovs_be32 *flagsp, ovs_be32 *maskp)
> > {
> > @@ -3064,6 +3121,11 @@ mf_parse(const struct mf_field *mf, const char *s,
> > mask->be32 = OVS_BE32_MAX;
> > break;
> >
> > + case MFS_IPV6_EXTHDR:
> > + ovs_assert(mf->n_bytes == sizeof(uint16_t));
> > + error = mf_from_ipv6_exthdr_flags_string(s, &value->u16,
> > &mask->u16);
> > + break;
> > +
> > default:
> > OVS_NOT_REACHED();
> > }
> > @@ -3164,6 +3226,13 @@ mf_format_packet_type_string(ovs_be32 value,
> > ovs_be32 mask, struct ds *s)
> > format_packet_type_masked(s, value, mask);
> > }
> >
> > +static void
> > +mf_format_ipv6_exthdr_flags_string(uint16_t value, uint16_t mask, struct
> > ds *s)
> > +{
> > + format_flags_masked(s, NULL, packet_ipv6_exthdr_flag_to_string, value,
> > + mask, UINT16_MAX);
> > +}
> > +
> > /* Appends to 's' a string representation of field 'mf' whose value is in
> > * 'value' and 'mask'. 'mask' may be NULL to indicate an exact match. */
> > void
> > @@ -3237,6 +3306,11 @@ mf_format(const struct mf_field *mf,
> > mask ? mask->be32 : OVS_BE32_MAX, s);
> > break;
> >
> > + case MFS_IPV6_EXTHDR:
> > + mf_format_ipv6_exthdr_flags_string(value->u16,
> > + mask ? mask->u16 : UINT16_MAX, s);
> > + break;
> > +
> > default:
> > OVS_NOT_REACHED();
> > }
> > diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
> > index 28865f88c..ddccaafb5 100644
> > --- a/lib/meta-flow.xml
> > +++ b/lib/meta-flow.xml
> > @@ -4078,6 +4078,20 @@ r r c c c.
> > </header>
> > </diagram>
> > </field>
> > + <field id="MFF_IPV6_EXTHDR" title="IPv6 Extension Headers">
> > + <p>
> > + The least significant 9 bits hold the presence of extension headers
> > + in packet header, duplication and order correctness. Other bits are
> > + zero:
> > + </p>
> > +
> > + <diagram>
> > + <header name="MFF_IPV6_EXTHDR">
> > + <bits name="zero" above="7" below="0" width=".8"/>
> > + <bits name="label" above="9" width="1.0"/>
>
> We, probably, should list the actual bits with their values here.
>
> > + </header>
> > + </diagram>
> > + </field>
> >
> > <h2>IPv4/IPv6 Fields</h2>
> >
> > diff --git a/lib/nx-match.c b/lib/nx-match.c
> > index 440f5f763..a0640e42c 100644
> > --- a/lib/nx-match.c
> > +++ b/lib/nx-match.c
> > @@ -862,6 +862,14 @@ nxm_put_16(struct nxm_put_ctx *ctx,
> > nxm_put__(ctx, field, version, &value, NULL, sizeof value);
> > }
> >
> > +static void
> > +nxm_put_16m_le(struct nxm_put_ctx *ctx,
> > + enum mf_field_id field, enum ofp_version version,
> > + uint16_t value, uint16_t mask)
> > +{
> > + nxm_put(ctx, field, version, &value, &mask, sizeof value);
> > +}
> > +
> > static void
> > nxm_put_32m(struct nxm_put_ctx *ctx,
> > enum mf_field_id field, enum ofp_version version,
> > @@ -966,6 +974,9 @@ nxm_put_ip(struct nxm_put_ctx *ctx,
> > nxm_put_32m(ctx, MFF_IPV6_LABEL, oxm,
> > flow->ipv6_label, match->wc.masks.ipv6_label);
> >
> > + nxm_put_16m_le(ctx, MFF_IPV6_EXTHDR, oxm,
> > + flow->ipv6_exthdr, match->wc.masks.ipv6_exthdr);
> > +
> > if (match->wc.masks.nw_proto) {
> > nxm_put_8(ctx, MFF_IP_PROTO, oxm, flow->nw_proto);
> >
> > @@ -1051,7 +1062,7 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm,
> > const struct match *match,
> > ovs_be32 spi_mask;
> > int match_len;
> >
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > struct nxm_put_ctx ctx = { .output = b, .implied_ethernet = false };
> >
> > diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> > index 2f4cdd92c..a54b08658 100644
> > --- a/lib/odp-execute.c
> > +++ b/lib/odp-execute.c
> > @@ -553,6 +553,7 @@ odp_execute_set_action(struct dp_packet *packet, const
> > struct nlattr *a)
> > case OVS_KEY_ATTR_CT_ZONE:
> > case OVS_KEY_ATTR_CT_MARK:
> > case OVS_KEY_ATTR_CT_LABELS:
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS:
> > case __OVS_KEY_ATTR_MAX:
> > default:
> > OVS_NOT_REACHED();
> > @@ -665,6 +666,7 @@ odp_execute_masked_set_action(struct dp_packet *packet,
> > case OVS_KEY_ATTR_ICMP:
> > case OVS_KEY_ATTR_ICMPV6:
> > case OVS_KEY_ATTR_TCP_FLAGS:
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS:
> > case __OVS_KEY_ATTR_MAX:
> > default:
> > OVS_NOT_REACHED();
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 9a705cffa..47502ef38 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -178,6 +178,7 @@ ovs_key_attr_to_string(enum ovs_key_attr attr, char
> > *namebuf, size_t bufsize)
> > case OVS_KEY_ATTR_ETHERTYPE: return "eth_type";
> > case OVS_KEY_ATTR_IPV4: return "ipv4";
> > case OVS_KEY_ATTR_IPV6: return "ipv6";
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS: return "ipv6_ext";
> > case OVS_KEY_ATTR_TCP: return "tcp";
> > case OVS_KEY_ATTR_TCP_FLAGS: return "tcp_flags";
> > case OVS_KEY_ATTR_UDP: return "udp";
> > @@ -2746,6 +2747,8 @@ const struct attr_len_tbl
> > ovs_flow_key_attr_lens[OVS_KEY_ATTR_MAX + 1] = {
> > [OVS_KEY_ATTR_NSH] = { .len = ATTR_LEN_NESTED,
> > .next = ovs_nsh_key_attr_lens,
> > .next_max = OVS_NSH_KEY_ATTR_MAX },
> > + [OVS_KEY_ATTR_IPV6_EXTHDRS] = {
> > + .len = sizeof(struct ovs_key_ipv6_exthdrs) },
> > };
> >
> > /* Returns the correct length of the payload for a flow key attribute of
> > the
> > @@ -3255,6 +3258,7 @@ odp_mask_is_constant__(enum ovs_key_attr attr, const
> > void *mask, size_t size,
> > * that -1 becomes all-1-bits and 0 does not change. */
> > ovs_be16 be16 = (OVS_FORCE ovs_be16) constant;
> > uint32_t u32 = constant;
> > + uint16_t u16 = constant;
> > uint8_t u8 = constant;
> > const struct in6_addr *in6 = constant ? &in6addr_exact : &in6addr_any;
> >
> > @@ -3306,6 +3310,9 @@ odp_mask_is_constant__(enum ovs_key_attr attr, const
> > void *mask, size_t size,
> > && ipv6_addr_equals(&ipv6_mask->ipv6_dst, in6));
> > }
> >
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS:
> > + return is_all_byte(mask, size, u16);
> > +
> > case OVS_KEY_ATTR_ARP:
> > return is_all_byte(mask, OFFSETOFEND(struct ovs_key_arp, arp_tha),
> > u8);
> >
> > @@ -3535,6 +3542,23 @@ format_u8u(struct ds *ds, const char *name, uint8_t
> > key,
> > }
> > }
> >
> > +static void
> > +format_u16u(struct ds *ds, const char *name, uint16_t key,
> > + const uint16_t *mask, bool verbose)
> > +{
> > + bool mask_empty = mask && !*mask;
> > +
> > + if (verbose || !mask_empty) {
> > + bool mask_full = !mask || *mask == UINT16_MAX;
> > +
> > + ds_put_format(ds, "%s=%"PRIu16, name, key);
> > + if (!mask_full) { /* Partially masked. */
> > + ds_put_format(ds, "/%#"PRIx16, *mask);
> > + }
> > + ds_put_char(ds, ',');
> > + }
> > +}
> > +
> > static void
> > format_be16(struct ds *ds, const char *name, ovs_be16 key,
> > const ovs_be16 *mask, bool verbose)
> > @@ -4323,6 +4347,14 @@ format_odp_key_attr__(const struct nlattr *a, const
> > struct nlattr *ma,
> > verbose);
> > ds_chomp(ds, ',');
> > break;
> > + }
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS: {
> > + const struct ovs_key_ipv6_exthdrs *key = nl_attr_get(a);
> > + const struct ovs_key_ipv6_exthdrs *mask = ma ? nl_attr_get(ma) :
> > NULL;
> > +
> > + format_u16u(ds, "hdrs", key->hdrs, MASK(mask, hdrs), verbose);
> > + ds_chomp(ds, ',');
> > + break;
> > }
> > /* These have the same structure and format. */
> > case OVS_KEY_ATTR_TCP:
> > @@ -5949,6 +5981,10 @@ parse_odp_key_mask_attr__(struct parse_odp_context
> > *context, const char *s,
> > SCAN_FIELD("frag=", frag, ipv6_frag);
> > } SCAN_END(OVS_KEY_ATTR_IPV6);
> >
> > + SCAN_BEGIN("ipv6_ext(", struct ovs_key_ipv6_exthdrs) {
> > + SCAN_FIELD("hdrs=", u16, hdrs);
> > + } SCAN_END(OVS_KEY_ATTR_IPV6_EXTHDRS);
> > +
> > SCAN_BEGIN("tcp(", struct ovs_key_tcp) {
> > SCAN_FIELD("src=", be16, tcp_src);
> > SCAN_FIELD("dst=", be16, tcp_dst);
> > @@ -6154,6 +6190,10 @@ static void get_ipv6_key(const struct flow *, struct
> > ovs_key_ipv6 *,
> > bool is_mask);
> > static void put_ipv6_key(const struct ovs_key_ipv6 *, struct flow *,
> > bool is_mask);
> > +static void get_ipv6_exthdrs_key(const struct flow *,
> > + struct ovs_key_ipv6_exthdrs *);
> > +static void put_ipv6_exthdrs_key(const struct ovs_key_ipv6_exthdrs *,
> > + struct flow *);
> > static void get_arp_key(const struct flow *, struct ovs_key_arp *);
> > static void put_arp_key(const struct ovs_key_arp *, struct flow *);
> > static void get_nd_key(const struct flow *, struct ovs_key_nd *);
> > @@ -6180,7 +6220,7 @@ odp_flow_key_from_flow__(const struct
> > odp_flow_key_parms *parms,
> > /* New "struct flow" fields that are visible to the datapath
> > (including all
> > * data fields) should be translated into equivalent datapath flow
> > fields
> > * here (you will have to add a OVS_KEY_ATTR_* for them). */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > struct ovs_key_ethernet *eth_key;
> > size_t encap[FLOW_MAX_VLAN_HEADERS] = {0};
> > @@ -6326,6 +6366,16 @@ odp_flow_key_from_flow__(const struct
> > odp_flow_key_parms *parms,
> > ipv6_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_IPV6,
> > sizeof *ipv6_key);
> > get_ipv6_key(data, ipv6_key, export_mask);
> > +
> > + if (parms->support.ipv6_exthdrs && data->ipv6_exthdr) {
>
> I think, we need to set SLOW_MATCH somewhere after the upcall
> translation if we have a mask on that field, but the datapath
> actually doesn't support parsing it. We might have similar
> problme with existing nd_ext field too...
>
> > + struct ovs_key_ipv6_exthdrs *ipv6_exthdrs_key;
> > +
> > + ipv6_exthdrs_key =
> > + nl_msg_put_unspec_uninit(buf,
> > OVS_KEY_ATTR_IPV6_EXTHDRS,
> > + sizeof *ipv6_exthdrs_key);
> > +
> > + get_ipv6_exthdrs_key(data, ipv6_exthdrs_key);
> > + }
> > } else if (flow->dl_type == htons(ETH_TYPE_ARP) ||
> > flow->dl_type == htons(ETH_TYPE_RARP)) {
> > struct ovs_key_arp *arp_key;
> > @@ -6599,6 +6649,7 @@ odp_key_to_dp_packet(const struct nlattr *key, size_t
> > key_len,
> > case OVS_KEY_ATTR_VLAN:
> > case OVS_KEY_ATTR_IPV4:
> > case OVS_KEY_ATTR_IPV6:
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS:
> > case OVS_KEY_ATTR_TCP:
> > case OVS_KEY_ATTR_UDP:
> > case OVS_KEY_ATTR_ICMP:
> > @@ -6970,6 +7021,20 @@ parse_l2_5_onward(const struct nlattr
> > *attrs[OVS_KEY_ATTR_MAX + 1],
> > expected_bit = OVS_KEY_ATTR_IPV6;
> > }
> > }
> > + if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV6_EXTHDRS)) {
> > + const struct ovs_key_ipv6_exthdrs *ipv6_exthdrs_key;
> > +
> > + *expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV6_EXTHDRS;
> > +
> > + ipv6_exthdrs_key =
> > nl_attr_get(attrs[OVS_KEY_ATTR_IPV6_EXTHDRS]);
> > + put_ipv6_exthdrs_key(ipv6_exthdrs_key, flow);
> > +
> > + if (is_mask) {
> > + check_start = ipv6_exthdrs_key;
> > + check_len = sizeof *ipv6_exthdrs_key;
> > + expected_bit = OVS_KEY_ATTR_IPV6_EXTHDRS;
>
> This doesn't look correct. We're overriding the pointers just set
> for the IPv6 header itself. We need to check both of them, not
> only extension headers.
>
> > + }
> > + }
> > } else if (src_flow->dl_type == htons(ETH_TYPE_ARP) ||
> > src_flow->dl_type == htons(ETH_TYPE_RARP)) {
> > if (!is_mask) {
> > @@ -7286,7 +7351,7 @@ odp_flow_key_to_flow__(const struct nlattr *key,
> > size_t key_len,
> > /* New "struct flow" fields that are visible to the datapath
> > (including all
> > * data fields) should be translated from equivalent datapath flow
> > fields
> > * here (you will have to add a OVS_KEY_ATTR_* for them). */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > enum odp_key_fitness fitness = ODP_FIT_ERROR;
> > if (errorp) {
> > @@ -8107,6 +8172,44 @@ commit_set_ipv6_action(const struct flow *flow,
> > struct flow *base_flow,
> > }
> > }
> >
> > +static void
> > +get_ipv6_exthdrs_key(const struct flow *flow,
> > + struct ovs_key_ipv6_exthdrs *ipv6_exthdrs)
> > +{
> > + ipv6_exthdrs->hdrs = flow->ipv6_exthdr;
> > +}
> > +
> > +static void
> > +put_ipv6_exthdrs_key(const struct ovs_key_ipv6_exthdrs *ipv6_exthdrs,
> > + struct flow *flow)
> > +{
> > + flow->ipv6_exthdr = ipv6_exthdrs->hdrs;
> > +}
> > +
> > +static void
> > +commit_set_ipv6_exthdrs_action(const struct flow *flow, struct flow
> > *base_flow,
> > + struct ofpbuf *odp_actions,
> > + struct flow_wildcards *wc, bool use_masked)
> > +{
> > + struct ovs_key_ipv6_exthdrs key, mask, orig_mask, base;
> > + struct offsetof_sizeof ovs_key_ipv6_exthdrs_offsetof_sizeof_arr[] =
> > + OVS_KEY_IPV6_EXTHDRS_OFFSETOF_SIZEOF_ARR;
> > +
> > + get_ipv6_exthdrs_key(flow, &key);
> > + get_ipv6_exthdrs_key(base_flow, &base);
> > + get_ipv6_exthdrs_key(&wc->masks, &mask);
> > + memcpy(&orig_mask, &mask, sizeof mask);
> > + mask.hdrs = 0;
> > +
> > + if (commit(OVS_KEY_ATTR_IPV6_EXTHDRS, use_masked, &key, &base, &mask,
> > + sizeof key, ovs_key_ipv6_exthdrs_offsetof_sizeof_arr,
> > + odp_actions)) {
> > + put_ipv6_exthdrs_key(&base, base_flow);
> > + or_masks(&mask, &orig_mask,
> > ovs_key_ipv6_exthdrs_offsetof_sizeof_arr);
> > + put_ipv6_exthdrs_key(&mask, &wc->masks);
> > + }
> > +}
> > +
> > static void
> > get_arp_key(const struct flow *flow, struct ovs_key_arp *arp)
> > {
> > @@ -8308,6 +8411,8 @@ commit_set_nw_action(const struct flow *flow, struct
> > flow *base,
> >
> > case ETH_TYPE_IPV6:
> > commit_set_ipv6_action(flow, base, odp_actions, wc, use_masked);
> > + commit_set_ipv6_exthdrs_action(flow, base, odp_actions, wc,
> > + use_masked);
>
> The filed is read-only, right?
> Actions should not be able to set it. Or am I missing something?
>
> > if (base->nw_proto == IPPROTO_ICMPV6) {
> > /* Commit extended attrs first to make sure
> > correct options are added.*/
> > @@ -8706,7 +8811,7 @@ commit_odp_actions(const struct flow *flow, struct
> > flow *base,
> > /* If you add a field that OpenFlow actions can change, and that is
> > visible
> > * to the datapath (including all data fields), then you should also
> > add
> > * code here to commit changes to the field. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > enum slow_path_reason slow1, slow2;
> > bool mpls_done = false;
> > diff --git a/lib/odp-util.h b/lib/odp-util.h
> > index a1d0d0fba..9a8301228 100644
> > --- a/lib/odp-util.h
> > +++ b/lib/odp-util.h
> > @@ -138,16 +138,17 @@ void odp_portno_name_format(const struct hmap
> > *portno_names,
> > * OVS_KEY_ATTR_ENCAP 0 -- 4 4 (VLAN
> > encapsulation)
> > * OVS_KEY_ATTR_ETHERTYPE 2 2 4 8 (inner VLAN
> > ethertype)
> > * OVS_KEY_ATTR_IPV6 40 -- 4 44
> > + * OVS_KEY_ATTR_IPV6_EXTHDRS 2 2 4 8
> > * OVS_KEY_ATTR_ICMPV6 2 2 4 8
> > * OVS_KEY_ATTR_ND 28 -- 4 32
> > * ----------------------------------------------------------
> > - * total 616
> > + * total 624
> > *
> > * We include some slack space in case the calculation isn't quite right
> > or we
> > * add another field and forget to adjust this value.
> > */
> > -#define ODPUTIL_FLOW_KEY_BYTES 640
> > -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > +#define ODPUTIL_FLOW_KEY_BYTES 648
> > +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > /* A buffer with sufficient size and alignment to hold an nlattr-formatted
> > flow
> > * key. An array of "struct nlattr" might not, in theory, be sufficiently
> > @@ -207,7 +208,9 @@ int odp_flow_from_string(const char *s, const struct
> > simap *port_names,
> >
> > \
> > /* If true, it means that the datapath supports the IPv6 Neigh
> > \
> > * Discovery Extension bits. */
> > \
> > - ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension")
> > + ODP_SUPPORT_FIELD(bool, nd_ext, "IPv6 ND Extension")
> > \
> > +
> > \
> > + ODP_SUPPORT_FIELD(bool, ipv6_exthdrs, "IPv6 Extension headers")
> >
> > /* Indicates support for various fields. This defines how flows will be
> > * serialised. */
> > diff --git a/lib/ofp-match.c b/lib/ofp-match.c
> > index 86a082dde..6525922ef 100644
> > --- a/lib/ofp-match.c
> > +++ b/lib/ofp-match.c
> > @@ -65,7 +65,7 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
> > void
> > ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
> > {
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > /* Initialize most of wc. */
> > flow_wildcards_init_catchall(wc);
> > diff --git a/lib/packets.c b/lib/packets.c
> > index d0fba8176..ff38f1961 100644
> > --- a/lib/packets.c
> > +++ b/lib/packets.c
> > @@ -34,6 +34,7 @@
> > #include "odp-util.h"
> > #include "dp-packet.h"
> > #include "unaligned.h"
> > +#include "openflow/openflow.h"
> >
> > const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
> > const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
> > @@ -1082,6 +1083,33 @@ ipv6_is_cidr(const struct in6_addr *netmask)
> > return true;
> > }
> >
> > +const char *
> > +packet_ipv6_exthdr_flag_to_string(uint32_t flag)
> > +{
> > + switch (flag) {
> > + case OFPIEH12_NONEXT:
> > + return "nonext";
> > + case OFPIEH12_ESP:
> > + return "esp";
> > + case OFPIEH12_AUTH:
> > + return "auth";
> > + case OFPIEH12_DEST:
> > + return "dest";
> > + case OFPIEH12_FRAG:
> > + return "frag";
> > + case OFPIEH12_ROUTER:
> > + return "router";
> > + case OFPIEH12_HOP:
> > + return "hop";
> > + case OFPIEH12_UNREP:
> > + return "unrep";
> > + case OFPIEH12_UNSEQ:
> > + return "unseq";
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > /* Populates 'b' with an Ethernet II packet headed with the given
> > 'eth_dst',
> > * 'eth_src' and 'eth_type' parameters. A payload of 'size' bytes is
> > allocated
> > * in 'b' and returned. This payload may be populated with appropriate
> > diff --git a/lib/packets.h b/lib/packets.h
> > index 5bdf6e4bb..3becc1faf 100644
> > --- a/lib/packets.h
> > +++ b/lib/packets.h
> > @@ -901,6 +901,17 @@ struct tcp_header {
> > };
> > BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header));
> >
> > +/* IPV6 extension header OpenFlow flags */
> > +#define OFPIEH12_NONEXT 0x001
> > +#define OFPIEH12_ESP 0x002
> > +#define OFPIEH12_AUTH 0x004
> > +#define OFPIEH12_DEST 0x008
> > +#define OFPIEH12_FRAG 0x010
> > +#define OFPIEH12_ROUTER 0x020
> > +#define OFPIEH12_HOP 0x040
> > +#define OFPIEH12_UNREP 0x080
> > +#define OFPIEH12_UNSEQ 0x100
>
> I'm not sure if these bits should have OFPIEH12_ prefix,
> but it should definitely be 13 instead of 12.
>
I think I took the naming from openflow documentations and
/include/openflow/openflow-1.2.h
/* Bit definitions for IPv6 Extension Header pseudo-field. */
enum ofp12_ipv6exthdr_flags {
OFPIEH12_NONEXT = 1 << 0, /* "No next header" encountered. */
OFPIEH12_ESP = 1 << 1, /* Encrypted Sec Payload header present. */
OFPIEH12_AUTH = 1 << 2, /* Authentication header present. */
OFPIEH12_DEST = 1 << 3, /* 1 or 2 dest headers present. */
OFPIEH12_FRAG = 1 << 4, /* Fragment header present. */
OFPIEH12_ROUTER = 1 << 5, /* Router header present. */
OFPIEH12_HOP = 1 << 6, /* Hop-by-hop header present. */
OFPIEH12_UNREP = 1 << 7, /* Unexpected repeats encountered. */
OFPIEH12_UNSEQ = 1 << 8 /* Unexpected sequencing encountered. */
};
but yes if the version is from 1.3 then this needs to be fixed.
> > +
> > /* Connection states.
> > *
> > * Names like CS_RELATED are bit values, e.g. 1 << 2.
> > @@ -1612,6 +1623,7 @@ void packet_set_igmp3_query(struct dp_packet *,
> > uint8_t max_resp,
> > uint8_t qqic);
> > void packet_format_tcp_flags(struct ds *, uint16_t);
> > const char *packet_tcp_flag_to_string(uint32_t flag);
> > +const char *packet_ipv6_exthdr_flag_to_string(uint32_t flag);
> > void *compose_ipv6(struct dp_packet *packet, uint8_t proto,
> > const struct in6_addr *src, const struct in6_addr *dst,
> > uint8_t key_tc, ovs_be32 key_fl, uint8_t key_hl, int
> > size);
> > diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
> > index 4df630c62..21f32b935 100644
> > --- a/ofproto/ofproto-dpif-rid.h
> > +++ b/ofproto/ofproto-dpif-rid.h
> > @@ -100,7 +100,7 @@ struct rule;
> > /* Metadata for restoring pipeline context after recirculation. Helpers
> > * are inlined below to keep them together with the definition for easier
> > * updates. */
> > -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> >
> > struct frozen_metadata {
> > /* Metadata in struct flow. */
> > diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> > index 30e7caf54..e60abb00b 100644
> > --- a/ofproto/ofproto-dpif-sflow.c
> > +++ b/ofproto/ofproto-dpif-sflow.c
> > @@ -1065,6 +1065,7 @@ sflow_read_set_action(const struct nlattr *attr,
> > case OVS_KEY_ATTR_UNSPEC:
> > case OVS_KEY_ATTR_PACKET_TYPE:
> > case OVS_KEY_ATTR_NSH:
> > + case OVS_KEY_ATTR_IPV6_EXTHDRS:
> > case __OVS_KEY_ATTR_MAX:
> > default:
> > break;
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index cc9c1c628..2ff954782 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -4183,7 +4183,7 @@ compose_output_action__(struct xlate_ctx *ctx,
> > ofp_port_t ofp_port,
> >
> > /* If 'struct flow' gets additional metadata, we'll need to zero it out
> > * before traversing a patch port. */
> > - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> > + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> > memset(&flow_tnl, 0, sizeof flow_tnl);
> >
> > if (!check_output_prerequisites(ctx, xport, flow, check_stp)) {
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index a4c44052d..c0ecefde8 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -1535,6 +1535,52 @@ check_nd_extensions(struct dpif_backer *backer)
> > return !error;
> > }
> >
> > +/* Tests whether 'backer''s datapath supports IPv6 Extension headers.
> > + *
> > + * Returns false if 'backer' definitely does not support, true if it seems
> > + * to support. */
> > +static bool
> > +check_ipv6_extension_headers(struct dpif_backer *backer)
> > +{
> > +struct eth_header *eth;
> > +struct ofpbuf actions;
> > +struct dp_packet packet;
> > +struct flow flow;
> > +int error;
> > +struct ovs_key_nd_extensions key, mask;
> > +
> > +ofpbuf_init(&actions, 64);
> > +memset(&key, 0x53, sizeof key);
> > +memset(&mask, 0x7f, sizeof mask);
> > +commit_masked_set_action(&actions, OVS_KEY_ATTR_IPV6_EXTHDRS, &key, &mask,
> > + sizeof key);
> > +
> > +/* Compose a dummy ethernet packet. */
> > +dp_packet_init(&packet, ETH_HEADER_LEN);
> > +eth = dp_packet_put_zeros(&packet, ETH_HEADER_LEN);
> > +eth->eth_type = htons(0x1234);
> > +
> > +flow_extract(&packet, &flow);
> > +
> > +/* Execute the actions. On datapaths without support fails with EINVAL. */
> > +struct dpif_execute execute = {
> > +.actions = actions.data,
> > +.actions_len = actions.size,
> > +.packet = &packet,
> > +.flow = &flow,
> > +.probe = true,
> > +};
> > +error = dpif_execute(backer->dpif, &execute);
> > +
> > +dp_packet_uninit(&packet);
> > +ofpbuf_uninit(&actions);
> > +
> > +VLOG_INFO("%s: Datapath %s IPv6 Extension headers",
> > dpif_name(backer->dpif),
> > + error ? "does not support" : "supports");
> > +
> > +return !error;
> > +}
> > +
>
> For some reason indentation in that block is lost.
>
Yes, not sure why this happened.
> > /* Tests whether 'backer''s datapath supports the OVS_ACTION_ATTR_ADD_MPLS
> > * action. */
> > static bool
> > @@ -1653,6 +1699,7 @@ check_support(struct dpif_backer *backer)
> > backer->rt_support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
> > backer->rt_support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer);
> > backer->rt_support.odp.nd_ext = check_nd_extensions(backer);
> > +backer->rt_support.odp.ipv6_exthdrs = check_ipv6_extension_headers(backer);
>
> Ditto.
>
> > }
> >
> > static int
> > diff --git a/tests/ofproto.at b/tests/ofproto.at
> > index 156d3e058..282bdc338 100644
> > --- a/tests/ofproto.at
> > +++ b/tests/ofproto.at
> > @@ -2291,7 +2291,7 @@ head_table() {
> > actions: output group set_field strip_vlan push_vlan mod_nw_ttl
> > dec_ttl set_mpls_ttl dec_mpls_ttl push_mpls pop_mpls set_queue
> > supported on Set-Field: metadata in_port_oxm eth_{src,dst}
> > vlan_{vid,pcp} mpls_{label,tc} ip_{src,dst} ipv6_{src,dst,label} ip_dscp
> > nw_ecn arp_{op,spa,tpa,sha,tha} tcp_{src,dst} udp_{src,dst} sctp_{src,dst}
> > icmp_{type,code} icmpv6_{type,code} nd_{target,sll,tll}
> > matching:
> > - exact match or wildcard: metadata in_port_oxm eth_{src,dst,type}
> > vlan_{vid,pcp} mpls_{label,tc} ip_{src,dst} ipv6_{src,dst,label} nw_proto
> > ip_dscp nw_ecn arp_{op,spa,tpa,sha,tha} tcp_{src,dst} udp_{src,dst}
> > sctp_{src,dst} icmp_{type,code} icmpv6_{type,code} nd_{target,sll,tll}
> > + exact match or wildcard: metadata in_port_oxm eth_{src,dst,type}
> > vlan_{vid,pcp} mpls_{label,tc} ip_{src,dst} ipv6_{src,dst,label,ext}
> > nw_proto ip_dscp nw_ecn arp_{op,spa,tpa,sha,tha} tcp_{src,dst}
> > udp_{src,dst} sctp_{src,dst} icmp_{type,code} icmpv6_{type,code}
> > nd_{target,sll,tll}
> >
> > ' "$1"
> > }
> > @@ -2353,7 +2353,7 @@ head_table () {
> > actions: output group set_field strip_vlan push_vlan mod_nw_ttl
> > dec_ttl set_mpls_ttl dec_mpls_ttl push_mpls pop_mpls set_queue
> > supported on Set-Field:
> > tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},metadata0...metadata63}
> > metadata in_{port,port_oxm} pkt_mark ct_{mark,label} reg0...reg15
> > xreg0...xreg7 xxreg0...xxreg3 eth_{src,dst} vlan_{tci,vid,pcp}
> > mpls_{label,tc,ttl} ip_{src,dst} ipv6_{src,dst,label} nw_tos ip_dscp
> > nw_{ecn,ttl} arp_{op,spa,tpa,sha,tha} tcp_{src,dst} udp_{src,dst}
> > sctp_{src,dst} icmp_{type,code} icmpv6_{type,code}
> > nd_{target,sll,tll,reserved,options_type} nsh_{flags,spi,si,c1...c4,ttl}
> > matching:
> > - arbitrary mask: dp_hash
> > tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},gtpu_{flags,msgtype},metadata0...metadata63}
> > metadata pkt_mark
> > ct_{state,mark,label,nw_{src,dst},ipv6_{src,dst},tp_{src,dst}} reg0...reg15
> > xreg0...xreg7 xxreg0...xxreg3 eth_{src,dst} vlan_{tci,vid} ip_{src,dst}
> > ipv6_{src,dst,label} ip_frag arp_{spa,tpa,sha,tha} tcp_{src,dst,flags}
> > udp_{src,dst} sctp_{src,dst} nd_{target,sll,tll} nsh_{flags,c1...c4}
> > + arbitrary mask: dp_hash
> > tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},gtpu_{flags,msgtype},metadata0...metadata63}
> > metadata pkt_mark
> > ct_{state,mark,label,nw_{src,dst},ipv6_{src,dst},tp_{src,dst}} reg0...reg15
> > xreg0...xreg7 xxreg0...xxreg3 eth_{src,dst} vlan_{tci,vid} ip_{src,dst}
> > ipv6_{src,dst,label,ext} ip_frag arp_{spa,tpa,sha,tha} tcp_{src,dst,flags}
> > udp_{src,dst} sctp_{src,dst} nd_{target,sll,tll} nsh_{flags,c1...c4}
> > exact match or wildcard: recirc_id packet_type conj_id
> > in_{port,port_oxm} actset_output ct_{zone,nw_proto} eth_type vlan_pcp
> > mpls_{label,tc,bos,ttl} nw_{proto,tos} ip_dscp nw_{ecn,ttl} arp_op
> > icmp_{type,code} icmpv6_{type,code} nd_{reserved,options_type}
> > nsh_{mdtype,np,spi,si,ttl}
> >
> > ' "$1"
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 95383275a..b54dcbf47 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -7143,3 +7143,34 @@ OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0050: *0000
> > *0000 *5002 *2000 *b85e *0000
> >
> > OVS_TRAFFIC_VSWITCHD_STOP
> > AT_CLEANUP
> > +
> > +AT_SETUP([ipv6 extension headers])
> > +OVS_TRAFFIC_VSWITCHD_START()
> > +
> > +ADD_NAMESPACES(at_ns1, at_ns2)
> > +
> > +AT_CHECK([ovs-vsctl add-port br0 p1 -- set interface p1 type=internal])
> > +AT_CHECK([ip link set p1 netns at_ns1])
> > +AT_CHECK([ip netns exec at_ns1 ip -6 addr add dev p1 fe80::1/64])
> > +AT_CHECK([ip netns exec at_ns1 ip link set p1 up])
> > +
> > +AT_CHECK([ovs-vsctl add-port br0 p2 -- set interface p2 type=internal])
> > +AT_CHECK([ip link set p2 netns at_ns2])
> > +AT_CHECK([ip netns exec at_ns2 ip -6 addr add dev p2 fe80::2/64])
> > +AT_CHECK([ip netns exec at_ns2 ip link set p2 up])
> > +
> > +AT_CHECK([ovs-ofctl add-flow br0 "priority=2,ipv6_ext=frag,action=normal"])
> > +
> > +NS_CHECK_EXEC([at_ns2], [tcpdump -l -n -xx -U -i p2 > p2.pcap &])
> > +
> > +sleep 1
> > +
> > +NS_CHECK_EXEC([at_ns1], [$PYTHON3 $srcdir/sendpkt.py p1 8E 07 06 D3 C9 B5
> > F2 AF 1A 44 D3 5B 86 DD 60 00 00 00 00 08 2C 40 FE 80 00 00 00 00 00 00 00
> > 00 00 00 00 00 00 01 FE 80 00 00 00 00 00 00 00 00 00 00 00 00 00 02 3B 00
> > 00 00 00 00 00 00 > /dev/null])
> > +
> > +OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0000: 8e07 06d3 c9b5 f2af 1a44
> > d35b 86dd 6000" 2>&1 1>/dev/null])
> > +OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0010: 0000 0008 2c40 fe80 0000
> > 0000 0000 0000" 2>&1 1>/dev/null])
> > +OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0020: 0000 0000 0001 fe80 0000
> > 0000 0000 0000" 2>&1 1>/dev/null])
> > +OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0030: 0000 0000 0002 3b00 0000
> > 0000 0000" 2>&1 1>/dev/null])
> > +
> > +OVS_TRAFFIC_VSWITCHD_STOP
> > +AT_CLEANUP
>
> It's good to have a system test for a sanity check, but new OpenFlow
> field requires at least OpenFlow conformace unit tests, new datapath
> match also requires tests for parsing, translation, dependencies.
>
> Please, add:
> - positive and negative tests to the tests/ovs-ofctl.at
> for different versions of OpenFlow with NXM/OXM, binary
> representation of the match.
> - tests for combinations of flags (see some examples for tcp_flags
> in the tests/ovs-ofctl.at).
> - tests in tests/odp.at for the datapath flows.
> - test in tests/ofp-actions.at to check that the filed is actually
> read-only.
> - OSS-Fuzz dictionary update at tests/oss-fuzz/config/odp.dict
> - Some tests in tests/ofproto-dpif.c to check that flow translation
> is working as expected and packets are matching or not matching
> on the flow depending on the flags.
> - Probbaly, some tests in tests/ofp-print.at
ok.
>
> Please, aslo update the feature support table in the docs:
> Documentation/faq/releases.rst
ok
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
