On Thu, Jul 04, 2019 at 02:21:52PM +0200, Johannes Berg wrote: > On Thu, 2019-07-04 at 14:17 +0200, Michal Kubecek wrote: > > On Thu, Jul 04, 2019 at 02:03:02PM +0200, Johannes Berg wrote: > > > On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote: > > > > > > > > There is still the question if it it should be implemented as a nested > > > > attribute which could look like the current compact form without the > > > > "list" flag (if there is no mask, it's a list). Or an unstructured data > > > > block consisting of u32 bit length > > > > > > You wouldn't really need the length, since the attribute has a length > > > already :-) > > > > It has byte length, not bit length. The bitmaps we are dealing with > > can have any bit length, not necessarily multiples of 8 (or even 32). > > Not sure why that matters? You have the mask, so you don't really need > to additionally say that you're only going up to a certain bit? > > I mean, say you want to set some bits <=17, why would you need to say > that they're <=17 if you have a > value: 0b00000000'000000xx'xxxxxxxx'xxxxxxxx > mask: 0b00000000'00000011'11111111'11111111
One scenario that I can see from the top of my head would be user running ethtool -s <dev> advertise 0x... with hex value representing some subset of link modes. Now if ethtool version is behind kernel and recognizes fewer link modes than kernel but in a way that the number rounded up to bytes or words would be the same, kernel has no way to recognize of those zero bits on top of the mask are zero on purpose or just because userspace doesn't know about them. In general, I believe the absence of bit length information is something protocols would have to work around sometimes. The submitted implementation doesn't have this problem as it can tell kernel "this is a list" (i.e. I'm not sending a value/mask pair, I want exactly these bits to be set). Thus it can easily implement requests of both types (value/mask or just value): ethtool -s <dev> advertise 0x2f ethtool -s <dev> advertise 0x08/0x0c ethtool -s <dev> advertise 100baseT/Full off 1000baseT/Full on and could be as easily extended to support also ethtool -s <dev> advertise 100baseT/Full 1000baseT/Full Michal
