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=YzZBer09; 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 AF7535A0262 for ; Fri, 10 Jul 2026 23:31:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1783719089; 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=BjA/2XDWvh5a1hzsi9koj2CIzz7TfmQAN1e1dCiBzJc=; b=YzZBer09VQUud126y43pQfXpxr3QjmMOEMyymMec2uDsNFiTyRrbMid9XU5ZEiDImbrUHF pxG+vsd20GP2NBBa36dPCd7TT8PTmCOKgdz56wjlGez/fqq9kujilwTkoiJ3NS3k6PLfsJ 2oBiv6zYoDDLc5iW9qNpqxjnhoN4FpM= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-351-h-fd0QYHOZmsdMucUW-dgQ-1; Fri, 10 Jul 2026 17:31:27 -0400 X-MC-Unique: h-fd0QYHOZmsdMucUW-dgQ-1 X-Mimecast-MFC-AGG-ID: h-fd0QYHOZmsdMucUW-dgQ_1783719086 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-51c21c01cf3so25787501cf.2 for ; Fri, 10 Jul 2026 14:31:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783719086; x=1784323886; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=BjA/2XDWvh5a1hzsi9koj2CIzz7TfmQAN1e1dCiBzJc=; b=GYTPLN0CVCv3m1DjPPbmFTo8yRzQ1BhcOB/Wjpanfvf5TRROLr4FCCS51BkPxyUVo+ hddpFrJa6CPOMg4s93PqQewbBWfzc85S2O01kTGEzS+hu/LQxgk2JUa24Wv7e3XLDUB+ zSN65Lt7pAqzwDyVTAYSmNaW6rDHAojE+IgTOihovIGD/tvvhDjZVR4ng7I1goUW9umo holhYsS3JrvGGS6tJf8yKecnYmFRNT5tNswCwC+QZBc8H7DjuE0PlX6exByHX94k+pyu o30kLajnMvvTKIWcy+IRQCvDU67F0rvh/1oOX2apcQvvYfIel045taO9ROTbu+pEzRR+ kxXQ== X-Forwarded-Encrypted: i=1; AHgh+Rrp+C57viiSG0LWnfb768XuEOkibMyrssJtXSJYZ7J6b9b5zOL3ZmtSVmcc7bhY0mUYyell+Qwod7M=@passt.top X-Gm-Message-State: AOJu0YxlaK+WUGxlziSuWSRwPdGYSONNE7kyQ9wQI31gq0/mzKgWYqHZ 2X+rPS+Bjnz7AA4X8iIwmpjXSQpyt5lbZ5SyuNzz97Mu7TMOh5sOFwsCr7PhiYvu0peQcc3I2hg mRWyOL0VT75ducXyPyUdt6+ye/9zoMBqwz32nt7cpWx+k1CLB1OpNvg== X-Gm-Gg: AfdE7cl0E5qWbHbThUaLeuU4GGzAXKsIMm5qjmOtOPQpeqPpYIAb8UyQEXEMjdM++Jd PXwg91LlTDqjuUazeY0jhBg+223WgvTsmK8HtTaJBwcz6vGYF2SSRfEcevVHMJQ2vsayJl8icpW DN5ICvTE4GkMM5R8GQKW049rGZcs8ADYfD4iVGPInXVrxE6xd291naTpeVhIxMnMJR5MZR4LDHk AR806wfgNSi5sza1Vim6r7QKByn9+Eoo3QIX9fYrsitSwNi/RTO3WeI/p2ne5uVqlesBZW7kDxh vKM/clDcKpA4ZvWPHjhJZMUfMmFxLklXkF9V7VBQsvJ5pZnTYW5ou07Zja1+a/H3gZLomjCpL5c 4wXt1N5wuIjdrsax0RlisfDwnhA2waiIDwGcYymxWztx7sk8+eMwESRD1nXg8FI2umxgYbrnA6F +Di4w= X-Received: by 2002:a05:622a:1909:b0:517:c6b1:bffa with SMTP id d75a77b69052e-51cbf2778c0mr7126841cf.48.1783719085891; Fri, 10 Jul 2026 14:31:25 -0700 (PDT) X-Received: by 2002:a05:622a:1909:b0:517:c6b1:bffa with SMTP id d75a77b69052e-51cbf2778c0mr7126471cf.48.1783719085062; Fri, 10 Jul 2026 14:31:25 -0700 (PDT) Received: from [192.168.2.15] (lnsm6-montreal02-142-122-43-198.internet.virginmobile.ca. [142.122.43.198]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-51caafd8b44sm24345861cf.30.2026.07.10.14.31.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 10 Jul 2026 14:31:24 -0700 (PDT) Message-ID: <769f4409-c317-4736-98b3-6fe8f6d4078d@redhat.com> Date: Fri, 10 Jul 2026 17:31:23 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 02/14] passt, pasta: Introduce unified multi-address data structures To: David Gibson References: <20260626024519.3701556-1-jmaloy@redhat.com> <20260626024519.3701556-3-jmaloy@redhat.com> From: Jon Maloy In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: I6MzBaEmhPsfs-yigKFQfZBngOpEPw2hVwa__0vXA7U_1783719086 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: FPKHIR6HAR2OZGBQ4ZWX52PCPO2JBBS5 X-Message-ID-Hash: FPKHIR6HAR2OZGBQ4ZWX52PCPO2JBBS5 X-MailFrom: jmaloy@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: 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: On 2026-07-09 00:05, David Gibson wrote: > On Thu, Jun 25, 2026 at 10:45:07PM -0400, Jon Maloy wrote: >> As preparation for supporting multiple addresses per interface, >> we replace the single addr/prefix_len fields with an array. The >> array consists of a new struct inany_addr_entry containing an >> address and prefix length, both in inany_addr format. >> >> Despite some code refactoring, there are only two real functional >> changes: >> - The indicated IPv6 prefix length is now properly stored, instead >> of being ignored and overridden with the hardcoded value 64, as >> has been the case until now. >> - Since even IPv4 addresses now are stored in IPv6 format, we >> also store the corresponding prefix length in that format, >> i.e. using the range [96,128] instead of [0,32]. >> >> In conf_ip6(), the explicit IN6_IS_ADDR_UNSPECIFIED() check on the >> address at the end of the function is no longer needed: for >> host-discovered addresses, it is now checked inside the if (!a) block >> before calling fwd_set_addr(); for user-provided addresses (via -a), >> validation already rejects unspecified addresses at parse time. >> >> Signed-off-by: Jon Maloy >> >> --- >> v2: -Using inany_addr instead of protocol specific addresses as >> entry address field. >> >> v3: -Merging into one array, directly in struct ctx >> -Changed prefix_len and flags fields in struct inany_addr_entry >> to uint8_t, since that makes the struct directly migratable >> >> v4: -Updated according to changes in previous commits >> -Updated according to feedback from David G. >> -Squashed IP4_MASK macro commit into this one >> >> v6: -Renamed and moved some definitions >> -Introduced fwd_set_addr() and fwd_get_addr() already in this commit >> -Eliminated first_v4/v6() functions, replaced with fwd_get_addr() >> -Some other changes as suggested by David G. >> -I kept the flag CONF_ADDR_LINKLOCAL, since it will be >> needed later in an address selection function. >> >> v7: -Introduced CONF_ADDR_GENERATED flag >> -Other fixes based on feedback from David and Stefano. >> -I changed signature of inany_prefix_len(), but I did not change >> its semantics, since the premise of David's comment is wrong: the >> caller does *not* explicitly know he is dealing with an IPv4 address. >> In fact, there are examples later in this series where it may be an >> IPv6 address, and the caller just trusts he gets the return value in >> the appropriate format. >> -Intoduced the inverse of inany_prefix_len(), called inany_prefix_len6() >> which always returns the prefix in IPv6 or mapped IPv4 format. >> The name of the function isn't great, but any alternative I came up >> with became too long to be practical. >> >> v8: -Added inany_prefix_v4() (David G.) to extract IPv4 address and >> prefix length simultaneously, replacing inany_prefix_len(). >> -Removed both inany_prefix_len() and inany_prefix_len6() >> auto-detecting functions. Prefix length attached to an inany is >> now always stored in IPv6 format (96-128 for IPv4-mapped). >> -fwd_set_addr() now requires prefix_len in IPv6 format; callers >> updated to pass prefix_len + 96 for IPv4 addresses. >> -Fixed -n option to store prefix_len in IPv6 format. >> -Fixed addr_seen regression: when address was set via -a, >> addr_seen was not initialised in conf_ip4(). >> -Replaced assert() with graceful no-op in dhcp.c and ndp.c when >> no address is configured (David G.). >> -Note: addr_seen is still initialised both at conf_ip*() time >> and at DHCP/DHCPv6 send time (David's concern). This >> inconsistency will be addressed in a follow-up. >> -Introduced a new CONF_ADDR_ANY macro tobe used for address >> selection, as suggested by Stefano. >> -Deferred introduction of an explicit scope field (replacing >> CONF_ADDR_LINKLOCAL flag) to a later commit or series. >> --- >> arp.c | 12 ++++- >> conf.c | 142 +++++++++++++++++++++++++++++++------------------------ >> dhcp.c | 19 ++++++-- >> dhcpv6.c | 15 ++++-- >> fwd.c | 111 +++++++++++++++++++++++++++++++++++-------- >> fwd.h | 4 ++ >> inany.h | 28 +++++++++++ >> ip.h | 2 + >> ndp.c | 19 ++++++-- >> passt.h | 66 +++++++++++++++++++++++--- >> pasta.c | 26 ++++++---- >> tap.c | 8 +++- >> 12 files changed, 342 insertions(+), 110 deletions(-) >> >> diff --git a/arp.c b/arp.c >> index bb042e95..0392861d 100644 >> --- a/arp.c >> +++ b/arp.c >> @@ -41,6 +41,8 @@ >> static bool ignore_arp(const struct ctx *c, >> const struct arphdr *ah, const struct arpmsg *am) >> { >> + const struct guest_addr *a; >> + >> if (ah->ar_hrd != htons(ARPHRD_ETHER) || >> ah->ar_pro != htons(ETH_P_IP) || >> ah->ar_hln != ETH_ALEN || >> @@ -54,7 +56,8 @@ static bool ignore_arp(const struct ctx *c, >> return true; >> >> /* Don't resolve the guest's assigned address, either. */ >> - if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if (a && !memcmp(am->tip, inany_v4(&a->addr), sizeof(am->tip))) >> return true; >> >> return false; >> @@ -123,12 +126,17 @@ int arp(const struct ctx *c, struct iov_tail *data) >> */ >> void arp_send_init_req(const struct ctx *c) >> { >> + const struct guest_addr *a; >> struct { >> struct ethhdr eh; >> struct arphdr ah; >> struct arpmsg am; >> } __attribute__((__packed__)) req; >> >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if (!a) >> + return; >> + >> /* Ethernet header */ >> req.eh.h_proto = htons(ETH_P_ARP); >> memcpy(req.eh.h_dest, MAC_BROADCAST, sizeof(req.eh.h_dest)); >> @@ -145,7 +153,7 @@ void arp_send_init_req(const struct ctx *c) >> memcpy(req.am.sha, c->our_tap_mac, sizeof(req.am.sha)); >> memcpy(req.am.sip, &c->ip4.our_tap_addr, sizeof(req.am.sip)); >> memcpy(req.am.tha, MAC_BROADCAST, sizeof(req.am.tha)); >> - memcpy(req.am.tip, &c->ip4.addr, sizeof(req.am.tip)); >> + memcpy(req.am.tip, inany_v4(&a->addr), sizeof(req.am.tip)); >> >> debug("Sending initial ARP request for guest MAC address"); >> tap_send_single(c, &req, sizeof(req)); >> diff --git a/conf.c b/conf.c >> index 4755a9f4..ac8facc8 100644 >> --- a/conf.c >> +++ b/conf.c >> @@ -373,13 +373,16 @@ static int conf_ip4_prefix(const char *arg) >> >> /** >> * conf_ip4() - Verify or detect IPv4 support, get relevant addresses >> + * @c: Execution context >> * @ifi: Host interface to attempt (0 to determine one) >> - * @ip4: IPv4 context (will be written) >> * >> * Return: interface index for IPv4, or 0 on failure. >> */ >> -static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) >> +static unsigned int conf_ip4(struct ctx *c, unsigned int ifi) >> { >> + const struct guest_addr *a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + struct ip4_ctx *ip4 = &c->ip4; >> + >> if (!ifi) >> ifi = nl_get_ext_if(nl_sock, AF_INET); >> >> @@ -398,60 +401,58 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) >> } >> } >> >> - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) { >> + if (!a) { >> + struct in_addr addr; >> + int prefix_len; >> int rc = nl_addr_get(nl_sock, ifi, AF_INET, >> - &ip4->addr, &ip4->prefix_len, NULL); >> + &addr, &prefix_len, NULL); >> if (rc < 0) { >> debug("Couldn't discover IPv4 address: %s", >> strerror_(-rc)); >> return 0; >> } >> - } >> + if (IN4_IS_ADDR_UNSPECIFIED(&addr)) >> + return 0; > > Nit: If we get here, the netlink probe succeeded, and the host > apparently has an address of 0.0.0.0. That's pretty strange, and > probably warrants a warn(). ok. > >> >> - if (!ip4->prefix_len) { >> - in_addr_t addr = ntohl(ip4->addr.s_addr); >> - if (IN_CLASSA(addr)) >> - ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); >> - else if (IN_CLASSB(addr)) >> - ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); >> - else if (IN_CLASSC(addr)) >> - ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); >> - else >> - ip4->prefix_len = 32; >> + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, >> + prefix_len + 96); >> } >> >> - ip4->addr_seen = ip4->addr; >> - >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); > > Nit: maybe assert(a), since I doubt static checkers will be able to > reason that the fwd_set_addr() above means this fwd_get_addr() can't > fail. Also, I think that go inside the if block, since the other case > means we already have a perfectly good a. I think my conclusion was that the static checker ignores assert() as a conditonal test, but I can try it. > >> + ip4->addr_seen = *inany_v4(&a->addr); >> ip4->our_tap_addr = ip4->guest_gw; >> >> - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) >> - return 0; >> - >> return ifi; >> } >> >> /** >> * conf_ip4_local() - Configure IPv4 addresses and attributes for local mode >> - * @ip4: IPv4 context (will be written) >> + * @c: Execution context (will be written) >> */ >> -static void conf_ip4_local(struct ip4_ctx *ip4) >> +static void conf_ip4_local(struct ctx *c) >> { >> - ip4->addr_seen = ip4->addr = IP4_LL_GUEST_ADDR; >> - ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW; >> - ip4->prefix_len = IP4_LL_PREFIX_LEN; >> + struct ip4_ctx *ip4 = &c->ip4; >> >> + ip4->addr_seen = IP4_LL_GUEST_ADDR; >> + ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW; >> ip4->no_copy_addrs = ip4->no_copy_routes = true; >> + fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), >> + CONF_ADDR_GENERATED | CONF_ADDR_LINKLOCAL, >> + IP4_LL_PREFIX_LEN + 96); >> } >> >> /** >> * conf_ip6() - Verify or detect IPv6 support, get relevant addresses >> + * @c: Execution context >> * @ifi: Host interface to attempt (0 to determine one) >> - * @ip6: IPv6 context (will be written) >> * >> * Return: interface index for IPv6, or 0 on failure. >> */ >> -static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) >> +static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) >> { >> + struct ip6_ctx *ip6 = &c->ip6; >> + const struct guest_addr *a; >> + union inany_addr addr; >> int prefix_len = 0; >> int rc; >> >> @@ -472,21 +473,28 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) >> } >> } >> >> - rc = nl_addr_get(nl_sock, ifi, AF_INET6, >> - IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, >> + rc = nl_addr_get(nl_sock, ifi, AF_INET6, &addr.a6, >> &prefix_len, &ip6->our_tap_ll); >> if (rc < 0) { >> debug("Couldn't discover IPv6 address: %s", strerror_(-rc)); >> return 0; >> } >> >> - ip6->addr_seen = ip6->addr; >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); >> + if (!a) { >> + if (IN6_IS_ADDR_UNSPECIFIED(&addr)) >> + return 0; >> + >> + fwd_set_addr(c, &addr, CONF_ADDR_HOST, prefix_len); >> + ip6->addr_seen = addr.a6; >> + } else { >> + ip6->addr_seen = a->addr.a6; >> + } >> >> if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) >> ip6->our_tap_ll = ip6->guest_gw; >> >> - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) || >> - IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) >> + if (IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) >> return 0; >> >> return ifi; >> @@ -494,13 +502,13 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) >> >> /** >> * conf_ip6_local() - Configure IPv6 addresses and attributes for local mode >> - * @ip6: IPv6 context (will be written) >> + * @c: Execution context (will be written) >> */ >> -static void conf_ip6_local(struct ip6_ctx *ip6) >> +static void conf_ip6_local(struct ctx *c) >> { >> - ip6->our_tap_ll = ip6->guest_gw = IP6_LL_GUEST_GW; >> + c->ip6.our_tap_ll = c->ip6.guest_gw = IP6_LL_GUEST_GW; >> >> - ip6->no_copy_addrs = ip6->no_copy_routes = true; >> + c->ip6.no_copy_addrs = c->ip6.no_copy_routes = true; >> } >> >> /** >> @@ -784,6 +792,7 @@ enum passt_modes conf_mode(int argc, char *argv[]) >> static void conf_print(const struct ctx *c) >> { >> char buf[INANY_ADDRSTRLEN]; >> + const struct guest_addr *a; >> int i; >> >> if (c->fd_control_listen >= 0) >> @@ -831,16 +840,19 @@ static void conf_print(const struct ctx *c) >> inet_ntop(AF_INET, &c->ip4.map_host_loopback, >> buf, sizeof(buf))); >> >> - if (!c->no_dhcp) { >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if (a && !c->no_dhcp) { >> uint32_t mask; >> + int plen = 0; >> >> - mask = htonl(0xffffffff << (32 - c->ip4.prefix_len)); >> + inany_prefix_v4(&a->addr, a->prefix_len, &plen); >> + mask = IN4_MASK(plen); >> >> info("DHCP:"); >> info(" assign: %s", >> - inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf))); >> + inany_ntop(&a->addr, buf, sizeof(buf))); >> info(" mask: %s", >> - inet_ntop(AF_INET, &mask, buf, sizeof(buf))); >> + inet_ntop(AF_INET, &mask, buf, sizeof(buf))); >> info(" router: %s", >> inet_ntop(AF_INET, &c->ip4.guest_gw, >> buf, sizeof(buf))); >> @@ -851,8 +863,8 @@ static void conf_print(const struct ctx *c) >> break; >> if (!i) >> info("DNS:"); >> - inet_ntop(AF_INET, &c->ip4.dns[i], buf, sizeof(buf)); >> - info(" %s", buf); >> + info(" %s", inet_ntop(AF_INET, &c->ip4.dns[i], >> + buf, sizeof(buf))); >> } >> >> for (i = 0; *c->dns_search[i].n; i++) { >> @@ -877,13 +889,15 @@ static void conf_print(const struct ctx *c) >> else >> goto dns6; >> >> - info(" assign: %s", >> - inet_ntop(AF_INET6, &c->ip6.addr, buf, sizeof(buf))); >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, >> + CONF_ADDR_LINKLOCAL); >> + if (a) >> + info(" assign: %s", >> + inany_ntop(&a->addr, buf, sizeof(buf))); >> info(" router: %s", >> inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf))); >> info(" our link-local: %s", >> - inet_ntop(AF_INET6, &c->ip6.our_tap_ll, >> - buf, sizeof(buf))); >> + inet_ntop(AF_INET6, &c->ip6.our_tap_ll, buf, sizeof(buf))); >> >> dns6: >> for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) { >> @@ -891,8 +905,10 @@ dns6: >> break; >> if (!i) >> info("DNS:"); >> - inet_ntop(AF_INET6, &c->ip6.dns[i], buf, sizeof(buf)); >> - info(" %s", buf); >> + info(" %s", >> + inet_ntop(AF_INET6, &c->ip6.dns[i], >> + buf, sizeof(buf))); >> + >> } >> >> for (i = 0; *c->dns_search[i].n; i++) { >> @@ -1580,21 +1596,19 @@ void conf(struct ctx *c, int argc, char **argv) >> IN6_IS_ADDR_V4COMPAT(&addr.a6)) >> die("Invalid address: %s", optarg); >> >> + /* Legacy behaviour: replace existing address if any */ >> + fwd_set_addr(c, &addr, CONF_ADDR_USER, prefix_len); >> if (inany_v4(&addr)) { >> - c->ip4.addr = *inany_v4(&addr); >> - c->ip4.prefix_len = prefix_len - 96; >> + c->ip4.no_copy_addrs = true; >> c->ip4.addr_fixed = true; >> - if (c->mode == MODE_PASTA) >> - c->ip4.no_copy_addrs = true; >> } else { >> - c->ip6.addr = addr.a6; >> + c->ip6.no_copy_addrs = true; >> c->ip6.addr_fixed = true; >> - if (c->mode == MODE_PASTA) >> - c->ip6.no_copy_addrs = true; >> } >> break; >> } >> case 'n': { >> + struct guest_addr *a; >> int plen; >> >> if (addr_has_prefix_len) >> @@ -1604,8 +1618,12 @@ void conf(struct ctx *c, int argc, char **argv) >> if (plen < 0) >> die("Invalid prefix length: %s", optarg); >> >> - prefix_len_from_opt = plen + 96; >> - c->ip4.prefix_len = plen; >> + prefix_len_from_opt = plen; > > Two problems here. First it will need a rebase, because I renamed > prefix_len_from_opt to opt_n. More importantly, you're changing the > encoding of the variable from v6 style to v4-style. Which is > fine... except you don't also change the other place that it's used > (where -n is parsed before -a). TBH, if you want to change the > encoding of this variable, I'd put it in a separate patch, so it > doesn't get lost in this large one. > ok. >> + >> + for_each_addr(a, c->addrs, c->addr_count, AF_INET) { >> + a->prefix_len = plen + 96; >> + break; >> + } >> break; >> } >> case 'M': >> @@ -1801,9 +1819,9 @@ void conf(struct ctx *c, int argc, char **argv) >> >> nl_sock_init(c, false); >> if (!v6_only && !c->splice_only) >> - c->ifi4 = conf_ip4(ifi4, &c->ip4); >> + c->ifi4 = conf_ip4(c, ifi4); >> if (!v4_only && !c->splice_only) >> - c->ifi6 = conf_ip6(ifi6, &c->ip6); >> + c->ifi6 = conf_ip6(c, ifi6); >> >> if (c->ifi4 && c->mtu < IPV4_MIN_MTU) { >> warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)", >> @@ -1826,7 +1844,7 @@ void conf(struct ctx *c, int argc, char **argv) >> if (!c->ifi4 && !v6_only) { >> if (!c->splice_only) { >> info("IPv4: no external interface as template, use local mode"); >> - conf_ip4_local(&c->ip4); >> + conf_ip4_local(c); >> } >> c->ifi4 = -1; >> } >> @@ -1834,7 +1852,7 @@ void conf(struct ctx *c, int argc, char **argv) >> if (!c->ifi6 && !v4_only) { >> if (!c->splice_only) { >> info("IPv6: no external interface as template, use local mode"); >> - conf_ip6_local(&c->ip6); >> + conf_ip6_local(c); >> } >> c->ifi6 = -1; >> } >> @@ -1905,7 +1923,7 @@ void conf(struct ctx *c, int argc, char **argv) >> if (!c->ifi6) { >> c->no_ndp = 1; >> c->no_dhcpv6 = 1; >> - } else if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { >> + } else if (!fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0)) { >> c->no_dhcpv6 = 1; >> } >> >> diff --git a/dhcp.c b/dhcp.c >> index 1ff8cba9..727485f7 100644 >> --- a/dhcp.c >> +++ b/dhcp.c >> @@ -303,6 +303,7 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) >> int dhcp(const struct ctx *c, struct iov_tail *data) >> { >> char macstr[ETH_ADDRSTRLEN]; >> + const struct guest_addr *a; >> size_t mlen, dlen, opt_len; >> struct in_addr mask, dst; >> struct ethhdr eh_storage; >> @@ -313,8 +314,10 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> const struct udphdr *uh; >> struct msg m_storage; >> struct msg const *m; >> + struct in_addr addr; >> struct msg reply; >> unsigned int i; >> + int plen; >> >> eh = IOV_REMOVE_HEADER(data, eh_storage); >> iph = IOV_PEEK_HEADER(data, iph_storage); >> @@ -344,6 +347,14 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> m->op != BOOTREQUEST) >> return -1; >> >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_USER, 0); >> + if (!a) >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if (!a) >> + return -1; >> + >> + addr = *inany_prefix_v4(&a->addr, a->prefix_len, &plen); >> + > > I think an assert that inany_prefix_v4() returns non-NULL is probably > a good idea. I doubt that static checkers will be able to reason that > calling with AF_INET above means it must succeed here. Same comment as above. > >> reply.op = BOOTREPLY; >> reply.htype = m->htype; >> reply.hlen = m->hlen; >> @@ -352,7 +363,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> reply.secs = 0; >> reply.flags = m->flags; >> reply.ciaddr = m->ciaddr; >> - reply.yiaddr = c->ip4.addr; >> + reply.yiaddr = addr; >> reply.siaddr = 0; >> reply.giaddr = m->giaddr; >> memcpy(&reply.chaddr, m->chaddr, sizeof(reply.chaddr)); >> @@ -404,7 +415,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> >> info(" from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr))); >> >> - mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len)); >> + mask.s_addr = IN4_MASK(plen); >> memcpy(opts[1].s, &mask, sizeof(mask)); >> memcpy(opts[3].s, &c->ip4.guest_gw, sizeof(c->ip4.guest_gw)); >> memcpy(opts[54].s, &c->ip4.our_tap_addr, sizeof(c->ip4.our_tap_addr)); >> @@ -412,7 +423,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> /* If the gateway is not on the assigned subnet, send an option 121 >> * (Classless Static Routing) adding a dummy route to it. >> */ >> - if ((c->ip4.addr.s_addr & mask.s_addr) >> + if ((addr.s_addr & mask.s_addr) >> != (c->ip4.guest_gw.s_addr & mask.s_addr)) { >> /* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */ >> opts[121].slen = 14; >> @@ -471,7 +482,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) >> if (m->flags & FLAG_BROADCAST) >> dst = in4addr_broadcast; >> else >> - dst = c->ip4.addr; >> + dst = addr; >> >> tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, &reply, dlen); >> >> diff --git a/dhcpv6.c b/dhcpv6.c >> index 29c7e320..f64cbc24 100644 >> --- a/dhcpv6.c >> +++ b/dhcpv6.c >> @@ -318,7 +318,7 @@ static bool dhcpv6_opt(struct iov_tail *data, uint16_t type) >> * false otherwise and @data is unmodified >> */ >> static bool dhcpv6_ia_notonlink(struct iov_tail *data, >> - struct in6_addr *la) >> + const struct in6_addr *la) >> { >> int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }; >> struct opt_ia_addr opt_addr_storage; >> @@ -567,6 +567,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, >> struct opt_hdr client_id_storage; >> /* cppcheck-suppress [variableScope,unmatchedSuppression] */ >> struct opt_ia_na ia_storage; >> + const struct guest_addr *a; >> const struct in6_addr *src; >> struct msg_hdr mh_storage; >> const struct msg_hdr *mh; >> @@ -574,6 +575,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, >> const struct udphdr *uh; >> size_t mlen, n; >> >> + a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); >> + >> uh = IOV_REMOVE_HEADER(data, uh_storage); >> if (!uh) >> return -1; >> @@ -627,7 +630,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, >> if (mh->type == TYPE_CONFIRM && server_id) >> return -1; >> >> - if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) { >> + if (a && dhcpv6_ia_notonlink(data, &a->addr.a6)) { >> >> dhcpv6_send_ia_notonlink(c, saddr, data, >> &client_id_base, >> @@ -682,7 +685,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, >> resp.hdr.xid = mh->xid; >> >> tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); >> - c->ip6.addr_seen = c->ip6.addr; >> + if (a) >> + c->ip6.addr_seen = a->addr.a6; >> >> return 1; >> } >> @@ -694,6 +698,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, >> void dhcpv6_init(const struct ctx *c) >> { >> time_t y2k = 946684800; /* Epoch to 2000-01-01T00:00:00Z, no mktime() */ >> + const struct guest_addr *a; >> uint32_t duid_time; >> >> duid_time = htonl(difftime(time(NULL), y2k)); >> @@ -706,5 +711,7 @@ void dhcpv6_init(const struct ctx *c) >> memcpy(resp_not_on_link.server_id.duid_lladdr, >> c->our_tap_mac, sizeof(c->our_tap_mac)); >> >> - resp.ia_addr.addr = c->ip6.addr; >> + a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); >> + if (a) >> + resp.ia_addr.addr = a->addr.a6; >> } >> diff --git a/fwd.c b/fwd.c >> index 042158cf..f2ca34c2 100644 >> --- a/fwd.c >> +++ b/fwd.c >> @@ -243,6 +243,61 @@ void fwd_neigh_table_init(const struct ctx *c) >> fwd_neigh_table_update(c, &mga, c->our_tap_mac, true); >> } >> >> +/** >> + * fwd_set_addr() - Add or update an address in the unified address array >> + * @c: Execution context >> + * @addr: Address to add (IPv4-mapped or IPv6) >> + * @flags: CONF_ADDR_* flags for this address >> + * @prefix_len: Prefix length in IPv6 format (96-128 for IPv4-mapped) >> + * >> + * Find the first existing entry of the same address family and >> + * overwrite it, or create a new one if none exists >> + */ >> +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, >> + uint8_t flags, int prefix_len) >> +{ >> + struct guest_addr *a; >> + >> + for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) { >> + goto found; >> + } >> + >> + if (c->addr_count >= MAX_GUEST_ADDRS) >> + return; >> + >> + a = &c->addrs[c->addr_count++]; >> + >> +found: >> + a->addr = *addr; >> + a->prefix_len = prefix_len; >> + a->flags = flags; >> +} >> + >> +/** >> + * fwd_get_addr() - Get guest address entry matching criteria >> + * @c: Execution context >> + * @af: Address family (AF_INET, AF_INET6, or AF_UNSPEC for any) >> + * @incl: Required flags (any-match), CONF_ADDR_ANY for any >> + * @excl: Flags that must not be present >> + * >> + * Return: first address entry matching criteria, or NULL >> + */ >> +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af, >> + uint8_t incl, uint8_t excl) >> +{ >> + const struct guest_addr *a; >> + >> + for_each_addr(a, c->addrs, c->addr_count, af) { >> + if (!(a->flags & incl)) >> + continue; >> + if (a->flags & excl) >> + continue; >> + return a; >> + } >> + >> + return NULL; >> +} >> + >> /* Forwarding table storage, generally accessed via pointers in struct ctx */ >> static struct fwd_table fwd_in; >> static struct fwd_table fwd_out; >> @@ -794,8 +849,10 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) >> * translation, false otherwise >> */ >> static bool fwd_guest_accessible4(const struct ctx *c, >> - const struct in_addr *addr) >> + const struct in_addr *addr) >> { >> + const struct guest_addr *a; >> + >> if (IN4_IS_ADDR_LOOPBACK(addr)) >> return false; >> >> @@ -810,7 +867,8 @@ static bool fwd_guest_accessible4(const struct ctx *c, >> /* For IPv4, addr_seen is initialised to addr, so is always a valid >> * address >> */ >> - if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr) || >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || >> IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) >> return false; >> >> @@ -828,10 +886,13 @@ static bool fwd_guest_accessible4(const struct ctx *c, >> static bool fwd_guest_accessible6(const struct ctx *c, >> const struct in6_addr *addr) >> { >> + const struct guest_addr *a; >> + >> if (IN6_IS_ADDR_LOOPBACK(addr)) >> return false; >> >> - if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr)) >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); >> + if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) >> return false; >> >> /* For IPv6, addr_seen starts unspecified, because we don't know what LL >> @@ -876,16 +937,21 @@ static bool fwd_guest_accessible(const struct ctx *c, >> static void nat_outbound(const struct ctx *c, const union inany_addr *addr, >> union inany_addr *translated) >> { >> - if (inany_equals4(addr, &c->ip4.map_host_loopback)) >> + const struct guest_addr *ga; >> + >> + if (inany_equals4(addr, &c->ip4.map_host_loopback)) { >> *translated = inany_loopback4; >> - else if (inany_equals6(addr, &c->ip6.map_host_loopback)) >> + } else if (inany_equals6(addr, &c->ip6.map_host_loopback)) { >> *translated = inany_loopback6; >> - else if (inany_equals4(addr, &c->ip4.map_guest_addr)) >> - *translated = inany_from_v4(c->ip4.addr); >> - else if (inany_equals6(addr, &c->ip6.map_guest_addr)) >> - translated->a6 = c->ip6.addr; >> - else >> + } else if (inany_equals4(addr, &c->ip4.map_guest_addr)) { >> + ga = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + *translated = ga ? ga->addr : inany_any4; >> + } else if (inany_equals6(addr, &c->ip6.map_guest_addr)) { >> + ga = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); >> + translated->a6 = ga ? ga->addr.a6 : in6addr_any; >> + } else { >> *translated = *addr; >> + } >> } >> >> /** >> @@ -990,16 +1056,23 @@ bool nat_inbound(const struct ctx *c, const union inany_addr *addr, >> } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback) && >> inany_equals6(addr, &in6addr_loopback)) { >> translated->a6 = c->ip6.map_host_loopback; >> - } else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && >> - inany_equals4(addr, &c->ip4.addr)) { >> - *translated = inany_from_v4(c->ip4.map_guest_addr); >> - } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) && >> - inany_equals6(addr, &c->ip6.addr)) { >> - translated->a6 = c->ip6.map_guest_addr; >> - } else if (fwd_guest_accessible(c, addr)) { >> - *translated = *addr; >> } else { >> - return false; >> + const struct guest_addr *ga4, *ga6; >> + >> + ga4 = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + ga6 = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); >> + >> + if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && >> + ga4 && inany_equals(addr, &ga4->addr)) { >> + *translated = inany_from_v4(c->ip4.map_guest_addr); >> + } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) && >> + ga6 && inany_equals(addr, &ga6->addr)) { >> + translated->a6 = c->ip6.map_guest_addr; >> + } else if (fwd_guest_accessible(c, addr)) { >> + *translated = *addr; >> + } else { >> + return false; >> + } >> } >> >> return true; >> diff --git a/fwd.h b/fwd.h >> index b60697d9..d677440f 100644 >> --- a/fwd.h >> +++ b/fwd.h >> @@ -79,5 +79,9 @@ void fwd_neigh_table_free(const struct ctx *c, >> void fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, >> uint8_t *mac); >> void fwd_neigh_table_init(const struct ctx *c); >> +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, >> + uint8_t flags, int prefix_len); >> +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af, >> + uint8_t incl, uint8_t excl); >> >> #endif /* FWD_H */ >> diff --git a/inany.h b/inany.h >> index 6bf3ecaa..95756eb8 100644 >> --- a/inany.h >> +++ b/inany.h >> @@ -102,6 +102,34 @@ static inline struct in_addr *inany_v4(const union inany_addr *addr) >> return (struct in_addr *)&addr->v4mapped.a4; >> } >> >> +/** inany_prefix_v4() - Extract IPv4 address and prefix length from inany >> + * @addr: IPv4 (as mapped) or IPv6 address >> + * @prefix_len: Prefix length in IPv6 format (96-128 for IPv4) >> + * @v4_plen: Set to IPv4 prefix length (0-32) on success >> + * >> + * Return: IPv4 address if @addr is IPv4, NULL otherwise >> + */ >> +static inline struct in_addr *inany_prefix_v4(const union inany_addr *addr, >> + int prefix_len, int *v4_plen) >> +{ >> + struct in_addr *v4 = inany_v4(addr); >> + >> + if (!v4) >> + return NULL; >> + *v4_plen = prefix_len - 96; >> + return v4; >> +} >> + >> +/** inany_af - Get address family of IPv[46] address >> + * @addr: IPv4 or IPv6 address >> + * >> + * Return: AF_INET for IPv4, AF_INET6 for IPv6 >> + */ >> +static inline sa_family_t inany_af(const union inany_addr *addr) >> +{ >> + return inany_v4(addr) ? AF_INET : AF_INET6; >> +} >> + >> /** inany_default_prefix_len() - Get default prefix length for address >> * @addr: IPv4 or iPv6 address >> * >> diff --git a/ip.h b/ip.h >> index aab9b86a..ac93c543 100644 >> --- a/ip.h >> +++ b/ip.h >> @@ -19,6 +19,8 @@ >> (ntohl(((struct in_addr *)(a))->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) >> #define IN4_IS_ADDR_MULTICAST(a) \ >> (IN_MULTICAST(ntohl(((struct in_addr *)(a))->s_addr))) >> +#define IN4_MASK(prefix) \ >> + ((prefix) <= 0 ? 0 : htonl(0xffffffff << (32 - (prefix)))) >> #define IN4_ARE_ADDR_EQUAL(a, b) \ >> (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) >> #define IN4ADDR_LOOPBACK_INIT \ >> diff --git a/ndp.c b/ndp.c >> index 1f2bcb0c..a6a79055 100644 >> --- a/ndp.c >> +++ b/ndp.c >> @@ -257,7 +257,6 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) >> .valid_lifetime = ~0U, >> .pref_lifetime = ~0U, >> }, >> - .prefix = c->ip6.addr, >> .source_ll = { >> .header = { >> .type = OPT_SRC_L2_ADDR, >> @@ -265,8 +264,15 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) >> }, >> }, >> }; >> + const struct guest_addr *a; >> unsigned char *ptr = NULL; >> >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); > > Don't you need to exclude link local addresses? Yes. > >> + if (!a) >> + return; >> + >> + ra.prefix = a->addr.a6; >> + >> ptr = &ra.var[0]; >> >> if (c->mtu) { >> @@ -460,6 +466,7 @@ first: >> */ >> void ndp_send_init_req(const struct ctx *c) >> { >> + const struct guest_addr *a; >> struct ndp_ns ns = { >> .ih = { >> .icmp6_type = NS, >> @@ -468,8 +475,14 @@ void ndp_send_init_req(const struct ctx *c) >> .icmp6_solicited = 0, /* Reserved */ >> .icmp6_override = 0, /* Reserved */ >> }, >> - .target_addr = c->ip6.addr >> + .target_addr = IN6ADDR_ANY_INIT >> }; >> + >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); > > Again, don't you need to exclude linklocal, as the old code implicitly did? > >> + if (!a) >> + return; >> + >> + ns.target_addr = a->addr.a6; >> debug("Sending initial NDP NS request for guest MAC address"); >> - ndp_send(c, &c->ip6.addr, &ns, sizeof(ns)); >> + ndp_send(c, &a->addr.a6, &ns, sizeof(ns)); >> } >> diff --git a/passt.h b/passt.h >> index 16506dcd..cafa35ea 100644 >> --- a/passt.h >> +++ b/passt.h >> @@ -64,11 +64,29 @@ enum passt_modes { >> MODE_VU, >> }; >> >> +/* Maximum number of addresses in context address array */ >> +#define MAX_GUEST_ADDRS 32 >> + >> +/** >> + * struct guest_addr - Unified IPv4/IPv6 address entry >> + * @addr: IPv4 (as mapped) or IPv6 address >> + * @prefix_len: Prefix length in IPv6/IPv4-mapped [0,128]/[96,128] format >> + * @flags: CONF_ADDR_* flags >> + */ >> +struct guest_addr { >> + union inany_addr addr; >> + uint8_t prefix_len; >> + uint8_t flags; >> +#define CONF_ADDR_USER BIT(0) /* User set via -a */ >> +#define CONF_ADDR_HOST BIT(1) /* From host interface */ >> +#define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ >> +#define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ >> +#define CONF_ADDR_ANY 0xff /* Match any flag */ > > In general I like CONF_ADDR_ANY instead of 0 being special-cased. But > with LINKLOCAL in the mix it doesn't quite work. ANY meaning "USER or > HOST or GENERATED" makes sense. ANY meaning "USER or HOST or > GENERATED or LINKLOCAL" is nonsense. I suggest you move LINKLOCAL > into a different field, as Stefano has suggested to handle this (maybe > "provenance" for USER/HOST/GENERATED and "flags" for LINKLOCAL?). Or > you could remove the LINKLOCAL flag from the structure entirely: the > information is already present in the address itself. I was hoping to postpone this to a commit after this series, but I see your point. I can try with a 'scope' field, as suggested earlier, or just skip it altogheter and using an is_link_local() function. > >> +}; >> + >> /** >> * struct ip4_ctx - IPv4 execution context >> - * @addr: IPv4 address assigned to guest >> * @addr_seen: Latest IPv4 address seen as source from tap >> - * @prefixlen: IPv4 prefix length (netmask) >> * @guest_gw: IPv4 gateway as seen by the guest >> * @map_host_loopback: Outbound connections to this address are NATted to the >> * host's 127.0.0.1 >> @@ -87,9 +105,7 @@ enum passt_modes { >> */ >> struct ip4_ctx { >> /* PIF_TAP addresses */ >> - struct in_addr addr; >> struct in_addr addr_seen; >> - int prefix_len; >> struct in_addr guest_gw; >> struct in_addr map_host_loopback; >> struct in_addr map_guest_addr; >> @@ -110,7 +126,6 @@ struct ip4_ctx { >> >> /** >> * struct ip6_ctx - IPv6 execution context >> - * @addr: IPv6 address assigned to guest >> * @addr_seen: Latest IPv6 global/site address seen as source from tap >> * @addr_ll_seen: Latest IPv6 link-local address seen as source from tap >> * @guest_gw: IPv6 gateway as seen by the guest >> @@ -131,7 +146,6 @@ struct ip4_ctx { >> */ >> struct ip6_ctx { >> /* PIF_TAP addresses */ >> - struct in6_addr addr; >> struct in6_addr addr_seen; >> struct in6_addr addr_ll_seen; >> struct in6_addr guest_gw; >> @@ -190,6 +204,8 @@ struct ip6_ctx { >> * @fqdn: Guest FQDN >> * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled >> * @ip6: IPv6 configuration >> + * @addrs: Unified address array for both IPv4 (mapped) and IPv6 >> + * @addr_count: Number of active entries in @addrs array >> * @pasta_ifn: Name of namespace interface for pasta >> * @pasta_ifi: Index of namespace interface for pasta >> * @pasta_conf_ns: Configure namespace after creating it >> @@ -273,6 +289,9 @@ struct ctx { >> int ifi6; >> struct ip6_ctx ip6; >> >> + struct guest_addr addrs[MAX_GUEST_ADDRS]; >> + int addr_count; >> + >> char pasta_ifn[IF_NAMESIZE]; >> unsigned int pasta_ifi; >> int pasta_conf_ns; >> @@ -317,6 +336,41 @@ struct ctx { >> >> extern struct ctx passt_ctx; >> >> +/** >> + * next_addr_idx_() - Find next address index matching family filter >> + * @addrs: Array of guest addresses >> + * @count: Number of addresses in array >> + * @i: Starting index >> + * @af: Address family filter: AF_INET, AF_INET6, or AF_UNSPEC for all >> + * >> + * Return: next matching index, or count if none found >> + */ >> +static inline int next_addr_idx_(const struct guest_addr *addrs, int count, >> + int i, sa_family_t af) >> +{ >> + for (; i < count; i++) { >> + sa_family_t entry_af; >> + >> + entry_af = inany_v4(&addrs[i].addr) ? AF_INET : AF_INET6; > > You can reuse inany_af() here. Ok. > >> + >> + if (af == AF_UNSPEC || af == entry_af) >> + return i; >> + } >> + return i; >> +} >> + >> +/** >> + * for_each_addr() - Iterate over addresses in array >> + * @a: Pointer variable for current entry (struct guest_addr *) >> + * @addrs: Array of guest addresses (e.g., c->addrs) >> + * @count: Number of addresses (e.g., c->addr_count) >> + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all >> + */ >> +#define for_each_addr(a, addrs, count, af) \ >> + for (int i_ = next_addr_idx_((addrs), (count), 0, (af)); \ >> + i_ < (count) && ((a) = &(addrs)[i_], true); \ >> + i_ = next_addr_idx_((addrs), (count), i_ + 1, (af))) >> + >> void proto_update_l2_buf(const unsigned char *eth_d); >> >> #endif /* PASST_H */ >> diff --git a/pasta.c b/pasta.c >> index 4e7ee542..9ef3ac00 100644 >> --- a/pasta.c >> +++ b/pasta.c >> @@ -330,6 +330,8 @@ void pasta_ns_conf(struct ctx *c) >> >> if (c->pasta_conf_ns) { >> unsigned int flags = IFF_UP; >> + const struct guest_addr *a; >> + int plen = 0; >> >> if (c->mtu) >> nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); >> @@ -341,10 +343,17 @@ void pasta_ns_conf(struct ctx *c) >> >> if (c->ifi4) { > > Nit: I think you can reduce the very deep nesting here, by moving the > fwd_get_addr() and changing the if below to.. This is already done in a later patch, by introducing two subfuntions. However, this has caused lot of trouble at each rebase, so I think I will move it to a separate, standalone patch before this series. /jon > > >> if (c->ip4.no_copy_addrs) { > > .. if (no_copy_addrs && a && (v4 = inany_v4(a...))) > > It means the address search will happen even when we don't need it, > but it's once per startup, so we don't really care. > >> - rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, >> - AF_INET, >> - &c->ip4.addr, >> - c->ip4.prefix_len); >> + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0); >> + if (a) { >> + const struct in_addr *v4; >> + >> + v4 = inany_prefix_v4(&a->addr, >> + a->prefix_len, >> + &plen); >> + rc = nl_addr_set(nl_sock_ns, >> + c->pasta_ifi, AF_INET, >> + v4, plen); >> + } > > > >> } else { >> rc = nl_addr_dup(nl_sock, c->ifi4, >> nl_sock_ns, c->pasta_ifi, >> @@ -397,11 +406,12 @@ ipv4_done: >> 0, IFF_NOARP); >> >> if (c->ip6.no_copy_addrs) { > > I think a similar transformation should be possible here. > > >> - if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { >> + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0); >> + if (a) >> rc = nl_addr_set(nl_sock_ns, >> - c->pasta_ifi, AF_INET6, >> - &c->ip6.addr, 64); >> - } >> + c->pasta_ifi, >> + AF_INET6, &a->addr.a6, >> + a->prefix_len); >> } else { >> rc = nl_addr_dup(nl_sock, c->ifi6, >> nl_sock_ns, c->pasta_ifi, >> diff --git a/tap.c b/tap.c >> index d4189617..12630f58 100644 >> --- a/tap.c >> +++ b/tap.c >> @@ -1005,8 +1005,12 @@ resume: >> c->ip6.addr_seen = *saddr; >> } >> >> - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) >> - c->ip6.addr = *saddr; >> + if (!c->ip6.addr_fixed && > > This addr_fixed check is unnnecessary - addr_fixed can only be set if > an address is specified, so the fwd_get_addr() below can't fail. > >> + !fwd_get_addr(c, AF_INET6, CONF_ADDR_ANY, 0)) { >> + union inany_addr addr = { .a6 = *saddr }; >> + >> + fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64); >> + } >> } else if (!c->ip6.addr_fixed && >> !IN6_IS_ADDR_UNSPECIFIED(saddr)) { >> c->ip6.addr_seen = *saddr; >> -- >> 2.52.0 >> >