rtl8xxxu does not handle ampdu_action properly

rtl8xxxu does not handle ampdu_action properly

[Anssi Hannula]

Hi!

Just a quick note that I noticed that rtl8xxxu does not seem to handle
ampdu_action properly.

The driver should react to a IEEE80211_AMPDU_TX_START by either calling
ieee80211_start_tx_ba_cb_irqsafe() or by returning
IEEE80211_AMPDU_TX_START_IMMEDIATE [1] or by just returning a failure
(ath10k, ath11k, wlcore do so for "fw-managed" case), but rtl8xxxu
simply sets ampdu_factor and ampdu_density via register writes and then
returns 0.
I didn't look through the agg-tx code thoroughly on what the end effect
of this is but looks like at least ieee80211_agg_tx_operational() will
never be called.

There is a similar issue with IEEE80211_AMPDU_TX_STOP_CONT and
ieee80211_stop_tx_ba_cb_irqsafe().

Quick grepping suggests all other drivers handle these properly.

[1]
https://www.kernel.org/doc/html/latest/driver-api/80211/mac80211-advanced.html?highlight=mac80211#c.ieee80211_ampdu_mlme_action

-- 
Anssi Hannula / Bitwise Oy
+358 503803997

Re: rtl8xxxu does not handle ampdu_action properly

[Anssi Hannula]

On 13.2.2020 12.41, Anssi Hannula wrote:
> Hi!
>
> Just a quick note that I noticed that rtl8xxxu does not seem to handle
> ampdu_action properly.
>
> The driver should react to a IEEE80211_AMPDU_TX_START by either calling
> ieee80211_start_tx_ba_cb_irqsafe() or by returning
> IEEE80211_AMPDU_TX_START_IMMEDIATE [1] or by just returning a failure
> (ath10k, ath11k, wlcore do so for "fw-managed" case), but rtl8xxxu
> simply sets ampdu_factor and ampdu_density via register writes and then
> returns 0.
> I didn't look through the agg-tx code thoroughly on what the end effect
> of this is but looks like at least ieee80211_agg_tx_operational() will
> never be called.

Looking slightly further, there is a commented-out check for
(tx_info->flags & IEEE80211_TX_CTL_AMPDU) in rtl8xxxu_tx(). Commenting
that out may be working around (by always enabling ampdu?) the issue of
ieee80211_agg_tx_operational() call never having been made.

> There is a similar issue with IEEE80211_AMPDU_TX_STOP_CONT and
> ieee80211_stop_tx_ba_cb_irqsafe().
>
> Quick grepping suggests all other drivers handle these properly.
>
> [1]
> https://www.kernel.org/doc/html/latest/driver-api/80211/mac80211-advanced.html?highlight=mac80211#c.ieee80211_ampdu_mlme_action
>


-- 
Anssi Hannula / Bitwise Oy
+358 503803997