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=J1P7GkFH; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 10E405A026E for ; Wed, 15 Jul 2026 09:11:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1784099490; 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=eBwRfW/KPv/mjdv+1xIUJf7st9ElT22NHxUuJdoU7kU=; b=J1P7GkFHhStMKb/9WDfhid6yhbACH4yB5CLdAIAlSctCHUwsBA2YSN8ul8hogQw29kthan we5fKMGdgSu1P9X8wjoSHCQWkNWMm6KbT7sDCmokeGyhiyhI+lGi2vCbn5ZSw0go6VaMv1 NMIo25iDQsq5CTfhjWWiT/ft4SsB8r8= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-435-_jRV_0TPO9muRgiJm16iDA-1; Wed, 15 Jul 2026 03:11:29 -0400 X-MC-Unique: _jRV_0TPO9muRgiJm16iDA-1 X-Mimecast-MFC-AGG-ID: _jRV_0TPO9muRgiJm16iDA_1784099488 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-493dc8408fdso14759625e9.1 for ; Wed, 15 Jul 2026 00:11:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784099488; x=1784704288; h=date:content-transfer-encoding:content-type:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=eBwRfW/KPv/mjdv+1xIUJf7st9ElT22NHxUuJdoU7kU=; b=P1+vBoi3zfCeqntDw7jTGXJI+Rr4VI8cEMhFHCNsRG1SCnw98aJX4V0oy4dAfCTFQ7 1A4UqYtcJogUtukYS8uxxIkV0Ixt+1pDCxEAnBqxUZS2LyqY6dR+m0bXkXlrfNu+mK42 bNbDl4Ds9XKE/kAGC+YLUxIhEMRg8aUr/+IuELpTCjUo32HL4ZRa4w94SBBKX3dJPB6v PLfsPqKWUSlcJ6F32+PbUQ06c+MrI2YkOSUHPH/HakqGesBKwsLYj+CQUmp59NgOzrS/ X1UzToVzynkPRclq0fP+1WKiFc+zE8KG9P8SdupIqfVocphACGBjsM8dXHwxkdlNFkVw vT+Q== X-Forwarded-Encrypted: i=1; AHgh+Rpo7SPu1HqSPOac2erplgp+/LLR0dRba7beJvA9Vmk6Jmrea4KeWHtFUxfbLkMby78FCwCkfhhzhgQ=@passt.top X-Gm-Message-State: AOJu0Ywl8b3AYjx+RFgb1h0qHPitqVthB8UVUsq1WJEbmyC3SgTKlWBS K7fo4Qv3PQxOWEwalTOrap473pXfcN0W2VGFpx/yCH0IXm1ihgOSAvh1UYAGg9QZGCM+iq/1n+3 eg/s6SyH2aGFgdXZV70fAnkjvgzdBqGtP8UhCVU+WKq3T/HYKWkkjFA== X-Gm-Gg: AfdE7cnocpaZyhtrc5GJ9yu2vTGeJCHLWNTV8xk/9lTNFsANiNOdZeJ/R80sb8VCBsW ZEuX/fe46Dgjmm3DCKhizVS3oadbYUX/POVYev0KxuqakgdTxlotm5BhciUmQ0ApQ+r2wX8viK0 ErXvqjpPv2swAFQmjSwBq5nzO6pqSIic2Ezn1RhBpJYhjsOpW7TirJ8RaNoYMeIkvJVB1e6RQoU G4t6dpC6PcsuIR/STu3Jvjjg8Ep3rpmVkwRxLAF0KZJL7JRT88J9xJw5cZhw6MkRoQBb+ZV04ys F8k0xoq6yl8fE+Ol3E5DB/DNYMHT5Bm0nkRAt/a2oNhLBB2b8CuXOJZnXWyeTNlYTmraadeGJO0 ztmuH3JVUKKch+7mfZsa10w== X-Received: by 2002:a05:600c:e557:20b0:493:f822:ac5e with SMTP id 5b1f17b1804b1-493f8818052mr123377275e9.23.1784099487575; Wed, 15 Jul 2026 00:11:27 -0700 (PDT) X-Received: by 2002:a05:600c:e557:20b0:493:f822:ac5e with SMTP id 5b1f17b1804b1-493f8818052mr123376975e9.23.1784099487041; Wed, 15 Jul 2026 00:11:27 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4950a2edd6fsm128997185e9.11.2026.07.15.00.11.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Jul 2026 00:11:25 -0700 (PDT) From: Stefano Brivio To: Jon Maloy Subject: Re: [PATCH v10] pasta: Refactor pasta_ns_conf() to reduce nesting Message-ID: <20260715091124.410d2ed6@elisabeth> In-Reply-To: <20260713172653.2821620-1-jmaloy@redhat.com> References: <20260713172653.2821620-1-jmaloy@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 Date: Wed, 15 Jul 2026 09:11:25 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: lempWnOd6i0AWG-AFjo7VDzuClZKpkkgLaVQ37cC5aU_1784099488 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: HFNDRSLK6ZFM3MBGNIPPZ7WXWQYMPR5K X-Message-ID-Hash: HFNDRSLK6ZFM3MBGNIPPZ7WXWQYMPR5K 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: david@gibson.dropbear.id.au, 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 Mon, 13 Jul 2026 13:26:53 -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. > > No functional change. > > Signed-off-by: Jon Maloy > > --- > 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. > > v10: - Applied all suggestions from David, more for symmetry reasons > than for logical correctess. > --- > pasta.c | 220 +++++++++++++++++++++++++++++--------------------------- > 1 file changed, 115 insertions(+), 105 deletions(-) > > 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"); > } > > +/** > + * 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 && !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 == AF_INET6 && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { else-after-return here (also reported by clang-tidy, try 'make clang-tidy') and... > + return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6, > + &c->ip6.addr, 64); > + } (an extra newline here would be nice here for readability) > + 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 +385,69 @@ 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: > + proto_update_l2_buf(c->guest_mac); > > - 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)); > - } > + if (!c->pasta_conf_ns) > + return; > > - 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", > + 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) { > + 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 { > + 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) { > + 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) { ...here, too. > + 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)); > + } > } > > /** -- Stefano