public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Jon Maloy <jmaloy@redhat.com>
To: passt-dev@passt.top, sbrivio@redhat.com, lvivier@redhat.com,
	dgibson@redhat.com, jmaloy@redhat.com
Subject: [PATCH v9 1/2] tcp: leverage support of SO_PEEK_OFF socket option when available
Date: Fri, 12 Jul 2024 15:04:49 -0400	[thread overview]
Message-ID: <20240712190450.1261907-2-jmaloy@redhat.com> (raw)
In-Reply-To: <20240712190450.1261907-1-jmaloy@redhat.com>

From linux-6.9.0 the kernel will contain
commit 05ea491641d3 ("tcp: add support for SO_PEEK_OFF socket option").

This new feature makes is possible to call recv_msg(MSG_PEEK) and make
it start reading data from a given offset set by the SO_PEEK_OFF socket
option. This way, we can avoid repeated reading of already read bytes of
a received message, hence saving read cycles when forwarding TCP
messages in the host->name space direction.

In this commit, we add functionality to leverage this feature when
available, while we fall back to the previous behavior when not.

Measurements with iperf3 shows that throughput increases with 15-20
percent in the host->namespace direction when this feature is used.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>

 ---
v2: - Some smaller changes as suggested by David Gibson and Stefano Brivio.
    - Moved initial set_peek_offset(0) to only the locations where the socket is set
      to ESTABLISHED.
    - Removed the per-packet synchronization between sk_peek_off and
      already_sent. Instead only doing it in retransmit situations.
    - The problem I found when trouble shooting the occasionally occurring
      out of synch values between 'already_sent' and 'sk_peek_offset' may
      have deeper implications that we may need to be investigate.

v3: - Rebased to most recent version of tcp.c, plus the previous
      patch in this series.
    - Some changes based on feedback from PASST team

v4: - Some small changes based on feedback from Stefan/David.

v5: - Re-added accidentally dropped set_peek_offset() line.
      Thank you, David.

v6: - Adapted to new file structure. No code changes.

v7: - Added a tcp_rst() when we tcp_set_peek_offset() fails.
      Suggested by David and Stefano.
---
 tcp.c     | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 tcp.h     |  3 +++
 tcp_buf.c | 25 +++++++++++++++++++------
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/tcp.c b/tcp.c
index 698e7ec..e7e0a98 100644
--- a/tcp.c
+++ b/tcp.c
@@ -373,6 +373,9 @@ static union inany_addr low_rtt_dst[LOW_RTT_TABLE_SIZE];
 
 char		tcp_buf_discard		[MAX_WINDOW];
 
+/* Does the kernel support TCP_PEEK_OFF? */
+bool peek_offset_cap;
+
 /* sendmsg() to socket */
 static struct iovec	tcp_iov			[UIO_MAXIOV];
 
