We replace the fwd_guest_accessible4() and fwd_guest_accessible6() functions with a unified fwd_guest_accessible() function that handles both address families. With the unified address array, we can check all configured addresses in a single pass using for_each_addr() with family filter AF_UNSPEC. Signed-off-by: Jon Maloy --- v6: -Some fixes based on feedback from David Gibson v7: -Added curly brackets to for_each_addr() in fwd_guest_accessible(), as suggested by Stefano Brivio. --- fwd.c | 69 +++++++++++++---------------------------------------------- 1 file changed, 15 insertions(+), 54 deletions(-) diff --git a/fwd.c b/fwd.c index 14ce0a7..e676c18 100644 --- a/fwd.c +++ b/fwd.c @@ -988,19 +988,19 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) } /** - * fwd_guest_accessible4() - Is IPv4 address guest-accessible + * fwd_guest_accessible() - Is address guest-accessible * @c: Execution context - * @addr: Host visible IPv4 address + * @addr: Host visible address (IPv4 or IPv6) * * Return: true if @addr on the host is accessible to the guest without * translation, false otherwise */ -static bool fwd_guest_accessible4(const struct ctx *c, - const struct in_addr *addr) +static bool fwd_guest_accessible(const struct ctx *c, + const union inany_addr *addr) { const struct guest_addr *a; - if (IN4_IS_ADDR_LOOPBACK(addr)) + if (inany_is_loopback(addr)) return false; /* In socket interfaces 0.0.0.0 generally means "any" or unspecified, @@ -1008,38 +1008,18 @@ static bool fwd_guest_accessible4(const struct ctx *c, * that has a different meaning for host and guest, we can't let it * through untranslated. */ - if (IN4_IS_ADDR_UNSPECIFIED(addr)) + if (inany_is_unspecified(addr)) return false; - /* For IPv4, addr_seen is initialised to addr, so is always a valid - * address + /* Check against all configured guest addresses */ + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { + if (inany_equals(addr, &a->addr)) + return false; + } + /* Also check addr_seen: it tracks the address the guest is actually + * using, which may differ from configured addresses. */ - a = fwd_get_addr(c, AF_INET, 0, 0); - if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || - IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) - return false; - - return true; -} - -/** - * fwd_guest_accessible6() - Is IPv6 address guest-accessible - * @c: Execution context - * @addr: Host visible IPv6 address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible6(const struct ctx *c, - const struct in6_addr *addr) -{ - const struct guest_addr *a; - - if (IN6_IS_ADDR_LOOPBACK(addr)) - return false; - - a = fwd_get_addr(c, AF_INET6, 0, 0); - if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) + if (inany_equals4(addr, &c->ip4.addr_seen)) return false; /* For IPv6, addr_seen starts unspecified, because we don't know what LL @@ -1047,31 +1027,12 @@ static bool fwd_guest_accessible6(const struct ctx *c, * 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)) + inany_equals6(addr, &c->ip6.addr_seen)) return false; return true; } -/** - * fwd_guest_accessible() - Is IPv[46] address guest-accessible - * @c: Execution context - * @addr: Host visible IPv[46] address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible(const struct ctx *c, - const union inany_addr *addr) -{ - const struct in_addr *a4 = inany_v4(addr); - - if (a4) - return fwd_guest_accessible4(c, a4); - - return fwd_guest_accessible6(c, &addr->a6); -} - /** * nat_outbound() - Apply address translation for outbound (TAP to HOST) * @c: Execution context -- 2.52.0

Add nl_addr_get_all() to read all addresses from the template interface into c->addrs[] array, rather than just selecting the "best" one. This allows multi-address configurations where the template interface has multiple IPv4 or IPv6 addresses assigned to it, all of which will now be copied to the guest namespace when using --config-net. For IPv6, the function also captures the link-local address into c->ip6.our_tap_ll as a side effect. Update conf_ip4() and conf_ip6() to use nl_addr_get_all() when no user-specified addresses are present. Signed-off-by: Jon Maloy Reviewed-by: David Gibson --- v7: -Excluded all link local addresses from host to be added to array -Minor comment change suggested by Stefano --- conf.c | 51 ++++++++++++++++++++++++++------------------------- netlink.c | 53 +++++++++++++++++++++++++++++------------------------ netlink.h | 3 +-- 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/conf.c b/conf.c index 8b4a7a0..924ade2 100644 --- a/conf.c +++ b/conf.c @@ -736,6 +736,7 @@ static int conf_ip4_prefix(const char *arg) static unsigned int conf_ip4(struct ctx *c, unsigned int ifi) { struct ip4_ctx *ip4 = &c->ip4; + const struct guest_addr *a; if (!ifi) ifi = nl_get_ext_if(nl_sock, AF_INET); @@ -755,24 +756,24 @@ static unsigned int conf_ip4(struct ctx *c, unsigned int ifi) } } - if (!fwd_get_addr(c, AF_INET, 0, 0)) { - struct in_addr addr; - int prefix_len; - int rc = nl_addr_get(nl_sock, ifi, AF_INET, - &addr, &prefix_len, NULL); + a = fwd_get_addr(c, AF_INET, CONF_ADDR_USER, 0); + if (!a) { + int rc = nl_addr_get_all(c, nl_sock, ifi, AF_INET); + if (rc < 0) { - debug("Couldn't discover IPv4 address: %s", + debug("Couldn't discover IPv4 addresses: %s", strerror_(-rc)); return 0; } - if (IN4_IS_ADDR_UNSPECIFIED(&addr)) + if (!rc || !fwd_get_addr(c, AF_INET, 0, 0)) return 0; - fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, - prefix_len); - ip4->addr_seen = addr; + a = fwd_get_addr(c, AF_INET, CONF_ADDR_HOST, 0); } + if (a) + ip4->addr_seen = *inany_v4(&a->addr); + ip4->our_tap_addr = ip4->guest_gw; return ifi; @@ -805,8 +806,6 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) { struct ip6_ctx *ip6 = &c->ip6; const struct guest_addr *a; - union inany_addr addr; - int prefix_len = 0; int rc; if (!ifi) @@ -826,24 +825,26 @@ static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) } } - rc = 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; - } - - a = fwd_get_addr(c, AF_INET6, 0, 0); + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_USER, 0); if (!a) { - if (IN6_IS_ADDR_UNSPECIFIED(&addr)) + rc = nl_addr_get_all(c, nl_sock, ifi, AF_INET6); + if (rc < 0) { + debug("Couldn't discover IPv6 addresses: %s", + strerror_(-rc)); return 0; - - fwd_set_addr(c, &addr, CONF_ADDR_HOST, prefix_len); - ip6->addr_seen = addr.a6; + } + a = fwd_get_addr(c, AF_INET6, CONF_ADDR_HOST, 0); } else { - ip6->addr_seen = a->addr.a6; + rc = nl_addr_get_ll(nl_sock, ifi, &ip6->our_tap_ll); + if (rc < 0) { + debug("Couldn't get link-local address: %s", + strerror_(-rc)); + } } + 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/netlink.c b/netlink.c index e07b47f..3f5a812 100644 --- a/netlink.c +++ b/netlink.c @@ -747,20 +747,20 @@ int nl_addr_set_ll_nodad(int s, unsigned int ifi) } /** - * nl_addr_get() - Get most specific global address, given interface and family + * nl_addr_get_all() - Get all addresses for a given interface into ctx + * @c: Execution context * @s: Netlink socket - * @ifi: Interface index in outer network namespace - * @af: Address family - * @addr: Global address to fill - * @prefix_len: Mask or prefix length, to fill - * @addr_l: Link-scoped address to fill (for IPv6) + * @ifi: Interface index + * @af: Address family (AF_INET or AF_INET6) * - * Return: 0 on success, negative error code on failure + * Populates c->addrs[] with all non-deprecated addresses from the interface. + * For IPv6, also captures link-local address in c->ip6.our_tap_ll. + * Skips IPv6 link-local addresses for the main array. + * + * Return: number of addresses added, or negative error code on failure */ -int nl_addr_get(int s, unsigned int ifi, sa_family_t af, - void *addr, int *prefix_len, void *addr_l) +int nl_addr_get_all(struct ctx *c, int s, unsigned int ifi, sa_family_t af) { - uint8_t prefix_max = 0, prefix_max_ll = 0; struct req_t { struct nlmsghdr nlh; struct ifaddrmsg ifa; @@ -771,6 +771,7 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af, struct nlmsghdr *nh; char buf[NLBUFSIZ]; ssize_t status; + int count = 0; uint32_t seq; seq = nl_send(s, &req, RTM_GETADDR, NLM_F_DUMP, sizeof(req)); @@ -784,27 +785,31 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af, for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { - if ((af == AF_INET && rta->rta_type != IFA_LOCAL) || - (af == AF_INET6 && rta->rta_type != IFA_ADDRESS)) - continue; + union inany_addr addr; - if (ifa->ifa_prefixlen > prefix_max && addr && - (af == AF_INET || ifa->ifa_scope < RT_SCOPE_LINK)) { - memcpy(addr, RTA_DATA(rta), RTA_PAYLOAD(rta)); + if (af == AF_INET && rta->rta_type != IFA_LOCAL) + continue; + if (af == AF_INET6 && rta->rta_type != IFA_ADDRESS) + continue; - prefix_max = *prefix_len = ifa->ifa_prefixlen; + /* Skip link-local addresses (kernel auto-configures) */ + if (ifa->ifa_scope == RT_SCOPE_LINK) { + if (af == AF_INET) + continue; + memcpy(&c->ip6.our_tap_ll, RTA_DATA(rta), + sizeof(c->ip6.our_tap_ll)); + continue; } - if (addr_l && - af == AF_INET6 && ifa->ifa_scope == RT_SCOPE_LINK && - ifa->ifa_prefixlen > prefix_max_ll) { - memcpy(addr_l, RTA_DATA(rta), RTA_PAYLOAD(rta)); + inany_from_af(&addr, af, RTA_DATA(rta)); + fwd_set_addr(c, &addr, CONF_ADDR_HOST, + ifa->ifa_prefixlen); - prefix_max_ll = ifa->ifa_prefixlen; - } + count++; } } - return status; + + return status < 0 ? status : count; } /** diff --git a/netlink.h b/netlink.h index b22f485..3af6d58 100644 --- a/netlink.h +++ b/netlink.h @@ -19,8 +19,7 @@ int nl_route_get_def(int s, unsigned int ifi, sa_family_t af, void *gw); int nl_route_set_def(int s, unsigned int ifi, sa_family_t af, const void *gw); int nl_route_dup(int s_src, unsigned int ifi_src, int s_dst, unsigned int ifi_dst, sa_family_t af); -int nl_addr_get(int s, unsigned int ifi, sa_family_t af, - void *addr, int *prefix_len, void *addr_l); +int nl_addr_get_all(struct ctx *c, int s, unsigned int ifi, sa_family_t af); bool nl_neigh_mac_get(int s, const union inany_addr *addr, int ifi, unsigned char *mac); int nl_addr_set(int s, unsigned int ifi, sa_family_t af, -- 2.52.0

We remove the addr_seen field in struct ip4_ctx and replace it by setting a new CONF_ADDR_OBSERVED flag in the corresponding entry in the unified address array. The observed IPv4 address is always added at or moved to position 0, increasing chances for a fast lookup. Signed-off-by: Jon Maloy --- v4: - Removed migration protocol update, to be added in later commit - Allow only one OBSERVED address at a time - Some other changes based on feedback from David G v5: - Allowing multiple observed IPv4 addresses v6: - Refactored fwd_set_addr(), notably: o Limited number of allowed observed addresses to four per protocol o I kept the memmove() calls, since I find no more elegant way to do this. Performance cost should be minimal, since these parts of the code will execute only very exceptionally. Note that removing the 'oldest' entry implicitly means removing the least used one, since the latter will migrate to the highest position after a few iterations of remove/add. o Also kept the prefix_len update. Not sure about this, but I cannot see how the current approach can cause any harm. - Other changes suggested by David G, notably reversing some residues after an accidental merge/re-split with the next commit. v7: - Changed fwd_set_addr() to only accept keeping one observed-only address per protocol, as suggested by David. - Eliminated redundant tap_check_src_addr4() call level. - I keep fwd_select_addr() for the same pragmatic reason it was introduced: to avoid ugly, deeply indented code that tends to wrap across several lines. --- conf.c | 6 --- fwd.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++------- fwd.h | 4 ++ migrate.c | 17 +++++++- passt.h | 6 +-- tap.c | 8 +++- 6 files changed, 136 insertions(+), 29 deletions(-) diff --git a/conf.c b/conf.c index 924ade2..f503d0f 100644 --- a/conf.c +++ b/conf.c @@ -767,13 +767,8 @@ 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); } - if (a) - ip4->addr_seen = *inany_v4(&a->addr); - ip4->our_tap_addr = ip4->guest_gw; return ifi; @@ -787,7 +782,6 @@ static void conf_ip4_local(struct ctx *c) { struct ip4_ctx *ip4 = &c->ip4; - ip4->addr_seen = IP4_LL_GUEST_ADDR; ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW; ip4->no_copy_addrs = ip4->no_copy_routes = true; fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), diff --git a/fwd.c b/fwd.c index d3f576a..8c7bf91 100644 --- a/fwd.c +++ b/fwd.c @@ -28,6 +28,7 @@ #include "inany.h" #include "fwd.h" #include "passt.h" +#include "conf.h" #include "lineread.h" #include "flow_table.h" #include "netlink.h" @@ -260,21 +261,68 @@ 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) { - struct guest_addr *a; + struct guest_addr *a, *arr = &c->addrs[0], *rm = NULL; + int count = c->addr_count; + int af_cnt = 0; - for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) { - goto found; + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { + if (!inany_equals(&a->addr, addr)) + continue; + + /* Adjust and update prefix_len if provided and applicable */ + if (prefix_len && !(a->flags & CONF_ADDR_USER)) + a->prefix_len = inany_prefix_len(addr, prefix_len); + + /* Nothing more to change */ + if ((a->flags & flags) == flags) + return; + + a->flags |= flags; + if (!(flags & CONF_ADDR_OBSERVED)) + return; + + /* Observed address moves to position 0: remove, re-add later */ + prefix_len = a->prefix_len; + memmove(a, a + 1, (&arr[count] - (a + 1)) * sizeof(*a)); + c->addr_count = --count; + break; } - if (c->addr_count >= MAX_GUEST_ADDRS) + if (count >= MAX_GUEST_ADDRS) { + debug("Address table full, can't add address"); return; + } - a = &c->addrs[c->addr_count++]; - -found: + /* Add to head or tail, depending on flag */ + if (flags & CONF_ADDR_OBSERVED) { + a = &arr[0]; + memmove(&arr[1], a, count * sizeof(*a)); + } else { + a = &arr[count]; + } + c->addr_count = ++count; a->addr = *addr; a->prefix_len = inany_prefix_len6(addr, prefix_len); a->flags = flags; + + if (!(flags & CONF_ADDR_OBSERVED)) + return; + + /* Remove excess observed-only address if more than one */ + for (int i = count - 1; i >= 0; i--) { + a = &arr[i]; + if (inany_af(&a->addr) != inany_af(addr)) + continue; + if (a->flags != CONF_ADDR_OBSERVED) + continue; + if (!rm) + rm = a; + af_cnt++; + } + if (af_cnt > 1) { + memmove(rm, rm + 1, (&arr[count] - (rm + 1)) * sizeof(*rm)); + c->addr_count--; + } } /** @@ -985,6 +1033,38 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) ((ini->oport == 53) || (ini->oport == 853)); } +/** + * fwd_select_addr() - Select address with priority-based search + * @c: Execution context + * @af: Address family (AF_INET or AF_INET6) + * @primary: Primary flags to match (or 0 to skip) + * @secondary: Secondary flags to match (or 0 to skip) + * @skip: Flags to exclude from search + * + * Search for address entries in priority order. + * + * Return: pointer to selected address entry, or NULL if none found + */ +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, + int primary, int secondary, int skip) +{ + const struct guest_addr *a; + + if (primary) { + a = fwd_get_addr(c, af, primary, skip); + if (a) + return a; + } + + if (secondary) { + a = fwd_get_addr(c, af, secondary, skip); + if (a) + return a; + } + + return NULL; +} + /** * fwd_guest_accessible() - Is address guest-accessible * @c: Execution context @@ -1014,11 +1094,6 @@ static bool fwd_guest_accessible(const struct ctx *c, if (inany_equals(addr, &a->addr)) return false; } - /* Also check addr_seen: it tracks the address the guest is actually - * using, which may differ from configured addresses. - */ - if (inany_equals4(addr, &c->ip4.addr_seen)) - 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 @@ -1214,10 +1289,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, * match. */ if (inany_v4(&ini->eaddr)) { - if (c->host_lo_to_ns_lo) + if (c->host_lo_to_ns_lo) { tgt->eaddr = inany_loopback4; - else - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); + } else { + const struct guest_addr *a; + + a = fwd_select_addr(c, AF_INET, + CONF_ADDR_OBSERVED, + CONF_ADDR_USER | + CONF_ADDR_HOST, 0); + if (!a) + return PIF_NONE; + + tgt->eaddr = a->addr; + } tgt->oaddr = inany_any4; } else { if (c->host_lo_to_ns_lo) @@ -1252,7 +1337,14 @@ uint8_t fwd_nat_from_host(const struct ctx *c, tgt->oport = ini->eport; if (inany_v4(&tgt->oaddr)) { - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); + const struct guest_addr *a; + + a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, + CONF_ADDR_USER | CONF_ADDR_HOST, 0); + if (!a) + return PIF_NONE; + + tgt->eaddr = a->addr; } else { if (inany_is_linklocal6(&tgt->oaddr)) tgt->eaddr.a6 = c->ip6.addr_ll_seen; diff --git a/fwd.h b/fwd.h index c5a1068..9893856 100644 --- a/fwd.h +++ b/fwd.h @@ -25,6 +25,10 @@ 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 af, uint8_t incl, uint8_t excl); +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, + int primary, int secondary, int skip); +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, + uint8_t flags, int prefix_len); /** * struct fwd_rule - Forwarding rule governing a range of ports diff --git a/migrate.c b/migrate.c index 1e8858a..1e02720 100644 --- a/migrate.c +++ b/migrate.c @@ -18,6 +18,8 @@ #include "util.h" #include "ip.h" #include "passt.h" +#include "conf.h" +#include "fwd.h" #include "inany.h" #include "flow.h" #include "flow_table.h" @@ -57,11 +59,18 @@ static int seen_addrs_source_v2(struct ctx *c, struct migrate_seen_addrs_v2 addrs = { .addr6 = c->ip6.addr_seen, .addr6_ll = c->ip6.addr_ll_seen, - .addr4 = c->ip4.addr_seen, }; + const struct guest_addr *a; (void)stage; + /* IPv4 observed address, with fallback to configured 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); + memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac)); if (write_all_buf(fd, &addrs, sizeof(addrs))) @@ -90,7 +99,11 @@ static int seen_addrs_target_v2(struct ctx *c, c->ip6.addr_seen = addrs.addr6; c->ip6.addr_ll_seen = addrs.addr6_ll; - c->ip4.addr_seen = addrs.addr4; + + if (addrs.addr4.s_addr) + fwd_set_addr(c, &inany_from_v4(addrs.addr4), + CONF_ADDR_OBSERVED, 0); + memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac)); return 0; diff --git a/passt.h b/passt.h index f75656d..5da1d55 100644 --- a/passt.h +++ b/passt.h @@ -64,8 +64,9 @@ enum passt_modes { MODE_VU, }; -/* Maximum number of addresses in context address array */ +/* Limits on number of addresses in context address array */ #define MAX_GUEST_ADDRS 32 +#define MAX_OBSERVED_ADDRS 4 /** * struct guest_addr - Unified IPv4/IPv6 address entry @@ -81,11 +82,11 @@ struct guest_addr { #define CONF_ADDR_HOST BIT(1) /* From host interface */ #define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ +#define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ }; /** * struct ip4_ctx - IPv4 execution context - * @addr_seen: Latest IPv4 address seen as source from tap * @guest_gw: IPv4 gateway as seen by the guest * @map_host_loopback: Outbound connections to this address are NATted to the * host's 127.0.0.1 @@ -101,7 +102,6 @@ struct guest_addr { * @no_copy_addrs: Don't copy all addresses when configuring namespace */ struct ip4_ctx { - struct in_addr addr_seen; struct in_addr guest_gw; struct in_addr map_host_loopback; struct in_addr map_guest_addr; diff --git a/tap.c b/tap.c index eb93f74..7f04e12 100644 --- a/tap.c +++ b/tap.c @@ -47,6 +47,7 @@ #include "ip.h" #include "iov.h" #include "passt.h" +#include "fwd.h" #include "arp.h" #include "dhcp.h" #include "ndp.h" @@ -756,9 +757,12 @@ resume: continue; } - if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) - c->ip4.addr_seen.s_addr = iph->saddr; + if (iph->saddr) { + const union inany_addr *addr; + addr = &inany_from_v4(*(struct in_addr *) &iph->saddr); + fwd_set_addr(c, addr, CONF_ADDR_OBSERVED, 0); + } if (!iov_drop_header(&data, hlen)) continue; if (iov_tail_size(&data) != l4len) -- 2.52.0

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. The separate check against addr_seen in fwd_guest_accessible() can now be removed because the observed address is now in the unified array, and the existing for_each_addr() loop already checks against all addresses, including this one. 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. v7: - Added a commit at the beginning of the series addressing Stefano's concern about DHCPv6 reply addresses. - Some other updates based on feedback from David and Stefano. --- conf.c | 4 ---- dhcpv6.c | 4 +--- fwd.c | 38 +++++++++++++++++++++++--------------- inany.h | 3 +++ migrate.c | 37 +++++++++++++++++++++++++++---------- passt.h | 4 ---- pasta.c | 11 +++++++---- tap.c | 17 +++++------------ 8 files changed, 66 insertions(+), 52 deletions(-) diff --git a/conf.c b/conf.c index f503d0f..3cb3553 100644 --- a/conf.c +++ b/conf.c @@ -827,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) { @@ -836,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 0a064a9..447aaba 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -567,8 +567,8 @@ 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; + const struct guest_addr *a; struct msg_hdr mh_storage; const struct msg_hdr *mh; struct udphdr uh_storage; @@ -683,8 +683,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, resp.hdr.xid = mh->xid; tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); - if (a) - c->ip6.addr_seen = a->addr.a6; return 1; } diff --git a/fwd.c b/fwd.c index 8c7bf91..b177be9 100644 --- a/fwd.c +++ b/fwd.c @@ -1095,14 +1095,6 @@ static bool fwd_guest_accessible(const struct ctx *c, 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 ll = inany_is_linklocal6(&tgt->oaddr); + uint8_t excl = ll ? ~CONF_ADDR_LINKLOCAL : CONF_ADDR_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 0450c45..ddcf93d 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) }) + /** 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..2dc4dd9 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) + 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 5da1d55..3ef84eb 100644 --- a/passt.h +++ b/passt.h @@ -121,8 +121,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] @@ -138,8 +136,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 f949dc2..f7f1e07 100644 --- a/pasta.c +++ b/pasta.c @@ -366,6 +366,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); @@ -415,12 +416,14 @@ void pasta_ns_conf(struct ctx *c) if (!c->ifi6) return; - rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, - &c->ip6.addr_ll_seen); - if (rc < 0) + 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_OBSERVED, 0); + } rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); if (rc < 0) warn("Can't set nodad for LL in namespace: %s", diff --git a/tap.c b/tap.c index 7f04e12..7f7f0ce 100644 --- a/tap.c +++ b/tap.c @@ -933,20 +933,13 @@ resume: continue; } - if (IN6_IS_ADDR_LINKLOCAL(saddr)) { - c->ip6.addr_ll_seen = *saddr; + if (!IN6_IS_ADDR_UNSPECIFIED(saddr)) { + uint8_t flags = CONF_ADDR_OBSERVED; - 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 }; + if (IN6_IS_ADDR_LINKLOCAL(saddr)) + flags |= CONF_ADDR_LINKLOCAL; - fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64); - } - } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ - c->ip6.addr_seen = *saddr; + fwd_set_addr(c, &inany_from_v6(*saddr), flags, 0); } if (proto == IPPROTO_ICMPV6) { -- 2.52.0

