net80211: Fix traffic hang on STA/AP VAPs on a multi-VAP interface

[adrian]

Summary

This took an embarrasingly long time to find.

The state changes for a radio with a STA /and/ AP VAP gets a bit messy.
The AP maps are marked as waiting, waiting for the STA AP to find a
channel to use before the AP VAPs become active.

However, the code path that clears the OACTIVE flag on a VAP only runs
during a successful run of ieee80211_newstate_cb().

So here is how it goes:

• the STA VAP goes down and needs to scan;
• the AP vap goes RUN->INIT; but it doesn't YET call ieee80211_newstate_cb();
• a send on this VAP causes the VAP to set the OACTIVE flag here;
• then the STA VAP finishes scan and goes to RUN;
• .. then the AP VAP goes INIT->RUN;
• /then/ the ieee80211_newstate_cb() is called, but it sees the state go RUN->RUN;
• .. which results in the OACTIVE flag never being cleared.

This clears the OACTIVE flag when a VAP transitions RUN->RUN; the
driver layer or net80211 layer can set it if required in a subsequent
transmit.

[adrian]

link

[bz]

While your description is verbose and expressive I still found it a bit hard to follow; if I get it correctly wakeupwaiting calls hostap_newstate in your case; that updates INIT->RUN, calls ieee80211_create_ibss() which in turn ends up in ieee80211_sta_join1() which then triggers the deferred callback for the AP vap where we only see RUN->RUN (and funnily enough calls back into hostap_newstate again which only hits the debug printf.

You probably want to be clear in the commit message that "a send on this VAP" means the AP VAP and add the call-graphs there in full which will explain some of the longer comments better in one place.

sys/net80211/ieee80211_proto.c

2589

Why don't you move half of the work from below up here but duplicate it?
And then ta single sentence is probably enough to be added that a RUN->RUN transition can also happen on STA + AP VAPs. The remainder of the information is in the proposed commit message already or should probably be there with the remainder of the call-chain. I feel splitting the information over three places will only make it harder to understand later what was going on or why the change was made.

[bz]

Was there a PR for this somewhere? If so, please mention that.

[adrian]

In D34920#791558, @bz wrote:

Was there a PR for this somewhere? If so, please mention that.

I don't think I created a PR at the time, no.

[adrian]

Take on comments from bz@

[adrian]

hi! see if those comment/commit updates help!