1. Patchew
  2. MPTCP
  3. View series

[PATCH mptcp-next v3] mptcp: add a new sysctl for make after break timeout

Paolo Abeni posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/fe8c2c5502b7a924bdcba6621486e1e68861f9fd.1694426964.git.pabeni@redhat.com
Maintainers: Jonathan Corbet <corbet@lwn.net>, Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <martineau@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Documentation/networking/mptcp-sysctl.rst | 11 +++++++++++
net/mptcp/ctrl.c                          | 16 ++++++++++++++++
net/mptcp/protocol.c                      |  6 +++---
net/mptcp/protocol.h                      |  1 +
4 files changed, 31 insertions(+), 3 deletions(-)
[PATCH mptcp-next v3] mptcp: add a new sysctl for make after break timeout
Posted by Paolo Abeni 1 year, 1 month ago 
The MPTCP protocol allows sockets with no alive subflows to stay
in ESTABLISHED status for and user-defined timeout, to allow for
later subflows creation.

Currently such timeout is constant - TCP_TIMEWAIT_LEN. Let the
user-space configure them via a newly added sysctl, to better cope
with busy servers and simplify (make them faster) the relevant
pktdrill tests.

Note that the new know does not apply to orphaned MPTCP socket
waiting for the data_fin handshake completion: they always wait
TCP_TIMEWAIT_LEN.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
 - fix doc intentation (Matttbe)
 - use mptcp_close_timeout() in __mptcp_close_ssk, too (Mat)

v1 -> v2:
 - add the doc for the new knob (Matttbe)
 - fix a couple of bad/strange indentation (Matttbe)
---
 Documentation/networking/mptcp-sysctl.rst | 11 +++++++++++
 net/mptcp/ctrl.c                          | 16 ++++++++++++++++
 net/mptcp/protocol.c                      |  6 +++---
 net/mptcp/protocol.h                      |  1 +
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 15f1919d640c..69975ce25a02 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -25,6 +25,17 @@ add_addr_timeout - INTEGER (seconds)
 
 	Default: 120
 
+close_timeout - INTEGER (seconds)
+	Set the make-after-break timeout: in absence of any close or
+	shutdown syscall, MPTCP sockets will maintain the status
+	unchanged for such time, after the last subflow removal, before
+	moving to TCP_CLOSE.
+
+	The default value matches TCP_TIMEWAIT_LEN. This is a per-namespace
+	sysctl.
+
+	Default: 60
+
 checksum_enabled - BOOLEAN
 	Control whether DSS checksum can be enabled.
 
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index e72b518c5d02..13fe0748dde8 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -27,6 +27,7 @@ struct mptcp_pernet {
 #endif
 
 	unsigned int add_addr_timeout;
+	unsigned int close_timeout;
 	unsigned int stale_loss_cnt;
 	u8 mptcp_enabled;
 	u8 checksum_enabled;
@@ -65,6 +66,13 @@ unsigned int mptcp_stale_loss_cnt(const struct net *net)
 	return mptcp_get_pernet(net)->stale_loss_cnt;
 }
 
+unsigned int mptcp_close_timeout(const struct sock *sk)
+{
+	if (sock_flag(sk, SOCK_DEAD))
+		return TCP_TIMEWAIT_LEN;
+	return mptcp_get_pernet(sock_net(sk))->close_timeout;
+}
+
 int mptcp_get_pm_type(const struct net *net)
 {
 	return mptcp_get_pernet(net)->pm_type;
@@ -79,6 +87,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 {
 	pernet->mptcp_enabled = 1;
 	pernet->add_addr_timeout = TCP_RTO_MAX;
+	pernet->close_timeout = TCP_TIMEWAIT_LEN;
 	pernet->checksum_enabled = 0;
 	pernet->allow_join_initial_addr_port = 1;
 	pernet->stale_loss_cnt = 4;
@@ -141,6 +150,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.mode = 0644,
 		.proc_handler = proc_dostring,
 	},
+	{
+		.procname = "close_timeout",
+		.maxlen = sizeof(unsigned int),
+		.mode = 0644,
+		.proc_handler = proc_dointvec_jiffies,
+	},
 	{}
 };
 
@@ -163,6 +178,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	table[4].data = &pernet->stale_loss_cnt;
 	table[5].data = &pernet->pm_type;
 	table[6].data = &pernet->scheduler;
