Re: [PATCH net v2] net: usb: Remove disruptive netif_wake_queue in rtl8150_set_multicast
From: Jakub Kicinski <kuba@kernel.org>
Date: 2025-09-23 23:37:29
Also in:
linux-kernel-mentees, linux-usb, lkml
On Wed, 24 Sep 2025 01:20:39 +0200 Michal Pecio wrote:
With the patch, it all goes away and doesn't show up even after a few minutes. I also tried with two TCP streams to a real machine and only observed a 20KB/s decrease in throughput while the ifconfig allmulti loop is running, probably due to USB bandwidth. So it looks OK.
Excellent, could you send an official Tested-by tag?
But one annoying problem is that those control requests are posted asynchronously and under my test they seem to accumulate faster than they drain. I get brief or not so brief lockups when USB core cleans this up on sudden disconnection. And rtl8150_disconnect() should kill them, but it doesn't. Not sure how this is supposed to work in a well-behaved net driver? Is this callback expected to return without sleeping and have an immediate effect? I can't see this working with USB.
The set_rx_mode callback is annoying because it can't sleep. Leading to no end of issues in the drivers. The best way to deal with this IMHO is to do the confirm from a work item. Don't try to kick off the config asynchronously, instead schedule a work which takes a snapshot of the config, and then synchronously configs the device. The work give us the dirty tracking for free (if a config change is made while work is running it will get re-scheduled). And obviously if there's only one work we can't build up a queue, new request before work had a chance to run will do nothing. We have added a todo to do something along these lines in the core 3+ years ago but nobody had the time to tackle this. The work and taking a snapshot of the rx config are not driver-specific, so it could all be done in the core and then call a (new) driver NDO.