public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net: usb: r8152: fix transmit queue timeout
@ 2026-01-14  2:56 insyelu
  2026-01-14  4:38 ` Hayes Wang
  2026-01-16  2:37 ` [PATCH v2] " insyelu
  0 siblings, 2 replies; 19+ messages in thread
From: insyelu @ 2026-01-14  2:56 UTC (permalink / raw)
  To: andrew+netdev, davem, nic_swsd, tiwai
  Cc: hayeswang, linux-usb, netdev, linux-kernel, insyelu

When the TX queue length reaches the threshold, the netdev watchdog
immediately detects a TX queue timeout.

This patch updates the transmit queue's trans_start timestamp upon
completion of each asynchronous USB URB submission on the TX path,
ensuring the network watchdog correctly reflects ongoing transmission
activity.

Signed-off-by: insyelu <insyelu@gmail.com>
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa5192583860..afec602a5fdb 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1954,6 +1954,8 @@ static void write_bulk_callback(struct urb *urb)
 
 	if (!skb_queue_empty(&tp->tx_queue))
 		tasklet_schedule(&tp->tx_tl);
+
+	netif_trans_update(netdev);
 }
 
 static void intr_callback(struct urb *urb)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-14  2:56 [PATCH] net: usb: r8152: fix transmit queue timeout insyelu
@ 2026-01-14  4:38 ` Hayes Wang
  2026-01-15  1:37   ` lu lu
  2026-01-16  2:37 ` [PATCH v2] " insyelu
  1 sibling, 1 reply; 19+ messages in thread
From: Hayes Wang @ 2026-01-14  4:38 UTC (permalink / raw)
  To: insyelu, andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de
  Cc: linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

insyelu <insyelu@gmail.com>
> Sent: Wednesday, January 14, 2026 10:56 AM
[...]
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the transmit queue's trans_start timestamp upon
> completion of each asynchronous USB URB submission on the TX path,
> ensuring the network watchdog correctly reflects ongoing transmission
> activity.
> 
> Signed-off-by: insyelu <insyelu@gmail.com>
> ---
>  drivers/net/usb/r8152.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index fa5192583860..afec602a5fdb 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1954,6 +1954,8 @@ static void write_bulk_callback(struct urb *urb)
> 
>         if (!skb_queue_empty(&tp->tx_queue))
>                 tasklet_schedule(&tp->tx_tl);
> +
> +       netif_trans_update(netdev);
>  }

Based on the definition of netif_trans_update(), I think it would be better to move it into tx_agg_fill().
Such as

--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
        ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
        if (ret < 0)
                usb_autopm_put_interface_async(tp->intf);
+       else
+               netif_trans_update(tp->netdev);

 out_tx_fill:
        return ret;

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-14  4:38 ` Hayes Wang
@ 2026-01-15  1:37   ` lu lu
  2026-01-15 11:42     ` Hayes Wang
  0 siblings, 1 reply; 19+ messages in thread
From: lu lu @ 2026-01-15  1:37 UTC (permalink / raw)
  To: Hayes Wang
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hayes Wang <hayeswang@realtek.com> 于2026年1月14日周三 12:38写道:
>
> insyelu <insyelu@gmail.com>
> > Sent: Wednesday, January 14, 2026 10:56 AM
> [...]
> > When the TX queue length reaches the threshold, the netdev watchdog
> > immediately detects a TX queue timeout.
> >
> > This patch updates the transmit queue's trans_start timestamp upon
> > completion of each asynchronous USB URB submission on the TX path,
> > ensuring the network watchdog correctly reflects ongoing transmission
> > activity.
> >
> > Signed-off-by: insyelu <insyelu@gmail.com>
> > ---
> >  drivers/net/usb/r8152.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> > index fa5192583860..afec602a5fdb 100644
> > --- a/drivers/net/usb/r8152.c
> > +++ b/drivers/net/usb/r8152.c
> > @@ -1954,6 +1954,8 @@ static void write_bulk_callback(struct urb *urb)
> >
> >         if (!skb_queue_empty(&tp->tx_queue))
> >                 tasklet_schedule(&tp->tx_tl);
> > +
> > +       netif_trans_update(netdev);
> >  }
>
> Based on the definition of netif_trans_update(), I think it would be better to move it into tx_agg_fill().
> Such as

