tcp: bound SYN-ACK timers to reqsk timeout range
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 tcp: bound SYN-ACK timers to reqsk timeout range
From: Zhiling Zou <[email protected]>
tcp_synack_retries supplies the SYN-ACK retry limit used by request socket timers. The same effective limit can also come from TCP_SYNCNT through icsk_syn_retries, while TCP_DEFER_ACCEPT can keep an ACKed request alive until rskq_defer_accept is reached.
The request socket timeout counter is incremented before it is used to compute the next timeout. tcp_reqsk_timeout() and the Fast Open SYN-ACK timer shift req->timeout by req->num_timeout. Excessive retry or defer-accept limits can therefore drive these timer paths into invalid shift counts before the request expires.
Limit tcp_synack_retries to the request socket timer range, clamp the effective retry and defer-accept limits in the regular request socket timer path, clamp the Fast Open retry limit, and make the request socket timeout helper saturate before shifting.
--- include/net/tcp.h | 19 +++++++++++++++---- net/ipv4/inet_connection_sock.c | 6 +++++- net/ipv4/sysctl_net_ipv4.c | 2 ++ net/ipv4/tcp_timer.c | 11 ++++++++--- 4 files changed, 30 insertions(+), 8 deletions(-)
include/net/tcp.h +15 -4
@@ -183,6 +183,7 @@ static_assert((1 << ATO_BITS) > TCP_DELACK_MAX);#define MAX_TCP_KEEPINTVL 32767#define MAX_TCP_KEEPCNT 127#define MAX_TCP_SYNCNT 127 +#define MAX_TCP_SYNACK_RETRIES 63/* Ensure that TCP PAWS checks are relaxed after ~2147 seconds * to avoid overflows. This assumes a clock smaller than 1 Mhz.
@@ -882,12 +883,22 @@ static inline u32 __tcp_set_rto(const struct tcp_sock *tp)return usecs_to_jiffies ((tp->srtt_us >>3 ) + tp->rttvar_us); } -static inline unsigned long tcp_reqsk_timeout (struct request_sock *req) +static inline unsigned long tcp_reqsk_timeout_sk (const struct sock *sk, +struct request_sock *req) { -u64 timeout = (u64 )req->timeout << req->num_timeout; +u64 timeout = req->timeout; +u32 rto_max =tcp_rto_max (sk); + +if (req->num_timeout >=BITS_PER_TYPE (u64) || + timeout >U64_MAX >> req->num_timeout) +return rto_max; + +return (unsigned long )min_t (u64, timeout << req->num_timeout, rto_max); +} -return (unsigned long )min_t (u64, timeout, -tcp_rto_max (req->rsk_listener)); +static inline unsigned long tcp_reqsk_timeout (struct request_sock *req) +{ +return tcp_reqsk_timeout_sk (req->rsk_listener, req); }u32 tcp_delack_max (const struct sock *sk);
net/ipv4/inet_connection_sock.c +5 -1
@@ -1056,6 +1056,8 @@ static void reqsk_timer_handler(struct timer_list *t) net =sock_net (sk_listener); max_syn_ack_retries =READ_ONCE (icsk->icsk_syn_retries) ? :READ_ONCE (net->ipv4.sysctl_tcp_synack_retries); + max_syn_ack_retries =min_t (int, max_syn_ack_retries, +MAX_TCP_SYNACK_RETRIES );/* Normally all the openreqs are young and become mature * (i.e. converted to established socket) for first timeout. * If synack was not acknowledged for 1 second, it means
@@ -1086,7 +1088,9 @@ static void reqsk_timer_handler(struct timer_list *t) } } -syn_ack_recalc (req, max_syn_ack_retries,READ_ONCE (queue->rskq_defer_accept), +syn_ack_recalc (req, max_syn_ack_retries, +min_t (u8,READ_ONCE (queue->rskq_defer_accept), +MAX_TCP_SYNACK_RETRIES ), &expire, &resend);tcp_syn_ack_timeout (req);
net/ipv4/sysctl_net_ipv4.c +2 -0
@@ -35,6 +35,7 @@ static int ip_ttl_min = 1;static int ip_ttl_max =255 ;static int tcp_syn_retries_min =1 ;static int tcp_syn_retries_max =MAX_TCP_SYNCNT ; +static int tcp_synack_retries_max =MAX_TCP_SYNACK_RETRIES ;static int tcp_syn_linear_timeouts_max =MAX_TCP_SYNCNT ;static unsigned long ip_ping_group_range_min[] = {0 ,0 };static unsigned long ip_ping_group_range_max[] = {GID_T_MAX ,GID_T_MAX };
@@ -1034,6 +1035,7 @@ static struct ctl_table ipv4_net_table[] = { .maxlen = sizeof(u8), .mode = 0644, .proc_handler = proc_dou8vec_minmax, + .extra2 = &tcp_synack_retries_max },#ifdef CONFIG_SYN_COOKIES {
net/ipv4/tcp_timer.c +8 -3
@@ -458,6 +458,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req) {struct inet_connection_sock *icsk =inet_csk (sk);struct tcp_sock *tp =tcp_sk (sk); +struct net *net =sock_net (sk);int max_retries;tcp_syn_ack_timeout (req);
@@ -465,8 +466,12 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)/* Add one more retry for fastopen. * Paired with WRITE_ONCE() in tcp_sock_set_syncnt() */ - max_retries =READ_ONCE (icsk->icsk_syn_retries) ? : -READ_ONCE (sock_net (sk)->ipv4.sysctl_tcp_synack_retries) +1 ; + max_retries =READ_ONCE (icsk->icsk_syn_retries); +if (!max_retries) { + max_retries =READ_ONCE (net->ipv4.sysctl_tcp_synack_retries); + max_retries++; + } + max_retries =min_t (int, max_retries,MAX_TCP_SYNACK_RETRIES );if (req->num_timeout >= max_retries) {tcp_write_err (sk);
@@ -488,7 +493,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)if (!tp->retrans_stamp) tp->retrans_stamp =tcp_time_stamp_ts (tp);tcp_reset_xmit_timer (sk,ICSK_TIME_RETRANS , - req->timeout << req->num_timeout, false); +tcp_reqsk_timeout_sk (sk, req), false); } static bool tcp_rtx_probe0_timed_out(const struct sock *sk,
Signature · 1 line
2.43.0
Quoted 3 lines · Ren Wei
From: Zhiling Zou <[email protected]>
...
net/ipv4/tcp_timer.c static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req) · line 473
Quoted 7 lines
net/ipv4/tcp_timer.c +5
-2- max_retries =READ_ONCE (icsk->icsk_syn_retries) ? : -READ_ONCE (sock_net (sk)->ipv4.sysctl_tcp_synack_retries) +1 ; + max_retries =READ_ONCE (icsk->icsk_syn_retries); +if (!max_retries) { + max_retries =READ_ONCE (net->ipv4.sysctl_tcp_synack_retries); + max_retries++; + }
Please do not change this part which looks good, let's avoid code churn.
Quoted 16 lines · Eric Dumazet
Quoted 3 lines · Ren Wei
From: Zhiling Zou <[email protected]>
...
net/ipv4/tcp_timer.c static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req) · line 473
Quoted 7 lines
net/ipv4/tcp_timer.c +5
-2- max_retries =READ_ONCE (icsk->icsk_syn_retries) ? : -READ_ONCE (sock_net (sk)->ipv4.sysctl_tcp_synack_retries) +1 ; + max_retries =READ_ONCE (icsk->icsk_syn_retries); +if (!max_retries) { + max_retries =READ_ONCE (net->ipv4.sysctl_tcp_synack_retries); + max_retries++; + }
Quoted 1 line
Please do not change this part which looks good, let's avoid code churn.
Thanks for the review.
I kept the existing max_retries calculation unchanged in v2 and only added the clamp there.