public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-5.10] net: usb: r8152: fix transmit queue timeout
       [not found] <20260214212452.782265-1-sashal@kernel.org>
@ 2026-02-14 21:22 ` Sasha Levin
  2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] net: usb: sr9700: remove code to drive nonexistent multicast filter Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14 21:22 UTC (permalink / raw)
  To: patches, stable
  Cc: Mingj Ye, Hayes Wang, Jakub Kicinski, Sasha Levin, pabeni,
	neil.armstrong, gustavoars, andriy.shevchenko, kees, senozhatsky,
	rawal.abhishek92, phahn-oss, yicong, yelangyan, ebiggers,
	enelsonmoore, linux-usb

From: Mingj Ye <insyelu@gmail.com>

[ Upstream commit 833dcd75d54f0bf5aa0a0781ff57456b421fbb40 ]

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>
Reviewed-by: Hayes Wang <hayeswang@realtek.com>
Link: https://patch.msgid.link/20260120015949.84996-1-insyelu@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of commit: "net: usb: r8152: fix transmit queue timeout"

### 1. COMMIT MESSAGE ANALYSIS

The subject explicitly says **"fix transmit queue timeout"** — this is a
clear bug fix. The commit message explains:
- **Bug**: When the TX queue length reaches a threshold, the netdev
  watchdog immediately detects a TX queue timeout (spurious timeout).
- **Root cause**: The `trans_start` timestamp of the transmit queue is
  not being updated when USB URBs are successfully submitted
  asynchronously.
- **Fix**: Call `netif_trans_update()` after each successful
  `usb_submit_urb()` to keep the watchdog informed that transmission
  activity is ongoing.

The commit is reviewed by the Realtek maintainer (Hayes Wang) and merged
by the network maintainer (Jakub Kicinski). Strong trust indicators.

### 2. CODE CHANGE ANALYSIS

The diff is **+2 lines** — extremely small and surgical:

```c
ret = usb_submit_urb(agg->urb, GFP_ATOMIC);
if (ret < 0)
    usb_autopm_put_interface_async(tp->intf);
