diff mbox series

[net-next,v4,5/6] mptcp: support rstreason for passive reset

Message ID 20240411115630.38420-6-kerneljasonxing@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Implement reset reason mechanism to detect | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 944 this patch: 944
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 954 this patch: 954
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 955 this patch: 955
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-04-12--06-00 (tests: 962)

Commit Message

Jason Xing April 11, 2024, 11:56 a.m. UTC 
From: Jason Xing <kernelxing@tencent.com>

It relys on what reset options in the skb are as rfc8684 says. Reusing
this logic can save us much energy. This patch replaces most of the prior
NOT_SPECIFIED reasons.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 net/mptcp/subflow.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Comments

Eric Dumazet April 16, 2024, 6:33 a.m. UTC | #1 
On Thu, Apr 11, 2024 at 1:57 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> It relys on what reset options in the skb are as rfc8684 says. Reusing
> this logic can save us much energy. This patch replaces most of the prior
> NOT_SPECIFIED reasons.
>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  net/mptcp/subflow.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index ba0a252c113f..25eaad94cb79 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -308,8 +308,12 @@ static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
>                 return dst;
>
>         dst_release(dst);
> -       if (!req->syncookie)
> -               tcp_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> +       if (!req->syncookie) {
> +               struct mptcp_ext *mpext = mptcp_get_ext(skb);
> +               enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
> +
> +               tcp_request_sock_ops.send_reset(sk, skb, reason);
> +       }
>         return NULL;
>  }
>
> @@ -375,8 +379,12 @@ static struct dst_entry *subflow_v6_route_req(const struct sock *sk,
>                 return dst;
>
>         dst_release(dst);
> -       if (!req->syncookie)
> -               tcp6_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> +       if (!req->syncookie) {
> +               struct mptcp_ext *mpext = mptcp_get_ext(skb);
> +               enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
> +
> +               tcp6_request_sock_ops.send_reset(sk, skb, reason);
> +       }
>         return NULL;
>  }
>  #endif
> @@ -783,6 +791,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
>         bool fallback, fallback_is_fatal;
>         struct mptcp_sock *owner;
>         struct sock *child;
> +       enum sk_rst_reason reason;

reverse xmas tree ?

>
>         pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
>
> @@ -911,7 +920,8 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
>         tcp_rsk(req)->drop_req = true;
>         inet_csk_prepare_for_destroy_sock(child);
>         tcp_done(child);
> -       req->rsk_ops->send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> +       reason = convert_mptcp_reason(mptcp_get_ext(skb)->reset_reason);
> +       req->rsk_ops->send_reset(sk, skb, reason);
>
>         /* The last child reference will be released by the caller */
>         return child;
> --
> 2.37.3
>
Jason Xing April 16, 2024, 7:45 a.m. UTC | #2 
On Tue, Apr 16, 2024 at 2:33 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Apr 11, 2024 at 1:57 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
> >
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > It relys on what reset options in the skb are as rfc8684 says. Reusing
> > this logic can save us much energy. This patch replaces most of the prior
> > NOT_SPECIFIED reasons.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  net/mptcp/subflow.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index ba0a252c113f..25eaad94cb79 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -308,8 +308,12 @@ static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
> >                 return dst;
> >
> >         dst_release(dst);
> > -       if (!req->syncookie)
> > -               tcp_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > +       if (!req->syncookie) {
> > +               struct mptcp_ext *mpext = mptcp_get_ext(skb);
> > +               enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
> > +
> > +               tcp_request_sock_ops.send_reset(sk, skb, reason);
> > +       }
> >         return NULL;
> >  }
> >
> > @@ -375,8 +379,12 @@ static struct dst_entry *subflow_v6_route_req(const struct sock *sk,
> >                 return dst;
> >
> >         dst_release(dst);
> > -       if (!req->syncookie)
> > -               tcp6_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > +       if (!req->syncookie) {
> > +               struct mptcp_ext *mpext = mptcp_get_ext(skb);
> > +               enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
> > +
> > +               tcp6_request_sock_ops.send_reset(sk, skb, reason);
> > +       }
> >         return NULL;
> >  }
> >  #endif
> > @@ -783,6 +791,7 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
> >         bool fallback, fallback_is_fatal;
> >         struct mptcp_sock *owner;
> >         struct sock *child;
> > +       enum sk_rst_reason reason;
>
> reverse xmas tree ?

