On Sat, Mar 21, 2026 at 08:43:29PM -0400, 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. > > The observed IPv6 address is always added/moved to position 0 > in the array, improving chances for fast lookup. > > This completes the unification of address storage for both IPv4 and > IPv6, enabling future support for multiple guest addresses per family. > > Signed-off-by: Jon Maloy > > --- > v5: - Made to use same algorithm and function as IPv4 for inserting > observed into the array. > > v6: - Re-introduced code that by accident had been moved to the > previous commit. > - Some fixes based on feedback from David G. > --- > conf.c | 6 ------ > dhcpv6.c | 20 +++++++++++-------- > dhcpv6.h | 2 +- > fwd.c | 38 ++++++++++++++++++++++-------------- > inany.h | 3 +++ > migrate.c | 33 +++++++++++++++++++++++-------- > passt.h | 4 ---- > pasta.c | 7 +++++-- > tap.c | 58 ++++++++++++++++++++++++++++++++++++------------------- > 9 files changed, 107 insertions(+), 64 deletions(-) > > diff --git a/conf.c b/conf.c > index 1c9f07c..320a9e4 100644 > --- a/conf.c > +++ b/conf.c > @@ -767,8 +767,6 @@ static unsigned int conf_ip4(struct ctx *c, unsigned int ifi) > } > if (!rc || !fwd_get_addr(c, AF_INET, 0, 0)) > return 0; > - > - a = fwd_get_addr(c, AF_INET, CONF_ADDR_HOST, 0); This looks like it belongs in a different patch. > } > > ip4->our_tap_addr = ip4->guest_gw; > @@ -829,7 +827,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) > strerror_(-rc)); > return 0; > } > - a = fwd_get_addr(c, AF_INET6, CONF_ADDR_HOST, 0); > } else { > rc = nl_addr_get_ll(nl_sock, ifi, &ip6->our_tap_ll); > if (rc < 0) { > @@ -838,9 +835,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) > } > } > > - if (a) > - ip6->addr_seen = a->addr.a6; > - > if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) > ip6->our_tap_ll = ip6->guest_gw; > > diff --git a/dhcpv6.c b/dhcpv6.c > index 3a007bf..313c243 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -382,8 +382,12 @@ static void dhcpv6_send_ia_notonlink(struct ctx *c, > { > const struct in6_addr *src = &c->ip6.our_tap_ll; > struct opt_hdr *ia = (struct opt_hdr *)resp_not_on_link.var; > + const struct in6_addr *dst = tap_ip6_daddr(c, src); > size_t n; > > + if (!dst) > + return; > + > info("DHCPv6: received CONFIRM with inappropriate IA," > " sending NotOnLink status in REPLY"); > > @@ -405,7 +409,7 @@ static void dhcpv6_send_ia_notonlink(struct ctx *c, > > resp_not_on_link.hdr.xid = xid; > > - tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, > + tap_udp6_send(c, src, 547, dst, 546, > xid, &resp_not_on_link, n); > } > > @@ -549,7 +553,7 @@ static size_t dhcpv6_client_fqdn_fill(const struct iov_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 = NULL; > const struct opt_hdr *client_id = NULL; > @@ -565,9 +569,9 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > /* cppcheck-suppress [variableScope,unmatchedSuppression] */ > struct opt_hdr client_id_storage; > /* cppcheck-suppress [variableScope,unmatchedSuppression] */ > + const struct in6_addr *src, *dst; Inserting this here means the cppcheck suppression now applies to it, instead of what it was supposed to. > 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; > struct udphdr uh_storage; > @@ -593,9 +597,12 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > if (mlen + sizeof(*uh) != ntohs(uh->len) || mlen < sizeof(*mh)) > return -1; > > - c->ip6.addr_ll_seen = *saddr; > + /* Guest LL address already recorded by tap_check_src_addr6() */ Nit: AFAICT, this was already redundant before this patch. So I'd prefer to see it removed as a preliminary cleanup rather than folded into this patch. Since it was already redundant, I don't think the comment is helpful - it's explaining code that's no longer there. > src = &c->ip6.our_tap_ll; > + dst = tap_ip6_daddr(c, src); > + if (!dst) > + return -1; > > mh = IOV_REMOVE_HEADER(data, mh_storage); > if (!mh) > @@ -683,10 +690,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > > resp.hdr.xid = mh->xid; > > - tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, > - mh->xid, &resp, n); > - if (a) > - c->ip6.addr_seen = a->addr.a6; > + tap_udp6_send(c, src, 547, dst, 546, mh->xid, &resp, n); > > return 1; > } > diff --git a/dhcpv6.h b/dhcpv6.h > index c706dfd..eda133f 100644 > --- a/dhcpv6.h > +++ b/dhcpv6.h > @@ -7,7 +7,7 @@ > #define DHCPV6_H > > int dhcpv6(struct ctx *c, struct iov_tail *data, > - struct in6_addr *saddr, struct in6_addr *daddr); > + const struct in6_addr *daddr); > void dhcpv6_init(const struct ctx *c); > > #endif /* DHCPV6_H */ > diff --git a/fwd.c b/fwd.c > index 28a721e..b3f5dc0 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -1095,14 +1095,6 @@ static bool fwd_guest_accessible(const struct ctx *c, > if (inany_equals(addr, &a->addr)) > return false; > > - /* For IPv6, addr_seen starts unspecified, because we don't know what LL > - * address the guest will take until we see it. Only check against it > - * if it has been set to a real address. > - */ > - if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen) && > - inany_equals6(addr, &c->ip6.addr_seen)) > - return false; > - > return true; > } > > @@ -1305,10 +1297,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, > } > tgt->oaddr = inany_any4; > } else { > - if (c->host_lo_to_ns_lo) > + if (c->host_lo_to_ns_lo) { > tgt->eaddr = inany_loopback6; > - else > - tgt->eaddr.a6 = c->ip6.addr_seen; > + } else { > + const struct guest_addr *a; > + > + a = fwd_select_addr(c, AF_INET6, > + CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | > + CONF_ADDR_HOST, > + CONF_ADDR_LINKLOCAL); > + if (!a) > + return PIF_NONE; > + tgt->eaddr = a->addr; > + } > tgt->oaddr = inany_any6; > } > > @@ -1346,10 +1348,16 @@ uint8_t fwd_nat_from_host(const struct ctx *c, > > tgt->eaddr = a->addr; > } else { > - if (inany_is_linklocal6(&tgt->oaddr)) > - tgt->eaddr.a6 = c->ip6.addr_ll_seen; > - else > - tgt->eaddr.a6 = c->ip6.addr_seen; > + bool linklocal = inany_is_linklocal6(&tgt->oaddr); > + int excl = linklocal ? 0 : CONF_ADDR_LINKLOCAL; This correctly excludes linklocal addresses if oaddr is global, but doesn't exclude global addresses if oaddr is linklocal. > + const struct guest_addr *a; > + > + a = fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | CONF_ADDR_HOST, excl); > + if (!a) > + return PIF_NONE; > + > + tgt->eaddr = a->addr; > } > > return PIF_TAP; > diff --git a/inany.h b/inany.h > index 7b23cb0..82dd102 100644 > --- a/inany.h > +++ b/inany.h > @@ -60,6 +60,9 @@ extern const union inany_addr inany_any4; > #define inany_from_v4(a4) \ > ((union inany_addr)INANY_INIT4((a4))) > > +#define inany_from_v6(v6) \ > + ((union inany_addr){ .a6 = (v6) }) > + Hm, there's probably a bunch of existing places we could use this for greater clarity. > /** union sockaddr_inany - Either a sockaddr_in or a sockaddr_in6 > * @sa_family: Address family, AF_INET or AF_INET6 > * @sa: Plain struct sockaddr (useful to avoid casts) > diff --git a/migrate.c b/migrate.c > index 1e02720..a92301b 100644 > --- a/migrate.c > +++ b/migrate.c > @@ -56,21 +56,30 @@ struct migrate_seen_addrs_v2 { > static int seen_addrs_source_v2(struct ctx *c, > const struct migrate_stage *stage, int fd) > { > - struct migrate_seen_addrs_v2 addrs = { > - .addr6 = c->ip6.addr_seen, > - .addr6_ll = c->ip6.addr_ll_seen, > - }; > + struct migrate_seen_addrs_v2 addrs = { 0 }; > const struct guest_addr *a; > > (void)stage; > > - /* IPv4 observed address, with fallback to configured address */ > + /* IPv4 observed address, with fallback to any other non-LL address */ > a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, > CONF_ADDR_USER | CONF_ADDR_HOST, > CONF_ADDR_LINKLOCAL); > if (a) > addrs.addr4 = *inany_v4(&a->addr); > > + /* IPv6 observed address, with fallback to any other non-LL address */ > + a = fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | CONF_ADDR_HOST, > + CONF_ADDR_LINKLOCAL); > + if (a) > + addrs.addr6 = a->addr.a6; > + > + /* IPv6 link-local address */ > + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_LINKLOCAL, 0); > + if (a) > + addrs.addr6_ll = a->addr.a6; > + > memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); > > if (write_all_buf(fd, &addrs, sizeof(addrs))) > @@ -91,19 +100,27 @@ static int seen_addrs_target_v2(struct ctx *c, > const struct migrate_stage *stage, int fd) > { > struct migrate_seen_addrs_v2 addrs; > + struct in6_addr addr6, addr6_ll; > > (void)stage; > > if (read_all_buf(fd, &addrs, sizeof(addrs))) > return errno; > > - c->ip6.addr_seen = addrs.addr6; > - c->ip6.addr_ll_seen = addrs.addr6_ll; > - > if (addrs.addr4.s_addr) > fwd_set_addr(c, &inany_from_v4(addrs.addr4), > CONF_ADDR_OBSERVED, 0); > > + addr6 = addrs.addr6; > + if (!IN6_IS_ADDR_UNSPECIFIED(&addr6)) > + fwd_set_addr(c, &inany_from_v6(addr6), > + CONF_ADDR_OBSERVED, 0); > + > + addr6_ll = addrs.addr6_ll; > + if (!IN6_IS_ADDR_UNSPECIFIED(&addr6_ll)) > + fwd_set_addr(c, &inany_from_v6(addr6_ll), > + CONF_ADDR_OBSERVED | CONF_ADDR_LINKLOCAL, 0); > + > memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac)); > > return 0; > diff --git a/passt.h b/passt.h > index 5452225..db2f10d 100644 > --- a/passt.h > +++ b/passt.h > @@ -124,8 +124,6 @@ struct ip4_ctx { > > /** > * 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 to the > * host's [::1] > @@ -141,8 +139,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 b8d7cf4..fcb169a 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -370,6 +370,7 @@ static int pasta_conf_routes(struct ctx *c, sa_family_t af, int ifi, > void pasta_ns_conf(struct ctx *c) > { > unsigned int flags = IFF_UP; > + struct in6_addr addr_ll; > int rc; > > rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); > @@ -417,11 +418,13 @@ void pasta_ns_conf(struct ctx *c) > if (!c->ifi6) > goto done; > > - rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, > - &c->ip6.addr_ll_seen); > + rc = 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_LINKLOCAL | CONF_ADDR_HOST, 0); HOST is not correct, this is coming from the guest. Since we're peering into the guest to get this, I think it makes sense to tag it as OBSERVED. At some point we might need a distinction between addresses observed "on the wire" and addresses obtained by looking at the guest's netlink, but for now I think OBSERVED covers it. > rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); > if (rc < 0) > diff --git a/tap.c b/tap.c > index c75a4df..07f92bb 100644 > --- a/tap.c > +++ b/tap.c > @@ -173,19 +173,51 @@ 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); > } > > +/** > + * 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 *addr) > +{ > + uint8_t flags = CONF_ADDR_OBSERVED; > + > + if (IN6_IS_ADDR_UNSPECIFIED(addr)) > + return; > + > + if (IN6_IS_ADDR_LINKLOCAL(addr)) > + flags |= CONF_ADDR_LINKLOCAL; > + > + fwd_set_addr(c, &inany_from_v6(*addr), flags, 0); > +} > + > /** > * tap_ip6_daddr() - Normal IPv6 destination address for inbound packets > * @c: Execution context > * @src: Source address > * > - * Return: pointer to IPv6 address > + * Return: pointer to IPv6 address, NULL if no suitable address found > */ > 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 guest_addr *a; > + > + if (IN6_IS_ADDR_LINKLOCAL(src)) { > + /* Link-local: first LL address in array */ > + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_LINKLOCAL, 0); > + } else { > + /* Global: observed non-LL first, then any non-LL */ > + a = fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED, > + CONF_ADDR_USER | CONF_ADDR_HOST, > + CONF_ADDR_LINKLOCAL); > + } > + > + if (a) > + return &a->addr.a6; > + > + debug("No suitable IPv6 guest address found"); > + return NULL; > } > > /** > @@ -958,21 +990,7 @@ resume: > continue; > } > > - if (IN6_IS_ADDR_LINKLOCAL(saddr)) { > - c->ip6.addr_ll_seen = *saddr; > - > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen)) { > - c->ip6.addr_seen = *saddr; > - } > - > - if (!fwd_get_addr(c, AF_INET6, 0, 0)) { > - union inany_addr addr = { .a6 = *saddr }; > - > - fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64); > - } > - } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ > - c->ip6.addr_seen = *saddr; > - } > + tap_check_src_addr6(c, saddr); > > if (proto == IPPROTO_ICMPV6) { > struct iov_tail ndp_data; > @@ -1003,7 +1021,7 @@ resume: > if (proto == IPPROTO_UDP) { > struct iov_tail uh_data = data; > > - if (dhcpv6(c, &uh_data, saddr, daddr)) > + if (dhcpv6(c, &uh_data, daddr)) > continue; > } > > -- > 2.52.0 > -- 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