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=dnDgY4XO; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 1C5455A0265 for ; Tue, 03 Mar 2026 02:52:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1772502755; bh=3q9kzF02N5qHXx0KFEibMycxN4sexK2p0xtNb0s4TjY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=dnDgY4XOzVTU6oRjKUT5fiIVsz1GsXN6LJlwLdUu+cQ+U/FzmjuG7PzgCfSXYyl3m cHp5Ti+IfERk6oZ2VuIpO2SOKZd4SLn1H2DrRpEH+pmbooKOsBHn84K9/pT8qaXIaF JmLlWgx298M3IaoUWUuyj3rsIRa3JCfwJHwDgcRcpTzZStz+lwUc6wiiQuxI1bznsp q279dyrO3enSPpNe67ZQfGqdB06tS9a7TAA5PEESxmZFF1ymoO97YwCDXACt9tPGJ7 fufbZ8M26v/FcFEk5RFptH6/zGVqMn4USjoixZrkEyzXhUqp8YvRVZyUTJByoLengS U6aPtOiiXKDlA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fPzMR06QZz4wCZ; Tue, 03 Mar 2026 12:52:35 +1100 (AEDT) Date: Tue, 3 Mar 2026 12:52:31 +1100 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v5 08/13] ip: Track observed guest IPv6 addresses in unified address array Message-ID: References: <20260222174445.743845-1-jmaloy@redhat.com> <20260222174445.743845-9-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Ql8DXpheIU261ulm" Content-Disposition: inline In-Reply-To: <20260222174445.743845-9-jmaloy@redhat.com> Message-ID-Hash: ZSQBSUVB5QZZKMAHJSHER2LB3Y62REHF X-Message-ID-Hash: ZSQBSUVB5QZZKMAHJSHER2LB3Y62REHF 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: --Ql8DXpheIU261ulm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 22, 2026 at 12:44:40PM -0500, Jon Maloy wrote: > We remove the addr_seen and addr_ll_seen fields in struct ip6_ctx > and replace them by setting CONF_ADDR_OBSERVED and CONF_ADDR_LINKLOCAL > flags in the corresponding entry in the unified address array. >=20 > The observed IPv6 address is always added/moved to position 0 > in the array, improving chances for fast lookup. >=20 > This completes the unification of address storage for both IPv4 and > IPv6, enabling future support for multiple guest addresses per family. >=20 > Signed-off-by: Jon Maloy >=20 > --- > v5: - Made to use same algorithm and function as IPv4 for inserting > observed into the array. > --- > conf.c | 2 -- > dhcpv6.c | 6 +----- > dhcpv6.h | 2 +- > migrate.c | 27 +++++++++++++++++++------- > passt.h | 4 ---- > pasta.c | 17 +++++++++++++---- > tap.c | 57 +++++++++++++++++++++++++++++++++---------------------- > 7 files changed, 69 insertions(+), 46 deletions(-) As noted in the previous patch, there are several hunks from there that ought to be in this patch. > diff --git a/conf.c b/conf.c > index faa60ec..f622424 100644 > --- a/conf.c > +++ b/conf.c > @@ -816,8 +816,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned = int ifi) > e->flags =3D CONF_ADDR_HOST; > } > =20 > - ip6->addr_seen =3D e->addr.a6; > - > if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) > ip6->our_tap_ll =3D ip6->guest_gw; > =20 > diff --git a/dhcpv6.c b/dhcpv6.c > index 55c17cf..ba200e5 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -549,7 +549,7 @@ static size_t dhcpv6_client_fqdn_fill(const struct io= v_tail *data, > * Return: 0 if it's not a DHCPv6 message, 1 if handled, -1 on failure > */ > int dhcpv6(struct ctx *c, struct iov_tail *data, > - const struct in6_addr *saddr, const struct in6_addr *daddr) > + const struct in6_addr *daddr) > { > const struct opt_server_id *server_id =3D NULL; > struct inany_addr_entry *e =3D first_v6(c); > @@ -591,8 +591,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > if (mlen + sizeof(*uh) !=3D ntohs(uh->len) || mlen < sizeof(*mh)) > return -1; > =20 > - c->ip6.addr_ll_seen =3D *saddr; > - Removing this without a replacement doesn't look correct to me. Possibly it's redundant anyway with code in tap.c, but if that's so removing it in a separate patch with its own rationale would make it clearer. > src =3D &c->ip6.our_tap_ll; > =20 > mh =3D IOV_REMOVE_HEADER(data, mh_storage); > @@ -683,8 +681,6 @@ 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); > - if (e) > - c->ip6.addr_seen =3D e->addr.a6; > =20 > return 1; > } > diff --git a/dhcpv6.h b/dhcpv6.h > index c706dfd..8cbc769 100644 > --- a/dhcpv6.h > +++ b/dhcpv6.h > @@ -7,7 +7,7 @@ > #define DHCPV6_H > =20 > int dhcpv6(struct ctx *c, struct iov_tail *data, > - struct in6_addr *saddr, struct in6_addr *daddr); > + struct in6_addr *daddr); > void dhcpv6_init(const struct ctx *c); > =20 > #endif /* DHCPV6_H */ > diff --git a/migrate.c b/migrate.c > index f026e95..1e2830f 100644 > --- a/migrate.c > +++ b/migrate.c > @@ -56,10 +56,7 @@ struct migrate_seen_addrs_v1 { > static int seen_addrs_source_v1(struct ctx *c, > const struct migrate_stage *stage, int fd) > { > - struct migrate_seen_addrs_v1 addrs =3D { > - .addr6 =3D c->ip6.addr_seen, > - .addr6_ll =3D c->ip6.addr_ll_seen, > - }; > + struct migrate_seen_addrs_v1 addrs =3D { 0 }; > const struct inany_addr_entry *e; > =20 > (void)stage; > @@ -69,6 +66,15 @@ static int seen_addrs_source_v1(struct ctx *c, > if (e) > addrs.addr4 =3D *inany_v4(&e->addr); > =20 > + /* IPv6 observed address */ > + e =3D fwd_get_addr(c, AF_INET6, CONF_ADDR_OBSERVED, 0); As for v4, you need a fall back if there's no observed address. > + if (e) { > + if (e->flags & CONF_ADDR_LINKLOCAL) > + addrs.addr6_ll =3D e->addr.a6; > + else > + addrs.addr6 =3D e->addr.a6; > + } This isn't right. You need to populate *both* addr6 and addr6_ll, not one or the other. > + > memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); > =20 > if (write_all_buf(fd, &addrs, sizeof(addrs))) > @@ -89,6 +95,7 @@ static int seen_addrs_target_v1(struct ctx *c, > const struct migrate_stage *stage, int fd) > { > struct migrate_seen_addrs_v1 addrs; > + struct in6_addr addr6, addr6_ll; > struct in_addr addr4; > =20 > (void)stage; > @@ -96,14 +103,20 @@ static int seen_addrs_target_v1(struct ctx *c, > if (read_all_buf(fd, &addrs, sizeof(addrs))) > return errno; > =20 > - c->ip6.addr_seen =3D addrs.addr6; > - c->ip6.addr_ll_seen =3D addrs.addr6_ll; > - > /* Copy from packed struct to avoid alignment issues */ > addr4 =3D addrs.addr4; > + addr6 =3D addrs.addr6; > + addr6_ll =3D addrs.addr6_ll; > + > if (addr4.s_addr) > fwd_set_addr(c, &inany_from_v4(addr4), CONF_ADDR_OBSERVED, 0); > =20 > + /* Prefer global over link-local if both present */ > + if (!IN6_IS_ADDR_UNSPECIFIED(&addr6)) > + fwd_set_addr(c, &inany_from_v6(addr6), CONF_ADDR_OBSERVED, 0); > + else if (!IN6_IS_ADDR_UNSPECIFIED(&addr6_ll)) > + fwd_set_addr(c, &inany_from_v6(addr6_ll), CONF_ADDR_OBSERVED, 0); Likewise, you need to put both the LL and non-LL observed addresses back into the table, not just one. > memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac)); > =20 > return 0; > diff --git a/passt.h b/passt.h > index b808a19..e7489ca 100644 > --- a/passt.h > +++ b/passt.h > @@ -112,8 +112,6 @@ struct ip4_ctx { > =20 > /** > * struct ip6_ctx - IPv6 execution context > - * @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 > * @map_host_loopback: Outbound connections to this address are NATted t= o the > * host's [::1] > @@ -129,8 +127,6 @@ struct ip4_ctx { > * @no_copy_addrs: Don't copy all addresses when configuring namespace > */ > struct ip6_ctx { > - struct in6_addr addr_seen; > - struct in6_addr addr_ll_seen; > struct in6_addr guest_gw; > struct in6_addr map_host_loopback; > struct in6_addr map_guest_addr; > diff --git a/pasta.c b/pasta.c > index 7a081e7..56b2e0a 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -46,6 +46,8 @@ > =20 > #include "util.h" > #include "passt.h" > +#include "conf.h" > +#include "fwd.h" > #include "isolation.h" > #include "netlink.h" > #include "log.h" > @@ -384,12 +386,15 @@ void pasta_ns_conf(struct ctx *c) > ipv4_done: > =20 > if (c->ifi6) { > - rc =3D nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, > - &c->ip6.addr_ll_seen); > - if (rc < 0) { > + struct in6_addr addr_ll; > + > + rc =3D nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, &addr_ll); > + if (rc < 0) > warn("Can't get LL address from namespace: %s", > strerror_(-rc)); > - } > + else > + fwd_set_addr(c, &inany_from_v6(addr_ll), > + CONF_ADDR_OBSERVED, 0); > =20 > rc =3D nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); > if (rc < 0) { > @@ -403,6 +408,10 @@ ipv4_done: > =20 > if (c->ip6.no_copy_addrs) { > for_each_addr(e, c, AF_INET6) { > + /* Skip, kernel auto-configures */ > + if (e->flags & CONF_ADDR_LINKLOCAL) > + continue; This seems like it belongs in an earlier patch - it's not really related to observed addresses specifically. > rc =3D nl_addr_set(nl_sock_ns, > c->pasta_ifi, > AF_INET6, &e->addr.a6, > diff --git a/tap.c b/tap.c > index 30e52f7..875c2bf 100644 > --- a/tap.c > +++ b/tap.c > @@ -174,6 +174,17 @@ static void tap_check_src_addr4(struct ctx *c, const= struct in_addr *addr) > fwd_set_addr(c, &inany_from_v4(*addr), CONF_ADDR_OBSERVED, 0); > } > =20 > +/** > + * tap_check_src_addr6() - Note an IPv6 address seen in guest traffic > + * @c: Execution context > + * @addr: IPv6 address seen as source from guest > + */ > +static void tap_check_src_addr6(struct ctx *c, const struct in6_addr *ad= dr) > +{ > + if (!IN6_IS_ADDR_UNSPECIFIED(addr)) > + fwd_set_addr(c, &inany_from_v6(*addr), CONF_ADDR_OBSERVED, 0); > +} > + > /** > * tap_ip6_daddr() - Normal IPv6 destination address for inbound packets > * @c: Execution context > @@ -184,9 +195,25 @@ static void tap_check_src_addr4(struct ctx *c, const= struct in_addr *addr) > const struct in6_addr *tap_ip6_daddr(const struct ctx *c, > const struct in6_addr *src) > { > - if (IN6_IS_ADDR_LINKLOCAL(src)) > - return &c->ip6.addr_ll_seen; > - return &c->ip6.addr_seen; > + const struct inany_addr_entry *e; > + > + if (IN6_IS_ADDR_LINKLOCAL(src)) { > + /* Link-local: first LL address (observed is at front) */ > + e =3D fwd_get_addr(c, AF_INET6, CONF_ADDR_LINKLOCAL, 0); > + } else { > + /* Global: observed non-LL first, then any non-LL */ Do you need this in addition to your logic to put observed addresses at the beginning of the array? > + e =3D fwd_get_addr(c, AF_INET6, CONF_ADDR_OBSERVED, > + CONF_ADDR_LINKLOCAL); > + if (!e) > + e =3D fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); > + } > + > + if (e) > + return &e->addr.a6; > + > + /* Last resort: first IPv6 address */ > + e =3D first_v6(c); I think we should fail rather than falling back further. We really mustn't mix linklocal and non-linklocal addresses in a packet. > + return e ? &e->addr.a6 : &in6addr_any; > } > =20 > /** > @@ -786,7 +813,7 @@ resume: > =20 > if (iph->saddr) > tap_check_src_addr4(c, > - (const struct in_addr *)&iph->saddr); > + (const struct in_addr *)&iph->saddr); Unrelated whitespace change > =20 > if (!iov_drop_header(&data, hlen)) > continue; > @@ -958,24 +985,8 @@ resume: > continue; > } > =20 > - if (IN6_IS_ADDR_LINKLOCAL(saddr)) { > - c->ip6.addr_ll_seen =3D *saddr; > - > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen)) { > - c->ip6.addr_seen =3D *saddr; > - } > - > - if (!first_v6(c) && c->addr_count < INANY_MAX_ADDRS) { > - struct inany_addr_entry *e; > - > - e =3D &c->addrs[c->addr_count++]; > - e->addr.a6 =3D *saddr; > - e->prefix_len =3D 64; > - e->flags =3D CONF_ADDR_LINKLOCAL; > - } > - } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ > - c->ip6.addr_seen =3D *saddr; > - } > + if (!IN6_IS_ADDR_UNSPECIFIED(saddr)) Redundant with the check inside tap_check_src_addr6(). > + tap_check_src_addr6(c, saddr); > =20 > if (proto =3D=3D IPPROTO_ICMPV6) { > struct iov_tail ndp_data; > @@ -1006,7 +1017,7 @@ resume: > if (proto =3D=3D IPPROTO_UDP) { > struct iov_tail uh_data =3D data; > =20 > - if (dhcpv6(c, &uh_data, saddr, daddr)) > + if (dhcpv6(c, &uh_data, daddr)) > continue; > } > =20 > --=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 --Ql8DXpheIU261ulm Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmmPt4ACgkQzQJF27ox 2GcSlQ/9GxPCYobgAOolmboXbX8o5OZZvTbDhwEVe0InK8Be7xgEJJrn6r6e5UgL 6BpLMjVlHrVfAYV8ZBZso2nJHmA3xP8ncIFsadi8ixCxhPcjTjheicQOZyoXlvbO XfwFW1SDYCaxc/H6RlVOJjQKD4qa+UYqgE6LOjPpGuO6J0D754QYSIlopTeEGO1X Z3OpfK0TEkkssY2IQawZN4hgqCc/zPim6D6mlmDHWKTUtKnwtU2xTb+6l/wFrfUx bwGKQdJNYqzy5TVCN8n4exXZbMAwtMD+ppdTBjAd79OB27HyC+MxEy7dP6OFzo41 WizBKSXeLmSI447HJE8gsrG2xNu5X+jFPA0VijIgcGs+BK4ckNfBmvhMXRzUN1Zr 0rgbq/qsWiIfIdbqjvxfef6EE3/d8BoY77eGM6CHvAmwPdI4tpalW0OVEU0Nf7uP ieeU+whwdtZkXQdunAuA+VltxoZQRKsi+i3o4GJrCdp65uoGrKBKQ9KA22s8Pfps 2upu1ZlYNFsTqAY3fE4PQR9NWPcNpqmuqIXEFieZ7LwAqRV7uWOhvmHSnJ8QFgBy Z01px3lY3kfu/WRye2bvfVuBPMpf7Z5AAqE4/1bAnZxZigUPeqFiUDgTCUgcFB0X GY81zL9u2pIveA1xti1nW/kAWdKj3sA0eiPj4V+1gkSSwxE9Xe4= =Tcd2 -----END PGP SIGNATURE----- --Ql8DXpheIU261ulm--