+	table[7].data = &pernet->close_timeout;
 
 	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
 				     ARRAY_SIZE(mptcp_sysctl_table));
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3facdc006adc..1a0b463f8c97 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2377,8 +2377,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	if (msk->in_accept_queue && msk->first == ssk &&
 	    (sock_flag(sk, SOCK_DEAD) || sock_flag(ssk, SOCK_DEAD))) {
 		/* ensure later check in mptcp_worker() will dispose the msk */
-		mptcp_set_close_tout(sk, tcp_jiffies32 - (TCP_TIMEWAIT_LEN + 1));
 		sock_set_flag(sk, SOCK_DEAD);
+		mptcp_set_close_tout(sk, tcp_jiffies32 - (mptcp_close_timeout(sk) + 1));
 		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
 		mptcp_subflow_drop_ctx(ssk);
 		goto out_release;
@@ -2506,7 +2506,7 @@ static bool mptcp_close_tout_expired(const struct sock *sk)
 		return false;
 
 	return time_after32(tcp_jiffies32,
-		  inet_csk(sk)->icsk_mtup.probe_timestamp + TCP_TIMEWAIT_LEN);
+		  inet_csk(sk)->icsk_mtup.probe_timestamp + mptcp_close_timeout(sk));
 }
 
 static void mptcp_check_fastclose(struct mptcp_sock *msk)
@@ -2649,7 +2649,7 @@ void mptcp_reset_tout_timer(struct mptcp_sock *msk, unsigned long fail_tout)
 		return;
 
 	close_timeout = inet_csk(sk)->icsk_mtup.probe_timestamp - tcp_jiffies32 + jiffies +
