Re: [PATCH net v3 1/2] tcp: exclude old ACKs from tcp fast path

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Documentation/networking/snmp_counter.rst contains a verbatim
  paste of the tcp_rcv_established() "fast path is disabled when:" list…

--

tcp: exclude old ACKs from tcp fast path

This changes the header prediction test in tcp_rcv_established() so that
segments whose ack_seq is below SND.UNA (in addition to those above
SND.NXT) are sent to the slow path, where tcp_ack() applies RFC5961
validation. A matching bullet is added to the comment listing the
conditions that disable the fast path.

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 0f60a1dbf9274..92bc60716f33d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6490,6 +6490,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
>   *	  or pure receivers (this means either the sequence number or the ack
>   *	  value must stay constant)
>   *	- Unexpected TCP option.
> + *	- ACK sequence number is outside [SND.UNA, SND.NXT].

[Severity: Low]
This isn't a bug, but should Documentation/networking/snmp_counter.rst be
updated together with this comment?

That document reproduces this exact list and says it is a copy of the
kernel comment:

  When kernel receives a TCP packet, it has two paths to handler the
  packet, one is fast path, another is slow path. The comment in kernel
  code provides a good explanation of them, I pasted them below::

    It is split into a fast path and a slow path. The fast path is
    disabled when:
    ...
    - Data is sent in both directions. The fast path only supports pure senders
      or pure receivers (this means either the sequence number or the ack
      value must stay constant)
    - Unexpected TCP option.

The pasted copy still ends at "- Unexpected TCP option." and has no
bullet for the new ACK window condition, so after this patch a reader of
the documentation would conclude that a segment carrying an ACK below
SND.UNA is still eligible for header prediction (and therefore still
counted in TcpExtTCPHPAcks / TcpExtTCPHPHits rather than being handled in
the slow path).

>   *
>   *	When these conditions are not satisfied it drops into a standard
>   *	receive procedure patterned after RFC793 to handle all cases.
> @@ -6539,7 +6540,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>  
>  	if ((tcp_flag_word(th) & TCP_HP_BITS) == tp->pred_flags &&
>  	    TCP_SKB_CB(skb)->seq == tp->rcv_nxt &&
> -	    !after(TCP_SKB_CB(skb)->ack_seq, tp->snd_nxt)) {
> +	    between(TCP_SKB_CB(skb)->ack_seq, tp->snd_una, tp->snd_nxt)) {
>  		int tcp_header_len = tp->tcp_header_len;
>  		s32 delta = 0;
>  		int flag = 0;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914090408.1435080-1-inbal.lipshtat%40mail.huji.ac.il