Re: [PATCH 1/4] tap: Don't pcap frames that didn't get sent

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 16 Feb 2023 16:20:44 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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@gibson.dropbear.id.au> wrote:
> >   
> > > 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.  
> 
> 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.

Actually, I was thinking of the passt case too: the send() in
tap_send_remainder() might fail -- but then, contrary to what I wrote,
there's nothing to truncate, because the socket back-end in QEMU
doesn't deliver partial frames either. So this patch is actually
correct in that regard.

Unrelated: given that QEMU is going to have a "reconnect" option:
  http://patchwork.ozlabs.org/project/qemu-devel/patch/20230119101645.2001683-1-lvivier@redhat.com/

perhaps, if tap_send_remainder() fails, we should now close the socket
to give the guest the best chances to recover? Compared to terminating
the process, this has the advantage of keeping the whole state.

-- 
Stefano