[PATCH mptcp-next v5 0/2] send MP_FAIL with MP_RST and others

[PATCH mptcp-next v5 0/2] send MP_FAIL with MP_RST and others

[Geliang Tang]

v5:
 - Since MP_FAIL could be sent with MP_RST and DSS, and MP_RST could be
sent with MP_FAIL and FASTCLOSE, this patch fixed the MP_RST sending in
mptcp_established_options() and mptcp_write_options().

v4:
 - add Matt's patch, fix the missing ";" in it, dropped two empty lines
and added my Tested-by tag.

v3:
 - update patch 1 as Matt suggested.
 - do more cleanups in patch 1.

v2:
 - rename patch 1 as a squash-to patch.
 - print out reset_transient in patch 2 too. Please keep patch 2 as a
standalone patch, not a squash-to patch.

Two small patches about sending MP_FAIL with MP_RST.

Geliang Tang (2):
  Squash to "mptcp: implement fastclose xmit path"
  mptcp: print out reset infos of MP_RST

 net/mptcp/options.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

-- 
2.31.1

[PATCH mptcp-next v5 1/2] Squash to "mptcp: implement fastclose xmit path"

[Geliang Tang]

MP_FAIL could be sent with MP_RST at the same time, fix this in
mptcp_established_options().

MP_RST could be sent with FASTCLOSE at the same time, fix this in
mptcp_write_options().

Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/options.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 8a1020e4285c..73242304ada5 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -829,8 +829,12 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
 
 	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST)) {
 		if (mptcp_established_options_fastclose(sk, &opt_size, remaining, opts) ||
-		    mptcp_established_options_mp_fail(sk, &opt_size, remaining, opts) ||
-		    mptcp_established_options_rst(sk, skb, &opt_size, remaining, opts)) {
+		    mptcp_established_options_mp_fail(sk, &opt_size, remaining, opts)) {
+			*size += opt_size;
+			remaining -= opt_size;
+		}
+		/* MP_RST can be used with MP_FASTCLOSE and MP_FAIL if there is room */
+		if (mptcp_established_options_rst(sk, skb, &opt_size, remaining, opts)) {
 			*size += opt_size;
 			remaining -= opt_size;
 		}
@@ -1257,6 +1261,7 @@ static u16 mptcp_make_csum(const struct mptcp_ext *mpext)
 void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 			 struct mptcp_out_options *opts)
 {
+	/* MP_FAIL is mutually exclusive with others except RST and DSS */
 	if (unlikely(OPTION_MPTCP_FAIL & opts->suboptions)) {
 		const struct sock *ssk = (const struct sock *)tp;
 		struct mptcp_subflow_context *subflow;
@@ -1271,7 +1276,16 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 		ptr += 2;
 	}
 
-	/* DSS, MPC, MPJ, ADD_ADDR, FASTCLOSE and RST are mutually exclusive,
+	/* RST is mutually exclusive with others except MP_FAIL and FASTCLOSE */
+	if (unlikely(OPTION_MPTCP_RST & opts->suboptions)) {
+		*ptr++ = mptcp_option(MPTCPOPT_RST,
+				      TCPOLEN_MPTCP_RST,
+				      opts->reset_transient,
+				      opts->reset_reason);
+		return;
+	}
+
+	/* DSS, MPC, MPJ, ADD_ADDR and FASTCLOSE are mutually exclusive,
 	 * see mptcp_established_options*()
 	 */
 	if (likely(OPTION_MPTCP_DSS & opts->suboptions)) {
@@ -1458,15 +1472,8 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
 				ptr += 1;
 			}
 		}