We update the migration protocol to version 3 to support distributing multiple addresses from the unified address array. The new protocol migrates all address entries in the array, along with their prefix lengths and flags, and leaves it to the receiver to filter which ones he wants to apply. Signed-off-by: Jon Maloy --- v4: - Broke out as separate commit - Made number of transferable addresses variable v6: - Separated internal and wire transfer format v7: - Using uint32_t instead of uint8_t for fields in migration format - Replaced term "wire format" with "migration format" - Some other minor changes after feedback from Stefano. --- migrate.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/migrate.c b/migrate.c index 2dc4dd9..93f67ae 100644 --- a/migrate.c +++ b/migrate.c @@ -44,6 +44,71 @@ struct migrate_seen_addrs_v2 { unsigned char mac[ETH_ALEN]; } __attribute__((packed)); +/** + * Migration format flags for address migration (v3) + * These are stable values - do not change existing assignments + */ +#define MIGRATE_ADDR_USER BIT(0) +#define MIGRATE_ADDR_HOST BIT(1) +#define MIGRATE_ADDR_LINKLOCAL BIT(2) +#define MIGRATE_ADDR_OBSERVED BIT(3) + +/** + * struct migrate_addr_v3 - Migration format for a single address entry + * @addr: IPv6 or IPv4-mapped address (16 bytes) + * @prefix_len: Prefix length + * @flags: MIGRATE_ADDR_* flags (migration format) + */ +struct migrate_addr_v3 { + struct in6_addr addr; + uint32_t prefix_len; + uint32_t flags; +} __attribute__((__packed__)); + +/** + * flags_to_migration() - Convert internal flags to stable migration format + * @flags: Internal CONF_ADDR_* flags + * + * Return: Migration format MIGRATE_ADDR_* flags + */ +static uint8_t flags_to_migration(uint8_t flags) +{ + uint8_t migration = 0; + + if (flags & CONF_ADDR_USER) + migration |= MIGRATE_ADDR_USER; + if (flags & CONF_ADDR_HOST) + migration |= MIGRATE_ADDR_HOST; + if (flags & CONF_ADDR_LINKLOCAL) + migration |= MIGRATE_ADDR_LINKLOCAL; + if (flags & CONF_ADDR_OBSERVED) + migration |= MIGRATE_ADDR_OBSERVED; + + return migration; +} + +/** + * flags_from_migration() - Convert migration format flags to internal format + * @migration: Migration format MIGRATE_ADDR_* flags + * + * Return: Internal CONF_ADDR_* flags + */ +static uint8_t flags_from_migration(uint8_t migration) +{ + uint8_t flags = 0; + + if (migration & MIGRATE_ADDR_USER) + flags |= CONF_ADDR_USER; + if (migration & MIGRATE_ADDR_HOST) + flags |= CONF_ADDR_HOST; + if (migration & MIGRATE_ADDR_LINKLOCAL) + flags |= CONF_ADDR_LINKLOCAL; + if (migration & MIGRATE_ADDR_OBSERVED) + flags |= CONF_ADDR_OBSERVED; + + return flags; +} + /** * seen_addrs_source_v2() - Copy and send guest observed addresses from source * @c: Execution context @@ -126,6 +191,99 @@ static int seen_addrs_target_v2(struct ctx *c, return 0; } +/** + * addrs_source_v3() - Send all addresses with flags from source + * @c: Execution context + * @stage: Migration stage, unused + * @fd: File descriptor for state transfer + * + * Send all address entries using a stable migration format. Each field is + * serialised explicitly to avoid coupling the migration format to internal + * structure layout or flag bit assignments. + * + * Return: 0 on success, positive error code on failure + */ +/* cppcheck-suppress [constParameterCallback, unmatchedSuppression] */ +static int addrs_source_v3(struct ctx *c, + const struct migrate_stage *stage, int fd) +{ + uint8_t addr_count = c->addr_count; + const struct guest_addr *a; + + (void)stage; + + /* Send count first */ + if (write_all_buf(fd, &addr_count, sizeof(addr_count))) + return errno; + + /* Send each address in stable migration format */ + for_each_addr(a, c->addrs, c->addr_count, 0) { + struct migrate_addr_v3 migration = { + .addr = a->addr.a6, + .prefix_len = htonl(a->prefix_len), + .flags = htonl(flags_to_migration(a->flags)), + }; + + if (write_all_buf(fd, &migration, sizeof(migration))) + return errno; + } + + /* Send MAC address */ + if (write_all_buf(fd, c->guest_mac, ETH_ALEN)) + return errno; + + return 0; +} + +/** + * addrs_target_v3() - Receive addresses on target + * @c: Execution context + * @stage: Migration stage, unused + * @fd: File descriptor for state transfer + * + * Receive address entries from the stable migration format and merge only + * observed addresses into local array. Source sends all addresses for + * forward compatibility, but target only applies those marked as observed. + * + * Return: 0 on success, positive error code on failure + */ +static int addrs_target_v3(struct ctx *c, + const struct migrate_stage *stage, int fd) +{ + uint8_t addr_count, i; + + (void)stage; + + if (read_all_buf(fd, &addr_count, sizeof(addr_count))) + return errno; + + if (addr_count > MAX_GUEST_ADDRS) + addr_count = MAX_GUEST_ADDRS; + + /* Read each address from stable migration format */ + for (i = 0; i < addr_count; i++) { + struct migrate_addr_v3 migration; + struct guest_addr addr; + + if (read_all_buf(fd, &migration, sizeof(migration))) + return errno; + + addr.addr.a6 = migration.addr; + addr.prefix_len = ntohl(migration.prefix_len); + addr.flags = flags_from_migration(ntohl(migration.flags)); + + if (addr.flags & CONF_ADDR_OBSERVED) { + fwd_set_addr(c, &addr.addr, addr.flags, + addr.prefix_len); + } + } + + if (read_all_buf(fd, c->guest_mac, ETH_ALEN)) + return errno; + + return 0; +} + /* Stages for version 2 */ static const struct migrate_stage stages_v2[] = { { @@ -146,8 +304,29 @@ static const struct migrate_stage stages_v2[] = { { 0 }, }; +/* Stages for version 3 (all addresses, with flags) */ +static const struct migrate_stage stages_v3[] = { + { + .name = "addresses", + .source = addrs_source_v3, + .target = addrs_target_v3, + }, + { + .name = "prepare flows", + .source = flow_migrate_source_pre, + .target = NULL, + }, + { + .name = "transfer flows", + .source = flow_migrate_source, + .target = flow_migrate_target, + }, + { 0 }, +}; + /* Supported encoding versions, from latest (most preferred) to oldest */ static const struct migrate_version versions[] = { + { 3, stages_v3, }, { 2, stages_v2, }, /* v1 was released, but not widely used. It had bad endianness for the * MSS and omitted timestamps, which meant it usually wouldn't work. -- 2.52.0

