From: Marcelo Ricardo Leitner <marcelo.leitner@gmail•com>
To: Xin Long <lucien.xin@gmail•com>
Cc: network dev <netdev@vger•kernel.org>,
linux-sctp@vger•kernel.org, davem@davemloft•net,
Neil Horman <nhorman@tuxdriver•com>
Subject: Re: [PATCH net] sctp: use the pmtu from the icmp packet to update transport pathmtu
Date: Mon, 15 Oct 2018 19:27:08 -0300 [thread overview]
Message-ID: <20181015222708.GF6761@localhost.localdomain> (raw)
In-Reply-To: <de1ad572beb97767e535b7cf60da7b1de6cbfd4f.1539604709.git.lucien.xin@gmail.com>
On Mon, Oct 15, 2018 at 07:58:29PM +0800, Xin Long wrote:
> Other than asoc pmtu sync from all transports, sctp_assoc_sync_pmtu
> is also processing transport pmtu_pending by icmp packets. But it's
> meaningless to use sctp_dst_mtu(t->dst) as new pmtu for a transport.
>
> The right pmtu value should come from the icmp packet, and it would
> be saved into transport->mtu_info in this patch and used later when
> the pmtu sync happens in sctp_sendmsg_to_asoc or sctp_packet_config.
>
> Besides, without this patch, as pmtu can only be updated correctly
> when receiving a icmp packet and no place is holding sock lock, it
> will take long time if the sock is busy with sending packets.
>
> Note that it doesn't process transport->mtu_info in .release_cb(),
> as there is no enough information for pmtu update, like for which
> asoc or transport. It is not worth traversing all asocs to check
> pmtu_pending. So unlike tcp, sctp does this in tx path, for which
> mtu_info needs to be atomic_t.
>
> Signed-off-by: Xin Long <lucien.xin@gmail•com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail•com>
> ---
> include/net/sctp/structs.h | 2 ++
> net/sctp/associola.c | 3 ++-
> net/sctp/input.c | 1 +
> net/sctp/output.c | 6 ++++++
> 4 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 28a7c8e..a11f937 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -876,6 +876,8 @@ struct sctp_transport {
> unsigned long sackdelay;
> __u32 sackfreq;
>
> + atomic_t mtu_info;
> +
> /* When was the last time that we heard from this transport? We use
> * this to pick new active and retran paths.
> */
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 297d9cf..a827a1f 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1450,7 +1450,8 @@ void sctp_assoc_sync_pmtu(struct sctp_association *asoc)
> /* Get the lowest pmtu of all the transports. */
> list_for_each_entry(t, &asoc->peer.transport_addr_list, transports) {
> if (t->pmtu_pending && t->dst) {
> - sctp_transport_update_pmtu(t, sctp_dst_mtu(t->dst));
> + sctp_transport_update_pmtu(t,
> + atomic_read(&t->mtu_info));
> t->pmtu_pending = 0;
> }
> if (!pmtu || (t->pathmtu < pmtu))
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 9bbc5f9..5c36a99 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -395,6 +395,7 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
> return;
>
> if (sock_owned_by_user(sk)) {
> + atomic_set(&t->mtu_info, pmtu);
> asoc->pmtu_pending = 1;
> t->pmtu_pending = 1;
> return;
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 7f849b0..67939ad 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -120,6 +120,12 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
> sctp_assoc_sync_pmtu(asoc);
> }
>
> + if (asoc->pmtu_pending) {
> + if (asoc->param_flags & SPP_PMTUD_ENABLE)
> + sctp_assoc_sync_pmtu(asoc);
> + asoc->pmtu_pending = 0;
> + }
> +
> /* If there a is a prepend chunk stick it on the list before
> * any other chunks get appended.
> */
> --
> 2.1.0
>
next prev parent reply other threads:[~2018-10-16 6:14 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-15 11:58 [PATCH net] sctp: use the pmtu from the icmp packet to update transport pathmtu Xin Long
2018-10-15 22:27 ` Marcelo Ricardo Leitner [this message]
2018-10-16 5:55 ` David Miller
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=20181015222708.GF6761@localhost.localdomain \
--to=marcelo.leitner@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=linux-sctp@vger$(echo .)kernel.org \
--cc=lucien.xin@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nhorman@tuxdriver$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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