* [RFC] mac80211: fix AMPDU_TX_START ssn
@ 2009-12-31 19:01 Christian Lamparter
2010-01-03 23:52 ` [PATCH] mac80211: fix ampdu_action tx_start ssn Christian Lamparter
0 siblings, 1 reply; 2+ messages in thread
From: Christian Lamparter @ 2009-12-31 19:01 UTC (permalink / raw)
To: linux-wireless; +Cc: Johannes Berg
The start_seq_num is taken from the station's tid_seq[tid].
(agg-tx.c / line 304). This would be fine, but there is a
small pitfall that we know too well: tid_seq is meant
to produce the sequence, which can be insert into an
IEEE 802.11 frame sequence_control field without
the 4 bit left shift (which is usually necessary to
accommodate for frame fragmenting).
So tid_seq is _increased_ in 0x10 steps, but some of
the code in agg-tx.c doesn't know about that, e.g:
ieee80211_send_addba_request: (line 91)
mgmt->u.action.u.addba_req.start_seq_num =
cpu_to_le16(start_seq_num << 4);
Now, ADDBA recipient certainly gets a wrong BA starting
point and may decide to drop some of the incoming
aggregated frames, since it might think that they
were already too old.
---
Hmm, I'm not sure what to do with the debug_sta.c
counters? Obviously, they are now >> 4 as well,
but on the other hand just like the tid_rx side
always was...
Note: By the looks of it: iwlagn/ath9k are probably
unaffected, but carl9170 was: so no need for stable.
---
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index ceda366..5aa8f4a 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -301,7 +301,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
* call back right away, it must see that the flow has begun */
*state |= HT_ADDBA_REQUESTED_MSK;
- start_seq_num = sta->tid_seq[tid];
+ start_seq_num = sta->tid_seq[tid] >> 4;
ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
pubsta, tid, &start_seq_num);
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH] mac80211: fix ampdu_action tx_start ssn
2009-12-31 19:01 [RFC] mac80211: fix AMPDU_TX_START ssn Christian Lamparter
@ 2010-01-03 23:52 ` Christian Lamparter
0 siblings, 0 replies; 2+ messages in thread
From: Christian Lamparter @ 2010-01-03 23:52 UTC (permalink / raw)
To: linux-wireless; +Cc: linville
The start_seq_num is taken from the station's tid_seq[tid].
This is fine, except tid_seq sequence counter is shifted
by 4 bits to accommodate for frame fragmentation.
Both (iwlagn & ath9k) were unaffected by this minor glitch,
because they don't read the *ssn for the AMPDU_TX_START action.
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index ceda366..5aa8f4a 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -301,7 +301,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
* call back right away, it must see that the flow has begun */
*state |= HT_ADDBA_REQUESTED_MSK;
- start_seq_num = sta->tid_seq[tid];
+ start_seq_num = sta->tid_seq[tid] >> 4;
ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
pubsta, tid, &start_seq_num);
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-01-03 23:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-31 19:01 [RFC] mac80211: fix AMPDU_TX_START ssn Christian Lamparter
2010-01-03 23:52 ` [PATCH] mac80211: fix ampdu_action tx_start ssn Christian Lamparter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).