We extend NDP to advertise all suitable IPv6 prefixes in Router Advertisements, per RFC 4861. Observed and link-local addresses, plus addresses with a prefix length != 64, are excluded. Signed-off-by: Jon Maloy --- v6: -Adapted to previous changes in series v7: -Adapted to previous changes in series -Use struct initializer for source link-layer address option -Other minor fixes based on feedback from Stefano --- conf.c | 19 ++++++--- fwd.c | 4 ++ migrate.c | 5 +++ ndp.c | 123 +++++++++++++++++++++++++++++++++++++----------------- passt.h | 3 +- 5 files changed, 108 insertions(+), 46 deletions(-) diff --git a/conf.c b/conf.c index 7c705de..97c66c9 100644 --- a/conf.c +++ b/conf.c @@ -1216,7 +1216,7 @@ static void conf_print(const struct ctx *c) } if (c->ifi6) { - bool has_dhcpv6 = false; + bool has_ndp = false, has_dhcpv6 = false; const char *head; if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback)) @@ -1225,18 +1225,25 @@ static void conf_print(const struct ctx *c) buf, sizeof(buf))); for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { + if (a->flags & CONF_ADDR_SLAAC) + has_ndp = true; if (a->flags & CONF_ADDR_DHCPV6) has_dhcpv6 = true; } - if (c->no_ndp && !has_dhcpv6) + if (!has_ndp && !has_dhcpv6) goto dns6; - a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); - if (!c->no_ndp && a) { + if (has_ndp) { info("NDP:"); - info(" assign: %s", - inany_ntop(&a->addr, buf, sizeof(buf))); + head = "assign: "; + for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { + if (!(a->flags & CONF_ADDR_SLAAC)) + continue; + inany_ntop(&a->addr, buf, sizeof(buf)); + info(" %s: %s/%d", head, buf, a->prefix_len); + head = " "; + } } if (has_dhcpv6) { diff --git a/fwd.c b/fwd.c index 2b444fb..2bc8e33 100644 --- a/fwd.c +++ b/fwd.c @@ -302,6 +302,10 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr, } else if (!(flags & CONF_ADDR_LINKLOCAL)) { if (!c->no_dhcpv6) flags |= CONF_ADDR_DHCPV6; + + /* NDP/RA only if prefix is /64 */ + if (!c->no_ndp && prefix_len == 64) + flags |= CONF_ADDR_SLAAC; } /* Add to head or tail, depending on flag */ diff --git a/migrate.c b/migrate.c index adcbc63..f019924 100644 --- a/migrate.c +++ b/migrate.c @@ -54,6 +54,7 @@ struct migrate_seen_addrs_v2 { #define MIGRATE_ADDR_OBSERVED BIT(3) #define MIGRATE_ADDR_DHCP BIT(4) #define MIGRATE_ADDR_DHCPV6 BIT(5) +#define MIGRATE_ADDR_SLAAC BIT(6) /** * struct migrate_addr_v3 - Migration format for a single address entry @@ -89,6 +90,8 @@ static uint8_t flags_to_migration(uint8_t flags) migration |= MIGRATE_ADDR_DHCP; if (flags & CONF_ADDR_DHCPV6) migration |= MIGRATE_ADDR_DHCPV6; + if (flags & CONF_ADDR_SLAAC) + migration |= MIGRATE_ADDR_SLAAC; return migration; } @@ -115,6 +118,8 @@ static uint8_t flags_from_migration(uint8_t migration) flags |= CONF_ADDR_DHCP; if (migration & MIGRATE_ADDR_DHCPV6) flags |= CONF_ADDR_DHCPV6; + if (migration & MIGRATE_ADDR_SLAAC) + flags |= CONF_ADDR_SLAAC; return flags; } diff --git a/ndp.c b/ndp.c index 3750fc5..bb8374a 100644 --- a/ndp.c +++ b/ndp.c @@ -32,6 +32,8 @@ #include "passt.h" #include "tap.h" #include "log.h" +#include "fwd.h" +#include "conf.h" #define RT_LIFETIME 65535 @@ -99,6 +101,16 @@ struct opt_prefix_info { uint32_t reserved; } __attribute__((packed)); +/** + * struct ndp_prefix - Prefix Information option with prefix + * @info: Prefix Information option header + * @prefix: IPv6 prefix + */ +struct ndp_prefix { + struct opt_prefix_info info; + struct in6_addr prefix; +} __attribute__((__packed__)); + /** * struct opt_mtu - Maximum transmission unit (MTU) option * @header: Option header @@ -140,27 +152,23 @@ struct opt_dnssl { } __attribute__((packed)); /** - * struct ndp_ra - NDP Router Advertisement (RA) message + * struct ndp_ra_hdr - NDP Router Advertisement fixed header * @ih: ICMPv6 header * @reachable: Reachability time, after confirmation (ms) * @retrans: Time between retransmitted NS messages (ms) - * @prefix_info: Prefix Information option - * @prefix: IPv6 prefix - * @mtu: MTU option - * @source_ll: Target link-layer address - * @var: Variable fields */ -struct ndp_ra { +struct ndp_ra_hdr { struct icmp6hdr ih; uint32_t reachable; uint32_t retrans; - struct opt_prefix_info prefix_info; - struct in6_addr prefix; - struct opt_l2_addr source_ll; +} __attribute__((__packed__)); - unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + - sizeof(struct opt_dnssl)]; -} __attribute__((packed, aligned(__alignof__(struct in6_addr)))); +/* Maximum RA message size: hdr + prefixes + source_ll + mtu + rdnss + dnssl */ +#define NDP_RA_MAX_SIZE (sizeof(struct ndp_ra_hdr) + \ + MAX_GUEST_ADDRS * sizeof(struct ndp_prefix) + \ + sizeof(struct opt_l2_addr) + \ + sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + \ + sizeof(struct opt_dnssl)) /** * struct ndp_ns - NDP Neighbor Solicitation (NS) message @@ -231,6 +239,42 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) ndp_na(c, &in6addr_ll_all_nodes, addr); } +/** + * ndp_prefix_fill() - Fill prefix options for all suitable addresses + * @c: Execution context + * @buf: Buffer to write prefix options into + * + * Fills buffer with Prefix Information options for all non-linklocal, + * non-observed addresses with prefix_len == 64 + * + * Return: number of bytes written + */ +static size_t ndp_prefix_fill(const struct ctx *c, unsigned char *buf) +{ + const struct guest_addr *a; + struct ndp_prefix *p; + size_t offset = 0; + + for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { + if (!(a->flags & CONF_ADDR_SLAAC)) + continue; + + p = (struct ndp_prefix *)(buf + offset); + p->info.header.type = OPT_PREFIX_INFO; + p->info.header.len = 4; /* 4 * 8 = 32 bytes */ + p->info.prefix_len = 64; + p->info.prefix_flags = 0xc0; /* L, A flags */ + p->info.valid_lifetime = ~0U; + p->info.pref_lifetime = ~0U; + p->info.reserved = 0; + p->prefix = a->addr.a6; + + offset += sizeof(struct ndp_prefix); + } + + return offset; +} + /** * ndp_ra() - Send an NDP Router Advertisement (RA) message * @c: Execution context @@ -238,7 +282,15 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) */ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) { - struct ndp_ra ra = { + unsigned char buf[NDP_RA_MAX_SIZE] + __attribute__((__aligned__(__alignof__(struct in6_addr)))); + struct ndp_ra_hdr *hdr = (struct ndp_ra_hdr *)buf; + struct opt_l2_addr *source_ll; + unsigned char *ptr; + size_t prefix_len; + + /* Build RA header */ + *hdr = (struct ndp_ra_hdr){ .ih = { .icmp6_type = RA, .icmp6_code = 0, @@ -247,31 +299,26 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) .icmp6_rt_lifetime = htons_constant(RT_LIFETIME), .icmp6_addrconf_managed = 1, }, - .prefix_info = { - .header = { - .type = OPT_PREFIX_INFO, - .len = 4, - }, - .prefix_len = 64, - .prefix_flags = 0xc0, /* prefix flags: L, A */ - .valid_lifetime = ~0U, - .pref_lifetime = ~0U, - }, - .source_ll = { - .header = { - .type = OPT_SRC_L2_ADDR, - .len = 1, - }, - }, }; - const struct guest_addr *a = fwd_get_addr(c, AF_INET6, 0, 0); - unsigned char *ptr = NULL; - ASSERT(a); - - ra.prefix = a->addr.a6; + /* Fill prefix options */ + prefix_len = ndp_prefix_fill(c, (unsigned char *)(hdr + 1)); + if (prefix_len == 0) { + /* No suitable prefixes to advertise */ + return; + } - ptr = &ra.var[0]; + /* Add source link-layer address option */ + ptr = (unsigned char *)(hdr + 1) + prefix_len; + source_ll = (struct opt_l2_addr *)ptr; + *source_ll = (struct opt_l2_addr) { + .header = { + .type = OPT_SRC_L2_ADDR, + .len = 1, + }, + }; + memcpy(source_ll->mac, c->our_tap_mac, ETH_ALEN); + ptr += sizeof(struct opt_l2_addr); if (c->mtu) { struct opt_mtu *mtu = (struct opt_mtu *)ptr; @@ -345,10 +392,8 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) } } - memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); - /* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */ - ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra); + ndp_send(c, dst, buf, ptr - buf); } /** diff --git a/passt.h b/passt.h index 028eb7c..2c633e4 100644 --- a/passt.h +++ b/passt.h @@ -84,7 +84,8 @@ struct guest_addr { #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ #define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ #define CONF_ADDR_DHCP BIT(5) /* Advertise via DHCP (IPv4) */ -#define CONF_ADDR_DHCPV6 BIT(6) /* Advertise via DHCPv6 (IPv6) */ +#define CONF_ADDR_DHCPV6 BIT(6) /* Advertise via DHCPv6 */ +#define CONF_ADDR_SLAAC BIT(7) /* Advertise via NDP/RA (/64) */ }; /** -- 2.52.0

On Sun, Apr 12, 2026 at 08:53:09PM -0400, Jon Maloy wrote:

We replace the fwd_guest_accessible4() and fwd_guest_accessible6() functions with a unified fwd_guest_accessible() function that handles both address families. With the unified address array, we can check all configured addresses in a single pass using for_each_addr() with family filter AF_UNSPEC.

Signed-off-by: Jon Maloy

Reviewed-by: David Gibson Small pre-existing nit noted below, but it probably goes away later in the series anyway.

--- v6: -Some fixes based on feedback from David Gibson v7: -Added curly brackets to for_each_addr() in fwd_guest_accessible(), as suggested by Stefano Brivio. --- fwd.c | 69 +++++++++++++---------------------------------------------- 1 file changed, 15 insertions(+), 54 deletions(-)

diff --git a/fwd.c b/fwd.c index 14ce0a7..e676c18 100644 --- a/fwd.c +++ b/fwd.c @@ -988,19 +988,19 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) }

