| Message ID | 20250312032146.674-1-vulab@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | sctp: handle error of sctp_sf_heartbeat() in sctp_sf_do_asconf() | expand |
> In sctp_sf_do_asconf(), SCTP_DISPOSITION_NOMEM error code returned > from sctp_sf_heartbeat() represent a failure of sent HEARTBEAT. The heartbeat? Would the error predicate “return value != SCTP_DISPOSITION_CONSUME” be safer? https://elixir.bootlin.com/linux/v6.14-rc6/source/include/net/sctp/sm.h#L43 > return value of sctp_sf_heartbeat() needs to be checked and propagates > to caller function. Will imperative wordings be more desirable for such a change description? https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc6#n94 Regards, Markus
On Tue, Mar 11, 2025 at 11:22 PM Wentao Liang <vulab@iscas.ac.cn> wrote: > > In sctp_sf_do_asconf(), SCTP_DISPOSITION_NOMEM error code returned > from sctp_sf_heartbeat() represent a failure of sent HEARTBEAT. The > return value of sctp_sf_heartbeat() needs to be checked and propagates > to caller function. Returning this error to the caller will only result in the packet being discarded, without reverting any changes already made in sctp_sf_do_asconf(). Moreover, this error is not fatal. Instead, it serves as an optimization to confirm the new destination as quickly as possible, as introduced in: commit 6af29ccc223b0feb6fc6112281c3fa3cdb1afddf Author: Michio Honda <micchie@sfc.wide.ad.jp> Date: Thu Jun 16 17:14:34 2011 +0900 sctp: Bundle HEAERTBEAT into ASCONF_ACK Ignoring this error is entirely reasonable, especially considering that running out of memory (nomem) is an unlikely scenario. Thanks. > > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn> > --- > net/sctp/sm_statefuns.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c > index a0524ba8d787..89100546670a 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -3973,8 +3973,10 @@ enum sctp_disposition sctp_sf_do_asconf(struct net *net, > asconf_ack->dest = chunk->source; > sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(asconf_ack)); > if (asoc->new_transport) { > - sctp_sf_heartbeat(ep, asoc, type, asoc->new_transport, commands); > - ((struct sctp_association *)asoc)->new_transport = NULL; > + if (SCTP_DISPOSITION_NOMEM == sctp_sf_heartbeat(ep, asoc, type, asoc->new_transport, commands)) { > + ((struct sctp_association *)asoc)->new_transport = NULL; > + return SCTP_DISPOSITION_NOMEM; > + } > } > > return SCTP_DISPOSITION_CONSUME; > -- > 2.42.0.windows.2 >
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index a0524ba8d787..89100546670a 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -3973,8 +3973,10 @@ enum sctp_disposition sctp_sf_do_asconf(struct net *net, asconf_ack->dest = chunk->source; sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(asconf_ack)); if (asoc->new_transport) { - sctp_sf_heartbeat(ep, asoc, type, asoc->new_transport, commands); - ((struct sctp_association *)asoc)->new_transport = NULL; + if (SCTP_DISPOSITION_NOMEM == sctp_sf_heartbeat(ep, asoc, type, asoc->new_transport, commands)) { + ((struct sctp_association *)asoc)->new_transport = NULL; + return SCTP_DISPOSITION_NOMEM; + } } return SCTP_DISPOSITION_CONSUME;
In sctp_sf_do_asconf(), SCTP_DISPOSITION_NOMEM error code returned from sctp_sf_heartbeat() represent a failure of sent HEARTBEAT. The return value of sctp_sf_heartbeat() needs to be checked and propagates to caller function. Signed-off-by: Wentao Liang <vulab@iscas.ac.cn> --- net/sctp/sm_statefuns.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)