On 2018-11-27 12:11, Xin Long wrote:
> sctp_assoc_update_frag_point() should be called whenever asoc->pathmtu
> changes, but we missed one place in sctp_association_init(). It would
> cause frag_point is zero when sending data.
>
> As says in Jakub's reproducer, if sp->pathmtu is set by socketopt, the
> new asoc->pathmtu inherits it in sctp_association_init(). Later when
> transports are added and their pmtu >= asoc->pathmtu, it will never
> call sctp_assoc_update_frag_point() to set frag_point.
>
> This patch is to fix it by updating frag_point after asoc->pathmtu is
> set as sp->pathmtu in sctp_association_init(). Note that it moved them
> after sctp_stream_init(), as stream->si needs to be set first.
>
> Frag_point's calculation is also related with datachunk's type, so it
> needs to update frag_point when stream->si may be changed in
> sctp_process_init().
>
> v1->v2:
> - call sctp_assoc_update_frag_point() separately in sctp_process_init
> and sctp_association_init, per Marcelo's suggestion.
>
> Fixes: 2f5e3c9df693 ("sctp: introduce sctp_assoc_update_frag_point")
> Reported-by: Jakub Audykowicz <jakub.audykowicz@xxxxxxxxx>
> Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx>
> ---
> net/sctp/associola.c | 7 ++++---
> net/sctp/sm_make_chunk.c | 3 +++
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 6a28b96..dd77ec3 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -118,9 +118,6 @@ static struct sctp_association *sctp_association_init(
> asoc->flowlabel = sp->flowlabel;
> asoc->dscp = sp->dscp;
>
> - /* Initialize default path MTU. */
> - asoc->pathmtu = sp->pathmtu;
> -
> /* Set association default SACK delay */
> asoc->sackdelay = msecs_to_jiffies(sp->sackdelay);
> asoc->sackfreq = sp->sackfreq;
> @@ -252,6 +249,10 @@ static struct sctp_association *sctp_association_init(
> 0, gfp))
> goto fail_init;
>
> + /* Initialize default path MTU. */
> + asoc->pathmtu = sp->pathmtu;
> + sctp_assoc_update_frag_point(asoc);
> +
> /* Assume that peer would support both address types unless we are
> * told otherwise.
> */
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 4a4fd19..f4ac6c5 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -2462,6 +2462,9 @@ int sctp_process_init(struct sctp_association *asoc, struct sctp_chunk *chunk,
> asoc->c.sinit_max_instreams, gfp))
> goto clean_up;
>
> + /* Update frag_point when stream_interleave may get changed. */
> + sctp_assoc_update_frag_point(asoc);
> +
> if (!asoc->temp && sctp_assoc_set_id(asoc, gfp))
> goto clean_up;
>
For what it's worth, I can confirm this works as a fix for my issue:)
