Re: [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete request modifier

[Nikolay Aleksandrov <razor@blackwall.org>]

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@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@kernel.org/t/#m23018ce831dae16d42cb9c393c7c6bad1bc621c3