Bugzilla – Bug 2307
m_receivedAtLeastOneMpdu is not always reset
Last modified: 2016-05-13 05:30:23 UTC
Created attachment 2291 [details] Proposed patch When RTS/CTS and A-MPDU are enabled, sometimes RTS is received, answered by CTS and followed by A-MPDU without any MPDU's successfully received. After that, the following assertion is triggered: http://code.nsnam.org/ns-3-dev/file/028452e3b558/src/wifi/model/mac-low.cc#l846 I checked that m_lastReceivedHdr corresponds to RTS in my case. On reception of RTS ReceiveOk sets m_lastReceivedHdr but leaves m_receivedAtLeastOneMpdu unchanged. Patch to reset m_receivedAtLeastOneMpdu in the corresponding branch is attached. It fixes things in my case, but maybe other branches should be checked or the code should be refactored somehow so it is easy to check that m_receivedAtLeastOneMpdu is maintained properly in all cases.
This change should not hurt, but I am a bit astonished because normally this should have been reset before sending the RTS frame. What happened just before the RTS? Which frame was received? Feel free to upload traces or a test case.
Additional question: is it 802.11n or 802.11ac?
(In reply to sebastien.deronne from comment #2) > Additional question: is it 802.11n or 802.11ac? ac with ShortGuardEnabled
Do you observe a single VHT MPDU before the issue is occurring? I will add another proposal to fix the bug, I suspect a mistake that might happen in case of a single VHT MPDU.
Created attachment 2293 [details] Other proposal that might fix the bug Could you please check if this fixes the problem?
(In reply to sebastien.deronne from comment #5) > Created attachment 2293 [details] > Other proposal that might fix the bug > > Could you please check if this fixes the problem? Just checked, it does not fix the problem.
Could you please add a test case and/or traces?
(In reply to sebastien.deronne from comment #7) > Could you please add a test case and/or traces? I have some custom applications, need to reduce it down to testcase. I will log more MacLow traces and update bugreport once I figure out why m_receivedAtLeastOneMpdu is set to true at the moment of receiving RTS.
(In reply to Alexander Krotov from comment #8) > (In reply to sebastien.deronne from comment #7) > > Could you please add a test case and/or traces? > > I have some custom applications, need to reduce it down to testcase. > I will log more MacLow traces and update bugreport once I figure out why > m_receivedAtLeastOneMpdu is set to true at the moment of receiving RTS. Thanks, it will be more helpful.
(In reply to sebastien.deronne from comment #9) > (In reply to Alexander Krotov from comment #8) > > (In reply to sebastien.deronne from comment #7) > > > Could you please add a test case and/or traces? > > > > I have some custom applications, need to reduce it down to testcase. > > I will log more MacLow traces and update bugreport once I figure out why > > m_receivedAtLeastOneMpdu is set to true at the moment of receiving RTS. > > Thanks, it will be more helpful. What I figured out so far from MacLow traces and additional logging that I added. On sender, AggregateToAmpdu is called. I output "hdr", it appears to be "CTL_BACKREQ". Normally it is QoS data (QOSDATA), but here it is block ack request. Therefore, this check fails: http://code.nsnam.org/ns-3-dev/file/028452e3b558/src/wifi/model/mac-low.cc#l2975 Ack policy is *not* set to NORMAL_ACK. On the receiver, MacLow::DeaggregateAmpduAndReceive is called. Check fails because normalAck is false: http://code.nsnam.org/ns-3-dev/file/028452e3b558/src/wifi/model/mac-low.cc#l2846 As a result, A-MPDU ends, but this line is not reached when last MPDU is processed: http://code.nsnam.org/ns-3-dev/file/028452e3b558/src/wifi/model/mac-low.cc#l2872 Here is how it looks for ...:01 sending data to ...:03 : [mac=00:00:00:00:00:01] Adding packet with Sequence number 154 to A-MPDU, packet size = 1538, A-MPDU size = 37054 [mac=00:00:00:00:00:01] Adding packet with Sequence number 155 to A-MPDU, packet size = 1538, A-MPDU size = 38598 [mac=00:00:00:00:00:01] Adding packet with Sequence number 156 to A-MPDU, packet size = 1538, A-MPDU size = 40142 [mac=00:00:00:00:00:01] no more packets can be aggregated to satisfy PPDU <= aPPDUMaxTime [mac=00:00:00:00:00:01] tx unicast A-MPDU [mac=00:00:00:00:00:01] startTx size=40142, to=00:00:00:00:00:03, listener=0x2226e70 [mac=00:00:00:00:00:01] MacLow:SendRtsForPacket(0x2225480) [mac=00:00:00:00:00:01] MacLow:ForwardDown(0x2225480, 0x22b6b30, 0x7ffc348ce4b0, mode: VhtMcs0 txpwrlvl: 0 retries: 0 channel width: 80 Short GI: 1 Nss: 1 Ness: 0 MPDU aggregation: 0 STBC: 0) [mac=00:00:00:00:00:01] send CTL_RTS, to=00:00:00:00:00:03, size=20, mode=VhtMcs0, duration=+10062000.0ns, seq=0x830 [mac=00:00:00:00:00:01] MacLow:CtsTimeout(0x2225480) [mac=00:00:00:00:00:01] cts timeout [mac=00:00:00:00:00:01] MacLow:StartTransmission(0x2225480, 0x22e6e70, 0x2227058, [send rts=0, next size=0, dur=+0.0ns, ack=normal], 0x2226e70) [mac=00:00:00:00:00:01] MacLow:CancelAllEvents(0x2225480) [mac=00:00:00:00:00:01] startTx size=1508, to=00:00:00:00:00:03, listener=0x2226e70 [mac=00:00:00:00:00:01] MacLow:SendRtsForPacket(0x2225480) ... It continues to fail RTS transmissions until the limit. Then: [mac=00:00:00:00:00:01] MacLow:SendRtsForPacket(0x2225480) [mac=00:00:00:00:00:01] MacLow:ForwardDown(0x2225480, 0x22acff0, 0x7ffc348ce4b0, mode: VhtMcs0 txpwrlvl: 0 retries: 6 channel width: 80 Short GI: 1 Nss: 1 Ness: 0 MPDU aggregation: 0 STBC: 0) [mac=00:00:00:00:00:01] send CTL_RTS, to=00:00:00:00:00:03, size=20, mode=VhtMcs0, duration=+555000.0ns, seq=0x830 [mac=00:00:00:00:00:01] MacLow:CtsTimeout(0x2225480) [mac=00:00:00:00:00:01] cts timeout [mac=00:00:00:00:00:01] Flush aggregate queue [mac=00:00:00:00:00:01] MacLow:StartTransmission(0x2225480, 0x22fce30, 0x2227058, [send rts=0, next size=0, dur=+0.0ns, ack=compressed-block-ack], 0x2226e70) [mac=00:00:00:00:00:01] MacLow:CancelAllEvents(0x2225480) [mac=00:00:00:00:00:01] Aggregating to CTL_BACKREQ <- My debug message at the beginning of MacLow::AggregateToAmpdu [mac=00:00:00:00:00:01] Adding packet with Sequence number 157 to A-MPDU, packet size = 1538, A-MPDU size = 1542 [mac=00:00:00:00:00:01] Adding packet with Sequence number 158 to A-MPDU, packet size = 1538, A-MPDU size = 3086 [mac=00:00:00:00:00:01] Adding packet with Sequence number 159 to A-MPDU, packet size = 1538, A-MPDU size = 4630 Is it ok that packets are aggregated to block ack request in the first place?
Created attachment 2295 [details] Proposed patch This patch works for me and introduces minimal changes. Just reset m_receivedAtLeastOneAmpdu regardless of ACK policy, by checking for the last A-MPDU subframe.
(In reply to Alexander Krotov from comment #11) > Created attachment 2295 [details] > Proposed patch > > This patch works for me and introduces minimal changes. > > Just reset m_receivedAtLeastOneAmpdu regardless of ACK policy, by checking > for the last A-MPDU subframe. I agree with the patch, thanks for taking care of that. I anyway suggest to add also attachment 2293 [details] which brings some more safety in case of single VHT MPDU, and certainly does not hurt anything.
(In reply to sebastien.deronne from comment #12) > I anyway suggest to add also attachment 2293 [details] which brings some > more safety in case of single VHT MPDU, and certainly does not hurt anything. I applied it and it works too, it is ok as a cleanup patch.
fixed in changeset 11916:5737f7450650
I just found that m_receivedAtLeastOneMpdu is still not maintained properly. While receiving A-MPDU, it is possible that STA synchronizes to someone else's interfering transmission instead of the last MPDU. If interfering transmission has PLCP header, STA may fail to receive it with "drop packet because plcp preamble/header reception failed" message in YansWifiPhy::StartReceivePacket. In this case, MacLow::ReceiveError is not triggered and STA remains in m_receivedAtLeastOneMpdu=true state. This way STA may miss the end of A-MPDU while listening to other transmissions. I discovered it because this line triggered assertion in one of my scripts: http://code.nsnam.org/ns-3-dev/file/80f998db356f/src/wifi/model/mac-low.cc#l864 STA missed the end of A-MPDU and ReceiveError was tirggered for unrelated RTS frame later. Assertion failed because MacLow thought it received RTS a part of A-MPDU. Any ideas how to fix it properly? Looks like the approach of transmitting A-MPDU as many separate MPDUs leads to more and more problems, and that "bool isEndOfFrame" does not help much. Bug #2368 is related, because STA may actually switch to receiving other frame transmission while receiving A-MPDU, need to handle this case properly.
Alexander, could you please attach a test case?
Alexander, any test case available? I suggest to investigate how it is handled in the field to mimic this in the simulator and avoid further issues with this flag. I am currently thinking of a timer that gets triggered when the A-MPDU reception is supposed to be finished.
Created attachment 2406 [details] proposal to get rid of the m_receivedAtLeastOneMpdu flag I made a patch that addresses a cleaner solution in replacement of the m_receivedAtLeastOneMpdu flag. When a first MPDU in an A-MPDU is received, a Block Ack response is scheduled. This removes a lot of complexity we had introduced in our code, and it will definitely fix all bugs related to the m_receivedAtLeastOneMpdu flag. Some more testing should be done to be sure we do not introduce new issues.
I suggest to deliver my solution, so that tests can be run by everybody using ns-3-dev.
I wonder whether the conversion to nanoseconds in the Tag is needed; can a Time object instead be passed to the Tag? Otherwise, I don't have other comments.
Created attachment 2414 [details] proposal 2 to get rid of the m_receivedAtLeastOneMpdu flag I reworked a bit the previous patch so that we use a Time in the tag. I reworked also the tag so that we make clear that the number of MPDUs is the remaining number of MPDUs. I think it is ready to be pushed.
(In reply to sebastien.deronne from comment #21) > Created attachment 2414 [details] > proposal 2 to get rid of the m_receivedAtLeastOneMpdu flag > > I reworked a bit the previous patch so that we use a Time in the tag. > I reworked also the tag so that we make clear that the number of MPDUs is > the remaining number of MPDUs. I think it is ready to be pushed. To serialize Time, call GetTimeStep() to extract the underlying int64_t, then when deserializing, just call the constructor with that int64_t. i.e. do not convert to NanoSeconds in serialization. Otherwise OK for me.
Created attachment 2416 [details] final proposal to get rid of the m_receivedAtLeastOneMpdu flag Included Tom's suggestion
fixed in changeset 12112:ad2fe4acfeea