netdev - Re: [PATCH net,v2] net: flow_offload: skip hw stats check for FLOW_ACTION_HW_STATS_DONT

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] [day] [month] [year] [list]

Message-ID: <20200506114417.GB2269@nanopsycho.orion>
Date:   Wed, 6 May 2020 13:44:17 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Edward Cree <ecree@...arflare.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        netfilter-devel@...r.kernel.org, davem@...emloft.net,
        netdev@...r.kernel.org
Subject: Re: [PATCH net,v2] net: flow_offload: skip hw stats check for
 FLOW_ACTION_HW_STATS_DONT_CARE

Wed, May 06, 2020 at 01:33:27PM CEST, ecree@...arflare.com wrote:
>On 06/05/2020 00:29, Jakub Kicinski wrote:
>> IIRC we went from the pure bitfield implementation (which was my
>> preference) to one where 0 means disabled.
>>
>> Unfortunately we ended up with a convoluted API where drivers have to
>> check for magic 0 or 'any' values.
>Yeah, I said something dumb a couple of threads ago and combined the
> good idea (a DISABLED bit) with the bad idea (0 as magic DONT_CARE
> value), sorry for leading Pablo on a bit of a wild goose chase there.
>(It has some slightly nice properties if you're trying to write out-of-
> tree drivers that work with multiple kernel versions, but that's never
> a good argument for anything, especially when it requires a bunch of
> extra code in the in-tree drivers to handle it.)
>
>> On Tue, 5 May 2020 23:43:21 +0200 Pablo Neira Ayuso wrote:
>>> And what is the semantic for 0 (no bit set) in the kernel in your
>>> proposal?
>It's illegal, the kernel never does it, and if it ever does then the
> correct response from drivers is to say "None of the things I can
> support (including DISABLED) were in the bitmask, so -EOPNOTSUPP".
>Which is what drivers written in the natural way will do, for free.
>
>>> Jiri mentioned there will be more bits coming soon. How will you
>>> extend this model (all bit set on for DONT_CARE) if new bits with
>>> specific semantics are showing up?
>If those bits are additive (e.g. a new type like IMMEDIATE and

They are additive.


> DISABLED), then all-bits-on works fine.  If they're orthogonal flags,
> ideally there should be two bits, one for "flag OFF is acceptable"
> and one for "flag ON is acceptable", that way 0b11 still means don't
> care.  And 0b00 gets EOPNOTSUPP regardless of the rest of the bits.
>
>>> Combining ANY | DISABLED is non-sense, it should be rejected.
>It's not nonsense; it means what it says ("I accept any of the modes
> (which enable stats); I also accept disabled stats").
>
>-ed