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=dW3vsq7p; 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 7636F5A004E for ; Thu, 05 Feb 2026 01:17:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770250670; 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=uIz2ePcRjA66yDF29F52LKIa5hBHYc5lKnNgeVEjptg=; b=dW3vsq7psCvpIn6lq4vcGZ/+JG9L5e8EmmnC8Ek9fkXbMty2Bb/UghlOfydhK43M/dB5kQ vIZN9+EBbNPdjHJvmrLpPbgx+5s4eJWTqOkLfwzre2QbfmqWkeXBi5AI/rrHjphj00BO/C RO+tZaVLInSuVE4NZDXzVPX3HiZSGuA= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-382-S5CvlQ-SOdK1bFKNRhMvVw-1; Wed, 04 Feb 2026 19:17:48 -0500 X-MC-Unique: S5CvlQ-SOdK1bFKNRhMvVw-1 X-Mimecast-MFC-AGG-ID: S5CvlQ-SOdK1bFKNRhMvVw_1770250668 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-4325cc15176so283168f8f.1 for ; Wed, 04 Feb 2026 16:17:48 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770250667; x=1770855467; 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=uIz2ePcRjA66yDF29F52LKIa5hBHYc5lKnNgeVEjptg=; b=TibfMatls++OvDvZm/n6Ix2JP9CKHo7HxXT3FBvx0cQwZHeQd2SCWAtP0VQvM9TRiA WtKSd1tXHXSgxGgW7G3V6KXcjSiuFmXH4fAKPZs+dcnvcfaVMHy26mazr7JTuESBVz0h eq1JWup/49gIm1LBpolwXdVbHkXDrGUoVi7AjAzgOLjE2nqcgiNaTMYNXUeJbHDn39UL Cyqn2Br4b15YXbytNlgnwSgPC3Xnoy6K5wZZAY7rdX4fAUg+jF2RB9/UBc/AUpz4LCie XbsfxWoBxywcu/A35Bt7L6FRt+li/CRf9Ao2tSEGK59zZvGP1rM+20vuGGEz4QktNG83 gtCw== X-Gm-Message-State: AOJu0Yy4DZFOPktu+s9sH7jWDNd8q6b5iRrFgj8hbYWwI53RrIS8GdpL TgAIpCrA2S9e7kxqe/23hzmnk2NsnBzE8Wle5i9EqCq3h7jj/WsUOzxwZbacjBituwfnGVrRSmS TKq7QwXOZ2G+WE8+zDj8mYo0n2sPhjlt5qa7RyDGpTxHjyp5DGsyaoQdtK4WLxA== X-Gm-Gg: AZuq6aItHfjA7nCKK6OkRiNMsX1TOYv0fKfF++Uxug3cNFw95D65qf3PxNAcXQ0sRXT 7ATGrx1atWD6eYtYMVQ9yXXc8dzMsP7afzGytYMuptaakw7xTPzgSO7dCppxNQLlWLTPqiQJbof kjxVkGSdHtgRNfFxzr+fkixfgabGsAdlmBn3wmG3TyPmwno8jLnF+h1mwkZCW6v9nBgDHbBO0PL rveHebYSJI72Q03xR/c1BtZTPaZ87AM1rzj/RJDAbpgKyrRoWyea9cW1lV5yajmQT2ECl82w+L7 3DuYw/NPEpxnaZmMk7z926sdMrDL4WoT9NXtYNKj2jnY2vtcAb56qe3c3r/c2iDgyX464A2lbBG vUkeWisyWVJPObJhWwuUXWM9x5pu1G4RNy+UWMA== X-Received: by 2002:a5d:5f92:0:b0:430:fb6d:1442 with SMTP id ffacd0b85a97d-43618063b88mr7520387f8f.62.1770250666715; Wed, 04 Feb 2026 16:17:46 -0800 (PST) X-Received: by 2002:a5d:5f92:0:b0:430:fb6d:1442 with SMTP id ffacd0b85a97d-43618063b88mr7520359f8f.62.1770250666174; Wed, 04 Feb 2026 16:17:46 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43617e3a486sm10171387f8f.16.2026.02.04.16.17.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Feb 2026 16:17:45 -0800 (PST) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 4/4] tcp: Send TCP keepalive segments after a period of tap-side inactivity Message-ID: <20260205011744.4243b8ce@elisabeth> In-Reply-To: <20260204114137.2784090-5-david@gibson.dropbear.id.au> References: <20260204114137.2784090-1-david@gibson.dropbear.id.au> <20260204114137.2784090-5-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: Thu, 05 Feb 2026 01:17:45 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SlLdbFUXI4UpUWd1KeONYwUTQKIPJs5TI-T_txyczEk_1770250668 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: AYVOVM7X4YXALGBEBOW3YLMKIOSJSIU4 X-Message-ID-Hash: AYVOVM7X4YXALGBEBOW3YLMKIOSJSIU4 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 Wed, 4 Feb 2026 21:41:37 +1000 David Gibson wrote: > There are several circumstances in which a live, but idle TCP connection > can be forgotten by a guest, with no "on the wire" indication that this has > happened. The most obvious is if the guest abruptly reboots. A more > subtle case can happen with a half-closed connection, specifically one > in FIN_WAIT_2 state on the guest. A connection can, legitimately, remain > in this state indefinitely. If however, a socket in this state is closed > by userspace, Linux at least will remove the kernel socket after 60s > (or as configured in the net.ipv4.tcp_fin_timeout sysctl). > > Because there's no on the wire indication in these cases, passt will > pointlessly retain the connection in its flow table, at least until it is > removed by the inactivity timeout after several hours. > > To avoid keeping connections around for so long in this state, add > functionality to periodically send TCP keepalive segments to the guest if > we've seen no activity on the tap interface. If the guest is no longer > aware of the connection, it should respond with an RST which will let > passt remove the stale entry. > > To do this we use a method similar to the inactivity timeout - a 1-bit > page replacement / clock algorithm, but with a shorter interval, and only > checking for tap side activity. Currently we use a 300s interval, meaning > we'll send a keepalive after 5-10 minutes of (tap side) inactivity. > > Link: https://bugs.passt.top/show_bug.cgi?id=179 > > Signed-off-by: David Gibson > --- > tcp.c | 39 +++++++++++++++++++++++++++++++++++++++ > tcp.h | 2 ++ > tcp_conn.h | 2 ++ > 3 files changed, 43 insertions(+) > > diff --git a/tcp.c b/tcp.c > index acdac7df..bf57be23 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -206,6 +206,12 @@ > * keepalives) will be removed between INACTIVITY_INTERVAL s and > * 2*INACTIVITY_INTERVAL s after the last activity. > * > + * - KEEPALIVE_INTERVAL: if a connection has had no tap-side activity for an > + * entire interval, send a tap-side keepalive. If the endpoint is no longer > + * aware of the connection (due to a reboot, or a kernel timeout in FIN_WAIT_2 > + * state) that should trigger an RST, so we won't keep track of connections > + * that the guest endpoint no longer cares about. > + * > * Summary of data flows (with ESTABLISHED event) > * ---------------------------------------------- > * > @@ -342,6 +348,7 @@ enum { > #define RTO_INIT_AFTER_SYN_RETRIES 3 /* s, RFC 6298 */ > > #define INACTIVITY_INTERVAL 7200 /* s */ > +#define KEEPALIVE_INTERVAL 30 /* s */ By the way, once we're done testing this, I'm not sure which value to use. MUST-28 in RFC 9293 (3.8.4. TCP Keep-Alives) says we should "default to no less than two hours". It looks a bit too long to be useful, to me. > > #define LOW_RTT_TABLE_SIZE 8 > #define LOW_RTT_THRESHOLD 10 /* us */ > @@ -2263,6 +2270,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, > } > > conn->inactive = false; > + conn->tap_inactive = false; > > if (th->ack && !(conn->events & ESTABLISHED)) > tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq)); > @@ -2884,6 +2892,36 @@ int tcp_init(struct ctx *c) > return 0; > } > > +/** > + * tcp_keepalive() - Send keepalives for connections which need it > + * @: Execution context * @c: Execution context * @now: Current timestamp > + */ > +static void tcp_keepalive(struct ctx *c, const struct timespec *now) > +{ > + union flow *flow; > + > + if (now->tv_sec - c->tcp.keepalive_run < KEEPALIVE_INTERVAL) > + return; > + > + c->tcp.keepalive_run = now->tv_sec; > + > + flow_foreach(flow) { > + struct tcp_tap_conn *conn = &flow->tcp; > + > + if (flow->f.type != FLOW_TCP) > + continue; > + > + if (conn->tap_inactive) { > + flow_dbg(conn, "No tap activity for least %us, send keepalive", s/least// (or "for at least") > + KEEPALIVE_INTERVAL); > + tcp_send_flag(c, conn, KEEPALIVE); Do we have to make sure we don't interpret replies to keepalives as duplicate ACKs (by setting some internal flow flag)? I haven't really checked that path. > + } > + > + /* Ready to check fot next interval */ for > + conn->tap_inactive = true; > + } > +} > + > /** > * tcp_inactivity() - Scan for and close long-inactive connections > * @: Execution context > @@ -2927,6 +2965,7 @@ void tcp_timer(struct ctx *c, const struct timespec *now) > if (c->mode == MODE_PASTA) > tcp_splice_refill(c); > > + tcp_keepalive(c, now); > tcp_inactivity(c, now); > } > > diff --git a/tcp.h b/tcp.h > index e104d453..2739f309 100644 > --- a/tcp.h > +++ b/tcp.h > @@ -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 > + * @keepalive_run: Time we last issued tap-side keepalives > * @inactivity_run: Time we last scanned for inactive connections > */ > struct tcp_ctx { > @@ -48,6 +49,7 @@ struct tcp_ctx { > int rto_max; > uint8_t syn_retries; > uint8_t syn_linear_timeouts; > + time_t keepalive_run; > time_t inactivity_run; > }; > > diff --git a/tcp_conn.h b/tcp_conn.h > index 7197ff63..c82e1441 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 > + * @tapinactive: No tao activity within the current KEEPALIVE_INTERVAL tap_inactive, tap activity > * @inactive: No activity within the current INACTIVITY_INTERVAL > * @sock: Socket descriptor number > * @events: Connection events, implying connection states > @@ -58,6 +59,7 @@ 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 tap_inactive :1; > bool inactive :1; > > int sock :FD_REF_BITS; The rest looks all good to me, but I didn't test it at all. -- Stefano