/** - * fwd_guest_accessible4() - Is IPv4 address guest-accessible + * fwd_guest_accessible() - Is address guest-accessible * @c: Execution context - * @addr: Host visible IPv4 address + * @addr: Host visible address (IPv4 or IPv6) * * Return: true if @addr on the host is accessible to the guest without * translation, false otherwise */ -static bool fwd_guest_accessible4(const struct ctx *c, - const struct in_addr *addr) +static bool fwd_guest_accessible(const struct ctx *c, + const union inany_addr *addr) { const struct guest_addr *a;

- if (IN4_IS_ADDR_LOOPBACK(addr)) + if (inany_is_loopback(addr)) return false;

/* In socket interfaces 0.0.0.0 generally means "any" or unspecified, @@ -1008,38 +1008,18 @@ static bool fwd_guest_accessible4(const struct ctx *c, * that has a different meaning for host and guest, we can't let it * through untranslated. */ - if (IN4_IS_ADDR_UNSPECIFIED(addr)) + if (inany_is_unspecified(addr)) return false;

- /* For IPv4, addr_seen is initialised to addr, so is always a valid - * address + /* Check against all configured guest addresses */ + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { + if (inany_equals(addr, &a->addr)) + return false; + } + /* Also check addr_seen: it tracks the address the guest is actually + * using, which may differ from configured addresses. */ - a = fwd_get_addr(c, AF_INET, 0, 0); - if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || - IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) - return false; - - return true; -} - -/** - * fwd_guest_accessible6() - Is IPv6 address guest-accessible - * @c: Execution context - * @addr: Host visible IPv6 address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible6(const struct ctx *c, - const struct in6_addr *addr) -{ - const struct guest_addr *a; - - if (IN6_IS_ADDR_LOOPBACK(addr)) - return false; - - a = fwd_get_addr(c, AF_INET6, 0, 0); - if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) + if (inany_equals4(addr, &c->ip4.addr_seen)) return false;

/* For IPv6, addr_seen starts unspecified, because we don't know what LL @@ -1047,31 +1027,12 @@ static bool fwd_guest_accessible6(const struct ctx *c, * if it has been set to a real address. */ if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen) &&

Pre-existing nit: this test is unnecessary, we already checked that this address is not unspecified, so if addr_seen *is* unspecified, they can't be equal.

- IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr_seen)) + inany_equals6(addr, &c->ip6.addr_seen)) return false;

return true; }

-/** - * fwd_guest_accessible() - Is IPv[46] address guest-accessible - * @c: Execution context - * @addr: Host visible IPv[46] address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible(const struct ctx *c, - const union inany_addr *addr) -{ - const struct in_addr *a4 = inany_v4(addr); - - if (a4) - return fwd_guest_accessible4(c, a4); - - return fwd_guest_accessible6(c, &addr->a6); -} - /** * nat_outbound() - Apply address translation for outbound (TAP to HOST) * @c: Execution context -- 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

On Sun, Apr 12, 2026 at 08:53:13PM -0400, Jon Maloy wrote:

After the previous changes in this series it becomes possible to simplify the pasta_ns_conf() function.

We extract address and route configuration into helper functions pasta_conf_addrs() and pasta_conf_routes(), reducing nesting and improving readability.

To allow pasta_conf_addrs() to handle both address families uniformly, we change nl_addr_set() to take a union inany_addr pointer instead of void pointer, moving the address family handling into the function itself.

We also fix a bug where the IPv6 code path incorrectly wrote to req.set.a4.rta_l.rta_type instead of req.set.a6.rta_l.rta_type.

Signed-off-by: Jon Maloy

Reviewed-by: David Gibson Very happy to see the confusing gotos gone. Couple of minor suggestions below, plus noted a pre-existing oddity that might be worth altering / commenting as a separate matter.

--- v7: -Removed redundant argument 'af' in nl_addr_set() -Removed redundant label and 'goto's in pasta_ns_conf() -Since I excluded addition of all LINKLOCAL addresses from host address array in a previous commit I can now omit this test in pasta_conf_addrs() as suggested by David. -Here is the example of agnostic usage of inany_prefix_len() I referred to in a previous commit. --- netlink.c | 17 ++-- netlink.h | 4 +- pasta.c | 232 ++++++++++++++++++++++++++---------------------------- 3 files changed, 123 insertions(+), 130 deletions(-)

diff --git a/netlink.c b/netlink.c index 3f5a812..3d25212 100644 --- a/netlink.c +++ b/netlink.c @@ -863,15 +863,15 @@ int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr) * nl_addr_set() - Set IP addresses for given interface and address family * @s: Netlink socket * @ifi: Interface index - * @af: Address family * @addr: Global address to set * @prefix_len: Mask or prefix length to set * * Return: 0 on success, negative error code on failure */ -int nl_addr_set(int s, unsigned int ifi, sa_family_t af, - const void *addr, int prefix_len) +int nl_addr_set(int s, unsigned int ifi, const union inany_addr *addr, + int prefix_len) { + sa_family_t af = inany_af(addr); struct req_t { struct nlmsghdr nlh; struct ifaddrmsg ifa; @@ -905,21 +905,22 @@ int nl_addr_set(int s, unsigned int ifi, sa_family_t af,

len = offsetof(struct req_t, set.a6) + sizeof(req.set.a6);

- memcpy(&req.set.a6.l, addr, sizeof(req.set.a6.l)); + memcpy(&req.set.a6.l, &addr->a6, sizeof(req.set.a6.l)); req.set.a6.rta_l.rta_len = rta_len; - req.set.a4.rta_l.rta_type = IFA_LOCAL; - memcpy(&req.set.a6.a, addr, sizeof(req.set.a6.a)); + req.set.a6.rta_l.rta_type = IFA_LOCAL; + memcpy(&req.set.a6.a, &addr->a6, sizeof(req.set.a6.a)); req.set.a6.rta_a.rta_len = rta_len; req.set.a6.rta_a.rta_type = IFA_ADDRESS; } else { + const struct in_addr *v4 = inany_v4(addr);

I suspect the static checkers won't be able to deduce that this cannot be NULL, because of the earlier inany_af() call - in a sense you're checking the address family twice. I think it would be more elegant (and avoid possible checker false positives) to have the if based on the return value from inany_v4(), then set req.ifa.ifa_family to constant values in each branch

size_t rta_len = RTA_LENGTH(sizeof(req.set.a4.l));

len = offsetof(struct req_t, set.a4) + sizeof(req.set.a4);

- memcpy(&req.set.a4.l, addr, sizeof(req.set.a4.l)); + memcpy(&req.set.a4.l, v4, sizeof(req.set.a4.l)); req.set.a4.rta_l.rta_len = rta_len; req.set.a4.rta_l.rta_type = IFA_LOCAL; - memcpy(&req.set.a4.a, addr, sizeof(req.set.a4.a)); + memcpy(&req.set.a4.a, v4, sizeof(req.set.a4.a)); req.set.a4.rta_a.rta_len = rta_len; req.set.a4.rta_a.rta_type = IFA_ADDRESS; } diff --git a/netlink.h b/netlink.h index 3af6d58..ec859b7 100644 --- a/netlink.h +++ b/netlink.h @@ -22,8 +22,8 @@ int nl_route_dup(int s_src, unsigned int ifi_src, int nl_addr_get_all(struct ctx *c, int s, unsigned int ifi, sa_family_t af); bool nl_neigh_mac_get(int s, const union inany_addr *addr, int ifi, unsigned char *mac); -int nl_addr_set(int s, unsigned int ifi, sa_family_t af, - const void *addr, int prefix_len); +int nl_addr_set(int s, unsigned int ifi, const union inany_addr *addr, + int prefix_len); int nl_addr_get_ll(int s, unsigned int ifi, struct in6_addr *addr); int nl_addr_set_ll_nodad(int s, unsigned int ifi); int nl_addr_dup(int s_src, unsigned int ifi_src, diff --git a/pasta.c b/pasta.c index b3936f5..f949dc2 100644 --- a/pasta.c +++ b/pasta.c @@ -46,6 +46,7 @@

#include "util.h" #include "passt.h" +#include "conf.h" #include "isolation.h" #include "netlink.h" #include "log.h" @@ -303,13 +304,69 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, die_perror("Failed to join network namespace"); }

+/** + * pasta_conf_addrs() - Configure addresses for one address family in namespace + * @c: Execution context + * @af: Address family (AF_INET or AF_INET6) + * @ifi: Host interface index for this address family + * @no_copy: If true, set addresses from c->addrs; if false, copy from host + * + * Return: 0 on success, negative error code on failure + */ +static int pasta_conf_addrs(struct ctx *c, sa_family_t af, + int ifi, bool no_copy) +{ + const struct guest_addr *a; + + if (!ifi) + return 0; + + if (!no_copy) + return nl_addr_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af); + + for_each_addr(a, c->addrs, c->addr_count, af) { + int rc; + + rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, &a->addr, + inany_prefix_len(&a->addr, a->prefix_len)); + if (rc < 0) + return rc; + } + return 0; +} + +/** + * pasta_conf_routes() - Configure routes for one address family in namespace + * @c: Execution context + * @af: Address family (AF_INET or AF_INET6) + * @ifi: Host interface index for this address family + * @no_copy: If true, set default route; if false, copy routes from host + * + * Return: 0 on success, negative error code on failure + */ +static int pasta_conf_routes(struct ctx *c, sa_family_t af, int ifi, + bool no_copy) +{ + const void *gw = (af == AF_INET) ? + (const void *)&c->ip4.guest_gw : (const void *)&c->ip6.guest_gw; + + if (!ifi) + return 0; + + if (no_copy)

It'd be nicer to have the !no_copy branch first for consistency with pasta_conf_addrs().

+ return nl_route_set_def(nl_sock_ns, c->pasta_ifi, af, gw); + + return nl_route_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af); +} + /** * pasta_ns_conf() - Set up loopback and tap interfaces in namespace as needed * @c: Execution context */ void pasta_ns_conf(struct ctx *c) { - int rc = 0; + unsigned int flags = IFF_UP; + int rc;

rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); if (rc < 0) @@ -328,127 +385,62 @@ void pasta_ns_conf(struct ctx *c) die("Couldn't set MAC address in namespace: %s", strerror_(-rc));

- if (c->pasta_conf_ns) { - unsigned int flags = IFF_UP; - const struct guest_addr *a; - int plen; - - if (c->mtu) - nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); - - if (c->ifi6) /* Avoid duplicate address detection on link up */ - flags |= IFF_NOARP; - - nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags); - - if (c->ifi4) { - if (c->ip4.no_copy_addrs) { - for_each_addr(a, c->addrs, c->addr_count, AF_INET) { - plen = inany_prefix_len(&a->addr, - a->prefix_len); - rc = nl_addr_set(nl_sock_ns, - c->pasta_ifi, AF_INET, - inany_v4(&a->addr), - plen); - if (rc < 0) - break; - } - } else { - rc = nl_addr_dup(nl_sock, c->ifi4, - nl_sock_ns, c->pasta_ifi, - AF_INET); - } - - if (c->ifi4 == -1 && rc == -ENOTSUP) { - warn("IPv4 not supported, disabling"); - c->ifi4 = 0; - goto ipv4_done; - } - - if (rc < 0) { - die("Couldn't set IPv4 address(es) in namespace: %s", - strerror_(-rc)); - } - - if (c->ip4.no_copy_routes) { - rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi, - AF_INET, - &c->ip4.guest_gw); - } else { - rc = nl_route_dup(nl_sock, c->ifi4, nl_sock_ns, - c->pasta_ifi, AF_INET); - } - - if (rc < 0) { - die("Couldn't set IPv4 route(s) in guest: %s", - strerror_(-rc)); - } - } -ipv4_done: - - if (c->ifi6) { - rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, - &c->ip6.addr_ll_seen); - if (rc < 0) { - warn("Can't get LL address from namespace: %s", - strerror_(-rc)); - } - - rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); - if (rc < 0) { - warn("Can't set nodad for LL in namespace: %s", - strerror_(-rc)); - } - - /* We dodged DAD: re-enable neighbour solicitations */ - nl_link_set_flags(nl_sock_ns, c->pasta_ifi, - 0, IFF_NOARP); - - if (c->ip6.no_copy_addrs) { - for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { - rc = nl_addr_set(nl_sock_ns, - c->pasta_ifi, - AF_INET6, &a->addr.a6, - a->prefix_len); - if (rc < 0) - break; - } - } else { - rc = nl_addr_dup(nl_sock, c->ifi6, - nl_sock_ns, c->pasta_ifi, - AF_INET6); - } - - if (rc < 0) { - die("Couldn't set IPv6 address(es) in namespace: %s", - strerror_(-rc)); - } - - if (c->ip6.no_copy_routes) { - rc = nl_route_set_def(nl_sock_ns, c->pasta_ifi, - AF_INET6, - &c->ip6.guest_gw); - } else { - rc = nl_route_dup(nl_sock, c->ifi6, - nl_sock_ns, c->pasta_ifi, - AF_INET6); - } - - if (c->ifi6 == -1 && rc == -ENOTSUP) { - warn("IPv6 not supported, disabling"); - c->ifi6 = 0; - goto ipv6_done; - } - - if (rc < 0) { - die("Couldn't set IPv6 route(s) in guest: %s", - strerror_(-rc)); - } - } + proto_update_l2_buf(c->guest_mac); + + if (!c->pasta_conf_ns) + return; + + if (c->mtu) + nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); + + if (c->ifi6) /* Avoid duplicate address detection on link up */ + flags |= IFF_NOARP;

