All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <edumazet@google.com>
To: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>
Cc: netdev@vger.kernel.org, Neal Cardwell <ncardwell@google.com>,
	 Dragos Tatulea <dtatulea@nvidia.com>,
	eric.dumazet@gmail.com,  Eric Dumazet <edumazet@google.com>
Subject: [PATCH net-next 2/2] tcp: no longer abort SYN_SENT when receiving some ICMP (II)
Date: Wed, 17 Apr 2024 16:57:56 +0000	[thread overview]
Message-ID: <20240417165756.2531620-3-edumazet@google.com> (raw)
In-Reply-To: <20240417165756.2531620-1-edumazet@google.com>

Notes:

 - A prior version of this patch in commit
   0a8de364ff7a ("tcp: no longer abort SYN_SENT when
   receiving some ICMP") had to be reverted.

 - We found the root cause, and fixed it in prior patch
   in the series.

 - Many thanks to Dragos Tatulea !

Currently, non fatal ICMP messages received on behalf
of SYN_SENT sockets do call tcp_ld_RTO_revert()
to implement RFC 6069, but immediately call tcp_done(),
thus aborting the connect() attempt.

This violates RFC 1122 following requirement:

4.2.3.9  ICMP Messages
...
          o    Destination Unreachable -- codes 0, 1, 5

                 Since these Unreachable messages indicate soft error
                 conditions, TCP MUST NOT abort the connection, and it
                 SHOULD make the information available to the
                 application.

This patch makes sure non 'fatal' ICMP[v6] messages do not
abort the connection attempt.

It enables RFC 6069 for SYN_SENT sockets as a result.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Tested-by: Dragos Tatulea <dtatulea@nvidia.com>
---
 net/ipv4/tcp_ipv4.c | 6 ++++++
 net/ipv6/tcp_ipv6.c | 9 ++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a717db99972d977a64178d7ed1109325d64a6d51..4b50d09f84b9ae7fec98a71bedf39594ab85e5ea 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -482,6 +482,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 	const int code = icmp_hdr(skb)->code;
 	struct sock *sk;
 	struct request_sock *fastopen;
+	bool harderr = false;
 	u32 seq, snd_una;
 	int err;
 	struct net *net = dev_net(skb->dev);
@@ -555,6 +556,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 		goto out;
 	case ICMP_PARAMETERPROB:
 		err = EPROTO;
+		harderr = true;
 		break;
 	case ICMP_DEST_UNREACH:
 		if (code > NR_ICMP_UNREACH)
@@ -579,6 +581,7 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 		}
 
 		err = icmp_err_convert[code].errno;
+		harderr = icmp_err_convert[code].fatal;
 		/* check if this ICMP message allows revert of backoff.
 		 * (see RFC 6069)
 		 */
@@ -605,6 +608,9 @@ int tcp_v4_err(struct sk_buff *skb, u32 info)
 		if (inet_test_bit(RECVERR, sk))
 			ip_icmp_error(sk, skb, err, th->dest, info, (u8 *)th);
 
+		if (!harderr)
+			break;
+
 		if (!sock_owned_by_user(sk)) {
 			WRITE_ONCE(sk->sk_err, err);
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index bb7c3caf4f8536dabdcb3dbe7c90aff9c8985c90..f31527f0a13dee9488ce72834d89524e83f61e5f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -382,7 +382,7 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	struct tcp_sock *tp;
 	__u32 seq, snd_una;
 	struct sock *sk;
-	bool fatal;
+	bool harderr;
 	int err;
 
 	sk = __inet6_lookup_established(net, net->ipv4.tcp_death_row.hashinfo,
@@ -403,9 +403,9 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		return 0;
 	}
 	seq = ntohl(th->seq);
-	fatal = icmpv6_err_convert(type, code, &err);
+	harderr = icmpv6_err_convert(type, code, &err);
 	if (sk->sk_state == TCP_NEW_SYN_RECV) {
-		tcp_req_err(sk, seq, fatal);
+		tcp_req_err(sk, seq, harderr);
 		return 0;
 	}
 
@@ -490,6 +490,9 @@ static int tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 		ipv6_icmp_error(sk, skb, err, th->dest, ntohl(info), (u8 *)th);
 
+		if (!harderr)
+			break;
+
 		if (!sock_owned_by_user(sk)) {
 			WRITE_ONCE(sk->sk_err, err);
 			sk_error_report(sk);		/* Wake people up to see the error (see connect in sock.c) */
-- 
2.44.0.683.g7961c838ac-goog


  parent reply	other threads:[~2024-04-17 16:58 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17 16:57 [PATCH net-next 0/2] tcp: tcp_v4_err() changes Eric Dumazet
2024-04-17 16:57 ` [PATCH net-next 1/2] tcp: conditionally call ip_icmp_error() from tcp_v4_err() Eric Dumazet
2024-04-17 17:08   ` Maciej Żenczykowski
2024-04-18  3:22   ` Jason Xing
2024-04-18  6:45     ` Eric Dumazet
2024-04-18  6:53       ` Jason Xing
2024-04-18  8:02   ` Paolo Abeni
2024-04-18  8:03     ` Eric Dumazet
2024-04-18  9:26       ` Eric Dumazet
2024-04-18  9:58         ` Paolo Abeni
2024-04-18 10:15           ` Eric Dumazet
2024-04-18 10:22             ` Eric Dumazet
2024-04-18 10:36               ` Eric Dumazet
2024-04-18 17:46             ` David Ahern
2024-04-18 17:47               ` Eric Dumazet
2024-04-18 18:02                 ` David Ahern
2024-04-18 18:09                   ` Eric Dumazet
2024-04-18 18:09                 ` Jakub Kicinski
2024-04-18 18:14                   ` Eric Dumazet
2024-04-18 20:20                   ` David Ahern
2024-04-18 17:56         ` David Ahern
2024-04-17 16:57 ` Eric Dumazet [this message]
2024-04-18  3:24   ` [PATCH net-next 2/2] tcp: no longer abort SYN_SENT when receiving some ICMP (II) Jason Xing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240417165756.2531620-3-edumazet@google.com \
    --to=edumazet@google.com \
    --cc=davem@davemloft.net \
    --cc=dtatulea@nvidia.com \
    --cc=eric.dumazet@gmail.com \
    --cc=kuba@kernel.org \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.