Bug 2471

Summary:	unable to disable Block Ack agreement for 802.11n
Product:	ns-3	Reporter:	Tom Henderson <tomh>
Component:	wifi	Assignee:	sebastien.deronne
Status:	RESOLVED FIXED
Severity:	normal	CC:	ns-bugs
Priority:	P5
Version:	unspecified
Hardware:	All
OS:	All
Attachments:	proposed fix for now proposed solution for now patch of ht-wifi-network example documentation update

Description Tom Henderson 2016-08-07 15:12:19 UTC

Bug 2116 introduced patches to refactor the aggregation API, and introduce a number of new attributes, one for each access category, such as:

    .AddAttribute ("VO_BlockAckThreshold",
                   "If number of packets in VO queue reaches this value, "
                   "block ack mechanism is used. If this value is 0, block ack is never used.",
                   UintegerValue (0),
                   MakeUintegerAccessor (&RegularWifiMac::SetVoBlockAckThreshold),
                   MakeUintegerChecker<uint8_t> (0, 64))


In the current code base, these values all default to 0 and are not later set to a higher value by any of the ConfigureStandard() methods.

However, the condition that "if the value is 0, block ack is never used" is not enforced in the code.

EdcaTxopN::SetupBlockAckIfNeeded() checks:

  if ((m_blockAckThreshold > 0 && packets >= m_blockAckThreshold) || (packets > 1 && m_mpduAggregator != 0) || m_stationManager->HasVhtSupported ())

so it may be the case that m_mpduAggregator exists despite this threshold being zero.

Some questions:

1) should aggregation be allowed to be enabled if this threshold is zero?

2) in 802.11ac, we should not be able to disable this, if I understand correctly.  Should the comment that 'zero value disables block ack' be clarified as only holding true for 802.11n?

3) should the check on HasVhtSupported() also check whether the remote station has VhtSupported()? 

4) what should be the policy for defaults on a per-access-category basis?  Enabled for BE only, or enabled for BE, BK, and VI, or enabled for all?

5) Should the non-zero defaults for 802.11n be handled by directly configuring the attribute default to the value '2' instead of '0', or should it be handled in the Configure802.11n() post-initialization code?

Comment 1 sebastien.deronne 2016-08-13 10:51:44 UTC

I do not see anything about that parameter in the std, so I am maybe thinking to replace it. The standard defines dot11ImmediateBlockAckOptionImplemented to indicate whether the station does support Immediate Block Ack. I would then replace this parameter by a boolean to indicate whether Immediate Block Ack is enabled or not. It think it should be defined per device and not per AC.

Comment 2 Tom Henderson 2016-08-13 12:15:00 UTC

