Bugzilla – Bug 2454
DsrRouting::NotifyDataReceipt is also triggered for wifi management packets
Last modified: 2016-08-02 10:24:47 UTC
Created attachment 2497 [details] example script This script has been submitted to the ns-3 users group some time ago. The user had a strange crash, and he was thinking that it was dependent on DSR. It turns out that DSR is fine, it's Wi-Fi that is sending to L3 a management packet: ns3::WifiActionHeader (category=0, value=0) ns3::MgtAddBaRequestHeader Strange enough, maned-routing-compare example works. Upon further research, I found that the problem is this: If you set a ns3::ConstantRateWifiManager without explicitly setting the data rates (i.e., "wifi.SetRemoteStationManager ("ns3::ConstantRateWifiManager");"), then the bug happens. If you specify the DataMode, then all is fine. Setting the ControlMode is indifferent. Beat me if I can see the connection between these two problems, but there is one. The obvious solution could be to avoid to have ConstantRateWifiManager set without a corresponding DataMode, but I feel that this would be just hiding the dust under the carpet.
I took some time to investigate this bug, and my conclusion is that it is not a wifi one. Issue is actually well located in DSR: void DsrRouting::NotifyDataReceipt (std::string context, Ptr<const Packet> p) { ... newP->RemoveHeader(llc); ... } Packets going in NotifyDataReceipt are coming from the wifi trace source "PhyRxEnd": void DsrRouting::ConnectCallbacks () { // Connect the callbacks Config::Connect ("NodeList/*/DeviceList/*/$ns3::WifiNetDevice/Phy/PhyRxEnd", MakeCallback (&DsrRouting::NotifyDataReceipt, this)); } PhyRxEnd just forwards any packet that has been completely received from the channel medium by the device, and not only data packets! It is crashing here because NotifyDataReceipt is removing LLC header on ADDBA packet, which is a wifi management frame. The fact the issue was seen only when setting ConstantRateWifiManager without explicitly setting the data rates is because it was sending ADDBA at a very low bitrate and was thus successfully captured by PhyRxEnd; but when setting a higher bitrate for ConstantRateWifiManager, ADDBA is not successfully received and NotifyDataReceipt is not triggered.
Moving to DSR since it is not a wifi bug.
Ach ! Thanks for looking at it, you're right. However this raises even more problems (on the DSR side). We can let it skip all the non-data packets (easy), but what about the aggregated packets (e.g., A-MPDU) ? A temporary solution is: 1) Skip non-data and 2) Warn that aggregation must be disabled for DSR. A better solution would be to pick the packets at IP level. (In reply to sebastien.deronne from comment #1) > I took some time to investigate this bug, and my conclusion is that it is > not a wifi one. Issue is actually well located in DSR: > > void DsrRouting::NotifyDataReceipt (std::string context, Ptr<const Packet> p) > { > ... > newP->RemoveHeader(llc); > ... > } > > Packets going in NotifyDataReceipt are coming from the wifi trace source > "PhyRxEnd": > > void DsrRouting::ConnectCallbacks () > { > // Connect the callbacks > Config::Connect > ("NodeList/*/DeviceList/*/$ns3::WifiNetDevice/Phy/PhyRxEnd", > MakeCallback (&DsrRouting::NotifyDataReceipt, this)); > } > > PhyRxEnd just forwards any packet that has been completely received from the > channel medium by the device, and not only data packets! > > It is crashing here because NotifyDataReceipt is removing LLC header on > ADDBA packet, which is a wifi management frame. > > The fact the issue was seen only when setting ConstantRateWifiManager > without explicitly setting the data rates is because it was sending ADDBA at > a very low bitrate and was thus successfully captured by PhyRxEnd; but when > setting a higher bitrate for ConstantRateWifiManager, ADDBA is not > successfully received and NotifyDataReceipt is not triggered.
Created attachment 2501 [details] Possible fix This should fix the problem, but I'm not sure how it will work in case of aggregated Wi-Fi frames.
Patch should be ok.
Created attachment 2507 [details] Fix The best pants is no pants. The best solution for a bugged function is to remove it. As a side note, DSR makes heavy use of "God-like" functionalities. Normally this is a bad idea.
looks OK to me to merge
pushed in changeset 12232:812955886e7f