tcp: Send TCP keepalive segments after a period of tap-side inactivityHEADmaster

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 <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

3 files changed, 43 insertions, 0 deletions

diff --git a/tcp.c b/tcp.c
index 60e2487..88ad886 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 */
 
 #define LOW_RTT_TABLE_SIZE		8
 #define LOW_RTT_THRESHOLD		10 /* us */
@@ -2265,6 +2272,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));
@@ -2887,6 +2895,36 @@ int tcp_init(struct ctx *c)
 }
 
 /**
+ * tcp_keepalive() - Send keepalives for connections which need it
+ * @:	Execution context
+ */
+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",
+				 KEEPALIVE_INTERVAL);
+			tcp_send_flag(c, conn, KEEPALIVE);
+		}
+
+		/* Ready to check fot next interval */
+		conn->tap_inactive = true;
+	}
+}
+
+/**
  * tcp_inactivity() - Scan for and close long-inactive connections
  * @:	Execution context
  */
@@ -2929,6 +2967,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 e104d45..2739f30 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 7197ff6..c82e144 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
  * @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;