Hi,
Just a quick note -- was testing fragmentation and happened to use ath9k
as the test station, this failed utterly, it sent increasing sequence
numbers and no fragment number.
johannes
Johannes Berg wrote:
> Just a quick note -- was testing fragmentation and happened to use ath9k
> as the test station, this failed utterly, it sent increasing sequence
> numbers and no fragment number.
Sequence number handling is pretty much borked in ath9k.
There is a comment in [email protected] - and it doesn't make much sense.
Removing all crappy code in ath9k dealing with this and leaving everything
to mac80211 would certainly be the better option, but we still have to figure
out how to manage BA windows.
Sujith
On Fri, 2009-03-13 at 07:34 +0530, Sujith wrote:
> Sequence number handling is pretty much borked in ath9k.
> There is a comment in [email protected] - and it doesn't make much sense.
Indeed, that doesn't seem to reflect the code at all.
> Removing all crappy code in ath9k dealing with this and leaving everything
> to mac80211 would certainly be the better option, but we still have to figure
> out how to manage BA windows.
Can you explain a little what is required?
For ampdu_action(), I recently saw something, somewhere, saying
"mac80211 expects us to fill in the seqno variable" or so, but this is
only true if you don't use mac80211's sequence numbers, otherwise it is
fine to leave it at the pre-assigned value.
johannes
Sujith wrote:
> Johannes Berg wrote:
> > Just a quick note -- was testing fragmentation and happened to use ath9k
> > as the test station, this failed utterly, it sent increasing sequence
> > numbers and no fragment number.
>
> Sequence number handling is pretty much borked in ath9k.
> There is a comment in [email protected] - and it doesn't make much sense.
> Removing all crappy code in ath9k dealing with this and leaving everything
> to mac80211 would certainly be the better option, but we still have to figure
> out how to manage BA windows.
>
And a quick hack to fix this would be to not increment the internal sequence
number when the frame is fragmented. But that is what it would be, a hack. :)
Sujith
Johannes Berg wrote:
> > Removing all crappy code in ath9k dealing with this and leaving everything
> > to mac80211 would certainly be the better option, but we still have to figure
> > out how to manage BA windows.
>
> Can you explain a little what is required?
>
Managing per-TID state to handle Block ACKs, failed sub-frames,
sub-frame retries and other window management stuff.
ath9k manages all this internally.
> For ampdu_action(), I recently saw something, somewhere, saying
> "mac80211 expects us to fill in the seqno variable" or so, but this is
> only true if you don't use mac80211's sequence numbers, otherwise it is
> fine to leave it at the pre-assigned value.
ampdu_action() requires the driver to set the starting sequence number
for initiating an ADDBA session, ath9k was filling it incorrectly.
This was fixed in the patch, "ath9k: Fix bug in TX aggregation".
Sujith
On Fri, 2009-03-13 at 16:18 +0530, Sujith wrote:
> Managing per-TID state to handle Block ACKs, failed sub-frames,
> sub-frame retries and other window management stuff.
> ath9k manages all this internally.
Ok. I'd have to look into it to understand what we could do there.
> > For ampdu_action(), I recently saw something, somewhere, saying
> > "mac80211 expects us to fill in the seqno variable" or so, but this is
> > only true if you don't use mac80211's sequence numbers, otherwise it is
> > fine to leave it at the pre-assigned value.
>
> ampdu_action() requires the driver to set the starting sequence number
> for initiating an ADDBA session, ath9k was filling it incorrectly.
> This was fixed in the patch, "ath9k: Fix bug in TX aggregation".
Yes, but it only requires it to do that if the driver assigns sequence
numbers, if it relies on mac80211 then it doesn't need to touch the
value :)
johannes