Bug 2941

Summary: Order bit of Frame control field of WifiMacHeader not correctly set for some frames
Product: ns-3 Reporter: Rediet <redieteab.orange>
Component: wifiAssignee: sebastien.deronne
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs
Priority: P3    
Version: ns-3.28   
Hardware: All   
OS: All   
Attachments: patch to change default order bit

Description Rediet 2018-07-06 08:47:33 UTC
Created attachment 3123 [details]
patch to change default order bit

The order bit (m_ctrlOrder attribute of WifiMacHeader) was not correctly set in some management and control frames, namely ADDBA Request/Response and DELBA among others.

The IEEE 802.11-2016 standard states that:
9.2.4.1.10 +HTC/Order subfield
The +HTC/Order subfield is 1 bit in length. It is used for two purposes:
— It is set to 1 in a non-QoS Data frame transmitted by a non-QoS STA to indicate that the frame contains an MSDU, or fragment thereof, that is being transferred using the StrictlyOrdered service class.
— It is set to 1 in a QoS Data or Management frame transmitted with a value of HT_GF, HT_MF, or VHT for the FORMAT parameter of the TXVECTOR to indicate that the frame contains an HT Control field.
Otherwise, the +HTC/Order subfield is set to 0.
NOTE—The +HTC/Order subfield is always set to 0 for frames transmitted by a DMG STA.

This led to erroneous decoding in pcaps; Wireshark expected to have an HT/VHT/HE control field, since the bit was set to 1 for these management frames.

Considering that, within the same standard, it is stated that:
5.1.3 MSDU ordering
[...]
Note that the use of the StrictlyOrdered service class is obsolete and the 
StrictlyOrdered service class might be removed in a future revision of the standard.
[...]

It is thus better to set this order bit to 0 by default and only to 1 when needed (i.e. once HT/VHT/HE control fields will be implemented).
Comment 1 Rediet 2018-07-06 08:48:50 UTC
All tests pass except
    devices-mesh-dot11s-regression
    devices-mesh-flame-regression
    routing-aodv-regression
Pcaps should probably be regenerated. Is there a way to do that?
Comment 2 sebastien.deronne 2018-07-09 16:01:13 UTC
(In reply to Rediet from comment #1)
> All tests pass except
>     devices-mesh-dot11s-regression
>     devices-mesh-flame-regression
>     routing-aodv-regression
> Pcaps should probably be regenerated. Is there a way to do that?

You can use the -u option.
Comment 3 sebastien.deronne 2018-07-09 16:02:03 UTC
(In reply to Rediet from comment #0)
> Created attachment 3123 [details]
> patch to change default order bit
> 
> The order bit (m_ctrlOrder attribute of WifiMacHeader) was not correctly set
> in some management and control frames, namely ADDBA Request/Response and
> DELBA among others.
> 
> The IEEE 802.11-2016 standard states that:
> 9.2.4.1.10 +HTC/Order subfield
> The +HTC/Order subfield is 1 bit in length. It is used for two purposes:
> — It is set to 1 in a non-QoS Data frame transmitted by a non-QoS STA to
> indicate that the frame contains an MSDU, or fragment thereof, that is being
> transferred using the StrictlyOrdered service class.
> — It is set to 1 in a QoS Data or Management frame transmitted with a value
> of HT_GF, HT_MF, or VHT for the FORMAT parameter of the TXVECTOR to indicate
> that the frame contains an HT Control field.
> Otherwise, the +HTC/Order subfield is set to 0.
> NOTE—The +HTC/Order subfield is always set to 0 for frames transmitted by a
> DMG STA.
> 
> This led to erroneous decoding in pcaps; Wireshark expected to have an
> HT/VHT/HE control field, since the bit was set to 1 for these management
> frames.
> 

+1
> Considering that, within the same standard, it is stated that:
> 5.1.3 MSDU ordering
> [...]
> Note that the use of the StrictlyOrdered service class is obsolete and the 
> StrictlyOrdered service class might be removed in a future revision of the
> standard.
> [...]
> 
> It is thus better to set this order bit to 0 by default and only to 1 when
> needed (i.e. once HT/VHT/HE control fields will be implemented).
Comment 4 Rediet 2018-07-10 03:28:59 UTC
(In reply to sebastien.deronne from comment #2)
> (In reply to Rediet from comment #1)
> > All tests pass except
> >     devices-mesh-dot11s-regression
> >     devices-mesh-flame-regression
> >     routing-aodv-regression
> > Pcaps should probably be regenerated. Is there a way to do that?
> 
> You can use the -u option.

Thanks. Should I update the patch with the new set of pcaps? Or will you do it upon integration in ns-3-dev?
Comment 5 sebastien.deronne 2018-07-10 05:36:47 UTC
(In reply to Rediet from comment #4)
> (In reply to sebastien.deronne from comment #2)
> > (In reply to Rediet from comment #1)
> > > All tests pass except
> > >     devices-mesh-dot11s-regression
> > >     devices-mesh-flame-regression
> > >     routing-aodv-regression
> > > Pcaps should probably be regenerated. Is there a way to do that?
> > 
> > You can use the -u option.
> 
> Thanks. Should I update the patch with the new set of pcaps? Or will you do
> it upon integration in ns-3-dev?

As u like, I always run tests before pushing, so I can do it if you want.

I will see whether I am still pushing this for the upcoming release, but I guess it should not hurt.
Comment 6 Rediet 2018-07-10 05:40:52 UTC
(In reply to sebastien.deronne from comment #5)
> As u like, I always run tests before pushing, so I can do it if you want.
> 
> I will see whether I am still pushing this for the upcoming release, but I
> guess it should not hurt.

It would be great if you could (for both the generation part and pushing it for the coming release). Thanks.
Comment 7 sebastien.deronne 2018-07-11 05:34:50 UTC
Fix pushed in changeset 13683:33e49d55cb16