On Wed, Feb 04, 2026 at 08:01:45PM -0500, Jon Maloy wrote:
On 2026-02-04 08:16, David Gibson wrote:
On Fri, Jan 30, 2026 at 04:44:40PM -0500, Jon Maloy wrote:
As a preparation for handling multiple addresses, we update fwd_guest_accessible4() and fwd_guest_accessible6() to check against all addresses in the unified addrs[] array using the for_each_addr() macro.
This ensures that when multiple addresses are configured via -a options, inbound traffic for any of them is correctly detected as having no valid forwarding path, and subsequently dropped. This occurs when a peer address collides with an address the guest is using, and we have no translation for it.
Signed-off-by: Jon Maloy
--- v2: Updated commit log to make it clearer v3: Adapted to changes earlier in the series --- fwd.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/fwd.c b/fwd.c index 54248a3..20d581d 100644 --- a/fwd.c +++ b/fwd.c @@ -502,6 +502,8 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) static bool fwd_guest_accessible4(const struct ctx *c, const struct in_addr *addr)
With the changes to this point, there's no longer a point to having separate fwd_guest_accessible4() and fwd_guest_accessible6() functions. fwd_guest_accessible() can check against the combined list in a single pass.
Absolutely. I have already identified several functions which can be unified in a similar way, and have already implemented some. But I still want to wait with such changes until this series has been applied.
That doesn't make a lot of sense to me. I mean, I understand doing things one step at a time. But in this case keeping the old structure means a bunch of extra churn, and the intermediate steps are more confusing to read and review than they need to be.
This is an example of the simplfications that I think will outweigh the complications introduced by using merged list for v4 and v6 addresses.
{ + const struct inany_addr_entry *e; + if (IN4_IS_ADDR_LOOPBACK(addr)) return false; @@ -513,12 +515,15 @@ static bool fwd_guest_accessible4(const struct ctx *c, if (IN4_IS_ADDR_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(c, e, AF_INET) + if (IN4_ARE_ADDR_EQUAL(addr, inany_v4(&e->addr))) + return false; + + /* Also check addr_seen: it tracks the address the guest is actually + * using, which may differ from configured addresses. */ - if ((first_v4(c) && - IN4_ARE_ADDR_EQUAL(addr, inany_v4(&first_v4(c)->addr))) || - IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) + if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) return false; return true; @@ -535,11 +540,15 @@ static bool fwd_guest_accessible4(const struct ctx *c, static bool fwd_guest_accessible6(const struct ctx *c, const struct in6_addr *addr) { + const struct inany_addr_entry *e; + if (IN6_IS_ADDR_LOOPBACK(addr)) return false; - if (first_v6(c) && IN6_ARE_ADDR_EQUAL(addr, &first_v6(c)->addr.a6)) - return false; + /* Check against all configured guest addresses */ + for_each_addr(c, e, AF_INET6) + if (IN6_ARE_ADDR_EQUAL(addr, &e->addr.a6)) + return false; /* For IPv6, addr_seen starts unspecified, because we don't know what LL * address the guest will take until we see it. Only check against it @@ -714,7 +723,7 @@ bool nat_inbound(const struct ctx *c, const union inany_addr *addr, first_v4(c) && inany_equals(addr, &first_v4(c)->addr)) { *translated = inany_from_v4(c->ip4.map_guest_addr); } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) && - first_v6(c) && inany_equals6(addr, &first_v6(c)->addr.a6)) { + first_v6(c) && inany_equals(addr, &first_v6(c)->addr)) { translated->a6 = c->ip6.map_guest_addr; } else if (fwd_guest_accessible(c, addr)) { *translated = *addr; -- 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