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  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:   Wed, 6 May 2020 12:33:27 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Pablo Neira Ayuso <pablo@...filter.org>
CC:     <netfilter-devel@...r.kernel.org>, <davem@...emloft.net>,
        <netdev@...r.kernel.org>, <jiri@...nulli.us>
Subject: Re: [PATCH net,v2] net: flow_offload: skip hw stats check for
 FLOW_ACTION_HW_STATS_DONT_CARE

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

Powered by blists - more mailing lists