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
