public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>, passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v2 04/18] tcp: Don't keep compute total bytes in a message until we need it
Date: Fri,  9 Dec 2022 16:42:14 +1100	[thread overview]
Message-ID: <20221209054228.4085990-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20221209054228.4085990-1-david@gibson.dropbear.id.au>

tcp[46]_l2_buf_bytes keep track of the total number of bytes we have
queued to send to the tap interface.  tcp_l2_buf_flush_passt() uses this
to determine if sendmsg() has sent all the data we requested, or whether
we need to resend a trailing portion.

However, the logic for finding where we're up to in the case of a short
sendmsg() can equally well tell whether we've had one at all, without
knowing the total number in advance.  This does require an extra loop after
each sendmsg(), but it's doing simple arithmetic on values we've already
been accessing, and it leads to overall simpler code.

tcp[46]_l2_flags_buf_bytes were being calculated, but never used for
anything, so simply remove them.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tcp.c | 55 +++++++++++++++++++------------------------------------
 1 file changed, 19 insertions(+), 36 deletions(-)

diff --git a/tcp.c b/tcp.c
index fe788b4..4050ea6 100644
--- a/tcp.c
+++ b/tcp.c
@@ -476,7 +476,6 @@ static struct tcp4_l2_buf_t {
 tcp4_l2_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp4_l2_buf_used;
-static size_t tcp4_l2_buf_bytes;
 
 /**
  * tcp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections
@@ -507,7 +506,6 @@ struct tcp6_l2_buf_t {
 tcp6_l2_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp6_l2_buf_used;
-static size_t tcp6_l2_buf_bytes;
 
 /* recvmsg()/sendmsg() data for tap */
 static char 		tcp_buf_discard		[MAX_WINDOW];
@@ -555,7 +553,6 @@ static struct tcp4_l2_flags_buf_t {
 tcp4_l2_flags_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp4_l2_flags_buf_used;
-static size_t tcp4_l2_flags_buf_bytes;
 
 /**
  * tcp6_l2_flags_buf_t - IPv6 packet buffers for segments without data (flags)
@@ -585,7 +582,6 @@ static struct tcp6_l2_flags_buf_t {
 tcp6_l2_flags_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp6_l2_flags_buf_used;
-static size_t tcp6_l2_flags_buf_bytes;
 
 /* TCP connections */
 union tcp_conn tc[TCP_MAX_CONNS];
@@ -1418,19 +1414,17 @@ static int tcp_l2_buf_write_one(struct ctx *c, const struct iovec *iov)
  * tcp_l2_buf_flush_passt() - Send a message on the passt tap interface
  * @c:		Execution context
  * @mh:		Message header that was partially sent by sendmsg()
- * @buf_bytes:	Total number of bytes to send
  */
-static void tcp_l2_buf_flush_passt(const struct ctx *c,
-				   const struct msghdr *mh, size_t buf_bytes)
+static void tcp_l2_buf_flush_passt(const struct ctx *c, const struct msghdr *mh)
 {
-	size_t end = 0, missing, sent;
+	size_t end = 0, missing;
 	struct iovec *iov;
 	unsigned int i;
-	ssize_t n;
+	ssize_t sent;
 	char *p;
 
-	n = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT);
-	if (n < 0 || ((sent = (size_t)n) == buf_bytes))
+	sent = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT);
+	if (sent < 0)
 		return;
 
 	/* Ensure a complete last message on partial sendmsg() */
@@ -1441,6 +1435,9 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c,
 	}
 
 	missing = end - sent;
+	if (!missing)
+		return;
+
 	p = (char *)iov->iov_base + iov->iov_len - missing;
 	if (send(c->fd_tap, p, missing, MSG_NOSIGNAL))
 		debug("TCP: failed to flush %lu missing bytes to tap", missing);
@@ -1451,19 +1448,18 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c,
  * @c:		Execution context
  * @mh:		Message header pointing to buffers, msg_iovlen not set
  * @buf_used:	Pointer to count of used buffers, set to 0 on return
- * @buf_bytes:	Pointer to count of buffer bytes, set to 0 on return
  */
 static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh,
-			     unsigned int *buf_used, size_t *buf_bytes)
+			     unsigned int *buf_used)
 {
+	size_t i;
+
 	if (!(mh->msg_iovlen = *buf_used))
 		return;
 
 	if (c->mode == MODE_PASST) {
-		tcp_l2_buf_flush_passt(c, mh, *buf_bytes);
+		tcp_l2_buf_flush_passt(c, mh);
 	} else {
-		size_t i;
-
 		for (i = 0; i < mh->msg_iovlen; i++) {
 			struct iovec *iov = &mh->msg_iov[i];
 
@@ -1471,7 +1467,7 @@ static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh,
 				i--;
 		}
 	}
-	*buf_used = *buf_bytes = 0;
+	*buf_used = 0;
 
 	pcap_multiple(mh->msg_iov, mh->msg_iovlen, sizeof(uint32_t));
 }
