[PATCH v2] tap: Drop frames if no client connected
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
On Fri, 12 Sep 2025 16:17:05 +0800
Yumei Huang
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
Signed-off-by: Yumei Huang
Applied. Sorry for the delay. -- Stefano
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
Signed-off-by: Yumei Huang
Reviewed-by: David Gibson
--- 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
participants (3)
-
David Gibson
-
Stefano Brivio
-
Yumei Huang