[PATCH] net: usb: cdc_ether: add quirk for AMI BMC stale link events

[PATCH] net: usb: cdc_ether: add quirk for AMI BMC stale link events

[Jinhui Guo]

On AMD Genoa/Turin platforms the BMC-provided USB-Ethernet gadget
(American Megatrends, VID 0x046b PID 0xffb0) intermittently fails to
respond to ARP after AC cold boot.  usbmon captures a stale
NETWORK_CONNECTION(off) immediately followed by
NETWORK_CONNECTION(on) on the interrupt endpoint (~130us apart)
after enumeration.  Because alloc_netdev() leaves
__LINK_STATE_NOCARRIER cleared, netif_carrier_ok() returns true
when the spurious OFF arrives, so usbnet_cdc_status() cannot
recognise it as redundant and schedules EVENT_LINK_CHANGE.
__handle_link_change() then calls unlink_urbs(), killing ~60 rx
URBs whose payload has already been DMA'd into memory — xHCI trace
confirms them completing as -ECONNRESET with non-zero residual
length.  rx_complete() drops these unconditionally.  The following
ON restores the carrier and re-submits URBs, but the ARP reply is
already lost; the interface looks "up but silent" until ifdown/ifup.

Fix this by adding a device-specific quirk with FLAG_LINK_INTR set,
which makes usbnet_probe() call netif_carrier_off() after bind.
With initial carrier == OFF, usbnet_cdc_status() recognises the
spurious OFF as matching the current state and drops it; the
subsequent ON is the first real event and brings the link up
cleanly without ever tearing down the rx queue.  The scheduled
link-change kevent is harmless because EVENT_DEV_OPEN is not yet
set at probe time.

This is applied as a device-specific quirk rather than a change to
the shared cdc_info driver_info because some CDC devices never
send NETWORK_CONNECTION notifications; forcing carrier off for
them would leave the link permanently DOWN.  Restricting the change
to this VID/PID keeps that class of device untouched.

Tested on Genoa and Turin across 100+ AC cold boot cycles; ping
first-packet success rate went from intermittent to 100%.

Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
---
 drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a0a5740590b9..b5e4b195b69a 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -538,6 +538,16 @@ static const struct driver_info	cdc_info = {
 	.manage_power =	usbnet_manage_power,
 };
 
+static const struct driver_info ami_bmc_info = {
+	.description =	"AMI BMC USB Ethernet",
+	.flags =	FLAG_ETHER | FLAG_POINTTOPOINT | FLAG_LINK_INTR,
+	.bind =		usbnet_cdc_bind,
+	.unbind =	usbnet_cdc_unbind,
+	.status =	usbnet_cdc_status,
+	.set_rx_mode =	usbnet_cdc_update_filter,
+	.manage_power =	usbnet_manage_power,
+};
+
 static const struct driver_info	zte_cdc_info = {
 	.description =	"ZTE CDC Ethernet Device",
 	.flags =	FLAG_ETHER | FLAG_POINTTOPOINT,
@@ -946,6 +956,12 @@ static const struct usb_device_id	products[] = {
 				      USB_CDC_SUBCLASS_ETHERNET,
 				      USB_CDC_PROTO_NONE),
 	.driver_info = (unsigned long)&wwan_info,
+}, {
+	/* AMI BMC gadget */
+	USB_DEVICE_AND_INTERFACE_INFO(0x046b, 0xffb0, USB_CLASS_COMM,
+				      USB_CDC_SUBCLASS_ETHERNET,
+				      USB_CDC_PROTO_NONE),
+	.driver_info = (unsigned long)&ami_bmc_info,
 }, {
 	USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
 			USB_CDC_PROTO_NONE),
-- 
2.20.1

Re: [PATCH] net: usb: cdc_ether: add quirk for AMI BMC stale link events

[Jinhui Guo]

Hi,

To help review this patch, here is the runtime evidence captured on
one of the affected Genoa boxes. All timestamps below are from the
same failing boot; I have trimmed unrelated lines and replaced host-
specific addresses with placeholders.

1. usbmon: the two back-to-back NETWORK_CONNECTION notifications

Right after enumeration the BMC gadget delivers two class
notifications on the interrupt endpoint within ~130us of each other.
Both are USB_CDC_NOTIFY_NETWORK_CONNECTION (bNotificationType = 0x00,
bmRequestType = 0xa1); the first has wValue = 0 (link OFF), the
second has wValue = 1 (link ON):

ffff9c48c... Ii:1:007:1 -115:1 8
ffff9c48c... Ii:1:007:1 0:1 8 = a1000000 00000000 0000        <- OFF
ffff9c48c... Ii:1:007:1 -115:1 8
ffff9c48c... Ii:1:007:1 0:1 8 = a1000000 01000000 0000        <- ON

Timing between the two Ii completions is ~130us. There is no
carrier-off event in between and no earlier NETWORK_CONNECTION was
seen, so the OFF is purely a stale event generated by the BMC
firmware on cold boot.

2. Why the OFF is not filtered out

