* [PATCH v9] pasta: Refactor pasta_ns_conf() to reduce nesting
@ 2026-07-10 23:53 Jon Maloy
2026-07-13 4:29 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Jon Maloy @ 2026-07-10 23:53 UTC (permalink / raw)
To: sbrivio, david, jmaloy, passt-dev
We 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.
There is no functional change in this commit.
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
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.
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.
v9: - Broke out as standalone from multi-addr series.
- Adapted to code conditions before the introduction of
the unified address type.
---
pasta.c | 226 +++++++++++++++++++++++++++++---------------------------
1 file changed, 117 insertions(+), 109 deletions(-)
diff --git a/pasta.c b/pasta.c
index 4e7ee542..fa9a341e 100644
--- a/pasta.c
+++ b/pasta.c
@@ -303,13 +303,72 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
die_perror("Failed to join network namespace");
}
+/**
+ * pasta_conf_addrs() - Configure addresses for one address family in namespace
+ * @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 == AF_INET) {
+ return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET,
+ &c->ip4.addr, c->ip4.prefix_len);
+ }
+
+ if (!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 namespace
+ * @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 == AF_INET)
+ gw = &c->ip4.guest_gw;
+ else
+ gw = &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 = 0;
+ unsigned int flags = IFF_UP;
+ int rc;
rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP);
if (rc < 0)
@@ -328,116 +387,65 @@ void pasta_ns_conf(struct ctx *c)
die("Couldn't set MAC address in namespace: %s",
strerror_(-rc));
- if (c->pasta_conf_ns) {
- unsigned int flags = 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 |= IFF_NOARP;
-
- nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
-
- if (c->ifi4) {
- if (c->ip4.no_copy_addrs) {
- rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
- AF_INET,
- &c->ip4.addr,
- c->ip4.prefix_len);
- } else {
- rc = nl_addr_dup(nl_sock, c->ifi4,
- nl_sock_ns, c->pasta_ifi,
- AF_INET);
- }
-
- if (c->ifi4 == -1 && rc == -ENOTSUP) {
- warn("IPv4 not supported, disabling");
- c->ifi4 = 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 = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET,
- &c->ip4.guest_gw);
- } else {
- rc = 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:
-
- if (c->ifi6) {
- rc = 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 = 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 = nl_addr_set(nl_sock_ns,
- c->pasta_ifi, AF_INET6,
- &c->ip6.addr, 64);
- }
- } else {
- rc = 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 = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
- AF_INET6,
- &c->ip6.guest_gw);
- } else {
- rc = nl_route_dup(nl_sock, c->ifi6,
- nl_sock_ns, c->pasta_ifi,
- AF_INET6);
- }
-
- if (c->ifi6 == -1 && rc == -ENOTSUP) {
- warn("IPv6 not supported, disabling");
- c->ifi6 = 0;
- goto ipv6_done;
- }
-
- if (rc < 0) {
- die("Couldn't set IPv6 route(s) in guest: %s",
- strerror_(-rc));
- }
- }
+ proto_update_l2_buf(c->guest_mac);
+
+ if (!c->pasta_conf_ns)
+ return;
+
+ 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 |= IFF_NOARP;
+
+ nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
+
+ rc = pasta_conf_addrs(c, AF_INET, c->ifi4, c->ip4.no_copy_addrs);
+ if (c->ifi4 == -1 && rc == -ENOTSUP) {
+ warn("IPv4 not supported, disabling");
+ c->ifi4 = 0;
+ } else if (rc < 0) {
+ die("Couldn't set IPv4 address(es) in namespace: %s",
+ strerror_(-rc));
+ } else if (c->ifi4) {
+ rc = 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:
- proto_update_l2_buf(c->guest_mac);
+ if (!c->ifi6)
+ return;
+
+ rc = 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 = 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 = pasta_conf_addrs(c, AF_INET6, c->ifi6, c->ip6.no_copy_addrs);
+ if (c->ifi6 == -1 && rc == -ENOTSUP) {
+ warn("IPv6 not supported, disabling");
+ c->ifi6 = 0;
+ return;
+ } else if (rc < 0) {
+ die("Couldn't set IPv6 address(es) in namespace: %s",
+ strerror_(-rc));
+ }
+
+ rc = 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));
}
/**
--
2.52.0
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH v9] pasta: Refactor pasta_ns_conf() to reduce nesting
2026-07-10 23:53 [PATCH v9] pasta: Refactor pasta_ns_conf() to reduce nesting Jon Maloy
@ 2026-07-13 4:29 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2026-07-13 4:29 UTC (permalink / raw)
To: Jon Maloy; +Cc: sbrivio, passt-dev
[-- Attachment #1: Type: text/plain, Size: 9626 bytes --]
On Fri, Jul 10, 2026 at 07:53:09PM -0400, Jon Maloy wrote:
> We 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.
>
> There is no functional change in this commit.
>
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
On the whole looks much nicer, but a couple of comments below.
> ---
> 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.
>
> 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.
>
> v9: - Broke out as standalone from multi-addr series.
> - Adapted to code conditions before the introduction of
> the unified address type.
> ---
> pasta.c | 226 +++++++++++++++++++++++++++++---------------------------
> 1 file changed, 117 insertions(+), 109 deletions(-)
>
> diff --git a/pasta.c b/pasta.c
> index 4e7ee542..fa9a341e 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -303,13 +303,72 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid,
> die_perror("Failed to join network namespace");
> }
>
> +/**
> + * pasta_conf_addrs() - Configure addresses for one address family in namespace
> + * @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 == AF_INET) {
I think this also needs to check !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);
> + }
> +
> + if (!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 namespace
> + * @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 == AF_INET)
> + gw = &c->ip4.guest_gw;
> + else
> + gw = &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 = 0;
> + unsigned int flags = IFF_UP;
> + int rc;
>
> rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP);
> if (rc < 0)
> @@ -328,116 +387,65 @@ void pasta_ns_conf(struct ctx *c)
> die("Couldn't set MAC address in namespace: %s",
> strerror_(-rc));
>
> - if (c->pasta_conf_ns) {
> - unsigned int flags = 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 |= IFF_NOARP;
> -
> - nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
> -
> - if (c->ifi4) {
> - if (c->ip4.no_copy_addrs) {
> - rc = nl_addr_set(nl_sock_ns, c->pasta_ifi,
> - AF_INET,
> - &c->ip4.addr,
> - c->ip4.prefix_len);
> - } else {
> - rc = nl_addr_dup(nl_sock, c->ifi4,
> - nl_sock_ns, c->pasta_ifi,
> - AF_INET);
> - }
> -
> - if (c->ifi4 == -1 && rc == -ENOTSUP) {
> - warn("IPv4 not supported, disabling");
> - c->ifi4 = 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 = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
> - AF_INET,
> - &c->ip4.guest_gw);
> - } else {
> - rc = 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:
> -
> - if (c->ifi6) {
> - rc = 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 = 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 = nl_addr_set(nl_sock_ns,
> - c->pasta_ifi, AF_INET6,
> - &c->ip6.addr, 64);
> - }
> - } else {
> - rc = 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 = nl_route_set_def(nl_sock_ns, c->pasta_ifi,
> - AF_INET6,
> - &c->ip6.guest_gw);
> - } else {
> - rc = nl_route_dup(nl_sock, c->ifi6,
> - nl_sock_ns, c->pasta_ifi,
> - AF_INET6);
> - }
> -
> - if (c->ifi6 == -1 && rc == -ENOTSUP) {
> - warn("IPv6 not supported, disabling");
> - c->ifi6 = 0;
> - goto ipv6_done;
> - }
> -
> - if (rc < 0) {
> - die("Couldn't set IPv6 route(s) in guest: %s",
> - strerror_(-rc));
> - }
> - }
> + proto_update_l2_buf(c->guest_mac);
> +
> + if (!c->pasta_conf_ns)
> + return;
> +
> + 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 |= IFF_NOARP;
> +
> + nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags);
> +
I realise it puts some of the nesting back in, but I think you still
need the if (c->ifi4) around the next chunk - a configuration with
only IPv6 configured is certainly supported.
> + rc = pasta_conf_addrs(c, AF_INET, c->ifi4, c->ip4.no_copy_addrs);
> + if (c->ifi4 == -1 && rc == -ENOTSUP) {
> + warn("IPv4 not supported, disabling");
> + c->ifi4 = 0;
> + } else if (rc < 0) {
> + die("Couldn't set IPv4 address(es) in namespace: %s",
> + strerror_(-rc));
> + } else if (c->ifi4) {
> + rc = 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:
>
> - proto_update_l2_buf(c->guest_mac);
> + if (!c->ifi6)
> + return;
For symmetry with IPv4 I'd put this chunk within the if, rather than
using an early return here.
> +
> + rc = 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 = 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 = pasta_conf_addrs(c, AF_INET6, c->ifi6, c->ip6.no_copy_addrs);
> + if (c->ifi6 == -1 && rc == -ENOTSUP) {
> + warn("IPv6 not supported, disabling");
> + c->ifi6 = 0;
> + return;
> + } else if (rc < 0) {
> + die("Couldn't set IPv6 address(es) in namespace: %s",
> + strerror_(-rc));
> + }
> +
> + rc = 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));
> }
>
> /**
> --
> 2.52.0
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-13 5:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-10 23:53 [PATCH v9] pasta: Refactor pasta_ns_conf() to reduce nesting Jon Maloy
2026-07-13 4:29 ` David Gibson
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).