netdev - Re: [RFC PATCH net-next 1/2] tcp: RTO Restart (RTOR)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1512071215580.17913@melkinpaasi.cs.helsinki.fi>
Date:	Mon, 7 Dec 2015 12:22:08 +0200 (EET)
From:	"Ilpo Järvinen" <ilpo.jarvinen@...sinki.fi>
To:	Per Hurtig <per.hurtig@....se>
cc:	David Miller <davem@...emloft.net>, edumazet@...gle.com,
	ncardwell@...gle.com, nanditad@...gle.com, tom@...bertland.com,
	ycheng@...gle.com, viro@...iv.linux.org.uk, fw@...len.de,
	mleitner@...hat.com, daniel@...earbox.net, willemb@...gle.com,
	pasi.sarolahti@....fi, stephen@...workplumber.org,
	Netdev <netdev@...r.kernel.org>, anna.brunstrom@....se,
	apetlund@...ula.no, michawe@....uio.no, mohammad.rajiullah@....se
Subject: Re: [RFC PATCH net-next 1/2] tcp: RTO Restart (RTOR)

On Mon, 7 Dec 2015, Per Hurtig wrote:

> This patch implements the RTO restart modification (RTOR). When data is
> ACKed, and the RTO timer is restarted, the time elapsed since the last
> outstanding segment was transmitted is subtracted from the calculated RTO
> value. This way, the RTO timer will expire after exactly RTO seconds, and
> not RTO + RTT [+ delACK] seconds.
> 
> This patch also implements a new sysctl (tcp_timer_restart) that is used
> to control the timer restart behavior.
> 
> Signed-off-by: Per Hurtig <per.hurtig@....se>
> ---
>  Documentation/networking/ip-sysctl.txt | 12 ++++++++++++
>  include/net/tcp.h                      |  4 ++++
>  net/ipv4/sysctl_net_ipv4.c             | 10 ++++++++++
>  net/ipv4/tcp_input.c                   | 24 ++++++++++++++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 2ea4c45..4094128 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -591,6 +591,18 @@ tcp_syn_retries - INTEGER
>  	with the current initial RTO of 1second. With this the final timeout
>  	for an active TCP connection attempt will happen after 127seconds.
>  
> +tcp_timer_restart - INTEGER
> +	Controls how the RTO and PTO timers are restarted (RTOR and TLPR).
> +	If set (per timer or combined) the timers are restarted with
> +	respect to the earliest outstanding segment, to not extend tail loss
> +	latency unnecessarily.
> +	Possible values:
> +		0 disables RTOR and TLPR.
> +		1 enables RTOR.
> +		2 enables TLPR.
> +		3 enables RTOR and TLPR.
> +	Default: 3
> +
>  tcp_timestamps - BOOLEAN
>  	Enable timestamps as defined in RFC1323.
>  

[...snip...]

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index fdd88c3..66e0425 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c

[...snip...]

>  /* Restart timer after forward progress on connection.
>   * RFC2988 recommends to restart timer to now+rto.
>   */
> @@ -3027,6 +3040,17 @@ void tcp_rearm_rto(struct sock *sk)
>  			 */
>  			if (delta > 0)
>  				rto = delta;
> +		} else if (icsk->icsk_pending == ICSK_TIME_RETRANS &&
> +			   (sysctl_tcp_timer_restart == 1 ||
> +			    sysctl_tcp_timer_restart == 3) &&

Use a bit operation here instead? Also I think that this sysctl would 
benefit from named constants rather than use of literals (similar 
comment applies to the other patch too).

> +			   (tp->packets_out + tcp_unsent_pkts(sk) <
> +			    TCP_RTORESTART_THRESH)) {
> +			struct sk_buff *skb = tcp_write_queue_head(sk);
> +			const u32 rto_time_stamp = tcp_skb_timestamp(skb);
> +			s32 delta = (s32)(tcp_time_stamp - rto_time_stamp);
> +
> +			if (delta > 0 && rto > delta)
> +				rto -= delta;
>  		}
>  		inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, rto,
>  					  TCP_RTO_MAX);


-- 
 i.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html