At the moment the OFF arrives, alloc_netdev() has already run but
netif_carrier_off() has not, so __LINK_STATE_NOCARRIER is clear and
netif_carrier_ok() returns 1. usbnet_cdc_status() therefore takes
the wValue-differs branch and calls usbnet_link_change(dev, 0, 0),
which schedules EVENT_LINK_CHANGE. This is confirmed by tracing at
the cdc_ether entry:

[Test] CDC status: set carrier 0, current carrier 1     <- OFF, propagated
[Test] CDC status: set carrier 1, current carrier 0     <- ON, propagated

The ~130us window between the two events is much shorter than the
kevent workqueue latency, so both are queued before
__handle_link_change() runs and are processed in order.

3. xHCI trace: rx URBs killed with data already DMA'd

When __handle_link_change() processes the OFF it takes the
!netif_carrier_ok branch and calls unlink_urbs() on dev->rxq (~60
URBs at the moment of failure). The xhci_ring events show that
several TRBs are already in a completed state when Stop Ring runs;
their Transfer Events carry a non-zero transferred length and are
delivered to the driver as -ECONNRESET:

xhci_handle_command:    Stop Ring, slot 3 ep 1
xhci_handle_transfer:   TRB[...] type 'Transfer' len 1514 status 'Stopped'
xhci_urb_giveback:      ep1in-bulk urb ffff8a... status -108 -> -104 length 1514/1514
xhci_urb_giveback:      ep1in-bulk urb ffff8a... status -104 length 1500/1514
...

(-104 == -ECONNRESET, -108 == -ESHUTDOWN. Both cases hit the same
goto block in rx_complete().)

rx_complete() unconditionally drops these frames:

switch (urb_status) {
case -ECONNRESET:
case -ESHUTDOWN:
        goto block;
}

so the ARP reply that happened to have been DMA'd into one of these
URBs just before Stop Ring is discarded, even though the payload was
already resident in memory. The subsequent ON re-arms the rx queue,
but by then the reply is gone and the interface stays "up but
silent" until the peer times out and re-ARPs, or until the operator
runs ifdown/ifup. This matches the observed intermittent behaviour:
whether the ARP reply falls into the tiny "DMA done, giveback not
yet" window is a race, so the failure only reproduces on some
fraction of AC cold boots.

4. Why DC reboots don't hit this

On DC reboot the BMC USB gadget state is preserved, so the stale
OFF/ON pair is not queued and the sequence above never starts. This
matches every failing boot we captured being an AC cold boot.

5. Why FLAG_LINK_INTR fixes it

With FLAG_LINK_INTR the probe path calls
usbnet_link_change(dev, 0, 0), which sets carrier off before
register_netdev() exposes the netdev. When the stale OFF arrives,
usbnet_cdc_status() sees netif_carrier_ok() == !!event->wValue
(both 0) and returns without calling usbnet_link_change(), so no
EVENT_LINK_CHANGE is queued and unlink_urbs() never runs. The
following ON is the first real state change and brings the link up
cleanly, without ever tearing down the rx queue.

I chose to add this as a device quirk rather than modifying cdc_info
because commit 71cc1fa9f2d7 ("cdc_ether: Set FLAG_LINK_INTR for all
devices with a status endpoint") was reverted in the same series
after it broke CDC devices that never send NETWORK_CONNECTION
notifications; restricting FLAG_LINK_INTR to 046b:ffb0 keeps those
devices untouched.

Verified across 100+ AC cold boot cycles on both Genoa and Turin
reference boards: first-packet ping success went from intermittent
to 100%. DC reboot and ifdown/ifup paths are unchanged.

Happy to provide fuller usbmon / xhci trace captures if that would
help.

Thanks,
Jinhui

Re: [PATCH] net: usb: cdc_ether: add quirk for AMI BMC stale link events

[patchwork-bot+netdevbpf]

Hello:

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

On Thu, 30 Jul 2026 13:13:41 +0800 you wrote:
> On AMD Genoa/Turin platforms the BMC-provided USB-Ethernet gadget
> (American Megatrends, VID 0x046b PID 0xffb0) intermittently fails to
> respond to ARP after AC cold boot.  usbmon captures a stale
> NETWORK_CONNECTION(off) immediately followed by
> NETWORK_CONNECTION(on) on the interrupt endpoint (~130us apart)
> after enumeration.  Because alloc_netdev() leaves
> __LINK_STATE_NOCARRIER cleared, netif_carrier_ok() returns true
> when the spurious OFF arrives, so usbnet_cdc_status() cannot
> recognise it as redundant and schedules EVENT_LINK_CHANGE.
> __handle_link_change() then calls unlink_urbs(), killing ~60 rx
> URBs whose payload has already been DMA'd into memory — xHCI trace
> confirms them completing as -ECONNRESET with non-zero residual
> length.  rx_complete() drops these unconditionally.  The following
> ON restores the carrier and re-submits URBs, but the ARP reply is
> already lost; the interface looks "up but silent" until ifdown/ifup.
> 
> [...]

Here is the summary with links:
  - net: usb: cdc_ether: add quirk for AMI BMC stale link events
    https://git.kernel.org/netdev/net-next/c/089ca284afb0

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