@@ -388,6 +391,25 @@ static_assert(ARRAY_SIZE(tc_hash) >= FLOW_MAX,
 int init_sock_pool4		[TCP_SOCK_POOL_SIZE];
 int init_sock_pool6		[TCP_SOCK_POOL_SIZE];
 
+/**
+ * tcp_set_peek_offset() - Set SO_PEEK_OFF offset on a socket if supported
+ * @s:          Socket to update
+ * @offset:     Offset in bytes
+ *
+ * Return:      -1 when it fails, 0 otherwise.
+ */
+int tcp_set_peek_offset(int s, int offset)
+{
+	if (!peek_offset_cap)
+		return 0;
+
+	if (setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &offset, sizeof(offset))) {
+		err("Failed to set SO_PEEK_OFF to %i in socket %i", offset, s);
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * tcp_conn_epoll_events() - epoll events mask for given connection state
  * @events:	Current connection events
@@ -1948,6 +1970,10 @@ static int tcp_data_from_tap(struct ctx *c, struct tcp_tap_conn *conn,
 			   "fast re-transmit, ACK: %u, previous sequence: %u",
 			   max_ack_seq, conn->seq_to_tap);
 		conn->seq_to_tap = max_ack_seq;
+		if (tcp_set_peek_offset(conn->sock, 0)) {
+			tcp_rst(c, conn);
+			return -1;
+		}
 		tcp_data_from_sock(c, conn);
 	}
 
@@ -2040,6 +2066,10 @@ static void tcp_conn_from_sock_finish(struct ctx *c, struct tcp_tap_conn *conn,
 	conn->seq_ack_to_tap = conn->seq_from_tap;
 
 	conn_event(c, conn, ESTABLISHED);
+	if (tcp_set_peek_offset(conn->sock, 0)) {
+		tcp_rst(c, conn);
+		return;
+	}
 
 	/* The client might have sent data already, which we didn't
 	 * dequeue waiting for SYN,ACK from tap -- check now.
@@ -2120,6 +2150,8 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, sa_family_t af,
 			goto reset;
 
 		conn_event(c, conn, ESTABLISHED);
+		if (tcp_set_peek_offset(conn->sock, 0))
+			goto reset;
 
 		if (th->fin) {
 			conn->seq_from_tap++;
@@ -2368,8 +2400,12 @@ void tcp_timer_handler(struct ctx *c, union epoll_ref ref)
 			flow_dbg(conn, "ACK timeout, retry");
 			conn->retrans++;
 			conn->seq_to_tap = conn->seq_ack_from_tap;
-			tcp_data_from_sock(c, conn);
-			tcp_timer_ctl(c, conn);
+			if (tcp_set_peek_offset(conn->sock, 0)) {
+				tcp_rst(c, conn);
+			} else {
+				tcp_data_from_sock(c, conn);
+				tcp_timer_ctl(c, conn);
+			}
 		}
 	} else {
 		struct itimerspec new = { { 0 }, { ACT_TIMEOUT, 0 } };
@@ -2660,7 +2696,8 @@ static void tcp_sock_refill_init(const struct ctx *c)
  */
 int tcp_init(struct ctx *c)
 {
-	unsigned b;
+	unsigned int b, optv = 0;
+	int s;
 
 	for (b = 0; b < TCP_HASH_TABLE_SIZE; b++)
 		tc_hash[b] = FLOW_SIDX_NONE;
@@ -2684,6 +2721,17 @@ int tcp_init(struct ctx *c)
 		NS_CALL(tcp_ns_socks_init, c);
 	}
 
+	/* Probe for SO_PEEK_OFF support */
+	s = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
+	if (s < 0) {
+		warn_perror("Temporary TCP socket creation failed");
+	} else {
+		if (!setsockopt(s, SOL_SOCKET, SO_PEEK_OFF, &optv, sizeof(int)))
+			peek_offset_cap = true;
+		close(s);
+	}
+	info("SO_PEEK_OFF%ssupported", peek_offset_cap ? " " : " not ");
+
 	return 0;
 }
 
diff --git a/tcp.h b/tcp.h
index a9b8bf8..e9ff019 100644
--- a/tcp.h
+++ b/tcp.h
@@ -24,6 +24,9 @@ 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, const unsigned char *eth_s);
+int tcp_set_peek_offset(int s, int offset);
+
+extern bool peek_offset_cap;
 
 /**
  * union tcp_epoll_ref - epoll reference portion for TCP connections
diff --git a/tcp_buf.c b/tcp_buf.c
index ff9f797..11dce02 100644
--- a/tcp_buf.c
+++ b/tcp_buf.c
@@ -408,6 +408,7 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 	uint32_t already_sent, seq;
 	struct iovec *iov;
 
+	/* How much have we read/sent since last received ack ? */
 	already_sent = conn->seq_to_tap - conn->seq_ack_from_tap;
 
 	if (SEQ_LT(already_sent, 0)) {
@@ -416,6 +417,10 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 			   conn->seq_ack_from_tap, conn->seq_to_tap);
 		conn->seq_to_tap = conn->seq_ack_from_tap;
 		already_sent = 0;
+		if (tcp_set_peek_offset(s, 0)) {
+			tcp_rst(c, conn);
+			return -1;
+		}
 	}
 
 	if (!wnd_scaled || already_sent >= wnd_scaled) {
@@ -433,11 +438,16 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 		iov_rem = (wnd_scaled - already_sent) % mss;
 	}
 
