allow to configure tcp_retries1 and tcp_retries2 per TCP socket
Subsystems (3)
PATCH 1/1 allow to configure tcp_retries1 and tcp_retries2 per TCP socket
Hi,
The included patch adds support for setting the tcp_retries1 and tcp_retries2 options in a per socket fashion as it is done for the keepalive options TCP_KEEPIDLE, TCP_KEEPCNT and TCP_KEEPINTVL.
The issue I am trying to solve is that when a socket has data queued for delivering, the keepalive logic is not triggered. Instead, the tcp_retries1/2 parameters are used to determine how many delivering attempts should be performed before giving up.
The patch is very straight forward and just replicates similar functionality. There is one thing I am not completely sure and is if the new per-socket fields should go into inet_connection_sock instead of into tcp_sock.
Regards
include/linux/tcp.h +5 -0
@@ -105,6 +105,8 @@ enum {#define TCP_COOKIE_TRANSACTIONS 15/* TCP Cookie Transactions */ #define TCP_THIN_LINEAR_TIMEOUTS 16/* Use linear timeouts for thin streams*/ #define TCP_THIN_DUPACK 17/* Fast retrans. after 1 dupack */ +#define TCP_RETRIES1 18/* Number of attempts to retransmit packet normally */ +#define TCP_RETRIES2 19/* Number of attempts to retransmit packet */ /* for TCP_INFO socket option */ #define TCPI_OPT_TIMESTAMPS 1
@@ -424,6 +426,9 @@ struct tcp_sock {unsigned int keepalive_time;/* time before keep alive takes place */ unsigned int keepalive_intvl;/* time interval between keep alive probes */ +int retries1;/* number of attempts to retransmit packed normally */ +int retries2;/* number of attempts to retransmit packed */ +int linger2;/* Receiver side RTT estimation */
include/net/tcp.h +12 -0
@@ -138,6 +138,8 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);#define MAX_TCP_KEEPIDLE 32767#define MAX_TCP_KEEPINTVL 32767#define MAX_TCP_KEEPCNT 127 +#define MAX_TCP_RETRIES1 255 +#define MAX_TCP_RETRIES2 32767#define MAX_TCP_SYNCNT 127#define TCP_SYNQ_INTERVAL (HZ/5)/* Period of SYNACK timer */
@@ -1041,6 +1043,16 @@ static inline u32 keepalive_time_elapsed(const struct tcp_sock *tp) tcp_time_stamp - tp->rcv_tstamp); } +static inline int tcp_retries1_when (const struct tcp_sock *tp) +{ +return tp->retries1 ? : sysctl_tcp_retries1; +} + +static inline int tcp_retries2_when (const struct tcp_sock *tp) +{ +return tp->retries2 ? : sysctl_tcp_retries2; +} +static inline int tcp_fin_time (const struct sock *sk) {int fin_timeout =tcp_sk (sk)->linger2 ? : sysctl_tcp_fin_timeout;
net/ipv4/sysctl_net_ipv4.c +1 -1
@@ -23,7 +23,7 @@#include <net/inet_frag.h> static int zero; -static int tcp_retr1_max =255 ; +static int tcp_retr1_max =MAX_TCP_RETRIES1 ;static int ip_local_port_range_min[] = {1 ,1 };static int ip_local_port_range_max[] = {65535 ,65535 };
net/ipv4/tcp.c +18 -0
@@ -2319,6 +2319,18 @@ static int do_tcp_setsockopt(struct sock *sk, int level,else tp->keepalive_probes = val;break ; +case TCP_RETRIES1 : +if (val <1 || val >MAX_TCP_RETRIES1 ) + err = -EINVAL ; +else + tp->retries1 = val; +break ; +case TCP_RETRIES2 : +if (val <1 || val >MAX_TCP_RETRIES2 ) + err = -EINVAL ; +else + tp->retries2 = val; +break ;case TCP_SYNCNT :if (val <1 || val >MAX_TCP_SYNCNT ) err = -EINVAL ;
@@ -2511,6 +2523,12 @@ static int do_tcp_getsockopt(struct sock *sk, int level,case TCP_KEEPCNT : val =keepalive_probes (tp);break ; +case TCP_RETRIES1 : + val =tcp_retries1_when (tp); +break ; +case TCP_RETRIES2 : + val =tcp_retries2_when (tp); +break ;case TCP_SYNCNT : val = icsk->icsk_syn_retries ? : sysctl_tcp_syn_retries;break ;
net/ipv4/tcp_timer.c +5 -4
@@ -167,6 +167,7 @@ static bool retransmits_timed_out(struct sock *sk,static int tcp_write_timeout (struct sock *sk) {struct inet_connection_sock *icsk =inet_csk (sk); +struct tcp_sock *tp =tcp_sk (sk);int retry_until;bool do_reset;
@@ -175,14 +176,14 @@ static int tcp_write_timeout(struct sock *sk)dst_negative_advice (sk); retry_until = icsk->icsk_syn_retries ? : sysctl_tcp_syn_retries; }else { -if (retransmits_timed_out (sk, sysctl_tcp_retries1)) { +if (retransmits_timed_out (sk,tcp_retries1_when (tp))) {/* Black hole detection */ tcp_mtu_probing (icsk, sk);dst_negative_advice (sk); } - retry_until = sysctl_tcp_retries2; + retry_until =tcp_retries2_when (tp);if (sock_flag (sk,SOCK_DEAD )) {const int alive = (icsk->icsk_rto <TCP_RTO_MAX );
@@ -290,7 +291,7 @@ static void tcp_probe_timer(struct sock *sk)* with RFCs, only probe timer combines both retransmission timeout * and probe timeout in one bottle. --ANK */ - max_probes = sysctl_tcp_retries2; + max_probes =tcp_retries2_when (tp);if (sock_flag (sk,SOCK_DEAD )) {const int alive = ((icsk->icsk_rto << icsk->icsk_backoff) <TCP_RTO_MAX );
@@ -437,7 +438,7 @@ out_reset_timer: icsk->icsk_rto =min (icsk->icsk_rto <<1 ,TCP_RTO_MAX ); }inet_csk_reset_xmit_timer (sk,ICSK_TIME_RETRANS , icsk->icsk_rto,TCP_RTO_MAX ); -if (retransmits_timed_out (sk, sysctl_tcp_retries1 +1 )) +if (retransmits_timed_out (sk,tcp_retries1_when (tp) +1 ))__sk_dst_reset (sk); out:;
Quoted 8 lines · Salvador Fandino
The included patch adds support for setting the
tcp_retries1andtcp_retries2options in a per socket fashion as it is done for the keepalive optionsTCP_KEEPIDLE,TCP_KEEPCNTandTCP_KEEPINTVL.The issue I am trying to solve is that when a socket has data queued for delivering, the keepalive logic is not triggered. Instead, the tcp_retries1/2 parameters are used to determine how many delivering attempts should be performed before giving up.
And why exactly do you need new tunables to solve this?
Quoted 5 lines
The patch is very straight forward and just replicates similar functionality. There is one thing I am not completely sure and is if the new per-socket fields should go into
inet_connection_sockinstead of intotcp_sock.
tcp_sock is already quite big (>2k on 64bit)
IMHO any new fields in there need very good justification.
-Andi
Signature · 1 line
[email protected] -- Speaking for myself only.
Quoted 8 lines · Andi Kleen
Quoted 4 lines · Salvador Fandino
The included patch adds support for setting the
tcp_retries1andtcp_retries2options in a per socket fashion as it is done for the keepalive optionsTCP_KEEPIDLE,TCP_KEEPCNTandTCP_KEEPINTVL.
Quoted 7 lines
Quoted 5 lines
The issue I am trying to solve is that when a socket has data queued for delivering, the keepalive logic is not triggered. Instead, the tcp_retries1/2 parameters are used to determine how many delivering attempts should be performed before giving up.
And why exactly do you need new tunables to solve this?
How else could it be solved?
I can think of making the retransmission logic to also honor the keepalive settings, switching to sending packets every keepintvl seconds when the elapsed time goes over keepidle and abort after keepcnt.
Or, make retransmits_timed_out() also consider (keepidle+keepcnt*keepintvl) as a ceiling.
But frankly, I don't like any one of them. IMO, leaving alone backward compatibility issues, it would make more sense to do it the other way and change the keepalive logic to follow the same sending pattern used for data retransmissions, using keepidle, retries1 and retries2 as its parameters.
Well, another option would be to use keepcnt as retries2 when defined in tcp_sock. IMO it would make sense, but could be confusing for the user.
Quoted 9 lines
Quoted 5 lines
The patch is very straight forward and just replicates similar functionality. There is one thing I am not completely sure and is if the new per-socket fields should go into
inet_connection_sockinstead of intotcp_sock.
tcp_sockis already quite big (>2k on 64bit)IMHO any new fields in there need very good justification.
If this is a problem, there are some room for optimization in the inet_connection_sock and tcp_sock structures. For instance, keepalive_time and keepalive_intvl are limited to MAX_KEEPALIVE_TIME * HZ, that is 32767 * 1000 ==> 25 bits, so they would fit in a u32.
retries1 and retries2 fields also fit in u32 and actually, a per socket retries1 field is not absolutely required because the check against retries2 is always performed, so the impact of this patch on the structure size could be limited to 4 bytes.
- Salva