Messages in this thread | | | Subject | Re: [PATCH net-next v6] net: openvswitch: IPv6: Add IPv6 extension header support | From | Nicolas Dichtel <> | Date | Fri, 1 Oct 2021 09:21:29 +0200 |
| |
Le 30/09/2021 à 18:11, Cpp Code a écrit : > On Wed, Sep 29, 2021 at 6:19 AM Jakub Kicinski <kuba@kernel.org> wrote: >> >> On Wed, 29 Sep 2021 08:19:05 +0200 Nicolas Dichtel wrote: >>>> /* Insert a kernel only KEY_ATTR */ >>>> #define OVS_KEY_ATTR_TUNNEL_INFO __OVS_KEY_ATTR_MAX >>>> #undef OVS_KEY_ATTR_MAX >>>> #define OVS_KEY_ATTR_MAX __OVS_KEY_ATTR_MAX >>> Following the other thread [1], this will break if a new app runs over an old >>> kernel. >> >> Good point. >> >>> Why not simply expose this attribute to userspace and throw an error if a >>> userspace app uses it? >> >> Does it matter if it's exposed or not? Either way the parsing policy >> for attrs coming from user space should have a reject for the value. >> (I say that not having looked at the code, so maybe I shouldn't...) > > To remove some confusion, there are some architectural nuances if we > want to extend code without large refactor. > The ovs_key_attr is defined only in kernel side. Userspace side is > generated from this file. As well the code can be built without kernel > modules. > The code inside OVS repository and net-next is not identical, but I > try to keep some consistency. I didn't get why OVS_KEY_ATTR_TUNNEL_INFO cannot be exposed to userspace.
> > JFYI This is the file responsible for generating userspace part: > https://github.com/openvswitch/ovs/blob/master/build-aux/extract-odp-netlink-h > This is the how corresponding file for ovs_key_attr looks inside OVS: > https://github.com/openvswitch/ovs/blob/master/datapath/linux/compat/include/linux/openvswitch.h > one can see there are more values than in net-next version. There are still some '#ifdef __KERNEL__'. The standard 'make headers_install' filters them. Why not using this standard mechanism?
In this file, there are two attributes (OVS_KEY_ATTR_PACKET_TYPE and OVS_KEY_ATTR_ND_EXTENSIONS) that doesn't exist in the kernel. This will also breaks if an old app runs over a new kernel. I don't see how it is possible to keep the compat between {old|new} {kernel|app}.
Regards, Nicolas
| |