Created attachment 2326 [details]
Proposed patch

Only ConstantRate wifi manager is currently supported for HT and VHT.

We do no plan to extend Onoe in the short/average term for HT/VHT.
Ideal rate manager is being extended for HT/VHT by Tom, and will be ready in ns-3.25. HT Minstrel is also in progress, and is likely to be also part of ns-3.25. Please refer to bug 1797.

I thus suggest to reject this bug.
However, I am not very familiar with Onoe and high latency, so it's better that Tom and Matias give their opinion on this.

(In reply to sebastien.deronne from comment #2)
> Only ConstantRate wifi manager is currently supported for HT and VHT.

Well, HT/VHT rates are not selected, but the problem is not related to HT/VHT rates here. I am aware of this and it is another bug. The problem is that rate for ADDBA request is never selected if high latency rate control is used. I also wonder if we should use PushFront instead of m_low->StartTransmission at the end of EdcaTxopN::SendAddBaRequest, but definitely m_stationManager->PrepareForQueue should be called beforehand.

(In reply to sebastien.deronne from comment #2)
> Only ConstantRate wifi manager is currently supported for HT and VHT.
> 
> We do no plan to extend Onoe in the short/average term for HT/VHT.
> Ideal rate manager is being extended for HT/VHT by Tom, and will be ready in
> ns-3.25. HT Minstrel is also in progress, and is likely to be also part of
> ns-3.25. Please refer to bug 1797.
> 
> I thus suggest to reject this bug.
> However, I am not very familiar with Onoe and high latency, so it's better
> that Tom and Matias give their opinion on this.

I do not think we need to reject the bug if Alexander is interested in extending Onoe scope.

I am not familiar with Onoe and I don't see tests for it, but I have some questions about it.

- the tags (three per packet) do not seem to get cleaned up (removed) anywhere; should they?

- Alexander mentioned that ADDBA request misses this tag.  ADDBA request is a management action frame; what is the practical consequence of not tagging such frames?  I'm wondering what breaks if it is not tagged.

(In reply to Tom Henderson from comment #4)
> - the tags (three per packet) do not seem to get cleaned up (removed)
> anywhere; should they?

Probably they should be removed on MacLow when it calls GetDataTxVector, GetCtsToSelfTxVector, GetRtsTxVector, otherwise they remain attached to UDP and TCP packets after transmission over Wi-Fi network. They can only be accessed by WifiRemoteStationManager, so it may require some API changes. Nothing wrong happens, WifiRemoteStationManager checks that only one tag is attached at a time, it just wastes memory until packet reaches final destination.

> - Alexander mentioned that ADDBA request misses this tag.  ADDBA request is a management action frame; what is the practical consequence of not tagging such frames?  I'm wondering what breaks if it is not tagged.

GetDataTxVector is called in any case to find out frame duration and check whether RTS threshold is reached. If high latency rate control is used, GetDataTxVector requires that packet is tagged as it expects to get rate from tag, not directly from rate control.

#0  0x00007fffec02bcc9 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x00007fffec02f0d8 in __GI_abort () at abort.c:89
#2  0x00007fffecb54535 in __gnu_cxx::__verbose_terminate_handler() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007fffecb526d6 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007fffecb52703 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff2619b35 in ns3::WifiRemoteStationManager::GetDataTxVector (this=0x71c3c0, address=..., header=0x71c8f8, packet=...)
    at ../src/wifi/model/wifi-remote-station-manager.cc:729
#6  0x00007ffff25a8806 in ns3::MacLow::GetDataTxVector (this=0x71c760, packet=..., hdr=0x71c8f8) at ../src/wifi/model/mac-low.cc:1382
#7  0x00007ffff25a13e5 in ns3::MacLow::NeedRts (this=0x71c760) at ../src/wifi/model/mac-low.cc:827
#8  0x00007ffff25a0e27 in ns3::MacLow::StartTransmission (this=0x71c760, packet=..., hdr=0x71e2e0, params=..., listener=0x71e0c0) at ../src/wifi/model/mac-low.cc:792
#9  0x00007ffff268e54d in ns3::EdcaTxopN::SendAddBaRequest (this=0x71e220, dest=..., tid=0 '\000', startSeq=0, timeout=0, immediateBAck=true)
    at ../src/wifi/model/edca-txop-n.cc:1457
#10 0x00007ffff268cb7f in ns3::EdcaTxopN::SetupBlockAckIfNeeded (this=0x71e220) at ../src/wifi/model/edca-txop-n.cc:1315
#11 0x00007ffff267e37f in ns3::EdcaTxopN::NotifyAccessGranted (this=0x71e220) at ../src/wifi/model/edca-txop-n.cc:495
#12 0x00007ffff268f828 in ns3::EdcaTxopN::Dcf::DoNotifyAccessGranted (this=0x71e3c0) at ../src/wifi/model/edca-txop-n.cc:62
#13 0x00007ffff25ed17d in ns3::DcfState::NotifyAccessGranted (this=0x71e3c0) at ../src/wifi/model/dcf-manager.cc:165
#14 0x00007ffff25f095a in ns3::DcfManager::DoGrantAccess (this=0x71cdd0) at ../src/wifi/model/dcf-manager.cc:558
#15 0x00007ffff25f0cad in ns3::DcfManager::AccessTimeout (this=0x71cdd0) at ../src/wifi/model/dcf-manager.cc:575
#16 0x00007ffff25f717d in ns3::EventImpl* ns3::MakeEvent<void (ns3::DcfManager::*)(), ns3::DcfManager*>(void (ns3::DcfManager::*)(), ns3::DcfManager*)::EventMemberImpl0::Notify() (this=0x6572b0) at ./ns3/make-event.h:323
#17 0x00007fffeff47811 in ns3::EventImpl::Invoke (this=0x6572b0) at ../src/core/model/event-impl.cc:51
#18 0x00007fffeff4c3b8 in ns3::DefaultSimulatorImpl::ProcessOneEvent (this=0x7175f0) at ../src/core/model/default-simulator-impl.cc:149
#19 0x00007fffeff4c732 in ns3::DefaultSimulatorImpl::Run (this=0x7175f0) at ../src/core/model/default-simulator-impl.cc:202
#20 0x00007fffeff48548 in ns3::Simulator::Run () at ../src/core/model/simulator.cc:204
#21 0x000000000040b1ac in main (argc=1, argv=0x7fffffffe438) at ../scratch/wifi-bug.cc:100

Hi. I have several considerations.

It is true that Onoe is the only rate control that uses the high-latency API. However, Minstrel is also high-latency, but does not use the API. It emulates the MRR-chain in the manager.
Then, I think bug 889 is not totally correct (I was not aware of this bug).
I imagine Minstrel was implemented this way because with the high-latency API was not enough. Minstrel-HT follows this idea too.

I agree that the high-latency API is not used correctly for aggregation.
But, although PrepareForQueue is not called for A-MPDU, it is called for each MPDU, when enqueueing it.
Should we use the tags of the MPDUs? or, should we remove tagging MPDUs?

In my opinion, a correct extension of this API for A-MPDUs is to remove tagging of MPDUs (so as to ask for a rate only once). Then, ask the manager for a rate when the A-MPDU is complete, as suggested by Alexander.
This is also related with bug 2316

Regarding tag removal: the tags should only be removed when the packet fails all retransmissions or is received successfully. Buy I imagine that by that time, the packet is destroyed.

In summary:
- If we want to extend this API for A-MPDUs, we should consider this issue, jointly with bug 2316, and be careful of calling PrepareForQueue before a GetDataTxVector is called.

- Alexander, I recommend you use Minstrel and Minstrel-HT (coming soon), if you are interested in the rate control done in real devices.

Matias, do you have some updates for this bug?

(In reply to sebastien.deronne from comment #7)
> Matias, do you have some updates for this bug?

Hi.
I don't. However, I think that the check we are implementing in bug 1797 will solve the original issue, as won't be possible to use onoe with aggregation.

I can also look what is necessary to extend the high-latency functions for aggregation, but it won't be used by any rate adaptation algorithm.
Perhaps we can evaluate the relevance of having those functions as new high-latency algorithms as minstrel and misntrel-ht don't use them.

So, with the fix delivered for bug 2347, does it mean this bug should be closed as well?

(In reply to sebastien.deronne from comment #9)
> So, with the fix delivered for bug 2347, does it mean this bug should be
> closed as well?

I think the bug reference should be 1797.  I think that if Alexander wants to submit a new patch rebased on ns-3-dev that removes the constraint for this rate control, and document that it works now for Ht/Vht, we can keep it open; otherwise, close it.  Marking as NEEDINFO.

(In reply to Tom Henderson from comment #10)
> (In reply to sebastien.deronne from comment #9)
> > So, with the fix delivered for bug 2347, does it mean this bug should be
> > closed as well?
> 
> I think the bug reference should be 1797.  I think that if Alexander wants
> to submit a new patch rebased on ns-3-dev that removes the constraint for
> this rate control, and document that it works now for Ht/Vht, we can keep it
> open; otherwise, close it.  Marking as NEEDINFO.

The current patch from Alexander is not enough. For Onoe to work with HT/VHT, in addition to Alexander modifications, Onoe needs to be extended so as to use HT/VHT rates.

Matias, Alexander, does one of you plan to work on this implementation?

(In reply to sebastien.deronne from comment #12)
> Matias, Alexander, does one of you plan to work on this implementation?

I don't.

Should we then reject this bug because it is out of scope and that it will never be addressed?

Matias, what do you think about just accepting Alexander's patch for now (is there a harm in it as is?) and then moving the summary of the remaining work on this topic to a "suggested projects" page in case we find someone who wants to contribute for this feature?

(In reply to sebastien.deronne from comment #15)
> Matias, what do you think about just accepting Alexander's patch for now (is
> there a harm in it as is?) and then moving the summary of the remaining work
> on this topic to a "suggested projects" page in case we find someone who
> wants to contribute for this feature?

In my opinion Alexander's patch has one issue:
Currently, when not using the high-latency api the tx-vector is asked for the first MPDU which arrives to MacLow and that same tx-vector (m_currentTxVector) is used for the new A-MPDU.
With Alexander's patch, a new tx-vector is asked for the A-MPDU in PrepareForQueue.

However, I think the initial problem appears because m_currentTxVector is not used everywhere it should be.
I propose a new patch to solve this problem and also solves Alexander's initial issue.
To test it, it is required to disable the check for HT support in Onoe.

Created attachment 2625 [details]
Proposed patch v2

Alexander, could you provide your feedback/feeling on Matias's proposal?

Since Onoe Rate Control is the only High Latency one but cannot be used with HT, Matias suggested to reject this bug