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

On Thu, 11 Sep 2025 16:55:19 +0800 Yumei Huang wrote:

If no client is attached, discard outgoing frames and report them as sent. This mimics the behavior of a physical host with its network cable unplugged.

Suggested-by: David Gibson Signed-off-by: Yumei Huang

Thanks, the fix itself obviously makes sense, but I have a few questions and comments: - first off, what happens if we don't return early in tap_send_frames()? Commit messages for fixes (assuming this is a fix) should always say what concrete problem we had, what is going to be fixed, or if we're not aware of any real issue but things are just fragile / wrong - until a while ago, this couldn't happen at all. We were just blocking the whole execution as long as the tap / guest / container interface wasn't up and running. I wonder when this changed and if it makes sense to go back to the previous behaviour. I had just a quick look and I wonder if I accidentally broke this in c9b241346569 ("conf, passt, tap: Open socket and PID files before switching UID/GID"). Before that, main() would call tap_sock_init(), which would call tap_sock_unix_open(), a blocking function. Should we make the whole thing blocking again? If not, is there anything else that's breaking with that? Timers, other inputs, etc. I didn't really have time to investigate until now, I can try to have another look soon though, unless you find out more meanwhile.

--- tap.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tap.c b/tap.c index 7ba6399..e01219d 100644 --- a/tap.c +++ b/tap.c @@ -507,13 +507,17 @@ 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 */ 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) + /* If no client connected, account the frames have been sent */

I think the comment is redundant because, well, if c->fd_tap is -1 (obvious, documented), we return 'nframes' (also documented). If it's not redundant, for any reason, "to account" in this sense isn't transitive. You could say: "consider that the frames have been sent" but not "account that the frames have been sent". You can pick a different meaning of "to account" and say "account the frames as sent", though.

+ return nframes; + if (!nframes) return 0;

-- Stefano