Re: [PATCH v2 net] sctp: avoid livelock while updating retransmit path
From: netdev-bot+sashiko
Date: Mon Sep 07 2026 - 00:05:13 EST
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0
- [Medium] The permanent part of the commit message (everything above the
`---` line) is not a changelog.
--
sctp: avoid livelock while updating retransmit path
sctp_assoc_update_retran_path() could loop forever when every remaining
transport, including the current retran_path, was in SCTP_UNCONFIRMED
state, because the state check ran before the wraparound test. The patch
moves the candidate selection under a state test so the wraparound
comparison is always reached, and restores the NULL guard before assigning
asoc->peer.retran_path.
Link: https://lore.kernel.org/r/20260827075006.3979566-1-sunyiqixm@xxxxxxxxx/
The code change itself looks fine to me; my only comments are about the
commit message.
> sctp: avoid livelock while updating retransmit path
>
> --jv3lfyjkrnvkybth
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Thu, Aug 27, 2026 at 3:50 AM, Xin Long wrote:
> > After removing the continue, I think you can keep using
> > if (trans == asoc->peer.retran_path) here without 'last' needed.
>
> Yes. The v1 'last' variable was redundant once the SCTP_UNCONFIRMED
> path no longer uses continue. Drop it in this revision and retain the
> original wraparound comparison after the candidate-selection block.
>
> The reproducer is attached.
[Severity: Medium]
Was this text meant to end up above the --- separator? Everything above
--- is what git am records permanently, so the MIME boundary and headers:
--jv3lfyjkrnvkybth
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
get committed verbatim into the changelog.
The reply to the reviewer and the v1/v2 discussion ("The v1 'last'
variable was redundant once the SCTP_UNCONFIRMED path no longer uses
continue. Drop it in this revision") also read as thread material rather
than changelog material, and the same information is already recorded in
the "Changes in v2" block below the --- marker.
Can the statement "The reproducer is attached." be dropped or replaced?
Nothing is attached to the patch, and there is no selftest or
Reported-by/Closes tag pointing at one, so a reader of
git log net/sctp/associola.c has no way to reach the described ASCONF
wildcard DEL-IP scenario.
Would you mind respinning with only the last three paragraphs (the
livelock description, the fix description, and the NULL guard note) plus
the Fixes and Signed-off-by tags above the --- line?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902025206.phbpyxmpf4zrtdpx%40sunyiqi-llm-kernel