TX A-mpdu (without the 'A')

[Eugene Krasnikov]

I do remember i had the same problem that you see that only one packet
is sent per BA. I do not think i was able to fix this ever. But i do
remember that wcn36xx can receive more than one packet per BA, so the
problem is with TX a-mpdu.

When i was investigating this problem i had 3 theories what can be wrong:
1) First one that wcn36xx_dxe_tx_frame is sending packets after
packets instead of queuing them into the ring and then tell the
hardware to send all the packets in the ring.
2) Second theory was that we miss some flag in wcn36xx_tx_bd that will
tell hardware that this is A-MPDU
3) And finally last one is that wcn36xx does not configure HW in a
proper way when calling config_bss/config_sta/update_cfg/add_ba and so
on.

The easiest theory to check is probably number 3 because it's not a
data path so you can just dump SMD commands and compare them with
prima. First two theories are kind of difficult to debug because they
on the data path but i am sure you can find the fix for this issue!
Good luck:)


2014-02-27 15:55 GMT+00:00 Bob Copeland <me at bobcopeland.com>:
> Hi all,
>
> I'm picking up where Chun-Yeow left off, looking at TX ampdu for
> wcn36xx.  Has anyone had success with the latest driver?  Here's
> what I see:
>
>  - I'm testing with wcn36xx as a STA and ath9k as an AP with hostapd,
>    running TCP iperf from the station to the AP, and capturing
>    over the air with a third device.
>
>  - after RX BA session established, wcn36xx will start the TX
>    BA session, and the action frame exchange looks correct
>    (though see below with regard to start seq num)
>
>  - after a while wcn36xx will start sending a-mpdus, but there is
>    only ever one mpdu per a-mpdu.  That is, there will be only
>    one frame, then a BA, then another frame, another BA, etc.
>    BA bitmap is all-ones and ssn is last_frame-63, and the A-mpdu
>    reference numbers in radiotap change for each frame.
>
> Conversely, with prima, I see aggregates of 12 or so frames in the
> same setup.
>
> What I have tried so far:
>
>  - putting IEEE80211_TX_CTL_AMPDU frames on a queue and sending them
>    from a workqueue later in order to send more to the hardware at
>    once (printk shows I can queue 50+ frames this way)
>
>  - futzing with only setting end-of-packet on the last frame in an
>    aggregate; although I decided after more capturing and reading the
>    code that prima and unmodified wcn36xx behave the same in this regard
>    (one small difference: the valid bit on the BD DXE descriptor is only
>    assigned after the SKB one, but I doubt this makes a difference).
>
>  - on Pontus' suggestion, adding the following settings on bss_info_changed
>    (with various values, I still need to go back to prima to see what values
>    it uses).
>
> +    wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_MAX_MPDUS_IN_AMPDU, 64);
> +    wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_BA_THRESHOLD_HIGH, 10);
> +    wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_MAX_BA_SESSIONS, 2);
>
>  - fixing Start Sequence Number in wcn36xx -- I noticed when parsing out
>    the FW response from add-ba-session that SSN didn't match what mac80211
>    had sent.  ssn will be null by the time the TX ampdu is operational,
>    so we would pass '0' to the hardware when mac80211 is really starting
>    with, say, '2'.  This ugly hack got them in sync for at least the one
>    stream test but changed nothing in the operation:
>
> @@ -894,6 +899,7 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
>  {
>         struct wcn36xx *wcn = hw->priv;
>         struct wcn36xx_sta *sta_priv = NULL;
> +       static u16 tx_ssn = 0;
>
>         wcn36xx_dbg(WCN36XX_DBG_MAC, "mac ampdu action action %d tid %d\n",
>                     action, tid);
> @@ -913,10 +919,11 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
>                 wcn36xx_smd_del_ba(wcn, tid, get_sta_index(vif, sta_priv));
>                 break;
>         case IEEE80211_AMPDU_TX_START:
> +               tx_ssn = *ssn;
>                 ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid);
>                 break;
>         case IEEE80211_AMPDU_TX_OPERATIONAL:
> -               wcn36xx_smd_add_ba_session(wcn, sta, tid, ssn, 1,
> +               wcn36xx_smd_add_ba_session(wcn, sta, tid, ssn ? ssn : &tx_ssn, 1,
>                         get_sta_index(vif, sta_priv));
>                 break;
>         case IEEE80211_AMPDU_TX_STOP_FLUSH:
>
>
> The above tests were based on 7c0db3da "Merge pull request #134 from
>  pof2/refactoring" from wcn36xx.  I'll test again with more recent changes and
> do some more captures of prima for comparison purposes, but if anyone has any
> other suggestions or has it working for them, I'd like to hear about it.
> --
> Bob Copeland %% www.bobcopeland.com
>
> _______________________________________________
> wcn36xx mailing list
> wcn36xx at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/wcn36xx



-- 
Best regards,
Eugene