Pre-existing, but this looks weird. Why are we setting a property for IPv6 next to the IPv4 configuration code? And does IFF_NOARP actually affect IPv6 DAD? I thought that was the IFA_F_NODAD flag which we use elsewhere.

+ + nl_link_set_flags(nl_sock_ns, c->pasta_ifi, flags, flags); + + /* IPv4 configuration */ + rc = pasta_conf_addrs(c, AF_INET, c->ifi4, c->ip4.no_copy_addrs); + if (c->ifi4 == -1 && rc == -ENOTSUP) { + warn("IPv4 not supported, disabling"); + c->ifi4 = 0; + } else if (rc < 0) { + die("Couldn't set IPv4 address(es): %s", strerror_(-rc)); + } else if (c->ifi4) { + rc = pasta_conf_routes(c, AF_INET, c->ifi4, + c->ip4.no_copy_routes); + if (rc < 0) + die("Couldn't set IPv4 route(s): %s", strerror_(-rc)); } -ipv6_done:

- proto_update_l2_buf(c->guest_mac); + if (!c->ifi6) + return; + + rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, + &c->ip6.addr_ll_seen); + if (rc < 0) + warn("Can't get LL address from namespace: %s", + strerror_(-rc)); + + rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); + if (rc < 0) + warn("Can't set nodad for LL in namespace: %s", + strerror_(-rc)); + + /* We dodged DAD: re-enable neighbour solicitations */ + nl_link_set_flags(nl_sock_ns, c->pasta_ifi, 0, IFF_NOARP); + + rc = pasta_conf_addrs(c, AF_INET6, c->ifi6, c->ip6.no_copy_addrs); + if (c->ifi6 == -1 && rc == -ENOTSUP) { + warn("IPv6 not supported, disabling"); + c->ifi6 = 0; + } else if (rc < 0) { + die("Couldn't set IPv6 address(es): %s", strerror_(-rc)); + } else { + rc = pasta_conf_routes(c, AF_INET6, c->ifi6, + c->ip6.no_copy_routes); + if (rc < 0) + die("Couldn't set IPv6 route(s): %s", strerror_(-rc)); + } }

/** -- 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

On Sun, Apr 12, 2026 at 08:53:15PM -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.

The separate check against addr_seen in fwd_guest_accessible() can now be removed because the observed address is now in the unified array, and the existing for_each_addr() loop already checks against all addresses, including this one.

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.

v7: - Added a commit at the beginning of the series addressing Stefano's concern about DHCPv6 reply addresses. - Some other updates based on feedback from David and Stefano. --- conf.c | 4 ---- dhcpv6.c | 4 +--- fwd.c | 38 +++++++++++++++++++++++--------------- inany.h | 3 +++ migrate.c | 37 +++++++++++++++++++++++++++---------- passt.h | 4 ---- pasta.c | 11 +++++++---- tap.c | 17 +++++------------ 8 files changed, 66 insertions(+), 52 deletions(-)

diff --git a/conf.c b/conf.c index f503d0f..3cb3553 100644 --- a/conf.c +++ b/conf.c @@ -827,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) { @@ -836,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 0a064a9..447aaba 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -567,8 +567,8 @@ 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; + const struct guest_addr *a; struct msg_hdr mh_storage; const struct msg_hdr *mh; struct udphdr uh_storage; @@ -683,8 +683,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, resp.hdr.xid = mh->xid;

tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); - if (a) - c->ip6.addr_seen = a->addr.a6;

return 1; } diff --git a/fwd.c b/fwd.c index 8c7bf91..b177be9 100644 --- a/fwd.c +++ b/fwd.c @@ -1095,14 +1095,6 @@ static bool fwd_guest_accessible(const struct ctx *c, 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);

As for IPv4, do we actually need the two-phase search, or is the ordering of addresses in the array enough?

+ 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 ll = inany_is_linklocal6(&tgt->oaddr); + uint8_t excl = ll ? ~CONF_ADDR_LINKLOCAL : CONF_ADDR_LINKLOCAL;

Uuhhh... I recall Stefano suggested using this encoding technique of using ~flag to indicate a different meaning. But AFACT handling that encoding is not actually implemented in fwd_get_addr() in this series. Plus, that encoding technique doesn't really work any more if you allow multiple bits in the excl fields.

+ 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 0450c45..ddcf93d 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) }) + /** 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..2dc4dd9 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 */

Seems like this change belongs in the previous patch.

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) + if (addrs.addr4.s_addr) { fwd_set_addr(c, &inany_from_v4(addrs.addr4), CONF_ADDR_OBSERVED, 0); - + }

Another change that belongs in the previous patch.

+ 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 5da1d55..3ef84eb 100644 --- a/passt.h +++ b/passt.h @@ -121,8 +121,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] @@ -138,8 +136,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 f949dc2..f7f1e07 100644 --- a/pasta.c +++ b/pasta.c @@ -366,6 +366,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); @@ -415,12 +416,14 @@ void pasta_ns_conf(struct ctx *c) if (!c->ifi6) return;

- rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, - &c->ip6.addr_ll_seen); - if (rc < 0) + 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_OBSERVED, 0); + } rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); if (rc < 0) warn("Can't set nodad for LL in namespace: %s", diff --git a/tap.c b/tap.c index 7f04e12..7f7f0ce 100644 --- a/tap.c +++ b/tap.c @@ -933,20 +933,13 @@ resume: continue; }

- if (IN6_IS_ADDR_LINKLOCAL(saddr)) { - c->ip6.addr_ll_seen = *saddr; + if (!IN6_IS_ADDR_UNSPECIFIED(saddr)) { + uint8_t flags = CONF_ADDR_OBSERVED;

- 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 }; + if (IN6_IS_ADDR_LINKLOCAL(saddr)) + flags |= CONF_ADDR_LINKLOCAL;

- fwd_set_addr(c, &addr, CONF_ADDR_LINKLOCAL, 64); - } - } else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){ - c->ip6.addr_seen = *saddr; + fwd_set_addr(c, &inany_from_v6(*saddr), flags, 0); }

if (proto == IPPROTO_ICMPV6) { -- 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

On Sun, Apr 12, 2026 at 08:53:17PM -0400, Jon Maloy wrote:

We introduce a CONF_ADDR_DHCP flag to mark if an added address is eligible for DHCP advertisement. By doing this once and for all in the fwd_set_addr() function, the DHCP code only needs to check for this flag to know that all criteria for advertisement are fulfilled. Hence, we update the code in dhcp.c correspondingly.

We also let the conf_print() function use this flag to determine and print the selected address.

Signed-off-by: Jon Maloy

--- v6: -Split off from a commit handling both DHCP and DHCPv6

v7: -Modified DHCP advertisement eligibility criteria IPv4 addresses: We now permit link local addresses to be eligible if they were configured by the user. -Adapted to previous changes in this series --- conf.c | 5 +++-- dhcp.c | 14 +++++++++----- fwd.c | 8 ++++++++ migrate.c | 5 +++++ passt.h | 1 + 5 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/conf.c b/conf.c index 3cb3553..612df07 100644 --- a/conf.c +++ b/conf.c @@ -46,6 +46,7 @@ #include "lineread.h" #include "isolation.h" #include "log.h" +#include "fwd.h" #include "vhost_user.h"

#define NETNS_RUN_DIR "/run/netns" @@ -1181,8 +1182,8 @@ static void conf_print(const struct ctx *c) inet_ntop(AF_INET, &c->ip4.map_host_loopback, buf, sizeof(buf)));

- a = fwd_get_addr(c, AF_INET, 0, 0); - if (a && !c->no_dhcp) { + a = fwd_get_addr(c, AF_INET, CONF_ADDR_DHCP, 0); + if (a) { uint32_t mask;

mask = IN4_MASK(inany_prefix_len(&a->addr, diff --git a/dhcp.c b/dhcp.c index f0fa212..0f98cfc 100644 --- a/dhcp.c +++ b/dhcp.c @@ -31,6 +31,8 @@ #include "passt.h" #include "tap.h" #include "log.h" +#include "fwd.h" +#include "conf.h" #include "dhcp.h"

/** @@ -302,19 +304,18 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) */ int dhcp(const struct ctx *c, struct iov_tail *data) { + struct in_addr addr, mask, dst; char macstr[ETH_ADDRSTRLEN]; const struct guest_addr *a; size_t mlen, dlen, opt_len; - struct in_addr mask, dst; struct ethhdr eh_storage; struct iphdr iph_storage; struct udphdr uh_storage; + const struct udphdr *uh; const struct ethhdr *eh; const struct iphdr *iph; - const struct udphdr *uh; struct msg m_storage; struct msg const *m; - struct in_addr addr; struct msg reply; unsigned int i;

@@ -346,8 +347,11 @@ int dhcp(const struct ctx *c, struct iov_tail *data) m->op != BOOTREQUEST) return -1;

- a = fwd_get_addr(c, AF_INET, 0, 0); - assert(a); + /* Select address to offer */ + a = fwd_get_addr(c, AF_INET, CONF_ADDR_DHCP, 0); + if (!a) + return -1; + addr = *inany_v4(&a->addr);

reply.op = BOOTREPLY; diff --git a/fwd.c b/fwd.c index b177be9..39e52c4 100644 --- a/fwd.c +++ b/fwd.c @@ -293,6 +293,14 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr, return; }

+ /* Determine advertisement eligibility */ + if (inany_v4(addr)) { + if ((flags & CONF_ADDR_USER) || + (flags & CONF_ADDR_HOST && !(flags & CONF_ADDR_LINKLOCAL))) + if (!c->no_dhcp)

Nit: the !no_dhcp test can become part of the outermost if.

+ flags |= CONF_ADDR_DHCP; + } + /* Add to head or tail, depending on flag */ if (flags & CONF_ADDR_OBSERVED) { a = &arr[0]; diff --git a/migrate.c b/migrate.c index 93f67ae..afdc8b4 100644 --- a/migrate.c +++ b/migrate.c @@ -52,6 +52,7 @@ struct migrate_seen_addrs_v2 { #define MIGRATE_ADDR_HOST BIT(1) #define MIGRATE_ADDR_LINKLOCAL BIT(2) #define MIGRATE_ADDR_OBSERVED BIT(3) +#define MIGRATE_ADDR_DHCP BIT(4)

/** * struct migrate_addr_v3 - Migration format for a single address entry @@ -83,6 +84,8 @@ static uint8_t flags_to_migration(uint8_t flags) migration |= MIGRATE_ADDR_LINKLOCAL; if (flags & CONF_ADDR_OBSERVED) migration |= MIGRATE_ADDR_OBSERVED; + if (flags & CONF_ADDR_DHCP) + migration |= MIGRATE_ADDR_DHCP;

return migration; } @@ -105,6 +108,8 @@ static uint8_t flags_from_migration(uint8_t migration) flags |= CONF_ADDR_LINKLOCAL; if (migration & MIGRATE_ADDR_OBSERVED) flags |= CONF_ADDR_OBSERVED; + if (migration & MIGRATE_ADDR_DHCP) + flags |= CONF_ADDR_DHCP;

This is a change to the migration protocol, although not exactly a breaking change to it. You can avoid the question my moving this patch before the one that introduces v3 migration.

return flags; } diff --git a/passt.h b/passt.h index 3ef84eb..9508c2a 100644 --- a/passt.h +++ b/passt.h @@ -83,6 +83,7 @@ struct guest_addr { #define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ #define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ +#define CONF_ADDR_DHCP BIT(5) /* Advertise via DHCP (IPv4) */ };

/** -- 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

On Sun, Apr 12, 2026 at 08:53:19PM -0400, Jon Maloy wrote:

We extend NDP to advertise all suitable IPv6 prefixes in Router Advertisements, per RFC 4861. Observed and link-local addresses, plus addresses with a prefix length != 64, are excluded.

Signed-off-by: Jon Maloy

--- v6: -Adapted to previous changes in series

v7: -Adapted to previous changes in series -Use struct initializer for source link-layer address option -Other minor fixes based on feedback from Stefano --- conf.c | 19 ++++++--- fwd.c | 4 ++ migrate.c | 5 +++ ndp.c | 123 +++++++++++++++++++++++++++++++++++++----------------- passt.h | 3 +- 5 files changed, 108 insertions(+), 46 deletions(-)

diff --git a/conf.c b/conf.c index 7c705de..97c66c9 100644 --- a/conf.c +++ b/conf.c @@ -1216,7 +1216,7 @@ static void conf_print(const struct ctx *c) }

if (c->ifi6) { - bool has_dhcpv6 = false; + bool has_ndp = false, has_dhcpv6 = false;

Nit: has_slaac might be a better name, since NDP has a number of functions.

const char *head;

if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback)) @@ -1225,18 +1225,25 @@ static void conf_print(const struct ctx *c) buf, sizeof(buf)));

for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { + if (a->flags & CONF_ADDR_SLAAC) + has_ndp = true; if (a->flags & CONF_ADDR_DHCPV6) has_dhcpv6 = true; }

- if (c->no_ndp && !has_dhcpv6) + if (!has_ndp && !has_dhcpv6) goto dns6;

- a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL); - if (!c->no_ndp && a) { + if (has_ndp) { info("NDP:"); - info(" assign: %s", - inany_ntop(&a->addr, buf, sizeof(buf))); + head = "assign: "; + for_each_addr(a, c->addrs, c->addr_count, AF_INET6) {

Nit: AF_INET6 is redundant with the CONF_ADDR_SLAAC check.

+ if (!(a->flags & CONF_ADDR_SLAAC)) + continue; + inany_ntop(&a->addr, buf, sizeof(buf)); + info(" %s: %s/%d", head, buf, a->prefix_len); + head = " "; + } }

if (has_dhcpv6) { diff --git a/fwd.c b/fwd.c index 2b444fb..2bc8e33 100644 --- a/fwd.c +++ b/fwd.c @@ -302,6 +302,10 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr, } else if (!(flags & CONF_ADDR_LINKLOCAL)) { if (!c->no_dhcpv6) flags |= CONF_ADDR_DHCPV6; + + /* NDP/RA only if prefix is /64 */ + if (!c->no_ndp && prefix_len == 64) + flags |= CONF_ADDR_SLAAC; }

/* Add to head or tail, depending on flag */ diff --git a/migrate.c b/migrate.c index adcbc63..f019924 100644 --- a/migrate.c +++ b/migrate.c @@ -54,6 +54,7 @@ struct migrate_seen_addrs_v2 { #define MIGRATE_ADDR_OBSERVED BIT(3) #define MIGRATE_ADDR_DHCP BIT(4) #define MIGRATE_ADDR_DHCPV6 BIT(5) +#define MIGRATE_ADDR_SLAAC BIT(6)

Same comments again about migration protocol.

/** * struct migrate_addr_v3 - Migration format for a single address entry @@ -89,6 +90,8 @@ static uint8_t flags_to_migration(uint8_t flags) migration |= MIGRATE_ADDR_DHCP; if (flags & CONF_ADDR_DHCPV6) migration |= MIGRATE_ADDR_DHCPV6; + if (flags & CONF_ADDR_SLAAC) + migration |= MIGRATE_ADDR_SLAAC;

return migration; } @@ -115,6 +118,8 @@ static uint8_t flags_from_migration(uint8_t migration) flags |= CONF_ADDR_DHCP; if (migration & MIGRATE_ADDR_DHCPV6) flags |= CONF_ADDR_DHCPV6; + if (migration & MIGRATE_ADDR_SLAAC) + flags |= CONF_ADDR_SLAAC;

return flags; } diff --git a/ndp.c b/ndp.c index 3750fc5..bb8374a 100644 --- a/ndp.c +++ b/ndp.c @@ -32,6 +32,8 @@ #include "passt.h" #include "tap.h" #include "log.h" +#include "fwd.h" +#include "conf.h"

#define RT_LIFETIME 65535

@@ -99,6 +101,16 @@ struct opt_prefix_info { uint32_t reserved; } __attribute__((packed));

+/** + * struct ndp_prefix - Prefix Information option with prefix + * @info: Prefix Information option header + * @prefix: IPv6 prefix + */ +struct ndp_prefix { + struct opt_prefix_info info; + struct in6_addr prefix; +} __attribute__((__packed__)); + /** * struct opt_mtu - Maximum transmission unit (MTU) option * @header: Option header @@ -140,27 +152,23 @@ struct opt_dnssl { } __attribute__((packed));

/** - * struct ndp_ra - NDP Router Advertisement (RA) message + * struct ndp_ra_hdr - NDP Router Advertisement fixed header * @ih: ICMPv6 header * @reachable: Reachability time, after confirmation (ms) * @retrans: Time between retransmitted NS messages (ms) - * @prefix_info: Prefix Information option - * @prefix: IPv6 prefix - * @mtu: MTU option - * @source_ll: Target link-layer address - * @var: Variable fields */ -struct ndp_ra { +struct ndp_ra_hdr { struct icmp6hdr ih; uint32_t reachable; uint32_t retrans; - struct opt_prefix_info prefix_info; - struct in6_addr prefix; - struct opt_l2_addr source_ll; +} __attribute__((__packed__));

- unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + - sizeof(struct opt_dnssl)]; -} __attribute__((packed, aligned(__alignof__(struct in6_addr)))); +/* Maximum RA message size: hdr + prefixes + source_ll + mtu + rdnss + dnssl */ +#define NDP_RA_MAX_SIZE (sizeof(struct ndp_ra_hdr) + \ + MAX_GUEST_ADDRS * sizeof(struct ndp_prefix) + \ + sizeof(struct opt_l2_addr) + \ + sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + \ + sizeof(struct opt_dnssl))

/** * struct ndp_ns - NDP Neighbor Solicitation (NS) message @@ -231,6 +239,42 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) ndp_na(c, &in6addr_ll_all_nodes, addr); }

+/** + * ndp_prefix_fill() - Fill prefix options for all suitable addresses + * @c: Execution context + * @buf: Buffer to write prefix options into + * + * Fills buffer with Prefix Information options for all non-linklocal, + * non-observed addresses with prefix_len == 64 + * + * Return: number of bytes written + */ +static size_t ndp_prefix_fill(const struct ctx *c, unsigned char *buf) +{ + const struct guest_addr *a; + struct ndp_prefix *p; + size_t offset = 0; + + for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { + if (!(a->flags & CONF_ADDR_SLAAC)) + continue; + + p = (struct ndp_prefix *)(buf + offset); + p->info.header.type = OPT_PREFIX_INFO; + p->info.header.len = 4; /* 4 * 8 = 32 bytes */ + p->info.prefix_len = 64; + p->info.prefix_flags = 0xc0; /* L, A flags */ + p->info.valid_lifetime = ~0U; + p->info.pref_lifetime = ~0U; + p->info.reserved = 0; + p->prefix = a->addr.a6; + + offset += sizeof(struct ndp_prefix); + } + + return offset; +} + /** * ndp_ra() - Send an NDP Router Advertisement (RA) message * @c: Execution context @@ -238,7 +282,15 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr) */ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) { - struct ndp_ra ra = { + unsigned char buf[NDP_RA_MAX_SIZE] + __attribute__((__aligned__(__alignof__(struct in6_addr)))); + struct ndp_ra_hdr *hdr = (struct ndp_ra_hdr *)buf; + struct opt_l2_addr *source_ll; + unsigned char *ptr; + size_t prefix_len; + + /* Build RA header */ + *hdr = (struct ndp_ra_hdr){ .ih = { .icmp6_type = RA, .icmp6_code = 0, @@ -247,31 +299,26 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) .icmp6_rt_lifetime = htons_constant(RT_LIFETIME), .icmp6_addrconf_managed = 1, }, - .prefix_info = { - .header = { - .type = OPT_PREFIX_INFO, - .len = 4, - }, - .prefix_len = 64, - .prefix_flags = 0xc0, /* prefix flags: L, A */ - .valid_lifetime = ~0U, - .pref_lifetime = ~0U, - }, - .source_ll = { - .header = { - .type = OPT_SRC_L2_ADDR, - .len = 1, - }, - }, }; - const struct guest_addr *a = fwd_get_addr(c, AF_INET6, 0, 0); - unsigned char *ptr = NULL;

- ASSERT(a); - - ra.prefix = a->addr.a6; + /* Fill prefix options */ + prefix_len = ndp_prefix_fill(c, (unsigned char *)(hdr + 1)); + if (prefix_len == 0) { + /* No suitable prefixes to advertise */ + return;

Do we want this? I think it's still valid to present other options via NDP, even if we have no prefixes to advertise. And, I think we want to do so for at least the MTU.

+ }

- ptr = &ra.var[0]; + /* Add source link-layer address option */ + ptr = (unsigned char *)(hdr + 1) + prefix_len; + source_ll = (struct opt_l2_addr *)ptr; + *source_ll = (struct opt_l2_addr) { + .header = { + .type = OPT_SRC_L2_ADDR, + .len = 1, + }, + }; + memcpy(source_ll->mac, c->our_tap_mac, ETH_ALEN); + ptr += sizeof(struct opt_l2_addr);

if (c->mtu) { struct opt_mtu *mtu = (struct opt_mtu *)ptr; @@ -345,10 +392,8 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) } }

- memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); - /* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */ - ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra); + ndp_send(c, dst, buf, ptr - buf); }

/** diff --git a/passt.h b/passt.h index 028eb7c..2c633e4 100644 --- a/passt.h +++ b/passt.h @@ -84,7 +84,8 @@ struct guest_addr { #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ #define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ #define CONF_ADDR_DHCP BIT(5) /* Advertise via DHCP (IPv4) */ -#define CONF_ADDR_DHCPV6 BIT(6) /* Advertise via DHCPv6 (IPv6) */ +#define CONF_ADDR_DHCPV6 BIT(6) /* Advertise via DHCPv6 */

