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]

Date:   Thu, 21 May 2020 17:36:12 +0300
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Edward Cree <ecree@...arflare.com>, xiyou.wangcong@...il.com
Cc:     Vlad Buslov <vladbu@...lanox.com>, netdev@...r.kernel.org,
        davem@...emloft.net, jhs@...atatu.com, jiri@...nulli.us,
        dcaratti@...hat.com, marcelo.leitner@...il.com, kuba@...nel.org
Subject: Re: [PATCH net-next v2 0/4] Implement classifier-action terse dump mode

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
fl_dump_key() while ACT_SKIP_OPTIONS would skip outputting TCA_OPTIONS
compound attribute (and expensive call to tc_action_ops->dump()). This
approach would also leave the door open for further more fine-grained
flags, if the need arises. For example, new flags
CLS_SKIP_KEY_{L2,L3,L4} can be introduced to more precisely control
which parts of cls key should be skipped.

The main drawback of such approach is that it is impossible to come up
with universal set of flags that would be applicable for all
classifiers. Key (in some form) is applicable to most classifiers, but
it still doesn't make sense for matchall or bpf. Some classifiers have
'flags', some don't. Hardware-offloaded classifiers have in_hw_count.
Considering this, initial set of flags will be somewhat flower-centric.

What do you think?

Regards,
Vlad