Bugzilla – Full Text Bug Listing |
Summary: | Setting NqStaWifiMac Active Probing true, crashes simulation | ||
---|---|---|---|
Product: | ns-3 | Reporter: | Tomé Gomes <tomeggomes> |
Component: | wifi | Assignee: | Nicola Baldo <nicola> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | deanarm, ns-bugs |
Priority: | P5 | ||
Version: | ns-3.9 | ||
Hardware: | PC | ||
OS: | Linux | ||
Attachments: | delay first call to TryToEnsureAssociated |
Description
Tomé Gomes
2011-02-22 05:47:03 EST
Hi Nicola! This is my first bug report. I hope I did everything right and clean :) The bug is easy to reproduce but I assume that anyone has tried before. As you can see by the backtrace, it is trying to call a Callback but the Callback is nil, hence the crash. Why the callback is empty I don't know. Hope you can help! Tomé Thanks for pointing this out, Tomé. I was going to fix it, but it's less trivial than I initially supposed when I saw the bug report... Firstly there's the matter of the NULL callback being invoked, which appears to be a no-brainer...except I can see three ways of doing it and want guidance from the developers on which is preferable... (a) The most obvious possibility is for me to protect the offending invocation of m_linkDown in StaWifiMac::TryToEnsureAssociated() with a "if (!mlinkDown.IsNull ())". (b) Alternately, I could make RegularWifiMac::m_linkDown and RegularWifiMac::m_linkUp private, and add protected methods (say) RegularWifiMac::InvokeLinkDownCallback() and RegularWifiMac::InvokeLinkUpCallback() which do the IsNull() test and invoke the respective callback, such that these can be simply invoked from a RegularWifiMac derivative - StaWifiMac in this case. (c) Another possibility is to add the NULL callback check to the operator() method implementation for callbacks (or perhaps a new class of callbacks, for use where it is a non-fatal error for the callback to be NULL). This could give a performance penalty for such callbacks that are used in ways such that they are never NULL, but my suspicion is that the penalty would be negligible. Any thoughts? Even with this part of the problem fixed, the local change Tomé has made to third.cc still brings out a crash because it leads to StaWifiMac::SetActiveProbing() (as the set handler for attribute ActiveProbing) invoking StaWifiMac::TryToEnsureAssociated(), which then tries to queue a Probe Request for transmission at a time when NqosWifiMacHelper is still in the process of creating and configuring the objects in the node. The specific problem we have there is that StaWifiMac::m_phy is not yet set when StaWifiMac::SendProbeRequest() invokes StaWifiMac::GetSupportedRates(), so we get a NULL pointer dereference. Before correcting this, I wanted to find out what the "ActiveProbing" mode was originally supposed to do, as I'm not sure I have entirely figured it out yet. Is it the case that... - If ActiveProbing is false, then the STA - if not already associated - will take no action until it receives a Beacon from an AP with SSID matching the value in the Ssid attribute (inherited from WifiMac), at which point it will initiate association to that AP. - If ActiveProbing is true, then the STA - if not already associated - will with periodicity set by the attribute ProbeRequestTimeout (default currently 50 ms) attempt a broadcast active scan for the AP with SSID matching the value in the Ssid attribute. Is there any more or less to the intention than this? (In reply to comment #2) > Thanks for pointing this out, Tomé. Hello Dean! I thank you for your quick and complete answer ;) > > I was going to fix it, but it's less trivial than I initially supposed when I > saw the bug report... > > Firstly there's the matter of the NULL callback being invoked, which appears to > be a no-brainer...except I can see three ways of doing it and want guidance > from the developers on which is preferable... > > (a) The most obvious possibility is for me to protect the offending invocation > of m_linkDown in StaWifiMac::TryToEnsureAssociated() with a "if > (!mlinkDown.IsNull ())". > (b) Alternately, I could make RegularWifiMac::m_linkDown and > RegularWifiMac::m_linkUp private, and add protected methods (say) > RegularWifiMac::InvokeLinkDownCallback() and > RegularWifiMac::InvokeLinkUpCallback() which do the IsNull() test and invoke > the respective callback, such that these can be simply invoked from a > RegularWifiMac derivative - StaWifiMac in this case. > (c) Another possibility is to add the NULL callback check to the operator() > method implementation for callbacks (or perhaps a new class of callbacks, for > use where it is a non-fatal error for the callback to be NULL). This could give > a performance penalty for such callbacks that are used in ways such that they > are never NULL, but my suspicion is that the penalty would be negligible. > > Any thoughts? Are you just asking to me or to any other developers that are regarding this bug? I can understand the three possibilities but not their future impact so I'm not able to make the best choice. > > Even with this part of the problem fixed, the local change Tomé has made to > third.cc still brings out a crash because it leads to > StaWifiMac::SetActiveProbing() (as the set handler for attribute ActiveProbing) > invoking StaWifiMac::TryToEnsureAssociated(), which then tries to queue a Probe > Request for transmission at a time when NqosWifiMacHelper is still in the > process of creating and configuring the objects in the node. The specific > problem we have there is that StaWifiMac::m_phy is not yet set when > StaWifiMac::SendProbeRequest() invokes StaWifiMac::GetSupportedRates(), so we > get a NULL pointer dereference. > > Before correcting this, I wanted to find out what the "ActiveProbing" mode was > originally supposed to do, as I'm not sure I have entirely figured it out yet. > > Is it the case that... > > - If ActiveProbing is false, then the STA - if not already associated - will > take no action until it receives a Beacon from an AP with SSID matching the > value in the Ssid attribute (inherited from WifiMac), at which point it will > initiate association to that AP. > - If ActiveProbing is true, then the STA - if not already associated - will > with periodicity set by the attribute ProbeRequestTimeout (default currently 50 > ms) attempt a broadcast active scan for the AP with SSID matching the value in > the Ssid attribute. > > Is there any more or less to the intention than this? In this ubject I can say with 100% sure that you're right. Just to complement, in Active Scanning, the AP that receives the Probe Request will respond with a unicast Probe Response, that, when received by the STA will trigger normal association process. (In reply to comment #3) > Are you just asking to me or to any other developers that are regarding this > bug? I'm attempting to canvas wider opinion (in particular, I suspect Mathieu - if he's listening - would have valuable input here, as would Nicola), but that doesn't make any opinion you have less valuable. Created attachment 1053 [details] delay first call to TryToEnsureAssociated Hi Tomé & Dean, thanks for the report and for the investigation, and sorry for the late reply. I think that m_linkDown is intended to be always set, in fact it is set by WifiNetDevice::CompleteConfig for all types of devices (STA, AP, etc). The problem is that WifiNetDevice::CompleteConfig is called only at the end of the whole configuration, while setting ActiveProbing upon the creation of ns3::StaWifiMac causes StaWifiMac::TryToEnsureAssociated to be called before ifiNetDevice::CompleteConfig the attached patch fixes the problem, but I am not sure it is a nice fix, because if there is more than one STA in the scenario then it will cause all STAs to try to send a probe request at the same time, thus generating the well-known problems discussed within bug 912 and bug 388. One solution would be just to document that the ActiveProbing attribute needs to be set to true at a random time, for the sake of the simulation. Another solution would be to find an approach to solve 912 and 338, and apply the same philosophy to this bug. Any opinions? changeset: 6993:9f51576c4d7b applied the patch and updated the documentation of the ActiveProbing attribute to warn people that they should be careful when enabling it on multiple STAs. |