Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(312)

Issue 4685048: Monitor mode support in ns-3

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years ago by ashwin.narayan89
Modified:
7 years ago
CC:
ns-3-reviews_googlegroups.com, Lalith Suresh
Visibility:
Public.

Patch Set 1 #

Total comments: 54

Patch Set 2 : Further changes to Monitor Mode support #

Patch Set 3 : Fixed bugs, Wifi header manipulations, Click based example #

Patch Set 4 : Removing redundant parameters sent up the stack #

Patch Set 5 : Removed m_monitorMode flag in MacLow #

Total comments: 17

Patch Set 6 : Undoing redundant params, changed GetMonitorMode to IsMonitorMode #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1151 lines, -51 lines) Patch
M CHANGES.html View 1 2 3 4 5 1 chunk +10 lines, -1 line 0 comments Download
M RELEASE_NOTES View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
A src/click/examples/nsclick-monitor.click View 1 2 3 1 chunk +116 lines, -0 lines 0 comments Download
A src/click/examples/wifi-monitor-mac.cc View 1 2 3 4 1 chunk +271 lines, -0 lines 0 comments Download
M src/click/examples/wscript View 1 2 4 1 chunk +4 lines, -0 lines 0 comments Download
M src/click/model/ipv4-l3-click-protocol.cc View 1 2 3 4 5 2 chunks +15 lines, -7 lines 0 comments Download
M src/click/wscript View 1 2 4 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/examples/wifi-phy-test.cc View 4 5 5 chunks +5 lines, -5 lines 0 comments Download
M src/wifi/model/adhoc-wifi-mac.h View 1 4 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/model/adhoc-wifi-mac.cc View 1 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/wifi/model/ap-wifi-mac.h View 1 4 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/model/ap-wifi-mac.cc View 1 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/wifi/model/mac-low.h View 1 2 3 4 5 5 chunks +6 lines, -3 lines 0 comments Download
M src/wifi/model/mac-low.cc View 1 2 3 4 5 7 chunks +9 lines, -8 lines 0 comments Download
M src/wifi/model/mac-rx-middle.h View 1 4 2 chunks +3 lines, -2 lines 0 comments Download
M src/wifi/model/mac-rx-middle.cc View 1 4 2 chunks +2 lines, -2 lines 0 comments Download
A src/wifi/model/monitor-wifi-mac.h View 1 2 3 4 5 1 chunk +179 lines, -0 lines 0 comments Download
A src/wifi/model/monitor-wifi-mac.cc View 1 2 3 4 1 chunk +442 lines, -0 lines 0 comments Download
M src/wifi/model/regular-wifi-mac.h View 1 2 4 2 chunks +3 lines, -1 line 0 comments Download
M src/wifi/model/regular-wifi-mac.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/model/sta-wifi-mac.h View 1 4 1 chunk +1 line, -1 line 0 comments Download
M src/wifi/model/sta-wifi-mac.cc View 1 4 2 chunks +2 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-net-device.h View 1 2 3 4 5 3 chunks +12 lines, -1 line 0 comments Download
M src/wifi/model/wifi-net-device.cc View 1 2 3 4 5 3 chunks +27 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-phy.h View 1 2 3 4 5 2 chunks +3 lines, -1 line 0 comments Download
M src/wifi/model/wifi-phy-state-helper.h View 1 2 3 4 5 3 chunks +3 lines, -2 lines 0 comments Download
M src/wifi/model/wifi-phy-state-helper.cc View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M src/wifi/model/yans-wifi-phy.cc View 1 2 3 4 5 2 chunks +22 lines, -2 lines 0 comments Download
M src/wifi/wscript View 1 4 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 12
Nicola Baldo
I reviewed the code. I think the major issues are: 1) the "monitor" flag in ...
7 years ago (2011-07-15 15:33:27 UTC) #1
Tom Henderson
Most of my comments are inline; here are a few remaining comments. Support for this ...
7 years ago (2011-07-15 16:48:00 UTC) #2
ashwin.narayan89
Hello, Thank you for your comments. I'm sorry about the issues with the merge. I'll ...
7 years ago (2011-07-15 17:34:24 UTC) #3
Lalith Suresh
On Fri, Jul 15, 2011 at 10:18 PM, <tomh.org@gmail.com> wrote: > Most of my comments ...
7 years ago (2011-07-18 05:20:00 UTC) #4
Lalith Suresh
On Fri, Jul 15, 2011 at 9:03 PM, <nicobaldo@gmail.com> wrote: > I reviewed the code. ...
7 years ago (2011-07-18 05:28:19 UTC) #5
Lalith Suresh
On Fri, Jul 15, 2011 at 9:03 PM, <nicobaldo@gmail.com> wrote: > [snip] > > http://codereview.appspot.com/**4685048/diff/1/src/wifi/model/**mac-low.cc<http://codereview.appspot.com/4685048/diff/1/src/wifi/model/mac-low.cc> ...
7 years ago (2011-07-18 07:49:59 UTC) #6
ashwin.narayan89
Thank you for your comments again. I have managed to make the necessary corrections with ...
7 years ago (2011-07-18 08:33:30 UTC) #7
ashwin.narayan89
Hello, > http://codereview.appspot.com/**4685048/diff/1/src/wifi/model/** > wifi-phy.h#newcode154<http://codereview.appspot.com/4685048/diff/1/src/wifi/model/wifi-phy.h#newcode154> > src/wifi/model/wifi-phy.h:154: typedef Callback<void,Ptr<Packet>, > double, WifiMode, enum WifiPreamble, RadiotapHeader> ...
7 years ago (2011-07-19 09:51:51 UTC) #8
Tom Henderson
> > So it seems to me like we just need to enable the m_promisc ...
7 years ago (2011-07-19 14:32:40 UTC) #9
ashwin.narayan89
Following the previous discussion, I've removed the m_monitorMode flag in MacLow http://codereview.appspot.com/4685048/diff/1/src/wifi/model/mac-low.cc File src/wifi/model/mac-low.cc (right): ...
7 years ago (2011-07-21 18:09:19 UTC) #10
Nicola Baldo
Please find some detailed comments on the code below. As a more generic comment, I ...
7 years ago (2011-07-22 09:07:00 UTC) #11
ashwin.narayan89
7 years ago (2011-07-23 16:20:50 UTC) #12
Made corrections based on the comments. Reintroduced the redundant parameters
(SNR, WifiPreamble). Changed GetMonitorMode to IsMonitorMode.

