Bug 2408

Summary: Simulation fails when 802.11n/ac is running with HT Minstrel and pcap enabled
Product: ns-3 Reporter: sebastien.deronne
Component: wifiAssignee: sebastien.deronne
Status: RESOLVED FIXED    
Severity: normal CC: matis18, ns-bugs, tommaso.pecorella
Priority: P5    
Version: ns-3.25   
Hardware: All   
OS: All   
Attachments: script to reproduce
patch to fix pcap issues
patch for the root issue
path for the root issue v2
patch for the root issue v2

Description sebastien.deronne 2016-05-10 17:49:44 UTC
Created attachment 2427 [details]
script to reproduce

This is a bug reported by Ad Lee (https://groups.google.com/forum/#!topic/ns-3-users/FimhsQ_oJsA). The following issue is encountered when simulating 802.11ac network with MinstrelHtWifiManager(see attached script to reproduce) and when pcap enabled:

./waf --run "vht --pcap=true" 

assert failed. cond="m_buffer.GetSize () >= start + length", file=../src/network/model/packet.cc, line=234 ".
Comment 1 sebastien.deronne 2016-05-21 08:54:41 UTC
also fails for 802.11n simulation
Comment 2 sebastien.deronne 2016-05-21 10:07:57 UTC
Created attachment 2442 [details]
patch to fix pcap issues
Comment 3 Tommaso Pecorella 2016-05-21 10:13:35 UTC
Just a question: why in the first place the packet was trimmed ?
Comment 4 sebastien.deronne 2016-05-21 15:24:55 UTC
I honestly have no clue. I looked at the output before and after this change, pcap packets are still correctly printed without any difference.
Comment 5 Tommaso Pecorella 2016-05-21 15:51:13 UTC
I guess that the answer is in the comment:
/* For PCAP file, MPDU Delimiter and Padding should be removed by the MAC Driver */

The trimming was there to remove the padding (if there was a padding at all).

I don't mind the fix, but I fear that it's going to hide another bug.
Comment 6 sebastien.deronne 2016-05-21 15:57:56 UTC
You might be right, I will try to dig into this.
Comment 7 Tommaso Pecorella 2016-05-21 17:10:51 UTC
Created attachment 2447 [details]
patch for the root issue

The real problem was in the MinstrelHt.
It wasn't setting correctly the aggregation flag in TxVector, and the writer was blindly removing a header that wasn't there.
Comment 8 Matías Richart 2016-05-22 13:50:12 UTC
(In reply to Tommaso Pecorella from comment #7)
> Created attachment 2447 [details]
> patch for the root issue
> 
> The real problem was in the MinstrelHt.
> It wasn't setting correctly the aggregation flag in TxVector, and the writer
> was blindly removing a header that wasn't there.

You are right, there is a bug there.
I modified a bit your patch to maintain the idea of not enabling aggregation when sampling.
Comment 9 Matías Richart 2016-05-22 13:50:48 UTC
Created attachment 2450 [details]
path for the root issue v2
Comment 10 Matías Richart 2016-05-22 13:53:27 UTC
Created attachment 2451 [details]
patch for the root issue v2
Comment 11 sebastien.deronne 2016-05-22 15:18:08 UTC
Tommaso, thanks a lot for looking at it, that explains why I never got the issue with constant rate. Matias, thanks for reacting as well, I'll tests the patch, if it's working I think we can safely push to ns-3-dev.
Comment 12 Tommaso Pecorella 2016-05-22 15:34:35 UTC
You're welcome.

I have (just) another question. Mind that I'm not so fluent in wifi.
If I got it right, Minstrel sets the WifiTxVector for each packet.
Then the packet is queued, and an access is requested.
When the access is granted, at that moment the aggregation happens.

Now, what happens if two packets are in the queue and one was queued when the node was in sampling mode, and then other not ?

More in general I could ask why the fact that the packet can be aggregated or not is in the WifiTxVector, and if this is effectively checked.
Without checking the standard, I could understand that a packet could be aggregation-less, and that a station could have its aggregation enabled or not, but the two seems to be more orthogonal properties than joint ones.

(In reply to sebastien.deronne from comment #11)
> Tommaso, thanks a lot for looking at it, that explains why I never got the
> issue with constant rate. Matias, thanks for reacting as well, I'll tests
> the patch, if it's working I think we can safely push to ns-3-dev.
Comment 13 sebastien.deronne 2016-06-04 07:40:01 UTC
Matias has maybe a better idea about what is happening sampling mode is used?

WifiTxVector defines a flag to know whether a frame is aggregated or not.
It was mainly used for pcap so far, and I guess it is used now in HT Minstrel. 
We should maybe look further to see if everything is in line, but I suggest to create another thread for this later on, and deliver the patch asap.
Comment 14 Matías Richart 2016-06-05 05:33:03 UTC
(In reply to sebastien.deronne from comment #13)
> Matias has maybe a better idea about what is happening sampling mode is used?
> 
> WifiTxVector defines a flag to know whether a frame is aggregated or not.
> It was mainly used for pcap so far, and I guess it is used now in HT
> Minstrel. 
> We should maybe look further to see if everything is in line, but I suggest
> to create another thread for this later on, and deliver the patch asap.

Well, I made a quick check and txVector is not used to decide if a packet should be aggregated or not (or at least I couldn't find it). As Sebastien said, it is only use for pcap.
The intention in minstrel-ht was to use that flag for aggregation decisions of individual frames. So, there is a bug there.

I agree with Tommaso, about that the orthogonality, and I think that is followed in ns3. But as I said, currently we are not checking if individual frames are aggregation-less.
Comment 15 sebastien.deronne 2016-06-10 16:23:13 UTC
We need to keep those comments in mind.
In the meantime, patch should be delivered.
Comment 16 Tommaso Pecorella 2016-06-10 16:26:26 UTC
I agree, and I'd suggest to use my patch (i.e., to not use TxVector for the not-aggregate-this purpose).
Otherwise we'll just have an extremely tiny probability to trigger an obscure bug.


(In reply to sebastien.deronne from comment #15)
> We need to keep those comments in mind.
> In the meantime, patch should be delivered.
Comment 17 sebastien.deronne 2016-06-10 16:28:59 UTC
(In reply to Tommaso Pecorella from comment #16)
> I agree, and I'd suggest to use my patch (i.e., to not use TxVector for the
> not-aggregate-this purpose).
> Otherwise we'll just have an extremely tiny probability to trigger an
> obscure bug.
> 
> 
> (In reply to sebastien.deronne from comment #15)
> > We need to keep those comments in mind.
> > In the meantime, patch should be delivered.

Yes, I'll deliver the latest patch, my patch was not ok.
Comment 18 sebastien.deronne 2016-06-17 08:06:51 UTC
Fixed in changeset 12163:f328dd208ff7