[RFC] extract seqno from sta->last_seq_ctrl

[RFC] extract seqno from sta->last_seq_ctrl

[Denton Gentry]

[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.

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@gmail.com>
Date: Sun, 27 Jul 2014 08:24:36 -0700
Subject: [PATCH] extract seqno from sta->last_seq_ctrl

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

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;

                                __ieee80211_start_rx_ba_session(sta,
                                                0, 0,
-- 
2.0.0.526.g5318336

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [RFC] extract seqno from sta->last_seq_ctrl

[Kalle Valo]

Denton Gentry <denton.gentry@gmail.com> writes:

>>From d0647db58d13533e5441c07f95b23e912976c87b Mon Sep 17 00:00:00 2001
> From: Denton Gentry <denton.gentry@gmail.com>
> Date: Sun, 27 Jul 2014 08:24:36 -0700
> Subject: [PATCH] extract seqno from sta->last_seq_ctrl
>
> This depends on:
> * [PATCH v2] mac80211: add support for Rx reordering offloading
>
> 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.

I'll let Michal comment on the bug, but I cannot do anything with this
patch as it doesn't have the Signed-off-by tag. Can you resend, please?

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [RFC] extract seqno from sta->last_seq_ctrl

[Michal Kazior]

On 27 July 2014 19:51, Denton Gentry <denton.gentry@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@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ł

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k