Re: [PATCH v2] tap: Drop frames if no client connected

[David Gibson <david@gibson.dropbear.id.au>]

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

On Fri, Sep 12, 2025 at 04:17:05PM +0800, Yumei Huang wrote:
> If no client is attached, discard outgoing frames and report them as
> sent. Without this we will get an EBADF in either writev() (pasta) or
> sendmsg() (passt). That's basically harmless, but a bit ugly.
> Explicitly catching this case results in behaviour that's probably a
> bit clearer to debug if we hit it.
> 
> There are several different approaches we can take here. Here's some
> reasoning as David explained:
> 
> * Don't listen() until the tap connection is ready
> 
>  - It's not clear that the host rejecting the connection is better
>    than the host accepting, then the connection stalling until the
>    guest is ready.
>  - Would require substantial rework because we currently listen() as
>    we parse the command line and don't store the information we'd need
>    to do it later.
> 
> * Don't accept() until the tap connection is ready
> 
>  - To the peer, will behave basically the same as this patch - the
>    host will complete the TCP handshake, then the connection will stall
>    until the guest is ready.
>  - More work to implement, because essentially every sock-side handler
>    has to check fd_tap and abort early
> 
> * Drop packets in tap_send_frames(), but return 0
> 
>  - To the peer, would behave basically the same
>  - Would make the TCP code do a bunch of busy work attempting to
>    resend, probably to no avail
>  - Handling of errors returned by tap_send_frames() is on the basis
>    that it's probably a transient fault (buffer full) and we want to
>    resend very soon.  That approach doesn't make sense for a missing
>    guest.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>

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

> ---
>  tap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tap.c b/tap.c
> index 7ba6399..2e371b3 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -507,13 +507,16 @@ static size_t tap_send_frames_passt(const struct ctx *c,
>   * @iov must have total length @bufs_per_frame * @nframes, with each set of
>   * @bufs_per_frame contiguous buffers representing a single frame.
>   *
> - * Return: number of frames actually sent
> + * Return: number of frames actually sent, or accounted as sent

I don't think it's worth a respin, but this might be better phrased as:
	"number of frames actually sent, or silently dropped"

>   */
>  size_t tap_send_frames(const struct ctx *c, const struct iovec *iov,
>  		       size_t bufs_per_frame, size_t nframes)
>  {
>  	size_t m;
>  
> +	if (c->fd_tap == -1)
> +		return nframes;
> +
>  	if (!nframes)
>  		return 0;
>  
> -- 
> 2.47.0
> 

-- 
David Gibson (he or they)	| 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 --]