(In reply to sebastien.deronne from comment #1)
> I do not see anything about that parameter in the std, so I am maybe
> thinking to replace it. The standard defines
> dot11ImmediateBlockAckOptionImplemented to indicate whether the station does
> support Immediate Block Ack. I would then replace this parameter by a
> boolean to indicate whether Immediate Block Ack is enabled or not. It think
> it should be defined per device and not per AC.

I don't care so much about being able to disable it across the device; is it a feature needed?  I believe that by setting the thresholds to very high value, the existing API should be able to disable it.

What about a patch along these lines:


-   if ((m_blockAckThreshold > 0 && packets >= m_blockAckThreshold) || (packets > 1 && m_mpduAggregator != 0) || m_stationManager->HasVhtSupported ())
+   if ((m_blockAckThreshold > 0 && packets >= m_blockAckThreshold) || m_stationManager->HasVhtSupported ())

and

    .AddAttribute ("VO_BlockAckThreshold",
                   "If number of packets in VO queue reaches this value, "
-                    "block ack mechanism is used. If this value is 0, block ack is never used.",
+                    "block ack mechanism is used. If this value is set to a very large value, block ack is never used.",
                   UintegerValue (0),
                   MakeUintegerAccessor (&RegularWifiMac::SetVoBlockAckThreshold),
                   MakeUintegerChecker<uint8_t> (0, 64))


and then set the defaults to 1 packet.

Comment 3 sebastien.deronne 2016-08-16 15:56:10 UTC

(In reply to Tom Henderson from comment #2)
> (In reply to sebastien.deronne from comment #1)
> > I do not see anything about that parameter in the std, so I am maybe
> > thinking to replace it. The standard defines
> > dot11ImmediateBlockAckOptionImplemented to indicate whether the station does
> > support Immediate Block Ack. I would then replace this parameter by a
> > boolean to indicate whether Immediate Block Ack is enabled or not. It think
> > it should be defined per device and not per AC.
> 
> I don't care so much about being able to disable it across the device; is it
> a feature needed? 

It seemed to be more standard compliant. This parameter could be used only for non-HT, and have HT Block ACK always enabled for 802.11n/ac.

I believe that by setting the thresholds to very high
> value, the existing API should be able to disable it.

Setting to 0 disable Block ACK.

> 
> What about a patch along these lines:
> 
> 
> -   if ((m_blockAckThreshold > 0 && packets >= m_blockAckThreshold) ||
> (packets > 1 && m_mpduAggregator != 0) || m_stationManager->HasVhtSupported
> ())
> +   if ((m_blockAckThreshold > 0 && packets >= m_blockAckThreshold) ||
> m_stationManager->HasVhtSupported ())

It will cause issues for 802.11n.
If the user does not set it, A-MPDU will crash or will behave not as expected.

> 
> and
> 
>     .AddAttribute ("VO_BlockAckThreshold",
>                    "If number of packets in VO queue reaches this value, "
> -                    "block ack mechanism is used. If this value is 0, block
> ack is never used.",
> +                    "block ack mechanism is used. If this value is set to a
> very large value, block ack is never used.",
>                    UintegerValue (0),
>                    MakeUintegerAccessor
> (&RegularWifiMac::SetVoBlockAckThreshold),
>                    MakeUintegerChecker<uint8_t> (0, 64))
> 
> 
> and then set the defaults to 1 packet.

Comment 4 Tom Henderson 2016-08-17 01:15:52 UTC

Are you suggesting that there is a per-device (not per-AC) flag to enable/disable block ack, and MPDU aggregation can occur even if this is disabled (just using normal acks)?  And the existing attributes are preserved but the bit about the special '0' value is removed?

What happens if a STA is configured not to use block ack but gets a request from the peer?

Comment 5 sebastien.deronne 2016-08-17 15:57:55 UTC

(In reply to Tom Henderson from comment #4)
> Are you suggesting that there is a per-device (not per-AC) flag to
> enable/disable block ack, 

Indeed.

and MPDU aggregation can occur even if this is
> disabled (just using normal acks)?  

It won't matter of this flag, and will use HT immediate block ACK (which is different from immediate block ACK and always supported for HT/VHT deviced).

And the existing attributes are
> preserved but the bit about the special '0' value is removed?

We would remove those attributes in EdcaTxopN, and add this new parameter.

> 
> What happens if a STA is configured not to use block ack but gets a request
> from the peer?

The is known from the Capability Information, so the peer will never do so.

Comment 6 Tom Henderson 2016-09-08 13:44:17 UTC

We had a comment from someone off-list that, in practice, block ack is typically only used for best effort, and definitely not for voice.

Comment 7 sebastien.deronne 2016-11-01 12:29:44 UTC

(In reply to Tom Henderson from comment #6)
> We had a comment from someone off-list that, in practice, block ack is
> typically only used for best effort, and definitely not for voice.

Do you mean immediate Block Ack or HT immediate block ack?

Comment 8 sebastien.deronne 2016-12-14 12:58:55 UTC

I am looking back at this bug.
Why not simply adapt the documentation about this parameter, saying that for 802.11n  this won't have any effect if A-MPDU is enabled and that it is ignored in 802.11ac?

Also, we should change this check:

(packets > 1 && m_mpduAggregator != 0)

so that it effectively checks based on packet type, packet size and maximum A-MPDU size whether MPDU aggregation will be enabled for that transmission, otherwise it should not start ADDBA request.
OR we let MacLow triggers the transmission of ADDBA prior to the first A-MPDU transmission?

Comment 9 sebastien.deronne 2016-12-17 05:05:41 UTC

Created attachment 2706 [details]
proposed fix for now

I suggest to adapt the API documentation for now, and just make sure we establish BA session if A-MPDU is enabled for that tid (size > 0).

I plan a bigger improvement for next year to have something much more similar to what is implemented in Linux. My understanding is that BA session is established by rate adaptation callback when aggregation will be used. I'll open a different thread for this.

Since the standard only mentions BA session should be established prior to A-MPDU, we are still compliant but we sometimes establish a BA session while it will never be effectively used (i.e. if aggregation is not performed). For 802.11ac, current behavior is fine and BA session should always be established.

Comment 10 sebastien.deronne 2016-12-17 06:27:24 UTC

Created attachment 2707 [details]
proposed solution for now

add missing check to avoid segfault

Comment 11 Tom Henderson 2016-12-20 02:05:49 UTC

(In reply to sebastien.deronne from comment #10)
> Created attachment 2707 [details]
> proposed solution for now
> 
> add missing check to avoid segfault

I am OK with at least minimally fixing the documentation to align with the current behavior, but is it possible to disable block acks and aggregation for VO when using 11n?  It is not clear to me from perusing the code tonight.  Aggregation gets enabled for all classes when SetHtSupported() is called.  Then this is eventually called:

      GetVOQueue ()->GetMpduAggregator ()->SetMaxAmpduSize (m_voMaxAmpduSize);

where it has value of 0.  Then this value is only consulted in EdcaTxopN::NeedFragmentation-- is that correct understanding?

Comment 12 sebastien.deronne 2016-12-21 14:44:53 UTC

(In reply to Tom Henderson from comment #11)
> (In reply to sebastien.deronne from comment #10)
> > Created attachment 2707 [details]
> > proposed solution for now
> > 
> > add missing check to avoid segfault
> 
> I am OK with at least minimally fixing the documentation to align with the
> current behavior, but is it possible to disable block acks and aggregation
> for VO when using 11n?  It is not clear to me from perusing the code
> tonight.  Aggregation gets enabled for all classes when SetHtSupported() is
> called.  Then this is eventually called:
> 
>       GetVOQueue ()->GetMpduAggregator ()->SetMaxAmpduSize
> (m_voMaxAmpduSize);
> 
> where it has value of 0.  Then this value is only consulted in
> EdcaTxopN::NeedFragmentation-- is that correct understanding?

If you set A-MPDU size for AC_VO to 0, A-MPDU will be disabled. 
If you also set VO_BlockAckThreshold to 0 (default), then block ack will also be disabled. Does this answer your question?
Maybe we should add some use cases to our doc.

Comment 13 sebastien.deronne 2016-12-28 13:27:14 UTC

Fix delivered in changeset 12493:965f337cf77c in order to fix regression failure for wifi-aggregation after my delivery for bug 2591.
Can we close this bug or is there still questions/remarks?

Comment 14 Tom Henderson 2016-12-28 15:01:02 UTC

(In reply to sebastien.deronne from comment #13)
> Fix delivered in changeset 12493:965f337cf77c in order to fix regression
> failure for wifi-aggregation after my delivery for bug 2591.
> Can we close this bug or is there still questions/remarks?

I extended the ht-wifi-network example to allow user to set the User Priority in the manner documented in our documentation:

https://www.nsnam.org/docs/models/html/wifi-user.html#selection-of-the-access-category-ac

I also added configurable MCS option.

When I run with MCS 7, User Priority 7, I get same throughput as MCS 7, User Priority 0.

./waf --run 'ht-wifi-network --mcs=7 --userPriority=0'

MCS value		Channel width		short GI		Throughput
7			20 MHz			0			60.1648 Mbit/s


./waf --run 'ht-wifi-network --mcs=7 --userPriority=7
MCS value		Channel width		short GI		Throughput
7			20 MHz			0			60.1648 Mbit/s


So it seems that aggregation and BA must still be enabled?

Comment 15 Tom Henderson 2016-12-28 15:01:37 UTC

Created attachment 2720 [details]
patch of ht-wifi-network example

Comment 16 sebastien.deronne 2017-01-01 10:45:53 UTC

I am more suspecting the priority did not work correctly. Even with same aggregation configuration, results would be different because AC_VO use different AIFS and CW values.

Comment 17 sebastien.deronne 2017-01-01 10:55:50 UTC

(In reply to sebastien.deronne from comment #16)
> I am more suspecting the priority did not work correctly. Even with same
> aggregation configuration, results would be different because AC_VO use
> different AIFS and CW values.

Please give a try with TOS value 0xc0

Comment 18 sebastien.deronne 2017-01-07 05:22:02 UTC

(In reply to sebastien.deronne from comment #17)
> (In reply to sebastien.deronne from comment #16)
> > I am more suspecting the priority did not work correctly. Even with same
> > aggregation configuration, results would be different because AC_VO use
> > different AIFS and CW values.
> 
> Please give a try with TOS value 0xc0

Tom, was it also successful for you when setting TOS as suggested? Then I think this bug can be closed.

Comment 19 Tom Henderson 2017-01-09 01:46:46 UTC

(In reply to sebastien.deronne from comment #18)
> (In reply to sebastien.deronne from comment #17)
> > (In reply to sebastien.deronne from comment #16)
> > > I am more suspecting the priority did not work correctly. Even with same
> > > aggregation configuration, results would be different because AC_VO use
> > > different AIFS and CW values.
> > 
> > Please give a try with TOS value 0xc0
> 
> Tom, was it also successful for you when setting TOS as suggested? Then I
> think this bug can be closed.

Yes, I tried with a user priority instead of a TOS.  I checked and it seems now unused for 0xc0.  

In light of my mistake, I'd like to propose a documentation patch on how to set TOS and how it eventually maps to an Access Category.  If you agree, let's patch the documentation and then close this bug.

Comment 20 Tom Henderson 2017-01-09 01:47:50 UTC

Created attachment 2742 [details]
documentation update

Comment 21 sebastien.deronne 2017-01-09 04:45:48 UTC

(In reply to Tom Henderson from comment #19)
> (In reply to sebastien.deronne from comment #18)
> > (In reply to sebastien.deronne from comment #17)
> > > (In reply to sebastien.deronne from comment #16)
> > > > I am more suspecting the priority did not work correctly. Even with same
> > > > aggregation configuration, results would be different because AC_VO use
> > > > different AIFS and CW values.
> > > 
> > > Please give a try with TOS value 0xc0
> > 
> > Tom, was it also successful for you when setting TOS as suggested? Then I
> > think this bug can be closed.
> 
> Yes, I tried with a user priority instead of a TOS.  I checked and it seems
> now unused for 0xc0.  
> 
> In light of my mistake, I'd like to propose a documentation patch on how to
> set TOS and how it eventually maps to an Access Category.  If you agree,
> let's patch the documentation and then close this bug.

Tom, I am fine with that, thanks.

Comment 22 Tom Henderson 2017-01-10 19:48:38 UTC

(In reply to sebastien.deronne from comment #21)
> (In reply to Tom Henderson from comment #19)
> > (In reply to sebastien.deronne from comment #18)
> > > (In reply to sebastien.deronne from comment #17)
> > > > (In reply to sebastien.deronne from comment #16)
> > > > > I am more suspecting the priority did not work correctly. Even with same
> > > > > aggregation configuration, results would be different because AC_VO use
> > > > > different AIFS and CW values.
> > > > 
> > > > Please give a try with TOS value 0xc0
> > > 
> > > Tom, was it also successful for you when setting TOS as suggested? Then I
> > > think this bug can be closed.
> > 
> > Yes, I tried with a user priority instead of a TOS.  I checked and it seems
> > now unused for 0xc0.  
> > 
> > In light of my mistake, I'd like to propose a documentation patch on how to
> > set TOS and how it eventually maps to an Access Category.  If you agree,
> > let's patch the documentation and then close this bug.
> 
> Tom, I am fine with that, thanks.

I just pushed the documentation patch; I am OK to close this.

Comment 23 sebastien.deronne 2017-01-11 03:36:14 UTC

(In reply to Tom Henderson from comment #22)
> (In reply to sebastien.deronne from comment #21)
> > (In reply to Tom Henderson from comment #19)
> > > (In reply to sebastien.deronne from comment #18)
> > > > (In reply to sebastien.deronne from comment #17)
> > > > > (In reply to sebastien.deronne from comment #16)
> > > > > > I am more suspecting the priority did not work correctly. Even with same
> > > > > > aggregation configuration, results would be different because AC_VO use
> > > > > > different AIFS and CW values.
> > > > > 
> > > > > Please give a try with TOS value 0xc0
> > > > 
> > > > Tom, was it also successful for you when setting TOS as suggested? Then I
> > > > think this bug can be closed.
> > > 
> > > Yes, I tried with a user priority instead of a TOS.  I checked and it seems
> > > now unused for 0xc0.  
> > > 
> > > In light of my mistake, I'd like to propose a documentation patch on how to
> > > set TOS and how it eventually maps to an Access Category.  If you agree,
> > > let's patch the documentation and then close this bug.
> > 
> > Tom, I am fine with that, thanks.
> 
> I just pushed the documentation patch; I am OK to close this.

Thanks Tom