mbox series

[net-next,0/2] sctp: make the PLPMTUD probe more effective and efficient

Message ID cover.1624549642.git.lucien.xin@gmail.com
Headers show
Series sctp: make the PLPMTUD probe more effective and efficient | expand

Message

Xin Long June 24, 2021, 3:48 p.m. UTC 
As David Laight noticed, it currently takes quite some time to find
the optimal pmtu in the Search state, and also lacks the black hole
detection in the Search Complete state. This patchset is to address
them to mke the PLPMTUD probe more effective and efficient.

Xin Long (2):
  sctp: do black hole detection in search complete state
  sctp: send the next probe immediately once the last one is acked

 Documentation/networking/ip-sysctl.rst | 12 ++++++++----
 include/net/sctp/structs.h             |  3 ++-
 net/sctp/sm_statefuns.c                |  5 ++++-
 net/sctp/transport.c                   | 11 ++++-------
 4 files changed, 18 insertions(+), 13 deletions(-)

Comments

Marcelo Ricardo Leitner June 25, 2021, 1:11 a.m. UTC | #1 
On Thu, Jun 24, 2021 at 11:48:08AM -0400, Xin Long wrote:
> @@ -333,13 +328,15 @@ void sctp_transport_pl_recv(struct sctp_transport *t)

>  		t->pl.probe_size += SCTP_PL_MIN_STEP;

>  		if (t->pl.probe_size >= t->pl.probe_high) {

>  			t->pl.probe_high = 0;

> +			t->pl.raise_count = 0;

>  			t->pl.state = SCTP_PL_COMPLETE; /* Search -> Search Complete */

>  

>  			t->pl.probe_size = t->pl.pmtu;

>  			t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t);

>  			sctp_assoc_sync_pmtu(t->asoc);

>  		}

> -	} else if (t->pl.state == SCTP_PL_COMPLETE) {

> +	} else if (t->pl.state == SCTP_PL_COMPLETE && ++t->pl.raise_count == 30) {


Please either break the condition into 2 lines or even in 2 if()s. The
++ operator here can easily go unnoticed otherwise.

> +		/* Raise probe_size again after 30 * interval in Search Complete */

>  		t->pl.state = SCTP_PL_SEARCH; /* Search Complete -> Search */

>  		t->pl.probe_size += SCTP_PL_MIN_STEP;

>  	}

> -- 

> 2.27.0

>
Xin Long June 25, 2021, 4:23 p.m. UTC | #2 
On Thu, Jun 24, 2021 at 9:11 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>

> On Thu, Jun 24, 2021 at 11:48:08AM -0400, Xin Long wrote:

> > @@ -333,13 +328,15 @@ void sctp_transport_pl_recv(struct sctp_transport *t)

> >               t->pl.probe_size += SCTP_PL_MIN_STEP;

> >               if (t->pl.probe_size >= t->pl.probe_high) {

> >                       t->pl.probe_high = 0;

> > +                     t->pl.raise_count = 0;

> >                       t->pl.state = SCTP_PL_COMPLETE; /* Search -> Search Complete */

> >

> >                       t->pl.probe_size = t->pl.pmtu;

> >                       t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t);

> >                       sctp_assoc_sync_pmtu(t->asoc);

> >               }

> > -     } else if (t->pl.state == SCTP_PL_COMPLETE) {

> > +     } else if (t->pl.state == SCTP_PL_COMPLETE && ++t->pl.raise_count == 30) {

>

> Please either break the condition into 2 lines or even in 2 if()s. The

> ++ operator here can easily go unnoticed otherwise.

will change it to:
        } else if (t->pl.state == SCTP_PL_COMPLETE) {
                t->pl.raise_count++;
                if (t->pl.raise_count == 30) {

Thanks.
>

> > +             /* Raise probe_size again after 30 * interval in Search Complete */

> >               t->pl.state = SCTP_PL_SEARCH; /* Search Complete -> Search */

> >               t->pl.probe_size += SCTP_PL_MIN_STEP;

> >       }

> > --

> > 2.27.0

> >