This change belongs in the previous patch.

+#define CONF_ADDR_SLAAC BIT(7) /* Advertise via NDP/RA (/64) */ };

/** -- 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

On Sun, 12 Apr 2026 20:53:09 -0400 Jon Maloy wrote:

We replace the fwd_guest_accessible4() and fwd_guest_accessible6() functions with a unified fwd_guest_accessible() function that handles both address families. With the unified address array, we can check all configured addresses in a single pass using for_each_addr() with family filter AF_UNSPEC.

Signed-off-by: Jon Maloy

--- v6: -Some fixes based on feedback from David Gibson v7: -Added curly brackets to for_each_addr() in fwd_guest_accessible(), as suggested by Stefano Brivio. --- fwd.c | 69 +++++++++++++---------------------------------------------- 1 file changed, 15 insertions(+), 54 deletions(-)

diff --git a/fwd.c b/fwd.c index 14ce0a7..e676c18 100644 --- a/fwd.c +++ b/fwd.c @@ -988,19 +988,19 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) }

/** - * fwd_guest_accessible4() - Is IPv4 address guest-accessible + * fwd_guest_accessible() - Is address guest-accessible * @c: Execution context - * @addr: Host visible IPv4 address + * @addr: Host visible address (IPv4 or IPv6) * * Return: true if @addr on the host is accessible to the guest without * translation, false otherwise */ -static bool fwd_guest_accessible4(const struct ctx *c, - const struct in_addr *addr) +static bool fwd_guest_accessible(const struct ctx *c, + const union inany_addr *addr) { const struct guest_addr *a;

- if (IN4_IS_ADDR_LOOPBACK(addr)) + if (inany_is_loopback(addr)) return false;

/* In socket interfaces 0.0.0.0 generally means "any" or unspecified, @@ -1008,38 +1008,18 @@ static bool fwd_guest_accessible4(const struct ctx *c, * that has a different meaning for host and guest, we can't let it * through untranslated. */ - if (IN4_IS_ADDR_UNSPECIFIED(addr)) + if (inany_is_unspecified(addr)) return false;

- /* For IPv4, addr_seen is initialised to addr, so is always a valid - * address + /* Check against all configured guest addresses */ + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { + if (inany_equals(addr, &a->addr)) + return false; + }

A lookup function as I was suggesting in the review of 2/13 would also be convenient here.

+ /* Also check addr_seen: it tracks the address the guest is actually + * using, which may differ from configured addresses. */ - a = fwd_get_addr(c, AF_INET, 0, 0); - if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || - IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) - return false; - - return true; -} - -/** - * fwd_guest_accessible6() - Is IPv6 address guest-accessible - * @c: Execution context - * @addr: Host visible IPv6 address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible6(const struct ctx *c, - const struct in6_addr *addr) -{ - const struct guest_addr *a; - - if (IN6_IS_ADDR_LOOPBACK(addr)) - return false; - - a = fwd_get_addr(c, AF_INET6, 0, 0); - if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) + if (inany_equals4(addr, &c->ip4.addr_seen)) return false;

/* For IPv6, addr_seen starts unspecified, because we don't know what LL @@ -1047,31 +1027,12 @@ static bool fwd_guest_accessible6(const struct ctx *c, * 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)) + inany_equals6(addr, &c->ip6.addr_seen)) return false;

return true; }

-/** - * fwd_guest_accessible() - Is IPv[46] address guest-accessible - * @c: Execution context - * @addr: Host visible IPv[46] address - * - * Return: true if @addr on the host is accessible to the guest without - * translation, false otherwise - */ -static bool fwd_guest_accessible(const struct ctx *c, - const union inany_addr *addr) -{ - const struct in_addr *a4 = inany_v4(addr); - - if (a4) - return fwd_guest_accessible4(c, a4); - - return fwd_guest_accessible6(c, &addr->a6); -} - /** * nat_outbound() - Apply address translation for outbound (TAP to HOST) * @c: Execution context

-- Stefano

On Sun, 12 Apr 2026 20:53:11 -0400 Jon Maloy wrote:

Allow specifying multiple addresses per family with -a/--address. The first address of each family is used for DHCP/DHCPv6 assignment.

Signed-off-by: Jon Maloy

I guess it would be finally time to triumphally mention: Link: https://bugs.passt.top/show_bug.cgi?id=47 ...and Cc: its reporter on this one. :)

--- v2: - Adapted to previous code changes v3: - Adapted to single-array strategy - Changes according to feedback from S. Brivio and D. Gibson. v4: - Stripped down and adapted after feedback from David G. v6: - Adapted to previous changes in series - Removed the "one address" limitation for -n option v7: - Updated man page. --- conf.c | 7 ++++--- fwd.c | 4 +--- passt.1 | 7 +++---- pasta.c | 14 ++++++++------ 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/conf.c b/conf.c index 591f561..8b4a7a0 100644 --- a/conf.c +++ b/conf.c @@ -939,9 +939,11 @@ static void usage(const char *name, FILE *f, int status) " default: 65520: maximum 802.3 MTU minus 802.3 header\n" " length, rounded to 32 bits (IPv4 words)\n" " -a, --address ADDR Assign IPv4 or IPv6 address ADDR[/PREFIXLEN]\n" - " can be specified zero to two times (for IPv4 and IPv6)\n" + " can be specified up to a maximum of %d times\n" " default: use addresses from interface with default route\n" - " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n" + " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n", + MAX_GUEST_ADDRS); + FPRINTF(f, " default: netmask from matching address on the host\n" " -M, --mac-addr ADDR Use source MAC address ADDR\n" " default: 9a:55:9a:55:9a:55 (locally administered)\n" @@ -1898,7 +1900,6 @@ void conf(struct ctx *c, int argc, char **argv) IN6_IS_ADDR_V4COMPAT(&addr.a6)) die("Invalid address: %s", optarg);

- /* 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 = true; diff --git a/fwd.c b/fwd.c index e676c18..d3f576a 100644 --- a/fwd.c +++ b/fwd.c @@ -250,14 +250,12 @@ void fwd_neigh_table_init(const struct ctx *c) }

/** - * fwd_set_addr() - Add or update an address in the unified address array + * fwd_set_addr() - Update address entry, adding one if needed * @c: Execution context * @addr: Address to add (IPv4-mapped or IPv6) * @flags: CONF_ADDR_* flags for this address * @prefix_len: Prefix length in IPv6 or IPv4 format * - * 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) diff --git a/passt.1 b/passt.1 index 13e8df9..12ec857 100644 --- a/passt.1 +++ b/passt.1 @@ -164,16 +164,13 @@ An optional /\fIprefix_len\fR (0-32 for IPv4, 0-128 for IPv6) can be appended in CIDR notation (e.g. 192.0.2.1/24). This is an alternative to using the \fB-n\fR, \fB--netmask\fR option. Mixing CIDR notation with \fB-n\fR results in an error. -If a prefix length is assigned to an IPv6 address using this method, it will -in the current code version be overridden by the default value of 64. -This option can be specified zero (for defaults) to two times (once for IPv4, -once for IPv6). By default, assigned IPv4 and IPv6 addresses are taken from the host interfaces with the first default route, if any, for the corresponding IP version. If no default routes are available and there is any interface with any route for a given IP version, the first of these interfaces will be chosen instead. If no such interface exists for a given IP version, the link-local address 169.254.2.1 is assigned for IPv4, and no additional address will be assigned for IPv6. +This option can be given multiple times, indicating multiple different addresses.

.TP .BR \-n ", " \-\-netmask " " \fImask @@ -181,6 +178,8 @@ Assign IPv4 netmask \fImask\fR, expressed as dot-decimal or number of bits, via DHCP (option 1). Alternatively, the prefix length can be specified using CIDR notation with the \fB-a\fR, \fB--address\fR option (e.g. \fB-a\fR 192.0.2.1/24). Mixing \fB-n\fR with CIDR notation results in an error. +When indicated, this option sets the prefix length of the first configured +IPv4 address only. If no address is indicated, the netmask associated with the adopted host address, if any, is used. If an address is indicated, but without a prefix length, the netmask is determined based on the corresponding network class. In all other diff --git a/pasta.c b/pasta.c index c51e4cd..b3936f5 100644 --- a/pasta.c +++ b/pasta.c @@ -343,14 +343,15 @@ void pasta_ns_conf(struct ctx *c)

if (c->ifi4) { if (c->ip4.no_copy_addrs) { - a = fwd_get_addr(c, AF_INET, 0, 0); - if (a) { + for_each_addr(a, c->addrs, c->addr_count, AF_INET) { plen = inany_prefix_len(&a->addr, a->prefix_len); rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET, inany_v4(&a->addr), plen); + if (rc < 0) + break; } } else { rc = nl_addr_dup(nl_sock, c->ifi4, @@ -404,13 +405,14 @@ ipv4_done: 0, IFF_NOARP);

if (c->ip6.no_copy_addrs) { - a = fwd_get_addr(c, AF_INET6, 0, 0); - if (a) + for_each_addr(a, c->addrs, c->addr_count, AF_INET6) { rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, - AF_INET6, - &a->addr.a6, + AF_INET6, &a->addr.a6, a->prefix_len); + if (rc < 0) + break; + } } else { rc = nl_addr_dup(nl_sock, c->ifi6, nl_sock_ns, c->pasta_ifi,

-- Stefano

On Wed, 27 May 2026 13:40:51 +1000 David Gibson wrote:

On Sun, Apr 12, 2026 at 08:53:15PM -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.

The separate check against addr_seen in fwd_guest_accessible() can now be removed because the observed address is now in the unified array, and the existing for_each_addr() loop already checks against all addresses, including this one.

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.

v7: - Added a commit at the beginning of the series addressing Stefano's concern about DHCPv6 reply addresses. - Some other updates based on feedback from David and Stefano. --- conf.c | 4 ---- dhcpv6.c | 4 +--- fwd.c | 38 +++++++++++++++++++++++--------------- inany.h | 3 +++ migrate.c | 37 +++++++++++++++++++++++++++---------- passt.h | 4 ---- pasta.c | 11 +++++++---- tap.c | 17 +++++------------ 8 files changed, 66 insertions(+), 52 deletions(-)

diff --git a/conf.c b/conf.c index f503d0f..3cb3553 100644 --- a/conf.c +++ b/conf.c @@ -827,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) { @@ -836,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 0a064a9..447aaba 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -567,8 +567,8 @@ 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; + const struct guest_addr *a; struct msg_hdr mh_storage; const struct msg_hdr *mh; struct udphdr uh_storage; @@ -683,8 +683,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, resp.hdr.xid = mh->xid;

tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); - if (a) - c->ip6.addr_seen = a->addr.a6;

return 1; } diff --git a/fwd.c b/fwd.c index 8c7bf91..b177be9 100644 --- a/fwd.c +++ b/fwd.c @@ -1095,14 +1095,6 @@ static bool fwd_guest_accessible(const struct ctx *c, 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);

As for IPv4, do we actually need the two-phase search, or is the ordering of addresses in the array enough?

...

+ 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 ll = inany_is_linklocal6(&tgt->oaddr); + uint8_t excl = ll ? ~CONF_ADDR_LINKLOCAL : CONF_ADDR_LINKLOCAL;

Uuhhh... I recall Stefano suggested using this encoding technique of using ~flag to indicate a different meaning.

Hmm, no, wait, I was suggesting something different: instead of two incl, excl arguments, a single 'flags' argument which is interpreted as an exclusion mask if an inversion of a single bit is set, so that you could do: fwd_select_addr(c, AF_INET6, CONF_ADDR_OBSERVED); to get observed addresses, and: fwd_select_addr(c, AF_INET6, ~CONF_ADDR_OBSERVED); to get everything else (see that in use in conn_flag_do(), tcp.c). But if multiple bits can be filtered on, that won't work.

But AFACT handling that encoding is not actually implemented in fwd_get_addr() in this series.

The usage here is different: this seems to be meant to exclude link-local addresses (and, I concur, CONF_ADDR_LINKLOCAL should be another type of flag) if tgt->oaddr is not link-local, and exclude all other types if it is. I don't think it will work for all the other types because, if 'll' is false, we'll exclude CONF_ADDR_USER and CONF_ADDR_HOST anyway (which are the only ones being selected), but it's rather an issue with how 'excl' is set rather than a particular encoding.

Plus, that encoding technique doesn't really work any more if you allow multiple bits in the excl fields.

Right, the one I suggested won't work. The one that was intended here should still work, in principle, I guess.

...

+ 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;

-- Stefano

On Sun, 12 Apr 2026 20:53:17 -0400 Jon Maloy wrote:

We introduce a CONF_ADDR_DHCP flag to mark if an added address is eligible for DHCP advertisement. By doing this once and for all in the fwd_set_addr() function, the DHCP code only needs to check for this flag to know that all criteria for advertisement are fulfilled. Hence, we update the code in dhcp.c correspondingly.

We also let the conf_print() function use this flag to determine and print the selected address.

Signed-off-by: Jon Maloy

[...]

--- a/passt.h +++ b/passt.h @@ -83,6 +83,7 @@ struct guest_addr { #define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ #define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ +#define CONF_ADDR_DHCP BIT(5) /* Advertise via DHCP (IPv4) */

