tcp: add drop_reason support to tcp_disordered_ack()
authorEric Dumazet <edumazet@google.com>
Mon, 13 Jan 2025 13:55:56 +0000 (13:55 +0000)
committerJakub Kicinski <kuba@kernel.org>
Tue, 14 Jan 2025 21:28:13 +0000 (13:28 -0800)
Following patch is adding a new drop_reason to tcp_validate_incoming().

Change tcp_disordered_ack() to not return a boolean anymore,
but a drop reason.

Change its name to tcp_disordered_ack_check()

Refactor tcp_validate_incoming() to ease the code
review of the following patch, and reduce indentation
level.

This patch is a refactor, with no functional change.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Link: https://patch.msgid.link/20250113135558.3180360-2-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/ipv4/tcp_input.c

index 4811727b8a02258ec6fa1fd129beecf7cbb0f90e..24966dd3e49f698e110f8601e098b65afdf0718a 100644 (file)
@@ -4450,34 +4450,38 @@ static u32 tcp_tsval_replay(const struct sock *sk)
        return inet_csk(sk)->icsk_rto * 1200 / HZ;
 }
 
-static int tcp_disordered_ack(const struct sock *sk, const struct sk_buff *skb)
+static enum skb_drop_reason tcp_disordered_ack_check(const struct sock *sk,
+                                                    const struct sk_buff *skb)
 {
        const struct tcp_sock *tp = tcp_sk(sk);
        const struct tcphdr *th = tcp_hdr(skb);
-       u32 seq = TCP_SKB_CB(skb)->seq;
+       SKB_DR_INIT(reason, TCP_RFC7323_PAWS);
        u32 ack = TCP_SKB_CB(skb)->ack_seq;
+       u32 seq = TCP_SKB_CB(skb)->seq;
 
-       return  /* 1. Pure ACK with correct sequence number. */
-               (th->ack && seq == TCP_SKB_CB(skb)->end_seq && seq == tp->rcv_nxt) &&
+       /* 1. Is this not a pure ACK ? */
+       if (!th->ack || seq != TCP_SKB_CB(skb)->end_seq)
+               return reason;
 
-               /* 2. ... and duplicate ACK. */
-               ack == tp->snd_una &&
+       /* 2. Is its sequence not the expected one ? */
+       if (seq != tp->rcv_nxt)
+               return reason;
 
-               /* 3. ... and does not update window. */
-               !tcp_may_update_window(tp, ack, seq, ntohs(th->window) << tp->rx_opt.snd_wscale) &&
+       /* 3. Is this not a duplicate ACK ? */
+       if (ack != tp->snd_una)
+               return reason;
 
-               /* 4. ... and sits in replay window. */
-               (s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) <=
-               tcp_tsval_replay(sk);
-}
+       /* 4. Is this updating the window ? */
+       if (tcp_may_update_window(tp, ack, seq, ntohs(th->window) <<
+                                               tp->rx_opt.snd_wscale))
+               return reason;
 
-static inline bool tcp_paws_discard(const struct sock *sk,
-                                  const struct sk_buff *skb)
-{
-       const struct tcp_sock *tp = tcp_sk(sk);
+       /* 5. Is this not in the replay window ? */
+       if ((s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) >
+           tcp_tsval_replay(sk))
+               return reason;
 
-       return !tcp_paws_check(&tp->rx_opt, TCP_PAWS_WINDOW) &&
-              !tcp_disordered_ack(sk, skb);
+       return 0;
 }
 
 /* Check segment sequence number for validity.
@@ -5949,23 +5953,28 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
        SKB_DR(reason);
 
        /* RFC1323: H1. Apply PAWS check first. */
-       if (tcp_fast_parse_options(sock_net(sk), skb, th, tp) &&
-           tp->rx_opt.saw_tstamp &&
-           tcp_paws_discard(sk, skb)) {
-               if (!th->rst) {
-                       if (unlikely(th->syn))
-                               goto syn_challenge;
-                       NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
-                       if (!tcp_oow_rate_limited(sock_net(sk), skb,
-                                                 LINUX_MIB_TCPACKSKIPPEDPAWS,
-                                                 &tp->last_oow_ack_time))
-                               tcp_send_dupack(sk, skb);
-                       SKB_DR_SET(reason, TCP_RFC7323_PAWS);
-                       goto discard;
-               }
-               /* Reset is accepted even if it did not pass PAWS. */
-       }
-
+       if (!tcp_fast_parse_options(sock_net(sk), skb, th, tp) ||
+           !tp->rx_opt.saw_tstamp ||
+           tcp_paws_check(&tp->rx_opt, TCP_PAWS_WINDOW))
+               goto step1;
+
+       reason = tcp_disordered_ack_check(sk, skb);
+       if (!reason)
+               goto step1;
+       /* Reset is accepted even if it did not pass PAWS. */
+       if (th->rst)
+               goto step1;
+       if (unlikely(th->syn))
+               goto syn_challenge;
+
+       NET_INC_STATS(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
+       if (!tcp_oow_rate_limited(sock_net(sk), skb,
+                                 LINUX_MIB_TCPACKSKIPPEDPAWS,
+                                 &tp->last_oow_ack_time))
+               tcp_send_dupack(sk, skb);
+       goto discard;
+
+step1:
        /* Step 1: check sequence number */
        reason = tcp_sequence(tp, TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq);
        if (reason) {