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=202606 header.b=K9scHTKV; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 2ADE05A026E for ; Tue, 14 Jul 2026 02:05:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1783987509; bh=rgCLvRQrlCcXRLQKJyNyrEYnI1SnNZdxE3rx8V0KKz8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=K9scHTKVAh6em1bD06v9kiEg6V814JoVzrBzg/GaoErCccKoR1C9TuFAnILYfMs3/ eD2vDkoRTDO3M02J3CqwiDMaAk71EigfyodGRz2HATobp0u+bltt2MaMdu7MQ7jUPI fUcdqYpro9+e9z/d+iKW2jsSqt/FEpJz6DB/g7QuqMk1of72f/OUlUdWNFnVFLH4Ap gTYe6Siqxe3e5w+L2URwbJpMgL4kEZZJuo/ccWaGOn2hT4Chfw+wP7b4ASXuHy/JKH mNDnyL8of2OPLJDfKLN0g6KY0DTp4dO7GXNRRGUOZfD4IhYhbS7YxpqurPevS8zzB5 2/CRT+pVuSsNg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gzfh53jT6z4w2F; Tue, 14 Jul 2026 10:05:09 +1000 (AEST) Date: Tue, 14 Jul 2026 10:05:04 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v10] pasta: Refactor pasta_ns_conf() to reduce nesting Message-ID: References: <20260713172653.2821620-1-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="HQoFMe6CBDWstPmO" Content-Disposition: inline In-Reply-To: <20260713172653.2821620-1-jmaloy@redhat.com> Message-ID-Hash: MF5LJAHSFTSMHDFNEHUROANS3IMBOOYX X-Message-ID-Hash: MF5LJAHSFTSMHDFNEHUROANS3IMBOOYX 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: sbrivio@redhat.com, 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: --HQoFMe6CBDWstPmO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 13, 2026 at 01:26:53PM -0400, Jon Maloy wrote: > Extract address and route configuration into two new helper functions > pasta_conf_addrs() and pasta_conf_routes(), and replace the outer > if (c->pasta_conf_ns) block with an early return. This reduces > indentation and eliminates the goto labels. >=20 > No functional change. >=20 > Signed-off-by: Jon Maloy Reviewed-by: David Gibson >=20 > --- > v7: -Removed redundant argument 'af' in nl_addr_set() > -Removed redundant label and 'goto's in pasta_ns_conf() > -Since I excluded addition of all LINKLOCAL addresses from host > address array in a previous commit I can now omit this test > in pasta_conf_addrs() as suggested by David. > -Here is the example of agnostic usage of inany_prefix_len() > I referred to in a previous commit. >=20 > v8: -Do prefix_len conversion inside nl_addr_set() instead of in > the calling function. > -In nl_addr_set(), branch on inany_prefix_v4() return value instead > of inany_af() to avoid redundant AF check and potential static > checker false positives. (David) > -Swap branch order in pasta_conf_routes() for consistency with > pasta_conf_addrs(). (David) > -Defer investigation of pre-existing IFF_NOARP / DAD interaction > oddity noted by David. >=20 > v9: - Broke out as standalone from multi-addr series. > - Adapted to code conditions before the introduction of > the unified address type. >=20 > v10: - Applied all suggestions from David, more for symmetry reasons > than for logical correctess. > --- > pasta.c | 220 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 115 insertions(+), 105 deletions(-) >=20 > diff --git a/pasta.c b/pasta.c > index 4e7ee542..3401fda2 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -303,13 +303,70 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t= gid, > die_perror("Failed to join network namespace"); > } > =20 > +/** > + * pasta_conf_addrs() - Configure addresses for one address family in na= mespace > + * @c: Execution context > + * @af: Address family (AF_INET or AF_INET6) > + * @ifi: Host interface index for this address family > + * @no_copy: If true, set configured address; if false, copy from host > + * > + * Return: 0 on success, negative error code on failure > + */ > +static int pasta_conf_addrs(struct ctx *c, sa_family_t af, int ifi, > + bool no_copy) > +{ > + if (!ifi) > + return 0; > + > + if (!no_copy) > + return nl_addr_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af); > + > + if (af =3D=3D AF_INET && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr)) { > + return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET, > + &c->ip4.addr, c->ip4.prefix_len); > + } else if (af =3D=3D AF_INET6 && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)= ) { > + return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6, > + &c->ip6.addr, 64); > + } > + return 0; > +} > + > +/** > + * pasta_conf_routes() - Configure routes for one address family in name= space > + * @c: Execution context > + * @af: Address family (AF_INET or AF_INET6) > + * @ifi: Host interface index for this address family > + * @no_copy: If true, set default route; if false, copy routes from host > + * > + * Return: 0 on success, negative error code on failure > + */ > +static int pasta_conf_routes(struct ctx *c, sa_family_t af, int ifi, > + bool no_copy) > +{ > + const void *gw; > + > + if (af =3D=3D AF_INET) > + gw =3D &c->ip4.guest_gw; > + else > + gw =3D &c->ip6.guest_gw; > + > + if (!ifi) > + return 0; > + > + if (!no_copy) > + return nl_route_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af); > + > + return nl_route_set_def(nl_sock_ns, c->pasta_ifi, af, gw); > +} > + > /** > * pasta_ns_conf() - Set up loopback and tap interfaces in namespace as = needed > * @c: Execution context > */ > void pasta_ns_conf(struct ctx *c) > { > - int rc =3D 0; > + unsigned int flags =3D IFF_UP; > + int rc; > =20 > rc =3D nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); > if (rc < 0) > @@ -328,116 +385,69 @@ void pasta_ns_conf(struct ctx *c) > die("Couldn't set MAC address in namespace: %s", > strerror_(-rc)); > =20 > - if (c->pasta_conf_ns) { > - unsigned int flags =3D IFF_UP; > - > - if (c->mtu) > - nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); > - > - if (c->ifi6) /* Avoid duplicate address detection on link up */ > - flags |=3D IFF_NOARP; > - > - nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags); > - > - if (c->ifi4) { > - if (c->ip4.no_copy_addrs) { > - rc =3D nl_addr_set(nl_sock_ns, c->pasta_ifi, > - AF_INET, > - &c->ip4.addr, > - c->ip4.prefix_len); > - } else { > - rc =3D nl_addr_dup(nl_sock, c->ifi4, > - nl_sock_ns, c->pasta_ifi, > - AF_INET); > - } > - > - if (c->ifi4 =3D=3D -1 && rc =3D=3D -ENOTSUP) { > - warn("IPv4 not supported, disabling"); > - c->ifi4 =3D 0; > - goto ipv4_done; > - } > - > - if (rc < 0) { > - die("Couldn't set IPv4 address(es) in namespace: %s", > - strerror_(-rc)); > - } > - > - if (c->ip4.no_copy_routes) { > - rc =3D nl_route_set_def(nl_sock_ns, c->pasta_ifi, > - AF_INET, > - &c->ip4.guest_gw); > - } else { > - rc =3D nl_route_dup(nl_sock, c->ifi4, nl_sock_ns, > - c->pasta_ifi, AF_INET); > - } > - > - if (rc < 0) { > - die("Couldn't set IPv4 route(s) in guest: %s", > - strerror_(-rc)); > - } > - } > -ipv4_done: > + proto_update_l2_buf(c->guest_mac); > =20 > - if (c->ifi6) { > - rc =3D nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, > - &c->ip6.addr_ll_seen); > - if (rc < 0) { > - warn("Can't get LL address from namespace: %s", > - strerror_(-rc)); > - } > + if (!c->pasta_conf_ns) > + return; > =20 > - rc =3D nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); > - if (rc < 0) { > - warn("Can't set nodad for LL in namespace: %s", > - strerror_(-rc)); > - } > - > - /* We dodged DAD: re-enable neighbour solicitations */ > - nl_link_set_flags(nl_sock_ns, c->pasta_ifi, > - 0, IFF_NOARP); > - > - if (c->ip6.no_copy_addrs) { > - if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { > - rc =3D nl_addr_set(nl_sock_ns, > - c->pasta_ifi, AF_INET6, > - &c->ip6.addr, 64); > - } > - } else { > - rc =3D nl_addr_dup(nl_sock, c->ifi6, > - nl_sock_ns, c->pasta_ifi, > - AF_INET6); > - } > - > - if (rc < 0) { > - die("Couldn't set IPv6 address(es) in namespace: %s", > - strerror_(-rc)); > - } > - > - if (c->ip6.no_copy_routes) { > - rc =3D nl_route_set_def(nl_sock_ns, c->pasta_ifi, > - AF_INET6, > - &c->ip6.guest_gw); > - } else { > - rc =3D nl_route_dup(nl_sock, c->ifi6, > - nl_sock_ns, c->pasta_ifi, > - AF_INET6); > - } > - > - if (c->ifi6 =3D=3D -1 && rc =3D=3D -ENOTSUP) { > - warn("IPv6 not supported, disabling"); > - c->ifi6 =3D 0; > - goto ipv6_done; > - } > - > - if (rc < 0) { > - die("Couldn't set IPv6 route(s) in guest: %s", > + if (c->mtu) > + nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); > + > + if (c->ifi6) /* Avoid duplicate address detection on link up */ > + flags |=3D IFF_NOARP; > + > + nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags); > + > + if (c->ifi4) { > + rc =3D pasta_conf_addrs(c, AF_INET, c->ifi4, > + c->ip4.no_copy_addrs); > + if (c->ifi4 =3D=3D -1 && rc =3D=3D -ENOTSUP) { > + warn("IPv4 not supported, disabling"); > + c->ifi4 =3D 0; > + } else if (rc < 0) { > + die("Couldn't set IPv4 address(es) in namespace: %s", > + strerror_(-rc)); > + } else { > + rc =3D pasta_conf_routes(c, AF_INET, c->ifi4, > + c->ip4.no_copy_routes); > + if (rc < 0) > + die("Couldn't set IPv4 route(s) in guest: %s", > strerror_(-rc)); > - } > } > } > -ipv6_done: > =20 > - proto_update_l2_buf(c->guest_mac); > + if (c->ifi6) { > + rc =3D nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, > + &c->ip6.addr_ll_seen); > + if (rc < 0) > + warn("Can't get LL address from namespace: %s", > + strerror_(-rc)); > + > + rc =3D nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); > + if (rc < 0) > + warn("Can't set nodad for LL in namespace: %s", > + strerror_(-rc)); > + > + /* We dodged DAD: re-enable neighbour solicitations */ > + nl_link_set_flags(nl_sock_ns, c->pasta_ifi, 0, IFF_NOARP); > + > + rc =3D pasta_conf_addrs(c, AF_INET6, c->ifi6, > + c->ip6.no_copy_addrs); > + if (c->ifi6 =3D=3D -1 && rc =3D=3D -ENOTSUP) { > + warn("IPv6 not supported, disabling"); > + c->ifi6 =3D 0; > + return; > + } else if (rc < 0) { > + die("Couldn't set IPv6 address(es) in namespace: %s", > + strerror_(-rc)); > + } > + > + rc =3D pasta_conf_routes(c, AF_INET6, c->ifi6, > + c->ip6.no_copy_routes); > + if (rc < 0) > + die("Couldn't set IPv6 route(s) in guest: %s", > + strerror_(-rc)); > + } > } > =20 > /** > --=20 > 2.52.0 >=20 --=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 --HQoFMe6CBDWstPmO Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpVfS8ACgkQzQJF27ox 2GdFoA/+LszDzyIJM3PcpHeG9T+g3Dkn4pwtuNXXsvkzw0QLgteTFpl1Uol32SKL vp8eTe0hJw/XzAXo+5452ncFFxmoayQRqj5oUCmE1VOc1ATGUGVSJ7LenHzLUVK0 u0INid6Z7qUiqspZJH6/SItnI5vHj3G4nK4M84CE+S2tGzh2oqwF98VHZ1AwwigJ Qa5izmV+MJHNqfcH/1TkMWImKTNtLs7bW5QqOc5G17reV+dTx/Ps2R16jLPjhSXL dPGnpuZTQ/+eAY3Kx+FiJAF/nqkMz4GxdVjeiFJRXt+5TXtuNDCO0QbgK45gaQPe ukgizRJyBkYrQPch1tmchV0HaBUnyU5G3IbKV6iPJ5QoPmm65IEBka0UozzniN/K j7IHeH/LpWYFF/pcQ59wIf9aJe2uhmgNe05yXxvVQ4ky0uOwWdYfyP5CoMU+vpAx VQu48KNb4SzJbD5yK5lMmjoFDDgABaIR2cVbH1j9IzXYQ9J17/5pvcw5bEhdH3kf yXVQPFhYo2KstSSl/nA+W5GhcbF9q8O6xHVMZYah6et7KhJpIiHXaYs/UdzSPuzt +QWNYygKYNbTjem9WCFFh9PE7L6HaM4Llp7ZZKmUsJrIfISDPIcRqgAhUbkdaKRn FyB7UzIXEAKlCS7t+zHjehhosZq9uPg501SntTGU7l1pGuXhEXY= =1NJE -----END PGP SIGNATURE----- --HQoFMe6CBDWstPmO--