From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202510 header.b=UQWkKBN4; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 025835A061A for ; Mon, 20 Oct 2025 11:25:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1760952312; bh=a2V0jhLUxSItkV8LGJ2qaO5iVNX9nBehJM1VMauCCE4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UQWkKBN4us5yJIsS8v2GW7b0INOr3XRPKhcdY6tOjJr8ftYRuJ2/42U0qI2MA1vgY yib/d2qgcuV7VTv3BtTgTyLFYAgDA4ejqc6JVC9wlAES4H4RSq+AAySeF8GGd3D0Ju vVlnE9fD30MX/jZZkqx2ONVInY8ZioHX7PzzkmZUPKLvtTfCBpfOsW7CZ0Vhon331n vr7rN60/lTyG6DPcWjE8oy7wYuLgwGdb6T6O+wu45hobhsd4N6Dh5mQfZjwHTg5tRr d9b8HB6Dxe4nKDDiCKTMRTd43mxl0QZtxJX21AL29WPeOQw7pRn37JuAW34TLS7/sc xbn1GpdR6BSEA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cqqlX6q6fz4wBB; Mon, 20 Oct 2025 20:25:12 +1100 (AEDT) Date: Mon, 20 Oct 2025 20:24:53 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] tcp: Merge tcp_ns_sock_init[46]() into tcp_sock_init_one() Message-ID: References: <20251017003447.414103-1-david@gibson.dropbear.id.au> <20251017003447.414103-2-david@gibson.dropbear.id.au> <20251020080839.0b4d4f82@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Ge0i2vG5u6u+OpYc" Content-Disposition: inline In-Reply-To: <20251020080839.0b4d4f82@elisabeth> Message-ID-Hash: DA7HAADZC7P2HZDFJCLBDU4ZCLKTKNLN X-Message-ID-Hash: DA7HAADZC7P2HZDFJCLBDU4ZCLKTKNLN X-MailFrom: dgibson@gandalf.ozlabs.org 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: --Ge0i2vG5u6u+OpYc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 20, 2025 at 08:08:39AM +0200, Stefano Brivio wrote: > On Fri, 17 Oct 2025 11:34:45 +1100 > David Gibson wrote: >=20 > > Surprisingly little logic is shared between the path for creating a > > listen()ing socket in the guest namespace versus in the host namespace. > > Improve this, by extending tcp_sock_init_one() to take a pif parameter > > indicating where it should open the socket. This allows > > tcp_ns_sock_init[46]() to be removed entirely. > >=20 > > We generalise tcp_sock_init() in the same way, although we don't use it > > yet, due to some subtle differences in how we bind for -t versus -T. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 2 +- > > tcp.c | 96 ++++++++++++++++++---------------------------------------- > > tcp.h | 5 +-- > > 3 files changed, 33 insertions(+), 70 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 66b9e634..26f1bcc0 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -169,7 +169,7 @@ static void conf_ports_range_except(const struct ct= x *c, char optname, > > fwd->delta[i] =3D to - first; > > =20 > > if (optname =3D=3D 't') > > - ret =3D tcp_sock_init(c, addr, ifname, i); > > + ret =3D tcp_sock_init(c, PIF_HOST, addr, ifname, i); > > else if (optname =3D=3D 'u') > > ret =3D udp_sock_init(c, 0, addr, ifname, i); > > else > > diff --git a/tcp.c b/tcp.c > > index 0f9e9b3f..15c012d7 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -2515,29 +2515,38 @@ void tcp_sock_handler(const struct ctx *c, unio= n epoll_ref ref, > > /** > > * tcp_sock_init_one() - Initialise listening socket for address and p= ort > > * @c: Execution context > > + * @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE) > > * @addr: Pointer to address for binding, NULL for dual stack any > > * @ifname: Name of interface to bind to, NULL if not configured > > * @port: Port, host order > > * > > * Return: fd for the new listening socket, negative error code on fai= lure > > + * > > + * If pif =3D=3D PIF_SPLICE, must have already entered the namespace. > > */ > > -static int tcp_sock_init_one(const struct ctx *c, const union inany_ad= dr *addr, > > - const char *ifname, in_port_t port) > > +static int tcp_sock_init_one(const struct ctx *c, uint8_t pif, > > + const union inany_addr *addr, const char *ifname, > > + in_port_t port) > > { > > + const struct fwd_ports *fwd =3D pif =3D=3D PIF_HOST ? > > + &c->tcp.fwd_in : &c->tcp.fwd_out; >=20 > While I appreciate the resulting brevity, I wonder if it would make > more sense to have this as an explicit if / else clause, for > readability. Same for similar occurrences in the next patches (which I > didn't fully review, yet). >=20 > Another alternative is: >=20 > const struct fwd_ports *fwd; >=20 > fwd =3D (pif =3D=3D PIF_HOST) ? &c->tcp.fwd_in : &c->tcp.fwd_out; >=20 > ...still two lines of code, perhaps just slightly less readable than > the five obvious ones: >=20 > const struct fwd_ports *fwd; >=20 > if (pif =3D=3D PIF_HOST) > fwd =3D &c->tcp.fwd_in; > else > fwd =3D &c->tcp.fwd_out; Good point. I suspect this will be shuffled again in later patches, but I might as well go with the less obfuscated version in the meantime. --=20 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 --Ge0i2vG5u6u+OpYc Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj1/+QACgkQzQJF27ox 2Gf6hBAAiDX8m7Z9FR5JCt7KUe/OdMLiQ1y8mRNHom2gUxwPykUdm0x3f/PThM1v SupcAe11g86MVdHTN7oCfS+J30EEDeXlVWflxBuyU7e42h8+5lK3QW4fV9E+vrOp sD+su2b7Ht8SKhnzPtK32DkE3543AZreSIw5PRmnQAo078xju9upfBG4ptpv8j2i t0AK/7qUw0+s1ZBXsy7bvTR2S8hLKohNMZP9NZBu1mFXN7/l5402YUbjWotZG8JN x7Zha6n5MDPkUHParUFnAibCWoCBsrVo7KO9pA5l8sGZvhX10zNjrmn18cbw3Svu 4M+Q27UltGYtxIY8dm8PfT8BzrInfMcUjmxp1BsWbMEZsWhCFOYkYr1nQ9lcZVdP dsuOuzlb24MWA3fsS4Njjj7lDTvr52b7SWGUmjK9rIqFJ3z/pFZNds6RTmcFn1Xn dFNSnNnJueD090ZwMLRUjSqSDoYgw3r3TMrxTCZ4V9q5jxxUuR79DqbbJ5/5KPgx ZvndGU3oot8Uh9hecRsrEi/C8H4uUbeGLwLDwx55ZoRkLn5kYk7Nt93HGZsx72MH 5kjf1ntaIDPRn8PKIxzIqzRe7oWMw52Ir1VcLYIP199Q5k2IqYrDZLip830fbFw2 K80Y3ee7fCbvFwEnE3Ukt0jZ5HVg8zgWzniHnXH5sC470UG1rMI= =DWgt -----END PGP SIGNATURE----- --Ge0i2vG5u6u+OpYc--