From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=hZ/RLzVR; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id C68955A004E for ; Wed, 25 Feb 2026 07:15:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1772000147; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QdB86QxygQi3G1L+jSAj48vk0Tz9iiWP8Up6cmh+3bQ=; b=hZ/RLzVRMnss9FHbmlR8k1NY0xDYFGq0qkIPq+fju3K5KbvkYuNdbvNWOgRQtXgMZIiUGr PJE/Jp2q2N60JfVpYsZOBLbGGthkHYfNaDgsQIemAm0qClU3T2WoV7fsgZqsDXTwgxqSlb 54P7ktUtUIIzpuF2rDwhXsZcdtX3OPw= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-524-sRAOGQCHNky8vu_6y_tFpg-1; Wed, 25 Feb 2026 01:15:45 -0500 X-MC-Unique: sRAOGQCHNky8vu_6y_tFpg-1 X-Mimecast-MFC-AGG-ID: sRAOGQCHNky8vu_6y_tFpg_1772000145 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-4832c4621c2so61457625e9.3 for ; Tue, 24 Feb 2026 22:15:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772000144; x=1772604944; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QdB86QxygQi3G1L+jSAj48vk0Tz9iiWP8Up6cmh+3bQ=; b=hxCo+dpyR2HKuf6ZDIDKK98XSvbZ41nHzxmcs7YXsUv2vGWlahkgZPB6GLL4N/zUj1 BZTSQiuoVRrmpuG2a+c4Vlpk/qabBzHv3/WjKiXYtxlDGSHB79MSSCaZJJG68nrBpyic aOefqdI9ueq1zOg6iy95zW4mwsWryMCdDN5flH8c3EADFX4Yx4SCHAZkyhfIfBiHmV04 FNeL66HVXr0xCiXRq8mmd2UiP/38k2FbfJpTRPJWJqMt1LDw8TAYcRVWVGhKwxxuVDdt VQ9Oy3vDfu0vo3fled28uunAz9Vl1Bu6B/aKR+QZo1j75z3ee0BAe9H/mrQOXRfirPil Nhwg== X-Gm-Message-State: AOJu0YzFv5GQRdRyxy0v8uIuMQPhCjN5kenbb7cSMYeIMQqSZKU7mkWU LU6GVzU7j8tl5UtFvNHJgYKpPk69FhucpewF1Cw4y3q8ui3e1HXnirZRGwMEzyee3jLbmbs7Wb0 eBVF2R8RmoxGrbizmbuRdYcBdkeUpPpLATCaXDyeVMdEPjcD8UVLBjdvn1Xp5QA== X-Gm-Gg: ATEYQzxHezHr+Hen7hkdF1jCwURTP4yXiBUgnsw7jEfQhFtRucQ3ywZbAuwMnwhLMRb j0HCLurVHNEdZLU3geQk6EtEaXVhr2r/TsaJnE3/sdeQIZMswdEmubSZw0gAxCNggAcbYG099Ps cpyR0F/CqgjTxrV0uKtACgCe8oAjCLKRnEeAd2MjFQdC4ZqAwXme6eA9wE7aIASt07g5tUxZiX8 SA/E8tlUa0QzdfCaG8TJa+xhn8kzG7+oD0V6XQJLKVm3PBPespHIq/1eh9u+tdzspjtt5L7UCeS H+cvvOk9waoK9F+ZTCcAUj9G4z63EVrVd9NwJfHlwRXEOcKPHr5h+XZMnyx6gAYXot1oJ4TwzBG c/Jiec6cB7y6hAV8Y2GFb0kb+3sqSKQAx6tgidiB3iJwIw4j2yg== X-Received: by 2002:a05:600c:8716:b0:471:1717:411 with SMTP id 5b1f17b1804b1-483a95e9a7fmr251947365e9.24.1772000143651; Tue, 24 Feb 2026 22:15:43 -0800 (PST) X-Received: by 2002:a05:600c:8716:b0:471:1717:411 with SMTP id 5b1f17b1804b1-483a95e9a7fmr251946775e9.24.1772000142839; Tue, 24 Feb 2026 22:15:42 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-483bd68826asm69330625e9.0.2026.02.24.22.15.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Feb 2026 22:15:42 -0800 (PST) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 2/4] tcp: Re-introduce inactivity timeouts based on a clock algorithm Message-ID: <20260225071541.04f63648@elisabeth> In-Reply-To: <20260206061739.33648-3-david@gibson.dropbear.id.au> References: <20260206061739.33648-1-david@gibson.dropbear.id.au> <20260206061739.33648-3-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 25 Feb 2026 07:15:41 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 7EIw1VkjatOX-Ic-8ISdf7lubHGPODXCpUbpK5OY7Q0_1772000145 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: JNDYQ4ZOHTDBJ7X74YFNCP5WB674PMPE X-Message-ID-Hash: JNDYQ4ZOHTDBJ7X74YFNCP5WB674PMPE X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, 6 Feb 2026 17:17:37 +1100 David Gibson wrote: > We previously had a mechanism to remove TCP connections which were > inactive for 2 hours. That was broken for a long time, due to poor > interactions with the timerfd handling, so we removed it. > > Adding this long scale timer onto the timerfd handling, which mostly > handles much shorter timeouts is tricky to reason about. However, for the > inactivity timeouts, we don't require precision. Instead, we can use > a 1-bit page replacement / "clock" algorithm. Every INACTIVITY_INTERVAL > (2 hours), a global timer marks every TCP connection as tentatively > inactive. That flag is cleared if we get any events, either tap side or > socket side. > > If the inactive flag is still set when the next INACTIVITY_INTERVAL expires > then the connection has been inactive for an extended period and we reset > and close it. In practice this means that connections will be removed > after 2-4 hours of inactivity. > > This is not a true fix for bug 179, but it does mitigate the damage, by > limiting the time that inactive connections will remain around, > > Link: https://bugs.passt.top/show_bug.cgi?id=179 > Signed-off-by: David Gibson > --- > tcp.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++---- > tcp.h | 4 +++- > tcp_conn.h | 3 +++ > 3 files changed, 55 insertions(+), 5 deletions(-) > > diff --git a/tcp.c b/tcp.c > index f8663369..09929ee9 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -198,6 +198,13 @@ > * TCP_INFO, with a representable range from RTT_STORE_MIN (100 us) to > * RTT_STORE_MAX (3276.8 ms). The timeout value is clamped accordingly. > * > + * We also use a global interval timer for an activity timeout which doesn't > + * require precision: > + * > + * - INACTIVITY_INTERVAL: if a connection has had no activity for an entire > + * interval, close and reset it. This means that idle connections (without > + * keepalives) will be removed between INACTIVITY_INTERVAL seconds and > + * 2*INACTIVITY_INTERVAL seconds after the last activity. > * > * Summary of data flows (with ESTABLISHED event) > * ---------------------------------------------- > @@ -333,7 +340,8 @@ enum { > > #define RTO_INIT 1 /* s, RFC 6298 */ > #define RTO_INIT_AFTER_SYN_RETRIES 3 /* s, RFC 6298 */ > -#define ACT_TIMEOUT 7200 > + > +#define INACTIVITY_INTERVAL 7200 /* s */ > > #define LOW_RTT_TABLE_SIZE 8 > #define LOW_RTT_THRESHOLD 10 /* us */ > @@ -2254,6 +2262,8 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, > return 1; > } > > + conn->inactive = false; > + > if (th->ack && !(conn->events & ESTABLISHED)) > tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq)); > > @@ -2622,6 +2632,8 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, > return; > } > > + conn->inactive = false; > + > if ((conn->events & TAP_FIN_ACKED) && (events & EPOLLHUP)) { > conn_event(c, conn, CLOSED); > return; > @@ -2872,18 +2884,51 @@ int tcp_init(struct ctx *c) > return 0; > } > > +/** > + * tcp_inactivity() - Scan for and close long-inactive connections > + * @c: Execution context > + * @now: Current timestamp > + */ > +static void tcp_inactivity(struct ctx *c, const struct timespec *now) > +{ > + union flow *flow; > + > + if (now->tv_sec - c->tcp.inactivity_run < INACTIVITY_INTERVAL) > + return; > + > + debug("TCP inactivity scan"); > + c->tcp.inactivity_run = now->tv_sec; > + > + flow_foreach(flow) { Nit: this could be flow_foreach_of_type((flow), FLOW_TCP), or, given that it's the second usage of that, we could finally introduce a foreach_tcp_flow() macro, and rebuild foreach_established_tcp_flow() on top of that. Using foreach_established_tcp_flow() should be equivalent here by the way, because in all non-established cases we should have shorter timeouts, but it looks unnecessarily fragile. Same for tcp_keepalive() from 4/4. > + struct tcp_tap_conn *conn = &flow->tcp; > + > + if (flow->f.type != FLOW_TCP) > + continue; > + > + if (conn->inactive) { > + /* No activity in this interval, reset */ > + flow_dbg(conn, "Inactive for at least %us, resetting", > + INACTIVITY_INTERVAL); > + tcp_rst(c, conn); > + } > + > + /* Ready to check for next interval */ > + conn->inactive = true; > + } > +} > + > /** > * tcp_timer() - Periodic tasks: port detection, closed connections, pool refill > * @c: Execution context > * @now: Current timestamp > */ > -void tcp_timer(const struct ctx *c, const struct timespec *now) > +void tcp_timer(struct ctx *c, const struct timespec *now) > { > - (void)now; > - > tcp_sock_refill_init(c); > if (c->mode == MODE_PASTA) > tcp_splice_refill(c); > + > + tcp_inactivity(c, now); > } > > /** > diff --git a/tcp.h b/tcp.h > index 24b90870..e104d453 100644 > --- a/tcp.h > +++ b/tcp.h > @@ -21,7 +21,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, > int tcp_listen(const struct ctx *c, uint8_t pif, unsigned rule, > const union inany_addr *addr, const char *ifname, in_port_t port); > int tcp_init(struct ctx *c); > -void tcp_timer(const struct ctx *c, const struct timespec *now); > +void tcp_timer(struct ctx *c, const struct timespec *now); > void tcp_defer_handler(struct ctx *c); > > void tcp_update_l2_buf(const unsigned char *eth_d); > @@ -38,6 +38,7 @@ extern bool peek_offset_cap; > * @rto_max: Maximum retry timeout (in s) > * @syn_retries: SYN retries using exponential backoff timeout > * @syn_linear_timeouts: SYN retries before using exponential backoff timeout > + * @inactivity_run: Time we last scanned for inactive connections > */ > struct tcp_ctx { > struct fwd_ports fwd_in; > @@ -47,6 +48,7 @@ struct tcp_ctx { > int rto_max; > uint8_t syn_retries; > uint8_t syn_linear_timeouts; > + time_t inactivity_run; > }; > > #endif /* TCP_H */ > diff --git a/tcp_conn.h b/tcp_conn.h > index 21cea109..7197ff63 100644 > --- a/tcp_conn.h > +++ b/tcp_conn.h > @@ -16,6 +16,7 @@ > * @ws_from_tap: Window scaling factor advertised from tap/guest > * @ws_to_tap: Window scaling factor advertised to tap/guest > * @tap_mss: MSS advertised by tap/guest, rounded to 2 ^ TCP_MSS_BITS > + * @inactive: No activity within the current INACTIVITY_INTERVAL > * @sock: Socket descriptor number > * @events: Connection events, implying connection states > * @timer: timerfd descriptor for timeout events > @@ -57,6 +58,8 @@ struct tcp_tap_conn { > (conn->rtt_exp = MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RTT_STORE_MIN)))) > #define RTT_GET(conn) (RTT_STORE_MIN << conn->rtt_exp) > > + bool inactive :1; > + > int sock :FD_REF_BITS; > > uint8_t events; -- Stefano