public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Michal Pecio <michal.pecio@gmail.com>
Cc: I Viswanath <viswanathiyyappan@gmail.com>,
	andrew@lunn.ch, andrew+netdev@lunn.ch, davem@davemloft.net,
	david.hunter.linux@gmail.com, edumazet@google.com,
	linux-kernel-mentees@lists.linux.dev,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	netdev@vger.kernel.org, pabeni@redhat.com, petkan@nucleusys.com,
	skhan@linuxfoundation.org,
	syzbot+78cae3f37c62ad092caa@syzkaller.appspotmail.com
Subject: Re: [PATCH net v2] net: usb: Remove disruptive netif_wake_queue in rtl8150_set_multicast
Date: Tue, 23 Sep 2025 16:37:27 -0700	[thread overview]
Message-ID: <20250923163727.5e97abdb@kernel.org> (raw)
In-Reply-To: <20250924012039.66a2411c.michal.pecio@gmail.com>

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.

  reply	other threads:[~2025-09-23 23:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-20  4:50 [PATCH] net: usb: Remove disruptive netif_wake_queue in rtl8150_set_multicast I Viswanath
2025-09-20 15:30 ` Andrew Lunn
2025-09-20 16:52   ` viswanath
2025-09-20 17:28     ` Andrew Lunn
2025-09-20 18:18       ` [PATCH net v2] " I Viswanath
2025-09-23  1:07         ` Jakub Kicinski
2025-09-23  7:47           ` Michal Pecio
2025-09-23 14:28             ` Jakub Kicinski
2025-09-23 23:20               ` Michal Pecio
2025-09-23 23:37                 ` Jakub Kicinski [this message]
2025-09-25  5:59                   ` Deepak Sharma
2025-09-24  7:47         ` Michal Pecio
2025-09-24  8:02           ` viswanath
2025-09-24  9:36             ` Michal Pecio
2025-09-24 10:25               ` viswanath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250923163727.5e97abdb@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=david.hunter.linux@gmail.com \
    --cc=edumazet@google.com \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=michal.pecio@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=petkan@nucleusys.com \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+78cae3f37c62ad092caa@syzkaller.appspotmail.com \
    --cc=viswanathiyyappan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox