[PATCH net-next v9 00/10] introduce drop reasons for tcp receive path

[PATCH net-next v9 00/10] introduce drop reasons for tcp receive path

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

When I was debugging the reason about why the skb should be dropped in
syn cookie mode, I found out that this NOT_SPECIFIED reason is too
general. Thus I decided to refine it.

v9
Link: https://lore.kernel.org/netdev/20240222113003.67558-1-kerneljasonxing@gmail.com/
1. nit: remove one unneeded 'else' (David)
2. add reviewed-by tags (Eric, David)

v8
Link: https://lore.kernel.org/netdev/20240221025732.68157-1-kerneljasonxing@gmail.com/
1. refine part of codes in patch [03/10] and patch [10/10] (Eric)
2. squash patch [11/11] in the last version into patch [10/11] (Eric)
3. add reviewed-by tags (Eric)

v7
Link: https://lore.kernel.org/all/20240219032838.91723-1-kerneljasonxing@gmail.com/
1. fix some misspelled problem (Kuniyuki)
2. remove redundant codes in tcp_v6_do_rcv() (Kuniyuki)
3. add reviewed-by tag in patch [02/11] (Kuniyuki)

v6
Link: https://lore.kernel.org/all/c987d2c79e4a4655166eb8eafef473384edb37fb.camel@redhat.com/
Link: https://lore.kernel.org/all/CAL+tcoAgSjwsmFnDh_Gs9ZgMi-y5awtVx+4VhJPNRADjo7LLSA@mail.gmail.com/
1. Take one case into consideration in tcp_v6_do_rcv(), behave like old
days, or else it will trigger errors (Paolo).
2. Extend NO_SOCKET reason to consider two more reasons for request
socket and child socket.

v5:
Link: https://lore.kernel.org/netdev/20240213134205.8705-1-kerneljasonxing@gmail.com/
Link: https://lore.kernel.org/netdev/20240213140508.10878-1-kerneljasonxing@gmail.com/
1. Use SKB_DROP_REASON_IP_OUTNOROUTES instead of introducing a new
   one (Eric, David)
2. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket
   allocation (Eric)
3. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
4. avoid duplication of these opt_skb tests/actions (Eric)
5. Use new name (TCP_ABORT_ON_DATA) for readability (David)
6. Reuse IP_OUTNOROUTES instead of INVALID_DST (Eric)


---
HISTORY
This series is combined with 2 series sent before suggested by Jakub. So
I'm going to separately write changelogs for each of them.

PATCH 1/11 - 5/11
preivious Link: https://lore.kernel.org/netdev/20240213134205.8705-1-kerneljasonxing@gmail.com/
Summary
1. introduce all the dropreasons we need, [1/11] patch.
2. use new dropreasons in ipv4 cookie check, [2/11],[3/11] patch.
3. use new dropreasons ipv6 cookie check, [4/11],[5/11] patch.

v4:
Link: https://lore.kernel.org/netdev/20240212172302.3f95e454@kernel.org/
1. Fix misspelled name in Kdoc as suggested by Jakub.

v3:
Link: https://lore.kernel.org/all/CANn89iK40SoyJ8fS2U5kp3pDruo=zfQNPL-ppOF+LYaS9z-MVA@mail.gmail.com/
1. Split that patch into some smaller ones as suggested by Eric.

v2:
Link: https://lore.kernel.org/all/20240204104601.55760-1-kerneljasonxing@gmail.com/
1. change the title of 2/2 patch.
2. fix some warnings checkpatch tool showed before.
3. use return value instead of adding more parameters suggested by Eric.


PATCH 6/11 - 11/11
previous Link: https://lore.kernel.org/netdev/20240213140508.10878-1-kerneljasonxing@gmail.com/
v4:
Link: https://lore.kernel.org/netdev/CANn89iJar+H3XkQ8HpsirH7b-_sbFe9NBUdAAO3pNJK3CKr_bg@mail.gmail.com/
Link: https://lore.kernel.org/netdev/20240213131205.4309-1-kerneljasonxing@gmail.com/
Already got rid of @acceptable in tcp_rcv_state_process(), so I need to
remove *TCP_CONNREQNOTACCEPTABLE related codes which I wrote in the v3
series.

v3:
Link: https://lore.kernel.org/all/CANn89iK40SoyJ8fS2U5kp3pDruo=zfQNPL-ppOF+LYaS9z-MVA@mail.gmail.com/
1. Split that patch into some smaller ones as suggested by Eric.

v2:
Link: https://lore.kernel.org/all/20240204104601.55760-1-kerneljasonxing@gmail.com/
1. change the title of 2/2 patch.
2. fix some warnings checkpatch tool showed before.
3. use return value instead of adding more parameters suggested by Eric.

Jason Xing (10):
  tcp: add a dropreason definitions and prepare for cookie check
  tcp: directly drop skb in cookie check for ipv4
  tcp: use drop reasons in cookie check for ipv4
  tcp: directly drop skb in cookie check for ipv6
  tcp: use drop reasons in cookie check for ipv6
  tcp: introduce dropreasons in receive path
  tcp: add more specific possible drop reasons in
    tcp_rcv_synsent_state_process()
  tcp: add dropreasons in tcp_rcv_state_process()
  tcp: make the dropreason really work when calling
    tcp_rcv_state_process()
  tcp: make dropreason in tcp_child_process() work

 include/net/dropreason-core.h | 26 ++++++++++++++++++++++++--
 include/net/tcp.h             |  4 ++--
 net/ipv4/syncookies.c         | 21 ++++++++++++++++-----
 net/ipv4/tcp_input.c          | 24 ++++++++++++++++--------
 net/ipv4/tcp_ipv4.c           | 17 ++++++++++-------
 net/ipv4/tcp_minisocks.c      |  9 +++++----
 net/ipv6/syncookies.c         | 18 +++++++++++++++---
 net/ipv6/tcp_ipv6.c           | 22 ++++++++++++----------
 8 files changed, 100 insertions(+), 41 deletions(-)

-- 
2.37.3

[PATCH net-next v9 01/10] tcp: add a dropreason definitions and prepare for cookie check

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Adding one drop reason to detect the condition of skb dropped
because of hook points in cookie check and extending NO_SOCKET
to consider another two cases can be used later.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89iJ3gLMn5psbzfVCOo2=v4nMn4m41wpr6svxyAmO4R1m6g@mail.gmail.com/
1. add reviewed-by tag (Eric)

v7
Link: https://lore.kernel.org/all/20240219040630.94637-1-kuniyu@amazon.com/
1. nit: change "invalid" to "valid" (Kuniyuki)
2. add more description.

v6
Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
1. Modify the description NO_SOCKET to extend other two kinds of invalid
socket cases.
What I think about it is we can use it as a general indicator for three kinds of
sockets which are invalid/NULL, like what we did to TCP_FLAGS.
Any better ideas/suggestions are welcome :)

v5
Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
Link: https://lore.kernel.org/netdev/632c6fd4-e060-4b8e-a80e-5d545a6c6b6c@kernel.org/
1. Use SKB_DROP_REASON_IP_OUTNOROUTES instead of introducing a new one (Eric, David)
2. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
3. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
4. Reuse IP_OUTNOROUTES instead of INVALID_DST (Eric)
5. adjust the title and description.

v4
Link: https://lore.kernel.org/netdev/20240212172302.3f95e454@kernel.org/
1. fix misspelled name in kdoc as Jakub said
---
 include/net/dropreason-core.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 6d3a20163260..a871f061558d 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -54,6 +54,7 @@
 	FN(NEIGH_QUEUEFULL)		\
 	FN(NEIGH_DEAD)			\
 	FN(TC_EGRESS)			\
+	FN(SECURITY_HOOK)		\
 	FN(QDISC_DROP)			\
 	FN(CPU_BACKLOG)			\
 	FN(XDP)				\
@@ -105,7 +106,13 @@ enum skb_drop_reason {
 	SKB_CONSUMED,
 	/** @SKB_DROP_REASON_NOT_SPECIFIED: drop reason is not specified */
 	SKB_DROP_REASON_NOT_SPECIFIED,
-	/** @SKB_DROP_REASON_NO_SOCKET: socket not found */
+	/**
+	 * @SKB_DROP_REASON_NO_SOCKET: no valid socket that can be used.
+	 * Reason could be one of three cases:
+	 * 1) no established/listening socket found during lookup process
+	 * 2) no valid request socket during 3WHS process
+	 * 3) no valid child socket during 3WHS process
+	 */
 	SKB_DROP_REASON_NO_SOCKET,
 	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
 	SKB_DROP_REASON_PKT_TOO_SMALL,
@@ -271,6 +278,8 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_NEIGH_DEAD,
 	/** @SKB_DROP_REASON_TC_EGRESS: dropped in TC egress HOOK */
 	SKB_DROP_REASON_TC_EGRESS,
+	/** @SKB_DROP_REASON_SECURITY_HOOK: dropped due to security HOOK */
+	SKB_DROP_REASON_SECURITY_HOOK,
 	/**
 	 * @SKB_DROP_REASON_QDISC_DROP: dropped by qdisc when packet outputting (
 	 * failed to enqueue to current qdisc)
-- 
2.37.3

[PATCH net-next v9 02/10] tcp: directly drop skb in cookie check for ipv4

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Only move the skb drop from tcp_v4_do_rcv() to cookie_v4_check() itself,
no other changes made. It can help us refine the specific drop reasons
later.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89i+foA-AW3KCNw232eCC5GDi_3O0JG-mpvyiQJYuxKxnRA@mail.gmail.com/
1. add reviewed-by tag (Eric)

v7
Link: https://lore.kernel.org/all/20240219041350.95304-1-kuniyu@amazon.com/
1. add reviewed-by tag (Kuniyuki)
---
 net/ipv4/syncookies.c | 4 ++++
 net/ipv4/tcp_ipv4.c   | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index be88bf586ff9..38f331da6677 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -408,6 +408,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	struct rtable *rt;
 	__u8 rcv_wscale;
 	int full_space;
+	SKB_DR(reason);
 
 	if (!READ_ONCE(net->ipv4.sysctl_tcp_syncookies) ||
 	    !th->ack || th->rst)
@@ -477,10 +478,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	 */
 	if (ret)
 		inet_sk(ret)->cork.fl.u.ip4 = fl4;
+	else
+		goto out_drop;
 out:
 	return ret;
 out_free:
 	reqsk_free(req);
 out_drop:
+	kfree_skb_reason(skb, reason);
 	return NULL;
 }
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0c50c5a32b84..0a944e109088 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1915,7 +1915,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 		struct sock *nsk = tcp_v4_cookie_check(sk, skb);
 
 		if (!nsk)
