[PATCH net 0/2] sctp: improve the pmtu probe in Search Complete state

linux-sctp.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH net 0/2] sctp: improve the pmtu probe in Search Complete state
@ 2021-07-19 16:53 Xin Long
  2021-07-19 16:53 ` [PATCH net 1/2] sctp: improve the code for pmtu probe send and recv update Xin Long
  2021-07-19 16:53 ` [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state Xin Long
  0 siblings, 2 replies; 5+ messages in thread
From: Xin Long @ 2021-07-19 16:53 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, timo.voelker

Timo recently suggested to use the loss of (data) packets as
indication to send pmtu probe for Search Complete state, which
should also be implied by RFC8899. This patchset is to change
the current one that is doing probe with current pmtu all the
time.

Xin Long (2):
  sctp: improve the code for pmtu probe send and recv update
  sctp: send pmtu probe only if packet loss in Search Complete state

 include/net/sctp/structs.h |  5 +++--
 net/sctp/sm_statefuns.c    | 15 ++++++-------
 net/sctp/transport.c       | 45 +++++++++++++++++++++++---------------
 3 files changed, 37 insertions(+), 28 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net 1/2] sctp: improve the code for pmtu probe send and recv update
  2021-07-19 16:53 [PATCH net 0/2] sctp: improve the pmtu probe in Search Complete state Xin Long
@ 2021-07-19 16:53 ` Xin Long
  2021-07-19 16:53 ` [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state Xin Long
  1 sibling, 0 replies; 5+ messages in thread
From: Xin Long @ 2021-07-19 16:53 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, timo.voelker

This patch does 3 things:

  - make sctp_transport_pl_send() and sctp_transport_pl_recv()
    return bool type to decide if more probe is needed to send.

  - pr_debug() only when probe is really needed to send.

  - count pl.raise_count in sctp_transport_pl_send() instead of
    sctp_transport_pl_recv(), and it's only incremented for the
    1st probe for the same size.

These are preparations for the next patch to make probes happen
only when there's packet loss in Search Complete state.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h |  4 ++--
 net/sctp/sm_statefuns.c    | 15 +++++++-------
 net/sctp/transport.c       | 41 +++++++++++++++++++++-----------------
 3 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 32fc4a309df5..f3d414ed208e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1024,8 +1024,8 @@ bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu);
 void sctp_transport_immediate_rtx(struct sctp_transport *);
 void sctp_transport_dst_release(struct sctp_transport *t);
 void sctp_transport_dst_confirm(struct sctp_transport *t);
-void sctp_transport_pl_send(struct sctp_transport *t);
-void sctp_transport_pl_recv(struct sctp_transport *t);
+bool sctp_transport_pl_send(struct sctp_transport *t);
+bool sctp_transport_pl_recv(struct sctp_transport *t);
 
 
 /* This is the structure we use to queue packets as they come into
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 09a8f23ec709..32df65f68c12 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1109,12 +1109,12 @@ enum sctp_disposition sctp_sf_send_probe(struct net *net,
 	if (!sctp_transport_pl_enabled(transport))
 		return SCTP_DISPOSITION_CONSUME;
 
-	sctp_transport_pl_send(transport);
-
-	reply = sctp_make_heartbeat(asoc, transport, transport->pl.probe_size);
-	if (!reply)
-		return SCTP_DISPOSITION_NOMEM;
-	sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
+	if (sctp_transport_pl_send(transport)) {
+		reply = sctp_make_heartbeat(asoc, transport, transport->pl.probe_size);
+		if (!reply)
+			return SCTP_DISPOSITION_NOMEM;
+		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(reply));
+	}
 	sctp_add_cmd_sf(commands, SCTP_CMD_PROBE_TIMER_UPDATE,
 			SCTP_TRANSPORT(transport));
 
@@ -1274,8 +1274,7 @@ enum sctp_disposition sctp_sf_backbeat_8_3(struct net *net,
 		    !sctp_transport_pl_enabled(link))
 			return SCTP_DISPOSITION_DISCARD;
 
-		sctp_transport_pl_recv(link);
-		if (link->pl.state == SCTP_PL_COMPLETE)
+		if (sctp_transport_pl_recv(link))
 			return SCTP_DISPOSITION_CONSUME;
 
 		return sctp_sf_send_probe(net, ep, asoc, type, link, commands);
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 397a6244dd97..23e7bd3e3bd4 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -258,16 +258,12 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 	sctp_transport_pl_update(transport);
 }
 
-void sctp_transport_pl_send(struct sctp_transport *t)
+bool sctp_transport_pl_send(struct sctp_transport *t)
 {
-	pr_debug("%s: PLPMTUD: transport: %p, state: %d, pmtu: %d, size: %d, high: %d\n",
-		 __func__, t, t->pl.state, t->pl.pmtu, t->pl.probe_size, t->pl.probe_high);
-
-	if (t->pl.probe_count < SCTP_MAX_PROBES) {
-		t->pl.probe_count++;
-		return;
-	}
+	if (t->pl.probe_count < SCTP_MAX_PROBES)
+		goto out;
 
+	t->pl.probe_count = 0;
 	if (t->pl.state == SCTP_PL_BASE) {
 		if (t->pl.probe_size == SCTP_BASE_PLPMTU) { /* BASE_PLPMTU Confirmation Failed */
 			t->pl.state = SCTP_PL_ERROR; /* Base -> Error */
