[PATCH] wifi: mac80211: Add ADDBA Extension element parsing logics

[Johannes Berg]

On Thu, 2024-10-24 at 10:21 +0800, MeiChia Chiu wrote:
> Add ADDBA Extension element parsing logics in ADDBA request/response
> 
> To support BA size of 1024,
> the ADDBA Extension element is needed in ADDBA request/response.
> Therefore, parsing logics is added for this missing piece.

Ah yes, we had this only for the RX side, I guess.

> +++ b/net/mac80211/agg-tx.c
> @@ -66,10 +66,12 @@ static void ieee80211_send_addba_request(struct ieee80211_sub_if_data *sdata,
>  	struct ieee80211_local *local = sdata->local;
>  	struct sk_buff *skb;
>  	struct ieee80211_mgmt *mgmt;
> +	struct ieee80211_addba_ext_ie *addba_ext;
> +	u8 ext_size = agg_size >= 1024 ? 2 + sizeof(*addba_ext) : 0;
> +	u8 *pos;
>  	u16 capab;
>  
> -	skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom);
> -
> +	skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom + ext_size);

probably much simpler to just make that unconditional?
Like in ieee80211_send_addba_resp().

> +	if (agg_size >= 1024) {
> +		pos = skb_put_zero(skb, ext_size);
> +		*pos++ = WLAN_EID_ADDBA_EXT;
> +		*pos++ = sizeof(struct ieee80211_addba_ext_ie);
> +		addba_ext = (struct ieee80211_addba_ext_ie *)pos;
> +		addba_ext->data = u8_encode_bits(agg_size >> IEEE80211_ADDBA_EXT_BUF_SIZE_SHIFT,
> +						 IEEE80211_ADDBA_EXT_BUF_SIZE_MASK);
> +	}

Maybe we can reuse ieee80211_add_addbaext()?

Also you only described "parsing" in the commit message, so this isn't
really part of it ;-) Please complete the commit message.

> @@ -460,8 +471,11 @@ static void ieee80211_send_addba_with_timeout(struct sta_info *sta,
>  	sta->ampdu_mlme.addba_req_num[tid]++;
>  	spin_unlock_bh(&sta->lock);
>  
> -	if (sta->sta.deflink.he_cap.has_he) {
> +	if (sta->sta.deflink.eht_cap.has_eht) {
>  		buf_size = local->hw.max_tx_aggregation_subframes;
> +	} else if (sta->sta.deflink.he_cap.has_he) {
> +		buf_size = min_t(u16, local->hw.max_tx_aggregation_subframes,
> +				 IEEE80211_MAX_AMPDU_BUF_HE);
>  	} else {

That's related, but not precisely part of what you describe in the
commit message. Just make the commit message more general, I guess, such
as "support EHT 1024 aggregation size in TX" or so?

> @@ -970,6 +986,25 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local,
>  	amsdu = capab & IEEE80211_ADDBA_PARAM_AMSDU_MASK;
>  	tid = u16_get_bits(capab, IEEE80211_ADDBA_PARAM_TID_MASK);
>  	buf_size = u16_get_bits(capab, IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK);
> +	ext_elem_len = len - offsetof(struct ieee80211_mgmt,
> +				      u.action.u.addba_resp.variable);
> +
> +	if (ext_elem_len) {
> +		elems = ieee802_11_parse_elems(mgmt->u.action.u.addba_resp.variable,
> +					       ext_elem_len, true, NULL);
> +
> +		if (elems && !elems->parse_error) {
> +			if (sta->sta.deflink.eht_cap.has_eht && elems->addba_ext_ie) {
> +				u8 buf_size_1k = u8_get_bits(elems->addba_ext_ie->data,
> +							     IEEE80211_ADDBA_EXT_BUF_SIZE_MASK);
> +				buf_size |=
> +					((u16)buf_size_1k) << IEEE80211_ADDBA_EXT_BUF_SIZE_SHIFT;
> +			}
> +		}
> +
> +		kfree(elems);
> +	}

This is all also very similar to ieee80211_process_addba_request(), so
again perhaps it could be reused - would have to pass 'ext_elem_len' and
the buffer to a function.

johannes