-			goto discard;
+			return 0;
 		if (nsk != sk) {
 			if (tcp_child_process(sk, nsk, skb)) {
 				rsk = nsk;
-- 
2.37.3

[PATCH net-next v9 03/10] tcp: use drop reasons in cookie check for ipv4

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Now it's time to use the prepared definitions to refine this part.
Four reasons used might enough for now, I think.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
Link: https://lore.kernel.org/netdev/CANn89iLOxJxmOCH1LxXf-YwRBzKXcjPRmgQeQ6A3bKRmW8=ksg@mail.gmail.com/
1. add reviewed-by tag (David)
2. add reviewed-by tag (Eric)

v8
Link: https://lore.kernel.org/netdev/CANn89iL-FH6jzoxhyKSMioj-zdBsHqNpR7YTGz8ytM=FZSGrug@mail.gmail.com/
1. refine the codes (Eric)

v6:
Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
1. Not use NOMEM because of MPTCP (Kuniyuki). I chose to use NO_SOCKET as
an indicator which can be used as three kinds of cases to tell people that we're
unable to get a valid one. It's a relatively general reason like what we did
to TCP_FLAGS.
Any better ideas/suggestions are welcome :)

v5:
Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
Link: https://lore.kernel.org/netdev/632c6fd4-e060-4b8e-a80e-5d545a6c6b6c@kernel.org/
1. Use SKB_DROP_REASON_IP_OUTNOROUTES instead of introducing a new one (Eric, David)
2. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
3. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
---
 net/ipv4/syncookies.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index 38f331da6677..7972ad3d7c73 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -421,8 +421,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 		if (IS_ERR(req))
 			goto out;
 	}
-	if (!req)
+	if (!req) {
+		SKB_DR_SET(reason, NO_SOCKET);
 		goto out_drop;
+	}
 
 	ireq = inet_rsk(req);
 
@@ -434,8 +436,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	 */
 	RCU_INIT_POINTER(ireq->ireq_opt, tcp_v4_save_options(net, skb));
 
-	if (security_inet_conn_request(sk, skb, req))
+	if (security_inet_conn_request(sk, skb, req)) {
+		SKB_DR_SET(reason, SECURITY_HOOK);
 		goto out_free;
+	}
 
 	tcp_ao_syncookie(sk, skb, req, AF_INET);
 
@@ -452,8 +456,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 			   ireq->ir_loc_addr, th->source, th->dest, sk->sk_uid);
 	security_req_classify_flow(req, flowi4_to_flowi_common(&fl4));
 	rt = ip_route_output_key(net, &fl4);
-	if (IS_ERR(rt))
+	if (IS_ERR(rt)) {
+		SKB_DR_SET(reason, IP_OUTNOROUTES);
 		goto out_free;
+	}
 
 	/* Try to redo what tcp_v4_send_synack did. */
 	req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
@@ -476,10 +482,11 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
 	/* ip_queue_xmit() depends on our flow being setup
 	 * Normal sockets get it right from inet_csk_route_child_sock()
 	 */
-	if (ret)
-		inet_sk(ret)->cork.fl.u.ip4 = fl4;
-	else
+	if (!ret) {
+		SKB_DR_SET(reason, NO_SOCKET);
 		goto out_drop;
+	}
+	inet_sk(ret)->cork.fl.u.ip4 = fl4;
 out:
 	return ret;
 out_free:
-- 
2.37.3

[PATCH net-next v9 04/10] tcp: directly drop skb in cookie check for ipv6

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Like previous patch does, only moving skb drop logical code to
cookie_v6_check() for later refinement.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89iL8M=1vFdZ1hBb4POuz+MKQ50fmBAewfbowEH3jpEtpZQ@mail.gmail.com/
1. add reviewed-by tag (Eric)

v7:
Link: https://lore.kernel.org/all/20240219043815.98410-1-kuniyu@amazon.com/
1. refine the code (by removing redundant check), no functional changes. (Kuniyuki)

v6
Link: https://lore.kernel.org/all/c987d2c79e4a4655166eb8eafef473384edb37fb.camel@redhat.com/
Link: https://lore.kernel.org/all/CAL+tcoAgSjwsmFnDh_Gs9ZgMi-y5awtVx+4VhJPNRADjo7LLSA@mail.gmail.com/
1. take one case into consideration, behave like old days, or else it will trigger errors.

v5
Link: https://lore.kernel.org/netdev/CANn89iKz7=1q7e8KY57Dn3ED7O=RCOfLxoHQKO4eNXnZa1OPWg@mail.gmail.com/
1. avoid duplication of these opt_skb tests/actions (Eric)
---
 net/ipv6/syncookies.c | 4 ++++
 net/ipv6/tcp_ipv6.c   | 5 +----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index 6b9c69278819..ea0d9954a29f 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -177,6 +177,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 	struct sock *ret = sk;
 	__u8 rcv_wscale;
 	int full_space;
+	SKB_DR(reason);
 
 	if (!READ_ONCE(net->ipv4.sysctl_tcp_syncookies) ||
 	    !th->ack || th->rst)
@@ -256,10 +257,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 	ireq->ecn_ok &= cookie_ecn_ok(net, dst);
 
 	ret = tcp_get_cookie_sock(sk, skb, req, dst);
+	if (!ret)
+		goto out_drop;
 out:
 	return ret;
 out_free:
 	reqsk_free(req);
 out_drop:
+	kfree_skb_reason(skb, reason);
 	return NULL;
 }
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 57b25b1fc9d9..0c180bb8187f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1653,11 +1653,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	if (sk->sk_state == TCP_LISTEN) {
 		struct sock *nsk = tcp_v6_cookie_check(sk, skb);
 
-		if (!nsk)
-			goto discard;
-
 		if (nsk != sk) {
-			if (tcp_child_process(sk, nsk, skb))
+			if (nsk && tcp_child_process(sk, nsk, skb))
 				goto reset;
 			if (opt_skb)
 				__kfree_skb(opt_skb);
-- 
2.37.3

[PATCH net-next v9 05/10] tcp: use drop reasons in cookie check for ipv6

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Like what I did to ipv4 mode, refine this part: adding more drop
reasons for better tracing.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89i+b+bYqX0aVv9KtSm=nLmEQznamZmqaOzfqtJm_ux9JBw@mail.gmail.com/
1. add reviewed-by tag (Eric)

v6:
Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
1. Not use NOMEM because of MPTCP (Kuniyuki). I chose to use NO_SOCKET as
an indicator which can be used as three kinds of cases to tell people that we're
unable to get a valid one. It's a relatively general reason like what we did
to TCP_FLAGS.

v5:
Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
1. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
2. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
3. Reuse IP_OUTNOROUTES instead of INVALID_DST (Eric)
---
 net/ipv6/syncookies.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index ea0d9954a29f..8bad0a44a0a6 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -190,16 +190,20 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 		if (IS_ERR(req))
 			goto out;
 	}
-	if (!req)
+	if (!req) {
+		SKB_DR_SET(reason, NO_SOCKET);
 		goto out_drop;
+	}
 
 	ireq = inet_rsk(req);
 
 	ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
 	ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
 
-	if (security_inet_conn_request(sk, skb, req))
+	if (security_inet_conn_request(sk, skb, req)) {
+		SKB_DR_SET(reason, SECURITY_HOOK);
 		goto out_free;
+	}
 
 	if (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
 	    np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
@@ -236,8 +240,10 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 		security_req_classify_flow(req, flowi6_to_flowi_common(&fl6));
 
 		dst = ip6_dst_lookup_flow(net, sk, &fl6, final_p);
-		if (IS_ERR(dst))
+		if (IS_ERR(dst)) {
+			SKB_DR_SET(reason, IP_OUTNOROUTES);
 			goto out_free;
+		}
 	}
 
 	req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
@@ -257,8 +263,10 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
 	ireq->ecn_ok &= cookie_ecn_ok(net, dst);
 
 	ret = tcp_get_cookie_sock(sk, skb, req, dst);
-	if (!ret)
+	if (!ret) {
+		SKB_DR_SET(reason, NO_SOCKET);
 		goto out_drop;
+	}
 out:
 	return ret;
 out_free:
-- 
2.37.3

[PATCH net-next v9 06/10] tcp: introduce dropreasons in receive path

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Soon later patches can use these relatively more accurate
reasons to recognise and find out the cause.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
Link: https://lore.kernel.org/netdev/CANn89i+j55o_1B2SV56n=u=NHukmN_CoRib4VBzpUBVcKRjAMw@mail.gmail.com/
1. add reviewed-by tag (David)
2. add reviewed-by tag (Eric)

v7
Link: https://lore.kernel.org/all/20240219044744.99367-1-kuniyu@amazon.com/
1. nit: nit: s/. because of/ because/ (Kuniyuki)

v5:
Link: https://lore.kernel.org/netdev/3a495358-4c47-4a9f-b116-5f9c8b44e5ab@kernel.org/
1. Use new name (TCP_ABORT_ON_DATA) for readability (David)
2. change the title of this patch
---
 include/net/dropreason-core.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index a871f061558d..af7c7146219d 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -30,6 +30,7 @@
 	FN(TCP_AOFAILURE)		\
 	FN(SOCKET_BACKLOG)		\
 	FN(TCP_FLAGS)			\
+	FN(TCP_ABORT_ON_DATA)	\
 	FN(TCP_ZEROWINDOW)		\
 	FN(TCP_OLD_DATA)		\
 	FN(TCP_OVERWINDOW)		\
@@ -37,6 +38,7 @@
 	FN(TCP_RFC7323_PAWS)		\
 	FN(TCP_OLD_SEQUENCE)		\
 	FN(TCP_INVALID_SEQUENCE)	\
+	FN(TCP_INVALID_ACK_SEQUENCE)	\
 	FN(TCP_RESET)			\
 	FN(TCP_INVALID_SYN)		\
 	FN(TCP_CLOSE)			\
@@ -204,6 +206,11 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_SOCKET_BACKLOG,
 	/** @SKB_DROP_REASON_TCP_FLAGS: TCP flags invalid */
 	SKB_DROP_REASON_TCP_FLAGS,
+	/**
+	 * @SKB_DROP_REASON_TCP_ABORT_ON_DATA: abort on data, corresponding to
+	 * LINUX_MIB_TCPABORTONDATA
+	 */
+	SKB_DROP_REASON_TCP_ABORT_ON_DATA,
 	/**
 	 * @SKB_DROP_REASON_TCP_ZEROWINDOW: TCP receive window size is zero,
 	 * see LINUX_MIB_TCPZEROWINDOWDROP
@@ -228,13 +235,19 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_TCP_OFOMERGE,
 	/**
 	 * @SKB_DROP_REASON_TCP_RFC7323_PAWS: PAWS check, corresponding to
-	 * LINUX_MIB_PAWSESTABREJECTED
+	 * LINUX_MIB_PAWSESTABREJECTED, LINUX_MIB_PAWSACTIVEREJECTED
 	 */
 	SKB_DROP_REASON_TCP_RFC7323_PAWS,
 	/** @SKB_DROP_REASON_TCP_OLD_SEQUENCE: Old SEQ field (duplicate packet) */
 	SKB_DROP_REASON_TCP_OLD_SEQUENCE,
 	/** @SKB_DROP_REASON_TCP_INVALID_SEQUENCE: Not acceptable SEQ field */
 	SKB_DROP_REASON_TCP_INVALID_SEQUENCE,
+	/**
+	 * @SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE: Not acceptable ACK SEQ
+	 * field because ack sequence is not in the window between snd_una
+	 * and snd_nxt
+	 */
+	SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE,
 	/** @SKB_DROP_REASON_TCP_RESET: Invalid RST packet */
 	SKB_DROP_REASON_TCP_RESET,
 	/**
-- 
2.37.3

[PATCH net-next v9 07/10] tcp: add more specific possible drop reasons in tcp_rcv_synsent_state_process()

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

This patch does two things:
1) add two more new reasons
2) only change the return value(1) to various drop reason values
for the future use

For now, we still cannot trace those two reasons. We'll implement the full
function in the subsequent patch in this series.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89i+EF77F5ZJbbkiDQgwgAqSKWtD3djUF807zQ=AswGvosQ@mail.gmail.com/
1. add reviewed-by tag (Eric)
---
 net/ipv4/tcp_input.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 74c03f0a6c0c..83308cca1610 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6361,6 +6361,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 				inet_csk_reset_xmit_timer(sk,
 						ICSK_TIME_RETRANS,
 						TCP_TIMEOUT_MIN, TCP_RTO_MAX);
+			SKB_DR_SET(reason, TCP_INVALID_ACK_SEQUENCE);
 			goto reset_and_undo;
 		}
 
@@ -6369,6 +6370,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 			     tcp_time_stamp_ts(tp))) {
 			NET_INC_STATS(sock_net(sk),
 					LINUX_MIB_PAWSACTIVEREJECTED);
+			SKB_DR_SET(reason, TCP_RFC7323_PAWS);
 			goto reset_and_undo;
 		}
 
@@ -6572,7 +6574,8 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
 reset_and_undo:
 	tcp_clear_options(&tp->rx_opt);
 	tp->rx_opt.mss_clamp = saved_clamp;
-	return 1;
+	/* we can reuse/return @reason to its caller to handle the exception */
+	return reason;
 }
 
 static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
-- 
2.37.3

[PATCH net-next v9 08/10] tcp: add dropreasons in tcp_rcv_state_process()

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

In this patch, I equipped this function with more dropreasons, but
it still doesn't work yet, which I will do later.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/CAL+tcoCbsbM=HyXRqs2+QVrY8FSKmqYC47m87Axiyk1wk4omwQ@mail.gmail.com/
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. nit: remove unnecessary else (David)
2. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89iJJ9XTVeC=qbSNUnOhQMAsfBfouc9qUJY7MxgQtYGmB3Q@mail.gmail.com/
1. add reviewed-by tag (Eric)

v5:
Link: https://lore.kernel.org/netdev/3a495358-4c47-4a9f-b116-5f9c8b44e5ab@kernel.org/
1. Use new name (TCP_ABORT_ON_DATA) for readability (David)
---
 include/net/tcp.h    |  2 +-
 net/ipv4/tcp_input.c | 19 ++++++++++++-------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 58e65af74ad1..e5af9a5b411b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -348,7 +348,7 @@ void tcp_wfree(struct sk_buff *skb);
 void tcp_write_timer_handler(struct sock *sk);
 void tcp_delack_timer_handler(struct sock *sk);
 int tcp_ioctl(struct sock *sk, int cmd, int *karg);
-int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
+enum skb_drop_reason tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
 void tcp_rcv_established(struct sock *sk, struct sk_buff *skb);
 void tcp_rcv_space_adjust(struct sock *sk);
 int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 83308cca1610..5d874817a78d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6619,7 +6619,8 @@ static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
  *	address independent.
  */
 
-int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
+enum skb_drop_reason
+tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct inet_connection_sock *icsk = inet_csk(sk);
@@ -6635,7 +6636,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 
 	case TCP_LISTEN:
 		if (th->ack)
-			return 1;
+			return SKB_DROP_REASON_TCP_FLAGS;
 
 		if (th->rst) {
 			SKB_DR_SET(reason, TCP_RESET);
@@ -6704,8 +6705,12 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 				  FLAG_NO_CHALLENGE_ACK);
 
 	if ((int)reason <= 0) {
-		if (sk->sk_state == TCP_SYN_RECV)
-			return 1;	/* send one RST */
+		if (sk->sk_state == TCP_SYN_RECV) {
+			/* send one RST */
+			if (!reason)
+				return SKB_DROP_REASON_TCP_OLD_ACK;
+			return -reason;
+		}
 		/* accept old ack during closing */
 		if ((int)reason < 0) {
 			tcp_send_challenge_ack(sk);
@@ -6781,7 +6786,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 		if (READ_ONCE(tp->linger2) < 0) {
 			tcp_done(sk);
 			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
-			return 1;
+			return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
 		}
 		if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
 		    after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt)) {
@@ -6790,7 +6795,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 				tcp_fastopen_active_disable(sk);
 			tcp_done(sk);
 			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
-			return 1;
+			return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
 		}
 
 		tmo = tcp_fin_time(sk);
@@ -6855,7 +6860,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
 			    after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt)) {
 				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
 				tcp_reset(sk, skb);
-				return 1;
+				return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
 			}
 		}
 		fallthrough;
-- 
2.37.3

[PATCH net-next v9 09/10] tcp: make the dropreason really work when calling tcp_rcv_state_process()

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Update three callers including both ipv4 and ipv6 and let the dropreason
mechanism work in reality.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
1. add reviewed-by tag (David)

v8
Link: https://lore.kernel.org/netdev/CANn89i+Uikp=NvB7SVQpYnX-2FqJrH3hWw3sV0XpVcC55MiNUg@mail.gmail.com/
1. add reviewed-by tag (Eric)
---
 include/net/tcp.h        | 2 +-
 net/ipv4/tcp_ipv4.c      | 3 ++-
 net/ipv4/tcp_minisocks.c | 9 +++++----
 net/ipv6/tcp_ipv6.c      | 3 ++-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index e5af9a5b411b..1d9b2a766b5e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -396,7 +396,7 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw,
 struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
 			   struct request_sock *req, bool fastopen,
 			   bool *lost_race);
-int tcp_child_process(struct sock *parent, struct sock *child,
+enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
 		      struct sk_buff *skb);
 void tcp_enter_loss(struct sock *sk);
 void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost, int flag);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0a944e109088..c79e25549972 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1926,7 +1926,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 	} else
 		sock_rps_save_rxhash(sk, skb);
 
-	if (tcp_rcv_state_process(sk, skb)) {
+	reason = tcp_rcv_state_process(sk, skb);
+	if (reason) {
 		rsk = sk;
 		goto reset;
 	}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 9e85f2a0bddd..08d5b48540ea 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -911,11 +911,12 @@ EXPORT_SYMBOL(tcp_check_req);
  * be created.
  */
 
-int tcp_child_process(struct sock *parent, struct sock *child,
+enum skb_drop_reason
+tcp_child_process(struct sock *parent, struct sock *child,
 		      struct sk_buff *skb)
 	__releases(&((child)->sk_lock.slock))
 {
-	int ret = 0;
+	enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
 	int state = child->sk_state;
 
 	/* record sk_napi_id and sk_rx_queue_mapping of child. */
@@ -923,7 +924,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
 
 	tcp_segs_in(tcp_sk(child), skb);
 	if (!sock_owned_by_user(child)) {
-		ret = tcp_rcv_state_process(child, skb);
+		reason = tcp_rcv_state_process(child, skb);
 		/* Wakeup parent, send SIGIO */
 		if (state == TCP_SYN_RECV && child->sk_state != state)
 			parent->sk_data_ready(parent);
@@ -937,6 +938,6 @@ int tcp_child_process(struct sock *parent, struct sock *child,
 
 	bh_unlock_sock(child);
 	sock_put(child);
-	return ret;
+	return reason;
 }
 EXPORT_SYMBOL(tcp_child_process);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 0c180bb8187f..4f8464e04b7f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1663,7 +1663,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	} else
 		sock_rps_save_rxhash(sk, skb);
 
-	if (tcp_rcv_state_process(sk, skb))
+	reason = tcp_rcv_state_process(sk, skb);
+	if (reason)
 		goto reset;
 	if (opt_skb)
 		goto ipv6_pktoptions;
-- 
2.37.3

[PATCH net-next v9 10/10] tcp: make dropreason in tcp_child_process() work

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

It's time to let it work right now. We've already prepared for this:)

Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
--
v9
Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
Link: https://lore.kernel.org/netdev/CANn89iKE2vYz_6sYd=u3HbqdgiU0BWhdMY9-ivs0Rcht+X+Rfg@mail.gmail.com/
1. add reviewed-by tag (David)
2. add reviewed-by tag (Eric)

v8
Link: https://lore.kernel.org/netdev/CANn89i+huvL_Zidru_sNHbjwgM7==-q49+mgJq7vZPRgH6DgKg@mail.gmail.com/
Link: https://lore.kernel.org/netdev/CANn89iKmaZZSnk5+CCtSH43jeUgRWNQPV4cjc0vpWNT7nHnQQg@mail.gmail.com/
1. squash v7 patch [11/11] into the current patch.
2. refine the rcv codes. (Eric)

v7
Link: https://lore.kernel.org/all/20240219043815.98410-1-kuniyu@amazon.com/
1. adjust the related part of code only since patch [04/11] is changed.
---
 net/ipv4/tcp_ipv4.c | 12 +++++++-----
 net/ipv6/tcp_ipv6.c | 16 ++++++++++------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c79e25549972..a22ee5838751 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1907,7 +1907,6 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 		return 0;
 	}
 
-	reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	if (tcp_checksum_complete(skb))
 		goto csum_err;
 
@@ -1917,7 +1916,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
 		if (!nsk)
 			return 0;
 		if (nsk != sk) {
-			if (tcp_child_process(sk, nsk, skb)) {
+			reason = tcp_child_process(sk, nsk, skb);
+			if (reason) {
 				rsk = nsk;
 				goto reset;
 			}
@@ -2276,10 +2276,12 @@ int tcp_v4_rcv(struct sk_buff *skb)
 		if (nsk == sk) {
 			reqsk_put(req);
 			tcp_v4_restore_cb(skb);
-		} else if (tcp_child_process(sk, nsk, skb)) {
-			tcp_v4_send_reset(nsk, skb);
-			goto discard_and_relse;
 		} else {
+			drop_reason = tcp_child_process(sk, nsk, skb);
+			if (drop_reason) {
+				tcp_v4_send_reset(nsk, skb);
+				goto discard_and_relse;
+			}
 			sock_put(sk);
 			return 0;
 		}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 4f8464e04b7f..f677f0fa5196 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1623,7 +1623,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	if (np->rxopt.all)
 		opt_skb = skb_clone_and_charge_r(skb, sk);
 
-	reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
 		struct dst_entry *dst;
 
@@ -1654,8 +1653,11 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		struct sock *nsk = tcp_v6_cookie_check(sk, skb);
 
 		if (nsk != sk) {
-			if (nsk && tcp_child_process(sk, nsk, skb))
-				goto reset;
+			if (nsk) {
+				reason = tcp_child_process(sk, nsk, skb);
+				if (reason)
+					goto reset;
+			}
 			if (opt_skb)
 				__kfree_skb(opt_skb);
 			return 0;
@@ -1854,10 +1856,12 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
 		if (nsk == sk) {
 			reqsk_put(req);
 			tcp_v6_restore_cb(skb);
-		} else if (tcp_child_process(sk, nsk, skb)) {
-			tcp_v6_send_reset(nsk, skb);
-			goto discard_and_relse;
 		} else {
+			drop_reason = tcp_child_process(sk, nsk, skb);
+			if (drop_reason) {
+				tcp_v6_send_reset(nsk, skb);
+				goto discard_and_relse;
+			}
 			sock_put(sk);
 			return 0;
 		}
-- 
2.37.3

Re: [PATCH net-next v9 01/10] tcp: add a dropreason definitions and prepare for cookie check

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:42 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Adding one drop reason to detect the condition of skb dropped
> because of hook points in cookie check and extending NO_SOCKET
> to consider another two cases can be used later.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89iJ3gLMn5psbzfVCOo2=v4nMn4m41wpr6svxyAmO4R1m6g@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> 
> v7
> Link: https://lore.kernel.org/all/20240219040630.94637-1-kuniyu@amazon.com/
> 1. nit: change "invalid" to "valid" (Kuniyuki)
> 2. add more description.
> 
> v6
> Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
> 1. Modify the description NO_SOCKET to extend other two kinds of invalid
> socket cases.
> What I think about it is we can use it as a general indicator for three kinds of
> sockets which are invalid/NULL, like what we did to TCP_FLAGS.
> Any better ideas/suggestions are welcome :)
> 
> v5
> Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/632c6fd4-e060-4b8e-a80e-5d545a6c6b6c@kernel.org/
> 1. Use SKB_DROP_REASON_IP_OUTNOROUTES instead of introducing a new one (Eric, David)
> 2. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
> 3. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
> 4. Reuse IP_OUTNOROUTES instead of INVALID_DST (Eric)
> 5. adjust the title and description.
> 
> v4
> Link: https://lore.kernel.org/netdev/20240212172302.3f95e454@kernel.org/
> 1. fix misspelled name in kdoc as Jakub said
> ---
>  include/net/dropreason-core.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> index 6d3a20163260..a871f061558d 100644
> --- a/include/net/dropreason-core.h
> +++ b/include/net/dropreason-core.h
> @@ -54,6 +54,7 @@
>  	FN(NEIGH_QUEUEFULL)		\
>  	FN(NEIGH_DEAD)			\
>  	FN(TC_EGRESS)			\
> +	FN(SECURITY_HOOK)		\
>  	FN(QDISC_DROP)			\
>  	FN(CPU_BACKLOG)			\
>  	FN(XDP)				\
> @@ -105,7 +106,13 @@ enum skb_drop_reason {
>  	SKB_CONSUMED,
>  	/** @SKB_DROP_REASON_NOT_SPECIFIED: drop reason is not specified */
>  	SKB_DROP_REASON_NOT_SPECIFIED,
> -	/** @SKB_DROP_REASON_NO_SOCKET: socket not found */
> +	/**
> +	 * @SKB_DROP_REASON_NO_SOCKET: no valid socket that can be used.
> +	 * Reason could be one of three cases:
> +	 * 1) no established/listening socket found during lookup process
> +	 * 2) no valid request socket during 3WHS process
> +	 * 3) no valid child socket during 3WHS process
> +	 */
>  	SKB_DROP_REASON_NO_SOCKET,
>  	/** @SKB_DROP_REASON_PKT_TOO_SMALL: packet size is too small */
>  	SKB_DROP_REASON_PKT_TOO_SMALL,
> @@ -271,6 +278,8 @@ enum skb_drop_reason {
>  	SKB_DROP_REASON_NEIGH_DEAD,
>  	/** @SKB_DROP_REASON_TC_EGRESS: dropped in TC egress HOOK */
>  	SKB_DROP_REASON_TC_EGRESS,
> +	/** @SKB_DROP_REASON_SECURITY_HOOK: dropped due to security HOOK */
> +	SKB_DROP_REASON_SECURITY_HOOK,
>  	/**
>  	 * @SKB_DROP_REASON_QDISC_DROP: dropped by qdisc when packet outputting (
>  	 * failed to enqueue to current qdisc)
> -- 
> 2.37.3

Re: [PATCH net-next v9 02/10] tcp: directly drop skb in cookie check for ipv4

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:43 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Only move the skb drop from tcp_v4_do_rcv() to cookie_v4_check() itself,
> no other changes made. It can help us refine the specific drop reasons
> later.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>
> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89i+foA-AW3KCNw232eCC5GDi_3O0JG-mpvyiQJYuxKxnRA@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> 
> v7
> Link: https://lore.kernel.org/all/20240219041350.95304-1-kuniyu@amazon.com/
> 1. add reviewed-by tag (Kuniyuki)
> ---
>  net/ipv4/syncookies.c | 4 ++++
>  net/ipv4/tcp_ipv4.c   | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index be88bf586ff9..38f331da6677 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -408,6 +408,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  	struct rtable *rt;
>  	__u8 rcv_wscale;
>  	int full_space;
> +	SKB_DR(reason);
>  
>  	if (!READ_ONCE(net->ipv4.sysctl_tcp_syncookies) ||
>  	    !th->ack || th->rst)
> @@ -477,10 +478,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  	 */
>  	if (ret)
>  		inet_sk(ret)->cork.fl.u.ip4 = fl4;
> +	else
> +		goto out_drop;

nit: It would be nice to use the same style with IPv6 change.

	if (!ret)
		goto out_drop;

	inet_sk(ret)->cork.fl.u.ip4 = fl4;

Otherwise looks good.

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


>  out:
>  	return ret;
>  out_free:
>  	reqsk_free(req);
>  out_drop:
> +	kfree_skb_reason(skb, reason);
>  	return NULL;
>  }
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 0c50c5a32b84..0a944e109088 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1915,7 +1915,7 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  		struct sock *nsk = tcp_v4_cookie_check(sk, skb);
>  
>  		if (!nsk)
> -			goto discard;
> +			return 0;
>  		if (nsk != sk) {
>  			if (tcp_child_process(sk, nsk, skb)) {
>  				rsk = nsk;
> -- 
> 2.37.3

Re: [PATCH net-next v9 02/10] tcp: directly drop skb in cookie check for ipv4

[Kuniyuki Iwashima]

From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Fri, 23 Feb 2024 11:17:36 -0800
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Fri, 23 Feb 2024 18:28:43 +0800
> > From: Jason Xing <kernelxing@tencent.com>
> > 
> > Only move the skb drop from tcp_v4_do_rcv() to cookie_v4_check() itself,
> > no other changes made. It can help us refine the specific drop reasons
> > later.
> > 
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: David Ahern <dsahern@kernel.org>
> > --
> > v9
> > Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> > 1. add reviewed-by tag (David)
> > 
> > v8
> > Link: https://lore.kernel.org/netdev/CANn89i+foA-AW3KCNw232eCC5GDi_3O0JG-mpvyiQJYuxKxnRA@mail.gmail.com/
> > 1. add reviewed-by tag (Eric)
> > 
> > v7
> > Link: https://lore.kernel.org/all/20240219041350.95304-1-kuniyu@amazon.com/
> > 1. add reviewed-by tag (Kuniyuki)
> > ---
> >  net/ipv4/syncookies.c | 4 ++++
> >  net/ipv4/tcp_ipv4.c   | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> > index be88bf586ff9..38f331da6677 100644
> > --- a/net/ipv4/syncookies.c
> > +++ b/net/ipv4/syncookies.c
> > @@ -408,6 +408,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >  	struct rtable *rt;
> >  	__u8 rcv_wscale;
> >  	int full_space;
> > +	SKB_DR(reason);
> >  
> >  	if (!READ_ONCE(net->ipv4.sysctl_tcp_syncookies) ||
> >  	    !th->ack || th->rst)
> > @@ -477,10 +478,13 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >  	 */
> >  	if (ret)
> >  		inet_sk(ret)->cork.fl.u.ip4 = fl4;
> > +	else
> > +		goto out_drop;
> 
> nit: It would be nice to use the same style with IPv6 change.
> 
> 	if (!ret)
> 		goto out_drop;
> 
> 	inet_sk(ret)->cork.fl.u.ip4 = fl4;

I just noticed you changed so in the next patch, so please
ignore my comment.

Re: [PATCH net-next v9 03/10] tcp: use drop reasons in cookie check for ipv4

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:44 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Now it's time to use the prepared definitions to refine this part.
> Four reasons used might enough for now, I think.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> Link: https://lore.kernel.org/netdev/CANn89iLOxJxmOCH1LxXf-YwRBzKXcjPRmgQeQ6A3bKRmW8=ksg@mail.gmail.com/
> 1. add reviewed-by tag (David)
> 2. add reviewed-by tag (Eric)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89iL-FH6jzoxhyKSMioj-zdBsHqNpR7YTGz8ytM=FZSGrug@mail.gmail.com/
> 1. refine the codes (Eric)
> 
> v6:
> Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
> 1. Not use NOMEM because of MPTCP (Kuniyuki). I chose to use NO_SOCKET as
> an indicator which can be used as three kinds of cases to tell people that we're
> unable to get a valid one. It's a relatively general reason like what we did
> to TCP_FLAGS.
> Any better ideas/suggestions are welcome :)
> 
> v5:
> Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/632c6fd4-e060-4b8e-a80e-5d545a6c6b6c@kernel.org/
> 1. Use SKB_DROP_REASON_IP_OUTNOROUTES instead of introducing a new one (Eric, David)
> 2. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
> 3. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
> ---
>  net/ipv4/syncookies.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> index 38f331da6677..7972ad3d7c73 100644
> --- a/net/ipv4/syncookies.c
> +++ b/net/ipv4/syncookies.c
> @@ -421,8 +421,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  		if (IS_ERR(req))
>  			goto out;
>  	}
> -	if (!req)
> +	if (!req) {
> +		SKB_DR_SET(reason, NO_SOCKET);
>  		goto out_drop;
> +	}
>  
>  	ireq = inet_rsk(req);
>  
> @@ -434,8 +436,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  	 */
>  	RCU_INIT_POINTER(ireq->ireq_opt, tcp_v4_save_options(net, skb));
>  
> -	if (security_inet_conn_request(sk, skb, req))
> +	if (security_inet_conn_request(sk, skb, req)) {
> +		SKB_DR_SET(reason, SECURITY_HOOK);
>  		goto out_free;
> +	}
>  
>  	tcp_ao_syncookie(sk, skb, req, AF_INET);
>  
> @@ -452,8 +456,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  			   ireq->ir_loc_addr, th->source, th->dest, sk->sk_uid);
>  	security_req_classify_flow(req, flowi4_to_flowi_common(&fl4));
>  	rt = ip_route_output_key(net, &fl4);
> -	if (IS_ERR(rt))
> +	if (IS_ERR(rt)) {
> +		SKB_DR_SET(reason, IP_OUTNOROUTES);
>  		goto out_free;
> +	}
>  
>  	/* Try to redo what tcp_v4_send_synack did. */
>  	req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
> @@ -476,10 +482,11 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
>  	/* ip_queue_xmit() depends on our flow being setup
>  	 * Normal sockets get it right from inet_csk_route_child_sock()
>  	 */
> -	if (ret)
> -		inet_sk(ret)->cork.fl.u.ip4 = fl4;
> -	else
> +	if (!ret) {
> +		SKB_DR_SET(reason, NO_SOCKET);
>  		goto out_drop;
> +	}
> +	inet_sk(ret)->cork.fl.u.ip4 = fl4;
>  out:
>  	return ret;
>  out_free:
> -- 
> 2.37.3

Re: [PATCH net-next v9 04/10] tcp: directly drop skb in cookie check for ipv6

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:45 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Like previous patch does, only moving skb drop logical code to
> cookie_v6_check() for later refinement.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89iL8M=1vFdZ1hBb4POuz+MKQ50fmBAewfbowEH3jpEtpZQ@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> 
> v7:
> Link: https://lore.kernel.org/all/20240219043815.98410-1-kuniyu@amazon.com/
> 1. refine the code (by removing redundant check), no functional changes. (Kuniyuki)
> 
> v6
> Link: https://lore.kernel.org/all/c987d2c79e4a4655166eb8eafef473384edb37fb.camel@redhat.com/
> Link: https://lore.kernel.org/all/CAL+tcoAgSjwsmFnDh_Gs9ZgMi-y5awtVx+4VhJPNRADjo7LLSA@mail.gmail.com/
> 1. take one case into consideration, behave like old days, or else it will trigger errors.
> 
> v5
> Link: https://lore.kernel.org/netdev/CANn89iKz7=1q7e8KY57Dn3ED7O=RCOfLxoHQKO4eNXnZa1OPWg@mail.gmail.com/
> 1. avoid duplication of these opt_skb tests/actions (Eric)
> ---
>  net/ipv6/syncookies.c | 4 ++++
>  net/ipv6/tcp_ipv6.c   | 5 +----
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index 6b9c69278819..ea0d9954a29f 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -177,6 +177,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>  	struct sock *ret = sk;
>  	__u8 rcv_wscale;
>  	int full_space;
> +	SKB_DR(reason);
>  
>  	if (!READ_ONCE(net->ipv4.sysctl_tcp_syncookies) ||
>  	    !th->ack || th->rst)
> @@ -256,10 +257,13 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>  	ireq->ecn_ok &= cookie_ecn_ok(net, dst);
>  
>  	ret = tcp_get_cookie_sock(sk, skb, req, dst);
> +	if (!ret)
> +		goto out_drop;
>  out:
>  	return ret;
>  out_free:
>  	reqsk_free(req);
>  out_drop:
> +	kfree_skb_reason(skb, reason);
>  	return NULL;
>  }
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 57b25b1fc9d9..0c180bb8187f 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1653,11 +1653,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	if (sk->sk_state == TCP_LISTEN) {
>  		struct sock *nsk = tcp_v6_cookie_check(sk, skb);
>  
> -		if (!nsk)
> -			goto discard;
> -
>  		if (nsk != sk) {
> -			if (tcp_child_process(sk, nsk, skb))
> +			if (nsk && tcp_child_process(sk, nsk, skb))
>  				goto reset;
>  			if (opt_skb)
>  				__kfree_skb(opt_skb);
> -- 
> 2.37.3

Re: [PATCH net-next v9 05/10] tcp: use drop reasons in cookie check for ipv6

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:46 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Like what I did to ipv4 mode, refine this part: adding more drop
> reasons for better tracing.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89i+b+bYqX0aVv9KtSm=nLmEQznamZmqaOzfqtJm_ux9JBw@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> 
> v6:
> Link: https://lore.kernel.org/netdev/20240215210922.19969-1-kuniyu@amazon.com/
> 1. Not use NOMEM because of MPTCP (Kuniyuki). I chose to use NO_SOCKET as
> an indicator which can be used as three kinds of cases to tell people that we're
> unable to get a valid one. It's a relatively general reason like what we did
> to TCP_FLAGS.
> 
> v5:
> Link: https://lore.kernel.org/netdev/CANn89i+iELpsoea6+C-08m6+=JkneEEM=nAj-28eNtcOCkwQjw@mail.gmail.com/
> 1. Reuse SKB_DROP_REASON_NOMEM to handle failure of request socket allocation (Eric)
> 2. Reuse NO_SOCKET instead of introducing COOKIE_NOCHILD
> 3. Reuse IP_OUTNOROUTES instead of INVALID_DST (Eric)
> ---
>  net/ipv6/syncookies.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
> index ea0d9954a29f..8bad0a44a0a6 100644
> --- a/net/ipv6/syncookies.c
> +++ b/net/ipv6/syncookies.c
> @@ -190,16 +190,20 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>  		if (IS_ERR(req))
>  			goto out;
>  	}
> -	if (!req)
> +	if (!req) {
> +		SKB_DR_SET(reason, NO_SOCKET);
>  		goto out_drop;
> +	}
>  
>  	ireq = inet_rsk(req);
>  
>  	ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
>  	ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
>  
> -	if (security_inet_conn_request(sk, skb, req))
> +	if (security_inet_conn_request(sk, skb, req)) {
> +		SKB_DR_SET(reason, SECURITY_HOOK);
>  		goto out_free;
> +	}
>  
>  	if (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
>  	    np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
> @@ -236,8 +240,10 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>  		security_req_classify_flow(req, flowi6_to_flowi_common(&fl6));
>  
>  		dst = ip6_dst_lookup_flow(net, sk, &fl6, final_p);
> -		if (IS_ERR(dst))
> +		if (IS_ERR(dst)) {
> +			SKB_DR_SET(reason, IP_OUTNOROUTES);
>  			goto out_free;
> +		}
>  	}
>  
>  	req->rsk_window_clamp = tp->window_clamp ? :dst_metric(dst, RTAX_WINDOW);
> @@ -257,8 +263,10 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
>  	ireq->ecn_ok &= cookie_ecn_ok(net, dst);
>  
>  	ret = tcp_get_cookie_sock(sk, skb, req, dst);
> -	if (!ret)
> +	if (!ret) {
> +		SKB_DR_SET(reason, NO_SOCKET);
>  		goto out_drop;
> +	}
>  out:
>  	return ret;
>  out_free:
> -- 
> 2.37.3

Re: [PATCH net-next v9 06/10] tcp: introduce dropreasons in receive path

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:47 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Soon later patches can use these relatively more accurate
> reasons to recognise and find out the cause.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

one nit below.

> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> Link: https://lore.kernel.org/netdev/CANn89i+j55o_1B2SV56n=u=NHukmN_CoRib4VBzpUBVcKRjAMw@mail.gmail.com/
> 1. add reviewed-by tag (David)
> 2. add reviewed-by tag (Eric)
> 
> v7
> Link: https://lore.kernel.org/all/20240219044744.99367-1-kuniyu@amazon.com/
> 1. nit: nit: s/. because of/ because/ (Kuniyuki)
> 
> v5:
> Link: https://lore.kernel.org/netdev/3a495358-4c47-4a9f-b116-5f9c8b44e5ab@kernel.org/
> 1. Use new name (TCP_ABORT_ON_DATA) for readability (David)
> 2. change the title of this patch
> ---
>  include/net/dropreason-core.h | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> index a871f061558d..af7c7146219d 100644
> --- a/include/net/dropreason-core.h
> +++ b/include/net/dropreason-core.h
> @@ -30,6 +30,7 @@
>  	FN(TCP_AOFAILURE)		\
>  	FN(SOCKET_BACKLOG)		\
>  	FN(TCP_FLAGS)			\
> +	FN(TCP_ABORT_ON_DATA)	\

One more trailing tab ?


>  	FN(TCP_ZEROWINDOW)		\
>  	FN(TCP_OLD_DATA)		\
>  	FN(TCP_OVERWINDOW)		\
> @@ -37,6 +38,7 @@
>  	FN(TCP_RFC7323_PAWS)		\
>  	FN(TCP_OLD_SEQUENCE)		\
>  	FN(TCP_INVALID_SEQUENCE)	\
> +	FN(TCP_INVALID_ACK_SEQUENCE)	\
>  	FN(TCP_RESET)			\
>  	FN(TCP_INVALID_SYN)		\
>  	FN(TCP_CLOSE)			\
> @@ -204,6 +206,11 @@ enum skb_drop_reason {
>  	SKB_DROP_REASON_SOCKET_BACKLOG,
>  	/** @SKB_DROP_REASON_TCP_FLAGS: TCP flags invalid */
>  	SKB_DROP_REASON_TCP_FLAGS,
> +	/**
> +	 * @SKB_DROP_REASON_TCP_ABORT_ON_DATA: abort on data, corresponding to
> +	 * LINUX_MIB_TCPABORTONDATA
> +	 */
> +	SKB_DROP_REASON_TCP_ABORT_ON_DATA,
>  	/**
>  	 * @SKB_DROP_REASON_TCP_ZEROWINDOW: TCP receive window size is zero,
>  	 * see LINUX_MIB_TCPZEROWINDOWDROP
> @@ -228,13 +235,19 @@ enum skb_drop_reason {
>  	SKB_DROP_REASON_TCP_OFOMERGE,
>  	/**
>  	 * @SKB_DROP_REASON_TCP_RFC7323_PAWS: PAWS check, corresponding to
> -	 * LINUX_MIB_PAWSESTABREJECTED
> +	 * LINUX_MIB_PAWSESTABREJECTED, LINUX_MIB_PAWSACTIVEREJECTED
>  	 */
>  	SKB_DROP_REASON_TCP_RFC7323_PAWS,
>  	/** @SKB_DROP_REASON_TCP_OLD_SEQUENCE: Old SEQ field (duplicate packet) */
>  	SKB_DROP_REASON_TCP_OLD_SEQUENCE,
>  	/** @SKB_DROP_REASON_TCP_INVALID_SEQUENCE: Not acceptable SEQ field */
>  	SKB_DROP_REASON_TCP_INVALID_SEQUENCE,
> +	/**
> +	 * @SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE: Not acceptable ACK SEQ
> +	 * field because ack sequence is not in the window between snd_una
> +	 * and snd_nxt
> +	 */
> +	SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE,
>  	/** @SKB_DROP_REASON_TCP_RESET: Invalid RST packet */
>  	SKB_DROP_REASON_TCP_RESET,
>  	/**
> -- 
> 2.37.3

Re: [PATCH net-next v9 07/10] tcp: add more specific possible drop reasons in tcp_rcv_synsent_state_process()

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:48 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> This patch does two things:
> 1) add two more new reasons
> 2) only change the return value(1) to various drop reason values
> for the future use
> 
> For now, we still cannot trace those two reasons. We'll implement the full
> function in the subsequent patch in this series.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89i+EF77F5ZJbbkiDQgwgAqSKWtD3djUF807zQ=AswGvosQ@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> ---
>  net/ipv4/tcp_input.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 74c03f0a6c0c..83308cca1610 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6361,6 +6361,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>  				inet_csk_reset_xmit_timer(sk,
>  						ICSK_TIME_RETRANS,
>  						TCP_TIMEOUT_MIN, TCP_RTO_MAX);
> +			SKB_DR_SET(reason, TCP_INVALID_ACK_SEQUENCE);
>  			goto reset_and_undo;
>  		}
>  
> @@ -6369,6 +6370,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>  			     tcp_time_stamp_ts(tp))) {
>  			NET_INC_STATS(sock_net(sk),
>  					LINUX_MIB_PAWSACTIVEREJECTED);
> +			SKB_DR_SET(reason, TCP_RFC7323_PAWS);
>  			goto reset_and_undo;
>  		}
>  
> @@ -6572,7 +6574,8 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>  reset_and_undo:
>  	tcp_clear_options(&tp->rx_opt);
>  	tp->rx_opt.mss_clamp = saved_clamp;
> -	return 1;
> +	/* we can reuse/return @reason to its caller to handle the exception */
> +	return reason;
>  }
>  
>  static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
> -- 
> 2.37.3

Re: [PATCH net-next v9 08/10] tcp: add dropreasons in tcp_rcv_state_process()

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:49 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> In this patch, I equipped this function with more dropreasons, but
> it still doesn't work yet, which I will do later.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/CAL+tcoCbsbM=HyXRqs2+QVrY8FSKmqYC47m87Axiyk1wk4omwQ@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. nit: remove unnecessary else (David)
> 2. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89iJJ9XTVeC=qbSNUnOhQMAsfBfouc9qUJY7MxgQtYGmB3Q@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> 
> v5:
> Link: https://lore.kernel.org/netdev/3a495358-4c47-4a9f-b116-5f9c8b44e5ab@kernel.org/
> 1. Use new name (TCP_ABORT_ON_DATA) for readability (David)
> ---
>  include/net/tcp.h    |  2 +-
>  net/ipv4/tcp_input.c | 19 ++++++++++++-------
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 58e65af74ad1..e5af9a5b411b 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -348,7 +348,7 @@ void tcp_wfree(struct sk_buff *skb);
>  void tcp_write_timer_handler(struct sock *sk);
>  void tcp_delack_timer_handler(struct sock *sk);
>  int tcp_ioctl(struct sock *sk, int cmd, int *karg);
> -int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
> +enum skb_drop_reason tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb);
>  void tcp_rcv_established(struct sock *sk, struct sk_buff *skb);
>  void tcp_rcv_space_adjust(struct sock *sk);
>  int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp);
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 83308cca1610..5d874817a78d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6619,7 +6619,8 @@ static void tcp_rcv_synrecv_state_fastopen(struct sock *sk)
>   *	address independent.
>   */
>  
> -int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
> +enum skb_drop_reason
> +tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct inet_connection_sock *icsk = inet_csk(sk);
> @@ -6635,7 +6636,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  
>  	case TCP_LISTEN:
>  		if (th->ack)
> -			return 1;
> +			return SKB_DROP_REASON_TCP_FLAGS;
>  
>  		if (th->rst) {
>  			SKB_DR_SET(reason, TCP_RESET);
> @@ -6704,8 +6705,12 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  				  FLAG_NO_CHALLENGE_ACK);
>  
>  	if ((int)reason <= 0) {
> -		if (sk->sk_state == TCP_SYN_RECV)
> -			return 1;	/* send one RST */
> +		if (sk->sk_state == TCP_SYN_RECV) {
> +			/* send one RST */
> +			if (!reason)
> +				return SKB_DROP_REASON_TCP_OLD_ACK;
> +			return -reason;
> +		}
>  		/* accept old ack during closing */
>  		if ((int)reason < 0) {
>  			tcp_send_challenge_ack(sk);
> @@ -6781,7 +6786,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  		if (READ_ONCE(tp->linger2) < 0) {
>  			tcp_done(sk);
>  			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> -			return 1;
> +			return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
>  		}
>  		if (TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq &&
>  		    after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt)) {
> @@ -6790,7 +6795,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  				tcp_fastopen_active_disable(sk);
>  			tcp_done(sk);
>  			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
> -			return 1;
> +			return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
>  		}
>  
>  		tmo = tcp_fin_time(sk);
> @@ -6855,7 +6860,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>  			    after(TCP_SKB_CB(skb)->end_seq - th->fin, tp->rcv_nxt)) {
>  				NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPABORTONDATA);
>  				tcp_reset(sk, skb);
> -				return 1;
> +				return SKB_DROP_REASON_TCP_ABORT_ON_DATA;
>  			}
>  		}
>  		fallthrough;
> -- 
> 2.37.3

Re: [PATCH net-next v9 09/10] tcp: make the dropreason really work when calling tcp_rcv_state_process()

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:50 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> Update three callers including both ipv4 and ipv6 and let the dropreason
> mechanism work in reality.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>

two nits below.


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> 1. add reviewed-by tag (David)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89i+Uikp=NvB7SVQpYnX-2FqJrH3hWw3sV0XpVcC55MiNUg@mail.gmail.com/
> 1. add reviewed-by tag (Eric)
> ---
>  include/net/tcp.h        | 2 +-
>  net/ipv4/tcp_ipv4.c      | 3 ++-
>  net/ipv4/tcp_minisocks.c | 9 +++++----
>  net/ipv6/tcp_ipv6.c      | 3 ++-
>  4 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index e5af9a5b411b..1d9b2a766b5e 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -396,7 +396,7 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw,
>  struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>  			   struct request_sock *req, bool fastopen,
>  			   bool *lost_race);
> -int tcp_child_process(struct sock *parent, struct sock *child,
> +enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
>  		      struct sk_buff *skb);

Please fix indentation here,


>  void tcp_enter_loss(struct sock *sk);
>  void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost, int flag);
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 0a944e109088..c79e25549972 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1926,7 +1926,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	} else
>  		sock_rps_save_rxhash(sk, skb);
>  
> -	if (tcp_rcv_state_process(sk, skb)) {
> +	reason = tcp_rcv_state_process(sk, skb);
> +	if (reason) {
>  		rsk = sk;
>  		goto reset;
>  	}
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 9e85f2a0bddd..08d5b48540ea 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -911,11 +911,12 @@ EXPORT_SYMBOL(tcp_check_req);
>   * be created.
>   */
>  
> -int tcp_child_process(struct sock *parent, struct sock *child,
> +enum skb_drop_reason
> +tcp_child_process(struct sock *parent, struct sock *child,
>  		      struct sk_buff *skb)

and here.


>  	__releases(&((child)->sk_lock.slock))
>  {
> -	int ret = 0;
> +	enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
>  	int state = child->sk_state;
>  
>  	/* record sk_napi_id and sk_rx_queue_mapping of child. */
> @@ -923,7 +924,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
>  
>  	tcp_segs_in(tcp_sk(child), skb);
>  	if (!sock_owned_by_user(child)) {
> -		ret = tcp_rcv_state_process(child, skb);
> +		reason = tcp_rcv_state_process(child, skb);
>  		/* Wakeup parent, send SIGIO */
>  		if (state == TCP_SYN_RECV && child->sk_state != state)
>  			parent->sk_data_ready(parent);
> @@ -937,6 +938,6 @@ int tcp_child_process(struct sock *parent, struct sock *child,
>  
>  	bh_unlock_sock(child);
>  	sock_put(child);
> -	return ret;
> +	return reason;
>  }
>  EXPORT_SYMBOL(tcp_child_process);
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 0c180bb8187f..4f8464e04b7f 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1663,7 +1663,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	} else
>  		sock_rps_save_rxhash(sk, skb);
>  
> -	if (tcp_rcv_state_process(sk, skb))
> +	reason = tcp_rcv_state_process(sk, skb);
> +	if (reason)
>  		goto reset;
>  	if (opt_skb)
>  		goto ipv6_pktoptions;
> -- 
> 2.37.3
> 

Re: [PATCH net-next v9 10/10] tcp: make dropreason in tcp_child_process() work

[Kuniyuki Iwashima]

From: Jason Xing <kerneljasonxing@gmail.com>
Date: Fri, 23 Feb 2024 18:28:51 +0800
> From: Jason Xing <kernelxing@tencent.com>
> 
> It's time to let it work right now. We've already prepared for this:)
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> --
> v9
> Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> Link: https://lore.kernel.org/netdev/CANn89iKE2vYz_6sYd=u3HbqdgiU0BWhdMY9-ivs0Rcht+X+Rfg@mail.gmail.com/
> 1. add reviewed-by tag (David)
> 2. add reviewed-by tag (Eric)
> 
> v8
> Link: https://lore.kernel.org/netdev/CANn89i+huvL_Zidru_sNHbjwgM7==-q49+mgJq7vZPRgH6DgKg@mail.gmail.com/
> Link: https://lore.kernel.org/netdev/CANn89iKmaZZSnk5+CCtSH43jeUgRWNQPV4cjc0vpWNT7nHnQQg@mail.gmail.com/
> 1. squash v7 patch [11/11] into the current patch.
> 2. refine the rcv codes. (Eric)
> 
> v7
> Link: https://lore.kernel.org/all/20240219043815.98410-1-kuniyu@amazon.com/
> 1. adjust the related part of code only since patch [04/11] is changed.
> ---
>  net/ipv4/tcp_ipv4.c | 12 +++++++-----
>  net/ipv6/tcp_ipv6.c | 16 ++++++++++------
>  2 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index c79e25549972..a22ee5838751 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1907,7 +1907,6 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  		return 0;
>  	}
>  
> -	reason = SKB_DROP_REASON_NOT_SPECIFIED;
>  	if (tcp_checksum_complete(skb))
>  		goto csum_err;
>  
> @@ -1917,7 +1916,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
>  		if (!nsk)
>  			return 0;
>  		if (nsk != sk) {
> -			if (tcp_child_process(sk, nsk, skb)) {
> +			reason = tcp_child_process(sk, nsk, skb);
> +			if (reason) {
>  				rsk = nsk;
>  				goto reset;
>  			}
> @@ -2276,10 +2276,12 @@ int tcp_v4_rcv(struct sk_buff *skb)
>  		if (nsk == sk) {
>  			reqsk_put(req);
>  			tcp_v4_restore_cb(skb);
> -		} else if (tcp_child_process(sk, nsk, skb)) {
> -			tcp_v4_send_reset(nsk, skb);
> -			goto discard_and_relse;
>  		} else {
> +			drop_reason = tcp_child_process(sk, nsk, skb);
> +			if (drop_reason) {
> +				tcp_v4_send_reset(nsk, skb);
> +				goto discard_and_relse;
> +			}
>  			sock_put(sk);
>  			return 0;
>  		}
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 4f8464e04b7f..f677f0fa5196 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1623,7 +1623,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	if (np->rxopt.all)
>  		opt_skb = skb_clone_and_charge_r(skb, sk);
>  
> -	reason = SKB_DROP_REASON_NOT_SPECIFIED;
>  	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
>  		struct dst_entry *dst;
>  
> @@ -1654,8 +1653,11 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  		struct sock *nsk = tcp_v6_cookie_check(sk, skb);
>  
>  		if (nsk != sk) {
> -			if (nsk && tcp_child_process(sk, nsk, skb))
> -				goto reset;
> +			if (nsk) {
> +				reason = tcp_child_process(sk, nsk, skb);
> +				if (reason)
> +					goto reset;
> +			}
>  			if (opt_skb)
>  				__kfree_skb(opt_skb);
>  			return 0;
> @@ -1854,10 +1856,12 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb)
>  		if (nsk == sk) {
>  			reqsk_put(req);
>  			tcp_v6_restore_cb(skb);
> -		} else if (tcp_child_process(sk, nsk, skb)) {
> -			tcp_v6_send_reset(nsk, skb);
> -			goto discard_and_relse;
>  		} else {
> +			drop_reason = tcp_child_process(sk, nsk, skb);
> +			if (drop_reason) {
> +				tcp_v6_send_reset(nsk, skb);
> +				goto discard_and_relse;
> +			}
>  			sock_put(sk);
>  			return 0;
>  		}
> -- 
> 2.37.3

Re: [PATCH net-next v9 09/10] tcp: make the dropreason really work when calling tcp_rcv_state_process()

[Jason Xing]

On Sat, Feb 24, 2024 at 3:45 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Fri, 23 Feb 2024 18:28:50 +0800
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Update three callers including both ipv4 and ipv6 and let the dropreason
> > mechanism work in reality.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: David Ahern <dsahern@kernel.org>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> two nits below.
>
>
> > --
> > v9
> > Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> > 1. add reviewed-by tag (David)
> >
> > v8
> > Link: https://lore.kernel.org/netdev/CANn89i+Uikp=NvB7SVQpYnX-2FqJrH3hWw3sV0XpVcC55MiNUg@mail.gmail.com/
> > 1. add reviewed-by tag (Eric)
> > ---
> >  include/net/tcp.h        | 2 +-
> >  net/ipv4/tcp_ipv4.c      | 3 ++-
> >  net/ipv4/tcp_minisocks.c | 9 +++++----
> >  net/ipv6/tcp_ipv6.c      | 3 ++-
> >  4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index e5af9a5b411b..1d9b2a766b5e 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -396,7 +396,7 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw,
> >  struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
> >                          struct request_sock *req, bool fastopen,
> >                          bool *lost_race);
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb);
>
> Please fix indentation here,

Thanks for the check. I run the checkpatch locally everytime and it
only says it is just 'check' not 'warning' or even 'error'.

Have no idea whether I should 'fix' it.

>
>
> >  void tcp_enter_loss(struct sock *sk);
> >  void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost, int flag);
> > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > index 0a944e109088..c79e25549972 100644
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1926,7 +1926,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb)) {
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason) {
> >               rsk = sk;
> >               goto reset;
> >       }
> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> > index 9e85f2a0bddd..08d5b48540ea 100644
> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -911,11 +911,12 @@ EXPORT_SYMBOL(tcp_check_req);
> >   * be created.
> >   */
> >
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason
> > +tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb)
>
> and here.
>
>
> >       __releases(&((child)->sk_lock.slock))
> >  {
> > -     int ret = 0;
> > +     enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
> >       int state = child->sk_state;
> >
> >       /* record sk_napi_id and sk_rx_queue_mapping of child. */
> > @@ -923,7 +924,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       tcp_segs_in(tcp_sk(child), skb);
> >       if (!sock_owned_by_user(child)) {
> > -             ret = tcp_rcv_state_process(child, skb);
> > +             reason = tcp_rcv_state_process(child, skb);
> >               /* Wakeup parent, send SIGIO */
> >               if (state == TCP_SYN_RECV && child->sk_state != state)
> >                       parent->sk_data_ready(parent);
> > @@ -937,6 +938,6 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       bh_unlock_sock(child);
> >       sock_put(child);
> > -     return ret;
> > +     return reason;
> >  }
> >  EXPORT_SYMBOL(tcp_child_process);
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 0c180bb8187f..4f8464e04b7f 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1663,7 +1663,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb))
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason)
> >               goto reset;
> >       if (opt_skb)
> >               goto ipv6_pktoptions;
> > --
> > 2.37.3
> >

