NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/59943: usbnet(9) keeps failed packet in queue to retry indefinitely



>Number:         59943
>Category:       kern
>Synopsis:       usbnet(9) keeps failed packet in queue to retry indefinitely
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jan 26 03:00:00 +0000 2026
>Originator:     Taylor R Campbell
>Release:        current, 11, 10, 9, ...
>Organization:
ugroundhogday(4)
>Environment:
>Description:

	In usbnet_start_locked, whose job is to kick off USB transfers
	when there are packets queued for tx on the interface, if
	either:

	(a) the driver rejects the packet, or
	(b) the usbnet chain has no transfer allocated, or
	(c) usbd_transfer fails,

	the packet is left in the interface's tx queue and we break out
	of the loop.

523	while (cd->uncd_tx_cnt < un->un_tx_list_cnt) {
524		IFQ_POLL(&ifp->if_snd, m);
...
533		length = uno_tx_prepare(un, m, c);
534		if (length == 0) {
535			DPRINTF("uno_tx_prepare gave zero length", 0, 0, 0, 0);
536			if_statinc(ifp, if_oerrors);
537			break;
538		}
...
540		if (__predict_false(c->unc_xfer == NULL)) {
541			DPRINTF("unc_xfer is NULL", 0, 0, 0, 0);
542			if_statinc(ifp, if_oerrors);
543			break;
544		}
...
550		usbd_status err = usbd_transfer(c->unc_xfer);
551		if (err != USBD_IN_PROGRESS) {
552			DPRINTF("usbd_transfer on %#jx for %ju bytes: %jd",
553			    (uintptr_t)c->unc_buf, length, err, 0);
554			if_statinc(ifp, if_oerrors);
555			break;
556		}

	https://nxr.netbsd.org/xref/src/sys/dev/usb/usbnet.c?r=1.121#523

	These all strike me as wrong or bonkers.

	(a) If the driver rejects the packet, which in all the drivers
	    I reviewed happens only when the packet does not physically
	    fit in the buffer, surely it makes no sense to retry the
	    packet again because it will almost certainly fail again in
	    the same way.

	(b) The usbnet chain _cannot_ fail to have a transfer allocated
	    because usbnet_start_locked is gated on the success of
	    usbnet_init_tx_rx, which fails if it can't allocate a
	    transfer for every usbnet chain.

	(c) If the hardware fails to actually transfer the packet,
	    surely we should drop it rather than keep retrying, no?
	    That's what we do on usbnet_txeof if the transfer has
	    failed, for example.  The only case in which we should
	    retry, I think, is temporary memory shortage.

	I have seen some USB ethernet interfaces get stuck unable to
	transmit packets until I bring them down and back up again.
	Haven't investigated in a while but this might explain some of
	that.

>How-To-Repeat:

	cod inspection

>Fix:

	Patch in progress!



Home | Main Index | Thread Index | Old Index