Date:   Fri, 30 Sep 2022 17:34:47 +0300
From:   Nikolay Aleksandrov <razor@...ckwall.org>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
        Johannes Berg <johannes@...solutions.net>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Florent Fourcot <florent.fourcot@...irst.fr>,
        Guillaume Nault <gnault@...hat.com>,
        Nicolas Dichtel <nicolas.dichtel@...nd.com>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net-next] docs: netlink: clarify the historical baggage of
 Netlink flags

On 30/09/2022 17:24, Jamal Hadi Salim wrote:
> On Fri, Sep 30, 2022 at 7:29 AM Nikolay Aleksandrov <razor@...ckwall.org> wrote:
>>
>> On 30/09/2022 14:07, Jamal Hadi Salim wrote:
>>> On Wed, Sep 28, 2022 at 10:21 AM Jakub Kicinski <kuba@...nel.org> wrote:
>>>>
>>>> On Wed, 28 Sep 2022 10:03:07 +0300 Nikolay Aleksandrov wrote:
>>>>> The part about NLM_F_BULK is correct now, but won't be soon. I have patches to add
>>>>> bulk delete to mdbs as well, and IIRC there were plans for other object types.
>>>>> I can update the doc once they are applied, but IMO it will be more useful to explain
>>>>> why they are used instead of who's using them, i.e. the BULK was added to support
>>>>> flush for FDBs w/ filtering initially and it's a flag so others can re-use it
>>>>> (my first attempt targeted only FDBs[1], but after a discussion it became clear that
>>>>> it will be more beneficial if other object types can re-use it so moved to a flag).
>>>>> The first version of the fdb flush support used only netlink attributes to do the
>>>>> flush via setlink, later moved to a specific RTM command (RTM_FLUSHNEIGH)[2] and
>>>>> finally settled on the flag[3][4] so everyone can use it.
>>>>
>>>> I thought that's all FDB-ish stuff. Not really looking forward to the
>>>> use of flags spreading but within rtnl it may make some sense. We can
>>>> just update the docs tho, no?
>>>>
>>>> BTW how would you define the exact semantics of NLM_F_BULK vs it not
>>>> being set, in abstract terms?
>>>
>>> I missed the discussion.
>>> NLM_F_BULK looks strange. Why pollute the main header? Wouldnt NLM_F_ROOT
>>> have sufficed to trigger the semantic? Or better create your own
>>> "service header"
>>> and put fdb specific into that object header? i.e the idea in netlink
>>> being you specify:
>>> {Main header: Object specific header: Object specific attributes in TLVs}
>>> Main header should only be extended if you really really have to -
>>> i.e requires much
>>> longer review..
>>>
>>
>> I did that initially, my first submission used netlink attributes specifically
>> for fdbs, but then reviews suggested same functionality is needed for other object types
>> e.g. mdbs, vxlan db, neighbor entries and so on. My second attempt added a new RTM_ msg type that
>> deletes multiple objects, and finally all settled on the flag because all families can
>> make use of it, it modifies the delete op to act on multiple objects. For more context please
>> check the discussions bellow [1] is my first submission (all netlink), [2] is RTM_
>> and [3][4] are the flag:
> 
> I think what you are looking for is a way to either get or delete
> selective objects
> (dump and flush dont filter - they mean "everything"); iow, you send a filtering

They must be able to flush everything, too. Filter matching all/empty filter, we need
it for mdbs and possibly other object types would want that.

> expression and a get/del command alongside it. The filtering
> expression is very specific
> to the object and needs to be specified as such a TLV is appropriate.
> 

Right, and that is what got implemented. The filtering TLVs are bridge and fdb-specific
they don't affect any other subsystem. The BULK flag denotes the delete will
affect multiple objects.

> Really NLM_F_ROOT and _MATCH are sufficient. The filtering expression is
> the challenge.

NLM_F_ROOT isn't usable for a DEL expression because its bit is already used by NLM_F_NONREC
and it wouldn't be nice to change meaning of the bit based on the subsystem. NLM_F_MATCH's bit
actually matches NLM_F_BULK :)

> "functionality is needed for other objects" is a true statement; the question is
> whether we keep hacking into the kernel for these filtering expressions.> An idea i toyed with at one point was to send alongside the netlink
> request a lua
> program that will execute your selector. Then you dont have to keep hacking
> and extending netlink TLVs or adding more to the kernel.
> Lua is very appealing because you dont have to compile anything - and the
> whole program goes into the kernel as a string.
> Second best alternative could be to install a bpf program as your selector and
> invoke it from netlink code to select the entries that get deleted or returned.
> You will have to keep transactional state.
> 

Sometime back I played with a different idea - expressing the filters with the existing TLV objects
so whatever can be specified by user-space can also be used as a filter (also for filtering
dump requests) with some introspection. The lua idea sounds nice though.

> cheers,
> jamal

Powered by blists - more mailing lists