HI Hayes Wang,

To reduce the performance impact on the tx_tl tasklet’s transmit path,
netif_trans_update() has been moved from the main transmit path into
write_bulk_callback (the USB transfer completion callback).
The main considerations are as follows:
1. Reduce frequent tasklet overhead
netif_trans_update() is invoked frequently under high-throughput
conditions. Calling it directly in the main transmit path continuously
introduces a small but noticeable CPU overhead, degrading the
scheduling efficiency of the tx_tl tasklet.
2. Move non-critical operations out of the critical path
By deferring netif_trans_update() to the USB callback thread—and
ensuring it executes after tasklet_schedule(&tp->tx_tl)—the timestamp
update is removed from the critical transmit scheduling path, further
reducing the burden on tx_tl.
3. This design follows the approach used in mature USB NIC drivers
such as rtl8150

Although it is semantically more intuitive to call
netif_trans_update() at the point where tx_agg_fill is defined,
delaying the timestamp update is both reasonable and safe from a
performance standpoint—the network watchdog mechanism only requires a
coarse indication that “a transmission occurred recently” and does not
require strict synchronization with the submission time of each
individual packet.
If strict semantic consistency is preferred, I am happy to resubmit a
patch that aligns with the above rationale.

Best Regards,
insyelu
>
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
>         ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
>         if (ret < 0)
>                 usb_autopm_put_interface_async(tp->intf);
> +       else
> +               netif_trans_update(tp->netdev);
>
>  out_tx_fill:
>         return ret;
>
> Best Regards,
> Hayes
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-15  1:37   ` lu lu
@ 2026-01-15 11:42     ` Hayes Wang
  2026-01-16  2:10       ` lu lu
  0 siblings, 1 reply; 19+ messages in thread
From: Hayes Wang @ 2026-01-15 11:42 UTC (permalink / raw)
  To: lu lu
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

lu lu <insyelu@gmail.com>
> Sent: Thursday, January 15, 2026 9:37 AM
[...]
> To reduce the performance impact on the tx_tl tasklet’s transmit path,
> netif_trans_update() has been moved from the main transmit path into
> write_bulk_callback (the USB transfer completion callback).
> The main considerations are as follows:
> 1. Reduce frequent tasklet overhead
> netif_trans_update() is invoked frequently under high-throughput
> conditions. Calling it directly in the main transmit path continuously
> introduces a small but noticeable CPU overhead, degrading the
> scheduling efficiency of the tx_tl tasklet.
> 2. Move non-critical operations out of the critical path
> By deferring netif_trans_update() to the USB callback thread—and
> ensuring it executes after tasklet_schedule(&tp->tx_tl)—the timestamp
> update is removed from the critical transmit scheduling path, further
> reducing the burden on tx_tl.

Excuse me, I do not fully understand the reasoning above.
It seems that this change merely shifts the time (or effort) from tx_tl to the TX completion callback.

While the intention is to make tx_tl run faster, this also delays the completion of the callback,
which in turn may delay both the next callback execution and the next scheduling of tx_tl.

From this perspective, it is unclear what is actually being saved.

Have you observed a measurable difference based on testing?

If you want to reduce the frequency of calling netif_trans_update(),
you could try something like the following. This way,
netif_trans_update() would not be executed on every transmission.

--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2432,9 +2432,12 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)

        netif_tx_lock(tp->netdev);

-       if (netif_queue_stopped(tp->netdev) &&
-           skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
+       if (netif_queue_stopped(tp->netdev)) {
+           if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
                netif_wake_queue(tp->netdev);
+           else
+               netif_trans_update(tp->netdev);
+       }

        netif_tx_unlock(tp->netdev);

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-15 11:42     ` Hayes Wang
@ 2026-01-16  2:10       ` lu lu
  2026-01-16  3:11         ` Hayes Wang
  0 siblings, 1 reply; 19+ messages in thread
