On Wed, Aug 06, 2025 at 01:40:37AM +0300, Linus Torvalds wrote:
In particular, the whole *rest* of the code in that
if (!netif_carrier_ok(dev->net)) {no longer makes sense after we've turned the link on with that
if (test_and_clear_bit(EVENT_LINK_CARRIER_ON, &dev->flags)) netif_carrier_on(dev->net);sequence.
Put another way - once we've turned the carrier on, now that whole
/* kill URBs for reading packets to save bus bandwidth */ unlink_urbs(dev, &dev->rxq); /* * tx_timeout will unlink URBs for sending packets and * tx queue is stopped by netcore after link becomes off */thing makes no sense.
After taking a look further, I agree with you. I git-blamed the unlink_urbs()'s line and it's indeed expected to be called after link becomes off. So yes, it makes no sense to call that when we're turning the link on.
commit 4b49f58fff00e6e9b24eaa31d4c6324393d76b0a Author: Ming Lei ming.lei@canonical.com Date: Thu Apr 11 04:40:40 2013 +0000
usbnet: handle link change
The link change is detected via the interrupt pipe, and bulk pipes are responsible for transfering packets, so it is reasonable to stop bulk transfer after link is reported as off.
Even though my patch works on my machine. Something may go wrong.
So my gut feel is that the
if (test_and_clear_bit(EVENT_LINK_CARRIER_ON, &dev->flags)) netif_carrier_on(dev->net);should actually be done outside that if-statement entirely, because it literally ends up changing the thing that if-statement is testing.
Apart from moving it outside that if-statement, unlink_urbs() call should probably also be guarded as we agreed it makes no sense to call it when we're turning the link on.