Re: [PATCH net-next v9 06/10] tcp: introduce dropreasons in receive path

[Jason Xing]

On Sat, Feb 24, 2024 at 3:33 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Fri, 23 Feb 2024 18:28:47 +0800
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Soon later patches can use these relatively more accurate
> > reasons to recognise and find out the cause.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: David Ahern <dsahern@kernel.org>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> one nit below.
>
> > --
> > v9
> > Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> > Link: https://lore.kernel.org/netdev/CANn89i+j55o_1B2SV56n=u=NHukmN_CoRib4VBzpUBVcKRjAMw@mail.gmail.com/
> > 1. add reviewed-by tag (David)
> > 2. add reviewed-by tag (Eric)
> >
> > v7
> > Link: https://lore.kernel.org/all/20240219044744.99367-1-kuniyu@amazon.com/
> > 1. nit: nit: s/. because of/ because/ (Kuniyuki)
> >
> > v5:
> > Link: https://lore.kernel.org/netdev/3a495358-4c47-4a9f-b116-5f9c8b44e5ab@kernel.org/
> > 1. Use new name (TCP_ABORT_ON_DATA) for readability (David)
> > 2. change the title of this patch
> > ---
> >  include/net/dropreason-core.h | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
> > index a871f061558d..af7c7146219d 100644
> > --- a/include/net/dropreason-core.h
> > +++ b/include/net/dropreason-core.h
> > @@ -30,6 +30,7 @@
> >       FN(TCP_AOFAILURE)               \
> >       FN(SOCKET_BACKLOG)              \
> >       FN(TCP_FLAGS)                   \
> > +     FN(TCP_ABORT_ON_DATA)   \
>
> One more trailing tab ?

