linux-sctp.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Xin Long <lucien.xin@gmail.com>
To: David Laight <David.Laight@aculab.com>
Cc: network dev <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kuba@kernel.org" <kuba@kernel.org>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>
Subject: Re: [PATCHv2 net-next 00/14] sctp: implement RFC8899: Packetization Layer Path MTU Discovery for SCTP transport
Date: Tue, 22 Jun 2021 21:09:52 -0400	[thread overview]
Message-ID: <CADvbK_e7D4s81vS0rq=P4mQe47dshJgQzaWnrUyCi-Cis4xyhQ@mail.gmail.com> (raw)
In-Reply-To: <cfaa01992d064520b3a9138983e8ec41@AcuMS.aculab.com>

On Tue, Jun 22, 2021 at 6:13 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Xin Long
> > Sent: 22 June 2021 19:05
> >
> > Overview(From RFC8899):
> >
> >   In contrast to PMTUD, Packetization Layer Path MTU Discovery
> >   (PLPMTUD) [RFC4821] introduces a method that does not rely upon
> >   reception and validation of PTB messages.  It is therefore more
> >   robust than Classical PMTUD.  This has become the recommended
> >   approach for implementing discovery of the PMTU [BCP145].
> >
> >   It uses a general strategy in which the PL sends probe packets to
> >   search for the largest size of unfragmented datagram that can be sent
> >   over a network path.  Probe packets are sent to explore using a
> >   larger packet size.  If a probe packet is successfully delivered (as
> >   determined by the PL), then the PLPMTU is raised to the size of the
> >   successful probe.  If a black hole is detected (e.g., where packets
> >   of size PLPMTU are consistently not received), the method reduces the
> >   PLPMTU.
>
> This seems to take a long time (probably well over a minute)
> to determine the mtu.
I just noticed this is a misread of RFC8899, and the next probe packet
should be sent immediately once the ACK of the last probe is received,
instead of waiting the timeout, which should be for the missing probe.

I will fix this with:

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index d29b579da904..f3aca1acf93a 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1275,6 +1275,8 @@ enum sctp_disposition
sctp_sf_backbeat_8_3(struct net *net,
                        return SCTP_DISPOSITION_DISCARD;

                sctp_transport_pl_recv(link);
+               sctp_add_cmd_sf(commands, SCTP_CMD_PROBE_TIMER_UPDATE,
+                               SCTP_TRANSPORT(link));
                return SCTP_DISPOSITION_CONSUME;
        }

diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index f27b856ea8ce..88815b98d9d0 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -215,6 +215,11 @@ void sctp_transport_reset_probe_timer(struct
sctp_transport *transport)
 {
        int scale = 1;

+       if (transport->pl.probe_count == 0) {
+               if (!mod_timer(&transport->probe_timer, jiffies +
transport->rto))
+                       sctp_transport_hold(transport);
+               return;
+       }
        if (timer_pending(&transport->probe_timer))
                return;
        if (transport->pl.state == SCTP_PL_COMPLETE &&

Thanks for the comment.
>
> What is used for the actual mtu while this is in progress?
>
> Does packet loss and packet retransmission cause the mtu
> to be reduced as well?
No, the data packet is not a probe in this implementation.

>
> I can imagine that there is an expectation (from the application)
> that the mtu is that of an ethernet link - perhaps less a PPPoE
> header.
> Starting with an mtu of 1200 will break this assumption and may
> have odd side effects.
Starting searching from mtu of 1200, but the real pmtu will only be updated
when the search is done and optimal mtu is found.
So at the beginning, it will still use the dst mtu as before.

> For TCP/UDP the ICMP segmentation required error is immediate
> and gets used for the retransmissions.
> This code seems to be looking at separate timeouts - so a lot of
> packets could get discarded and application timers expire before
> if determines the correct mtu.
This patch will also process ICMP error msg, and gets the 'mtu' size from it
but before using it, it will verify(probe) it first:

see Patch: sctp: do state transition when receiving an icmp TOOBIG packet

>
> Maybe I missed something about this only being done on inactive
> paths?
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

  reply	other threads:[~2021-06-23  1:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22 18:04 Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 01/14] sctp: add pad chunk and its make function and event table Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 02/14] sctp: add probe_interval in sysctl and sock/asoc/transport Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 03/14] sctp: add SCTP_PLPMTUD_PROBE_INTERVAL sockopt for sock/asoc/transport Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 04/14] sctp: add the constants/variables and states and some APIs for transport Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 05/14] sctp: add the probe timer in transport for PLPMTUD Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 06/14] sctp: do the basic send and recv for PLPMTUD probe Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 07/14] sctp: do state transition when PROBE_COUNT == MAX_PROBES on HB send path Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 08/14] sctp: do state transition when a probe succeeds on HB ACK recv path Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 09/14] sctp: do state transition when receiving an icmp TOOBIG packet Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 10/14] sctp: enable PLPMTUD when the transport is ready Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 11/14] sctp: remove the unessessary hold for idev in sctp_v6_err Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 12/14] sctp: extract sctp_v6_err_handle function from sctp_v6_err Xin Long
2021-06-22 18:04 ` [PATCHv2 net-next 13/14] sctp: extract sctp_v4_err_handle function from sctp_v4_err Xin Long
2021-06-22 18:05 ` [PATCHv2 net-next 14/14] sctp: process sctp over udp icmp err on sctp side Xin Long
2021-06-22 18:40 ` [PATCHv2 net-next 00/14] sctp: implement RFC8899: Packetization Layer Path MTU Discovery for SCTP transport patchwork-bot+netdevbpf
2021-06-22 22:13 ` David Laight
2021-06-23  1:09   ` Xin Long [this message]
2021-06-23  3:48     ` Xin Long
2021-06-23  9:50       ` David Laight
2021-06-23 15:59         ` 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='CADvbK_e7D4s81vS0rq=P4mQe47dshJgQzaWnrUyCi-Cis4xyhQ@mail.gmail.com' \
    --to=lucien.xin@gmail.com \
    --cc=David.Laight@aculab.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCHv2 net-next 00/14] sctp: implement RFC8899: Packetization Layer Path MTU Discovery for SCTP transport' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).