Thread (6 messages) 6 messages, 3 authors, 2026-02-17

Re: [PATCH] net: usb: kaweth: remove TX queue manipulation in kaweth_set_rx_mode

From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-02-17 08:50:40
Also in: linux-usb, lkml

On 2/9/26 6:48 PM, Ziyi Guo wrote:
On Mon, Feb 9, 2026 at 7:56 AM Andrew Lunn [off-list ref] wrote:
quoted
I think there needs to be a better explanation why it is safe to
remove these stop and wake queue operations. r8152 does the same. Is
it also broken? Rather than removing these, should we actually be
waiting for the completion of the urb?

        Andrew
Hi Andrew,

Thank you for your time and email, here is my understanding (correct
me if I'm wrong).

I think ndo_set_rx_mode runs in atomic context with a spinlock held,
if we wait for the completion, this would sleep under a spinlock.

And it seems the architecture of r8152 is different from kaweth and
rtl8150. In r8152, the ndo_set_rx_mode callback (rtl8152_set_rx_mode)
won't directly call netif_stop_queue/netif_wake_queue. It only sets a
flag and schedules delayed work.
The function that does call netif_stop_queue/netif_wake_queue is
_rtl8152_set_rx_mode(), which runs from the work handler
rtl_work_func_t() under tp->control mutex, not from the ndo callback
under netif_addr_lock_bh.
Also, r8152's start_xmit never submits URBs directly. It only queues
SKBs to a software queue. Actual URB submission happens in a separate
tasklet using a pool of 4 independent TX aggregation buffers, each
with its own URB.

So even if netif_wake_queue() triggers start_xmit, it just adds an SKB
to the software queue, no URB is touched, and no double-submission can
occur.

In kaweth and rtl8150, start_xmit calls usb_submit_urb() directly on a
single shared tx_urb, so the netif_wake_queue() from the ndo callback
causes the stack to call start_xmit while that URB is still in-flight.
It looks like kaweth_set_rx_mode() is already functionally broken: the
real set_rx_mode action is performed by kaweth_async_set_rx_mode, which
in turn is not a no-op only at ndo_open() time.

So this looks safe to me.

/P
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help