Well, I noticed that there are no specific/accurate rules about adding
a trailing tab prior to the current change. Some added more than one,
some not.

>
>
> >       FN(TCP_ZEROWINDOW)              \
> >       FN(TCP_OLD_DATA)                \
> >       FN(TCP_OVERWINDOW)              \
> > @@ -37,6 +38,7 @@
> >       FN(TCP_RFC7323_PAWS)            \
> >       FN(TCP_OLD_SEQUENCE)            \
> >       FN(TCP_INVALID_SEQUENCE)        \
> > +     FN(TCP_INVALID_ACK_SEQUENCE)    \
> >       FN(TCP_RESET)                   \
> >       FN(TCP_INVALID_SYN)             \
> >       FN(TCP_CLOSE)                   \
> > @@ -204,6 +206,11 @@ enum skb_drop_reason {
> >       SKB_DROP_REASON_SOCKET_BACKLOG,
> >       /** @SKB_DROP_REASON_TCP_FLAGS: TCP flags invalid */
> >       SKB_DROP_REASON_TCP_FLAGS,
> > +     /**
> > +      * @SKB_DROP_REASON_TCP_ABORT_ON_DATA: abort on data, corresponding to
> > +      * LINUX_MIB_TCPABORTONDATA
> > +      */
> > +     SKB_DROP_REASON_TCP_ABORT_ON_DATA,
> >       /**
> >        * @SKB_DROP_REASON_TCP_ZEROWINDOW: TCP receive window size is zero,
> >        * see LINUX_MIB_TCPZEROWINDOWDROP
> > @@ -228,13 +235,19 @@ enum skb_drop_reason {
> >       SKB_DROP_REASON_TCP_OFOMERGE,
> >       /**
> >        * @SKB_DROP_REASON_TCP_RFC7323_PAWS: PAWS check, corresponding to
> > -      * LINUX_MIB_PAWSESTABREJECTED
> > +      * LINUX_MIB_PAWSESTABREJECTED, LINUX_MIB_PAWSACTIVEREJECTED
> >        */
> >       SKB_DROP_REASON_TCP_RFC7323_PAWS,
> >       /** @SKB_DROP_REASON_TCP_OLD_SEQUENCE: Old SEQ field (duplicate packet) */
> >       SKB_DROP_REASON_TCP_OLD_SEQUENCE,
> >       /** @SKB_DROP_REASON_TCP_INVALID_SEQUENCE: Not acceptable SEQ field */
> >       SKB_DROP_REASON_TCP_INVALID_SEQUENCE,
> > +     /**
> > +      * @SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE: Not acceptable ACK SEQ
> > +      * field because ack sequence is not in the window between snd_una
> > +      * and snd_nxt
> > +      */
> > +     SKB_DROP_REASON_TCP_INVALID_ACK_SEQUENCE,
> >       /** @SKB_DROP_REASON_TCP_RESET: Invalid RST packet */
> >       SKB_DROP_REASON_TCP_RESET,
> >       /**
> > --
> > 2.37.3

Re: [PATCH net-next v9 09/10] tcp: make the dropreason really work when calling tcp_rcv_state_process()

[Jason Xing]

On Sat, Feb 24, 2024 at 3:45 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> From: Jason Xing <kerneljasonxing@gmail.com>
> Date: Fri, 23 Feb 2024 18:28:50 +0800
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Update three callers including both ipv4 and ipv6 and let the dropreason
> > mechanism work in reality.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > Reviewed-by: David Ahern <dsahern@kernel.org>
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> two nits below.

My bad :( Unfortunately, until now, I know why every time I have to
encounter such an indentation problem. I set ts = 4 in my config file
of VIM...

Will update it in the next version soon.

Thanks,
Jason

>
>
> > --
> > v9
> > Link: https://lore.kernel.org/netdev/c5640fc4-16dc-4058-97c6-bd84bae4fda1@kernel.org/
> > 1. add reviewed-by tag (David)
> >
> > v8
> > Link: https://lore.kernel.org/netdev/CANn89i+Uikp=NvB7SVQpYnX-2FqJrH3hWw3sV0XpVcC55MiNUg@mail.gmail.com/
> > 1. add reviewed-by tag (Eric)
> > ---
> >  include/net/tcp.h        | 2 +-
> >  net/ipv4/tcp_ipv4.c      | 3 ++-
> >  net/ipv4/tcp_minisocks.c | 9 +++++----
> >  net/ipv6/tcp_ipv6.c      | 3 ++-
> >  4 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index e5af9a5b411b..1d9b2a766b5e 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -396,7 +396,7 @@ enum tcp_tw_status tcp_timewait_state_process(struct inet_timewait_sock *tw,
> >  struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
> >                          struct request_sock *req, bool fastopen,
> >                          bool *lost_race);
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb);
>
> Please fix indentation here,
>
>
> >  void tcp_enter_loss(struct sock *sk);
> >  void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost, int flag);
> > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> > index 0a944e109088..c79e25549972 100644
> > --- a/net/ipv4/tcp_ipv4.c
> > +++ b/net/ipv4/tcp_ipv4.c
> > @@ -1926,7 +1926,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb)) {
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason) {
> >               rsk = sk;
> >               goto reset;
> >       }
> > diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> > index 9e85f2a0bddd..08d5b48540ea 100644
> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -911,11 +911,12 @@ EXPORT_SYMBOL(tcp_check_req);
> >   * be created.
> >   */
> >
> > -int tcp_child_process(struct sock *parent, struct sock *child,
> > +enum skb_drop_reason
> > +tcp_child_process(struct sock *parent, struct sock *child,
> >                     struct sk_buff *skb)
>
> and here.
>
>
> >       __releases(&((child)->sk_lock.slock))
> >  {
> > -     int ret = 0;
> > +     enum skb_drop_reason reason = SKB_NOT_DROPPED_YET;
> >       int state = child->sk_state;
> >
> >       /* record sk_napi_id and sk_rx_queue_mapping of child. */
> > @@ -923,7 +924,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       tcp_segs_in(tcp_sk(child), skb);
> >       if (!sock_owned_by_user(child)) {
> > -             ret = tcp_rcv_state_process(child, skb);
> > +             reason = tcp_rcv_state_process(child, skb);
> >               /* Wakeup parent, send SIGIO */
> >               if (state == TCP_SYN_RECV && child->sk_state != state)
> >                       parent->sk_data_ready(parent);
> > @@ -937,6 +938,6 @@ int tcp_child_process(struct sock *parent, struct sock *child,
> >
> >       bh_unlock_sock(child);
> >       sock_put(child);
> > -     return ret;
> > +     return reason;
> >  }
> >  EXPORT_SYMBOL(tcp_child_process);
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 0c180bb8187f..4f8464e04b7f 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1663,7 +1663,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
> >       } else
> >               sock_rps_save_rxhash(sk, skb);
> >
> > -     if (tcp_rcv_state_process(sk, skb))
> > +     reason = tcp_rcv_state_process(sk, skb);
> > +     if (reason)
> >               goto reset;
> >       if (opt_skb)
> >               goto ipv6_pktoptions;
> > --
> > 2.37.3
> >