|
Bugzilla – Full Text Bug Listing |
| Summary: | BlockAckManager::GetNRetryNeededPackets missing some packets in the queue | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Matías Richart <matis18> |
| Component: | wifi | Assignee: | sebastien.deronne |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ns-bugs, tomh |
| Priority: | P3 | ||
| Version: | ns-3.26 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | Patch to increase iterator correctly | ||
(In reply to Matías Richart from comment #0) > Created attachment 2688 [details] > Patch to increase iterator correctly > > The method BlockAckManager::GetNRetryNeededPackets misses to count some > packets of the m_retryPackets queue. > The problem is that in some cases the iterator is increased twice. Inside > the while when advancing the fragments and then one more time outside. > Patch is attached. Thanks for reporting, do you have a way to reproduce the issue? (In reply to sebastien.deronne from comment #1) > (In reply to Matías Richart from comment #0) > > Created attachment 2688 [details] > > Patch to increase iterator correctly > > > > The method BlockAckManager::GetNRetryNeededPackets misses to count some > > packets of the m_retryPackets queue. > > The problem is that in some cases the iterator is increased twice. Inside > > the while when advancing the fragments and then one more time outside. > > Patch is attached. > > Thanks for reporting, do you have a way to reproduce the issue? I don't. I found the issue by analysing the function code to see if it works for some things I'm implementing. I think it does not generate any issues because the function is only used to compare with zero. I agree to push because it would not break anything and I actually prefer your solution, but I actually do not see a case where it would behave differently. (In reply to sebastien.deronne from comment #3) > I agree to push because it would not break anything and I actually prefer > your solution, but I actually do not see a case where it would behave > differently. Suppose a case where in the m_retryPacket queue there are 4 consecutive different packets with address and tid the one we are looking for. The function should return 4. But in the current code it will enter the if ((*it)->hdr.GetAddr1 () == recipient && (*it)->hdr.GetQosTid () == tid) and count one packet. Then will enter the while and ibcrease the iterator once. After, as it is not the end of the queue, it will increase the iterator again, missing to count the second packet. Ok I get it, thanks. I do not understand why we have not spotted this issue before... I think this should be pushed asap to the mainstream so setting to LAST_CALL and increasing a bit priority to normal. (In reply to sebastien.deronne from comment #5) > Ok I get it, thanks. I do not understand why we have not spotted this issue > before... > I think this should be pushed asap to the mainstream so setting to LAST_CALL > and increasing a bit priority to normal. +1 Fixed in changeset 12468:a626821a28ef |
Created attachment 2688 [details] Patch to increase iterator correctly The method BlockAckManager::GetNRetryNeededPackets misses to count some packets of the m_retryPackets queue. The problem is that in some cases the iterator is increased twice. Inside the while when advancing the fragments and then one more time outside. Patch is attached.