http://codereview.appspot.com/4685048/diff/19001/RELEASE_NOTES
File RELEASE_NOTES (left):

http://codereview.appspot.com/4685048/diff/19001/RELEASE_NOTES#oldcode28
RELEASE_NOTES:28: SpectrumPropagationLossModel class already supported by
On 2011/07/22 09:07:00, Nicola Baldo wrote:
> again, please be careful with the merging
Corrected

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/mac-low.cc
File src/wifi/model/mac-low.cc (left):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/mac-low.cc#ol...
src/wifi/model/mac-low.cc:665: MacLow::ReceiveOk (Ptr<Packet> packet, double
rxSnr, WifiMode txMode, WifiPreamble preamble)
Done

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/mac-low.cc
File src/wifi/model/mac-low.cc (right):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/mac-low.cc#ne...
src/wifi/model/mac-low.cc:1857: double
Corrected

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/monitor-wifi-...
File src/wifi/model/monitor-wifi-mac.cc (right):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/monitor-wifi-...
src/wifi/model/monitor-wifi-mac.cc:309: return m_low->GetBssid ();
Yes, BSSID to be set in MacLow as it used by MacLow::NotifyNav (), which is used
in Monitor Mode. I don't SSID would be used though. Would it be okay to use
NS_ABORT_MSGs for these methods where I return values?

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/monitor-wifi-...
File src/wifi/model/monitor-wifi-mac.h (right):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/monitor-wifi-...
src/wifi/model/monitor-wifi-mac.h:81: * The following functions have been
inherited from RegularWifiMac.
Corrected

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/monitor-wifi-...
src/wifi/model/monitor-wifi-mac.h:102: * The following functions have been
inherited from RegularMacWifi
Corrected

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/wifi-net-devi...
File src/wifi/model/wifi-net-device.cc (right):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/wifi-net-devi...
src/wifi/model/wifi-net-device.cc:298: Ptr<Packet> pCopy = packet->Copy ();
The packet is essentially copied to remove the LLC header from it (which should
be preserved in the original packet). The LLC header is needed for getting some
information from it in the code to follow. One can't remove the LLC header
without stripping off the other headers. Thus, I had to use RemoveHeader for
rtHeader and wmHeader.

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/wifi-net-devi...
File src/wifi/model/wifi-net-device.h (right):

http://codereview.appspot.com/4685048/diff/19001/src/wifi/model/wifi-net-devi...
src/wifi/model/wifi-net-device.h:90: * \returns monitor mode state of device
Corrected. Changed method to IsMonitorMode. Hope that's okay.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b