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

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

[Ziyi Guo]

kaweth_set_rx_mode(), the ndo_set_rx_mode callback, calls
netif_stop_queue() and netif_wake_queue(). These are TX queue flow
control functions unrelated to RX multicast configuration.

The premature netif_wake_queue() can re-enable TX while tx_urb is still
in-flight, leading to a double usb_submit_urb() on the same URB:

kaweth_start_xmit() {
    netif_stop_queue();
    usb_submit_urb(kaweth->tx_urb);
}

kaweth_set_rx_mode() {
    netif_stop_queue();
    netif_wake_queue();             // wakes TX queue before URB is done
}

kaweth_start_xmit() {
    netif_stop_queue();
    usb_submit_urb(kaweth->tx_urb); // URB submitted while active
}

This triggers the WARN in usb_submit_urb():

  "URB submitted while active"

This is a similar class of bug fixed in rtl8150 by 
commit 958baf5eaee3 ("net: usb: Remove disruptive netif_wake_queue in rtl8150_set_multicast")

Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
---
 drivers/net/usb/kaweth.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c
index c9efb7df892e..406d1bd070fa 100644
--- a/drivers/net/usb/kaweth.c
+++ b/drivers/net/usb/kaweth.c
@@ -765,8 +765,6 @@ static void kaweth_set_rx_mode(struct net_device *net)
 
 	netdev_dbg(net, "Setting Rx mode to %d\n", packet_filter_bitmap);
 
-	netif_stop_queue(net);
-
 	if (net->flags & IFF_PROMISC) {
 		packet_filter_bitmap |= KAWETH_PACKET_FILTER_PROMISCUOUS;
 	}
@@ -775,7 +773,6 @@ static void kaweth_set_rx_mode(struct net_device *net)
 	}
 
 	kaweth->packet_filter_bitmap = packet_filter_bitmap;
-	netif_wake_queue(net);
 }
 
 /****************************************************************
-- 
2.34.1

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

[Andrew Lunn]

On Mon, Feb 09, 2026 at 05:30:13AM +0000, Ziyi Guo wrote:
> kaweth_set_rx_mode(), the ndo_set_rx_mode callback, calls
> netif_stop_queue() and netif_wake_queue(). These are TX queue flow
> control functions unrelated to RX multicast configuration.

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

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

[Ziyi Guo]

On Mon, Feb 9, 2026 at 7:56 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> 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.

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

[Paolo Abeni]

On 2/9/26 6:48 PM, Ziyi Guo wrote:
> On Mon, Feb 9, 2026 at 7:56 AM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> 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

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

[Paolo Abeni]

On 2/17/26 9:50 AM, Paolo Abeni wrote:
> On 2/9/26 6:48 PM, Ziyi Guo wrote:
>> On Mon, Feb 9, 2026 at 7:56 AM Andrew Lunn <andrew@lunn.ch> wrote:
>>>
>>> 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.

@Ziyi Guo: I'm sorry I'm low on coffee and I did not notice a couple of
relevant things. This needs a fixes tag. Also please specify the target
tree ('net' in this case) on re-submit.

Thanks!

Paolo

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

[Ziyi Guo]

On Tue, Feb 17, 2026 at 3:56 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 2/17/26 9:50 AM, Paolo Abeni wrote:
> > 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.
>
> @Ziyi Guo: I'm sorry I'm low on coffee and I did not notice a couple of
> relevant things. This needs a fixes tag. Also please specify the target
> tree ('net' in this case) on re-submit.
>

Hi Paolo and other folks, thank you so much for your time and review,
and sure, I'll revise and send a v2 version patch, with the fixes tag
and target tree.