@@ -299,10 +295,20 @@ void sctp_transport_pl_send(struct sctp_transport *t)
 			sctp_assoc_sync_pmtu(t->asoc);
 		}
 	}
-	t->pl.probe_count = 1;
+
+out:
+	if (t->pl.state == SCTP_PL_COMPLETE && t->pl.raise_count < 30 &&
+	    !t->pl.probe_count)
+		t->pl.raise_count++;
+
+	pr_debug("%s: PLPMTUD: transport: %p, state: %d, pmtu: %d, size: %d, high: %d\n",
+		 __func__, t, t->pl.state, t->pl.pmtu, t->pl.probe_size, t->pl.probe_high);
+
+	t->pl.probe_count++;
+	return true;
 }
 
-void sctp_transport_pl_recv(struct sctp_transport *t)
+bool sctp_transport_pl_recv(struct sctp_transport *t)
 {
 	pr_debug("%s: PLPMTUD: transport: %p, state: %d, pmtu: %d, size: %d, high: %d\n",
 		 __func__, t, t->pl.state, t->pl.pmtu, t->pl.probe_size, t->pl.probe_high);
@@ -323,7 +329,7 @@ void sctp_transport_pl_recv(struct sctp_transport *t)
 		if (!t->pl.probe_high) {
 			t->pl.probe_size = min(t->pl.probe_size + SCTP_PL_BIG_STEP,
 					       SCTP_MAX_PLPMTU);
-			return;
+			return false;
 		}
 		t->pl.probe_size += SCTP_PL_MIN_STEP;
 		if (t->pl.probe_size >= t->pl.probe_high) {
@@ -335,14 +341,13 @@ void sctp_transport_pl_recv(struct sctp_transport *t)
 			t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t);
 			sctp_assoc_sync_pmtu(t->asoc);
 		}