Got it. Thanks!

>
> >
> >         pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
> >
> > @@ -911,7 +920,8 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
> >         tcp_rsk(req)->drop_req = true;
> >         inet_csk_prepare_for_destroy_sock(child);
> >         tcp_done(child);
> > -       req->rsk_ops->send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > +       reason = convert_mptcp_reason(mptcp_get_ext(skb)->reset_reason);
> > +       req->rsk_ops->send_reset(sk, skb, reason);
> >
> >         /* The last child reference will be released by the caller */
> >         return child;
> > --
> > 2.37.3
> >
Paolo Abeni April 16, 2024, 8:04 a.m. UTC | #3 
On Thu, 2024-04-11 at 19:56 +0800, Jason Xing wrote:
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index ba0a252c113f..25eaad94cb79 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -308,8 +308,12 @@ static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
>  		return dst;
>  
>  	dst_release(dst);
> -	if (!req->syncookie)
> -		tcp_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> +	if (!req->syncookie) {
> +		struct mptcp_ext *mpext = mptcp_get_ext(skb);
> +		enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);

Since you already have to repost, very minor nit: even the above
strictly speaking does not respect the reverse xmass tree.

		enum sk_rst_reason reason;

		reason = convert_mptcp_reason(mpext->reset_reason);
> +		tcp_request_sock_ops.send_reset(sk, skb, reason);

Cheers,

Paolo
Jason Xing April 16, 2024, 8:19 a.m. UTC | #4 
On Tue, Apr 16, 2024 at 4:04 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Thu, 2024-04-11 at 19:56 +0800, Jason Xing wrote:
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index ba0a252c113f..25eaad94cb79 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -308,8 +308,12 @@ static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
> >               return dst;
> >
> >       dst_release(dst);
> > -     if (!req->syncookie)
> > -             tcp_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
> > +     if (!req->syncookie) {
> > +             struct mptcp_ext *mpext = mptcp_get_ext(skb);
> > +             enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
>
> Since you already have to repost, very minor nit: even the above
> strictly speaking does not respect the reverse xmass tree.
>
>                 enum sk_rst_reason reason;
>
>                 reason = convert_mptcp_reason(mpext->reset_reason);
> > +             tcp_request_sock_ops.send_reset(sk, skb, reason);

Thanks, I will update this too.

>
> Cheers,
>
> Paolo
>
diff mbox series

Patch

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index ba0a252c113f..25eaad94cb79 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -308,8 +308,12 @@  static struct dst_entry *subflow_v4_route_req(const struct sock *sk,
 		return dst;
 
 	dst_release(dst);
-	if (!req->syncookie)
-		tcp_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
+	if (!req->syncookie) {
+		struct mptcp_ext *mpext = mptcp_get_ext(skb);
+		enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
+
+		tcp_request_sock_ops.send_reset(sk, skb, reason);
+	}
 	return NULL;
 }
 
@@ -375,8 +379,12 @@  static struct dst_entry *subflow_v6_route_req(const struct sock *sk,
 		return dst;
 
 	dst_release(dst);
-	if (!req->syncookie)
-		tcp6_request_sock_ops.send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
+	if (!req->syncookie) {
+		struct mptcp_ext *mpext = mptcp_get_ext(skb);
+		enum sk_rst_reason reason = convert_mptcp_reason(mpext->reset_reason);
+
+		tcp6_request_sock_ops.send_reset(sk, skb, reason);
+	}
 	return NULL;
 }
 #endif
@@ -783,6 +791,7 @@  static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 	bool fallback, fallback_is_fatal;
 	struct mptcp_sock *owner;
 	struct sock *child;
+	enum sk_rst_reason reason;
 
 	pr_debug("listener=%p, req=%p, conn=%p", listener, req, listener->conn);
 
@@ -911,7 +920,8 @@  static struct sock *subflow_syn_recv_sock(const struct sock *sk,
 	tcp_rsk(req)->drop_req = true;
 	inet_csk_prepare_for_destroy_sock(child);
 	tcp_done(child);
-	req->rsk_ops->send_reset(sk, skb, SK_RST_REASON_NOT_SPECIFIED);
+	reason = convert_mptcp_reason(mptcp_get_ext(skb)->reset_reason);
+	req->rsk_ops->send_reset(sk, skb, reason);
 
 	/* The last child reference will be released by the caller */
 	return child;