On Tue, Jul 22, 2025 at 01:25:50PM +0200, Johannes Berg wrote: > On Tue, 2025-07-22 at 17:16 +1000, Lachlan Hodges wrote: > > When parsing a TIM element from an S1G AP, ensure we correctly > > parse based on the S1G PPDU TIM PVB format, allowing an S1G STA > > to correctly enter/exit power save states to receive buffered > > unicast traffic. > > I think this isn't quite correct. > > > Also make sure we don't allocate over the > > mac80211 supported S1G PPDU AID count. > > But this specifically caught my eye - how can the client ensure it > doesn't allocate over, since it doesn't even allocate? First I thought > this really belongs into the AP side code (i.e. the other patch), but > you actually did something here: > > [...] > > Which ... seems questionable? At least in terms of message it's not > actually _invalid_ if it's out of this range, in theory S1G allows up to > 8191. Yes, from a client side it is. See comments below. > Also the parsing for S1G only parses a subset of the valid formats, > notably the format that mac80211 can generate. That seems questionable, > unless you expect there never to be another implementation (or the > mac80211 one to never be extended) and you can basically make up your > own standard? > > And even if you _do_ get an AID that's <1600 there's still no guarantee > that the encoding will be in the bitmap format, if only two stations > have traffic the AP might well decide to use the "Single AID" mode? > > So I'm overall a bit confused how all this is meant to work - even if > only partially - because AID<1600 is no guarantee that you can parse the > bitmap, so turning off powersave only for AID>=1600 will not really be > sufficient? This is something we discussed internally, and in retrospect mac80211 should be able to parse all types, even though it may only encode a single type. So actually my reasoning for this is not very good as it was essentially "nobody uses the other encoding modes" but in the future who knows if that will remain true... One of the things I was concerned with was adding a new bitmap specifically for S1G, but in actuality its probably more important to be "correct" and we could probably just use an anonymous union. After verifying with the standard and based on your feedback here is what I propose: For a first implementation, we still won't support page slicing - but this is fine as it is an advertised capability. A new bitmap should be added for S1G (as mentioned, probably a union of some sort with the existing bitmap) that allows us to use the _actual_ S1G AID count, this ensures correct interoperability into the future and is safe due to page slicing being a discrete capabilitity. So as per figure 9-214, we can still use a page slice number of 31 which states that the entire page indicated by the page index is encoded in the PVB, but given we now have a correctly sized AID bitmap we would correctly determine the page index. On the AP side, we would encode using block bitmap but correctly indicate the page index. On the STA side, we'd properly decode the PVB with the ability to decode all possible formats. So this would consist of block bitmap, single AID, OLB and ADE alongside their equivalent inverts. One concern I have is that without page slicing, we are limited to a TIM of size 255. A single page represents 2048 AIDs (page index = 2 bits), using block bitmap encoding we can overflow given worst case scenario. Now the easy answer is to probably encode until we hit the maximum length, allow STAs to clear their bit after the beacon and repeat. Though maybe this isn't the nicest? Not sure. Obviously this is an extreme case and would (hopefully) never happen in the real wordl but still needs to be considered. Internally, we default to block bitmap encoding and this isn't modified dynamically, same goes for most (if not all) vendor implementations - but as mentioned the client needs the ability to decode these other formats. Let me know your thoughts. lachlan