netdev - Re: [PATCH net-next v2 0/4] Implement classifier-action terse dump mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87imgo9c8m.fsf@buslov.dev>
Date:   Fri, 22 May 2020 19:16:09 +0300
From:   Vlad Buslov <vlad@...lov.dev>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Vlad Buslov <vladbu@...lanox.com>,
        Edward Cree <ecree@...arflare.com>, xiyou.wangcong@...il.com,
        netdev@...r.kernel.org, davem@...emloft.net, jhs@...atatu.com,
        jiri@...nulli.us, dcaratti@...hat.com, marcelo.leitner@...il.com
Subject: Re: [PATCH net-next v2 0/4] Implement classifier-action terse dump mode

On Thu 21 May 2020 at 20:02, Jakub Kicinski <kuba@...nel.org> wrote:
> On Thu, 21 May 2020 17:36:12 +0300 Vlad Buslov wrote:
>> Hi Edward, Cong,
>> 
>> On Mon 18 May 2020 at 18:37, Edward Cree <ecree@...arflare.com> wrote:
>> > On 15/05/2020 12:40, Vlad Buslov wrote:  
>> >> In order to
>> >> significantly improve filter dump rate this patch sets implement new
>> >> mode of TC filter dump operation named "terse dump" mode. In this mode
>> >> only parameters necessary to identify the filter (handle, action cookie,
>> >> etc.) and data that can change during filter lifecycle (filter flags,
>> >> action stats, etc.) are preserved in dump output while everything else
>> >> is omitted.  
>> > I realise I'm a bit late, but isn't this the kind of policy that shouldn't
>> >  be hard-coded in the kernel?  I.e. if next year it turns out that some
>> >  user needs one parameter that's been omitted here, but not the whole dump,
>> >  are they going to want to add another mode to the uapi?
>> > Should this not instead have been done as a set of flags to specify which
>> >  pieces of information the caller wanted in the dump, rather than a mode
>> >  flag selecting a pre-defined set?
>> >
>> > -ed  
>> 
>> I've been thinking some more about this. While the idea of making
>> fine-grained dump where user controls exact contents field-by-field is
>> unfeasible due to performance considerations, we can try to come up with
>> something more coarse-grained but not fully hardcoded (like current terse
>> dump implementation). Something like having a set of flags that allows
>> to skip output of groups of attributes.
>> 
>> For example, CLS_SKIP_KEY flag would skip the whole expensive classifier
>> key dump without having to go through all 200 lines of conditionals in
>
> Do you really need to dump classifiers? If you care about stats 
> the actions could be sufficient if the offload code was fixed
> appropriately... Sorry I had to say that.

Technically I need neither classifier nor action. All I need is cookie
and stats of single terminating action attached to filter (redirect,
drop, etc.). This can be achieved by making terse dump output that data
for last extension on filter. However, when I discussed my initial terse
dump idea with Jamal he asked me not to ossify such behavior to allow
for implementation of offloaded shared actions in future.

Speaking about shared action offload, I remember looking at some RFC
patches by Edward implementing such functionality and allowing action
stats update directly from act, as opposed to current design that relies
on classifier to update action stats from hardware. Is that what you
mean by appropriately fixing offload code? With such implementation,
just dumping relevant action types would also work. My only concern is
that the only way to dump actions is per-namespace as opposed to
per-Qdisc of filters, which would clash with any other cls/act users on
same machine/hypervisor.


[...]