+else
+    netif_trans_update(tp->netdev);
```

This adds an `else` branch after the `usb_submit_urb()` call. On
successful URB submission (`ret >= 0`), it calls
`netif_trans_update(tp->netdev)`, which updates the `trans_start`
timestamp of the network device's transmit queue.

**What `netif_trans_update()` does**: It's a standard kernel helper that
sets `txq->trans_start = jiffies`, telling the network watchdog "we are
actively transmitting." Without this update, the watchdog timer thinks
the queue has been idle since the last update and fires a spurious TX
timeout.

### 3. BUG SEVERITY

A **TX queue timeout** in a network driver is a significant user-visible
bug:
- The watchdog fires, potentially calling the driver's `ndo_tx_timeout`
  handler
- This can cause the network interface to reset or go down
- Results in **network connectivity loss** or **severe performance
  degradation**
- The r8152 driver is for **Realtek USB Ethernet adapters**, which are
  extremely common (used in USB-to-Ethernet dongles, docking stations,
  etc.)

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed**: 2 (adding an else branch with one function call)
- **Files touched**: 1 (`drivers/net/usb/r8152.c`)
- **Risk**: Extremely low. `netif_trans_update()` is a well-known,
  lightweight inline function. It simply updates a jiffies timestamp. It
  cannot cause crashes, data corruption, or any side effects.
- **Regression potential**: Near zero. This only affects the TX watchdog
  timer behavior and only updates the timestamp on *successful* URB
  submission.

### 5. USER IMPACT

- **HIGH impact** — r8152 is one of the most widely used USB Ethernet
  drivers. Realtek RTL8152/RTL8153 chips are in millions of USB Ethernet
  adapters.
- Users experience **spurious TX timeouts** that disrupt network
  connectivity.
- Affects any user with a Realtek USB Ethernet adapter under sufficient
  TX load.

### 6. STABLE KERNEL CRITERIA

| Criterion | Met? |
|-----------|------|
| Obviously correct and tested | YES — reviewed by Realtek maintainer,
simple logic |
| Fixes a real bug | YES — spurious TX queue timeouts |
| Important issue | YES — network connectivity loss |
| Small and contained | YES — 2 lines in one file |
| No new features | YES — just a watchdog timestamp update |
| No new APIs | YES |

### 7. DEPENDENCY CHECK

`netif_trans_update()` has been available in the kernel for a very long
time (since 4.7+). The r8152 driver and the `r8152_tx_agg_fill()`
function have been in stable trees for many kernel versions. This patch
applies cleanly with no dependencies on other commits.

### 8. CONCLUSION

This is a textbook stable backport candidate:
- Fixes a real, user-visible bug (spurious TX timeouts causing network
  disruption)
- Affects a widely-used USB Ethernet driver (Realtek r8152)
- Extremely small and surgical (2-line change)
- Zero regression risk (just updates a jiffies timestamp)
- Reviewed by the hardware vendor's maintainer
- No dependencies on other patches

**YES**

 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 2f3baa5f6e9c9..6b107cf5f37bd 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.51.0


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

* [PATCH AUTOSEL 6.19-5.10] net: usb: sr9700: remove code to drive nonexistent multicast filter
       [not found] <20260214212452.782265-1-sashal@kernel.org>
  2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] net: usb: r8152: fix transmit queue timeout Sasha Levin
@ 2026-02-14 21:23 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14 21:23 UTC (permalink / raw)
  To: patches, stable
  Cc: Ethan Nelson-Moore, Simon Horman, Jakub Kicinski, Sasha Levin,
	neil.armstrong, o.rempel, yelangyan, ebiggers, peter, pabeni,
	andrew, linux-usb

From: Ethan Nelson-Moore <enelsonmoore@gmail.com>

[ Upstream commit 9a9424c756feee9ee6e717405a9d6fa7bacdef08 ]

Several registers referenced in this driver's source code do not
actually exist (they are not writable and read as zero in my testing).
They exist in this driver because it originated as a copy of the dm9601
driver. Notably, these include the multicast filter registers - this
causes the driver to not support multicast packets correctly. Remove
the multicast filter code and register definitions. Instead, set the
chip to receive all multicast filter packets when any multicast
addresses are in the list.

Reviewed-by: Simon Horman <horms@kernel.org> (from v1)
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
Link: https://patch.msgid.link/20260203013924.28582-1-enelsonmoore@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Good - `RCR_ALL` is already defined in the header at line 58. The fix
uses an existing flag.

### User Impact

- **Who is affected**: Users with SR9700 USB Ethernet adapters
- **What breaks without the fix**: Multicast networking (mDNS, IPv6
  neighbor discovery, IGMP, etc.) doesn't work when specific multicast
  addresses are subscribed
- **Severity**: Medium-high for affected users - multicast is used
  extensively in modern networking (IPv6 relies on it heavily)

### Stable Criteria Assessment

1. **Obviously correct and tested**: Yes - author tested on real
   hardware, reviewed by Simon Horman
2. **Fixes a real bug**: Yes - multicast doesn't work on SR9700 hardware
3. **Important issue**: Yes - broken multicast means broken IPv6
   neighbor discovery, mDNS, etc.
4. **Small and contained**: Yes - ~20 lines of meaningful change in one
   driver
5. **No new features**: Correct - this removes broken code and enables
   proper multicast reception
6. **Applies cleanly**: The driver has been stable for years, should
   apply cleanly

### Concerns

- The Kconfig change (removing `select CRC32`) could potentially cause
  build issues if something else in the same config depends on CRC32
  being pulled in transitively. However, CRC32 is selected by many other
  drivers, so this is extremely unlikely to matter in practice.
- The removal of the `#include <linux/crc32.h>` is similarly safe.

### Verdict

This is a clear bug fix for a real hardware issue - the sr9700 driver
was attempting to program nonexistent multicast filter registers,
causing multicast to be broken. The fix is small, well-tested on real
hardware, reviewed, and low-risk (it makes the NIC more permissive,
never less). It meets all stable kernel criteria.

**YES**

 drivers/net/usb/Kconfig  |  1 -
 drivers/net/usb/sr9700.c | 25 ++++---------------------
 drivers/net/usb/sr9700.h |  7 +------
 3 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 856e648d804e0..da0f6a138f4fc 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -319,7 +319,6 @@ config USB_NET_DM9601
 config USB_NET_SR9700
 	tristate "CoreChip-sz SR9700 based USB 1.1 10/100 ethernet devices"
 	depends on USB_USBNET
