|
Bugzilla – Full Text Bug Listing |
| Summary: | Sequence Number passed to QosUtilsMapSeqControlToUniqueInteger instead of Sequence Control | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Steven Sultana <sultsteven> |
| Component: | wifi | Assignee: | sebastien.deronne |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | ns-bugs, tomh, tommaso.pecorella |
| Priority: | P3 | ||
| Version: | ns-3-dev | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
Patch to mac-low.*
Updated to include fixes for all instances RxCompleteBufferedPacketsWithSmallerSequence revised version of the patch |
||
Created attachment 2038 [details]
Updated to include fixes for all instances RxCompleteBufferedPacketsWithSmallerSequence
I agree with the fix, and furthermore it appears to fix an important issue encountered when using TCP with A-MPDU. Unless someone does not agree with the proposed patch, I suggest to deliver it before the upcoming release. I agree on pushing it,but it needs one important sub-fix: GetSequenceControl seems to be bugged (!)
In many places in the code, the Starting Sequence Control is used, but it is simply calculated on the fly, e.g.:
uint16_t startingSeqCtrl = ((*it).second.first.GetStartingSequence () << 4) & 0xfff0;
uint16_t guard = startingSeqCtrl;
Now, GetSequenceControl is similar but not identical:
uint16_t seqControl = (m_startingSeq << 4) | 0xfff0;
OR instead of AND... basically GetStartingSequenceControl will always return 0xfff0.
Note that GetStartingSequenceControl is never used in the code (!!) and it should be, in order to avoid small and scattered bugs (e.g., it should have been used in the "uint16_t startingSeqCtrl" setup.
Beside this, I agree that this should be pushed before 3.24, but please triple check the patch behaviour after fixing GetStartingSequenceControl.
Thanks Tommaso. I will work on a revised patch then. Created attachment 2136 [details]
revised version of the patch
I made a revised patch for this issue, with regards to Tommaso's comments.
The fix is quite straightforward, it should be delivered asap so that it can be included in the upcoming release.
(In reply to sebastien.deronne from comment #5) > Created attachment 2136 [details] > revised version of the patch > > I made a revised patch for this issue, with regards to Tommaso's comments. > The fix is quite straightforward, it should be delivered asap so that it can > be included in the upcoming release. Please commit it when you are ready. Tom, for me it is ready, and it improves TCP over A-MPDU. Committed in changeset 11622:d0da0aa55bda |
Created attachment 2026 [details] Patch to mac-low.* The QosUtilsMapSeqControlToUniqueInteger function expects a Sequence Control as the first parameter, but a Sequence Number is passed in MacLow::RxCompleteBufferedPacketsWithSmallerSequence. Patch attached.