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=202512 header.b=O9ZnbcJY; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 610AD5A0271 for ; Fri, 06 Feb 2026 09:24:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1770366274; bh=D3JWsSh1X7Qin+alV0CMngf82ndIsLigHA5Ga7/oE8c=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=O9ZnbcJY3luXDlKVIl6KBBlqEpiKIS1p9Ho7xi2kqbT0awU998N9k9vuJIKyOjR5C BCNvMzHB5KLRulxzka8HlxjoZNeEULCsTA4g8GrVVhSsR4P4tm2UJHFEH+AEuTmFJF G1C/nEohVy/fRgxRY5/HFKGeiOpNTd5C5LZEmbcratqwCSUcTH0v0jFp3UwFSduDv6 nv1tdYKTygX0fE/mYfySY7r7iwV4qvBuC3ZJuzqL10/kiDbGIX14UA6raLVvxXuA5a ro67sI3vBS9BBBCza/4mT3lN36ODyyvmbOREuZwDRcyY4ZvjD1CnXoFPErtZaaYlgZ 5RKb8YuuNmtHQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4f6nFG5qkHz4w9Q; Fri, 06 Feb 2026 19:24:34 +1100 (AEDT) Date: Fri, 6 Feb 2026 19:24:25 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v3 03/11] ip: Introduce unified multi-address data structures Message-ID: References: <20260130214447.2540791-1-jmaloy@redhat.com> <20260130214447.2540791-4-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2N5SyPxpPafSOqwR" Content-Disposition: inline In-Reply-To: <20260130214447.2540791-4-jmaloy@redhat.com> Message-ID-Hash: 5T365RAWNPTMVO4AXM3TUO6I7QGPAMQY X-Message-ID-Hash: 5T365RAWNPTMVO4AXM3TUO6I7QGPAMQY 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, dgibson@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: --2N5SyPxpPafSOqwR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jan 30, 2026 at 04:44:39PM -0500, 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. >=20 > There are only two functional changes: > - The indicated IPv6 prefix length is now properly stored, instead > of being ignored and overridden with the hardcoded value 64, as > 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]. >=20 > Signed-off-by: Jon Maloy >=20 > --- > v2: -Using inany_addr instead of protocol specific addresses as > entry address field. >=20 > 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 > --- > arp.c | 5 +- > conf.c | 145 ++++++++++++++++++++++++++++++++----------------------- > conf.h | 6 +++ > dhcp.c | 8 +-- > dhcpv6.c | 6 +-- > fwd.c | 17 ++++--- > ip.c | 11 +++-- > ip.h | 5 ++ > ndp.c | 6 +-- > passt.h | 123 +++++++++++++++++++++++++++++++++++++++++++--- > pasta.c | 15 ++++-- > tap.c | 5 +- > 12 files changed, 251 insertions(+), 101 deletions(-) >=20 > diff --git a/arp.c b/arp.c > index bb042e9..f16beac 100644 > --- a/arp.c > +++ b/arp.c > @@ -54,7 +54,8 @@ static bool ignore_arp(const struct ctx *c, > return true; > =20 > /* Don't resolve the guest's assigned address, either. */ > - if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) > + if (first_v4(c) && first_v4() isn't a great name as a global function. But then I guess later patches in the series will remove most of the uses, so it might be ok in the interim. > + !memcmp(am->tip, inany_v4(&first_v4(c)->addr), sizeof(am->tip))) > return true; > =20 > return false; > @@ -145,7 +146,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(&first_v4(c)->addr), sizeof(req.am.tip)); > =20 > 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 5188c02..bb6bcf8 100644 > --- a/conf.c > +++ b/conf.c > @@ -56,7 +56,7 @@ > #define IP4_LL_GUEST_GW (struct in_addr){ htonl_constant(0xa9fe0202) } > /* 169.254.2.2, libslirp default: 10.0.2.2 */ > =20 > -#define IP4_LL_PREFIX_LEN 16 > +#define IP4_LL_PREFIX_LEN 112 /* 96 + 16, in IPv6 format */ I don't think the encoding should be changed without also changing the #define name. > #define IP6_LL_GUEST_GW (struct in6_addr) \ > {{{ 0xfe, 0x80, 0, 0, 0, 0, 0, 0, \ > @@ -693,11 +693,11 @@ static int conf_ip4_prefix(const char *arg) > /** > * conf_ip4() - Verify or detect IPv4 support, get relevant addresses > * @ifi: Host interface to attempt (0 to determine one) > - * @ip4: IPv4 context (will be written) > + * @c: Execution context (will be written) Putting @c as the first parameter is a pretty strong convention. > * > * 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(unsigned int ifi, struct ctx *c) > { > if (!ifi) > ifi =3D nl_get_ext_if(nl_sock, AF_INET); > @@ -707,9 +707,9 @@ static unsigned int conf_ip4(unsigned int ifi, struct= ip4_ctx *ip4) > return 0; > } > =20 > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->guest_gw)) { > + if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.guest_gw)) { > int rc =3D nl_route_get_def(nl_sock, ifi, AF_INET, > - &ip4->guest_gw); > + &c->ip4.guest_gw); > if (rc < 0) { > debug("Couldn't discover IPv4 gateway address: %s", > strerror_(-rc)); > @@ -717,60 +717,57 @@ static unsigned int conf_ip4(unsigned int ifi, stru= ct ip4_ctx *ip4) > } > } > =20 > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) { > + if (!count_v4(c)) { Do you need count_v4() at this stage of the series? Testing first_v4() would work just as well here, and so far this is the only caller of count_v4(). > + struct in_addr addr =3D {0,}; > + int prefix_len =3D 0; > int rc =3D 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; > =20 > - if (!ip4->prefix_len) { > - in_addr_t addr =3D ntohl(ip4->addr.s_addr); > - if (IN_CLASSA(addr)) > - ip4->prefix_len =3D (32 - IN_CLASSA_NSHIFT); > - else if (IN_CLASSB(addr)) > - ip4->prefix_len =3D (32 - IN_CLASSB_NSHIFT); > - else if (IN_CLASSC(addr)) > - ip4->prefix_len =3D (32 - IN_CLASSC_NSHIFT); > - else > - ip4->prefix_len =3D 32; > + c->addrs[c->addr_count].addr =3D inany_from_v4(addr); > + c->addrs[c->addr_count].prefix_len =3D prefix_len + 96; > + c->addrs[c->addr_count].flags =3D INANY_ADDR_HOST; > + c->addr_count++; I suspect an add_prefix() type function will be worth it, by the time we're done. > + c->ip4.addr_seen =3D addr; > } > =20 > - ip4->addr_seen =3D ip4->addr; > - > - ip4->our_tap_addr =3D ip4->guest_gw; > - > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) > - return 0; > + c->ip4.our_tap_addr =3D c->ip4.guest_gw; > =20 > return ifi; > } > =20 > /** > * 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 =3D ip4->addr =3D IP4_LL_GUEST_ADDR; > - ip4->our_tap_addr =3D ip4->guest_gw =3D IP4_LL_GUEST_GW; > - ip4->prefix_len =3D IP4_LL_PREFIX_LEN; > + c->addrs[c->addr_count].addr =3D inany_from_v4(IP4_LL_GUEST_ADDR); > + c->ip4.addr_seen =3D *inany_v4(&c->addrs[c->addr_count].addr); > + c->ip4.our_tap_addr =3D c->ip4.guest_gw =3D IP4_LL_GUEST_GW; > + c->addrs[c->addr_count].prefix_len =3D IP4_LL_PREFIX_LEN; I'd add the 96 here, rather than putting it into the constant. > + c->addr_count++; > =20 > - ip4->no_copy_addrs =3D ip4->no_copy_routes =3D true; > + c->ip4.no_copy_addrs =3D c->ip4.no_copy_routes =3D true; > } > =20 > /** > * conf_ip6() - Verify or detect IPv6 support, get relevant addresses > * @ifi: Host interface to attempt (0 to determine one) > - * @ip6: IPv6 context (will be written) > + * @c: Execution 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(unsigned int ifi, struct ctx *c) > { > + struct inany_addr_entry *e; > + union inany_addr addr =3D {0,}; > int prefix_len =3D 0; > int rc; > =20 > @@ -782,8 +779,8 @@ static unsigned int conf_ip6(unsigned int ifi, struct= ip6_ctx *ip6) > return 0; > } > =20 > - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw)) { > - rc =3D nl_route_get_def(nl_sock, ifi, AF_INET6, &ip6->guest_gw); > + if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.guest_gw)) { > + rc =3D nl_route_get_def(nl_sock, ifi, AF_INET6, &c->ip6.guest_gw); > if (rc < 0) { > debug("Couldn't discover IPv6 gateway address: %s", > strerror_(-rc)); > @@ -791,21 +788,28 @@ static unsigned int conf_ip6(unsigned int ifi, stru= ct ip6_ctx *ip6) > } > } > =20 > - rc =3D nl_addr_get(nl_sock, ifi, AF_INET6, > - IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, > - &prefix_len, &ip6->our_tap_ll); > + rc =3D nl_addr_get(nl_sock, ifi, AF_INET6, &addr.a6, > + &prefix_len, &c->ip6.our_tap_ll); > if (rc < 0) { > debug("Couldn't discover IPv6 address: %s", strerror_(-rc)); > return 0; > } > =20 > - ip6->addr_seen =3D ip6->addr; > + if (!count_v6(c)) { Same comment as for count_v4(). > + c->addrs[c->addr_count].addr =3D addr; > + c->addrs[c->addr_count].prefix_len =3D prefix_len ? prefix_len : 64; > + c->addrs[c->addr_count].flags =3D INANY_ADDR_HOST; > + c->addr_count++; > + } > + > + e =3D first_v6(c); > + c->ip6.addr_seen =3D e->addr.a6; > =20 > - if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) > - ip6->our_tap_ll =3D ip6->guest_gw; > + if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.guest_gw)) > + c->ip6.our_tap_ll =3D c->ip6.guest_gw; > =20 > - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) || > - IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) > + if (IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6) || > + IN6_IS_ADDR_UNSPECIFIED(&c->ip6.our_tap_ll)) > return 0; > =20 > return ifi; > @@ -813,13 +817,13 @@ static unsigned int conf_ip6(unsigned int ifi, stru= ct ip6_ctx *ip6) > =20 > /** > * 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 =3D ip6->guest_gw =3D IP6_LL_GUEST_GW; > + c->ip6.our_tap_ll =3D c->ip6.guest_gw =3D IP6_LL_GUEST_GW; > =20 > - ip6->no_copy_addrs =3D ip6->no_copy_routes =3D true; > + c->ip6.no_copy_addrs =3D c->ip6.no_copy_routes =3D true; > } > =20 > /** > @@ -1145,13 +1149,15 @@ static void conf_print(const struct ctx *c) > buf4, sizeof(buf4))); > =20 > if (!c->no_dhcp) { > + struct inany_addr_entry *e =3D first_v4(c); You never test for e being NULL in this path. Maybe that's not possible unless c->no_dhcp, but that's pretty non-obvious from here. > uint32_t mask; > =20 > - mask =3D IN4_MASK(c->ip4.prefix_len); > + mask =3D IN4_MASK(e->prefix_len); > =20 > info("DHCP:"); > info(" assign: %s", > - inet_ntop(AF_INET, &c->ip4.addr, buf4, sizeof(buf4))); > + inet_ntop(AF_INET, inany_v4(&e->addr), > + buf4, sizeof(buf4))); > info(" mask: %s", > inet_ntop(AF_INET, &mask, buf4, sizeof(buf4))); > info(" router: %s", > @@ -1189,7 +1195,8 @@ static void conf_print(const struct ctx *c) > goto dns6; > =20 > info(" assign: %s", > - inet_ntop(AF_INET6, &c->ip6.addr, buf6, sizeof(buf6))); > + inet_ntop(AF_INET6, &first_v6(c)->addr.a6, Similarly no test for NULL first_v6() before dereferencing. > + buf6, sizeof(buf6))); > info(" router: %s", > inet_ntop(AF_INET6, &c->ip6.guest_gw, buf6, sizeof(buf6))); > info(" our link-local: %s", > @@ -1811,6 +1818,7 @@ void conf(struct ctx *c, int argc, char **argv) > } > case 'a': { > union inany_addr addr =3D { 0 }; > + struct inany_addr_entry *e; > int prefix_len =3D 0; > int af; > =20 > @@ -1828,13 +1836,20 @@ void conf(struct ctx *c, int argc, char **argv) > die("Can't mix CIDR with -n"); > =20 > if (af =3D=3D AF_INET) { > - c->ip4.addr =3D *inany_v4(&addr); > - c->ip4.prefix_len =3D prefix_len ? prefix_len - 96 : > - ip4_class_prefix_len(&c->ip4.addr); > + e =3D &c->addrs[c->addr_count]; > + e->addr =3D addr; > + e->prefix_len =3D prefix_len ? prefix_len : > + ip4_class_prefix_len(inany_v4(&addr)); > + e->flags =3D INANY_ADDR_CONFIGURED; > + c->addr_count++; > if (c->mode =3D=3D MODE_PASTA) > c->ip4.no_copy_addrs =3D true; > } else if (af =3D=3D AF_INET6) { > - c->ip6.addr =3D addr.a6; > + e =3D &c->addrs[c->addr_count]; > + e->addr =3D addr; > + e->prefix_len =3D prefix_len ? prefix_len : 64; > + e->flags =3D INANY_ADDR_CONFIGURED; > + c->addr_count++; > if (c->mode =3D=3D MODE_PASTA) > c->ip6.no_copy_addrs =3D true; > } else { > @@ -1842,14 +1857,21 @@ void conf(struct ctx *c, int argc, char **argv) > } > break; > } > - case 'n': > + case 'n': { > + struct inany_addr_entry *e; > + int plen; > + > if (prefix_from_cidr) > die("Can't use both -n and CIDR prefix length"); > - c->ip4.prefix_len =3D conf_ip4_prefix(optarg); > - if (c->ip4.prefix_len < 0) > + plen =3D conf_ip4_prefix(optarg); > + if (plen < 0) > die("Invalid netmask: %s", optarg); > + e =3D first_v4(c); > + if (e) > + e->prefix_len =3D plen + 96; > prefix_from_opt =3D true; > break; > + } > case 'M': > parse_mac(c->our_tap_mac, optarg); > break; > @@ -1993,9 +2015,9 @@ void conf(struct ctx *c, int argc, char **argv) > =20 > nl_sock_init(c, false); > if (!v6_only) > - c->ifi4 =3D conf_ip4(ifi4, &c->ip4); > + c->ifi4 =3D conf_ip4(ifi4, c); > if (!v4_only) > - c->ifi6 =3D conf_ip6(ifi6, &c->ip6); > + c->ifi6 =3D conf_ip6(ifi6, c); > =20 > if (c->ifi4 && c->mtu < IPV4_MIN_MTU) { > warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)", > @@ -2018,14 +2040,14 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi4 && !v6_only) { > info("IPv4: no external interface as template, use local mode"); > =20 > - conf_ip4_local(&c->ip4); > + conf_ip4_local(c); > c->ifi4 =3D -1; > } > =20 > if (!c->ifi6 && !v4_only) { > info("IPv6: no external interface as template, use local mode"); > =20 > - conf_ip6_local(&c->ip6); > + conf_ip6_local(c); > c->ifi6 =3D -1; > } > =20 > @@ -2134,7 +2156,8 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi6) { > c->no_ndp =3D 1; > c->no_dhcpv6 =3D 1; > - } else if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { > + } else if (!first_v6(c) || > + IN6_IS_ADDR_UNSPECIFIED(&first_v6(c)->addr.a6)) { > c->no_dhcpv6 =3D 1; > } > =20 > diff --git a/conf.h b/conf.h > index b45ad74..e86fe96 100644 > --- a/conf.h > +++ b/conf.h > @@ -6,6 +6,12 @@ > #ifndef CONF_H > #define CONF_H > =20 > +/* Flags indicating origin and role of an address > + * To be used in struct inany_addr_entry > + */ > +#define CONF_ADDR_CONFIGURED (1 << 0) /* User set via -a */ As Stefano said, I think "USER" or "CMDLINE" would be preferable to "CONFIGURED". > +#define CONF_ADDR_HOST (1 << 1) /* From host interface */ > + > enum passt_modes conf_mode(int argc, char *argv[]); > void conf(struct ctx *c, int argc, char **argv); > =20 > diff --git a/dhcp.c b/dhcp.c > index c552f01..933a8cf 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -352,7 +352,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > reply.secs =3D 0; > reply.flags =3D m->flags; > reply.ciaddr =3D m->ciaddr; > - reply.yiaddr =3D c->ip4.addr; > + reply.yiaddr =3D *inany_v4(&first_v4(c)->addr); Again, maybe it's impossible for first_v4() to be NULL if !no_dhcp, but that's not obvious from here. If it's your intention to rely on that, an ASSERT would make sense. > reply.siaddr =3D 0; > reply.giaddr =3D m->giaddr; > memcpy(&reply.chaddr, m->chaddr, sizeof(reply.chaddr)); > @@ -404,7 +404,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > =20 > info(" from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr))); > =20 > - mask.s_addr =3D IN4_MASK(c->ip4.prefix_len); > + mask.s_addr =3D IN4_MASK(first_v4(c)->prefix_len); - 96, since the entry has an IPv6 encoded prefix length. Or maybe it would be nicer to replace first_v4() with something that extracts the first v4 address calling inany_v4() for you, and also extracts the matching prefix, doing the re-encoding to IPv4 format. > 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 +412,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 ((inany_v4(&first_v4(c)->addr)->s_addr & mask.s_addr) > !=3D (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 =3D 14; > @@ -469,7 +469,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > if (m->flags & FLAG_BROADCAST) > dst =3D in4addr_broadcast; > else > - dst =3D c->ip4.addr; > + dst =3D *inany_v4(&first_v4(c)->addr); > =20 > tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, &reply, dlen); > =20 > diff --git a/dhcpv6.c b/dhcpv6.c > index e4df0db..a1e5f15 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -625,7 +625,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > if (mh->type =3D=3D TYPE_CONFIRM && server_id) > return -1; > =20 > - if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) { > + if (dhcpv6_ia_notonlink(data, &first_v6(c)->addr.a6)) { Likewise a helper that returns the first v6 address *as* a struct in6_addr might be neater. > =20 > dhcpv6_send_ia_notonlink(c, data, &client_id_base, > ntohs(client_id->l), mh->xid); > @@ -679,7 +679,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > =20 > tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, > mh->xid, &resp, n); > - c->ip6.addr_seen =3D c->ip6.addr; > + c->ip6.addr_seen =3D first_v6(c)->addr.a6; > =20 > return 1; > } > @@ -703,5 +703,5 @@ 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)); > =20 > - resp.ia_addr.addr =3D c->ip6.addr; > + resp.ia_addr.addr =3D first_v6(c)->addr.a6; > } > diff --git a/fwd.c b/fwd.c > index 44a0e10..54248a3 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -516,7 +516,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) || > + if ((first_v4(c) && > + IN4_ARE_ADDR_EQUAL(addr, inany_v4(&first_v4(c)->addr))) || > IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) > return false; > =20 > @@ -537,7 +538,7 @@ static bool fwd_guest_accessible6(const struct ctx *c, > if (IN6_IS_ADDR_LOOPBACK(addr)) > return false; > =20 > - if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr)) > + if (first_v6(c) && IN6_ARE_ADDR_EQUAL(addr, &first_v6(c)->addr.a6)) > return false; > =20 > /* For IPv6, addr_seen starts unspecified, because we don't know what LL > @@ -586,10 +587,10 @@ static void nat_outbound(const struct ctx *c, const= union inany_addr *addr, > *translated =3D inany_loopback4; > else if (inany_equals6(addr, &c->ip6.map_host_loopback)) > *translated =3D inany_loopback6; > - else if (inany_equals4(addr, &c->ip4.map_guest_addr)) > - *translated =3D inany_from_v4(c->ip4.addr); > - else if (inany_equals6(addr, &c->ip6.map_guest_addr)) > - translated->a6 =3D c->ip6.addr; > + else if (first_v4(c) && inany_equals4(addr, &c->ip4.map_guest_addr)) > + *translated =3D first_v4(c)->addr; > + else if (first_v6(c) && inany_equals6(addr, &c->ip6.map_guest_addr)) > + translated->a6 =3D first_v6(c)->addr.a6; Implementing the forward table for the tap pif will make this cleaner, when I get to it. > else > *translated =3D *addr; > } > @@ -710,10 +711,10 @@ bool nat_inbound(const struct ctx *c, const union i= nany_addr *addr, > inany_equals6(addr, &in6addr_loopback)) { > translated->a6 =3D c->ip6.map_host_loopback; > } else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && > - inany_equals4(addr, &c->ip4.addr)) { > + first_v4(c) && inany_equals(addr, &first_v4(c)->addr)) { > *translated =3D 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)) { > + first_v6(c) && inany_equals6(addr, &first_v6(c)->addr.a6)) { > translated->a6 =3D c->ip6.map_guest_addr; > } else if (fwd_guest_accessible(c, addr)) { > *translated =3D *addr; > diff --git a/ip.c b/ip.c > index 40dc24e..a86753f 100644 > --- a/ip.c > +++ b/ip.c > @@ -74,17 +74,18 @@ found: > * ip4_class_prefix_len() - Get class based prefix length for IPv4 addre= ss > * @addr: IPv4 address > * > - * Return: prefix length based on address class, or 32 for other > + * Return: prefix length in IPv6 format (96 + IPv4 prefix) based on addr= ess > + * class, or 128 for other Here again, I don't think you should change the encoding without changing the function name. As I've said, I think we should consistently use IPv6 encoding *when dealing with an inany*. But functions that deal specifically with IPv4 addresses, should use the IPv4 prefix length encoding. > */ > int ip4_class_prefix_len(const struct in_addr *addr) > { > in_addr_t a =3D ntohl(addr->s_addr); > =20 > if (IN_CLASSA(a)) > - return 32 - IN_CLASSA_NSHIFT; > + return 96 + 32 - IN_CLASSA_NSHIFT; > if (IN_CLASSB(a)) > - return 32 - IN_CLASSB_NSHIFT; > + return 96 + 32 - IN_CLASSB_NSHIFT; > if (IN_CLASSC(a)) > - return 32 - IN_CLASSC_NSHIFT; > - return 32; > + return 96 + 32 - IN_CLASSC_NSHIFT; > + return 128; > } > diff --git a/ip.h b/ip.h > index c829d84..91d6d9a 100644 > --- a/ip.h > +++ b/ip.h > @@ -137,6 +137,11 @@ static const struct in_addr in4addr_broadcast =3D { = 0xffffffff }; > #define IPV6_MIN_MTU 1280 > #endif > =20 > +/* Maximum number of addresses per address family */ > +#define IP4_MAX_ADDRS 16 > +#define IP6_MAX_ADDRS 16 Separate defines here don't really make sense now you have a single address array. > +#define INANY_MAX_ADDRS (IP4_MAX_ADDRS + IP6_MAX_ADDRS) > + > int ip4_class_prefix_len(const struct in_addr *addr); > =20 > #endif /* IP_H */ > diff --git a/ndp.c b/ndp.c > index eb9e313..f60c298 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -257,7 +257,7 @@ static void ndp_ra(const struct ctx *c, const struct = in6_addr *dst) > .valid_lifetime =3D ~0U, > .pref_lifetime =3D ~0U, > }, > - .prefix =3D c->ip6.addr, > + .prefix =3D first_v6(c)->addr.a6, > .source_ll =3D { > .header =3D { > .type =3D OPT_SRC_L2_ADDR, > @@ -466,8 +466,8 @@ void ndp_send_init_req(const struct ctx *c) > .icmp6_solicited =3D 0, /* Reserved */ > .icmp6_override =3D 0, /* Reserved */ > }, > - .target_addr =3D c->ip6.addr > + .target_addr =3D first_v6(c)->addr.a6 > }; > debug("Sending initial NDP NS request for guest MAC address"); > - ndp_send(c, &c->ip6.addr, &ns, sizeof(ns)); > + ndp_send(c, &first_v6(c)->addr.a6, &ns, sizeof(ns)); > } > diff --git a/passt.h b/passt.h > index 79d01dd..ec5517d 100644 > --- a/passt.h > +++ b/passt.h > @@ -64,11 +64,42 @@ enum passt_modes { > MODE_VU, > }; > =20 > +/* Flags indicating origin and role of an address > + * in struct inany_addr_entry > + */ > +#define INANY_ADDR_CONFIGURED BIT(0) /* User set via -a */ > +#define INANY_ADDR_HOST BIT(1) /* From host interface */ These appear to be redundant with with CONF_ADDR_*. These are specific to how we handle configuration, not general properties of a prefix, so I don't think these ones belong. > + > +/** > + * for_each_addr() - Iterate over addresses in unified array > + * @c: Pointer to struct ctx > + * @e: Pointer variable for current entry (struct inany_addr_entry *) Wll, @c going first is a strongish convention, but the loop variable going first in a for_each_() macro is an even stronger convention. > + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all Iterating over just one address type is going to become less and less common as we unify handling in various places. So it might be simpler to just have a single iterator, and explicitly filter the "wrong" type in the places we need to. > + * > + * Note: @_i is the internal loop counter, uses _next_addr_idx() helper > + */ > +#define for_each_addr(c, e, af) \ > + for (int _i =3D _next_addr_idx((c), 0, (af)); \ > + _i < (c)->addr_count && ((e) =3D &(c)->addrs[_i], true); \ > + _i =3D _next_addr_idx((c), _i + 1, (af))) > + > +/* first_v4(), first_v6(), count_v4(), count_v6() defined after struct c= tx */ > + > +/** > + * struct inany_addr_entry - 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: INANY_ADDR_* flags > + */ > +struct inany_addr_entry { > + union inany_addr addr; > + uint8_t prefix_len; > + uint8_t flags; > +}; > + > /** > * 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 t= o the > * host's 127.0.0.1 > @@ -84,10 +115,7 @@ enum passt_modes { > * @no_copy_addrs: Don't copy all addresses when configuring namespace > */ > 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; > @@ -107,7 +135,6 @@ struct ip4_ctx { > =20 > /** > * 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 > @@ -125,8 +152,6 @@ struct ip4_ctx { > * @no_copy_addrs: Don't copy all addresses when configuring namespace > */ > 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; > @@ -257,6 +282,10 @@ struct ctx { > int ifi6; > struct ip6_ctx ip6; > =20 > + /* Unified address array for both IPv4 (mapped) and IPv6 */ > + struct inany_addr_entry addrs[INANY_MAX_ADDRS]; > + int addr_count; > + THe new fields need to be added to the structure comment > char pasta_ifn[IF_NAMESIZE]; > unsigned int pasta_ifi; > int pasta_conf_ns; > @@ -294,6 +323,84 @@ struct ctx { > bool migrate_exit; > }; > =20 > +/** > + * first_v4() - Get first IPv4 address entry > + * @c: Pointer to struct ctx > + * > + * Return: pointer to first IPv4 entry, or NULL if none > + */ > +static inline struct inany_addr_entry *first_v4(const struct ctx *c) > +{ > + for (int i =3D 0; i < c->addr_count; i++) > + if (inany_v4(&c->addrs[i].addr)) > + return (struct inany_addr_entry *)&c->addrs[i]; > + return NULL; > +} > + > +/** > + * first_v6() - Get first IPv6 address entry > + * @c: Pointer to struct ctx > + * > + * Return: pointer to first IPv6 entry, or NULL if none > + */ > +static inline struct inany_addr_entry *first_v6(const struct ctx *c) > +{ > + for (int i =3D 0; i < c->addr_count; i++) > + if (!inany_v4(&c->addrs[i].addr)) > + return (struct inany_addr_entry *)&c->addrs[i]; > + return NULL; > +} > + > +/** > + * count_v4() - Count IPv4 addresses > + * @c: Pointer to struct ctx > + * > + * Return: number of IPv4 addresses > + */ > +static inline int count_v4(const struct ctx *c) > +{ > + int count =3D 0; > + > + for (int i =3D 0; i < c->addr_count; i++) > + if (inany_v4(&c->addrs[i].addr)) > + count++; > + return count; > +} > + > +/** > + * count_v6() - Count IPv6 addresses > + * @c: Pointer to struct ctx > + * > + * Return: number of IPv6 addresses > + */ > +static inline int count_v6(const struct ctx *c) > +{ > + int count =3D 0; > + > + for (int i =3D 0; i < c->addr_count; i++) > + if (!inany_v4(&c->addrs[i].addr)) > + count++; > + return count; > +} > + > +/** > + * _next_addr_idx() - Find next address index matching family filter > + * @c: Pointer to struct ctx > + * @i: Starting index > + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all > + * > + * Return: next matching index, or addr_count if none found > + */ > +static inline int _next_addr_idx(const struct ctx *c, int i, int af) > +{ > + while (i < c->addr_count) { > + if (!af || ((af =3D=3D AF_INET) =3D=3D !!inany_v4(&c->addrs[i].addr))) > + return i; > + i++; > + } > + return i; > +} This is a pretty fiddly way of doing the iteration. Simpler to have a plain loop through the array, and skip the entries you don't want. > + > void proto_update_l2_buf(const unsigned char *eth_d); > =20 > #endif /* PASST_H */ > diff --git a/pasta.c b/pasta.c > index c307b8a..08f35f4 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -338,10 +338,12 @@ void pasta_ns_conf(struct ctx *c) > =20 > if (c->ifi4) { > if (c->ip4.no_copy_addrs) { > + struct inany_addr_entry *e =3D first_v4(c); NULL check? > + > rc =3D nl_addr_set(nl_sock_ns, c->pasta_ifi, > AF_INET, > - &c->ip4.addr, > - c->ip4.prefix_len); > + inany_v4(&e->addr), > + e->prefix_len); > } else { > rc =3D nl_addr_dup(nl_sock, c->ifi4, > nl_sock_ns, c->pasta_ifi, > @@ -387,10 +389,13 @@ void pasta_ns_conf(struct ctx *c) > 0, IFF_NOARP); > =20 > if (c->ip6.no_copy_addrs) { > - if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { > + struct inany_addr_entry *e =3D first_v6(c); > + > + if (e && !IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6)) { If it was UNSPECIFIED, then first_v6() would return NULL, no? > rc =3D nl_addr_set(nl_sock_ns, > - c->pasta_ifi, AF_INET6, > - &c->ip6.addr, 64); > + c->pasta_ifi, > + AF_INET6, > + &e->addr.a6, 64); > } > } else { > rc =3D nl_addr_dup(nl_sock, c->ifi6, > diff --git a/tap.c b/tap.c > index 9d1344b..f3073c6 100644 > --- a/tap.c > +++ b/tap.c > @@ -951,8 +951,9 @@ resume: > c->ip6.addr_seen =3D *saddr; > } > =20 > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) > - c->ip6.addr =3D *saddr; > + if (first_v6(c) && > + IN6_IS_ADDR_UNSPECIFIED(&first_v6(c)->addr.a6)) Again, if the address were unspecified, first_v6() would return NULL, wouldn't it? > + first_v6(c)->addr.a6 =3D *saddr; > } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ > c->ip6.addr_seen =3D *saddr; > } > --=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 --2N5SyPxpPafSOqwR Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmFpSEACgkQzQJF27ox 2Gee4A//d+ZL4K1u6Bz6BbEVq9qysPHyrZqxdJ4KCciImISoeLn1iKkwt9GBLKHm ieChlLZ4d8Mn7gpm4dpfl0cH9V+zla+dt5Bc1kZ7vTrj5CYb8TbHeWkbrVWRrfzz Sr9Lc/xCSQminxgEx8F/CA4S3ahiNPsrxEmSvByEyqUGeLg9SZ2bSDE+yQNmW0hA J+Ig98ADSq7b4LDfP1fwoJj/lr3+RFMDIUyiVqHySj6wxqWyLbxjsRA1lW5Q97WE URg/EfyCHq5KIokfd63ESp5qrMaMGlJrD0JlhqOAHhxzE8HZbmmLdIVbl/n1jeHA b/EHHEtla8xDtKofXTsYBBmEs8HLKlnXnIlSo0ZqAGYvCVIEjoIqcQUkmmmfG9S3 WReDK1Gl2lEhE2XKtzJOKeq3moZu2RDpHQEn7OKqqfgtT4EVHXcONaL8LaGxd7Q/ 5j+4lhGPCLxh7q4Faxa50zw9OOvU+AhZiFkJmsyLBRbXaaaVRvCXdmhVXzSVCNR4 3eletxIOI5TT2caAX5RXOTveziAUsCn1UNKk//Yr0r97bDkqR/9E4z652P3pC8hF jy1XBS+JpXbXwifRgLD4yKaGbbaolsjsZobFDNzfbusvkM1LQNdPampv6kp5Rjo1 OOrMXKroswbqVizaXbGuZy6VzyBmDTpGCsXF6wQeMvpu3FDe7so= =QGaH -----END PGP SIGNATURE----- --2N5SyPxpPafSOqwR--