Re: [PATCH wireless-next v2 09/16] wifi: mac80211: Define layouts for SMD BSS Transition context

[Pooventhiran G <pooventhiran.g@oss.qualcomm.com>]

On 10/02/2026 01:39 am, Johannes Berg wrote:
> [dropping CC's, I have no idea why you CC'ed hardening?!?]
> 
> On Thu, 2026-09-24 at 08:10 +0530, Pooventhiran G wrote:
>> IEEE P802.11bn/D2.0, Aug 2026, subclause 37.16.9, defines the context
>> data to be transported during SMD BSS Transition (ST) laid out in
>> subclause 37.16. The station's context data is attached to the ST
>> Preparation or Execution Request action frames so that userspace
>> receives the context along with the relevant frame.
>>
>> Define the context data: per-TID sequence numbers (SN) in downlink (DL)
>> and uplink (UL) directions, packet number (PN) in DL, per-TID packet
>> numbers in UL, and per-TID BlockAck session parameters in DL and UL;
>> along with this, define an optional driver context. This data is used by
>> drivers to report the context along with the frame SKB.
> 
> 
> Just a couple of _very_ quick comments, having imported this to my tree
> to compare notes with the client side implementation I'm working on:
> 
>> Signed-off-by: Pooventhiran G <pooventhiran.g@oss.qualcomm.com>
>> ---
>>  include/linux/ieee80211-uhr.h | 86 +++++++++++++++++++++++++++++++++++++++++++
> 
> This isn't really mac80211 (commit subject), but I also don't think this
> should be defined here at all. I mean, I get why - the spec defines what
> needs to be transferred, but it doesn't actually define the _layout_ of
> the data and it's never used over the air, so I think it'd be confusing
> to have it here anyway.
> 
> Probably should define it in cfg80211.h instead, since it's used all the
> way through nl80211 down to the driver(s)?
> 

Sure, Johannes. I will move this to cfg80211.

>>  1 file changed, 86 insertions(+)
>>
>> diff --git a/include/linux/ieee80211-uhr.h b/include/linux/ieee80211-uhr.h
>> index c3f87d4c8bec..e6aaef9ae9e6 100644
>> --- a/include/linux/ieee80211-uhr.h
>> +++ b/include/linux/ieee80211-uhr.h
>> @@ -649,6 +649,92 @@ struct ieee80211_uhr_mode_change_tuple {
>>  	u8 variable[];
>>  } __packed;
>>  
>> +/*
>> + * Context information carried in SMD BSS Transition (refer IEEE P802.11bn/D2.0,
>> + * Aug 2026, subclause 37.16.9.
>> + */
>> +#define IEEE80211_SMD_CTX_NUM_VALID_CTX    8
> 
> Why is there such an arbitrary limitation?
> 

It was defined for the BITMAPs usage but I will clean this up as I will
replace BITMAP with individual valid:1 bitfields.

>> +/**
>> + * struct ieee80211_smd_ctx_ba - BlockAck parameters for DL and UL
>> + *
>> + * @amsdu_supported: Peer's capability to support A-MSDU within A-MPDU.
>> + * @ba_policy: BlockAck policy (0 = delayed BlockAck, 1 = immediate BlockAck)
>> + * @buffer_size: Reorder buffer size from ADDBA Request (10-bit, max 1023)
>> + * @timeout: BlockAck session timeout
>> + * @ext_no_frag: ADDBA Extension fragmentation support
>> + * @extfrag_level: ADDBA Extension HE fragmentation level
>> + * @ext_buffer_size: ADDBA Extension buffer size; combined with @buffer_size as
>> + *	(@ext_buffer_size << 10 | @buffer_size) to get the full reorder
>> + *	buffer size
>> + */
>> +struct ieee80211_smd_ctx_ba {
>> +	bool amsdu_supported;
>> +	u8 ba_policy;
>> +	u16 buffer_size;
>> +	u16 timeout;
>> +	bool ext_no_frag;
>> +	u8 extfrag_level;
>> +	u16 ext_buffer_size;
>> +};
> 
> What about WinStartO?
> 
> Though I honestly lost track of where the spec is going with all the
> options of transfer/not transfer - need to try to catch up next week.
> 

In ath12k implementation, WinStartO functionality to not exceed the BA window
on the target AP MLD is achieved differently. But I will translate that to
using WinStartO so that it is more generic.

