mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: netdev-bot+sashiko@kernel.org
To: sunyiqixm@gmail.com
Cc: lucien.xin@gmail.com, davem@davemloft.net, edumazet@google.com,
	horms@kernel.org, kuba@kernel.org, linux-kernel@vger.kernel.org,
	linux-sctp@vger.kernel.org, marcelo.leitner@gmail.com,
	netdev@vger.kernel.org, pabeni@redhat.com,
	stable@vger.kernel.org
Subject: Re: [PATCH v2 net] sctp: avoid livelock while updating retransmit path
Date: Mon, 07 Sep 2026 04:04:57 +0000	[thread overview]
Message-ID: <178875389773.219967.10749525397271653040@kernel.org> (raw)
In-Reply-To: <20260902025206.phbpyxmpf4zrtdpx@sunyiqi-llm-kernel>

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/

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

  reply	other threads:[~2026-09-07  4:04 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-27  7:50 [PATCH " Yiqi Sun
2026-08-27 17:18 ` Xin Long
2026-09-02  2:52   ` [PATCH v2 " Yiqi Sun
2026-09-07  4:04     ` netdev-bot+sashiko [this message]
2026-09-11  0:25     ` Xin Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178875389773.219967.10749525397271653040@kernel.org \
    --to=netdev-bot+sashiko@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=sunyiqixm@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®