-	} else if (unlikely(OPTION_MPTCP_RST & opts->suboptions)) {
-		/* RST is mutually exclusive with everything else */
-		*ptr++ = mptcp_option(MPTCPOPT_RST,
-				      TCPOLEN_MPTCP_RST,
-				      opts->reset_transient,
-				      opts->reset_reason);
-		return;
 	} else if (unlikely(OPTION_MPTCP_FASTCLOSE & opts->suboptions)) {
-		/* FASTCLOSE is mutually exclusive with everything else */
+		/* FASTCLOSE is mutually exclusive with others except RST */
 		*ptr++ = mptcp_option(MPTCPOPT_MP_FASTCLOSE,
 				      TCPOLEN_MPTCP_FASTCLOSE,
 				      0, 0);
-- 
2.31.1

[PATCH mptcp-next v5 2/2] mptcp: print out reset infos of MP_RST

[Geliang Tang]

This patch printed out the reset infos, reset_transient and reset_reason,
of MP_RST in mptcp_parse_option() to show that MP_RST is received.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/options.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 73242304ada5..417cbb947832 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -336,6 +336,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		flags = *ptr++;
 		mp_opt->reset_transient = flags & MPTCP_RST_TRANSIENT;
 		mp_opt->reset_reason = *ptr;
+		pr_debug("MP_RST: transient=%u reason=%u",
+			 mp_opt->reset_transient, mp_opt->reset_reason);
 		break;
 
 	case MPTCPOPT_MP_FAIL:
-- 
2.31.1

Re: [PATCH mptcp-next v5 1/2] Squash to "mptcp: implement fastclose xmit path"

[Mat Martineau]

On Thu, 2 Dec 2021, Geliang Tang wrote:

> MP_FAIL could be sent with MP_RST at the same time, fix this in
> mptcp_established_options().
>
> MP_RST could be sent with FASTCLOSE at the same time, fix this in
> mptcp_write_options().
>

Matthieu -

We talked about the MP_FAIL+DSS combination in the meeting this week, and 
I think there is a case I overlooked in that discussion: When a peer 
receives an MP_FAIL, it may need to respond with both MP_FAIL and an 
infinite mapping (which is a DSS option). So it seems reasonable to handle 
those in the same header.


> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Matthieu Baerts <matthieu.baerts@tessares.net>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
> net/mptcp/options.c | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index 8a1020e4285c..73242304ada5 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -829,8 +829,12 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
>
> 	if (unlikely(skb && TCP_SKB_CB(skb)->tcp_flags & TCPHDR_RST)) {
> 		if (mptcp_established_options_fastclose(sk, &opt_size, remaining, opts) ||
> -		    mptcp_established_options_mp_fail(sk, &opt_size, remaining, opts) ||
> -		    mptcp_established_options_rst(sk, skb, &opt_size, remaining, opts)) {
> +		    mptcp_established_options_mp_fail(sk, &opt_size, remaining, opts)) {
> +			*size += opt_size;
> +			remaining -= opt_size;
> +		}
> +		/* MP_RST can be used with MP_FASTCLOSE and MP_FAIL if there is room */
> +		if (mptcp_established_options_rst(sk, skb, &opt_size, remaining, opts)) {
> 			*size += opt_size;
> 			remaining -= opt_size;
> 		}
> @@ -1257,6 +1261,7 @@ static u16 mptcp_make_csum(const struct mptcp_ext *mpext)
> void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 			 struct mptcp_out_options *opts)
> {
> +	/* MP_FAIL is mutually exclusive with others except RST and DSS */
> 	if (unlikely(OPTION_MPTCP_FAIL & opts->suboptions)) {
> 		const struct sock *ssk = (const struct sock *)tp;
> 		struct mptcp_subflow_context *subflow;
> @@ -1271,7 +1276,16 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 		ptr += 2;
> 	}
>
> -	/* DSS, MPC, MPJ, ADD_ADDR, FASTCLOSE and RST are mutually exclusive,
> +	/* RST is mutually exclusive with others except MP_FAIL and FASTCLOSE */
> +	if (unlikely(OPTION_MPTCP_RST & opts->suboptions)) {
> +		*ptr++ = mptcp_option(MPTCPOPT_RST,
> +				      TCPOLEN_MPTCP_RST,
> +				      opts->reset_transient,
> +				      opts->reset_reason);
> +		return;
> +	}

Geliang -

Moving this before the DSS block undoes some of Paolo's optimization for 
DSS. It would be better to handle the mutual exclusion with FASTCLOSE in a 
way that does not add this extra conditional in the common DSS case.

> +
> +	/* DSS, MPC, MPJ, ADD_ADDR and FASTCLOSE are mutually exclusive,
> 	 * see mptcp_established_options*()
> 	 */
> 	if (likely(OPTION_MPTCP_DSS & opts->suboptions)) {
> @@ -1458,15 +1472,8 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> 				ptr += 1;
> 			}
> 		}
> -	} else if (unlikely(OPTION_MPTCP_RST & opts->suboptions)) {
> -		/* RST is mutually exclusive with everything else */
> -		*ptr++ = mptcp_option(MPTCPOPT_RST,
> -				      TCPOLEN_MPTCP_RST,
> -				      opts->reset_transient,
> -				      opts->reset_reason);
> -		return;
> 	} else if (unlikely(OPTION_MPTCP_FASTCLOSE & opts->suboptions)) {
> -		/* FASTCLOSE is mutually exclusive with everything else */
> +		/* FASTCLOSE is mutually exclusive with others except RST */
> 		*ptr++ = mptcp_option(MPTCPOPT_MP_FASTCLOSE,
> 				      TCPOLEN_MPTCP_FASTCLOSE,
> 				      0, 0);
> -- 
> 2.31.1
>
>
>

--
Mat Martineau
Intel