From: lu lu @ 2026-01-16  2:10 UTC (permalink / raw)
  To: Hayes Wang
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hayes Wang <hayeswang@realtek.com> 于2026年1月15日周四 19:42写道:
>
> lu lu <insyelu@gmail.com>
> > Sent: Thursday, January 15, 2026 9:37 AM
> [...]
> > To reduce the performance impact on the tx_tl tasklet’s transmit path,
> > netif_trans_update() has been moved from the main transmit path into
> > write_bulk_callback (the USB transfer completion callback).
> > The main considerations are as follows:
> > 1. Reduce frequent tasklet overhead
> > netif_trans_update() is invoked frequently under high-throughput
> > conditions. Calling it directly in the main transmit path continuously
> > introduces a small but noticeable CPU overhead, degrading the
> > scheduling efficiency of the tx_tl tasklet.
> > 2. Move non-critical operations out of the critical path
> > By deferring netif_trans_update() to the USB callback thread—and
> > ensuring it executes after tasklet_schedule(&tp->tx_tl)—the timestamp
> > update is removed from the critical transmit scheduling path, further
> > reducing the burden on tx_tl.
>
> Excuse me, I do not fully understand the reasoning above.
> It seems that this change merely shifts the time (or effort) from tx_tl to the TX completion callback.
>
> While the intention is to make tx_tl run faster, this also delays the completion of the callback,
> which in turn may delay both the next callback execution and the next scheduling of tx_tl.
>
> From this perspective, it is unclear what is actually being saved.
>
> Have you observed a measurable difference based on testing?

HW: RK3399-based development board (dual-core Cortex-A72 + quad-core Cortex-A53)
OS: LEDE (OpenWrt)
NIC: 2.5G USB Ethernet adapter connected via USB 3.0

Due to platform performance limitations, it cannot reach 2.5G. When
processing in the write_bulk_callback and during USB submission,
iperf3 stress testing shows a difference of approximately 8Mbit/s.
This may be related to the platform's big.LITTLE core configuration.


