Thread (5 messages) flat view 5 messages, 3 authors, 13d ago

Re: [PATCH v2 net] sctp: avoid livelock while updating retransmit path

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-07 04:04:58
Also in: linux-sctp, lkml, stable

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@gmail.com/ (local)

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:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help