Hi, Nice! I haven't gotten through it all yet, just some comments here also: > > Here a short overview of the patches in the series: > wifi: mac80211: Fix virtual monitor interface creation > This basically covers questions I have about the monitor handling, > just posed as a patch. (It has it's own comment section). Right, see my comments there. > wifi: mac80211: Fix debugfs handling for virtual monitor > Found during QA testing of this series. Otherwise unrelated. Haven't really figured this one out, I should ask Benjamin to take a look. > wifi: mac80211: Cleanup sta TXQs on flush > Another thing I found during QA while working on an unrelated > regression I caused. That looks fine, I think. > wifi: mac80211: Always provide the MMPDU TXQ > When only using TXQs for TX we can't have this queue optionally. > We probably want to discuss how to make that mandatory for all > drivers with native TXQ support. But this works, allowing to > soften the probably desired updates to (some) TXQ drivers. Yeah not sure how we could deal with the drivers - after all none other than iwlwifi opted in to this, I think, even through it exists for quite a while now. > wifi: mac80211: Convert vif->txq to an array > We need some more TXQs for the patch below. STA TXQs are already > in an array, so just put the VIF TXQ into one, too. Sure, looks mostly fine. > wifi: mac80211: Add new TX queues to replace legacy TX > This here starts the "core" of the patch series. > It's creating all the missing TXQs and updates the support > function for them. It also directly switches traffic to them, > when possible. (Only offchannel is sticking to legacy TX here.) I only got to this one for the individual reviews. > wifi: mac80211: Stop using legacy TX path > This is dropping the legacy TX support and moves offchannel TX to > the new alternate TXQ path: So far named TXQ_NOQUEUE. > With that mac80211 has two TX paths both using TXQ: > - The existing one, which uses the TXQ for queuing and > - TXQ_NOQUEUE. Which just puts frames into a TXQ and > immediately sends out the frame by also calling drv_tx() for > it. There never can be more than one frame in any of these > TXQs. They never see a wake_tx_queue call by the driver or > mac80211. Just reading the description I wondered what we gain here, but of course all the internal code path differences are removed, that's nice :-) > wifi: mac80211: Call ieee80211_tx_h_select_key only once > This is just a simple drive-by optimization. Not needed. Also I guess independent? > wifi: mac80211: Rename IEEE80211_TX_INTFL_OFFCHAN_TX_OK > Also functionality irrelevant. Just seems to be a good way to use > that as the official selector for the TXQ_NOQUEUE path and > represent that in the name. makes sense > wifi: mac80211: Simplify AMPDU handling > Also a kind of drive-by optimization. Uses TXQs to buffer frames > when AMPDU is started/stopped and even gets rid > IEEE80211_QUEUE_STOP_REASON_AGGREGATION. That's nice though, also a really nice cleanup. Not sure I'd necessarily call it an optimisation. > wifi: mac80211: Migrate TX to kthread > Moves all TX operation except TXQ_NOQUEUE to a new kthread. > This hooks into the existing txq scheduling and uses local->active_txqs > to determine which TXQs need to run. We may also consider > forcing all drivers to use ieee80211_txq_schedule_start() to get > rid of the code figuring that out per run... Forcing drivers is always hard ... will need to take a closer look at this. > wifi: mac80211: Drop wake_txqs_tasklet > Another drive-by cleanup/optimization which became possible due > to the kthread patch above. Feels a bit like that should just be part of the kthread patch? Why keep a useless tasklet around after that? > wifi: mac80211: Cleanup *ieee80211_wake_txq* naming > And finally a patch just renaming a few functions. Not sure if > that avoids or adds confusion... :) > As an outline: > When there are no fundamental concerns of the mayor changes in this > series I would try again to pick apart the PS buffering and filtered > frame mess. Which probably will have to be in one patch. Last time I tried > that without kthread it was not possible to rip it out independently. I don't see _fundamental_ concerns right now, but I'd probably still hold off on that for a while, perhaps even put that into a separate kernel release just in case? > From a performance point of view this series seems to be ok... :) > I did run some quick tests with my "normal" home network close to an > stil unpatched AP: 60s long tests with iperf3 using tcp got between > 484-524 MBit/s while the patched kernel was between 471-521MBit/s using > SMP. Uniprocessor performance is better in both cases, 484-530MBit/s for > the original and 563-676 MBIT/s for the patched variant. Interesting. Thanks for doing this! I need to review it much more carefully, but it looks really nice as far as I've seen. johannes
