ns-3 SpectrumWifiPhy

This is the review issue for the tracker issue https://www.nsnam.org/bugzilla/show_bug.cgi?id=2400

This is to track the review of SpectrumWifiPhy, which is proposed to allow Wi-Fi devices to connect to Spectrum channels.  This will allow SpectrumWifiPhy to receive signals that are not Wi-Fi, such as in Wi-Fi/LTE coexistence, or interference scenarios.

This version of SpectrumWifiPhy is very basic, in that it is for the most part a clone of YansWifiPhy, with some adaptor code at the very bottom of the Phy where the channel interconnection occurs.  Instead of YansWifiChannel, a SpectrumChannel type must be used (see src/spectrum).

The main difference is that when a foreign (non-Wi-Fi) signal is received, it is simply added to the InterferenceHelper and evaluated for CCA BUSY status, without further receive processing.  The performance of SpectrumWifiPhy and YansWifiPhy should be identical, except that SpectrumWifiPhy is sensitive by default to -62 dBm energy detection level of non-Wi-Fi signals.

Three example programs are provided, and one test suite.  The examples allow users to substitute SpectrumWifiPhy and YansWifiPhy under different conditions (signal loss, interference, saturating traffic) and compare results.

In the long run, SpectrumWifiPhy will use different interference trackers and error models more suited for the spectrum framework, but for the next release, it is proposed to simply reuse the existing Yans-compatible model and interference helper.


This is a large patch and users can experiment with it by cloning the spectrum-wifi branch from github: 

https://github.com/tomhenderson/ns-3-dev-git/tree/spectrum-wifi

Update for patch set 2:

Patch set 1 lacked two final things necessary for merging to ns-3-dev, IMO:
1) update the Wi-Fi .rst documentation
2) it was hardwired to the lower U-NII bands (5.180 GHz) and needs to be generalized (mainly, this involves making the spectrum model code more configurable)

Patch set 2 completes both issues.  In particular, I decided to remove the wifi-spectrum-helper.{cc,h} from src/wifi directory and place this code in the existing src/spectrum/wifi-spectrum-value-helper.{cc,h} files, with a bit of backward-compatible refactoring of the existing file which was used by the spectrum module.  I think this will be potentially less confusing to users.

I also made the default frequency subband decomposition to be 312.5 KHz (i.e. the OFDM subcarrier spacing), to enable frequency-selective fading models on a subcarrier basis.  I also removed the hardcoded limitations to the lower U-NII bands.

This patchset depends (and includes within) the patch to bug 2412 (frequency and channel width), still to be merged to ns-3-dev.

patchset 3:

- updated for the update to bug 2412 on ChannelNumber/Frequency coordination

- fixed small bug spotted by N. Sapoval on peeking vs. removing WifiPhyTag

- handle Sebastien's comments

[S. Deronne, 2016-05-05 09:00:58]

Tom,

I want first to highlight the great work that has been done here, it will open a
lot of perspectives for 802.11 PHY model improvement/extension in ns-3.

Those changes are very good at this point, I also suggest to update the
copyrights accordingly and I wrote some minor comments/suggestions in the code.

A point of improvement I'd like to discuss is the large amount of common code
between YansWifiPhy and SpectrumWifiPhy. Couldn’t we avoid that, e.g. by moving
some common parts to the WifiPhy? 
It might be hard to maintain if we keep those duplicates. For example, the
functions used to configure standards could be moved to the WifiPhy base class,
and keep mainly the rx and tx functions, and some other specific functions in
those Spectrum and Yans classes. 

