netdev - Re: [PATCH] tcp: Use BPF timeout setting for SYN ACK RTO

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b8700d59-d533-71ee-f8c3-b7f0906debc5@gmail.com>
Date:   Mon, 25 Oct 2021 09:54:13 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Akhmat Karakotov <hmukos@...dex-team.ru>, netdev@...r.kernel.org,
        Neal Cardwell <ncardwell@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>,
        Lawrence Brakmo <brakmo@...com>
Cc:     mitradir@...dex-team.ru, zeil@...dex-team.ru, brakmo@...com
Subject: Re: [PATCH] tcp: Use BPF timeout setting for SYN ACK RTO



On 10/25/21 5:12 AM, Akhmat Karakotov wrote:
> When setting RTO through BPF program, SYN ACK packets were unaffected and
> continued to use TCP_TIMEOUT_INIT constant. This patch makes SYN ACK
> retransmits use tcp_timeout_init() function instead.
> 

I do not think we want this, this will break existing BPF programs
that did not expect this risky change of behavior.

Please next time you send controversial TCP patches, add TCP experts/maintainers
in CC.

> Signed-off-by: Akhmat Karakotov <hmukos@...dex-team.ru>
> ---
>  net/ipv4/inet_connection_sock.c | 2 +-
>  net/ipv4/tcp_minisocks.c        | 4 ++--
>  net/ipv4/tcp_timer.c            | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 0d477c816309..41663d1ffd0a 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -870,7 +870,7 @@ static void reqsk_timer_handler(struct timer_list *t)
>  
>  		if (req->num_timeout++ == 0)
>  			atomic_dec(&queue->young);
> -		timeo = min(TCP_TIMEOUT_INIT << req->num_timeout, TCP_RTO_MAX);
> +		timeo = min(tcp_timeout_init((struct sock *)req) << req->num_timeout, TCP_RTO_MAX);
>  		mod_timer(&req->rsk_timer, jiffies + timeo);
>  
>  		if (!nreq)
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 0a4f3f16140a..8ddc3aa9e3a6 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -590,7 +590,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>  			 * it can be estimated (approximately)
>  			 * from another data.
>  			 */
> -			tmp_opt.ts_recent_stamp = ktime_get_seconds() - ((TCP_TIMEOUT_INIT/HZ)<<req->num_timeout);
> +			tmp_opt.ts_recent_stamp = ktime_get_seconds() - ((tcp_timeout_init((struct sock *)req)/HZ)<<req->num_timeout);
>  			paws_reject = tcp_paws_reject(&tmp_opt, th->rst);
>  		}
>  	}
> @@ -629,7 +629,7 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>  		    !inet_rtx_syn_ack(sk, req)) {
>  			unsigned long expires = jiffies;
>  
> -			expires += min(TCP_TIMEOUT_INIT << req->num_timeout,
> +			expires += min(tcp_timeout_init((struct sock *)req) << req->num_timeout,
>  				       TCP_RTO_MAX);
>  			if (!fastopen)
>  				mod_timer_pending(&req->rsk_timer, expires);
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index 20cf4a98c69d..0954e3685ad2 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -430,7 +430,7 @@ static void tcp_fastopen_synack_timer(struct sock *sk, struct request_sock *req)
>  	if (!tp->retrans_stamp)
>  		tp->retrans_stamp = tcp_time_stamp(tp);
>  	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
> -			  TCP_TIMEOUT_INIT << req->num_timeout, TCP_RTO_MAX);
> +			  tcp_timeout_init((struct sock *)req) << req->num_timeout, TCP_RTO_MAX);
>  }
>  
>  
>