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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAASuNyXe6OYPe0Utd4xG1-ZTHZet3sJDi2sxXJ-7RWQ-GaEW2w@mail.gmail.com>
Date:   Tue, 19 Oct 2021 12:30:22 -0700
From:   Cpp Code <cpp.code.lv@...il.com>
To:     Ilya Maximets <i.maximets@....org>
Cc:     netdev@...r.kernel.org, pshelar@....org,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        ovs dev <dev@...nvswitch.org>, linux-kernel@...r.kernel.org,
        Nicolas Dichtel <nicolas.dichtel@...nd.com>
Subject: Re: [ovs-dev] [PATCH net-next v7] net: openvswitch: IPv6: Add IPv6
 extension header support

On Fri, Oct 15, 2021 at 6:56 AM Ilya Maximets <i.maximets@....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@...il.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