On Sun, Apr 24, 2016 at 01:25:31PM +0800, Nathanael Rensen wrote:
> I have been using an rt2860 hostap for a few years and I have discovered
> that AMRR does not work properly for this driver. The symptom is that
> some stations get stuck at 1 Mbps and do not progress up to faster rates.
>
> Unlike many drivers, rt2860 does not keep the ieee80211_amrr_node on its
> rt2860_node. Instead it maintains an array of ieee80211_amrr_nodes on
> rt2860_softc indexed by WCID.
>
> This approach runs into trouble when a non-active node (e.g. when in
> IEEE80211_STA_COLLECT) exists with the same WCID as an active node.
> If during rt2860_updatestats() the non-active node is encountered prior
> to the active node, ieee80211_amrr_choose() performs the rate adjustment
> against the non-active node. The counters in the ieee80211_amrr_node
> structure get reset and then when the active node does show up in the
> iteration it doesn't get its rate adjusted.
>
> The approach that the diff below takes is to move the ieee80211_amrr_node
> onto the rt2860_node and maintain a WCID to rt2860_node mapping on
> rt2860_softc. This improves consistency with other drivers such as athn(4),
> bwi(4), iwn(4), rt2560 and others, which maintain the ieee80211_amrr_node
> on the device node respectively.
Interesting. It seems wrong to allow an aid to be reallocated while
another node that's using this aid is still cached. We'll run out
of space in the node cache long before AIDs are exhausted so the
current AID recycling behaviour makes no sense at all.
I'd like to know if the diff below also fixes the issue for you,
independently of your driver diff.
I can't test this myself right now, though, so if this diff
just blows up your system or something, don't be surprised :)
The idea is to prevent an aid from being reallocated until the
corresponding node is actually purged from the cache.
Index: ieee80211_node.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
retrieving revision 1.101
diff -u -p -r1.101 ieee80211_node.c
--- ieee80211_node.c 12 Apr 2016 14:33:27 -0000 1.101
+++ ieee80211_node.c 24 Apr 2016 06:22:03 -0000
@@ -1699,8 +1699,6 @@ ieee80211_node_leave(struct ieee80211com
if (ic->ic_node_leave != NULL)
(*ic->ic_node_leave)(ic, ni);
- IEEE80211_AID_CLR(ni->ni_associd, ic->ic_aid_bitmap);
- ni->ni_associd = 0;
ieee80211_node_newstate(ni, IEEE80211_STA_COLLECT);
#if NBRIDGE > 0
Your driver changes are interesting nevertheless. I won't be able to
test them myself during the coming week, so I'll look at them later.
> The diff below also introduces dedicated timers for AMRR and for scan
> instead of using the RT2860 GP interrupt, which also improves consistency
> with the way other drivers manage AMRR.
Can you please split your diff into separate submissions, one per topic?
That would make review and testing a lot easier.
