Re: [PATCH] net: usbnet: Avoid potential RCU stall on LINK_CHANGE event

Hi Jakub,

On Fri, Jul 18, 2025 at 04:18:25PM -0700, Jakub Kicinski wrote:

On Fri, 18 Jul 2025 09:07:26 +0000 John Ernberg wrote:

quoted

quoted

Thanks for the analysis, I think I may have misread the code.
What I was saying is that we are restoring the carrier while
we are still processing the previous carrier off event in
the workqueue. My thinking was that if we deferred the
netif_carrier_on() to the workqueue this race couldn't happen.

usbnet_bh() already checks netif_carrier_ok() - we're kinda duplicating
the carrier state with this RX_PAUSED workaround.

I don't feel strongly about this, but deferring the carrier_on()
the the workqueue would be a cleaner solution IMO.
  

I've been thinking about this idea, but I'm concerned for the opposite 
direction. I cannot think of a way to fully guarantee that the carrier 
isn't turned on again incorrectly if an off gets queued.

The most I came up with was adding an extra flag bit to set carrier on, 
and then test_and_clear_bit() it in the __handle_link_change() function.
And also clear_bit() in the usbnet_link_change() function if an off 
arrives. I cannot convince myself that there isn't a way for that to go 
sideways. But perhaps that would be robust enough?

I think it should be robust enough.. Unless my grep skills are failing
me - no drivers which call usbnet_link_change() twiddle the link state
directly.

Give it a go, if you think your initial patch is cleaner -- it's fine.

Apologies for the delay, I was stuck in a higher priority issue.

I've tested this approach and it looks promising. Will send this approach
as a v2 later today.

Thank you for the guidance, very much appreciated.

Best regards // John Ernberg