Wed, May 06, 2020 at 01:33:27PM CEST, ecree@xxxxxxxxxxxxxx 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