-	} else if (t->pl.state == SCTP_PL_COMPLETE) {
-		t->pl.raise_count++;
-		if (t->pl.raise_count == 30) {
-			/* 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;
-		}
+	} else if (t->pl.state == SCTP_PL_COMPLETE && t->pl.raise_count == 30) {
+		/* 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;
 	}
+
+	return t->pl.state == SCTP_PL_COMPLETE;
 }
 
 static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu)
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state
  2021-07-19 16:53 [PATCH net 0/2] sctp: improve the pmtu probe in Search Complete state Xin Long
  2021-07-19 16:53 ` [PATCH net 1/2] sctp: improve the code for pmtu probe send and recv update Xin Long
@ 2021-07-19 16:53 ` Xin Long
  2021-07-20 10:50   ` Jakub Kicinski
  1 sibling, 1 reply; 5+ messages in thread
From: Xin Long @ 2021-07-19 16:53 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, timo.voelker

This patch is to introduce last_rtx_chunks into sctp_transport to detect
if there's any packet retransmission/loss happened by checking against
asoc's rtx_data_chunks in sctp_transport_pl_send().

If there is, namely, transport->last_rtx_chunks != asoc->rtx_data_chunks,
the pmtu probe will be sent out. Otherwise, increment the pl.raise_count
and return when it's in Search Complete state.

With this patch, if in Search Complete state, which is a long period, it
doesn't need to keep probing the current pmtu unless there's data packet
loss. This will save quite some traffic.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/sctp/structs.h | 1 +
 net/sctp/transport.c       | 6 +++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index f3d414ed208e..651bba654d77 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -984,6 +984,7 @@ struct sctp_transport {
 	} cacc;
 
 	struct {
+		__u32 last_rtx_chunks;
 		__u16 pmtu;
 		__u16 probe_size;
 		__u16 probe_high;
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 23e7bd3e3bd4..a3d3ca6dd63d 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -263,6 +263,7 @@ bool sctp_transport_pl_send(struct sctp_transport *t)
 	if (t->pl.probe_count < SCTP_MAX_PROBES)
 		goto out;
 
+	t->pl.last_rtx_chunks = t->asoc->rtx_data_chunks;
 	t->pl.probe_count = 0;
 	if (t->pl.state == SCTP_PL_BASE) {
 		if (t->pl.probe_size == SCTP_BASE_PLPMTU) { /* BASE_PLPMTU Confirmation Failed */
@@ -298,8 +299,10 @@ bool sctp_transport_pl_send(struct sctp_transport *t)
 
 out:
 	if (t->pl.state == SCTP_PL_COMPLETE && t->pl.raise_count < 30 &&
-	    !t->pl.probe_count)
+	    !t->pl.probe_count && t->pl.last_rtx_chunks == t->asoc->rtx_data_chunks) {
 		t->pl.raise_count++;
+		return false;
+	}
 
 	pr_debug("%s: PLPMTUD: transport: %p, state: %d, pmtu: %d, size: %d, high: %d\n",
 		 __func__, t, t->pl.state, t->pl.pmtu, t->pl.probe_size, t->pl.probe_high);
@@ -313,6 +316,7 @@ bool sctp_transport_pl_recv(struct sctp_transport *t)
 	pr_debug("%s: PLPMTUD: transport: %p, state: %d, pmtu: %d, size: %d, high: %d\n",
 		 __func__, t, t->pl.state, t->pl.pmtu, t->pl.probe_size, t->pl.probe_high);
 
+	t->pl.last_rtx_chunks = t->asoc->rtx_data_chunks;
 	t->pl.pmtu = t->pl.probe_size;
 	t->pl.probe_count = 0;
 	if (t->pl.state == SCTP_PL_BASE) {
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state
  2021-07-19 16:53 ` [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state Xin Long
@ 2021-07-20 10:50   ` Jakub Kicinski
  2021-07-20 14:16     ` Xin Long
  0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2021-07-20 10:50 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, davem, linux-sctp, Marcelo Ricardo Leitner, timo.voelker

On Mon, 19 Jul 2021 12:53:23 -0400, Xin Long wrote:
> This patch is to introduce last_rtx_chunks into sctp_transport to detect
> if there's any packet retransmission/loss happened by checking against
> asoc's rtx_data_chunks in sctp_transport_pl_send().
> 
> If there is, namely, transport->last_rtx_chunks != asoc->rtx_data_chunks,
> the pmtu probe will be sent out. Otherwise, increment the pl.raise_count
> and return when it's in Search Complete state.
> 
> With this patch, if in Search Complete state, which is a long period, it
> doesn't need to keep probing the current pmtu unless there's data packet
> loss. This will save quite some traffic.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Can we get a Fixes tag, please?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state
  2021-07-20 10:50   ` Jakub Kicinski
@ 2021-07-20 14:16     ` Xin Long
  0 siblings, 0 replies; 5+ messages in thread
From: Xin Long @ 2021-07-20 14:16 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: network dev, davem, linux-sctp @ vger . kernel . org,
	Marcelo Ricardo Leitner, Timo Völker

On Tue, Jul 20, 2021 at 6:50 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 19 Jul 2021 12:53:23 -0400, Xin Long wrote:
> > This patch is to introduce last_rtx_chunks into sctp_transport to detect
> > if there's any packet retransmission/loss happened by checking against
> > asoc's rtx_data_chunks in sctp_transport_pl_send().
> >
> > If there is, namely, transport->last_rtx_chunks != asoc->rtx_data_chunks,
> > the pmtu probe will be sent out. Otherwise, increment the pl.raise_count
> > and return when it's in Search Complete state.
> >
> > With this patch, if in Search Complete state, which is a long period, it
> > doesn't need to keep probing the current pmtu unless there's data packet
> > loss. This will save quite some traffic.
> >
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
>
> Can we get a Fixes tag, please?
Fixes: 0dac127c0557 ("sctp: do black hole detection in search complete state")

Should I repost?

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-07-20 14:26 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-19 16:53 [PATCH net 0/2] sctp: improve the pmtu probe in Search Complete state Xin Long
2021-07-19 16:53 ` [PATCH net 1/2] sctp: improve the code for pmtu probe send and recv update Xin Long
2021-07-19 16:53 ` [PATCH net 2/2] sctp: send pmtu probe only if packet loss in Search Complete state Xin Long
2021-07-20 10:50   ` Jakub Kicinski
2021-07-20 14:16     ` Xin Long

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).