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 [off-list ref]
    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.

-- 
Ammar Faizi