>
> If you want to reduce the frequency of calling netif_trans_update(),
> you could try something like the following. This way,
> netif_trans_update() would not be executed on every transmission.
>
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2432,9 +2432,12 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
>
>         netif_tx_lock(tp->netdev);
>
> -       if (netif_queue_stopped(tp->netdev) &&
> -           skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> +       if (netif_queue_stopped(tp->netdev)) {
> +           if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
>                 netif_wake_queue(tp->netdev);
> +           else
> +               netif_trans_update(tp->netdev);
> +       }
The queue was stopped because it exceeded the threshold. Attempting to
refresh the time at this point is clearly too late.

Thank you for your suggestion. I will submit v2 (immediately updating
the transfer start time after successful USB submission).

Best Regards,
insyelu

>
>         netif_tx_unlock(tp->netdev);
>
> Best Regards,
> Hayes
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-14  2:56 [PATCH] net: usb: r8152: fix transmit queue timeout insyelu
  2026-01-14  4:38 ` Hayes Wang
@ 2026-01-16  2:37 ` insyelu
  2026-01-16 17:32   ` Andrew Lunn
  1 sibling, 1 reply; 19+ messages in thread
From: insyelu @ 2026-01-16  2:37 UTC (permalink / raw)
  To: andrew+netdev, davem, nic_swsd, tiwai
  Cc: hayeswang, linux-usb, netdev, linux-kernel, insyelu

When the TX queue length reaches the threshold, the netdev watchdog
immediately detects a TX queue timeout.

This patch updates the trans_start timestamp of the transmit queue
on every asynchronous USB URB submission along the transmit path,
ensuring that the network watchdog accurately reflects ongoing
transmission activity.

Signed-off-by: insyelu <insyelu@gmail.com>
---
v2: Update the transmit timestamp when submitting the USB URB.
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa5192583860..880b59ed5422 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
 	if (ret < 0)
 		usb_autopm_put_interface_async(tp->intf);
+	else
+		netif_trans_update(tp->netdev);
 
 out_tx_fill:
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-16  2:10       ` lu lu
@ 2026-01-16  3:11         ` Hayes Wang
  2026-01-16  7:30           ` lu lu
  0 siblings, 1 reply; 19+ messages in thread
From: Hayes Wang @ 2026-01-16  3:11 UTC (permalink / raw)
  To: lu lu
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

lu lu <insyelu@gmail.com>
> Sent: Friday, January 16, 2026 10:11 AM
[...]
> >         netif_tx_lock(tp->netdev);
> >
> > -       if (netif_queue_stopped(tp->netdev) &&
> > -           skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> > +       if (netif_queue_stopped(tp->netdev)) {
> > +           if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> >                 netif_wake_queue(tp->netdev);
> > +           else
> > +               netif_trans_update(tp->netdev);
> > +       }
> The queue was stopped because it exceeded the threshold. Attempting to
> refresh the time at this point is clearly too late.

Why would this be considered too late?
Based on RTL8152_TX_TIMEOUT, there are about 5 seconds to
wake the queue or update the timestamp before a TX timeout occurs.
I believe 5 seconds should be sufficient.

If there is no TX submission for 5 seconds after the driver stops the queue,
then something is already wrong.

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-16  3:11         ` Hayes Wang
@ 2026-01-16  7:30           ` lu lu
  2026-01-19  2:51             ` Hayes Wang
  0 siblings, 1 reply; 19+ messages in thread
From: lu lu @ 2026-01-16  7:30 UTC (permalink / raw)
  To: Hayes Wang
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hayes Wang <hayeswang@realtek.com> 于2026年1月16日周五 11:11写道:
>
> lu lu <insyelu@gmail.com>
> > Sent: Friday, January 16, 2026 10:11 AM
> [...]
> > >         netif_tx_lock(tp->netdev);
> > >
> > > -       if (netif_queue_stopped(tp->netdev) &&
> > > -           skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> > > +       if (netif_queue_stopped(tp->netdev)) {
> > > +           if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> > >                 netif_wake_queue(tp->netdev);
> > > +           else
> > > +               netif_trans_update(tp->netdev);
> > > +       }
> > The queue was stopped because it exceeded the threshold. Attempting to
> > refresh the time at this point is clearly too late.
>
> Why would this be considered too late?

if (netif_queue_stopped(tp->netdev)) {
    if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
        netif_wake_queue(tp->netdev);
    else
        netif_trans_update(tp->netdev);
}
The first time xmit stops the transmit queue, the queue is not full,
and it is successfully woken up afterward — OK.
The second time xmit stops the transmit queue, the network watchdog
times out immediately because the transmit timestamp was not refreshed
when the queue was last resumed — FAIL.
This scenario is logically possible.

There is no clear evidence that netif_trans_update imposes a
significant CPU load.
Please help me review:
https://patchwork.kernel.org/project/linux-usb/patch/20260116023725.8095-1-insyelu@gmail.com

> Based on RTL8152_TX_TIMEOUT, there are about 5 seconds to
> wake the queue or update the timestamp before a TX timeout occurs.
> I believe 5 seconds should be sufficient.

>
> If there is no TX submission for 5 seconds after the driver stops the queue,
> then something is already wrong.
>
> Best Regards,
> Hayes
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-16  2:37 ` [PATCH v2] " insyelu
@ 2026-01-16 17:32   ` Andrew Lunn
  0 siblings, 0 replies; 19+ messages in thread
From: Andrew Lunn @ 2026-01-16 17:32 UTC (permalink / raw)
  To: insyelu
  Cc: andrew+netdev, davem, nic_swsd, tiwai, hayeswang, linux-usb,
	netdev, linux-kernel

On Fri, Jan 16, 2026 at 10:37:25AM +0800, insyelu wrote:
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the trans_start timestamp of the transmit queue
> on every asynchronous USB URB submission along the transmit path,
> ensuring that the network watchdog accurately reflects ongoing
> transmission activity.
> 
> Signed-off-by: insyelu <insyelu@gmail.com>
> ---
> v2: Update the transmit timestamp when submitting the USB URB.

Always create a new thread for a new version of a patch. The CI/CD
system just thinks this is a discussion comment, so has ignored it.

    Andrew

---
pw-bot: cr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] net: usb: r8152: fix transmit queue timeout
@ 2026-01-19  2:28 insyelu
  2026-01-19  6:40 ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: insyelu @ 2026-01-19  2:28 UTC (permalink / raw)
  To: andrew+netdev, davem, nic_swsd, tiwai
  Cc: hayeswang, linux-usb, netdev, linux-kernel, insyelu

When the TX queue length reaches the threshold, the netdev watchdog
immediately detects a TX queue timeout.

This patch updates the trans_start timestamp of the transmit queue
on every asynchronous USB URB submission along the transmit path,
ensuring that the network watchdog accurately reflects ongoing
transmission activity.

Signed-off-by: insyelu <insyelu@gmail.com>
---
v2: Update the transmit timestamp when submitting the USB URB.
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa5192583860..880b59ed5422 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
 	if (ret < 0)
 		usb_autopm_put_interface_async(tp->intf);
+	else
+		netif_trans_update(tp->netdev);
 
 out_tx_fill:
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-16  7:30           ` lu lu
@ 2026-01-19  2:51             ` Hayes Wang
  2026-01-19  6:58               ` lu lu
  0 siblings, 1 reply; 19+ messages in thread
From: Hayes Wang @ 2026-01-19  2:51 UTC (permalink / raw)
  To: lu lu
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Original Message-----
> From: lu lu <insyelu@gmail.com>
[...]
> if (netif_queue_stopped(tp->netdev)) {
>     if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
>         netif_wake_queue(tp->netdev);
>     else
>         netif_trans_update(tp->netdev);
> }
> The first time xmit stops the transmit queue, the queue is not full,
> and it is successfully woken up afterward — OK.
> The second time xmit stops the transmit queue, the network watchdog
> times out immediately because the transmit timestamp was not refreshed
> when the queue was last resumed — FAIL.
> This scenario is logically possible.

This situation should not happen, because trans_start is also updated when the driver stops the TX queue.

https://elixir.bootlin.com/linux/v6.18.6/source/include/linux/netdevice.h#L3629

A TX timeout occurs only if the TX queue has been stopped for longer than RTL8152_TX_TIMEOUT.
It should not occur immediately when the driver stops the TX queue.

Therefore, what needs to be done is to update the timestamp when the TX queue is stopped.
Updating trans_start while the TX queue is not stopped is useless.

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-19  2:28 insyelu
@ 2026-01-19  6:40 ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2026-01-19  6:40 UTC (permalink / raw)
  To: insyelu
  Cc: andrew+netdev, davem, nic_swsd, tiwai, hayeswang, linux-usb,
	netdev, linux-kernel

On Mon, Jan 19, 2026 at 10:28:02AM +0800, insyelu wrote:
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the trans_start timestamp of the transmit queue
> on every asynchronous USB URB submission along the transmit path,
> ensuring that the network watchdog accurately reflects ongoing
> transmission activity.
> 
> Signed-off-by: insyelu <insyelu@gmail.com>
> ---
> v2: Update the transmit timestamp when submitting the USB URB.
> ---
>  drivers/net/usb/r8152.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index fa5192583860..880b59ed5422 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
>  	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
>  	if (ret < 0)
>  		usb_autopm_put_interface_async(tp->intf);
> +	else
> +		netif_trans_update(tp->netdev);
>  
>  out_tx_fill:
>  	return ret;
> -- 
> 2.34.1
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file,
  Documentation/process/submitting-patches.rst for how to do this
  correctly.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-19  2:51             ` Hayes Wang
@ 2026-01-19  6:58               ` lu lu
  2026-01-19 12:34                 ` Hayes Wang
  0 siblings, 1 reply; 19+ messages in thread
From: lu lu @ 2026-01-19  6:58 UTC (permalink / raw)
  To: Hayes Wang
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

Hayes Wang <hayeswang@realtek.com> 于2026年1月19日周一 10:51写道:
>
> Original Message-----
> > From: lu lu <insyelu@gmail.com>
> [...]
> > if (netif_queue_stopped(tp->netdev)) {
> >     if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
> >         netif_wake_queue(tp->netdev);
> >     else
> >         netif_trans_update(tp->netdev);
> > }
> > The first time xmit stops the transmit queue, the queue is not full,
> > and it is successfully woken up afterward — OK.
> > The second time xmit stops the transmit queue, the network watchdog
> > times out immediately because the transmit timestamp was not refreshed
> > when the queue was last resumed — FAIL.
> > This scenario is logically possible.
>
> This situation should not happen, because trans_start is also updated when the driver stops the TX queue.
>
> https://elixir.bootlin.com/linux/v6.18.6/source/include/linux/netdevice.h#L3629
>
> A TX timeout occurs only if the TX queue has been stopped for longer than RTL8152_TX_TIMEOUT.
> It should not occur immediately when the driver stops the TX queue.
Thank you for the correction! Upon review, I confirmed that
netif_tx_stop_queue() already updates trans_start,
so the timeout scenario I originally envisioned does not actually occur.

>
> Therefore, what needs to be done is to update the timestamp when the TX queue is stopped.
> Updating trans_start while the TX queue is not stopped is useless.
if (netif_queue_stopped(tp->netdev)) {
    if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
        netif_wake_queue(tp->netdev);
    else
        netif_trans_update(tp->netdev);
}
This change continuously updates the trans_start value, even when the
TX queue has been stopped and its length exceeds the threshold.
This may prevent the watchdog timer from ever timing out, thereby
masking potential transmission stall issues.

The timestamp should be updated only upon successful URB submission to
accurately reflect that the transport layer is still operational.

Best regards,
insyelu

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] net: usb: r8152: fix transmit queue timeout
@ 2026-01-19 10:56 insyelu
  2026-01-19 11:07 ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: insyelu @ 2026-01-19 10:56 UTC (permalink / raw)
  To: andrew+netdev, davem, nic_swsd, tiwai
  Cc: hayeswang, linux-usb, netdev, linux-kernel, insyelu

When the TX queue length reaches the threshold, the netdev watchdog
immediately detects a TX queue timeout.

This patch updates the trans_start timestamp of the transmit queue
on every asynchronous USB URB submission along the transmit path,
ensuring that the network watchdog accurately reflects ongoing
transmission activity.

Signed-off-by: insyelu <insyelu@gmail.com>
---
v2: Update the transmit timestamp when submitting the USB URB.
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa5192583860..880b59ed5422 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
 	if (ret < 0)
 		usb_autopm_put_interface_async(tp->intf);
+	else
+		netif_trans_update(tp->netdev);
 
 out_tx_fill:
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-19 10:56 insyelu
@ 2026-01-19 11:07 ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2026-01-19 11:07 UTC (permalink / raw)
  To: insyelu
  Cc: andrew+netdev, davem, nic_swsd, tiwai, hayeswang, linux-usb,
	netdev, linux-kernel

On Mon, Jan 19, 2026 at 06:56:47PM +0800, insyelu wrote:
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the trans_start timestamp of the transmit queue
> on every asynchronous USB URB submission along the transmit path,
> ensuring that the network watchdog accurately reflects ongoing
> transmission activity.
> 
> Signed-off-by: insyelu <insyelu@gmail.com>

We need a real name please.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] net: usb: r8152: fix transmit queue timeout
  2026-01-19  6:58               ` lu lu
@ 2026-01-19 12:34                 ` Hayes Wang
  0 siblings, 0 replies; 19+ messages in thread
From: Hayes Wang @ 2026-01-19 12:34 UTC (permalink / raw)
  To: lu lu
  Cc: andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

lu lu <insyelu@gmail.com>
> Sent: Monday, January 19, 2026 2:58 PM
[...]
> > Therefore, what needs to be done is to update the timestamp when the TX queue is stopped.
> > Updating trans_start while the TX queue is not stopped is useless.
> if (netif_queue_stopped(tp->netdev)) {
>     if (skb_queue_len(&tp->tx_queue) < tp->tx_qlen)
>         netif_wake_queue(tp->netdev);
>     else
>         netif_trans_update(tp->netdev);
> }
> This change continuously updates the trans_start value, even when the
> TX queue has been stopped and its length exceeds the threshold.
> This may prevent the watchdog timer from ever timing out, thereby
> masking potential transmission stall issues.
> 
> The timestamp should be updated only upon successful URB submission to
> accurately reflect that the transport layer is still operational.

Although I think a URB error and a transmission stall are different,
I am fine with the simpler approach in v2.

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2] net: usb: r8152: fix transmit queue timeout
@ 2026-01-20  1:59 Mingj Ye
  2026-01-20  2:49 ` Hayes Wang
  2026-01-21  2:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 19+ messages in thread
From: Mingj Ye @ 2026-01-20  1:59 UTC (permalink / raw)
  To: andrew+netdev, davem, nic_swsd, tiwai
  Cc: hayeswang, linux-usb, netdev, linux-kernel, Mingj Ye

When the TX queue length reaches the threshold, the netdev watchdog
immediately detects a TX queue timeout.

This patch updates the trans_start timestamp of the transmit queue
on every asynchronous USB URB submission along the transmit path,
ensuring that the network watchdog accurately reflects ongoing
transmission activity.

Signed-off-by: Mingj Ye <insyelu@gmail.com>
---
v2: Update the transmit timestamp when submitting the USB URB.
---
 drivers/net/usb/r8152.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index fa5192583860..880b59ed5422 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2449,6 +2449,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
 	if (ret < 0)
 		usb_autopm_put_interface_async(tp->intf);
+	else
+		netif_trans_update(tp->netdev);
 
 out_tx_fill:
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-20  1:59 Mingj Ye
@ 2026-01-20  2:49 ` Hayes Wang
  2026-01-21  2:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 19+ messages in thread
From: Hayes Wang @ 2026-01-20  2:49 UTC (permalink / raw)
  To: Mingj Ye, andrew+netdev@lunn.ch, davem@davemloft.net, nic_swsd,
	tiwai@suse.de
  Cc: linux-usb@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org

> From: Mingj Ye <insyelu@gmail.com>
> Sent: Tuesday, January 20, 2026 10:00 AM
[...]
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the trans_start timestamp of the transmit queue
> on every asynchronous USB URB submission along the transmit path,
> ensuring that the network watchdog accurately reflects ongoing
> transmission activity.
> 
> Signed-off-by: Mingj Ye <insyelu@gmail.com>
> ---
> v2: Update the transmit timestamp when submitting the USB URB.

Reviewed-by: Hayes Wang <hayeswang@realtek.com>

Best Regards,
Hayes


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2] net: usb: r8152: fix transmit queue timeout
  2026-01-20  1:59 Mingj Ye
  2026-01-20  2:49 ` Hayes Wang
@ 2026-01-21  2:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 19+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-01-21  2:40 UTC (permalink / raw)
  To: Mingj Ye
  Cc: andrew+netdev, davem, nic_swsd, tiwai, hayeswang, linux-usb,
	netdev, linux-kernel

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 20 Jan 2026 09:59:49 +0800 you wrote:
> When the TX queue length reaches the threshold, the netdev watchdog
> immediately detects a TX queue timeout.
> 
> This patch updates the trans_start timestamp of the transmit queue
> on every asynchronous USB URB submission along the transmit path,
> ensuring that the network watchdog accurately reflects ongoing
> transmission activity.
> 
> [...]

Here is the summary with links:
  - [v2] net: usb: r8152: fix transmit queue timeout
    https://git.kernel.org/netdev/net-next/c/833dcd75d54f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-01-21  2:40 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14  2:56 [PATCH] net: usb: r8152: fix transmit queue timeout insyelu
2026-01-14  4:38 ` Hayes Wang
2026-01-15  1:37   ` lu lu
2026-01-15 11:42     ` Hayes Wang
2026-01-16  2:10       ` lu lu
2026-01-16  3:11         ` Hayes Wang
2026-01-16  7:30           ` lu lu
2026-01-19  2:51             ` Hayes Wang
2026-01-19  6:58               ` lu lu
2026-01-19 12:34                 ` Hayes Wang
2026-01-16  2:37 ` [PATCH v2] " insyelu
2026-01-16 17:32   ` Andrew Lunn
  -- strict thread matches above, loose matches on Subject: below --
2026-01-19  2:28 insyelu
2026-01-19  6:40 ` Greg KH
2026-01-19 10:56 insyelu
2026-01-19 11:07 ` Greg KH
2026-01-20  1:59 Mingj Ye
2026-01-20  2:49 ` Hayes Wang
2026-01-21  2:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox