linux-kernel - Re: [PATCH net v2] net: usb: Remove disruptive netif_wake_queue in rtl8150_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250923163727.5e97abdb@kernel.org>
Date: Tue, 23 Sep 2025 16:37:27 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Michal Pecio <michal.pecio@...il.com>
Cc: I Viswanath <viswanathiyyappan@...il.com>, andrew@...n.ch,
 andrew+netdev@...n.ch, davem@...emloft.net, david.hunter.linux@...il.com,
 edumazet@...gle.com, linux-kernel-mentees@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
 netdev@...r.kernel.org, pabeni@...hat.com, petkan@...leusys.com,
 skhan@...uxfoundation.org,
 syzbot+78cae3f37c62ad092caa@...kaller.appspotmail.com
Subject: Re: [PATCH net v2] net: usb: Remove disruptive netif_wake_queue in
 rtl8150_set_multicast

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.