@@ -1484,17 +1480,14 @@ static void tcp_l2_flags_buf_flush(struct ctx *c)
 {
 	struct msghdr mh = { 0 };
 	unsigned int *buf_used;
-	size_t *buf_bytes;
 
 	mh.msg_iov	= tcp6_l2_flags_iov;
 	buf_used	= &tcp6_l2_flags_buf_used;
-	buf_bytes	= &tcp6_l2_flags_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 
 	mh.msg_iov	= tcp4_l2_flags_iov;
 	buf_used	= &tcp4_l2_flags_buf_used;
-	buf_bytes	= &tcp4_l2_flags_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 }
 
 /**
@@ -1505,17 +1498,14 @@ static void tcp_l2_data_buf_flush(struct ctx *c)
 {
 	struct msghdr mh = { 0 };
 	unsigned int *buf_used;
-	size_t *buf_bytes;
 
 	mh.msg_iov = tcp6_l2_iov;
 	buf_used	= &tcp6_l2_buf_used;
-	buf_bytes	= &tcp6_l2_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 
 	mh.msg_iov = tcp4_l2_iov;
 	buf_used	= &tcp4_l2_buf_used;
-	buf_bytes	= &tcp4_l2_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 }
 
 /**
@@ -1829,11 +1819,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 					  NULL, conn->seq_to_tap);
 	iov->iov_len = eth_len + sizeof(uint32_t);
 
-	if (CONN_V4(conn))
-		tcp4_l2_flags_buf_bytes += iov->iov_len;
-	else
-		tcp6_l2_flags_buf_bytes += iov->iov_len;
-
 	if (th->ack)
 		conn_flag(c, conn, ~ACK_TO_TAP_DUE);
 
@@ -1849,7 +1834,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 			memcpy(b4 + 1, b4, sizeof(*b4));
 			(iov + 1)->iov_len = iov->iov_len;
 			tcp4_l2_flags_buf_used++;
-			tcp4_l2_flags_buf_bytes += iov->iov_len;
 		}
 
 		if (tcp4_l2_flags_buf_used > ARRAY_SIZE(tcp4_l2_flags_buf) - 2)
@@ -1859,7 +1843,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 			memcpy(b6 + 1, b6, sizeof(*b6));
 			(iov + 1)->iov_len = iov->iov_len;
 			tcp6_l2_flags_buf_used++;
-			tcp6_l2_flags_buf_bytes += iov->iov_len;
 		}
 
 		if (tcp6_l2_flags_buf_used > ARRAY_SIZE(tcp6_l2_flags_buf) - 2)
@@ -2203,7 +2186,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn,
 		len = tcp_l2_buf_fill_headers(c, conn, b, plen, check, seq);
 
 		iov = tcp4_l2_iov + tcp4_l2_buf_used++;
-		tcp4_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len);
+		iov->iov_len = len + sizeof(b->vnet_len);
 		if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1)
 			tcp_l2_data_buf_flush(c);
 	} else if (CONN_V6(conn)) {
@@ -2212,7 +2195,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn,
 		len = tcp_l2_buf_fill_headers(c, conn, b, plen, NULL, seq);
 
 		iov = tcp6_l2_iov + tcp6_l2_buf_used++;
-		tcp6_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len);
+		iov->iov_len = len + sizeof(b->vnet_len);
 		if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1)
 			tcp_l2_data_buf_flush(c);
 	}
-- 
@@ -476,7 +476,6 @@ static struct tcp4_l2_buf_t {
 tcp4_l2_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp4_l2_buf_used;
-static size_t tcp4_l2_buf_bytes;
 
 /**
  * tcp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections
@@ -507,7 +506,6 @@ struct tcp6_l2_buf_t {
 tcp6_l2_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp6_l2_buf_used;
-static size_t tcp6_l2_buf_bytes;
 
 /* recvmsg()/sendmsg() data for tap */
 static char 		tcp_buf_discard		[MAX_WINDOW];
