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

On Wed, Nov 03, 2021 at 12:31:46PM +0300, Akhmat Karakotov wrote:

quoted

On Nov 3, 2021, at 02:17, Martin KaFai Lau [off-list ref] wrote:

On Tue, Nov 02, 2021 at 03:06:31PM -0700, Eric Dumazet wrote:

quoted

On 11/2/21 11:32 AM, Akhmat Karakotov wrote:

quoted

When setting RTO through BPF program, some SYN ACK packets were unaffected
and continued to use TCP_TIMEOUT_INIT constant. This patch adds timeout
option to struct request_sock. Option is initialized with TCP_TIMEOUT_INIT
and is reassigned through BPF using tcp_timeout_init call. SYN ACK
retransmits now use newly added timeout option.

Signed-off-by: Akhmat Karakotov <redacted>
---
include/net/request_sock.h      | 2 ++
net/ipv4/inet_connection_sock.c | 2 +-
net/ipv4/tcp_input.c            | 8 +++++---
net/ipv4/tcp_minisocks.c        | 4 ++--
4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 29e41ff3ec93..144c39db9898 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h

@@ -70,6 +70,7 @@ struct request_sock {

	struct saved_syn		*saved_syn;
	u32				secid;
	u32				peer_secid;
+	u32				timeout;
};

static inline struct request_sock *inet_reqsk(const struct sock *sk)

@@ -104,6 +105,7 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener,

	sk_node_init(&req_to_sk(req)->sk_node);
	sk_tx_queue_clear(req_to_sk(req));
	req->saved_syn = NULL;
+	req->timeout = 0;
	req->num_timeout = 0;
	req->num_retrans = 0;
	req->sk = NULL;

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 0d477c816309..c43cc1f22092 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(req->timeout << req->num_timeout, TCP_RTO_MAX);

I wonder how much time it will take to syzbot to trigger an overflow here and
other parts.

(Not sure BPF_SOCK_OPS_TIMEOUT_INIT has any sanity checks)

Not now.  It probably makes sense to take this chance to bound
it by TCP_RTO_MAX.

Where do you suggest to bound to TCP_RTO_MAX? In tcp_timeout_init?

Right, tcp_timeout_init should work.