On Wed, Feb 15, 2023 at 01:17:25PM +0100, Stefano Brivio wrote:On Fri, 27 Jan 2023 16:11:07 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:True.. that only applies for the pasta case, though. For passt we always send a whole frame, or the stream would get out of sync.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.To be really "correct", I guess we should also truncate messages in captures if they were sent partially, by returning the number of bytes sent from tap_send_frames_{pasta,passt}() and then modifying the argument to pcap_frame() in the pcap_multiple() loop.This is relevant because, if a packet has a checksum, we could consider it lost while checking captures. Still, it's a vast improvement on the original, so I would apply this even like it is -- except for two nits, below:Fixed. -- 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/~dgibsonSigned-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 af9bc15..dd22490 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 * + * Returns: number of frames successfully sentFor consistency: "Return:" -- I see now that one slipped through in pcap_frame(). I can "fix" this on merge or in a follow-up patch, too.+ * * #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; @@ -324,6 +326,8 @@ static void tap_send_frames_pasta(struct ctx *c, i--; } } + + return n; } /** @@ -356,10 +360,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 * + * Returns: number of frames successfully sentSame here.