I think the name might lead to confusion, especially the day we get a netlink monitoring for addresses, as it might seem to imply that this comes from an address dynamically configured on the host. Perhaps CONF_ADDR_DHCPOFFER? -- Stefano

On Sat, Jun 20, 2026 at 12:10:41AM +0200, Stefano Brivio wrote:

On Sun, 12 Apr 2026 20:53:14 -0400 Jon Maloy wrote:

...

We remove the addr_seen field in struct ip4_ctx and replace it by setting a new CONF_ADDR_OBSERVED flag in the corresponding entry in the unified address array.

The observed IPv4 address is always added at or moved to position 0, increasing chances for a fast lookup.

Signed-off-by: Jon Maloy

--- v4: - Removed migration protocol update, to be added in later commit - Allow only one OBSERVED address at a time - Some other changes based on feedback from David G v5: - Allowing multiple observed IPv4 addresses v6: - Refactored fwd_set_addr(), notably: o Limited number of allowed observed addresses to four per protocol o I kept the memmove() calls, since I find no more elegant way to do this. Performance cost should be minimal, since these parts of the code will execute only very exceptionally. Note that removing the 'oldest' entry implicitly means removing the least used one, since the latter will migrate to the highest position after a few iterations of remove/add. o Also kept the prefix_len update. Not sure about this, but I cannot see how the current approach can cause any harm. - Other changes suggested by David G, notably reversing some residues after an accidental merge/re-split with the next commit. v7: - Changed fwd_set_addr() to only accept keeping one observed-only address per protocol, as suggested by David.

Sorry, I just spotted this in David's review of v6. Actually, I think that keeping track of a few multiple observed addresses (especially with different scope) might be convenient and it would already be useful here together with 4/13 to avoid resolving via ARP any of a few addresses recently seen from the guest.

So.. not resolving ARPs is the one thing where we could actualy use multiple guest observed addresses - mostly we use it for directing traffic to the guest, for which we need a single address. But.. I feel like switching the ARP resolution from "everything except" to "only these" would be a better solution. That also lets the guest move to a brand new unused address without getting bogus DAD failures.

While ARP probes for duplicate addresses are usually coming from DHCP clients, there might other mechanisms to assign addresses using those.

Besides, I think David's suggestion was to keep a single observed address per IP version _and_ scope, not just one per IP version.

Ah, yes, that is a good point. That was absolutely my intention. There's no question we need separate observed addresses for IPv6 link-local and IPv6 global. (or at least no question as long as we accept the need for observed addresses at all, which is a different discussion).

If we just keep one per version, regardless of the scope, we'll now cycle between one link-local and one global unicast address (in most cases), right?

...

- Eliminated redundant tap_check_src_addr4() call level. - I keep fwd_select_addr() for the same pragmatic reason it was introduced: to avoid ugly, deeply indented code that tends to wrap across several lines. --- conf.c | 6 --- fwd.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++------- fwd.h | 4 ++ migrate.c | 17 +++++++- passt.h | 6 +-- tap.c | 8 +++- 6 files changed, 136 insertions(+), 29 deletions(-)

diff --git a/conf.c b/conf.c index 924ade2..f503d0f 100644 --- a/conf.c +++ b/conf.c @@ -767,13 +767,8 @@ 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); }

- if (a) - ip4->addr_seen = *inany_v4(&a->addr); - ip4->our_tap_addr = ip4->guest_gw;

return ifi; @@ -787,7 +782,6 @@ static void conf_ip4_local(struct ctx *c) { struct ip4_ctx *ip4 = &c->ip4;

- ip4->addr_seen = IP4_LL_GUEST_ADDR; ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW; ip4->no_copy_addrs = ip4->no_copy_routes = true; fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), diff --git a/fwd.c b/fwd.c index d3f576a..8c7bf91 100644 --- a/fwd.c +++ b/fwd.c @@ -28,6 +28,7 @@ #include "inany.h" #include "fwd.h" #include "passt.h" +#include "conf.h" #include "lineread.h" #include "flow_table.h" #include "netlink.h" @@ -260,21 +261,68 @@ 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) { - struct guest_addr *a; + struct guest_addr *a, *arr = &c->addrs[0], *rm = NULL; + int count = c->addr_count; + int af_cnt = 0;

- for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) { - goto found; + for_each_addr(a, c->addrs, c->addr_count, AF_UNSPEC) { + if (!inany_equals(&a->addr, addr)) + continue; + + /* Adjust and update prefix_len if provided and applicable */ + if (prefix_len && !(a->flags & CONF_ADDR_USER)) + a->prefix_len = inany_prefix_len(addr, prefix_len); + + /* Nothing more to change */ + if ((a->flags & flags) == flags) + return; + + a->flags |= flags; + if (!(flags & CONF_ADDR_OBSERVED)) + return; + + /* Observed address moves to position 0: remove, re-add later */ + prefix_len = a->prefix_len; + memmove(a, a + 1, (&arr[count] - (a + 1)) * sizeof(*a)); + c->addr_count = --count; + break; }

- if (c->addr_count >= MAX_GUEST_ADDRS) + if (count >= MAX_GUEST_ADDRS) { + debug("Address table full, can't add address"); return; + }

- a = &c->addrs[c->addr_count++]; - -found: + /* Add to head or tail, depending on flag */ + if (flags & CONF_ADDR_OBSERVED) { + a = &arr[0]; + memmove(&arr[1], a, count * sizeof(*a)); + } else { + a = &arr[count]; + } + c->addr_count = ++count; a->addr = *addr; a->prefix_len = inany_prefix_len6(addr, prefix_len); a->flags = flags; + + if (!(flags & CONF_ADDR_OBSERVED)) + return; + + /* Remove excess observed-only address if more than one */ + for (int i = count - 1; i >= 0; i--) { + a = &arr[i]; + if (inany_af(&a->addr) != inany_af(addr)) + continue; + if (a->flags != CONF_ADDR_OBSERVED) + continue; + if (!rm) + rm = a; + af_cnt++; + } + if (af_cnt > 1) { + memmove(rm, rm + 1, (&arr[count] - (rm + 1)) * sizeof(*rm)); + c->addr_count--; + } }

/** @@ -985,6 +1033,38 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) ((ini->oport == 53) || (ini->oport == 853)); }

+/** + * fwd_select_addr() - Select address with priority-based search + * @c: Execution context + * @af: Address family (AF_INET or AF_INET6) + * @primary: Primary flags to match (or 0 to skip) + * @secondary: Secondary flags to match (or 0 to skip) + * @skip: Flags to exclude from search + * + * Search for address entries in priority order. + * + * Return: pointer to selected address entry, or NULL if none found + */ +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, + int primary, int secondary, int skip) +{ + const struct guest_addr *a; + + if (primary) { + a = fwd_get_addr(c, af, primary, skip); + if (a) + return a; + } + + if (secondary) { + a = fwd_get_addr(c, af, secondary, skip); + if (a) + return a; + } + + return NULL; +} + /** * fwd_guest_accessible() - Is address guest-accessible * @c: Execution context @@ -1014,11 +1094,6 @@ static bool fwd_guest_accessible(const struct ctx *c, if (inany_equals(addr, &a->addr)) return false; } - /* Also check addr_seen: it tracks the address the guest is actually - * using, which may differ from configured addresses. - */ - if (inany_equals4(addr, &c->ip4.addr_seen)) - 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 @@ -1214,10 +1289,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, * match. */ if (inany_v4(&ini->eaddr)) { - if (c->host_lo_to_ns_lo) + if (c->host_lo_to_ns_lo) { tgt->eaddr = inany_loopback4; - else - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); + } else { + const struct guest_addr *a; + + a = fwd_select_addr(c, AF_INET, + CONF_ADDR_OBSERVED, + CONF_ADDR_USER | + CONF_ADDR_HOST, 0); + if (!a) + return PIF_NONE; + + tgt->eaddr = a->addr; + } tgt->oaddr = inany_any4; } else { if (c->host_lo_to_ns_lo) @@ -1252,7 +1337,14 @@ uint8_t fwd_nat_from_host(const struct ctx *c, tgt->oport = ini->eport;

if (inany_v4(&tgt->oaddr)) { - tgt->eaddr = inany_from_v4(c->ip4.addr_seen); + const struct guest_addr *a; + + a = fwd_select_addr(c, AF_INET, CONF_ADDR_OBSERVED, + CONF_ADDR_USER | CONF_ADDR_HOST, 0); + if (!a) + return PIF_NONE; + + tgt->eaddr = a->addr; } else { if (inany_is_linklocal6(&tgt->oaddr)) tgt->eaddr.a6 = c->ip6.addr_ll_seen; diff --git a/fwd.h b/fwd.h index c5a1068..9893856 100644 --- a/fwd.h +++ b/fwd.h @@ -25,6 +25,10 @@ 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 af, uint8_t incl, uint8_t excl); +const struct guest_addr *fwd_select_addr(const struct ctx *c, int af, + int primary, int secondary, int skip); +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, + uint8_t flags, int prefix_len);

/** * struct fwd_rule - Forwarding rule governing a range of ports diff --git a/migrate.c b/migrate.c index 1e8858a..1e02720 100644 --- a/migrate.c +++ b/migrate.c @@ -18,6 +18,8 @@ #include "util.h" #include "ip.h" #include "passt.h" +#include "conf.h" +#include "fwd.h" #include "inany.h" #include "flow.h" #include "flow_table.h" @@ -57,11 +59,18 @@ static int seen_addrs_source_v2(struct ctx *c, struct migrate_seen_addrs_v2 addrs = { .addr6 = c->ip6.addr_seen, .addr6_ll = c->ip6.addr_ll_seen, - .addr4 = c->ip4.addr_seen, }; + const struct guest_addr *a;

(void)stage;

+ /* IPv4 observed address, with fallback to configured 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); + memcpy(addrs.mac, c->guest_mac, sizeof(addrs.mac));

if (write_all_buf(fd, &addrs, sizeof(addrs))) @@ -90,7 +99,11 @@ static int seen_addrs_target_v2(struct ctx *c,

c->ip6.addr_seen = addrs.addr6; c->ip6.addr_ll_seen = addrs.addr6_ll; - c->ip4.addr_seen = addrs.addr4; + + if (addrs.addr4.s_addr) + fwd_set_addr(c, &inany_from_v4(addrs.addr4), + CONF_ADDR_OBSERVED, 0); + memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac));

return 0; diff --git a/passt.h b/passt.h index f75656d..5da1d55 100644 --- a/passt.h +++ b/passt.h @@ -64,8 +64,9 @@ enum passt_modes { MODE_VU, };

-/* Maximum number of addresses in context address array */ +/* Limits on number of addresses in context address array */ #define MAX_GUEST_ADDRS 32 +#define MAX_OBSERVED_ADDRS 4

/** * struct guest_addr - Unified IPv4/IPv6 address entry @@ -81,11 +82,11 @@ struct guest_addr { #define CONF_ADDR_HOST BIT(1) /* From host interface */ #define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ #define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ +#define CONF_ADDR_OBSERVED BIT(4) /* Seen in guest traffic */ };

/** * struct ip4_ctx - IPv4 execution context - * @addr_seen: Latest IPv4 address seen as source from tap * @guest_gw: IPv4 gateway as seen by the guest * @map_host_loopback: Outbound connections to this address are NATted to the * host's 127.0.0.1 @@ -101,7 +102,6 @@ struct guest_addr { * @no_copy_addrs: Don't copy all addresses when configuring namespace */ struct ip4_ctx { - struct in_addr addr_seen; struct in_addr guest_gw; struct in_addr map_host_loopback; struct in_addr map_guest_addr; diff --git a/tap.c b/tap.c index eb93f74..7f04e12 100644 --- a/tap.c +++ b/tap.c @@ -47,6 +47,7 @@ #include "ip.h" #include "iov.h" #include "passt.h" +#include "fwd.h" #include "arp.h" #include "dhcp.h" #include "ndp.h" @@ -756,9 +757,12 @@ resume: continue; }

- if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr) - c->ip4.addr_seen.s_addr = iph->saddr; + if (iph->saddr) { + const union inany_addr *addr;

+ addr = &inany_from_v4(*(struct in_addr *) &iph->saddr); + fwd_set_addr(c, addr, CONF_ADDR_OBSERVED, 0); + } if (!iov_drop_header(&data, hlen)) continue; if (iov_tail_size(&data) != l4len)

-- Stefano

-- 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