Bug 2096 - Wrong pcap information when MPDU aggregation is used
Wrong pcap information when MPDU aggregation is used
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: wifi
ns-3-dev
All All
: P5 minor
Assigned To: sebastien.deronne
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-15 14:41 EDT by sebastien.deronne
Modified: 2015-07-08 17:48 EDT (History)
2 users (show)

See Also:


Attachments
here is the attached pcap traces when A-MPDU is enabled (42.50 KB, application/octet-stream)
2015-04-15 14:49 EDT, sebastien.deronne
Details
Capture File with MPDU Status Flag in RadioTap Header (65.15 KB, application/octet-stream)
2015-06-16 09:55 EDT, Hany
Details
Patch file to solve the PCAP problem (26.22 KB, patch)
2015-06-21 11:19 EDT, Hany
Details | Diff
patch to add MCS and A-MPDU status fields in radiotap header (76.70 KB, patch)
2015-07-01 16:24 EDT, sebastien.deronne
Details | Diff
patch to add correct padding, MCS and A-MPDU status fields in radiotap header (79.92 KB, patch)
2015-07-02 13:01 EDT, sebastien.deronne
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sebastien.deronne 2015-04-15 14:41:23 EDT
As observed in the attached pcap traces, the information provided is erroneous when MPDU aggregation is used.
Comment 1 sebastien.deronne 2015-04-15 14:49:18 EDT
Created attachment 2017 [details]
here is the attached pcap traces when A-MPDU is enabled
Comment 2 sebastien.deronne 2015-06-06 06:18:45 EDT
I started to have a look at this, and it is pretty clear that no updates have been done to support 802.11n information in pcap files.

My first thoughts are that we should now extend radiotap header to PPI. 
I am not sure yet PPI is an extension of radiotap header or a new kind of header. We should maybe line up with Nicola Baldo who designed the radiotap header implementation.

I will start doing the "obvious" updates and I propose to pass directly the txvector information to PcapSniffRxEvent.
Comment 3 sebastien.deronne 2015-06-06 06:21:20 EDT
I of course meant PcapSniffTxEvent.

Since there is no distinction in ns-3 between rxvector and txvector, txvector will be used for both PcapSniffTxEvent and PcapSniffRxEvent. This could be changed later if needed.
Comment 4 Hany 2015-06-16 09:22:50 EDT
For the reported bug here is the current solution for it:

1. You can use RadioTap Header for MPDU Aggregation no need to replace with PPI Header. In ns3 you just need to extend the RadioTap Header and include A-MPDU Status Flag similar to the specifications mentioned in RadioTap Header website. When adding this new flag, you should pay attention for the alignment requirements. In the current implementation of the RadioTap Header in ns3, there is processing for the alignment and addition of the padding bytes required by various flags.

2. In the TxVector, you need to pass IsAggregated Flag similar to 802.11-2012 standard, so you can use it to determine whether to include A-MPDU Status Flag in the RadioTap Header or not.

3. The length Field in the A-MPDU SubFrame occupies 16 bits in the current implementation which is wrong and it should occupy 12 bits. As a result a shift of 4 bits to the left is required.

4. When MPDU aggregation is enabled in Wifi Device, the Wifi driver will provide MPDU SubFrames without the Deimiter and Padding. This is not the way done in ins3. ns3 provides MPDU is with its delimiter and padding and store it in the PCAP file. For this reason, WireShark interprets the QoS Frames as different type of frames.

5. Please find the following link, in which I have long discussion with people from Wireshark community to discuss this problem:

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=11277
Comment 5 sebastien.deronne 2015-06-16 09:38:27 EDT
Hany, thanks for this, it will certainly help to start with this.
Or have you worked on a patch already?
Comment 6 Hany 2015-06-16 09:55:15 EDT
You are welcome Sebestian. However, my current source files contain implementation for 802.11ad which is still not stable yet. After finishing implementation of 802.11ad, I will contribute to ns3 project. For the time being, I am happy to answer you and provide you with the modifications I did to fix the PCAP problem but not as a patch as this moment. Please check the attached wireshark file with my MPDU aggregation enabled.
Comment 7 Hany 2015-06-16 09:55:50 EDT
Created attachment 2066 [details]
Capture File with MPDU Status Flag in RadioTap Header
Comment 8 sebastien.deronne 2015-06-16 10:13:28 EDT
Ok, I will check with you for a solution.
Comment 9 Hany 2015-06-21 11:18:21 EDT
Hi Sebastien,

Please find the attached patch to fix the problem of the wrong information in the PCAP file. Here is a list of the modifications I did:

1. radio-tap-header in the network module
Add A-MPDU Flag support + Take care of Alignment according to the specifications of the Radio Tap Header since was missing in the current implementation.

2. wifi-tx-vector
Add MPDU aggregation support similar to the standard.

3. yans-wifi-helper
Check if A-MPDU aggregation is supported and based on that add A-MPDU Flag in the Radio Tap Header.

4. yans-wifi-phy
Pass new arguments to the Sniff Functions.

5. wifi-phy
Modify the Sniff function to accept new arguments and add new struct snrDbm. The new struct is necessary because of the limitation in the template, we can pass 8 arguments only. So I had to merge both signal and nise in one struct.

6. mac-low 
Check if A-MPDU aggregation is supported in the ForwardDown function by checking if queueSize is larger than one.
Comment 10 Hany 2015-06-21 11:19:01 EDT
Created attachment 2071 [details]
Patch file to solve the PCAP problem
Comment 11 sebastien.deronne 2015-07-01 16:24:35 EDT
Created attachment 2076 [details]
patch to add MCS and A-MPDU status fields in radiotap header

Hany's patch is working and almost ok.

I made a new patch based on what Hany has done but with those main improvements:

- add support for the A-MPDU reference number (instead of the dummy value used in Hany's patch).
- extend radiotap header with all existing bits (with dummy values for those that are not yet supported).
- add support for the MCS field (802.11n PHY information).

I am waiting for some feedback before committing this fix.
Comment 12 sebastien.deronne 2015-07-02 13:01:20 EDT
Created attachment 2077 [details]
patch to add correct padding, MCS and A-MPDU status fields in radiotap header

Made a new patch, I had forgotten the padding (mainly) in the previous patch.
Comment 13 sebastien.deronne 2015-07-08 17:48:35 EDT
fixed with changeset 11479:a3dcf66928f3