-	select CRC32
 	help
 	  This option adds support for CoreChip-sz SR9700 based USB 1.1
 	  10/100 Ethernet adapters.
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 820c4c5069792..a5d364fbc3639 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -18,7 +18,6 @@
 #include <linux/ethtool.h>
 #include <linux/mii.h>
 #include <linux/usb.h>
-#include <linux/crc32.h>
 #include <linux/usb/usbnet.h>
 
 #include "sr9700.h"
@@ -265,31 +264,15 @@ static const struct ethtool_ops sr9700_ethtool_ops = {
 static void sr9700_set_multicast(struct net_device *netdev)
 {
 	struct usbnet *dev = netdev_priv(netdev);
-	/* We use the 20 byte dev->data for our 8 byte filter buffer
-	 * to avoid allocating memory that is tricky to free later
-	 */
-	u8 *hashes = (u8 *)&dev->data;
 	/* rx_ctl setting : enable, disable_long, disable_crc */
 	u8 rx_ctl = RCR_RXEN | RCR_DIS_CRC | RCR_DIS_LONG;
 
-	memset(hashes, 0x00, SR_MCAST_SIZE);
-	/* broadcast address */
-	hashes[SR_MCAST_SIZE - 1] |= SR_MCAST_ADDR_FLAG;
-	if (netdev->flags & IFF_PROMISC) {
+	if (netdev->flags & IFF_PROMISC)
 		rx_ctl |= RCR_PRMSC;
-	} else if (netdev->flags & IFF_ALLMULTI ||
-		   netdev_mc_count(netdev) > SR_MCAST_MAX) {
-		rx_ctl |= RCR_RUNT;
-	} else if (!netdev_mc_empty(netdev)) {
-		struct netdev_hw_addr *ha;
-
-		netdev_for_each_mc_addr(ha, netdev) {
-			u32 crc = ether_crc(ETH_ALEN, ha->addr) >> 26;
-			hashes[crc >> 3] |= 1 << (crc & 0x7);
-		}
-	}
+	else if (netdev->flags & IFF_ALLMULTI || !netdev_mc_empty(netdev))
+		/* The chip has no multicast filter */
+		rx_ctl |= RCR_ALL;
 
-	sr_write_async(dev, SR_MAR, SR_MCAST_SIZE, hashes);
 	sr_write_reg_async(dev, SR_RCR, rx_ctl);
 }
 
diff --git a/drivers/net/usb/sr9700.h b/drivers/net/usb/sr9700.h
index ea2b4de621c86..c479908f7d823 100644
--- a/drivers/net/usb/sr9700.h
+++ b/drivers/net/usb/sr9700.h
@@ -104,9 +104,7 @@
 #define		WCR_LINKEN		(1 << 5)
 /* Physical Address Reg */
 #define	SR_PAR			0x10	/* 0x10 ~ 0x15 6 bytes for PAR */
-/* Multicast Address Reg */
-#define	SR_MAR			0x16	/* 0x16 ~ 0x1D 8 bytes for MAR */
-/* 0x1e unused */
+/* 0x16 --> 0x1E unused */
 /* Phy Reset Reg */
 #define	SR_PRR			0x1F
 #define		PRR_PHY_RST		(1 << 0)
@@ -161,9 +159,6 @@
 /* parameters */
 #define	SR_SHARE_TIMEOUT	1000
 #define	SR_EEPROM_LEN		256
-#define	SR_MCAST_SIZE		8
-#define	SR_MCAST_ADDR_FLAG	0x80
-#define	SR_MCAST_MAX		64
 #define	SR_TX_OVERHEAD		2	/* 2bytes header */
 #define	SR_RX_OVERHEAD		7	/* 3bytes header + 4crc tail */
 
-- 
2.51.0


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

end of thread, other threads:[~2026-02-14 21:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260214212452.782265-1-sashal@kernel.org>
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] net: usb: r8152: fix transmit queue timeout Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] net: usb: sr9700: remove code to drive nonexistent multicast filter Sasha Levin

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