[RFC] extract seqno from sta->last_seq_ctrl

[Michal Kazior]

On 27 July 2014 19:51, Denton Gentry <denton.gentry at gmail.com> wrote:
> [Sending to ath10k list as I think ath10k is the only impacted driver right now]
>
> I believe there is a minor problem in
> [PATCH v2] mac80211: add support for Rx reordering offloading
> http://www.spinics.net/lists/linux-wireless/msg124812.html
>
> sta->last_seq_ctrl is the seq_ctrl field from the last seen packet. To
> get the sequence number it needs to be byteswapped and shifted 4 bits;
> patch below.

Good catch!


> This mostly impacts Windows clients, because Windows sends 100-200
> frames before sending ADDBA. This results in a starting seqno which is
> quite large due to the 4 bit shift, and the ieee80211_sn_less() check
> at the top of ieee80211_sta_manage_reorder_buf drops subsequent frames
> until the sequence number catches up.
>
>
> From d0647db58d13533e5441c07f95b23e912976c87b Mon Sep 17 00:00:00 2001
> From: Denton Gentry <denton.gentry at gmail.com>
> Date: Sun, 27 Jul 2014 08:24:36 -0700
> Subject: [PATCH] extract seqno from sta->last_seq_ctrl

You're missing "mac80211: " prefix. Also this message doesn't seem to
tell much. I'd probably entitle it "mac80211: fix start_seq_num in Rx
reorder offload" and then follow that with details (what/why).


>
> This depends on:
> * [PATCH v2] mac80211: add support for Rx reordering offloading

This shouldn't be in the commit log.


>
> sta->last_seq_ctrl is the seq_ctrl field from the last
> header seen, need to byteswap and shift it 4 bits to
> extract the sequence number.
>
>  net/mac80211/iface.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index 77cd025..1903220 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -1171,7 +1171,7 @@ static void ieee80211_iface_work(struct work_struct *work)
>                         mutex_lock(&local->sta_mtx);
>                         sta = sta_info_get_bss(sdata, rx_agg->addr);
>                         if (sta) {
> -                               u16 last_seq = sta->last_seq_ctrl[rx_agg->tid];
> +                               u16 last_seq =
> le16_to_cpu(sta->last_seq_ctrl[rx_agg->tid]) >> 4;

The byteswap is already in place (Johannes fixed it).

The bitshift should be done with IEEE80211_SEQ_TO_SN macro.

Once you fix the commit log and rebase your patch you should submit
this to linux-wireless mailing list.


Michał