-	mh_sock.msg_iov = iov_sock;
-	mh_sock.msg_iovlen = fill_bufs + 1;
-
-	iov_sock[0].iov_base = tcp_buf_discard;
-	iov_sock[0].iov_len = already_sent;
+	/* Prepare iov according to kernel capability */
+	if (!peek_offset_cap) {
+		mh_sock.msg_iov = iov_sock;
+		iov_sock[0].iov_base = tcp_buf_discard;
+		iov_sock[0].iov_len = already_sent;
+		mh_sock.msg_iovlen = fill_bufs + 1;
+	} else {
+		mh_sock.msg_iov = &iov_sock[1];
+		mh_sock.msg_iovlen = fill_bufs;
+	}
 
 	if (( v4 && tcp4_payload_used + fill_bufs > TCP_FRAMES_MEM) ||
 	    (!v4 && tcp6_payload_used + fill_bufs > TCP_FRAMES_MEM)) {
@@ -478,7 +488,10 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 		return 0;
 	}
 
-	sendlen = len - already_sent;
+	sendlen = len;
+	if (!peek_offset_cap)
+		sendlen -= already_sent;
+
 	if (sendlen <= 0) {
 		conn_flag(c, conn, STALLED);
 		return 0;
-- 
@@ -408,6 +408,7 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 	uint32_t already_sent, seq;
 	struct iovec *iov;
 
+	/* How much have we read/sent since last received ack ? */
 	already_sent = conn->seq_to_tap - conn->seq_ack_from_tap;
 
 	if (SEQ_LT(already_sent, 0)) {
@@ -416,6 +417,10 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 			   conn->seq_ack_from_tap, conn->seq_to_tap);
 		conn->seq_to_tap = conn->seq_ack_from_tap;
 		already_sent = 0;
+		if (tcp_set_peek_offset(s, 0)) {
+			tcp_rst(c, conn);
+			return -1;
+		}
 	}
 
 	if (!wnd_scaled || already_sent >= wnd_scaled) {
@@ -433,11 +438,16 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 		iov_rem = (wnd_scaled - already_sent) % mss;
 	}
 
-	mh_sock.msg_iov = iov_sock;
-	mh_sock.msg_iovlen = fill_bufs + 1;
-
-	iov_sock[0].iov_base = tcp_buf_discard;
-	iov_sock[0].iov_len = already_sent;
+	/* Prepare iov according to kernel capability */
+	if (!peek_offset_cap) {
+		mh_sock.msg_iov = iov_sock;
+		iov_sock[0].iov_base = tcp_buf_discard;
+		iov_sock[0].iov_len = already_sent;
+		mh_sock.msg_iovlen = fill_bufs + 1;
+	} else {
+		mh_sock.msg_iov = &iov_sock[1];
+		mh_sock.msg_iovlen = fill_bufs;
+	}
 
 	if (( v4 && tcp4_payload_used + fill_bufs > TCP_FRAMES_MEM) ||
 	    (!v4 && tcp6_payload_used + fill_bufs > TCP_FRAMES_MEM)) {
@@ -478,7 +488,10 @@ int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn)
 		return 0;
 	}
 
-	sendlen = len - already_sent;
+	sendlen = len;
+	if (!peek_offset_cap)
+		sendlen -= already_sent;
+
 	if (sendlen <= 0) {
 		conn_flag(c, conn, STALLED);
 		return 0;
-- 
2.45.2


  reply	other threads:[~2024-07-12 19:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-12 19:04 [PATCH v9 0/2] Add support for SO_PEEK_OFF Jon Maloy
2024-07-12 19:04 ` Jon Maloy [this message]
2024-07-12 19:04 ` [PATCH v9 2/2] tcp: handle shrunk window advertisemenst from guest Jon Maloy
2024-07-15  0:34   ` David Gibson
2024-07-15 17:08     ` Stefano Brivio
2024-07-12 19:43 ` [PATCH v9 0/2] Add support for SO_PEEK_OFF Stefano Brivio
2024-07-15 16:58 ` Stefano Brivio
2024-07-15 18:52   ` Jon Maloy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240712190450.1261907-2-jmaloy@redhat.com \
    --to=jmaloy@redhat.com \
    --cc=dgibson@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).