Hello Johannes,
Seems current mac80211 A-MPDU reordering implementation don't work
correctly with drivers that prefer to report separate A-MSDU subframes
instead of one huge frame (using RX_FLAG_AMSDU_MORE). Seems there is a
table where index is a sequence number (ampdu sn), so don't work
correctly while we report few A-MSDU frames with the same SN.
We see this problem with ath10k driver. As I understand correctly, we
will have to change/rewrite mac80211 reordering code to support
correctly A-MSDU packets with RX_FLAG_AMSDU_MORE?
Or
Maybe we should report A-MSDU frames as a list to the mac80211 (skb->next set).
Queue such frame like this in mac80211 more or less:
while(skb->next && RX_FLAG_AMSDU_MORE)
__skb_queue_tail(frames, skb);
And dequeue into skb (mpdu) again:
while (RX_FLAG_AMSDU_MORE)
skb->next = __skb_dequeue(frames);
In such case we will have one skb (mpdu) that will contain all amsdu
subframes, so more or less no changes in reordering code will be
required. Insert will be "atomic" while we will add whole mpdu in one
step.
For sure we will have to handle this skb list in
ieee80211_amsdu_to_8023s() + dev_kfree_skb_any() + some more work I
think
What do you think?
As a temporary workaround we build one AMSDU big frame, but this is a
lot of memcp and looks odd when
few skb in driver --> one_big_skb --> amsdu_to_8023s again split this
big frame into skb list (almost the same we already have in the
driver).
BR
Janusz
On 21 July 2014 11:47, Johannes Berg <[email protected]> wrote:
> On Wed, 2014-07-09 at 10:16 +0200, Janusz Dziedzic wrote:
>> Hello Johannes,
>>
>> Seems current mac80211 A-MPDU reordering implementation don't work
>> correctly with drivers that prefer to report separate A-MSDU subframes
>> instead of one huge frame (using RX_FLAG_AMSDU_MORE). Seems there is a
>> table where index is a sequence number (ampdu sn), so don't work
>> correctly while we report few A-MSDU frames with the same SN.
>>
>> We see this problem with ath10k driver. As I understand correctly, we
>> will have to change/rewrite mac80211 reordering code to support
>> correctly A-MSDU packets with RX_FLAG_AMSDU_MORE?
>>
>> Or
>>
[...]
>> What do you think?
>
> Looks like Michal did the former in the meantime? I was moving so was
> out for a while ...
Correct. I've posted `mac80211: fix Rx reordering with
RX_FLAG_AMSDU_MORE` which makes rx reordering work with sk_buff_head
instead of sk_buff *.
Michał
On Wed, 2014-07-09 at 10:16 +0200, Janusz Dziedzic wrote:
> Hello Johannes,
>
> Seems current mac80211 A-MPDU reordering implementation don't work
> correctly with drivers that prefer to report separate A-MSDU subframes
> instead of one huge frame (using RX_FLAG_AMSDU_MORE). Seems there is a
> table where index is a sequence number (ampdu sn), so don't work
> correctly while we report few A-MSDU frames with the same SN.
>
> We see this problem with ath10k driver. As I understand correctly, we
> will have to change/rewrite mac80211 reordering code to support
> correctly A-MSDU packets with RX_FLAG_AMSDU_MORE?
>
> Or
>
> Maybe we should report A-MSDU frames as a list to the mac80211 (skb->next set).
> Queue such frame like this in mac80211 more or less:
> while(skb->next && RX_FLAG_AMSDU_MORE)
> __skb_queue_tail(frames, skb);
>
> And dequeue into skb (mpdu) again:
> while (RX_FLAG_AMSDU_MORE)
> skb->next = __skb_dequeue(frames);
>
> In such case we will have one skb (mpdu) that will contain all amsdu
> subframes, so more or less no changes in reordering code will be
> required. Insert will be "atomic" while we will add whole mpdu in one
> step.
> For sure we will have to handle this skb list in
> ieee80211_amsdu_to_8023s() + dev_kfree_skb_any() + some more work I
> think
>
> What do you think?
Looks like Michal did the former in the meantime? I was moving so was
out for a while ...
johannes