[PATCH] tap: Capture only packets that are actually sent

[PATCH] tap: Capture only packets that are actually sent

[Laurent Vivier]

In tap_send_frames(), if we failed to send all the frames, we must
only log the frames that have been sent, not all the frames we wanted
to send.

Fixes: dda7945ca9c9 ("pcap: Handle short writes in pcap_frame()")
Cc: david@gibson.dropbear.id.au
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---

Notes:
    David, I don't understand why you changed the 'm' by an 'n' in your
    commit.

 tap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tap.c b/tap.c
index c7b9372668ec..f4051cecab84 100644
--- a/tap.c
+++ b/tap.c
@@ -413,7 +413,7 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, size_t n)
 	if (m < n)
 		debug("tap: failed to send %zu frames of %zu", n - m, n);
 
-	pcap_multiple(iov, 1, n, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
+	pcap_multiple(iov, 1, m, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
 
 	return m;
 }
-- 
@@ -413,7 +413,7 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, size_t n)
 	if (m < n)
 		debug("tap: failed to send %zu frames of %zu", n - m, n);
 
-	pcap_multiple(iov, 1, n, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
+	pcap_multiple(iov, 1, m, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
 
 	return m;
 }
-- 
2.42.0

Re: [PATCH] tap: Capture only packets that are actually sent

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

On Thu, Mar 07, 2024 at 12:26:02PM +0100, Laurent Vivier wrote:
> In tap_send_frames(), if we failed to send all the frames, we must
> only log the frames that have been sent, not all the frames we wanted
> to send.
> 
> Fixes: dda7945ca9c9 ("pcap: Handle short writes in pcap_frame()")
> Cc: david@gibson.dropbear.id.au
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> 
> Notes:
>     David, I don't understand why you changed the 'm' by an 'n' in your
>     commit.

Huh.. I'm not sure either.  I can think of three possibilities:

 1) I just made a typo

 2) I was thinking of frames lost to write errors as being lost "on
    the wire", so should be still captured as "sent" on this side.
    But that's silly - excluding these is going to be more useful for
    debug in nearly every case.

 3) The original version of this patch might have predated b62ed9ca0
    "tap: Don't pcap frames that didn't get sent".  In which case I
    may have just missed this subtlety when rebasing.

In any case, this is a good change so,

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

>  tap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tap.c b/tap.c
> index c7b9372668ec..f4051cecab84 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -413,7 +413,7 @@ size_t tap_send_frames(const struct ctx *c, const struct iovec *iov, size_t n)
>  	if (m < n)
>  		debug("tap: failed to send %zu frames of %zu", n - m, n);
>  
> -	pcap_multiple(iov, 1, n, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
> +	pcap_multiple(iov, 1, m, c->mode == MODE_PASST ? sizeof(uint32_t) : 0);
>  
>  	return m;
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] tap: Capture only packets that are actually sent

[Stefano Brivio]

On Thu,  7 Mar 2024 12:26:02 +0100
Laurent Vivier <lvivier@redhat.com> wrote:

> In tap_send_frames(), if we failed to send all the frames, we must
> only log the frames that have been sent, not all the frames we wanted
> to send.
> 
> Fixes: dda7945ca9c9 ("pcap: Handle short writes in pcap_frame()")
> Cc: david@gibson.dropbear.id.au
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Applied.

-- 
Stefano