On Fri, 30 Apr 2021, Xin Long wrote:
> The same thing should be done for sctp_sf_do_dupcook_b().
> Meanwhile, SCTP_CMD_UPDATE_ASSOC cmd can be removed.
>
> v1->v2:
> - Fix the return value in sctp_sf_do_assoc_update().
>
> Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
> ---
> include/net/sctp/command.h | 1 -
> net/sctp/sm_sideeffect.c | 26 -------------------------
> net/sctp/sm_statefuns.c | 47 +++++++++++++++++++++++++++++-----------------
> 3 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index e8df72e..5e84888 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -68,7 +68,6 @@ enum sctp_verb {
> SCTP_CMD_ASSOC_FAILED, /* Handle association failure. */
> SCTP_CMD_DISCARD_PACKET, /* Discard the whole packet. */
> SCTP_CMD_GEN_SHUTDOWN, /* Generate a SHUTDOWN chunk. */
> - SCTP_CMD_UPDATE_ASSOC, /* Update association information. */
> SCTP_CMD_PURGE_OUTQUEUE, /* Purge all data waiting to be sent. */
> SCTP_CMD_SETUP_T2, /* Hi-level, setup T2-shutdown parms. */
> SCTP_CMD_RTO_PENDING, /* Set transport's rto_pending. */
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 0948f14..ce15d59 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -826,28 +826,6 @@ static void sctp_cmd_setup_t2(struct sctp_cmd_seq *cmds,
> asoc->timeouts[SCTP_EVENT_TIMEOUT_T2_SHUTDOWN] = t->rto;
> }
>
> -static void sctp_cmd_assoc_update(struct sctp_cmd_seq *cmds,
> - struct sctp_association *asoc,
> - struct sctp_association *new)
> -{
> - struct net *net = asoc->base.net;
> - struct sctp_chunk *abort;
> -
> - if (!sctp_assoc_update(asoc, new))
> - return;
> -
> - abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> - if (abort) {
> - sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0);
> - sctp_add_cmd_sf(cmds, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
> - }
> - sctp_add_cmd_sf(cmds, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNABORTED));
> - sctp_add_cmd_sf(cmds, SCTP_CMD_ASSOC_FAILED,
> - SCTP_PERR(SCTP_ERROR_RSRC_LOW));
> - SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
> - SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
> -}
> -
> /* Helper function to change the state of an association. */
> static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
> struct sctp_association *asoc,
> @@ -1301,10 +1279,6 @@ static int sctp_cmd_interpreter(enum sctp_event_type event_type,
> sctp_endpoint_add_asoc(ep, asoc);
> break;
>
> - case SCTP_CMD_UPDATE_ASSOC:
> - sctp_cmd_assoc_update(commands, asoc, cmd->obj.asoc);
> - break;
> -
> case SCTP_CMD_PURGE_OUTQUEUE:
> sctp_outq_teardown(&asoc->outqueue);
> break;
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index e8ccc4e..a428449 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1773,6 +1773,30 @@ enum sctp_disposition sctp_sf_do_5_2_3_initack(
> return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
> }
>
> +static int sctp_sf_do_assoc_update(struct sctp_association *asoc,
> + struct sctp_association *new,
> + struct sctp_cmd_seq *cmds)
> +{
> + struct net *net = asoc->base.net;
> + struct sctp_chunk *abort;
> +
> + if (!sctp_assoc_update(asoc, new))
> + return 0;
> +
> + abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> + if (abort) {
> + sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0);
> + sctp_add_cmd_sf(cmds, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
> + }
> + sctp_add_cmd_sf(cmds, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNABORTED));
> + sctp_add_cmd_sf(cmds, SCTP_CMD_ASSOC_FAILED,
> + SCTP_PERR(SCTP_ERROR_RSRC_LOW));
> + SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
> + SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
> +
> + return -ENOMEM;
> +}
> +
> /* Unexpected COOKIE-ECHO handler for peer restart (Table 2, action 'A')
> *
> * Section 5.2.4
> @@ -1853,21 +1877,8 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
> sctp_add_cmd_sf(commands, SCTP_CMD_PURGE_ASCONF_QUEUE, SCTP_NULL());
>
> /* Update the content of current association. */
> - if (sctp_assoc_update((struct sctp_association *)asoc, new_asoc)) {
> - struct sctp_chunk *abort;
> -
> - abort = sctp_make_abort(asoc, NULL, sizeof(struct sctp_errhdr));
> - if (abort) {
> - sctp_init_cause(abort, SCTP_ERROR_RSRC_LOW, 0);
> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
> - }
> - sctp_add_cmd_sf(commands, SCTP_CMD_SET_SK_ERR, SCTP_ERROR(ECONNABORTED));
> - sctp_add_cmd_sf(commands, SCTP_CMD_ASSOC_FAILED,
> - SCTP_PERR(SCTP_ERROR_RSRC_LOW));
> - SCTP_INC_STATS(net, SCTP_MIB_ABORTEDS);
> - SCTP_DEC_STATS(net, SCTP_MIB_CURRESTAB);
> + if (sctp_sf_do_assoc_update((struct sctp_association *)asoc, new_asoc, commands))
> goto nomem;
> - }
>
> repl = sctp_make_cookie_ack(asoc, chunk);
> if (!repl)
> @@ -1940,14 +1951,16 @@ static enum sctp_disposition sctp_sf_do_dupcook_b(
> if (!sctp_auth_chunk_verify(net, chunk, new_asoc))
> return SCTP_DISPOSITION_DISCARD;
>
> - /* Update the content of current association. */
> - sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
> sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
> SCTP_STATE(SCTP_STATE_ESTABLISHED));
> SCTP_INC_STATS(net, SCTP_MIB_CURRESTAB);
> sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMERS_START, SCTP_NULL());
>
> - repl = sctp_make_cookie_ack(new_asoc, chunk);
> + /* Update the content of current association. */
> + if (sctp_sf_do_assoc_update((struct sctp_association *)asoc, new_asoc, commands))
> + goto nomem;
Wouldn't it be better to do the update before SCTP_CMD_NEW_STATE?
Or do we have some reason to move to SCTP_STATE_ESTABLISHED even
if the update fails?
> +
> + repl = sctp_make_cookie_ack(asoc, chunk);
> if (!repl)
> goto nomem;
>
> --
> 2.1.0
