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=202602 header.b=h3o6+cjN; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 799935A0262 for ; Tue, 24 Mar 2026 04:31:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774323084; bh=ttQMLS2H21X81DxTyhlhL3W4vrQNzkStMJKXcunoIko=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=h3o6+cjN12cceiQuqQbcdsTNg5jkDwHm+orVQDtHmFtYi+ZqtGS4MAyA2oKf/RpZU n58tIkD3V4b4y1NIe96vUhear+CXt9RTH0C71yteaRqgUzjDkbJe3fgsZuaa/G8Z9J NQkF/S3WLMKI22uOvKnNo8ialYxLEgo3u9kBsVbHQj9nXff+fCIGbRDjabkZWw7RIc XTyJkFTvBJFsAsobOFxFJldb49K49oPVus/UdmebaC7EvyxjbCpFrN+nHqt+4SUaXX Po07dwXOBFc70Fqlebn7jAbA3toiki0tQWbI1fZaNCc7pKoXgX5zNWA2tdk+9QzjQs +x7Hx6MnLUInw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ffwYm4zksz4wCG; Tue, 24 Mar 2026 14:31:24 +1100 (AEDT) Date: Tue, 24 Mar 2026 14:31:19 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v6 02/13] ip: Introduce unified multi-address data structures Message-ID: References: <20260322004333.365713-1-jmaloy@redhat.com> <20260322004333.365713-3-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ar5JSBx9xR5IaKCa" Content-Disposition: inline In-Reply-To: <20260322004333.365713-3-jmaloy@redhat.com> Message-ID-Hash: 676OPLNUNPRGTTSJHYLUHHADHPO5L4L2 X-Message-ID-Hash: 676OPLNUNPRGTTSJHYLUHHADHPO5L4L2 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: --ar5JSBx9xR5IaKCa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Mar 21, 2026 at 08:43:22PM -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. >=20 > 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 > 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 >=20 > v4: -Updated according to changes in previous commits > -Updated according to feedback from David G. > -Squashed IP4_MASK macro commit into this one >=20 > 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. > --- > arp.c | 10 +++- > conf.c | 142 +++++++++++++++++++++++++++++-------------------------- > dhcp.c | 13 +++-- > dhcpv6.c | 15 ++++-- > fwd.c | 104 +++++++++++++++++++++++++++++++++------- > fwd.h | 4 ++ > igmp.c | 1 + > ip.h | 2 + > ndp.c | 16 +++++-- > passt.h | 82 ++++++++++++++++++++++++++++---- > pasta.c | 21 ++++---- > tap.c | 7 ++- > 12 files changed, 302 insertions(+), 115 deletions(-) >=20 > diff --git a/arp.c b/arp.c > index bb042e9..18ff5de 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 =3D fwd_get_addr(c, AF_INET, 0, 0); > + > if (ah->ar_hrd !=3D htons(ARPHRD_ETHER) || > ah->ar_pro !=3D htons(ETH_P_IP) || > ah->ar_hln !=3D ETH_ALEN || > @@ -54,7 +56,7 @@ 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 (a && !memcmp(am->tip, inany_v4(&a->addr), sizeof(am->tip))) > return true; > =20 > return false; > @@ -123,12 +125,16 @@ int arp(const struct ctx *c, struct iov_tail *data) > */ > void arp_send_init_req(const struct ctx *c) > { > + const struct guest_addr *a =3D fwd_get_addr(c, AF_INET, 0, 0); > struct { > struct ethhdr eh; > struct arphdr ah; > struct arpmsg am; > } __attribute__((__packed__)) req; > =20 > + if (!a) > + return; > + > /* Ethernet header */ > req.eh.h_proto =3D htons(ETH_P_ARP); > memcpy(req.eh.h_dest, MAC_BROADCAST, sizeof(req.eh.h_dest)); > @@ -145,7 +151,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)); > =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 9bcd9de..8f02494 100644 > --- a/conf.c > +++ b/conf.c > @@ -728,13 +728,15 @@ static int conf_ip4_prefix(const char *arg) > =20 > /** > * 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) > { > + struct ip4_ctx *ip4 =3D &c->ip4; > + > if (!ifi) > ifi =3D nl_get_ext_if(nl_sock, AF_INET); > =20 > @@ -753,60 +755,57 @@ static unsigned int conf_ip4(unsigned int ifi, stru= ct ip4_ctx *ip4) > } > } > =20 > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) { > + if (!fwd_get_addr(c, AF_INET, 0, 0)) { > + struct in_addr addr; > + int prefix_len; > 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; > + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, > + prefix_len + 96); > + ip4->addr_seen =3D addr; > } > =20 > - ip4->addr_seen =3D ip4->addr; > - > ip4->our_tap_addr =3D ip4->guest_gw; > =20 > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) > - return 0; > - > 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; > + struct ip4_ctx *ip4 =3D &c->ip4; > =20 > + ip4->addr_seen =3D IP4_LL_GUEST_ADDR; > + ip4->our_tap_addr =3D ip4->guest_gw =3D IP4_LL_GUEST_GW; > ip4->no_copy_addrs =3D ip4->no_copy_routes =3D true; > + fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), > + CONF_ADDR_HOST | CONF_ADDR_LINKLOCAL, CONF_ADDR_HOST isn't really true here. Maybe we need an additional flag for addresses that we just pluck out of the air for local mode. > + IP4_LL_PREFIX_LEN + 96); > } > =20 > /** > * 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) > { > + const struct guest_addr *a =3D fwd_get_addr(c, AF_INET6, 0, 0); > + struct ip6_ctx *ip6 =3D &c->ip6; > + union inany_addr addr; > int prefix_len =3D 0; > int rc; > =20 > @@ -827,21 +826,24 @@ 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, > + rc =3D 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; > } > =20 > - ip6->addr_seen =3D ip6->addr; > + if (!a) { > + fwd_set_addr(c, &addr, CONF_ADDR_HOST, prefix_len); > + ip6->addr_seen =3D addr.a6; > + } else { > + ip6->addr_seen =3D a->addr.a6; > + } > =20 > if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) > ip6->our_tap_ll =3D ip6->guest_gw; > =20 > - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) || > - IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) > + if (IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) For IPv4 we fail (return 0) if we have neither a user given address nor can get one from the host. Removing the equivalent check for IPv6 seems like it needs a justification. > return 0; > =20 > return ifi; > @@ -849,13 +851,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 > /** > @@ -1137,6 +1139,7 @@ enum passt_modes conf_mode(int argc, char *argv[]) > static void conf_print(const struct ctx *c) > { > char buf[INET6_ADDRSTRLEN]; > + const struct guest_addr *a; > int i; > =20 > if (c->ifi4 > 0 || c->ifi6 > 0) { > @@ -1182,19 +1185,19 @@ static void conf_print(const struct ctx *c) > inet_ntop(AF_INET, &c->ip4.map_host_loopback, > buf, sizeof(buf))); > =20 > - if (!c->no_dhcp) { > + a =3D fwd_get_addr(c, AF_INET, 0, 0); > + if (a && !c->no_dhcp) { > uint32_t mask; > =20 > - mask =3D htonl(0xffffffff << (32 - c->ip4.prefix_len)); > + mask =3D IN4_MASK(inany_prefix_len(a)); > =20 > info("DHCP:"); > - info(" assign: %s", > - inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf))); > - info(" mask: %s", > - inet_ntop(AF_INET, &mask, buf, sizeof(buf))); > - info(" router: %s", > - inet_ntop(AF_INET, &c->ip4.guest_gw, > - buf, sizeof(buf))); > + inany_ntop(&a->addr, buf, sizeof(buf)); > + info(" assign: %s", buf); > + inet_ntop(AF_INET, &mask, buf, sizeof(buf)); > + info(" mask: %s", buf); > + inet_ntop(AF_INET, &c->ip4.guest_gw, buf, sizeof(buf)); > + info(" router: %s", buf); > } > =20 > for (i =3D 0; i < ARRAY_SIZE(c->ip4.dns); i++) { > @@ -1228,13 +1231,14 @@ static void conf_print(const struct ctx *c) > else > goto dns6; > =20 > - info(" assign: %s", > - inet_ntop(AF_INET6, &c->ip6.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))); > + a =3D fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); > + if (a) > + inany_ntop(&a->addr, buf, sizeof(buf)); > + info(" assign: %s", !a ? "" : buf); > + inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf)); > + info(" router: %s", buf); > + inet_ntop(AF_INET6, &c->ip6.our_tap_ll, buf, sizeof(buf)); > + info(" our link-local: %s", buf); > =20 > dns6: > for (i =3D 0; i < ARRAY_SIZE(c->ip6.dns); i++) { > @@ -1887,16 +1891,12 @@ void conf(struct ctx *c, int argc, char **argv) > IN6_IS_ADDR_V4COMPAT(&addr.a6)) > die("Invalid address: %s", optarg); > =20 > - if (inany_v4(&addr)) { > - c->ip4.addr =3D *inany_v4(&addr); > - c->ip4.prefix_len =3D prefix_len - 96; > - if (c->mode =3D=3D MODE_PASTA) > - c->ip4.no_copy_addrs =3D true; > - } else { > - c->ip6.addr =3D addr.a6; > - if (c->mode =3D=3D MODE_PASTA) > - c->ip6.no_copy_addrs =3D true; > - } > + /* Legacy behaviour: replace existing address if any */ > + fwd_set_addr(c, &addr, CONF_ADDR_USER, prefix_len); > + if (inany_v4(&addr)) > + c->ip4.no_copy_addrs =3D true; > + else > + c->ip6.no_copy_addrs =3D true; > break; > } > case 'n': { > @@ -1910,7 +1910,13 @@ void conf(struct ctx *c, int argc, char **argv) > die("Invalid prefix length: %s", optarg); > =20 > prefix_len_from_opt =3D plen + 96; > - c->ip4.prefix_len =3D plen; > + > + for (int i =3D 0; i < c->addr_count; i++) { You can use for_each_addr() here, no? > + if (!inany_v4(&c->addrs[i].addr)) > + continue; > + c->addrs[i].prefix_len =3D prefix_len_from_opt; > + break; > + } > break; > } > case 'M': > @@ -2104,9 +2110,9 @@ void conf(struct ctx *c, int argc, char **argv) > =20 > nl_sock_init(c, false); > if (!v6_only && !c->splice_only) > - c->ifi4 =3D conf_ip4(ifi4, &c->ip4); > + c->ifi4 =3D conf_ip4(c, ifi4); > if (!v4_only && !c->splice_only) > - c->ifi6 =3D conf_ip6(ifi6, &c->ip6); > + c->ifi6 =3D conf_ip6(c, ifi6); > =20 > if (c->ifi4 && c->mtu < IPV4_MIN_MTU) { > warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)", > @@ -2129,7 +2135,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 =3D -1; > } > @@ -2137,7 +2143,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 =3D -1; > } > @@ -2202,7 +2208,7 @@ 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 (!fwd_get_addr(c, AF_INET6, 0, 0)) { > c->no_dhcpv6 =3D 1; > } > =20 > diff --git a/dhcp.c b/dhcp.c > index 1ff8cba..6d9def5 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -302,6 +302,7 @@ static void opt_set_dns_search(const struct ctx *c, s= ize_t max_len) > */ > int dhcp(const struct ctx *c, struct iov_tail *data) > { > + const struct guest_addr *a =3D fwd_get_addr(c, AF_INET, 0, 0); > char macstr[ETH_ADDRSTRLEN]; > size_t mlen, dlen, opt_len; > struct in_addr mask, dst; > @@ -313,6 +314,7 @@ 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; > =20 > @@ -344,6 +346,9 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > m->op !=3D BOOTREQUEST) > return -1; > =20 > + ASSERT(a); Rebasing will require s/ASSERT/assert/ > + addr =3D *inany_v4(&a->addr); > + > reply.op =3D BOOTREPLY; > reply.htype =3D m->htype; > reply.hlen =3D m->hlen; > @@ -352,7 +357,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 addr; > reply.siaddr =3D 0; > reply.giaddr =3D m->giaddr; > memcpy(&reply.chaddr, m->chaddr, sizeof(reply.chaddr)); > @@ -404,7 +409,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 htonl(0xffffffff << (32 - c->ip4.prefix_len)); > + mask.s_addr =3D IN4_MASK(inany_prefix_len(a)); > 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 +417,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) > !=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; > @@ -471,7 +476,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 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 97c04e2..3a007bf 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] =3D { OPT_IA_NA, OPT_IA_TA }; > struct opt_ia_addr opt_addr_storage; > @@ -566,6 +566,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; > @@ -573,6 +574,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > const struct udphdr *uh; > size_t mlen, n; > =20 > + a =3D fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); > + > uh =3D IOV_REMOVE_HEADER(data, uh_storage); > if (!uh) > return -1; > @@ -628,7 +631,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 (a && dhcpv6_ia_notonlink(data, &a->addr.a6)) { > =20 > dhcpv6_send_ia_notonlink(c, data, &client_id_base, > ntohs(client_id->l), mh->xid); > @@ -682,7 +685,8 @@ 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; > + if (a) > + c->ip6.addr_seen =3D a->addr.a6; > =20 > return 1; > } > @@ -693,6 +697,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > */ > void dhcpv6_init(const struct ctx *c) > { > + const struct guest_addr *a; > time_t y2k =3D 946684800; /* Epoch to 2000-01-01T00:00:00Z, no mktime()= */ > uint32_t duid_time; > =20 > @@ -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)); > =20 > - resp.ia_addr.addr =3D c->ip6.addr; > + a =3D fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); > + if (a) > + resp.ia_addr.addr =3D a->addr.a6; > } > diff --git a/fwd.c b/fwd.c > index bedbf98..9101d6d 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -249,6 +249,64 @@ void fwd_neigh_table_init(const struct ctx *c) > fwd_neigh_table_update(c, &mga, c->our_tap_mac, true); > } > =20 > +/** > + * 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/mapped format, 0-128 > + * > + * 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 (int i =3D 0; i < c->addr_count; i++) { > + a =3D &c->addrs[i]; > + if ((inany_v4(addr) && inany_v4(&a->addr)) || > + (!inany_v4(addr) && !inany_v4(&a->addr))) !!inany_v4(addr) =3D=3D !!inany_v4(&a->addr) > + goto found; > + } > + > + if (c->addr_count >=3D MAX_GUEST_ADDRS) > + return; > + > + a =3D &c->addrs[c->addr_count++]; > + > +found: > + a->addr =3D *addr; > + a->prefix_len =3D prefix_len; > + a->flags =3D flags; > +} > + > +/** > + * fwd_get_addr() - Get guest address entry matching criteria > + * @c: Execution context > + * @af: Address family (AF_INET, AF_INET6, or 0 for any) > + * @incl: Flags that must be present (any-match) > + * @excl: Flags that must not be present Theoretically you could filter on any boolean expression of the flags. I guess I wonder if "any of these but none of those" is sufficiently broad for whatever we're likely to want. I don't love that incl=3D=3D0 is a special case, though I can see the argument from convenience. > + * > + * Return: first address entry matching criteria, or NULL > + */ > +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t a= f, > + uint8_t incl, uint8_t excl) > +{ > + const struct guest_addr *a; > + > + for_each_addr(a, c, af) { > + if (incl && !(a->flags & incl)) > + continue; > + if (a->flags & excl) > + continue; > + return a; > + } > + > + return NULL; > +} > + > /** fwd_probe_ephemeral() - Determine what ports this host considers eph= emeral > * > * Work out what ports the host thinks are emphemeral and record it for = later > @@ -941,8 +999,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 =3D fwd_get_addr(c, AF_INET, 0, 0); > + > if (IN4_IS_ADDR_LOOPBACK(addr)) > return false; > =20 > @@ -957,7 +1017,7 @@ 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 ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || > IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) > return false; > =20 > @@ -975,10 +1035,12 @@ 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 =3D fwd_get_addr(c, AF_INET6, 0, 0); > + > if (IN6_IS_ADDR_LOOPBACK(addr)) > return false; > =20 > - if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr)) > + if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) > return false; > =20 > /* For IPv6, addr_seen starts unspecified, because we don't know what LL > @@ -1023,14 +1085,19 @@ static bool fwd_guest_accessible(const struct ctx= *c, > static void nat_outbound(const struct ctx *c, const union inany_addr *ad= dr, > union inany_addr *translated) > { > + const struct guest_addr *ga; > + > if (inany_equals4(addr, &c->ip4.map_host_loopback)) > *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 (inany_equals4(addr, &c->ip4.map_guest_addr)) { > + ga =3D fwd_get_addr(c, AF_INET, 0, 0); > + *translated =3D ga ? ga->addr : inany_any4; > + } else if (inany_equals6(addr, &c->ip6.map_guest_addr)) { > + ga =3D fwd_get_addr(c, AF_INET6, 0, 0); > + translated->a6 =3D ga ? ga->addr.a6 : in6addr_any; > + } > else > *translated =3D *addr; > } > @@ -1137,16 +1204,21 @@ 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 =3D c->ip6.map_host_loopback; > - } else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && > - inany_equals4(addr, &c->ip4.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)) { > - translated->a6 =3D c->ip6.map_guest_addr; > - } else if (fwd_guest_accessible(c, addr)) { > - *translated =3D *addr; > } else { > - return false; > + const struct guest_addr *ga4 =3D fwd_get_addr(c, AF_INET, 0, 0); > + const struct guest_addr *ga6 =3D fwd_get_addr(c, AF_INET6, 0, 0); > + > + if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && > + ga4 && inany_equals(addr, &ga4->addr)) { > + *translated =3D 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 =3D c->ip6.map_guest_addr; > + } else if (fwd_guest_accessible(c, addr)) { > + *translated =3D *addr; > + } else { > + return false; > + } > } > =20 > return true; > diff --git a/fwd.h b/fwd.h > index 958eee2..c5a1068 100644 > --- a/fwd.h > +++ b/fwd.h > @@ -23,6 +23,8 @@ struct flowside; > =20 > void fwd_probe_ephemeral(void); > bool fwd_port_is_ephemeral(in_port_t port); > +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t a= f, > + uint8_t incl, uint8_t excl); > =20 > /** > * struct fwd_rule - Forwarding rule governing a range of ports > @@ -141,5 +143,7 @@ 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); > =20 > #endif /* FWD_H */ > diff --git a/igmp.c b/igmp.c > index a3971fc..0e77584 100644 > --- a/igmp.c > +++ b/igmp.c > @@ -13,4 +13,5 @@ > */ > =20 > /* TO BE IMPLEMENTED */ > +/* cppcheck-suppress unusedFunction */ This seems an unrelated change for the patch. > __attribute__((__unused__)) static void unused(void) { } > diff --git a/ip.h b/ip.h > index d0de6c8..933d98c 100644 > --- a/ip.h > +++ b/ip.h > @@ -19,6 +19,8 @@ > (ntohl(((struct in_addr *)(a))->s_addr) >> IN_CLASSA_NSHIFT =3D=3D IN_L= OOPBACKNET) > #define IN4_IS_ADDR_MULTICAST(a) \ > (IN_MULTICAST(ntohl(((struct in_addr *)(a))->s_addr))) > +#define IN4_MASK(prefix) \ > + ((prefix) <=3D 0 ? 0 : htonl(0xffffffff << (32 - (prefix)))) > #define IN4_ARE_ADDR_EQUAL(a, b) \ > (((struct in_addr *)(a))->s_addr =3D=3D ((struct in_addr *)b)->s_addr) > #define IN4ADDR_LOOPBACK_INIT \ > diff --git a/ndp.c b/ndp.c > index 1f2bcb0..3750fc5 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 =3D ~0U, > .pref_lifetime =3D ~0U, > }, > - .prefix =3D c->ip6.addr, > .source_ll =3D { > .header =3D { > .type =3D OPT_SRC_L2_ADDR, > @@ -265,8 +264,13 @@ static void ndp_ra(const struct ctx *c, const struct= in6_addr *dst) > }, > }, > }; > + const struct guest_addr *a =3D fwd_get_addr(c, AF_INET6, 0, 0); > unsigned char *ptr =3D NULL; > =20 > + ASSERT(a); > + > + ra.prefix =3D a->addr.a6; > + > ptr =3D &ra.var[0]; > =20 > if (c->mtu) { > @@ -460,6 +464,7 @@ first: > */ > void ndp_send_init_req(const struct ctx *c) > { > + const struct guest_addr *a =3D fwd_get_addr(c, AF_INET6, 0, 0); > struct ndp_ns ns =3D { > .ih =3D { > .icmp6_type =3D NS, > @@ -468,8 +473,13 @@ 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 IN6ADDR_ANY_INIT > }; > + > + if (!a) > + return; > + > + ns.target_addr =3D 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 b614bdf..6bca778 100644 > --- a/passt.h > +++ b/passt.h > @@ -64,11 +64,31 @@ enum passt_modes { > MODE_VU, > }; > =20 > +/* Maximum number of addresses in context address array */ > +#define MAX_GUEST_ADDRS 32 > + > +/* Flags indicating origin and role of a guest address > + * To be used in struct guest_addr > + */ > +#define CONF_ADDR_USER BIT(0) /* User set via -a */ > +#define CONF_ADDR_HOST BIT(1) /* From host interface */ > +#define CONF_ADDR_LINKLOCAL BIT(2) /* Link-local address */ > + > +/** > + * 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; > +}; > + > /** > * 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 +104,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 +124,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 +141,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; > @@ -181,6 +195,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 > @@ -260,6 +276,9 @@ struct ctx { > int ifi6; > struct ip6_ctx ip6; > =20 > + struct guest_addr addrs[MAX_GUEST_ADDRS]; > + int addr_count; > + > char pasta_ifn[IF_NAMESIZE]; > unsigned int pasta_ifi; > int pasta_conf_ns; > @@ -301,6 +320,53 @@ struct ctx { > bool migrate_exit; > }; > =20 > +/** > + * inany_prefix_len() - Get address-family-appropriate prefix length > + * @a: Address entry > + * > + * Return: prefix length in native format (0-32 for IPv4, 0-128 for IPv6) I don't think this is a good interface: in order to properly use the result from this, you must already know if you're working with an IPv4 or IPv6 address, making the test inside redundant. The only current users know they're expecting a v4 address. A better option might be an extension/variant of inany_v4() which returns the v4-format prefix as well as the address itself when v4. > + */ > +static inline int inany_prefix_len(const struct guest_addr *a) > +{ > + if (inany_v4(&a->addr)) > + return a->prefix_len - 96; > + return a->prefix_len; > +} > + > +/** > + * _next_addr_idx() - Find next address index matching family filter Leading _ is reserved for system identifiers. Use a trailing _ if you just want to indicate an "internal" function. (The kernel can do this because it knows it is controlling the entire build; we link with the regular C library, so we can't). > + * @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, sa_family_t= af) > +{ > + for (; i < c->addr_count; i++) { > + sa_family_t entry_af; > + > + entry_af =3D inany_v4(&c->addrs[i].addr) ? AF_INET : AF_INET6; > + > + if (af =3D=3D AF_UNSPEC || af =3D=3D entry_af) > + return i; > + } > + return i; > +} > + > +/** > + * for_each_addr() - Iterate over addresses in unified array > + * @a: Pointer variable for current entry (struct guest_addr *) > + * @c: Pointer to struct ctx > + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all > + * > + * Note: @_i is the internal loop counter, uses _next_addr_idx() helper > + */ > +#define for_each_addr(a, c, af) \ > + for (int _i =3D _next_addr_idx((c), 0, (af)); \ > + _i < (c)->addr_count && ((a) =3D &(c)->addrs[_i], true); \ Fwiw, I think (a) =3D &(c)->addrs[i_], i_ < (c)->addr_count would also be valid. By the loop construction i_ should never be strictly greater than addr_count, and C allows you to to construct (but not dereference) a pointer at exactly the end of an allocated block. > + _i =3D _next_addr_idx((c), _i + 1, (af))) > + > void proto_update_l2_buf(const unsigned char *eth_d); > =20 > #endif /* PASST_H */ > diff --git a/pasta.c b/pasta.c > index bab945f..e88a893 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -330,6 +330,7 @@ void pasta_ns_conf(struct ctx *c) > =20 > if (c->pasta_conf_ns) { > unsigned int flags =3D IFF_UP; > + const struct guest_addr *a; > =20 > if (c->mtu) > nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); > @@ -341,10 +342,12 @@ void pasta_ns_conf(struct ctx *c) > =20 > if (c->ifi4) { > if (c->ip4.no_copy_addrs) { > - rc =3D nl_addr_set(nl_sock_ns, c->pasta_ifi, > - AF_INET, > - &c->ip4.addr, > - c->ip4.prefix_len); > + a =3D fwd_get_addr(c, AF_INET, 0, 0); > + if (a) > + rc =3D nl_addr_set(nl_sock_ns, > + c->pasta_ifi, AF_INET, > + inany_v4(&a->addr), > + a->prefix_len - 96); > } else { > rc =3D nl_addr_dup(nl_sock, c->ifi4, > nl_sock_ns, c->pasta_ifi, > @@ -397,11 +400,13 @@ ipv4_done: > 0, IFF_NOARP); > =20 > if (c->ip6.no_copy_addrs) { > - if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { > + a =3D fwd_get_addr(c, AF_INET6, 0, 0); > + if (a) > rc =3D 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 =3D nl_addr_dup(nl_sock, c->ifi6, > nl_sock_ns, c->pasta_ifi, > diff --git a/tap.c b/tap.c > index eaa6111..78a494a 100644 > --- a/tap.c > +++ b/tap.c > @@ -951,8 +951,11 @@ resume: > c->ip6.addr_seen =3D *saddr; > } > =20 > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) > - c->ip6.addr =3D *saddr; > + if (!fwd_get_addr(c, AF_INET6, 0, 0)) { > + union inany_addr addr =3D { .a6 =3D *saddr }; > + > + fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64); > + } > } 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 --ar5JSBx9xR5IaKCa Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnCBYYACgkQzQJF27ox 2GfveQ//Ty+nTX+wzDZug+ZAd8B/fJqgkBd/KNLya5XsoxrwL10rAsx4NAIXhVPl vVmuoDiAol4BIVoEiv9ntcPuuzJ4GSH1delkbPYUfaDuirxmz2jzvr98XtrdYDs8 0O7vlbvv66CImiPu30/D9w7haqC8ayw96EOjhVJSMFnzjwEqq8gTNEoCq17Jebya hm09csTDO819AolMrd+cS46QCDx/lW1cAAKlhhMkLOEtv1xJ/99SIBxapFBEjL9D jhIKSTlfcgBAU/gSJYUwyLrQZMZAWCDLQdFjoSG5DmwgM+GXeHcwdojFjPabdDky y5bSGY5i6Ug9amrgoJOZ4w74Sr+VM8vkfBi9EBHCT8cXiVIm1c4h+qP6Q4RhLFul yAg6NRZJFjKxG67WYq3b5Vk6kwWmAHyDNBNyCm6gPN0Z4uQotLRxQHGRUG1YyE20 4eB2F8u0j/PTR/r+sJ+m++305ltdfLaAmDiJ5T8NFqBtibZkHefEcfvp60fIHj7z rDzXp9FnnPsmN7fN9W7GNE8etUlyf6Fff6ZmObKW0l1/9h/zIjgNe2M1M3FoAImE RbT5pojYZbOBLvv9lb0Iu8FTFANwTN7E4RuFmlRl/O2z/aHE5HdBBm/U1BKUhDUQ /TGJUPexyNiw+bvGUamY9rlwFBJ/HDxnwjBKCs/Fc+ELoHQJmMo= =HZaC -----END PGP SIGNATURE----- --ar5JSBx9xR5IaKCa--