Bugzilla – Bug 881
Reorganise to allow wider use of WifiInformationElement
Last modified: 2020-04-12 08:20:34 UTC
As part of the mesh work, WifiInformationElement was created as a base class for IEs. While currently resident and only used in the mesh module, this and other related classes like WifiInformationElementVector obviously have significant potential for use in the wifi module, where they could ease (and/or tidy) implementation of further Wi-Fi 'features'. I'm working on such 'features' and so would like to open discussion on a reorganisation that would allow more general use of these objects. I'm therefore raising this bug as a place to hold this discussion and cite proposed code. Broadly speaking, what I am proposing initially is that the WifiInformationElement and WifiInformationElementVector classes be moved to the wifi module, with the mesh-specific deserialisation know-how extracted to a new subclass of WifiInformationElementVector, (perhaps called MeshInformationElementVector) in the mesh module.
At the following URL you'll find the sort of code I was thinking of: http://codereview.appspot.com/898045 The patch there splits the mesh-specific functionality out from WifiInformationElementVector, and places it in a derived class called MeshInformationElementVector. The WifiInformationElement class remains relatively unmodified, but is relocated with WifiInformationElementVector to the wifi module. WifiElementId is now typedef'd to be uint8_t with the Element IDs #defined, to allow the standard-defined ones (i.e., those in IEEE 802.11-2007) kept in the wifi module to be seperated from the pre-standard ones (i.e., for mesh) defined in the mesh module. I'm not sure I'm entirely comfortable with this approach; I really want some form of inheritance and I suspect there is a cleaner way to do this, but it is not obvious to me right now. While not strictly within the scope of this bug, there is code at... http://codereview.appspot.com/840046 ..., which builds on that in Issue 898045 to actually make use of the relocated classes in the wifi module, and thus may serve as some more context for discussion and/or thought on the general approach.
CCing Kirill since this issue affects the mesh code as well.
I've re-done the patch as proposed in Comment #1 after feedback from Mathieu in the ns-3-reviews group. As mentioned in that thread, the new proposed code is available for further review as the three changesets at the tip of http://code.nsnam.org/deanarm/ns-3-dev-bug-881/. I've also updated Issue 898045.
Given completion of review by Nicola and Kirill under Issue 898045, I've pushed some changes. This bug is thus fixed by the sum of changesets 6378:57485ed01268, 6379:f1031f074dbd, 6380:650233cda60e, 6381:cbc1b93b298a, 6382:9e5768e48981 and 6383:4dc3cdb53559 on ns-3-dev.
One further change added as changeset 6389:6e83ec53ba0a on ns-3-dev. This renames a WifiInformationElement method that I forgot about in the last bunch.
In the course of debugging bug 1770, which was an issue with the design of the SupportedRates information element, I found that the current implementation of information rates is still inconsistent between mesh and wifi modules. The patch for this bug 881 moved the WifiInformationElementVector from src/mesh into src/wifi, but there is no current use made of it in src/wifi. The current code in src/wifi makes use of the information elements as normal C++ classes. However, they are now derived from SimpleRefCount, and should be handled as reference counted objects. It seems to me that we ought to try to complete the job that Dean started and refactor to use WifiInformationElementVector for the management frame information elements within src/wifi; these include Ssid, SupportedRates, ExtendedSupportedRates, and HtCapabilities. I started to do this in the attached patch (for HtCapabilities alone) but the scope of this patch to tackle all of the IEs is unfortunately going to be a bit larger due to the pervasive use of the Ssid and SupportedRates objects as normal objects. This may simplify the serialization/deserialization code for management frames; presently, there are some XXX comments in those methods that suggest that it may not be fully tested.
Created attachment 1941 [details] example (draft) patch to refactor HtCapabilities IE
This is just a status update (April 2017) to note that this cleanup still should be done at some point, in my opinion. WifiInformationElementVector::DeserializeSingleIe () does not work at the moment, which is not an immediate problem since it is not being used anywhere (MeshInformationElementVector::DeserializeSingleIe () covers all of the cases for mesh at the moment). It would become a problem if another module or user's code tried to include WifiInformationElements such as 'Ssid' in a WifiInformationElementVector or a subclass of it. One problem with WifiInformationElementVector is that it is variable length, so can't deserialize itself properly since it doesn't know how large it is upon deserialization. This is the topic of bug 2505, and can be patched to fix the more general problem of deserializing variable-sized headers, but even once mesh ASCII tracing is fixed by the bug 2505 fix (when that is done), there still will be lingering issues with WifiInformationElementVector and use of WifiInformationElement that should be resolved.
*** Bug 1363 has been marked as a duplicate of this bug. ***
Bug 2945 is strongly related and should close this bug too once resolved.
Let's move this to bug 2945 *** This bug has been marked as a duplicate of bug 2945 ***