Bugzilla – Bug 2213
Inconsistencies may exist between the selected WifiMacHelper and the chosen 802.11 version
Last modified: 2016-02-05 15:51:21 UTC
Created attachment 2179 [details] Patch to refactor wifi mac helpers Initial discussion started in https://groups.google.com/forum/#!topic/ns-3-users/stajKPTjuGA. Before the introduction of 802.11n, there were NqosWifiMacHelper and QosWifiMacHelper, which were independent on the selected 802.11 version. 802.11n introduced a HtWifiMacHelper and 802.11ac introduced a VhtWifiMacHelper. Those ones are no longer independent on the selected standard, and we may observe inconsistencies when the user selects 802.11n or 802.11ac but keep Nqos or Qos WifiMacHelpers iso Ht or Vht WifiMacHelpers (or any other strange combinations). Here is an example that is NOK but which won't trigger any (meaningful!) assert with the current implementation: ... NqosWifiMacHelper wifiMac = NqosWifiMacHelper::Default (); ... wifi.SetStandard (WIFI_PHY_STANDARD_80211n_2_4GHZ); ... After looking back at the code and investigating a better solution, I propose the refactor the code as follows: - remove HtWifiMacHelper and VhtWifiMacHelper: they just set the correct flag at the MAC layer. - set those corresponding flags when setting the standard. - define a McsHelper to do some Mcs/bitrate translation. - trigger an assert when QOS is not enabled for HT/VHT (so this means QosWifiMacHelper is mandatory for selecting 802.11n/ac). Note that I plan to update the documentation later on, based on our final decision.
Instead of +StringValue +McsHelper::DataRateForVhtMcs (int mcs) +{ + std::stringstream sstmp; + std::string strtmp, dataRate; + + sstmp << mcs; + sstmp >> strtmp; + dataRate = "VhtMcs" + strtmp; + + return StringValue (dataRate); +} you could simply use +StringValue +McsHelper::DataRateForVhtMcs (int mcs) +{ + std::ostringstream oss; + oss << "VhtMcs" << mcs; + + return StringValue (oss.str ()); +} Moreover, the Doxygen description is somewhat misleading: "Converts a VHT MCS value into a DataRate value". It's not exactly a DataRate (if I'm right). Other than this, I'm ok with the proposed changes, but they will break a LOT of simulations. A more kind (to the users) approach is to deprecate the old helpers and remove them in 3.26.
(In reply to Tommaso Pecorella from comment #1) > Instead of > +StringValue > +McsHelper::DataRateForVhtMcs (int mcs) > +{ > + std::stringstream sstmp; > + std::string strtmp, dataRate; > + > + sstmp << mcs; > + sstmp >> strtmp; > + dataRate = "VhtMcs" + strtmp; > + > + return StringValue (dataRate); > +} > > you could simply use > +StringValue > +McsHelper::DataRateForVhtMcs (int mcs) > +{ > + std::ostringstream oss; > + oss << "VhtMcs" << mcs; > + > + return StringValue (oss.str ()); > +} > Thanks, it seems better this way. > Moreover, the Doxygen description is somewhat misleading: "Converts a VHT > MCS value into a DataRate value". It's not exactly a DataRate (if I'm right). You are right, I will adapt the description. > > Other than this, I'm ok with the proposed changes, but they will break a LOT > of simulations. > A more kind (to the users) approach is to deprecate the old helpers and > remove them in 3.26. Indeed, users will have to adapt their simulation scripts. The same applies for bug 2116. However, I think it's the good moment to change it, it's already too long that we provide a too "complex" API to configure HT parameters.
Tom, any thought on this?
In general, I would like to reduce the number of wifi helpers that users have to provide in the common cases. I left a comment in bug 2116 that IMO it would be preferable to allow users to specify a standard such as 802.11n or ac with a single statement and get a default configuration that enables the key features (e.g. aggregation). Regarding this patch in particular, can providing QoS be made default for 11n/ac and this QoS helper used to configure non-default QoS-related attributes if needed on a per-container basis? Rather than deleting the Ht helper, is there a need to keep around an Ht or Vht helper to provide per-container configuration of HT or VHT parameters (i.e. if we have a helper dedicated to QoS configuration, why wouldn't we have one for HT configuration?). Regarding DataRateForHtMcs (int mcs);, I did not understand how this would work since data rate depends not only on mcs but also on guard interval and channel width.
Wouldn't it be better to have a single helper (so even no distinction qos and nqos), and based on what is passed as standard to the wifi helper, we could select the appropriate parameters? DataRateForHtMcs simply does a conversion from an mcs (integer) to a HtMcsX (string). But it has no real purpose, I assume this can maybe simply be removed.
(In reply to sebastien.deronne from comment #5) > Wouldn't it be better to have a single helper (so even no distinction qos > and nqos), and based on what is passed as standard to the wifi helper, we > could select the appropriate parameters? That would be fine with me, but let's try to sketch it out a bit before deciding. It would probably be a good idea to prioritize how we would do this for both 11n and 11ac, and then we could see whether we want to spend the time to also support 11a/b/g and what additional wrinkles those cause. > > DataRateForHtMcs simply does a conversion from an mcs (integer) to a HtMcsX > (string). > But it has no real purpose, I assume this can maybe simply be removed. OK
Latest changes updated in the code review: https://codereview.appspot.com/281230043/
new patch set added to the review
pushed in changeset 11854:7c60a9f8f271