@@ -555,7 +553,6 @@ static struct tcp4_l2_flags_buf_t {
 tcp4_l2_flags_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp4_l2_flags_buf_used;
-static size_t tcp4_l2_flags_buf_bytes;
 
 /**
  * tcp6_l2_flags_buf_t - IPv6 packet buffers for segments without data (flags)
@@ -585,7 +582,6 @@ static struct tcp6_l2_flags_buf_t {
 tcp6_l2_flags_buf[TCP_FRAMES_MEM];
 
 static unsigned int tcp6_l2_flags_buf_used;
-static size_t tcp6_l2_flags_buf_bytes;
 
 /* TCP connections */
 union tcp_conn tc[TCP_MAX_CONNS];
@@ -1418,19 +1414,17 @@ static int tcp_l2_buf_write_one(struct ctx *c, const struct iovec *iov)
  * tcp_l2_buf_flush_passt() - Send a message on the passt tap interface
  * @c:		Execution context
  * @mh:		Message header that was partially sent by sendmsg()
- * @buf_bytes:	Total number of bytes to send
  */
-static void tcp_l2_buf_flush_passt(const struct ctx *c,
-				   const struct msghdr *mh, size_t buf_bytes)
+static void tcp_l2_buf_flush_passt(const struct ctx *c, const struct msghdr *mh)
 {
-	size_t end = 0, missing, sent;
+	size_t end = 0, missing;
 	struct iovec *iov;
 	unsigned int i;
-	ssize_t n;
+	ssize_t sent;
 	char *p;
 
-	n = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT);
-	if (n < 0 || ((sent = (size_t)n) == buf_bytes))
+	sent = sendmsg(c->fd_tap, mh, MSG_NOSIGNAL | MSG_DONTWAIT);
+	if (sent < 0)
 		return;
 
 	/* Ensure a complete last message on partial sendmsg() */
@@ -1441,6 +1435,9 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c,
 	}
 
 	missing = end - sent;
+	if (!missing)
+		return;
+
 	p = (char *)iov->iov_base + iov->iov_len - missing;
 	if (send(c->fd_tap, p, missing, MSG_NOSIGNAL))
 		debug("TCP: failed to flush %lu missing bytes to tap", missing);
