Nicolas Dichtel
2022-Sep-20 07:49 UTC
[Bridge] [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete request modifier
Le 13/04/2022 ? 12:51, Nikolay Aleksandrov a ?crit?:> Add a new delete request modifier called NLM_F_BULK which, when > supported, would cause the request to delete multiple objects. The flag > is a convenient way to signal that a multiple delete operation is > requested which can be gradually added to different delete requests. In > order to make sure older kernels will error out if the operation is not > supported instead of doing something unintended we have to break a > required condition when implementing support for this flag, f.e. for > neighbors we will omit the mandatory mac address attribute. > Initially it will be used to add flush with filtering support for bridge > fdbs, but it also opens the door to add similar support to others. > > Signed-off-by: Nikolay Aleksandrov <razor at blackwall.org> > --- > include/uapi/linux/netlink.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h > index 4c0cde075c27..855dffb4c1c3 100644 > --- a/include/uapi/linux/netlink.h > +++ b/include/uapi/linux/netlink.h > @@ -72,6 +72,7 @@ struct nlmsghdr { > > /* Modifiers to DELETE request */ > #define NLM_F_NONREC 0x100 /* Do not delete recursively */ > +#define NLM_F_BULK 0x200 /* Delete multiple objects */Sorry to reply to an old patch, but FWIW, this patch broke the uAPI. One of our applications was using NLM_F_EXCL with RTM_DELTFILTER. This is conceptually wrong but it was working. After this patch, the kernel returns an error (EOPNOTSUPP). Here is the patch series: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=92716869375b We probably can't do anything now, but to avoid this in the future, I see only two options: - enforce flags validation depending on the operation (but this may break some existing apps) - stop adding new flags that overlap between NEW and DEL operations (by adding a comment or defining dummy flags). Any thoughts? Regards, Nicolas
Nikolay Aleksandrov
2022-Sep-20 09:05 UTC
[Bridge] [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete request modifier
On 20/09/2022 10:49, Nicolas Dichtel wrote:> > Le 13/04/2022 ? 12:51, Nikolay Aleksandrov a ?crit?: >> Add a new delete request modifier called NLM_F_BULK which, when >> supported, would cause the request to delete multiple objects. The flag >> is a convenient way to signal that a multiple delete operation is >> requested which can be gradually added to different delete requests. In >> order to make sure older kernels will error out if the operation is not >> supported instead of doing something unintended we have to break a >> required condition when implementing support for this flag, f.e. for >> neighbors we will omit the mandatory mac address attribute. >> Initially it will be used to add flush with filtering support for bridge >> fdbs, but it also opens the door to add similar support to others. >> >> Signed-off-by: Nikolay Aleksandrov <razor at blackwall.org> >> --- >> include/uapi/linux/netlink.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h >> index 4c0cde075c27..855dffb4c1c3 100644 >> --- a/include/uapi/linux/netlink.h >> +++ b/include/uapi/linux/netlink.h >> @@ -72,6 +72,7 @@ struct nlmsghdr { >> >> /* Modifiers to DELETE request */ >> #define NLM_F_NONREC 0x100 /* Do not delete recursively */ >> +#define NLM_F_BULK 0x200 /* Delete multiple objects */ > Sorry to reply to an old patch, but FWIW, this patch broke the uAPI. > One of our applications was using NLM_F_EXCL with RTM_DELTFILTER. This is > conceptually wrong but it was working. After this patch, the kernel returns an > error (EOPNOTSUPP). > > Here is the patch series: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?h=92716869375b > > We probably can't do anything now, but to avoid this in the future, I see only > two options: > - enforce flags validation depending on the operation (but this may break some > existing apps) > - stop adding new flags that overlap between NEW and DEL operations (by adding > a comment or defining dummy flags). > > Any thoughts? >Personally I'd prefer to enforce validation so we don't lose the flags because of buggy user-space applications, but we can break someone (who arguably should fix their app though). We already had that discussion while the set was under review[1] and just to be a bit more confident I also tried searching for open-source buggy users, but didn't find any.> Regards, > Nicolas[1] https://lore.kernel.org/netdev/97774474-65a3-fa45-e0b9-8db6c748da28 at kernel.org/t/#m23018ce831dae16d42cb9c393c7c6bad1bc621c3