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@xxxxxxx> 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