@@ -1451,19 +1448,18 @@ static void tcp_l2_buf_flush_passt(const struct ctx *c,
  * @c:		Execution context
  * @mh:		Message header pointing to buffers, msg_iovlen not set
  * @buf_used:	Pointer to count of used buffers, set to 0 on return
- * @buf_bytes:	Pointer to count of buffer bytes, set to 0 on return
  */
 static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh,
-			     unsigned int *buf_used, size_t *buf_bytes)
+			     unsigned int *buf_used)
 {
+	size_t i;
+
 	if (!(mh->msg_iovlen = *buf_used))
 		return;
 
 	if (c->mode == MODE_PASST) {
-		tcp_l2_buf_flush_passt(c, mh, *buf_bytes);
+		tcp_l2_buf_flush_passt(c, mh);
 	} else {
-		size_t i;
-
 		for (i = 0; i < mh->msg_iovlen; i++) {
 			struct iovec *iov = &mh->msg_iov[i];
 
@@ -1471,7 +1467,7 @@ static void tcp_l2_buf_flush(struct ctx *c, struct msghdr *mh,
 				i--;
 		}
 	}
-	*buf_used = *buf_bytes = 0;
+	*buf_used = 0;
 
 	pcap_multiple(mh->msg_iov, mh->msg_iovlen, sizeof(uint32_t));
 }
@@ -1484,17 +1480,14 @@ static void tcp_l2_flags_buf_flush(struct ctx *c)
 {
 	struct msghdr mh = { 0 };
 	unsigned int *buf_used;
-	size_t *buf_bytes;
 
 	mh.msg_iov	= tcp6_l2_flags_iov;
 	buf_used	= &tcp6_l2_flags_buf_used;
-	buf_bytes	= &tcp6_l2_flags_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 
 	mh.msg_iov	= tcp4_l2_flags_iov;
 	buf_used	= &tcp4_l2_flags_buf_used;
-	buf_bytes	= &tcp4_l2_flags_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 }
 
 /**
@@ -1505,17 +1498,14 @@ static void tcp_l2_data_buf_flush(struct ctx *c)
 {
 	struct msghdr mh = { 0 };
 	unsigned int *buf_used;
-	size_t *buf_bytes;
 
 	mh.msg_iov = tcp6_l2_iov;
 	buf_used	= &tcp6_l2_buf_used;
-	buf_bytes	= &tcp6_l2_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 
 	mh.msg_iov = tcp4_l2_iov;
 	buf_used	= &tcp4_l2_buf_used;
-	buf_bytes	= &tcp4_l2_buf_bytes;
-	tcp_l2_buf_flush(c, &mh, buf_used, buf_bytes);
+	tcp_l2_buf_flush(c, &mh, buf_used);
 }
 
 /**
@@ -1829,11 +1819,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 					  NULL, conn->seq_to_tap);
 	iov->iov_len = eth_len + sizeof(uint32_t);
 
-	if (CONN_V4(conn))
-		tcp4_l2_flags_buf_bytes += iov->iov_len;
-	else
-		tcp6_l2_flags_buf_bytes += iov->iov_len;
-
 	if (th->ack)
 		conn_flag(c, conn, ~ACK_TO_TAP_DUE);
 
@@ -1849,7 +1834,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 			memcpy(b4 + 1, b4, sizeof(*b4));
 			(iov + 1)->iov_len = iov->iov_len;
 			tcp4_l2_flags_buf_used++;
-			tcp4_l2_flags_buf_bytes += iov->iov_len;
 		}
 
 		if (tcp4_l2_flags_buf_used > ARRAY_SIZE(tcp4_l2_flags_buf) - 2)
@@ -1859,7 +1843,6 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags)
 			memcpy(b6 + 1, b6, sizeof(*b6));
 			(iov + 1)->iov_len = iov->iov_len;
 			tcp6_l2_flags_buf_used++;
-			tcp6_l2_flags_buf_bytes += iov->iov_len;
 		}
 
 		if (tcp6_l2_flags_buf_used > ARRAY_SIZE(tcp6_l2_flags_buf) - 2)
@@ -2203,7 +2186,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn,
 		len = tcp_l2_buf_fill_headers(c, conn, b, plen, check, seq);
 
 		iov = tcp4_l2_iov + tcp4_l2_buf_used++;
-		tcp4_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len);
+		iov->iov_len = len + sizeof(b->vnet_len);
 		if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1)
 			tcp_l2_data_buf_flush(c);
 	} else if (CONN_V6(conn)) {
@@ -2212,7 +2195,7 @@ static void tcp_data_to_tap(struct ctx *c, struct tcp_tap_conn *conn,
 		len = tcp_l2_buf_fill_headers(c, conn, b, plen, NULL, seq);
 
 		iov = tcp6_l2_iov + tcp6_l2_buf_used++;
-		tcp6_l2_buf_bytes += iov->iov_len = len + sizeof(b->vnet_len);
+		iov->iov_len = len + sizeof(b->vnet_len);
 		if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1)
 			tcp_l2_data_buf_flush(c);
 	}
-- 
2.38.1


  parent reply	other threads:[~2022-12-09  5:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-09  5:42 [PATCH v2 00/18] RFC: Unify and simplify tap send path David Gibson
2022-12-09  5:42 ` [PATCH v2 01/18] pcap: Introduce pcap_frame() helper David Gibson
2023-01-04 17:45   ` Stefano Brivio
2023-01-05  4:47     ` David Gibson
2022-12-09  5:42 ` [PATCH v2 02/18] pcap: Replace pcapm() with pcap_multiple() David Gibson
2022-12-09  5:42 ` [PATCH v2 03/18] tcp: Combine two parts of passt tap send path together David Gibson
2022-12-09  5:42 ` David Gibson [this message]
2023-01-04 17:45   ` [PATCH v2 04/18] tcp: Don't keep compute total bytes in a message until we need it Stefano Brivio
2023-01-05  4:48     ` David Gibson
2022-12-09  5:42 ` [PATCH v2 05/18] tcp: Improve interface to tcp_l2_buf_flush() David Gibson
2023-01-04 17:45   ` Stefano Brivio
2023-01-05  4:53     ` David Gibson
2022-12-09  5:42 ` [PATCH v2 06/18] tcp: Combine two parts of pasta tap send path together David Gibson
2022-12-09  5:42 ` [PATCH v2 07/18] tap, tcp: Move tap send path to tap.c David Gibson
2022-12-09  5:42 ` [PATCH v2 08/18] util: Introduce hton*_constant() in place of #ifdefs David Gibson
2022-12-09  5:42 ` [PATCH v2 09/18] tcp, udp: Use named field initializers in iov_init functions David Gibson
2022-12-09  5:42 ` [PATCH v2 10/18] util: Parameterize ethernet header initializer macro David Gibson
2022-12-09  5:42 ` [PATCH v2 11/18] tcp: Remove redundant and incorrect initialization from *_iov_init() David Gibson
2022-12-09  5:42 ` [PATCH v2 12/18] tcp: Consolidate calculation of total frame size David Gibson
2022-12-09  5:42 ` [PATCH v2 13/18] tap: Add "tap headers" abstraction David Gibson
2022-12-09  5:42 ` [PATCH v2 14/18] tcp: Use abstracted tap header David Gibson
2022-12-09  5:42 ` [PATCH v2 15/18] tap: Use different io vector bases depending on tap type David Gibson
2022-12-09  5:42 ` [PATCH v2 16/18] udp: Use abstracted tap header David Gibson
2022-12-09  5:42 ` [PATCH v2 17/18] udp: Use tap_send_frames() David Gibson
2023-01-04 17:45   ` Stefano Brivio
2023-01-05  4:54     ` David Gibson
2022-12-09  5:42 ` [PATCH v2 18/18] tap: Improve handling of partial frame sends David Gibson
2023-01-04 17:45   ` Stefano Brivio
2023-01-05  4:57     ` David Gibson
2023-01-04 17:45 ` [PATCH v2 00/18] RFC: Unify and simplify tap send path Stefano Brivio

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=20221209054228.4085990-5-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --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).