Re: [ovs-dev] [PATCH net-next v7] net: openvswitch: IPv6: Add IPv6 extension header support

[Cpp Code <cpp.code.lv@gmail.com>]

On Fri, Oct 15, 2021 at 6:56 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 10/14/21 23:18, Toms Atteka wrote:
> > This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and
> > packets can be filtered using ipv6_ext flag.
> >
> > Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
> > ---
> >  include/uapi/linux/openvswitch.h |  16 +++-
> >  net/openvswitch/flow.c           | 140 +++++++++++++++++++++++++++++++
> >  net/openvswitch/flow.h           |  14 ++++
> >  net/openvswitch/flow_netlink.c   |  24 +++++-
> >  4 files changed, 192 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> > index a87b44cd5590..763adf3dce23 100644
> > --- a/include/uapi/linux/openvswitch.h
> > +++ b/include/uapi/linux/openvswitch.h
> > @@ -344,8 +344,17 @@ enum ovs_key_attr {
> >       OVS_KEY_ATTR_NSH,       /* Nested set of ovs_nsh_key_* */
> >
> >  #ifdef __KERNEL__
> > -     OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ip_tunnel_info */
> > +     OVS_KEY_ATTR_TUNNEL_INFO,/* struct ip_tunnel_info */
> > +     __OVS_KEY_ATTR_PADDING_1,/* Padding to match field count with ovs */
> >  #endif
> > +
> > +#ifndef __KERNEL__
> > +     __OVS_KEY_ATTR_PADDING_2,/* Padding to match field count with ovs */
> > +     __OVS_KEY_ATTR_PADDING_3,/* Padding to match field count with ovs */
> > +#endif
> > +
> > +     OVS_KEY_ATTR_IPV6_EXTHDRS,  /* struct ovs_key_ipv6_exthdr */
> > +
> >       __OVS_KEY_ATTR_MAX
> >  };
>
> Not a full review, but, I think, that we should not add paddings, and
> define OVS_KEY_ATTR_IPV6_EXTHDRS before the OVS_KEY_ATTR_TUNNEL_INFO
> instead.  See my comments for v6:
>   https://lore.kernel.org/netdev/8c4ee3e8-0400-ee6e-b12c-327806f26dae@ovn.org/T/#u
>
> Best regards, Ilya Maximets.

Maybe I am still missing the idea, but I think it's not possible.

If we consider userspace code:
OVS_KEY_ATTR_NSH,       /* Nested set of ovs_nsh_key_* */

#ifdef __KERNEL__
/* Only used within kernel data path. */
OVS_KEY_ATTR_TUNNEL_INFO,  /* struct ovs_tunnel_info */
#endif

#ifndef __KERNEL__
/* Only used within userspace data path. */
OVS_KEY_ATTR_PACKET_TYPE,  /* be32 packet type */
OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
#endif

__OVS_KEY_ATTR_MAX
};

adding OVS_KEY_ATTR_IPV6_EXTHDRS before OVS_KEY_ATTR_PACKET_TYPE will
conflict on kernel update.

And we will see following warning:
00001|odp_util(handler1)|WARN|attribute packet_type has length 2 but
should have length 4

Best,
Tom