On Wed, Jul 23, 2025 at 09:39:47AM +0200, Johannes Berg wrote: > Yes, they just go to net/mac80211/tests/. You can put something in the > same patch or if you feel that gets too big into a separate patch in the > series, I don't really care too much. I am playing around with them now, so will include them in the next patchset :) > > whereas (and, do correct me if im > > wrong) on the AP side the AID is passed down from hostap via > > NL80211_ATTR_PEER_AID, which is then validated using the > > NLA_POLICY_RANGE where the max is IEEE80211_MAX_AID. So if we were to > > limit the AID to 1600 for S1G, we'd need some way to validate it within > > cfg80211. Don't think this would be too hard, since we can confirm we > > have an S1G station via the presence of the S1G capabilities within > > nl80211_set_station_tdls(): > > > > ... > > if (info->attrs[NL80211_ATTR_S1G_CAPABILITY]) > > params->link_sta_params.s1g_capa = > > nla_data(info->attrs[NL80211_ATTR_S1G_CAPABILITY]); > > ... > > > > where we could maybe perform some extra validation on the AID? I know > > its maybe not the prettiest since we aren't technically using the > > explicit nl80211 intrinsic validation - but I would say it gets the job > > done. > > Ohh! Now I see where you're coming from, I was completely handwaving > away the _practicalities_ of checking the limit in my head. > > I mean, sure, we currently use a policy ... Using the S1G_CAPABILITY > attribute wouldn't work if hostapd isn't including it at that time, but > mac80211 can always also do the validation. Worst case we can even > expose a new max-AID attribute, per interface type. > > If we wanted to extend it later to up to 8191 then we'd need to remove > the IEEE80211_MAX_AID policy anyway since it'd be limiting to much less. > > So I don't really see this as too much an issue, I'd probably just have > added a comment in nl80211.h that it's practically 1600 for S1G for now, > and added validation in mac80211. Then if _later_ we extend it, we can > add a feature flag/feature attribute for "now we support bigger AIDs for > S1G". Sure > > I think what I'll do from here, is wait for your response to my > > comments, mainly on the maximum AID count and any other comments you may > > have, and work on v2 which would roughly consist of the following: > > > > 1. On the AP side, we simply support block bitmap encoding. Patch 1 > > would more or less be the same (with the addition potentially of > > new AID validation) though I'd add some details in there and > > change some comments etc. > > Sure. I write in a comment on the other patch that > IEEE80211_MAX_AID_S1G_NO_PS should probably not be in ieee80211.h, but I > guess with the changes to validation etc. you'd renamed that anyway and > put it in a different place. Or remove it. Yep sorry I did read that (and will respond after this email). > > 2. Support decoding the other encoding modes on the STA side. I think > > while a bit more work upfront, worth it in the long run. > > 3. Add in some kunit tests to tests both the encoding of block bitmap > > and also the decoding of all the formats from Annex L. > > 4. Fix up the AID stuff depending on what you think, If we go with the > > hard limit of 1600, ensure on the AP side we perform that validation. > > I really don't mind either way. Like I said above, it's a bit of a > question of complexity in the AP encoding implementation. Maybe just > start updating the AP encoding implementation and see how hard the TIM > element length validation would be to add to it, vs. a priori knowledge > that it'll always fit? Based on our discussion now (going to read patch #1 email after this) Ill stick to 1600 limit, Ill leave that for response on patch #1 since seems to be more relevent there. lachlan
