On Fri, Jan 30, 2026 at 04:44:46PM -0500, Jon Maloy wrote: > We remove the addr_seen and addr_ll_seen fields in struct ip6_ctx > and replace them by setting INANY_ADDR_OBSERVED and INANY_ADDR_LINKLOCAL > flags in the corresponding entry in the unified address array. > If the seen address is not present in the array we add it first. > > 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 A number of the comments on 9/11 will also apply here. > --- > v3: - Adapted to previous changes in this series > --- > conf.c | 5 ---- > dhcpv6.c | 6 ++--- > dhcpv6.h | 2 +- > fwd.c | 62 +++++++++++++++++++++++++++++++------------ > migrate.c | 64 +++++++++++++++++++++++++++++++++++++++------ > passt.h | 5 +--- > pasta.c | 15 ++++++++++- > tap.c | 78 +++++++++++++++++++++++++++++++++++++++++++++---------- > 8 files changed, 185 insertions(+), 52 deletions(-) > > diff --git a/conf.c b/conf.c > index c5818a8..6892cea 100644 > --- a/conf.c > +++ b/conf.c > @@ -764,7 +764,6 @@ static void conf_ip4_local(struct ctx *c) > */ > static unsigned int conf_ip6(unsigned int ifi, struct ctx *c) > { > - struct inany_addr_entry *e; > union inany_addr addr = {0,}; > int prefix_len = 0; > int rc; > @@ -800,10 +799,6 @@ static unsigned int conf_ip6(unsigned int ifi, struct ctx *c) > c->addr_count++; > } > > - e = first_v6(c); > - if (e) > - c->ip6.addr_seen = e->addr.a6; > - > if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.guest_gw)) > c->ip6.our_tap_ll = c->ip6.guest_gw; > > diff --git a/dhcpv6.c b/dhcpv6.c > index a1e5f15..67307d9 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -546,10 +546,11 @@ 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; > + Unrelated whitespace change. > /* The _storage variables can't be local to the blocks they're used in, > * because IOV_*_HEADER() may return pointers to them which are > * dereferenced afterwards. Since we don't have Rust-like lifetime > @@ -587,8 +588,6 @@ 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; > - It's stored differently, but we still want to update the observed ip6_ll address here. > src = &c->ip6.our_tap_ll; > > mh = IOV_REMOVE_HEADER(data, mh_storage); > @@ -679,7 +678,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > > tap_udp6_send(c, src, 547, tap_ip6_daddr(c, src), 546, > mh->xid, &resp, n); > - c->ip6.addr_seen = first_v6(c)->addr.a6; > > 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 > > 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); > > #endif /* DHCPV6_H */ > diff --git a/fwd.c b/fwd.c > index 5af482d..fc26eba 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -515,6 +515,36 @@ static const struct in_addr *fwd_guest_addr4(const struct ctx *c) > return NULL; > } > > +/** > + * fwd_guest_addr6() - Get first observed IPv6 guest address > + * @c: Execution context > + * @linklocal: If true, find link-local address; if false, find global > + * > + * Return: first observed IPv6 address, if any, > + * or first configured, if any, or NULL > + */ > +static const struct in6_addr *fwd_guest_addr6(const struct ctx *c, > + bool linklocal) > +{ > + const struct inany_addr_entry *e; > + > + /* Find first observed address of matching scope */ > + for_each_addr(c, e, AF_INET6) { > + if (!!(e->flags & INANY_ADDR_LINKLOCAL) != linklocal) > + continue; > + if (e->flags & INANY_ADDR_OBSERVED) > + return &e->addr.a6; > + } > + > + /* Fallback to first address of matching scope */ > + for_each_addr(c, e, AF_INET6) { > + if (!!(e->flags & INANY_ADDR_LINKLOCAL) == linklocal) > + return &e->addr.a6; > + } > + > + return NULL; > +} > + > /** > * fwd_guest_accessible4() - Is IPv4 address guest-accessible > * @c: Execution context > @@ -563,19 +593,11 @@ static bool fwd_guest_accessible6(const struct ctx *c, > if (IN6_IS_ADDR_LOOPBACK(addr)) > return false; > > - /* Check against all configured guest addresses */ > + /* Check against all guest addresses (configured, host, or observed) */ > for_each_addr(c, e, AF_INET6) > if (IN6_ARE_ADDR_EQUAL(addr, &e->addr.a6)) > 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) && > - IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr_seen)) > - return false; > - > return true; > } > > @@ -798,10 +820,16 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, > } > 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 in6_addr *guest_addr6; > + > + guest_addr6 = fwd_guest_addr6(c, false); > + if (!guest_addr6) > + return PIF_NONE; > + tgt->eaddr.a6 = *guest_addr6; > + } > tgt->oaddr = inany_any6; > } > > @@ -833,10 +861,12 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto, > return PIF_NONE; > tgt->eaddr = inany_from_v4(*guest_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); > + const struct in6_addr *guest_addr6 = fwd_guest_addr6(c, linklocal); > + > + if (!guest_addr6) > + return PIF_NONE; > + tgt->eaddr.a6 = *guest_addr6; > } > > return PIF_TAP; > diff --git a/migrate.c b/migrate.c > index a84af1c..20da1f7 100644 > --- a/migrate.c > +++ b/migrate.c > @@ -66,10 +66,7 @@ struct migrate_seen_addrs_v3 { > 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 inany_addr_entry *e; > > (void)stage; > @@ -82,6 +79,23 @@ static int seen_addrs_source_v2(struct ctx *c, > } > } > > + /* Find first observed IPv6 addresses (global and link-local) */ > + for_each_addr(c, e, AF_INET6) { > + struct in6_addr tmp; > + > + if (!(e->flags & INANY_ADDR_OBSERVED)) > + continue; > + if (e->flags & INANY_ADDR_LINKLOCAL) { > + tmp = addrs.addr6_ll; > + if (IN6_IS_ADDR_UNSPECIFIED(&tmp)) > + addrs.addr6_ll = e->addr.a6; > + } else { > + tmp = addrs.addr6; > + if (IN6_IS_ADDR_UNSPECIFIED(&tmp)) > + addrs.addr6 = e->addr.a6; > + } > + } You could reuse fwd_guest_addr6() here. > + > memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); > > if (write_all_buf(fd, &addrs, sizeof(addrs))) > @@ -117,6 +131,37 @@ static void migrate_observed_addr4(struct ctx *c, const struct in_addr *addr) > } > } > > +/** > + * migrate_observed_addr6() - Add observed IPv6 address to address array > + * @c: Execution context > + * @addr: IPv6 address to add > + */ > +static void migrate_observed_addr6(struct ctx *c, const struct in6_addr *addr) > +{ > + uint8_t flags = INANY_ADDR_OBSERVED; > + struct inany_addr_entry *e; > + > + if (IN6_IS_ADDR_UNSPECIFIED(addr)) > + return; > + > + if (IN6_IS_ADDR_LINKLOCAL(addr)) > + flags |= INANY_ADDR_LINKLOCAL; > + > + for_each_addr(c, e, AF_INET6) { > + if (IN6_ARE_ADDR_EQUAL(addr, &e->addr.a6)) { > + e->flags |= flags; > + return; > + } > + } > + > + if (c->addr_count < INANY_MAX_ADDRS) { > + c->addrs[c->addr_count].addr.a6 = *addr; > + c->addrs[c->addr_count].prefix_len = 0; > + c->addrs[c->addr_count].flags = flags; > + c->addr_count++; > + } Similar comments to v4: * Can use a unified function for migration and non-migration updates * Better to stick to 1 LL and 1 non-LL observed address for now, I think. > +} > + > /** > * seen_addrs_target_v2() - Receive observed addresses on target > * @c: Execution context > @@ -129,6 +174,7 @@ 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; > struct in_addr addr4; > > (void)stage; > @@ -136,12 +182,14 @@ static int seen_addrs_target_v2(struct ctx *c, > if (read_all_buf(fd, &addrs, sizeof(addrs))) > return errno; > > - c->ip6.addr_seen = addrs.addr6; > - c->ip6.addr_ll_seen = addrs.addr6_ll; > - > - /* Avoid alignment warning */ > + /* Copy from packed struct to avoid alignment issues */ > addr4 = addrs.addr4; > + addr6 = addrs.addr6; > + addr6_ll = addrs.addr6_ll; > + > migrate_observed_addr4(c, &addr4); > + migrate_observed_addr6(c, &addr6); > + migrate_observed_addr6(c, &addr6_ll); > > memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac)); > > diff --git a/passt.h b/passt.h > index 73084ae..7040781 100644 > --- a/passt.h > +++ b/passt.h > @@ -70,6 +70,7 @@ enum passt_modes { > #define INANY_ADDR_CONFIGURED BIT(0) /* User set via -a */ > #define INANY_ADDR_HOST BIT(1) /* From host interface */ > #define INANY_ADDR_OBSERVED BIT(2) /* Seen in guest traffic */ > +#define INANY_ADDR_LINKLOCAL BIT(3) /* Link-local address */ Isn't this redundant with the contents of the address itself? > /** > * for_each_addr() - Iterate over addresses in unified array > @@ -134,8 +135,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] > @@ -151,8 +150,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 8cb5873..7c2efd8 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -350,12 +350,19 @@ static void pasta_ns_conf_ip4(struct ctx *c) > */ > static void pasta_ns_conf_ip6(struct ctx *c) > { > + struct in6_addr addr_ll; > int rc = 0; > > - 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 if (c->addr_count < INANY_MAX_ADDRS) { > + c->addrs[c->addr_count].addr.a6 = addr_ll; > + c->addrs[c->addr_count].prefix_len = 64; > + c->addrs[c->addr_count].flags = > + INANY_ADDR_OBSERVED | INANY_ADDR_LINKLOCAL; > + c->addr_count++; > } > > rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); > @@ -373,6 +380,11 @@ static void pasta_ns_conf_ip6(struct ctx *c) > for_each_addr(c, e, AF_INET6) { > if (IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6)) > continue; > + > + /* Skip link-local - kernel auto-configures */ > + if (e->flags & INANY_ADDR_LINKLOCAL) > + continue; > + I'm not sure we need this. The kernel _will_ auto-configure, but if the user has specified a specific one, we should probably use it. Usually I wouldn't expect there to be an LL address in the array at this point > rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, > AF_INET6, &e->addr.a6, e->prefix_len); > if (rc < 0) > @@ -440,6 +452,7 @@ void pasta_ns_conf(struct ctx *c) > > if (c->ifi6) > pasta_ns_conf_ip6(c); > + Unrelated whitespace change. > } > > proto_update_l2_buf(c->guest_mac); > diff --git a/tap.c b/tap.c > index f2512f2..ba60f4b 100644 > --- a/tap.c > +++ b/tap.c > @@ -193,6 +193,41 @@ static void tap_check_src_addr4(struct ctx *c, const struct in_addr *addr) > debug("Added new observed IPv4 address"); > } > > +/** > + * tap_check_src_addr6() - Note an IPv6 address seen in guest traffic > + * @c: Execution context > + * @addr: IPv6 address seen as source from guest > + * > + * Add the address to addrs[] with OBSERVED flag if not already present. > + * Link-local addresses are also marked with LINKLOCAL flag. > + */ > +static void tap_check_src_addr6(struct ctx *c, const struct in6_addr *addr) > +{ > + uint8_t flags = INANY_ADDR_OBSERVED; > + struct inany_addr_entry *e; > + > + if (IN6_IS_ADDR_LINKLOCAL(addr)) > + flags |= INANY_ADDR_LINKLOCAL; > + > + /* Check if already in array */ > + for_each_addr(c, e, AF_INET6) { > + if (IN6_ARE_ADDR_EQUAL(addr, &e->addr.a6)) { > + e->flags |= flags; > + return; > + } > + } > + > + /* Add new entry if space available */ > + if (c->addr_count < INANY_MAX_ADDRS) { > + c->addrs[c->addr_count].addr.a6 = *addr; > + c->addrs[c->addr_count].prefix_len = 0; > + c->addrs[c->addr_count].flags = flags; > + c->addr_count++; > + } else { > + warn("Address table full, can't add IPv6 address"); > + } > +} > + > /** > * tap_ip6_daddr() - Normal IPv6 destination address for inbound packets > * @c: Execution context > @@ -203,9 +238,33 @@ 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; > + bool want_ll = IN6_IS_ADDR_LINKLOCAL(src); > + const struct inany_addr_entry *e; > + > + /* Find first observed address of matching scope */ > + for_each_addr(c, e, AF_INET6) { > + bool is_ll = !!(e->flags & INANY_ADDR_LINKLOCAL); > + > + if (is_ll != want_ll) > + continue; > + if (e->flags & INANY_ADDR_OBSERVED) > + return &e->addr.a6; > + } > + > + /* Fallback to first address of matching scope */ > + for_each_addr(c, e, AF_INET6) { > + bool is_ll = !!(e->flags & INANY_ADDR_LINKLOCAL); > + > + if (is_ll == want_ll) > + return &e->addr.a6; > + } > + > + /* Last resort: return first IPv6 address */ I think we have to match scope - an LL address is not meaningful to a peer who's not in scope. So I think all you can do is return NULL at this point. > + e = first_v6(c); > + if (e) > + return &e->addr.a6; > + > + return &in6addr_any; > } > > /** > @@ -976,15 +1035,8 @@ 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; > - } > - } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ > - c->ip6.addr_seen = *saddr; > - } > + if (!IN6_IS_ADDR_UNSPECIFIED(saddr)) > + tap_check_src_addr6(c, saddr); > > if (proto == IPPROTO_ICMPV6) { > struct iov_tail ndp_data; > @@ -1015,7 +1067,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