Same comment also applies for the helper, can’t we imagine a WifiHelper as a
base class that handles all the common part, and inherit Yans and Spectrum to
handle their own specific part? 
For example, the functions related to pcap handling are identical, this should
be avoided (we already have this problem in Wave, and I'd like to change that). 

Note that this work to move some part to a base class can be done separately
from those changes, but I think it's the good timeline to do it now. I am ok to
help on this if you run a bit out of time.

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
File src/wifi/model/spectrum-wifi-phy.cc (right):

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
src/wifi/model/spectrum-wifi-phy.cc:80: .AddAttribute ("DisableWifiReception",
"Prevent Wi-Fi frame sync from ever happening",
What is the purpose of this attribute?
I understand that setting this to true will block all wifi receptions.
Is it maybe a parameter needed for mixed signals configurations when there is no
interest in Wi-Fi? Or is it something you forgot to clean?

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
File src/wifi/model/wifi-spectrum-helper.cc (right):

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
src/wifi/model/wifi-spectrum-helper.cc:46: NS_FATAL_ERROR ("Only channel 36, 40,
44, and 48 supported initially");
Does it cost a lot to extend to other channels?
I have the impression it should be quite trivial, but I am maybe wrong.

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
src/wifi/model/wifi-spectrum-helper.cc:62: WifiSpectrumHelper::GetSpectrumModel
(WifiMode mode)
The WifiMode parameter is not used here, so I suggest to clean it.

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
src/wifi/model/wifi-spectrum-helper.cc:73: double bandBandwidth = 5e6; // 5 Mhz
resource blocks:  145 bands
Can't we add a parameter to configure bandBandwidth?
Note that it should have some impact on the computation of txPowerPerBand (the
hardcoded value 4 should be changed as well).

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
File src/wifi/model/wifi-spectrum-phy-interface.cc (right):

https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
src/wifi/model/wifi-spectrum-phy-interface.cc:100: if (m_txPsd)
What is the purpose of this? 
m_txPsd seems never set.

[Tom Henderson, 2016-05-05 18:19:05]

On 2016/05/05 09:00:58, S. Deronne wrote:
> Tom,
> 
> I want first to highlight the great work that has been done here, it will open
a
> lot of perspectives for 802.11 PHY model improvement/extension in ns-3.

Thank you for the fast review.

> 
> Those changes are very good at this point, I also suggest to update the
> copyrights accordingly and I wrote some minor comments/suggestions in the
code.
> 
> A point of improvement I'd like to discuss is the large amount of common code
> between YansWifiPhy and SpectrumWifiPhy. Couldn’t we avoid that, e.g. by
moving
> some common parts to the WifiPhy? 
> It might be hard to maintain if we keep those duplicates. For example, the
> functions used to configure standards could be moved to the WifiPhy base
class,
> and keep mainly the rx and tx functions, and some other specific functions in
> those Spectrum and Yans classes. 
> 
> Same comment also applies for the helper, can’t we imagine a WifiHelper as a
> base class that handles all the common part, and inherit Yans and Spectrum to
> handle their own specific part? 
> For example, the functions related to pcap handling are identical, this should
> be avoided (we already have this problem in Wave, and I'd like to change
that). 
> 
> Note that this work to move some part to a base class can be done separately
> from those changes, but I think it's the good timeline to do it now. I am ok
to
> help on this if you run a bit out of time.

I agree on reducing code duplication.

> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> File src/wifi/model/spectrum-wifi-phy.cc (right):
> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> src/wifi/model/spectrum-wifi-phy.cc:80: .AddAttribute ("DisableWifiReception",
> "Prevent Wi-Fi frame sync from ever happening",
> What is the purpose of this attribute?
> I understand that setting this to true will block all wifi receptions.
> Is it maybe a parameter needed for mixed signals configurations when there is
no
> interest in Wi-Fi? Or is it something you forgot to clean?

It is intended for supporting LAA, where the SpectrumWifiPhy is reused in the
LteEnbNetDevice to perform carrier sense.

> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> File src/wifi/model/wifi-spectrum-helper.cc (right):
> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> src/wifi/model/wifi-spectrum-helper.cc:46: NS_FATAL_ERROR ("Only channel 36,
40,
> 44, and 48 supported initially");
> Does it cost a lot to extend to other channels?
> I have the impression it should be quite trivial, but I am maybe wrong.

No, I just haven't done it yet.

> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> src/wifi/model/wifi-spectrum-helper.cc:62:
WifiSpectrumHelper::GetSpectrumModel
> (WifiMode mode)
> The WifiMode parameter is not used here, so I suggest to clean it.

This will be part of cleaning this code to support multiple frequencies (I can
evaluate whether it is needed once I do those changes).

> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> src/wifi/model/wifi-spectrum-helper.cc:73: double bandBandwidth = 5e6; // 5
Mhz
> resource blocks:  145 bands
> Can't we add a parameter to configure bandBandwidth?
> Note that it should have some impact on the computation of txPowerPerBand (the
> hardcoded value 4 should be changed as well).

Yes.

> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> File src/wifi/model/wifi-spectrum-phy-interface.cc (right):
> 
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> src/wifi/model/wifi-spectrum-phy-interface.cc:100: if (m_txPsd)
> What is the purpose of this? 
> m_txPsd seems never set.

This may be some vestigial code; I agree that it should probably be deleted.

One of the objects needs to keep the transmit power spectral density around; I
think it is appropriate to keep it in class SpectrumWifiPhy because it is needed
there.

[S. Deronne, 2016-07-04 18:07:00]

On 2016/05/05 18:19:05, Tom Henderson wrote:
> On 2016/05/05 09:00:58, S. Deronne wrote:
> > Tom,
> > 
> > I want first to highlight the great work that has been done here, it will
open
> a
> > lot of perspectives for 802.11 PHY model improvement/extension in ns-3.
> 
> Thank you for the fast review.
> 
> > 
> > Those changes are very good at this point, I also suggest to update the
> > copyrights accordingly and I wrote some minor comments/suggestions in the
> code.
> > 
> > A point of improvement I'd like to discuss is the large amount of common
code
> > between YansWifiPhy and SpectrumWifiPhy. Couldn’t we avoid that, e.g. by
> moving
> > some common parts to the WifiPhy? 
> > It might be hard to maintain if we keep those duplicates. For example, the
> > functions used to configure standards could be moved to the WifiPhy base
> class,
> > and keep mainly the rx and tx functions, and some other specific functions
in
> > those Spectrum and Yans classes. 
> > 
> > Same comment also applies for the helper, can’t we imagine a WifiHelper as a
> > base class that handles all the common part, and inherit Yans and Spectrum
to
> > handle their own specific part? 
> > For example, the functions related to pcap handling are identical, this
should
> > be avoided (we already have this problem in Wave, and I'd like to change
> that). 
> > 
> > Note that this work to move some part to a base class can be done separately
> > from those changes, but I think it's the good timeline to do it now. I am ok
> to
> > help on this if you run a bit out of time.
> 
> I agree on reducing code duplication.
> 
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> > File src/wifi/model/spectrum-wifi-phy.cc (right):
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> > src/wifi/model/spectrum-wifi-phy.cc:80: .AddAttribute
("DisableWifiReception",
> > "Prevent Wi-Fi frame sync from ever happening",
> > What is the purpose of this attribute?
> > I understand that setting this to true will block all wifi receptions.
> > Is it maybe a parameter needed for mixed signals configurations when there
is
> no
> > interest in Wi-Fi? Or is it something you forgot to clean?
> 
> It is intended for supporting LAA, where the SpectrumWifiPhy is reused in the
> LteEnbNetDevice to perform carrier sense.
> 
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > File src/wifi/model/wifi-spectrum-helper.cc (right):
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > src/wifi/model/wifi-spectrum-helper.cc:46: NS_FATAL_ERROR ("Only channel 36,
> 40,
> > 44, and 48 supported initially");
> > Does it cost a lot to extend to other channels?
> > I have the impression it should be quite trivial, but I am maybe wrong.
> 
> No, I just haven't done it yet.
> 
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > src/wifi/model/wifi-spectrum-helper.cc:62:
> WifiSpectrumHelper::GetSpectrumModel
> > (WifiMode mode)
> > The WifiMode parameter is not used here, so I suggest to clean it.
> 
> This will be part of cleaning this code to support multiple frequencies (I can
> evaluate whether it is needed once I do those changes).
> 
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > src/wifi/model/wifi-spectrum-helper.cc:73: double bandBandwidth = 5e6; // 5
> Mhz
> > resource blocks:  145 bands
> > Can't we add a parameter to configure bandBandwidth?
> > Note that it should have some impact on the computation of txPowerPerBand
(the
> > hardcoded value 4 should be changed as well).
> 
> Yes.
> 
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > File src/wifi/model/wifi-spectrum-phy-interface.cc (right):
> > 
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > src/wifi/model/wifi-spectrum-phy-interface.cc:100: if (m_txPsd)
> > What is the purpose of this? 
> > m_txPsd seems never set.
> 
> This may be some vestigial code; I agree that it should probably be deleted.
> 
> One of the objects needs to keep the transmit power spectral density around; I
> think it is appropriate to keep it in class SpectrumWifiPhy because it is
needed
> there.

Tom,
Don't you plan to remove the duplicated code between Yans and Spectrum?

[Tom Henderson, 2016-07-04 18:10:50]

On 2016/07/04 18:07:00, S. Deronne wrote:
> On 2016/05/05 18:19:05, Tom Henderson wrote:
> > On 2016/05/05 09:00:58, S. Deronne wrote:
> > > Tom,
> > > 
> > > I want first to highlight the great work that has been done here, it will
> open
> > a
> > > lot of perspectives for 802.11 PHY model improvement/extension in ns-3.
> > 
> > Thank you for the fast review.
> > 
> > > 
> > > Those changes are very good at this point, I also suggest to update the
> > > copyrights accordingly and I wrote some minor comments/suggestions in the
> > code.
> > > 
> > > A point of improvement I'd like to discuss is the large amount of common
> code
> > > between YansWifiPhy and SpectrumWifiPhy. Couldn’t we avoid that, e.g. by
> > moving
> > > some common parts to the WifiPhy? 
> > > It might be hard to maintain if we keep those duplicates. For example, the
> > > functions used to configure standards could be moved to the WifiPhy base
> > class,
> > > and keep mainly the rx and tx functions, and some other specific functions
> in
> > > those Spectrum and Yans classes. 
> > > 
> > > Same comment also applies for the helper, can’t we imagine a WifiHelper as
a
> > > base class that handles all the common part, and inherit Yans and Spectrum
> to
> > > handle their own specific part? 
> > > For example, the functions related to pcap handling are identical, this
> should
> > > be avoided (we already have this problem in Wave, and I'd like to change
> > that). 
> > > 
> > > Note that this work to move some part to a base class can be done
separately
> > > from those changes, but I think it's the good timeline to do it now. I am
ok
> > to
> > > help on this if you run a bit out of time.
> > 
> > I agree on reducing code duplication.
> > 
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> > > File src/wifi/model/spectrum-wifi-phy.cc (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/spectrum-wifi-...
> > > src/wifi/model/spectrum-wifi-phy.cc:80: .AddAttribute
> ("DisableWifiReception",
> > > "Prevent Wi-Fi frame sync from ever happening",
> > > What is the purpose of this attribute?
> > > I understand that setting this to true will block all wifi receptions.
> > > Is it maybe a parameter needed for mixed signals configurations when there
> is
> > no
> > > interest in Wi-Fi? Or is it something you forgot to clean?
> > 
> > It is intended for supporting LAA, where the SpectrumWifiPhy is reused in
the
> > LteEnbNetDevice to perform carrier sense.
> > 
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > File src/wifi/model/wifi-spectrum-helper.cc (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > src/wifi/model/wifi-spectrum-helper.cc:46: NS_FATAL_ERROR ("Only channel
36,
> > 40,
> > > 44, and 48 supported initially");
> > > Does it cost a lot to extend to other channels?
> > > I have the impression it should be quite trivial, but I am maybe wrong.
> > 
> > No, I just haven't done it yet.
> > 
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > src/wifi/model/wifi-spectrum-helper.cc:62:
> > WifiSpectrumHelper::GetSpectrumModel
> > > (WifiMode mode)
> > > The WifiMode parameter is not used here, so I suggest to clean it.
> > 
> > This will be part of cleaning this code to support multiple frequencies (I
can
> > evaluate whether it is needed once I do those changes).
> > 
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > src/wifi/model/wifi-spectrum-helper.cc:73: double bandBandwidth = 5e6; //
5
> > Mhz
> > > resource blocks:  145 bands
> > > Can't we add a parameter to configure bandBandwidth?
> > > Note that it should have some impact on the computation of txPowerPerBand
> (the
> > > hardcoded value 4 should be changed as well).
> > 
> > Yes.
> > 
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > File src/wifi/model/wifi-spectrum-phy-interface.cc (right):
> > > 
> > >
> >
>
https://codereview.appspot.com/293380043/diff/1/src/wifi/model/wifi-spectrum-...
> > > src/wifi/model/wifi-spectrum-phy-interface.cc:100: if (m_txPsd)
> > > What is the purpose of this? 
> > > m_txPsd seems never set.
> > 
> > This may be some vestigial code; I agree that it should probably be deleted.
> > 
> > One of the objects needs to keep the transmit power spectral density around;
I
> > think it is appropriate to keep it in class SpectrumWifiPhy because it is
> needed
> > there.
> 
> Tom,
> Don't you plan to remove the duplicated code between Yans and Spectrum?

I did not do it in this revision; if you want me to add 'remove code
duplication' I could add another patch on top of this.

I'm looking for concurrence from wi-fi maintainers to keep proceeding with the
current design; what has mostly changed since the last patch is the
implementation of the spectrum value helper, and the addition of .rst
documentation.

I think that code duplication could be removed in a cleanup phase upon doing the
actual merge, but can stage a patch if desired for review.

[S. Deronne, 2016-07-04 20:27:07]

https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
File src/spectrum/model/wifi-spectrum-value-helper.cc (right):

https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
src/spectrum/model/wifi-spectrum-value-helper.cc:190: NS_LOG_DEBUG ("Added
signal power to subbands 36-63, 65-92, 100-127, 129-156, 164-191, 193-220,
228-255, 257-284");
debug statement to be extended for 160 MHz subbands

https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
src/spectrum/model/wifi-spectrum-value-helper.cc:194: break;
two breaks: one can be removed

[Tom Henderson, 2016-07-16 00:24:40]

On 2016/07/04 20:27:07, S. Deronne wrote:
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> File src/spectrum/model/wifi-spectrum-value-helper.cc (right):
> 
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> src/spectrum/model/wifi-spectrum-value-helper.cc:190: NS_LOG_DEBUG ("Added
> signal power to subbands 36-63, 65-92, 100-127, 129-156, 164-191, 193-220,
> 228-255, 257-284");
> debug statement to be extended for 160 MHz subbands
> 
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> src/spectrum/model/wifi-spectrum-value-helper.cc:194: break;
> two breaks: one can be removed


the above were handled; also,
- updated for the update to bug 2412 on ChannelNumber/Frequency coordination 
- fixed small bug spotted by N. Sapoval on peeking vs. removing WifiPhyTag

This patch now depends on 2412, which now depends on the small patch in 2452.

[Tom Henderson, 2016-07-23 04:55:39]

On 2016/07/16 00:24:40, Tom Henderson wrote:
> On 2016/07/04 20:27:07, S. Deronne wrote:
> >
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> > File src/spectrum/model/wifi-spectrum-value-helper.cc (right):
> > 
> >
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> > src/spectrum/model/wifi-spectrum-value-helper.cc:190: NS_LOG_DEBUG ("Added
> > signal power to subbands 36-63, 65-92, 100-127, 129-156, 164-191, 193-220,
> > 228-255, 257-284");
> > debug statement to be extended for 160 MHz subbands
> > 
> >
>
https://codereview.appspot.com/293380043/diff/60001/src/spectrum/model/wifi-s...
> > src/spectrum/model/wifi-spectrum-value-helper.cc:194: break;
> > two breaks: one can be removed
> 
> 
> the above were handled; also,
> - updated for the update to bug 2412 on ChannelNumber/Frequency coordination 
> - fixed small bug spotted by N. Sapoval on peeking vs. removing WifiPhyTag
> 
> This patch now depends on 2412, which now depends on the small patch in 2452.


Note, this has been merged to ns-3-dev on 22 July 2016 in changeset
12219:a7e5825d00b6 so the issue is now closed