Re: [RFC 05/12] fwd: Check all configured addresses in guest accessibility functions

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3682 bytes --]

On Sun, Dec 14, 2025 at 08:54:34PM -0500, Jon Maloy wrote:
> We update fwd_guest_accessible4() and fwd_guest_accessible6() to check
> against all addresses in the addrs[] array, not just addrs[0].
> 
> This ensures that when multiple addresses are configured via -a options,
> traffic using any of them is correctly identified as guest traffic for
> NAT and forwarding decisions.

That last paragraph is not an accurate.  fwd_guest_accessible() isn't
about "identifying guest traffic".  It's about detecting inbound
traffic that we have *no way* to forward to the guest and dropping it.
This occurs when we have a peer address that we have no translation
for, but collides with an address the guest is using.

> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  fwd.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/fwd.c b/fwd.c
> index 408af30..ece381d 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)
>  {
> +	int i;
> +
>  	if (IN4_IS_ADDR_LOOPBACK(addr))
>  		return false;
>  
> @@ -513,11 +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 (i = 0; i < c->ip4.addr_count; i++)
> +		if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addrs[i].addr))
> +			return false;
> +
> +	/* Also check addr_seen: it tracks the address the guest is actually
> +	 * using, which may differ from configured addresses.
>  	 */
> -	if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addrs[0].addr) ||
> -	    IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen))
> +	if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen))

Really an overall series comment, rather than specific to this patch:

If we're allowing multiple addresses, it doesn't make sense to leave
the 'addr_seen' mechanism as-is.  If the guest actually uses multiple
addresses, then addr_seen will bounce around between them in a not
very meaningful way.

Personally, I've never been super-convinced that allowing the guest to
just use an arbitrary address we didn't give it is a good idea.  But,
I guess it's an established feature now.  I think the way to do that
in a multi-address environment would be to add addresses we observe
the guest using to the list of addresses.  They should probably be
flagged as having been observed coming from the guest, rather than
coming from either -a or the host.

>  		return false;
>  
>  	return true;
> @@ -534,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)
>  {
> +	int i;
> +
>  	if (IN6_IS_ADDR_LOOPBACK(addr))
>  		return false;
>  
> -	if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addrs[0].addr))
> -		return false;
> +	/* Check against all configured guest addresses */
> +	for (i = 0; i < c->ip6.addr_count; i++)
> +		if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addrs[i].addr))
> +			return false;
>  
>  	/* For IPv6, addr_seen starts unspecified, because we don't know what LL
>  	 * address the guest will take until we see it.  Only check against it
> -- 
> 2.51.1
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]