From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=YVl3XeUj; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id CD2175A0271 for ; Thu, 11 Sep 2025 11:54:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1757584472; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=JkpHm44HdL3InEMbbN+/2Q9ZnhqUc/lN3IhMzYWFhgw=; b=YVl3XeUjPsIvUzMdJYj/SoRceV4LiQR4otlWgzYpzfPf/mG/lremy2EaQ7JKqbh8wekLIN 7Wv5EC0tyBi3xYVL3a6ozDyaFUTVwBHXnoBNoeD7hHfctkJNrlCTdeo3oy0wT0MnQkh1Ap OP6V/DENKHoHR/sjajpm+1UczwzB79U= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-630-JBJ1pJ_ONmKvQVyAKIUd9w-1; Thu, 11 Sep 2025 05:54:30 -0400 X-MC-Unique: JBJ1pJ_ONmKvQVyAKIUd9w-1 X-Mimecast-MFC-AGG-ID: JBJ1pJ_ONmKvQVyAKIUd9w_1757584469 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-45de07b831dso3423315e9.1 for ; Thu, 11 Sep 2025 02:54:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757584469; x=1758189269; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=JkpHm44HdL3InEMbbN+/2Q9ZnhqUc/lN3IhMzYWFhgw=; b=pN+okd4m1+fmlnkg81Nd+gC5lehJ1wrfI8YPgymFB9hECCVtJjza6nSGNN003prIYC zn27t9ioz7yn2kGJ0VQdVprwx8lFET8aqHIm1XRsqJCqMCpYpGnuQTDL6WMuTQdJNHXk msqcqh+ykCWOhJ5yKOooPsxXiJL6upXjapgN5gPmcMvFBDqViUFIQlEsiuaxlMjnwUOg W7t0VUd+drH4gil9E09rrNmZhvZVPJzXKib1DGtyExAoDP9jJPO3riCCsZRy5rnUGync yARhAZoarb651V1xLASMjH+f92RQml9GbwGxPG5YS/AdkjuQ+pxDAM11xBRwwgd0YJXt +u+g== X-Gm-Message-State: AOJu0YwE6ZcYiLNxAneFdm4Ob5NERgRgm9GVd7i0wNIzvCJG0tPuA5sB sD0mOyn7L5TximHsFiV3vWYNRuDv2pVhYbbL2yrJV0UoZDYcX/OksmFRTsipsg4QaGF8c8sVHo3 dV90tZ/S+3Vbe6gLTSwPqzvHoXCl5IuK2Hv90a0loepcNPU9KqT6WoA== X-Gm-Gg: ASbGncsiNCgbqNL/ECRLPYSc/sWgVuKLb8XkUqqI3eao6JCR2Nf4fmo+5INUlS4rNfN Hrq5a8D5cgOJnBSiFKwnRSAkDAN7A2gBSPvL1EJFnBBPV7MHS5xA27JGSa/bfkW0/YumbCWatlb 3/ftcNLL8F66HRaaH5ONxYRdWOIHDZuArcvxRZqDyAUa8abfJc+2b+Q8i86PqhLlbumEbUjpz7t FxChBFly1lUV9Hi8w73hdVJSrwoauXwrv7YIOGSj5nG6pBvr6+1/TVb0PueqGVKBPf5kGnhEygL cFh7n1E1EYzSMnkwdOA7C+VNwM35nWv3gGR8KadtWTEo8TUac5Xi+Gtvnt4RY36FunPi X-Received: by 2002:a05:600c:1912:b0:45d:d099:873 with SMTP id 5b1f17b1804b1-45de2547b9emr159126975e9.6.1757584468811; Thu, 11 Sep 2025 02:54:28 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFbX85eWAS95GbSkwnqQJjP7CUJyhY2I3bg5tY0E8ZDP92UGdzFTtnmuJIACCV9GpVS9KWjhQ== X-Received: by 2002:a05:600c:1912:b0:45d:d099:873 with SMTP id 5b1f17b1804b1-45de2547b9emr159126735e9.6.1757584468304; Thu, 11 Sep 2025 02:54:28 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45e01baa70dsm20232895e9.15.2025.09.11.02.54.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Sep 2025 02:54:27 -0700 (PDT) Date: Thu, 11 Sep 2025 11:54:25 +0200 From: Stefano Brivio To: Yumei Huang Subject: Re: [PATCH] tap: Drop frames if no client connected Message-ID: <20250911115425.79eaaac5@elisabeth> In-Reply-To: <20250911085519.24395-1-yuhuang@redhat.com> References: <20250911085519.24395-1-yuhuang@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: doH5eAPS_JGFTfeDlM451vRtGY9Mmy0oDpH94q5U-XE_1757584469 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 4GOF7TGM2BIZ5WXYQBGKVEPF7GZWJIKW X-Message-ID-Hash: 4GOF7TGM2BIZ5WXYQBGKVEPF7GZWJIKW X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, david@gibson.dropbear.id.au X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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