From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
To: Nikolay Aleksandrov <razor@blackwall.org>, netdev@vger.kernel.org
Cc: dsahern@kernel.org, bridge@lists.linux-foundation.org,
idosch@idosch.org, roopa@nvidia.com, kuba@kernel.org,
davem@davemloft.net
Subject: Re: [Bridge] [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete request modifier
Date: Wed, 21 Sep 2022 08:43:37 +0200 [thread overview]
Message-ID: <99b9a532-5feb-fe00-3d4e-29d560d34dc0@6wind.com> (raw)
In-Reply-To: <f26fa81a-dc13-6a27-2e63-74b13359756e@blackwall.org>
Le 20/09/2022 à 11:05, Nikolay Aleksandrov a écrit :
> 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
Thanks for the link. Finally, someone has (almost) complained :D
> tried searching for open-source buggy users, but didn't find any.
The trend seems to let someone else add another specific flag if needed. Thus,
it seems that checking flags is the way to go.
The pro is that if someone complains, the patch could be reverted, which is not
the case for a new feature like this bulk for example.
Regards,
Nicolas
next prev parent reply other threads:[~2022-09-21 6:43 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-13 10:51 [Bridge] [PATCH net-next v4 00/12] net: bridge: add flush filtering support Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 01/12] net: rtnetlink: add msg kind names Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 02/12] net: rtnetlink: add helper to extract msg type's kind Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 03/12] net: rtnetlink: use BIT for flag values Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 04/12] net: netlink: add NLM_F_BULK delete request modifier Nikolay Aleksandrov
2022-09-20 7:49 ` Nicolas Dichtel
2022-09-20 9:05 ` Nikolay Aleksandrov
2022-09-21 6:43 ` Nicolas Dichtel [this message]
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 05/12] net: rtnetlink: add bulk delete support flag Nikolay Aleksandrov
2022-04-13 12:06 ` Ido Schimmel
2022-04-13 12:21 ` Nikolay Aleksandrov
2022-04-14 0:42 ` David Ahern
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 06/12] net: add ndo_fdb_del_bulk Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 07/12] net: rtnetlink: add NLM_F_BULK support to rtnl_fdb_del Nikolay Aleksandrov
2022-04-13 12:20 ` Ido Schimmel
2022-04-13 12:21 ` Nikolay Aleksandrov
2022-04-13 12:35 ` Ido Schimmel
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 08/12] net: bridge: fdb: add ndo_fdb_del_bulk Nikolay Aleksandrov
2022-04-13 10:51 ` [Bridge] [PATCH net-next v4 09/12] net: bridge: fdb: add support for fine-grained flushing Nikolay Aleksandrov
2022-04-13 10:52 ` [Bridge] [PATCH net-next v4 10/12] net: rtnetlink: add ndm flags and state mask attributes Nikolay Aleksandrov
2022-04-13 10:52 ` [Bridge] [PATCH net-next v4 11/12] net: bridge: fdb: add support for flush filtering based on ndm flags and state Nikolay Aleksandrov
2022-04-13 10:52 ` [Bridge] [PATCH net-next v4 12/12] net: bridge: fdb: add support for flush filtering based on ifindex and vlan Nikolay Aleksandrov
2022-04-13 11:50 ` [Bridge] [PATCH net-next v4 00/12] net: bridge: add flush filtering support patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=99b9a532-5feb-fe00-3d4e-29d560d34dc0@6wind.com \
--to=nicolas.dichtel@6wind.com \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=idosch@idosch.org \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=razor@blackwall.org \
--cc=roopa@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).