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
