Re: [PATCH v2 8/9] ip: Track observed guest IPv4 addresses in unified address array

[Stefano Brivio <sbrivio@redhat.com>]

On Sun, 18 Jan 2026 17:16:11 -0500
Jon Maloy <jmaloy@redhat.com> wrote:

> We remove the addr_seen field in struct ip4_ctx and replace it by
> setting a new INANY_ADDR_OBSERVED flag in the corresponding entry in
> the new address array. If the seen address is not present in the
> array we add it first.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> ---
>  conf.c    |  3 ---
>  fwd.c     | 49 ++++++++++++++++++++++++++++++++++++++-----------
>  inany.h   |  1 +
>  migrate.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
>  passt.h   |  3 ---
>  tap.c     | 38 +++++++++++++++++++++++++++++++++-----
>  6 files changed, 114 insertions(+), 24 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 22c2222..8f0091d 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -787,8 +787,6 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
>  			ip4->addrs[i].prefix_len = ip4_default_prefix_len(a4);
>  	}
>  
> -	ip4->addr_seen = *inany_v4(&ip4->addrs[0].addr);
> -
>  	ip4->our_tap_addr = ip4->guest_gw;
>  
>  	if (!ip4->addr_count)
> @@ -804,7 +802,6 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
>  static void conf_ip4_local(struct ip4_ctx *ip4)
>  {
>  	ip4->addrs[0].addr = inany_from_v4(IP4_LL_GUEST_ADDR);
> -	ip4->addr_seen = *inany_v4(&ip4->addrs[0].addr);
>  	ip4->our_tap_addr = ip4->guest_gw = IP4_LL_GUEST_GW;
>  	ip4->addrs[0].prefix_len = IP4_LL_PREFIX_LEN;
>  	ip4->addr_count = 1;
> diff --git a/fwd.c b/fwd.c
> index f1db34c..08b0b83 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -491,6 +491,29 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini)
>  		((ini->oport == 53) || (ini->oport == 853));
>  }
>  
> +/**
> + * fwd_guest_addr4() - Get first observed IPv4 guest address
> + * @c:		Execution context
> + *
> + * Return: pointer to first observed IPv4 address, or first address if none
> + *         observed, or NULL if no addresses

Well, it's kind of obvious it's a pointer. What about:

 * Return: first observed IPv4 address if any, first configured if any, or NULL

?

> + */
> +static const struct in_addr *fwd_guest_addr4(const struct ctx *c)
> +{
> +	int i;
> +
> +	/* Find first observed address */
> +	for (i = 0; i < c->ip4.addr_count; i++)
> +		if (c->ip4.addrs[i].flags & INANY_ADDR_OBSERVED)
> +			return inany_v4(&c->ip4.addrs[i].addr);
> +
> +	/* Fallback to first address */
> +	if (c->ip4.addr_count > 0)
> +		return inany_v4(&c->ip4.addrs[0].addr);
> +
> +	return NULL;
> +}
> +
>  /**
>   * fwd_guest_accessible4() - Is IPv4 address guest-accessible
>   * @c:		Execution context
> @@ -515,17 +538,11 @@ static bool fwd_guest_accessible4(const struct ctx *c,
>  	if (IN4_IS_ADDR_UNSPECIFIED(addr))
>  		return false;
>  
> -	/* Check against all configured guest addresses */
> +	/* Check against all guest addresses */
>  	for (i = 0; i < c->ip4.addr_count; i++)
>  		if (IN4_ARE_ADDR_EQUAL(addr, inany_v4(&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.addr_seen))
> -		return false;
> -
>  	return true;
>  }
>  
> @@ -768,10 +785,16 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
>  		 * match.
>  		 */
>  		if (inany_v4(&ini->eaddr)) {
> -			if (c->host_lo_to_ns_lo)
> +			const struct in_addr *guest_addr;

cppcheck argues that:

fwd.c:1140:26: style: The scope of the variable 'guest_addr' can be reduced. [variableScope]
   const struct in_addr *guest_addr;
                         ^

> +
> +			if (c->host_lo_to_ns_lo) {
>  				tgt->eaddr = inany_loopback4;
> -			else
> -				tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
> +			} else {
> +				guest_addr = fwd_guest_addr4(c);
> +				if (!guest_addr)
> +					return PIF_NONE;
> +				tgt->eaddr = inany_from_v4(*guest_addr);
> +			}
>  			tgt->oaddr = inany_any4;
>  		} else {
>  			if (c->host_lo_to_ns_lo)
> @@ -803,7 +826,11 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
>  	tgt->oport = ini->eport;
>  
>  	if (inany_v4(&tgt->oaddr)) {
> -		tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
> +		const struct in_addr *guest_addr = fwd_guest_addr4(c);
> +
> +		if (!guest_addr)
> +			return PIF_NONE;
> +		tgt->eaddr = inany_from_v4(*guest_addr);
>  	} else {
>  		if (inany_is_linklocal6(&tgt->oaddr))
>  			tgt->eaddr.a6 = c->ip6.addr_ll_seen;
> diff --git a/inany.h b/inany.h
> index 07bfc3d..a334da9 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -300,6 +300,7 @@ int inany_prefix_pton(const char *src, union inany_addr *dst, int *prefix_len);
>  /* Flags for struct inany_addr_entry */
>  #define INANY_ADDR_CONFIGURED	(1 << 0)	/* User set via -a */
>  #define INANY_ADDR_HOST	(1 << 1)	/* From host interface */
> +#define INANY_ADDR_OBSERVED	(1 << 2)	/* Seen in guest traffic */
>  
>  /**
>   * struct inany_addr_entry - Unified IPv4/IPv6 address entry
> diff --git a/migrate.c b/migrate.c
> index 48d63a0..01be6f1 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -57,8 +57,16 @@ static int seen_addrs_source_v1(struct ctx *c,
>  	struct migrate_seen_addrs_v1 addrs = {
>  		.addr6 = c->ip6.addr_seen,
>  		.addr6_ll = c->ip6.addr_ll_seen,
> -		.addr4 = c->ip4.addr_seen,

This requires a new version of the migration data, and a specific
function to handle migration from v1 to v2. If it's trivial, migration
from v2 to v1 (keeping a single address) might also be convenient.

>  	};
> +	int i;
> +
> +	/* Find first observed IPv4 address */
> +	for (i = 0; i < c->ip4.addr_count; i++) {
> +		if (c->ip4.addrs[i].flags & INANY_ADDR_OBSERVED) {
> +			addrs.addr4 = *inany_v4(&c->ip4.addrs[i].addr);
> +			break;
> +		}
> +	}
>  
>  	(void)stage;
>  
> @@ -70,6 +78,33 @@ static int seen_addrs_source_v1(struct ctx *c,
>  	return 0;
>  }
>  
> +/**
> + * migrate_observed_addr4() - Add observed IPv4 address to ip4_ctx address array
> + * @c:		Execution context
> + * @addr:	IPv4 address to add
> + */
> +static void migrate_observed_addr4(struct ctx *c, const struct in_addr *addr)
> +{
> +	int i;
> +
> +	if (IN4_IS_ADDR_UNSPECIFIED(addr))
> +		return;
> +
> +	for (i = 0; i < c->ip4.addr_count; i++) {
> +		if (IN4_ARE_ADDR_EQUAL(addr, inany_v4(&c->ip4.addrs[i].addr))) {
> +			c->ip4.addrs[i].flags |= INANY_ADDR_OBSERVED;
> +			return;
> +		}
> +	}
> +
> +	if (c->ip4.addr_count < IP4_MAX_ADDRS) {
> +		c->ip4.addrs[c->ip4.addr_count].addr = inany_from_v4(*addr);
> +		c->ip4.addrs[c->ip4.addr_count].prefix_len = 0;
> +		c->ip4.addrs[c->ip4.addr_count].flags = INANY_ADDR_OBSERVED;
> +		c->ip4.addr_count++;
> +	}
> +}
> +
>  /**
>   * seen_addrs_target_v1() - Receive and use guest observed addresses on target
>   * @c:		Execution context
> @@ -82,6 +117,7 @@ static int seen_addrs_target_v1(struct ctx *c,
>  				const struct migrate_stage *stage, int fd)
>  {
>  	struct migrate_seen_addrs_v1 addrs;
> +	struct in_addr addr4;
>  
>  	(void)stage;
>  
> @@ -90,7 +126,11 @@ static int seen_addrs_target_v1(struct ctx *c,
>  
>  	c->ip6.addr_seen = addrs.addr6;
>  	c->ip6.addr_ll_seen = addrs.addr6_ll;
> -	c->ip4.addr_seen = addrs.addr4;
> +
> +	/* Avoid alignment warning */
> +	addr4 = addrs.addr4;
> +	migrate_observed_addr4(c, &addr4);
> +
>  	memcpy(c->guest_mac, addrs.mac, sizeof(c->guest_mac));
>  
>  	return 0;
> diff --git a/passt.h b/passt.h
> index 929b474..db09da5 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -68,7 +68,6 @@ enum passt_modes {
>   * struct ip4_ctx - IPv4 execution context
>   * @addrs:		IPv4 addresses assigned to guest
>   * @addr_count:	Number of addresses in addrs[] array
> - * @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
> @@ -87,7 +86,6 @@ struct ip4_ctx {
>  	struct inany_addr_entry addrs[IP4_MAX_ADDRS];
>  	int addr_count;
>  
> -	struct in_addr addr_seen;
>  	struct in_addr guest_gw;
>  	struct in_addr map_host_loopback;
>  	struct in_addr map_guest_addr;
> @@ -127,7 +125,6 @@ struct ip6_ctx {
>  	/* PIF_TAP addresses */
>  	struct inany_addr_entry addrs[IP6_MAX_ADDRS];
>  	int addr_count;
> -

Unrelated change.

>  	struct in6_addr addr_seen;
>  	struct in6_addr addr_ll_seen;
>  	struct in6_addr guest_gw;
> diff --git a/tap.c b/tap.c
> index 7c50013..1a52adb 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -161,6 +161,37 @@ void tap_send_single(const struct ctx *c, const void *data, size_t l2len)
>  	}
>  }
>  
> +/**
> + * tap_check_src_addr4() - Note an IPv4 address seen in guest traffic
> + * @c:		Execution context
> + * @addr:	IPv4 address seen as source from guest
> + *
> + * Add the address to ip4.addrs[] with OBSERVED flag if not already present.

Nit: we don't add . at the end of those descriptions (they're not
always full sentences).

> + */
> +static void tap_check_src_addr4(struct ctx *c, const struct in_addr *addr)
> +{
> +	int i;
> +
> +	/* Check if already in array */
> +	for (i = 0; i < c->ip4.addr_count; i++) {
> +		if (IN4_ARE_ADDR_EQUAL(addr, inany_v4(&c->ip4.addrs[i].addr))) {
> +			c->ip4.addrs[i].flags |= INANY_ADDR_OBSERVED;
> +			return;
> +		}
> +	}
> +
> +	/* Add new entry if space available */
> +	if (c->ip4.addr_count < IP4_MAX_ADDRS) {
> +		c->ip4.addrs[c->ip4.addr_count].addr = inany_from_v4(*addr);
> +		c->ip4.addrs[c->ip4.addr_count].prefix_len = 0;
> +		c->ip4.addrs[c->ip4.addr_count].flags = INANY_ADDR_OBSERVED;
> +		debug("added new IPv4 address at index %d", c->ip6.addr_count);
> +		c->ip4.addr_count++;
> +	} else {
> +		warn("IPv4 address table full, can't add new address");

This is a rather convenient vector for a denial of service: just have
passt logging to syslog (default) and then a guest cycling over a set
of addresses.

It should be debug() instead. And I guess we actually want to reuse the
least recently used (_OBSERVED) entry. Given that the array is short,
we could keep it ordered, too.

> +	}
> +}
> +
>  /**
>   * tap_ip6_daddr() - Normal IPv6 destination address for inbound packets
>   * @c:		Execution context
> @@ -771,8 +802,8 @@ resume:
>  			continue;
>  		}
>  
> -		if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
> -			c->ip4.addr_seen.s_addr = iph->saddr;
> +		if (iph->saddr)
> +			tap_check_src_addr4(c, (const struct in_addr *)&iph->saddr);
>  
>  		if (!iov_drop_header(&data, hlen))
>  			continue;
> @@ -950,9 +981,6 @@ resume:
>  			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen)) {
>  				c->ip6.addr_seen = *saddr;
>  			}
> -
> -			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addrs[0].addr.a6))
> -				c->ip6.addrs[0].addr.a6 = *saddr;
>  		} else if (!IN6_IS_ADDR_UNSPECIFIED(saddr)){
>  			c->ip6.addr_seen = *saddr;
>  		}

-- 
Stefano