Skip to content

Commit

Permalink
tcp: fix tcp_disordered_ack() vs usec TS resolution
Browse files Browse the repository at this point in the history
After commit 9394630 ("tcp: change data receiver flowlabel after one dup")
we noticed an increase of TCPACKSkippedPAWS events.

Neal Cardwell tracked the issue to tcp_disordered_ack() assumption
about remote peer TS clock.

RFC 1323 & 7323 are suggesting the following:
  "timestamp clock frequency in the range 1 ms to 1 sec per tick
   between 1ms and 1sec."

This has to be adjusted for 1 MHz clock frequency.

This hints at reorders of SACK packets on send side,
this might deserve a future patch.
(skb->ooo_okay is always set for pure ACK packets)

Fixes: 614e831 ("tcp: add support for usec resolution in TCP TS values")
Co-developed-by: Neal Cardwell <[email protected]>
Signed-off-by: Neal Cardwell <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>
Cc: David Morley <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jakub Kicinski <[email protected]>
  • Loading branch information
Eric Dumazet authored and kuba-moo committed Dec 9, 2023
1 parent a45f1e4 commit 9c25aae
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions net/ipv4/tcp_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -4368,14 +4368,31 @@ EXPORT_SYMBOL(tcp_do_parse_auth_options);
* up to bandwidth of 18Gigabit/sec. 8) ]
*/

/* Estimates max number of increments of remote peer TSval in
* a replay window (based on our current RTO estimation).
*/
static u32 tcp_tsval_replay(const struct sock *sk)
{
/* If we use usec TS resolution,
* then expect the remote peer to use the same resolution.
*/
if (tcp_sk(sk)->tcp_usec_ts)
return inet_csk(sk)->icsk_rto * (USEC_PER_SEC / HZ);

/* RFC 7323 recommends a TSval clock between 1ms and 1sec.
* We know that some OS (including old linux) can use 1200 Hz.
*/
return inet_csk(sk)->icsk_rto * 1200 / HZ;
}

static int tcp_disordered_ack(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;
u32 ack = TCP_SKB_CB(skb)->ack_seq;

return (/* 1. Pure ACK with correct sequence number. */
return /* 1. Pure ACK with correct sequence number. */
(th->ack && seq == TCP_SKB_CB(skb)->end_seq && seq == tp->rcv_nxt) &&

/* 2. ... and duplicate ACK. */
Expand All @@ -4385,7 +4402,8 @@ static int tcp_disordered_ack(const struct sock *sk, const struct sk_buff *skb)
!tcp_may_update_window(tp, ack, seq, ntohs(th->window) << tp->rx_opt.snd_wscale) &&

/* 4. ... and sits in replay window. */
(s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) <= (inet_csk(sk)->icsk_rto * 1024) / HZ);
(s32)(tp->rx_opt.ts_recent - tp->rx_opt.rcv_tsval) <=
tcp_tsval_replay(sk);
}

static inline bool tcp_paws_discard(const struct sock *sk,
Expand Down

0 comments on commit 9c25aae

Please sign in to comment.