In tap_send_frames() we send a number of frames to the tap device, then
also write them to the pcap capture file (if configured).  However the tap
send can partially fail (short write()s or similar), meaning that some
of the requested frames weren't actually sent, but we still write those
frames to the capture file.

We do give a debug message in this case, but it's misleading to add frames
that we know weren't sent to the capture file.  Rework to avoid this.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 tap.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/tap.c b/tap.c
index 716d887..ae60fd4 100644
--- a/tap.c
+++ b/tap.c
@@ -309,10 +309,12 @@ void tap_icmp6_send(const struct ctx *c,
  * @iov:	Array of buffers, each containing one frame
  * @n:		Number of buffers/frames in @iov
  *
+ * Return: number of frames successfully sent
+ *
  * #syscalls:pasta write
  */
-static void tap_send_frames_pasta(struct ctx *c,
-				  const struct iovec *iov, size_t n)
+static size_t tap_send_frames_pasta(struct ctx *c,
+				    const struct iovec *iov, size_t n)
 {
 	size_t i;
 
@@ -325,6 +327,8 @@ static void tap_send_frames_pasta(struct ctx *c,
 			iov--;
 		}
 	}
+
+	return n;
 }
 
 /**
@@ -357,10 +361,12 @@ static void tap_send_remainder(const struct ctx *c, const struct
iovec *iov,
  * @iov:	Array of buffers, each containing one frame
  * @n:		Number of buffers/frames in @iov
  *
+ * Return: number of frames successfully sent
+ *
  * #syscalls:passt sendmsg
  */
-static void tap_send_frames_passt(const struct ctx *c,
-				  const struct iovec *iov, size_t n)
+static size_t tap_send_frames_passt(const struct ctx *c,
+				    const struct iovec *iov, size_t n)
 {
 	struct msghdr mh = {
 		.msg_iov = (void *)iov,
@@ -371,7 +377,7 @@ static void tap_send_frames_passt(const struct ctx *c,
 
 	sent = sendmsg(c->fd_tap, &mh, MSG_NOSIGNAL | MSG_DONTWAIT);
 	if (sent < 0)
-		return;
+		return 0;
 
 	/* Check for any partial frames due to short send */
 	for (i = 0; i < n; i++) {
@@ -386,8 +392,7 @@ static void tap_send_frames_passt(const struct ctx *c,
 		i++;
 	}
 
-	if (i < n)
-		debug("tap: dropped %lu frames due to short send", n - i);
+	return i;
 }
 
 /**
@@ -398,15 +403,20 @@ static void tap_send_frames_passt(const struct ctx *c,
  */
 void tap_send_frames(struct ctx *c, const struct iovec *iov, size_t n)
 {
+	size_t m;
+
 	if (!n)
 		return;
 
 	if (c->mode == MODE_PASST)
-		tap_send_frames_passt(c, iov, n);
+		m = tap_send_frames_passt(c, iov, n);
 	else
-		tap_send_frames_pasta(c, iov, n);
+		m = tap_send_frames_pasta(c, iov, n);
+
+	if (m < n)
+		debug("tap: dropped %lu frames of %lu due to short send", n - m, n);
 
-	pcap_multiple(iov, n, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
+	pcap_multiple(iov, m, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
 }
 
 /**
-- 
2.39.1

This goto can be handled just as simply and more clearly with a do while.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 tcp.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/tcp.c b/tcp.c
index f028e01..b101441 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2186,13 +2186,12 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn
*conn)
 		iov_sock[fill_bufs].iov_len = iov_rem;
 
 	/* Receive into buffers, don't dequeue until acknowledged by guest. */
-recvmsg:
-	len = recvmsg(s, &mh_sock, MSG_PEEK);
-	if (len < 0) {
-		if (errno == EINTR)
-			goto recvmsg;
+	do
+		len = recvmsg(s, &mh_sock, MSG_PEEK);
+	while (len < 0 && errno == EINTR);
+
+	if (len < 0)
 		goto err;
-	}
 
 	if (!len)
 		goto zero_len;
-- 
2.39.1

On Thu, 16 Feb 2023 16:43:07 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  This is a handful of simple cleanups which I made
while investigating
 https://bugs.passt.top/show_bug.cgi?id=41.  Note that these don't
 themselves actually address that bug, they're just unrelated cleanups
 that happen to be in adjacent code.
 
 I've run basic tests on these, but not quite the complete test suite:
 I haven't yet had a chance to debug an unrelated problem with the IPv4
 performance tests failing.
 
 Changes since v1:
  * Minor comment and style fixes
 
 David Gibson (4):
   tap: Don't pcap frames that didn't get sent
   tap: Eliminate goto from tap_handler()
   tcp: Remove 'recvmsg' goto from tcp_data_from_sock
   tcp: Remove 'zero_len' goto from tcp_data_from_sock 
Applied.

-- 
Stefano