From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none 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=X0G0hPgK; 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 01C775A0271 for ; Sat, 15 Mar 2025 00:50:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1741996234; 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=r23S042YX7FFUo/vQMztnwVYT521UyWjpBqfX+WC9ok=; b=X0G0hPgKANj6l1eDQ/qVcDI4MZFoAkKK5ejWty6I5JfGtbeQIcfVS25O7AxuxUcNgTtiDq 2Z00gKNs/uPVk4M2ATuI3RuIIx9w56XY9uMt0Yd6SfAtKDW+jzlHKKLothTL4VD3TJBB93 FzHz+1J/kzKnd/COcq+2n2suHB58i3I= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-256-KSKZ1t79M_eSUHW6WljcbQ-1; Fri, 14 Mar 2025 19:50:32 -0400 X-MC-Unique: KSKZ1t79M_eSUHW6WljcbQ-1 X-Mimecast-MFC-AGG-ID: KSKZ1t79M_eSUHW6WljcbQ_1741996232 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-39134c762ebso1138831f8f.0 for ; Fri, 14 Mar 2025 16:50:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741996231; x=1742601031; 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=r23S042YX7FFUo/vQMztnwVYT521UyWjpBqfX+WC9ok=; b=YcYgqUMjlTKVBIAdwTNrGk+aq3wOzoK8UhgFrAROqgejsZ4SxZ0egvFeRqNWLdomOR HbEYkoEghIFQXB0SGFW2iKeg1tqa6aLBPJdmcvkjYey8oKEztMK1QFxYBQWiFQfDpYP8 4XVGpmI4vQRLhaIg1dr+JGJZLt88n3VzYmTceqRttu7Mw+mBsCMhEoM3RnZYRFrjqvZ4 hNpZY0ifM4Plc2281Jf4nVBhLtWCF/QvGabfpiLLyZqWDzRHEwTwu8js31DSnuUQADAL nqNxUW3fgRywcupOc/FBFtPj6fNBPvN1YiiHSCfwP6cRtfOMhASlCs0d87W0ikT54cnr 3omg== X-Gm-Message-State: AOJu0YxPpY7B6RluFVicRA6YNXn7u/1BmISiBwj4IixGfNb9U4+VJC8v 5FhhPCYuFLETtmSsFu0RYncM/fo3ldodPHiIYnJtviT+q2OyG3b/2OzojdsFpsgWoZfaRpG47fK /Ji1MdIOV3TbiVTJfqjmhrZicRhXpVuLcvl1iDUk6V2ntt1mQviHWI3ifAg== X-Gm-Gg: ASbGncuorGgkOoBEWcHZl54a8O7qNw5mAkJHh1GHc+SVkDomIj8VuRKXOCatUWTzxKA woreuhq/Nmy60q4G44rRZCVUn+1S6SHXr6cI0dL3UHumcUqmxia8GSv4hL1OtzJSLGsWIViWYjf nkCcdGIwSKrhBQsOkMmr7g0tgyFHDaumomdPq5C5TONp6v65SeZBKHDIpILhhTqKsb76rGsA37t l/G+nXCr0/mc8NyCUuxNtL/37u6+osnV3t2Narc85+czoarhc2MXCI4xzBFN42Cu2XV1BdJf9TD xYMf4yJUQd+8WWns2dXDfI7AKbM= X-Received: by 2002:a5d:5f85:0:b0:38d:d701:419c with SMTP id ffacd0b85a97d-3971f9e521fmr6542066f8f.41.1741996231036; Fri, 14 Mar 2025 16:50:31 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFeo+N2PGcKEAzlrRTKUnDgkXUZyPXrMjTrr+zGuwQP9VmtiIHJ1vDFl+v1Y4IkQCPIw7BXNw== X-Received: by 2002:a5d:5f85:0:b0:38d:d701:419c with SMTP id ffacd0b85a97d-3971f9e521fmr6542055f8f.41.1741996230688; Fri, 14 Mar 2025 16:50:30 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-395cb318aa1sm7245496f8f.64.2025.03.14.16.50.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Mar 2025 16:50:30 -0700 (PDT) Date: Sat, 15 Mar 2025 00:50:28 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2] conf: Unify several paths in conf_ports() Message-ID: <20250315005028.294ca8d3@elisabeth> In-Reply-To: <20250312034359.2776948-1-david@gibson.dropbear.id.au> References: <20250312034359.2776948-1-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: OIOAeQnUNcFbmARFwf-LrmdUwzyMYm8-O7-yhpJpCkU_1741996232 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: IFQHX2GQ5RM4ERT2W4SUFK2MKBYZB5VE X-Message-ID-Hash: IFQHX2GQ5RM4ERT2W4SUFK2MKBYZB5VE 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 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 Wed, 12 Mar 2025 14:43:59 +1100 David Gibson wrote: > In conf_ports() we have three different paths which actually do the setup > of an individual forwarded port: one for the "all" case, one for the > exclusions only case and one for the range of ports with possible > exclusions case. > > We can unify those cases using a new helper which handles a single range > of ports, with a bitmap of exclusions. Although this is slightly longer > (largely due to the new helpers function comment), it reduces duplicated > logic. It will also make future improvements to the tracking of port > forwards easier. > > The new conf_ports_range_except() function has a pretty prodigious > parameter list, but I still think it's an overall improvement in conceptual > complexity. > > Signed-off-by: David Gibson > --- > conf.c | 173 ++++++++++++++++++++++++++++++--------------------------- > 1 file changed, 90 insertions(+), 83 deletions(-) > > v2: > * Commit message updated slightly, but otherwise unmodified. > > > diff --git a/conf.c b/conf.c > index 065e7201..4e0099ba 100644 > --- a/conf.c > +++ b/conf.c > @@ -123,6 +123,75 @@ static int parse_port_range(const char *s, char **endptr, > return 0; > } > > +/** > + * conf_ports_range_except() - Set up forwarding for a range of ports minus a > + * bitmap of exclusions > + * @c: Execution context > + * @optname: Short option name, t, T, u, or U > + * @optarg: Option argument (port specification) > + * @fwd: Pointer to @fwd_ports to be updated > + * @addr: Listening address > + * @ifname: Listening interface > + * @first: First port to forward > + * @last: Last port to forward > + * @exclude: Bitmap of ports to exclude > + * @to: Port to translate @first to when forwarding > + * @weak: Ignore errors, as long as at least one port is mapped > + */ > +static void conf_ports_range_except(const struct ctx *c, char optname, > + const char *optarg, struct fwd_ports *fwd, > + const union inany_addr *addr, > + const char *ifname, > + uint16_t first, uint16_t last, > + const uint8_t *exclude, uint16_t to, > + bool weak) > +{ > + bool bound_one = false; > + unsigned i; > + int ret; > + > + if (first == 0) { > + die("Can't forward port 0 for option '-%c %s'", > + optname, optarg); > + } This introduces two subtle functional changes that are a bit unexpected given the commit message. Before: $ ./pasta -t 0 $ $ ./pasta -t 0-1025 Failed to bind port 1 (Permission denied) for option '-t 0-1025', exiting After: $ ./pasta -t 0 Can't forward port 0 for option '-t 0' $ ./pasta -t 0-1025 Can't forward port 0 for option '-t 0-1025' ...anyway, I doubt anybody would use -t 0 on purpose (to get a port automatically assigned), and while it probably works for TCP (check bound ports after starting pasta, use the assigned one), it wouldn't necessarily work as expected for UDP if the application relies on our flow tracking. For TCP, actually, -t 0 might be useful, see e.g. random_free_port() in Podman tests (/test/system/helpers.network.bash). We should print the port number that was bound, though, and document the feature. More than that: that could actually be the only race-free possibility of picking and forwarding a port where the number doesn't matter. In any case, given that it works by mistake now and it's undocumented, let me go ahead and apply this for the moment. We can add the "functionality" back later if it makes sense. -- Stefano