-			TCP_TIMEWAIT_LEN;
+			mptcp_close_timeout(sk);
 
 	/* the close timeout takes precedence on the fail one, and here at least one of
 	 * them is active
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index aebc0cc24dad..0147419065dc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -624,6 +624,7 @@ unsigned int mptcp_get_add_addr_timeout(const struct net *net);
 int mptcp_is_checksum_enabled(const struct net *net);
 int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
+unsigned int mptcp_close_timeout(const struct sock *sk);
 int mptcp_get_pm_type(const struct net *net);
 const char *mptcp_get_scheduler(const struct net *net);
 void mptcp_subflow_fully_established(struct mptcp_subflow_context *subflow,
-- 
2.41.0
Re: [PATCH mptcp-next v3] mptcp: add a new sysctl for make after break timeout
Posted by Matthieu Baerts 1 year, 1 month ago 
Hi Paolo, Mat,

On 11/09/2023 12:34, Paolo Abeni wrote:
> The MPTCP protocol allows sockets with no alive subflows to stay
> in ESTABLISHED status for and user-defined timeout, to allow for
> later subflows creation.
> 
> Currently such timeout is constant - TCP_TIMEWAIT_LEN. Let the
> user-space configure them via a newly added sysctl, to better cope
> with busy servers and simplify (make them faster) the relevant
> pktdrill tests.
> 
> Note that the new know does not apply to orphaned MPTCP socket
> waiting for the data_fin handshake completion: they always wait
> TCP_TIMEWAIT_LEN.

Thank you for your patch and reviews!

Now in our tree (feat. for net-next) with Mat's RvB tag:

New patches for t/upstream:
- e9a01d8bc89c: mptcp: add a new sysctl for make after break timeout
- Results: dbcf6d0f46fa..243780a48a72 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230912T170023

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-next v3] mptcp: add a new sysctl for make after break timeout
Posted by Mat Martineau 1 year, 1 month ago 
On Mon, 11 Sep 2023, Paolo Abeni wrote:

> The MPTCP protocol allows sockets with no alive subflows to stay
> in ESTABLISHED status for and user-defined timeout, to allow for
> later subflows creation.
>
> Currently such timeout is constant - TCP_TIMEWAIT_LEN. Let the
> user-space configure them via a newly added sysctl, to better cope
> with busy servers and simplify (make them faster) the relevant
> pktdrill tests.
>
> Note that the new know does not apply to orphaned MPTCP socket
> waiting for the data_fin handshake completion: they always wait
> TCP_TIMEWAIT_LEN.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
> - fix doc intentation (Matttbe)
> - use mptcp_close_timeout() in __mptcp_close_ssk, too (Mat)

Hi Paolo -

Thanks, v3 LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>


>
> v1 -> v2:
> - add the doc for the new knob (Matttbe)
> - fix a couple of bad/strange indentation (Matttbe)
> ---
> Documentation/networking/mptcp-sysctl.rst | 11 +++++++++++
> net/mptcp/ctrl.c                          | 16 ++++++++++++++++
> net/mptcp/protocol.c                      |  6 +++---
> net/mptcp/protocol.h                      |  1 +
> 4 files changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
> index 15f1919d640c..69975ce25a02 100644
> --- a/Documentation/networking/mptcp-sysctl.rst
> +++ b/Documentation/networking/mptcp-sysctl.rst
> @@ -25,6 +25,17 @@ add_addr_timeout - INTEGER (seconds)
>
> 	Default: 120
>
> +close_timeout - INTEGER (seconds)
> +	Set the make-after-break timeout: in absence of any close or
> +	shutdown syscall, MPTCP sockets will maintain the status
> +	unchanged for such time, after the last subflow removal, before
> +	moving to TCP_CLOSE.
> +
> +	The default value matches TCP_TIMEWAIT_LEN. This is a per-namespace
> +	sysctl.
> +
> +	Default: 60
> +
> checksum_enabled - BOOLEAN
> 	Control whether DSS checksum can be enabled.
>
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index e72b518c5d02..13fe0748dde8 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -27,6 +27,7 @@ struct mptcp_pernet {
> #endif
>
> 	unsigned int add_addr_timeout;
> +	unsigned int close_timeout;
> 	unsigned int stale_loss_cnt;
> 	u8 mptcp_enabled;
> 	u8 checksum_enabled;
> @@ -65,6 +66,13 @@ unsigned int mptcp_stale_loss_cnt(const struct net *net)
> 	return mptcp_get_pernet(net)->stale_loss_cnt;
> }
>
> +unsigned int mptcp_close_timeout(const struct sock *sk)
> +{
> +	if (sock_flag(sk, SOCK_DEAD))
> +		return TCP_TIMEWAIT_LEN;
> +	return mptcp_get_pernet(sock_net(sk))->close_timeout;
> +}
> +
> int mptcp_get_pm_type(const struct net *net)
> {
> 	return mptcp_get_pernet(net)->pm_type;
> @@ -79,6 +87,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
> {
> 	pernet->mptcp_enabled = 1;
> 	pernet->add_addr_timeout = TCP_RTO_MAX;
> +	pernet->close_timeout = TCP_TIMEWAIT_LEN;
> 	pernet->checksum_enabled = 0;
> 	pernet->allow_join_initial_addr_port = 1;
> 	pernet->stale_loss_cnt = 4;
> @@ -141,6 +150,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
> 		.mode = 0644,
> 		.proc_handler = proc_dostring,
> 	},
> +	{
> +		.procname = "close_timeout",
> +		.maxlen = sizeof(unsigned int),
> +		.mode = 0644,
> +		.proc_handler = proc_dointvec_jiffies,
> +	},
> 	{}
> };
>
> @@ -163,6 +178,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
> 	table[4].data = &pernet->stale_loss_cnt;
> 	table[5].data = &pernet->pm_type;
> 	table[6].data = &pernet->scheduler;
> +	table[7].data = &pernet->close_timeout;
>
> 	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
> 				     ARRAY_SIZE(mptcp_sysctl_table));
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 3facdc006adc..1a0b463f8c97 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2377,8 +2377,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
> 	if (msk->in_accept_queue && msk->first == ssk &&
> 	    (sock_flag(sk, SOCK_DEAD) || sock_flag(ssk, SOCK_DEAD))) {
> 		/* ensure later check in mptcp_worker() will dispose the msk */
> -		mptcp_set_close_tout(sk, tcp_jiffies32 - (TCP_TIMEWAIT_LEN + 1));
> 		sock_set_flag(sk, SOCK_DEAD);
> +		mptcp_set_close_tout(sk, tcp_jiffies32 - (mptcp_close_timeout(sk) + 1));
> 		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
> 		mptcp_subflow_drop_ctx(ssk);
> 		goto out_release;
> @@ -2506,7 +2506,7 @@ static bool mptcp_close_tout_expired(const struct sock *sk)
> 		return false;
>
> 	return time_after32(tcp_jiffies32,
> -		  inet_csk(sk)->icsk_mtup.probe_timestamp + TCP_TIMEWAIT_LEN);
> +		  inet_csk(sk)->icsk_mtup.probe_timestamp + mptcp_close_timeout(sk));
> }
>
> static void mptcp_check_fastclose(struct mptcp_sock *msk)
> @@ -2649,7 +2649,7 @@ void mptcp_reset_tout_timer(struct mptcp_sock *msk, unsigned long fail_tout)
> 		return;
>
> 	close_timeout = inet_csk(sk)->icsk_mtup.probe_timestamp - tcp_jiffies32 + jiffies +
> -			TCP_TIMEWAIT_LEN;
> +			mptcp_close_timeout(sk);
>
> 	/* the close timeout takes precedence on the fail one, and here at least one of
> 	 * them is active
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index aebc0cc24dad..0147419065dc 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -624,6 +624,7 @@ unsigned int mptcp_get_add_addr_timeout(const struct net *net);
> int mptcp_is_checksum_enabled(const struct net *net);
> int mptcp_allow_join_id0(const struct net *net);
> unsigned int mptcp_stale_loss_cnt(const struct net *net);
> +unsigned int mptcp_close_timeout(const struct sock *sk);
> int mptcp_get_pm_type(const struct net *net);
> const char *mptcp_get_scheduler(const struct net *net);
> void mptcp_subflow_fully_established(struct mptcp_subflow_context *subflow,
> -- 
> 2.41.0
>
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.