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 1/4] tap: Don't pcap frames that didn't get sent
Date: Thu, 16 Feb 2023 16:43:08 +1100	[thread overview]
Message-ID: <20230216054311.2131853-2-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20230216054311.2131853-1-david@gibson.dropbear.id.au>

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@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);
 }
 
 /**
-- 
@@ -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


  reply	other threads:[~2023-02-16  5:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-16  5:43 [PATCH v2 0/4] Some cleanups to tap and tcp David Gibson
2023-02-16  5:43 ` David Gibson [this message]
2023-02-16  5:43 ` [PATCH v2 2/4] tap: Eliminate goto from tap_handler() David Gibson
2023-02-16  5:43 ` [PATCH v2 3/4] tcp: Remove 'recvmsg' goto from tcp_data_from_sock David Gibson
2023-02-16  5:43 ` [PATCH v2 4/4] tcp: Remove 'zero_len' " David Gibson
2023-02-16 22:22 ` [PATCH v2 0/4] Some cleanups to tap and tcp Stefano Brivio
2023-02-16 22:55   ` David Gibson
2023-02-17  7:44     ` 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=20230216054311.2131853-2-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).