Re: [PATCH net v2 2/2] net: thunderbolt: Mark the connection down when bringing it up fails

On Mon, Aug 10, 2026 at 09:39:15AM +0000, Fan Ye via B4 Relay wrote:

> Every failure path in tbnet_connected_work() undoes its own work and
> returns, but none of them clears login_sent/login_received. The
> connection therefore still looks established, and the next
> tbnet_tear_down() takes its main branch and runs the whole teardown a
> second time over work that was already undone:
> 
>   thunderbolt-net 0-1.0 thunderbolt0: failed to allocate Rx HopID
>   thunderbolt 0000:78:00.0: RX ring 1 already stopped
>   WARNING: CPU: 0 PID: 235 at drivers/thunderbolt/nhi.c:773 tb_ring_stop
>    tbnet_tear_down -> tbnet_stop -> __dev_close_many
>   thunderbolt 0000:78:00.0: TX ring 1 already stopped
>   WARNING: CPU: 0 PID: 235 at drivers/thunderbolt/nhi.c:773 tb_ring_stop
> 
> (line 773 is the dev_WARN in tb_ring_stop() as of v6.17, which is what
> this was captured on; it is line 760 in current mainline)
> 
> It stops rings that were never started, which is what the two warnings
> above are, and on a kernel booted with panic_on_warn those are fatal.
> 
> It also releases net->remote_transmit_path. On the HopID mismatch path
> that one was never successfully allocated by this connection - the
> allocator handed out a different id precisely because the wanted one was
> already taken by somebody else - so this hands back an id the connection
> does not own, and it does so silently. The id is then free to be handed
> out again while its owner is still using it.
> 
> Mark the connection as no longer established on those paths. Only
> login_sent is cleared, which is enough for tbnet_tear_down() to leave the
> already unwound state alone; login_received records that the peer has
> logged in with us and carries the transmit path it gave us, and nothing
> on this side can make the peer send that again.
> 
> Skipping that block skips two things that are not just a repeat of the
> unwind. One is the logout request it would have sent to the peer. The
> other is net->remote_transmit_path = 0 at the end of it; that field is
> only read under the same login_sent && login_received guard and the
> peer's next login request overwrites it, so leaving it stale is
> harmless, but it is a clear that no longer happens. The rest of the
> block is either already undone by the unwind that just ran or was never
> done in the first place - the rings are not started and no buffers are
> allocated when the HopID mismatch is hit, and the paths are not enabled
> on any path that reaches err_stop_rings. The parts outside the block -
> carrier off, queue stopped, login stopped, and the state reset at the
> end - keep running as before.
> 
> Clearing login_sent also changes what the peer's next login request
> does: tbnet_handle_packet() re-queues our login work when it sees
> !login_sent, where before it would only have queued connected_work. That
> is the direction I want - it gives the connection a fresh login instead
> of retrying the bring-up on stale state - but it is a behaviour change
> beyond keeping tbnet_tear_down() out of the way.
> 
> Measured on a link between two ASMedia ASM4242 hosts by cycling the
> interface down and up 200 times from one of them over 80 minutes, and
> counting what the kernel logs on both.
> The mismatch is reached on its own during that, no fault injection, and
> both runs were started from a cold boot with no module reloads in
> between. Only the thunderbolt-net module differs between the two:
> 
>                                         without      with
>     failed to allocate Rx HopID          11 / 12     9 / 13
>     ring already stopped + WARNING       22 / 24     0 / 0
>                                      (host A / host B)
> 
> The race still happens as often as before - it is not what this patch
> addresses - but it no longer leaves a warning splat behind, and no longer
> releases a HopID that belongs to someone else.

Same recommendation, try to squeeze this saga to the point.

-- 
With Best Regards,
Andy Shevchenko