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: <20200521100214.700348e5@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 21 May 2020 10:02:14 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vlad Buslov <vladbu@...lanox.com>
Cc:     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 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.

> 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.

L2, L3, etc. will be meaningless for a lot of classifiers.

> 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?

Simplest heuristic is to dump everything that can't get changed without
a notification. Which I think you're quite close to already..