Re: [PATCH net-next v2] net: openvswitch: fix uAPI incompatibility with existing user space

[Roi Dayan]

On 2022-03-10 8:44 PM, Aaron Conole wrote:
> Ilya Maximets <i.maximets@xxxxxxx> writes:
>
> > Few years ago OVS user space made a strange choice in the commit [1]
> > to define types only valid for the user space inside the copy of a
> > kernel uAPI header.  '#ifndef __KERNEL__' and another attribute was
> > added later.
> >
> > This leads to the inevitable clash between user space and kernel types
> > when the kernel uAPI is extended.  The issue was unveiled with the
> > addition of a new type for IPv6 extension header in kernel uAPI.
> >
> > When kernel provides the OVS_KEY_ATTR_IPV6_EXTHDRS attribute to the
> > older user space application, application tries to parse it as
> > OVS_KEY_ATTR_PACKET_TYPE and discards the whole netlink message as
> > malformed.  Since OVS_KEY_ATTR_IPV6_EXTHDRS is supplied along with
> > every IPv6 packet that goes to the user space, IPv6 support is fully
> > broken.
> >
> > Fixing that by bringing these user space attributes to the kernel
> > uAPI to avoid the clash.  Strictly speaking this is not the problem
> > of the kernel uAPI, but changing it is the only way to avoid breakage
> > of the older user space applications at this point.
> >
> > These 2 types are explicitly rejected now since they should not be
> > passed to the kernel.  Additionally, OVS_KEY_ATTR_TUNNEL_INFO moved
> > out from the '#ifdef __KERNEL__' as there is no good reason to hide
> > it from the userspace.  And it's also explicitly rejected now, because
> > it's for in-kernel use only.
> >
> > Comments with warnings were added to avoid the problem coming back.
> >
> > (1 << type) converted to (1ULL << type) to avoid integer overflow on
> > OVS_KEY_ATTR_IPV6_EXTHDRS, since it equals 32 now.
> >
> >   [1] beb75a40fdc2 ("userspace: Switching of L3 packets in L2 pipeline")
> >
> > Fixes: 28a3f0601727 ("net: openvswitch: IPv6: Add IPv6 extension header support")
> > Link: https://lore.kernel.org/netdev/3adf00c7-fe65-3ef4-b6d7-6d8a0cad8a5f@xxxxxxxxxx
> > Link: https://github.com/openvswitch/ovs/commit/beb75a40fdc295bfd6521b0068b4cd12f6de507c
> > Reported-by: Roi Dayan <roid@xxxxxxxxxx>
> > Signed-off-by: Ilya Maximets <i.maximets@xxxxxxx>
> > ---
>
> Acked-by: Aaron Conole <aconole@xxxxxxxxxx>



I got to check traffic with the fix and I do get some traffic
but something is broken. I didn't investigate much but the quick
test shows me rules are not offloaded and dumping ovs rules gives
error like this

recirc_id(0),in_port(enp8s0f0_1),ct_state(-trk),eth(),eth_type(0x86dd),ipv6(frag=no)(bad 
key length 2, expected -1)(00 00/(bad mask length 2, expected -1)(00 
00), packets:2453, bytes:211594, used:0.004s, flags:S., 
actions:ct,recirc(0x2)