>> +/**
>> + * struct ieee80211_smd_ctx - IEEE 802.11bn SMD Roaming Context (refer
>> + *	IEEE P802.11bn/D2.0, Aug 2026, subclause 37.16.9)
>> + *
>> + * @valid_ctx_bmap: Bitmap indicating which context fields are valid;
>> + *	bit positions defined by IEEE80211_SMD_CTX_VALID_* constants
>> + * @pn_len: Length of PN in bytes; varies by cipher type
>> + *	(e.g. CCMP (6), GCMP-256 (16))
>> + * @dl: Down-link context data
>> + * @dl.valid_tid_bmap: valid DL TIDs for which context is present
>> + * @dl.sn: DL SN per-TID to be assigned next
>> + * @dl.pn: DL PN to be assigned next
>> + * @dl.ba: DL BlockAck parameters per-TID for the BlockAck session
>> + * @ul: Up-link context data
>> + * @ul.valid_tid_bmap: valid UL TIDs for which context is present
>> + * @ul.sn: UL SN per-TID to be checked next
>> + * @ul.pn: UL PN per-TID to be checked next
>> + * @ul.ba: UL BlockAck parameters per-TID for the BlockAck session
>> + * @drv_ctx_size: Number of valid bytes in @drv_ctx.
>> + * @drv_ctx: Variable-sized array of driver-specific context, counted by
>> + *	@drv_ctx_size. Opaque to the wireless core; interpreted by drivers.
>> + */
>> +struct ieee80211_smd_ctx {
>> +	DECLARE_BITMAP(valid_ctx_bmap, IEEE80211_SMD_CTX_NUM_VALID_CTX);
> 
> This seems overly complex for ... 4 bits? Could just have individual
> valid:1 bitfield entries or something?
> 

Yeah, I will change valid_ctx_bmap to bitfields.

>> +	u8 pn_len;
>> +
>> +	struct {
>> +		DECLARE_BITMAP(valid_tid_bmap, IEEE80211_SMD_CTX_NUM_TIDS);
>> +		u16 sn[IEEE80211_SMD_CTX_NUM_TIDS];
> 
> Aren't there more counters, e.g. for mgmt frames?
> 

Since std does not define it explicitly, ath12k implementation transfers this
in the driver data separately. But I will move this to proper definitions
since mgmt queues need to continue anyways.

>> +		u8 pn[IEEE80211_SMD_CTX_MAX_PN_LEN];
>> +		struct ieee80211_smd_ctx_ba ba[IEEE80211_SMD_CTX_NUM_TIDS];
>> +	} dl;
> 
> It feels like there should be some information here about "how many
> frames are buffered for each TID" and mac80211 could increment that by
> however many are buffered there, both to give the AP an ability to
> estimate the downlink drain time, as well as how much to bump the PN
> forward to by? Although the latter could just be like 1 million and
> nobody cares.
> 

Estimating the draining period is a work-in-progress and this series (and its
corresponding ath12k implementation) does not support it. Current
implementation works with a static configuration. I will add the required
changes as part upstreaming the dynamic estimation support.

Reg the PN offset, based on the below std language (subclause 37.16.9), the DL
SN and PN getting transferred here are already offset to reserve some space
for draining. I will add the required documentation to establish that.

`
NOTE 1—When the current AP MLD determines the next SN to be assigned for DL
individually addressed Data frames of each TID, the current AP MLD accounts
for some reserved set of SNs so that any DL MSDUs of that TID from the DS to
the current AP for the non-AP MLD can still be assigned SN that is less than
the “Next SN”. This reserved set of SNs also depends on when the “Next SN” is
sent to the target AP MLD—i.e., during ST preparation or during ST execution.
If the former, then more SNs need to be reserved because new DL MSDUs may keep
arriving the current AP MLD from the DS during the preparation timeout.

NOTE 2—When the current AP MLD determines the starting PN to be assigned for
DL individually addressed frames by the target AP MLD, the current AP MLD
selects a starting PN high enough such that the PN values used for the DL
frames from the target AP MLD will not repeat any PN values that were used by
the current AP MLD.
`

>> +	struct {
>> +		DECLARE_BITMAP(valid_tid_bmap, IEEE80211_SMD_CTX_NUM_TIDS);
> 
> again I think the whole DECLARE_BITMAP is a bit of a contortion when
> really you needed ... a u8? perhaps a u16 or u32 when accounting for
> mgmt? still not longer than the "unsigned long" this results in, and
> seems way more understandable?
> 
> (and wouldn't even have the kernel-doc problem)
> 

Yeah, I will clean up BITMAPs.

>> +		u16 sn[IEEE80211_SMD_CTX_NUM_TIDS];
>> +		u8 pn[IEEE80211_SMD_CTX_NUM_TIDS][IEEE80211_SMD_CTX_MAX_PN_LEN];
> 
> Does this even need to be per TID? I guess I could see that maybe it
> must be, but if so you forgot mgmt frames here too?
> 

Yes, UL PN is maintained per TID. I will add proper definitions for UL mgmt
information as well.

> I'd also love for you to think about hwsim supporting this - in that
> case mac80211 maintains a lot of this data. Having mac80211 fill in some
> data unconditionally though seems brittle - what if the driver just
> forgot the PN and then mac80211 says "sure the PN is 0" because
> something, so ideally drivers would somehow say "this should be filled
> in from mac80211 software state" or something.
> 

Yes, it is in the pipeline. I will add hwsim changes as well to this series.
For end-to-end upstream validation, if there is STA SMD support in upstream,
we can leverage that; otherwise, I will have some minimal changes locally just
to support AP testing and submit the AP changes. For hwsim, I will add APIs to
fill in these data that mac80211 maintains.

> johannes