public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: Jon Maloy <jmaloy@redhat.com>
Cc: david@gibson.dropbear.id.au, passt-dev@passt.top
Subject: Re: [PASST v10] passt, pasta: Introduce unified multi-address data structures
Date: Thu, 22 Oct 2026 00:33:50 +0200 (CEST)	[thread overview]
Message-ID: <20261022003349.4dfcb6f1@elisabeth> (raw)
In-Reply-To: <20261015015747.2453213-1-jmaloy@redhat.com>

Sorry for the delay. It mostly looks good to me with a few caveats.

First off, Coverity Scan complains quite a bit now (but I haven't looked
into it):

---
/home/sbrivio/passt/conf.c:444:3:
  Type: Dereference null return value (NULL_RETURNS)

/home/sbrivio/passt/conf.c:438:2: Call to null-returning function
  1. path: Condition "!a", taking false branch.
/home/sbrivio/passt/conf.c:444:3:
  2. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times).
/home/sbrivio/passt/inany.h:102:7: Call to null-returning function
  2.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  2.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  2.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch.
/home/sbrivio/passt/inany.h:102:2:
  2.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch.
/home/sbrivio/passt/inany.h:103:3:
  2.5. return_null: Explicitly returning null.
/home/sbrivio/passt/conf.c:444:3:
  3. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL".
/home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null
  4. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)".
/home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null
  5. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)".
/home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null
  6. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)".
/home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null
  7. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)".
/home/sbrivio/passt/pif.c:44:2:
  8. example_checked: Example 4 (cont.): "v4" has its value checked in "v4".
/home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null
  9. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)".

/home/sbrivio/passt/arp.c:60:2:
  Type: Dereference null return value (NULL_RETURNS)

/home/sbrivio/passt/arp.c:46:2: Call to null-returning function
  1. path: Condition "ah->ar_hrd != __bswap_16(1)", taking false branch.
/home/sbrivio/passt/arp.c:46:2:
  2. path: Condition "ah->ar_pro != __bswap_16(2048)", taking false branch.
/home/sbrivio/passt/arp.c:46:2:
  3. path: Condition "ah->ar_hln != 6", taking false branch.
/home/sbrivio/passt/arp.c:46:2:
  4. path: Condition "ah->ar_pln != 4", taking false branch.
/home/sbrivio/passt/arp.c:46:2:
  5. path: Condition "ah->ar_op != __bswap_16(1)", taking false branch.
/home/sbrivio/passt/arp.c:54:2:
  6. path: Condition "memcmp(am->sip, &inany_any4.v4mapped.a4, 4UL /* sizeof (am->sip) */)", taking true branch.
/home/sbrivio/passt/arp.c:54:2:
  7. path: Condition "!memcmp(am->sip, am->tip, 4UL /* sizeof (am->sip) */)", taking false branch.
/home/sbrivio/passt/arp.c:60:2:
  8. path: Condition "a", taking true branch.
/home/sbrivio/passt/arp.c:60:2:
  9. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times).
/home/sbrivio/passt/inany.h:102:7: Call to null-returning function
  9.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  9.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  9.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch.
/home/sbrivio/passt/inany.h:102:2:
  9.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch.
/home/sbrivio/passt/inany.h:103:3:
  9.5. return_null: Explicitly returning null.
/home/sbrivio/passt/arp.c:60:2:
  10. dereference: Passing null pointer "inany_v4(&a->addr)" to "memcmp", which dereferences it.
/home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null
  11. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)".
/home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null
  12. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)".
/home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null
  13. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)".
/home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null
  14. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)".
/home/sbrivio/passt/pif.c:44:2:
  15. example_checked: Example 4 (cont.): "v4" has its value checked in "v4".
/home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null
  16. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)".

/home/sbrivio/passt/fwd.c:876:2:
  Type: Dereference null return value (NULL_RETURNS)

/home/sbrivio/passt/fwd.c:861:2: Call to null-returning function
  1. path: Condition "(__bswap_32(((struct in_addr *)addr)->s_addr) >> 24) == 127", taking false branch.
/home/sbrivio/passt/fwd.c:869:2:
  2. path: Condition "((struct in_addr *)addr)->s_addr == (0U /* (((((in_addr_t)0 & 0xff000000U) >> 24) | (((in_addr_t)0 & 0xff0000U) >> 8)) | (((in_addr_t)0 & 0xff00U) << 8)) | (((in_addr_t)0 & 0xffU) << 24) */)", taking false branch.
/home/sbrivio/passt/fwd.c:876:2:
  3. path: Condition "a", taking true branch.
/home/sbrivio/passt/fwd.c:876:2:
  4. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times).
/home/sbrivio/passt/inany.h:102:7: Call to null-returning function
  4.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  4.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  4.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch.
/home/sbrivio/passt/inany.h:102:2:
  4.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch.
/home/sbrivio/passt/inany.h:103:3:
  4.5. return_null: Explicitly returning null.
/home/sbrivio/passt/fwd.c:876:2:
  5. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL".
/home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null
  6. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)".
/home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null
  7. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)".
/home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null
  8. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)".
/home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null
  9. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)".
/home/sbrivio/passt/pif.c:44:2:
  10. example_checked: Example 4 (cont.): "v4" has its value checked in "v4".
/home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null
  11. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)".

/home/sbrivio/passt/arp.c:156:2:
  Type: Dereference null return value (NULL_RETURNS)

/home/sbrivio/passt/arp.c:137:2: Call to null-returning function
  1. path: Condition "!a", taking false branch.
/home/sbrivio/passt/arp.c:156:2:
  2. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times).
/home/sbrivio/passt/inany.h:102:7: Call to null-returning function
  2.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  2.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  2.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch.
/home/sbrivio/passt/inany.h:102:2:
  2.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch.
/home/sbrivio/passt/inany.h:103:3:
  2.5. return_null: Explicitly returning null.
/home/sbrivio/passt/arp.c:156:2:
  3. dereference: Passing null pointer "inany_v4(&a->addr)" to "memcpy", which dereferences it. [Note: The source code implementation of the function has been overridden by a builtin model.]
/home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null
  4. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)".
/home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null
  5. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)".
/home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null
  6. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)".
/home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null
  7. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)".
/home/sbrivio/passt/pif.c:44:2:
  8. example_checked: Example 4 (cont.): "v4" has its value checked in "v4".
/home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null
  9. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)".

/home/sbrivio/passt/conf.c:423:2:
  Type: Dereference null return value (NULL_RETURNS)

/home/sbrivio/passt/conf.c:385:2: Call to null-returning function
  1. path: Condition "!ifi", taking false branch.
/home/sbrivio/passt/conf.c:388:2:
  2. path: Condition "!ifi", taking false branch.
/home/sbrivio/passt/conf.c:393:2:
  3. path: Condition "((struct in_addr *)&ip4->guest_gw)->s_addr == (0U /* (((((in_addr_t)0 & 0xff000000U) >> 24) | (((in_addr_t)0 & 0xff0000U) >> 8)) | (((in_addr_t)0 & 0xff00U) << 8)) | (((in_addr_t)0 & 0xffU) << 24) */)", taking false branch.
/home/sbrivio/passt/conf.c:403:2:
  4. path: Condition "!a", taking false branch.
/home/sbrivio/passt/conf.c:422:2:
  5. path: Condition "a", taking true branch.
/home/sbrivio/passt/conf.c:423:2:
  6. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times).
/home/sbrivio/passt/inany.h:102:7: Call to null-returning function
  6.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  6.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch.
/home/sbrivio/passt/inany.h:102:7:
  6.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch.
/home/sbrivio/passt/inany.h:102:2:
  6.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch.
/home/sbrivio/passt/inany.h:103:3:
  6.5. return_null: Explicitly returning null.
/home/sbrivio/passt/conf.c:423:2:
  7. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL".
/home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null
  8. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)".
/home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null
  9. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)".
/home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null
  10. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)".
/home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null
  11. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)".
/home/sbrivio/passt/pif.c:44:2:
  12. example_checked: Example 4 (cont.): "v4" has its value checked in "v4".
/home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null
  13. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)".

---

and a few comments inline:

On Wed, 14 Oct 2026 21:57:47 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> As preparation for supporting multiple addresses per interface,
> we replace the single addr/prefix_len fields with an array. The
> array consists of a new struct inany_addr_entry containing an
> address and prefix length, both in inany_addr format.
> 
> Despite some code refactoring, there are only two real functional
> changes:
> - The indicated IPv6 prefix length is now properly stored, instead
>   of being ignored and overridden with the hardcoded value 64, as
>   has been the case until now.
> - Since even IPv4 addresses now are stored in IPv6 format, we
>   also store the corresponding prefix length in that format,
>   i.e. using the range [96,128] instead of [0,32].
> 
> In conf_ip6(), the explicit IN6_IS_ADDR_UNSPECIFIED() check on the
> address at the end of the function is no longer needed: for
> host-discovered addresses, it is now checked inside the if (!a) block
> before calling fwd_set_addr(); for user-provided addresses (via -a),
> validation already rejects unspecified addresses at parse time.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v2: -Using inany_addr instead of protocol specific addresses as
>      entry address field.
> 
> v3: -Merging into one array, directly in struct ctx
>     -Changed prefix_len and flags fields in struct inany_addr_entry
>      to uint8_t, since that makes the struct directly migratable
> 
> v4: -Updated according to changes in previous commits
>     -Updated according to feedback from David G.
>     -Squashed IP4_MASK macro commit into this one
> 
> v6: -Renamed and moved some definitions
>     -Introduced fwd_set_addr() and fwd_get_addr() already in this commit
>     -Eliminated first_v4/v6() functions, replaced with fwd_get_addr()
>     -Some other changes as suggested by David G.
>     -I kept the flag CONF_ADDR_LINKLOCAL, since it will be
>      needed later in an address selection function.
> 
> v7: -Introduced CONF_ADDR_GENERATED flag
>     -Other fixes based on feedback from David and Stefano.
>     -I changed signature of inany_prefix_len(), but I did not change
>      its semantics, since the premise of David's comment is wrong: the
>      caller does *not* explicitly know he is dealing with an IPv4 address.
>      In fact, there are examples later in this series where it may be an
>      IPv6 address, and the caller just trusts he gets the return value in
>      the appropriate format.
>     -Intoduced the inverse of inany_prefix_len(), called inany_prefix_len6()
>      which always returns the prefix in IPv6 or mapped IPv4 format.
>      The name of the function isn't great, but any alternative I came up
>      with became too long to be practical.
> 
> v8: -Added inany_prefix_v4() (David G.) to extract IPv4 address and
>      prefix length simultaneously, replacing inany_prefix_len().
>     -Removed both inany_prefix_len() and inany_prefix_len6()
>      auto-detecting functions. Prefix length attached to an inany is
>      now always stored in IPv6 format (96-128 for IPv4-mapped).
>     -fwd_set_addr() now requires prefix_len in IPv6 format; callers
>      updated to pass prefix_len + 96 for IPv4 addresses.
>     -Fixed -n option to store prefix_len in IPv6 format.
>     -Fixed addr_seen regression: when address was set via -a,
>      addr_seen was not initialised in conf_ip4().
>     -Replaced assert() with graceful no-op in dhcp.c and ndp.c when
>      no address is configured (David G.).
>     -Note: addr_seen is still initialised both at conf_ip*() time
>      and at DHCP/DHCPv6 send time (David's concern). This
>      inconsistency will be addressed in a follow-up.
>     -Introduced a new CONF_ADDR_ANY macro tobe used for address
>      selection, as suggested by Stefano.
>     -Deferred introduction of an explicit scope field (replacing
>      CONF_ADDR_LINKLOCAL flag) to a later commit or series.
> 
> v9: -Rebased on changes made by David Gibson.
>     -Some minor changes based on feedback from David.
>     -Defined a new macro CONF_ADDR_GLOBAL to supplant CONF_ADDR_ANY.
>     -I did not follow up David's suggestion to introduce three
>      functions to replace the current fwd_get_addr(), because I
>      don't think it would solve any problems. The 'flags' field
>      encode both provenance and type properties orthogonally, and
>      callers need different combinations of them. That is the very
>      point of that field, and I see nothing confusing in it.
>      It is correct that some of the calls to fwd_get_addr() go away
>      in later commits, but enough of them remain to justify its
>      existence. Defining three new functions to replace it would only
>      make them wrappers around fwd_get_addr(), and serve no purpose in
>      my view.
> 
> v10: -Redefined and refactored function fwd_get_addr().
> ---
>  arp.c    |  12 ++++-
>  conf.c   | 152 ++++++++++++++++++++++++++++++++-----------------------
>  dhcp.c   |  20 ++++++--
>  dhcpv6.c |  15 ++++--
>  fwd.c    | 111 +++++++++++++++++++++++++++++++++-------
>  fwd.h    |   4 ++
>  inany.h  |  42 +++++++++++++++
>  ip.h     |   2 +
>  ndp.c    |  19 +++++--
>  passt.h  |  60 +++++++++++++++++++---
>  pasta.c  |  23 ++++++---
>  tap.c    |   8 ++-
>  12 files changed, 358 insertions(+), 110 deletions(-)
> 
> diff --git a/arp.c b/arp.c
> index bb042e95..9ee6ae65 100644
> --- a/arp.c
> +++ b/arp.c
> @@ -41,6 +41,8 @@
>  static bool ignore_arp(const struct ctx *c,
>  		       const struct arphdr *ah, const struct arpmsg *am)
>  {
> +	const struct guest_addr *a;
> +
>  	if (ah->ar_hrd != htons(ARPHRD_ETHER)	||
>  	    ah->ar_pro != htons(ETH_P_IP)	||
>  	    ah->ar_hln != ETH_ALEN		||
> @@ -54,7 +56,8 @@ static bool ignore_arp(const struct ctx *c,
>  		return true;
>  
>  	/* Don't resolve the guest's assigned address, either. */
> -	if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip)))
> +	a = fwd_get_addr(c, AF_INET, true, 0);
> +	if (a && !memcmp(am->tip, inany_v4(&a->addr), sizeof(am->tip)))
>  		return true;
>  
>  	return false;
> @@ -123,12 +126,17 @@ int arp(const struct ctx *c, struct iov_tail *data)
>   */
>  void arp_send_init_req(const struct ctx *c)
>  {
> +	const struct guest_addr *a;
>  	struct {
>  		struct ethhdr eh;
>  		struct arphdr ah;
>  		struct arpmsg am;
>  	} __attribute__((__packed__)) req;
>  
> +	a = fwd_get_addr(c, AF_INET, true, 0);
> +	if (!a)
> +		return;
> +
>  	/* Ethernet header */
>  	req.eh.h_proto = htons(ETH_P_ARP);
>  	memcpy(req.eh.h_dest, MAC_BROADCAST, sizeof(req.eh.h_dest));
> @@ -145,7 +153,7 @@ void arp_send_init_req(const struct ctx *c)
>  	memcpy(req.am.sha,	c->our_tap_mac,		sizeof(req.am.sha));
>  	memcpy(req.am.sip,	&c->ip4.our_tap_addr,	sizeof(req.am.sip));
>  	memcpy(req.am.tha,	MAC_BROADCAST,		sizeof(req.am.tha));
> -	memcpy(req.am.tip,	&c->ip4.addr,		sizeof(req.am.tip));
> +	memcpy(req.am.tip,	inany_v4(&a->addr),	sizeof(req.am.tip));
>  
>  	debug("Sending initial ARP request for guest MAC address");
>  	tap_send_single(c, &req, sizeof(req));
> diff --git a/conf.c b/conf.c
> index 149a4c79..6fbdc86d 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -372,13 +372,16 @@ static uint8_t conf_ip4_prefix(const char *arg)
>  
>  /**
>   * conf_ip4() - Verify or detect IPv4 support, get relevant addresses
> + * @c:		Execution context
>   * @ifi:	Host interface to attempt (0 to determine one)
> - * @ip4:	IPv4 context (will be written)
>   *
>   * Return: interface index for IPv4, or 0 on failure.
>   */
> -static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
> +static unsigned int conf_ip4(struct ctx *c, unsigned int ifi)
>  {
> +	const struct guest_addr *a = fwd_get_addr(c, AF_INET, true, 0);
> +	struct ip4_ctx *ip4 = &c->ip4;
> +
>  	if (!ifi)
>  		ifi = nl_get_ext_if(nl_sock, AF_INET);
>  
> @@ -397,67 +400,69 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4)
>  		}
>  	}
>  
> -	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) {
> +	if (!a) {
> +		struct in_addr addr;
> +		int prefix_len;
>  		int rc = nl_addr_get(nl_sock, ifi, AF_INET,
> -				     &ip4->addr, &ip4->prefix_len, NULL);
> +				     &addr, &prefix_len, NULL);
>  		if (rc < 0) {
>  			debug("Couldn't discover IPv4 address: %s",
>  			      strerror_(-rc));
>  			return 0;
>  		}
> +		if (IN4_IS_ADDR_UNSPECIFIED(&addr)) {
> +			warn("Host returned address 0.0.0.0");
> +			return 0;
> +		}

I don't understand exactly if this is the part breaking local mode
but... something does. Before this patch:

$ pasta -- pasta --config-net true || echo oops
No interfaces with usable IPv4 routes
No interfaces with usable IPv6 routes
IPv4: no external interface as template, use local mode
IPv6: no external interface as template, use local mode

With this patch:

$ ./pasta -- ./pasta --config-net true || echo oops
No interfaces with usable IPv4 routes
No interfaces with usable IPv6 routes
IPv4: no external interface as template, use local mode
IPv6: no external interface as template, use local mode
Couldn't set IPv4 route(s) in guest: Network is unreachable
oops

In general we shouldn't fail if there's no address configured.

> +		fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST,
> +			     prefix_len + 96);
>  	}
>  
> -	if (!ip4->prefix_len) {
> -		in_addr_t addr = ntohl(ip4->addr.s_addr);
> -		if (IN_CLASSA(addr))
> -			ip4->prefix_len = (32 - IN_CLASSA_NSHIFT);
> -		else if (IN_CLASSB(addr))
> -			ip4->prefix_len = (32 - IN_CLASSB_NSHIFT);
> -		else if (IN_CLASSC(addr))
> -			ip4->prefix_len = (32 - IN_CLASSC_NSHIFT);
> -		else
> -			ip4->prefix_len = 32;
> -	}

If you get rid of this fallback calculation, my understanding is that
any IPv4 address configured via -a without a specific prefix length is
a /32, correct? That would be an unintended functional change.

> -
> -	ip4->addr_seen = ip4->addr;
> -
> +	a = fwd_get_addr(c, AF_INET, true, 0);
> +	assert(a);
> +	ip4->addr_seen = *inany_v4(&a->addr);
>  	ip4->our_tap_addr = ip4->guest_gw;
>  
> -	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr))
> -		return 0;
> -
>  	return ifi;
>  }
>  
>  /**
>   * conf_ip4_local() - Configure IPv4 addresses and attributes for local mode
> - * @ip4:	IPv4 context (will be written)
> + * @c:		Execution context (will be written)
>   */
> -static void conf_ip4_local(struct ip4_ctx *ip4)
> +static void conf_ip4_local(struct ctx *c)
>  {
> -	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr))
> -		ip4->addr = IP4_LL_GUEST_ADDR;
> -	ip4->addr_seen = ip4->addr;
> +	const struct guest_addr *a = fwd_get_addr(c, AF_INET, true, 0);
> +	struct ip4_ctx *ip4 = &c->ip4;
> +
> +	if (!a) {
> +		fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR),
> +			     CONF_ADDR_GENERATED,
> +			     IP4_LL_PREFIX_LEN + 96);
> +		ip4->addr_seen = IP4_LL_GUEST_ADDR;
> +	} else {
> +		ip4->addr_seen = *inany_v4(&a->addr);
> +	}
>  
>  	if (IN4_IS_ADDR_UNSPECIFIED(&ip4->guest_gw))
>  		ip4->guest_gw = IP4_LL_GUEST_GW;
>  	ip4->our_tap_addr = ip4->guest_gw;
>  
> -	if (!ip4->prefix_len)
> -		ip4->prefix_len = IP4_LL_PREFIX_LEN;
> -
>  	ip4->no_copy_addrs = ip4->no_copy_routes = true;
>  }
>  
>  /**
>   * conf_ip6() - Verify or detect IPv6 support, get relevant addresses
> + * @c:		Execution context
>   * @ifi:	Host interface to attempt (0 to determine one)
> - * @ip6:	IPv6 context (will be written)
>   *
>   * Return: interface index for IPv6, or 0 on failure.
>   */
> -static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
> +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;
>  
> @@ -478,23 +483,30 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
>  		}
>  	}
>  
> -	rc = nl_addr_get(nl_sock, ifi, AF_INET6,
> -			 IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL,
> +	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;
>  	}
>  
> -	ip6->addr_seen = ip6->addr;
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +	if (!a) {
> +		if (IN6_IS_ADDR_UNSPECIFIED(&addr))
> +			return 0;
> +
> +		fwd_set_addr(c, &addr, CONF_ADDR_HOST, prefix_len);
> +		ip6->addr_seen = addr.a6;
> +	} else {
> +		ip6->addr_seen = a->addr.a6;
> +	}
>  
>  	if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw))
>  		ip6->our_tap_ll = ip6->guest_gw;
>  	else
>  		ip6->our_tap_addr = ip6->guest_gw;
>  
> -	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ||
> -	    IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll))
> +	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll))
>  		return 0;
>  
>  	return ifi;
> @@ -502,10 +514,12 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6)
>  
>  /**
>   * conf_ip6_local() - Configure IPv6 addresses and attributes for local mode
> - * @ip6:	IPv6 context (will be written)
> + * @c:		Execution context (will be written)
>   */
> -static void conf_ip6_local(struct ip6_ctx *ip6)
> +static void conf_ip6_local(struct ctx *c)
>  {
> +	struct ip6_ctx *ip6 = &c->ip6;
> +
>  	if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw))
>  		ip6->guest_gw = IP6_LL_GUEST_GW;
>  
> @@ -801,6 +815,7 @@ enum passt_modes conf_mode(int argc, char *argv[])
>  static void conf_print(const struct ctx *c)
>  {
>  	char buf[INANY_ADDRSTRLEN];
> +	const struct guest_addr *a;
>  	int i;
>  
>  	if (c->fd_control_listen >= 0)
> @@ -848,16 +863,19 @@ static void conf_print(const struct ctx *c)
>  			     inet_ntop(AF_INET, &c->ip4.map_host_loopback,
>  				       buf, sizeof(buf)));
>  
> -		if (!c->no_dhcp) {
> +		a = fwd_get_addr(c, AF_INET, true, 0);
> +		if (a && !c->no_dhcp) {
>  			uint32_t mask;
> +			int plen = 0;
>  
> -			mask = htonl(0xffffffff << (32 - c->ip4.prefix_len));
> +			inany_prefix_v4(&a->addr, a->prefix_len, &plen);
> +			mask = IN4_MASK(plen);
>  
>  			info("DHCP:");
>  			info("    assign: %s",
> -			     inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf)));
> +			     inany_ntop(&a->addr, buf, sizeof(buf)));
>  			info("    mask: %s",
> -			     inet_ntop(AF_INET, &mask,        buf, sizeof(buf)));
> +			     inet_ntop(AF_INET, &mask, buf, sizeof(buf)));
>  			info("    router: %s",
>  			     inet_ntop(AF_INET, &c->ip4.guest_gw,
>  				       buf, sizeof(buf)));
> @@ -868,8 +886,8 @@ static void conf_print(const struct ctx *c)
>  				break;
>  			if (!i)
>  				info("DNS:");
> -			inet_ntop(AF_INET, &c->ip4.dns[i], buf, sizeof(buf));
> -			info("    %s", buf);
> +			info("    %s", inet_ntop(AF_INET, &c->ip4.dns[i],
> +						 buf, sizeof(buf)));

Unrelated change?

>  		}
>  
>  		for (i = 0; *c->dns_search[i].n; i++) {
> @@ -894,13 +912,14 @@ static void conf_print(const struct ctx *c)
>  		else
>  			goto dns6;
>  
> -		info("    assign: %s",
> -		     inet_ntop(AF_INET6, &c->ip6.addr, buf, sizeof(buf)));
> +		a = fwd_get_addr(c, AF_INET6, true, 0);
> +		if (a)
> +			info("    assign: %s",
> +			     inany_ntop(&a->addr, buf, sizeof(buf)));
>  		info("    router: %s",
>  		     inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf)));
>  		info("    our link-local: %s",
> -		     inet_ntop(AF_INET6, &c->ip6.our_tap_ll,
> -			       buf, sizeof(buf)));
> +		     inet_ntop(AF_INET6, &c->ip6.our_tap_ll, buf, sizeof(buf)));
>  
>  dns6:
>  		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> @@ -908,8 +927,10 @@ dns6:
>  			    break;
>  			if (!i)
>  				info("DNS:");
> -			inet_ntop(AF_INET6, &c->ip6.dns[i], buf, sizeof(buf));
> -			info("    %s", buf);
> +			info("    %s",
> +			     inet_ntop(AF_INET6, &c->ip6.dns[i],
> +				       buf, sizeof(buf)));

Same here.

> +
>  		}
>  
>  		for (i = 0; *c->dns_search[i].n; i++) {
> @@ -1213,15 +1234,13 @@ static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n)
>  	    inany_is_loopback(&addr))
>  		goto bad;
>  
> +	fwd_set_addr(c, &addr, CONF_ADDR_USER, prefix_len);
>  	if (a4) {
> -		c->ip4.addr = *a4;
> -		c->ip4.prefix_len = prefix_len - 96;
> -		c->ip4.addr_fixed = true;
>  		c->ip4.no_copy_addrs = true;
> +		c->ip4.addr_fixed = true;
>  	} else {
> -		c->ip6.addr = addr.a6;
> -		c->ip6.addr_fixed = true;
>  		c->ip6.no_copy_addrs = true;
> +		c->ip6.addr_fixed = true;
>  	}
>  
>  	return is_prefix;
> @@ -1648,13 +1667,20 @@ void conf(struct ctx *c, int argc, char **argv)
>  		case 'a':
>  			opt_a_is_prefix = conf_addr(c, optarg, opt_n);
>  			break;
> -		case 'n':
> +		case 'n': {

See
https://archives.passt.top/passt-dev/20261008000617.0f8f22a9@elisabeth/
for some thoughts on this.

In general I think we should avoid introducing extra blocks unless
strictly necessary (there are a few cases where we need that).

Otherwise we should do it consistently... and that would look pretty
bad, I reckon.

> +			struct guest_addr *a;
> +
>  			if (opt_a_is_prefix)
>  				die("Redundant prefix length specification");
>  
> -			c->ip4.prefix_len = conf_ip4_prefix(optarg);
> -			opt_n = c->ip4.prefix_len + 96;
> +			opt_n = conf_ip4_prefix(optarg) + 96;
> +
> +			for_each_addr(a, c->addrs, c->addr_count, AF_INET) {
> +				a->prefix_len = opt_n;
> +				break;
> +			}
>  			break;
> +		}
>  		case 'M':
>  			p = optarg;
>  			if (!parse_mac(&p, c->our_tap_mac) ||
> @@ -1853,9 +1879,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  
>  	nl_sock_init(c, false);
>  	if (!v6_only && !c->splice_only)
> -		c->ifi4 = conf_ip4(ifi4, &c->ip4);
> +		c->ifi4 = conf_ip4(c, ifi4);
>  	if (!v4_only && !c->splice_only)
> -		c->ifi6 = conf_ip6(ifi6, &c->ip6);
> +		c->ifi6 = conf_ip6(c, ifi6);
>  
>  	if (c->ifi4 && c->mtu < IPV4_MIN_MTU) {
>  		warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)",
> @@ -1878,7 +1904,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  	if (!c->ifi4 && !v6_only) {
>  		if (!c->splice_only) {
>  			info("IPv4: no external interface as template, use local mode");
> -			conf_ip4_local(&c->ip4);
> +			conf_ip4_local(c);
>  		}
>  		c->ifi4 = -1;
>  	}
> @@ -1886,7 +1912,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  	if (!c->ifi6 && !v4_only) {
>  		if (!c->splice_only) {
>  			info("IPv6: no external interface as template, use local mode");
> -			conf_ip6_local(&c->ip6);
> +			conf_ip6_local(c);
>  		}
>  		c->ifi6 = -1;
>  	}
> @@ -1959,7 +1985,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  	if (!c->ifi6) {
>  		c->no_ndp = 1;
>  		c->no_dhcpv6 = 1;
> -	} else if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) {
> +	} else if (!fwd_get_addr(c, AF_INET6, true, 0)) {
>  		c->no_dhcpv6 = 1;
>  	}
>  
> diff --git a/dhcp.c b/dhcp.c
> index c3c74224..17eebc4a 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -303,7 +303,9 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
>  int dhcp(const struct ctx *c, struct iov_tail *data)
>  {
>  	char macstr[ETH_ADDRSTRLEN];
> +	const struct guest_addr *a;
>  	size_t mlen, dlen, opt_len;
> +	const struct in_addr *addr;
>  	struct in_addr mask, dst;
>  	struct ethhdr eh_storage;
>  	struct iphdr iph_storage;
> @@ -315,6 +317,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	struct msg const *m;
>  	struct msg reply;
>  	unsigned int i;
> +	int plen;
>  
>  	eh = IOV_REMOVE_HEADER(data, eh_storage);
>  	iph = IOV_PEEK_HEADER(data, iph_storage);
> @@ -344,6 +347,15 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	    m->op != BOOTREQUEST)
>  		return -1;
>  
> +	a = fwd_get_addr(c, AF_INET, true, CONF_ADDR_USER);
> +	if (!a)
> +		a = fwd_get_addr(c, AF_INET, true, 0);
> +	if (!a)
> +		return -1;
> +
> +	addr = inany_prefix_v4(&a->addr, a->prefix_len, &plen);
> +	assert(addr);
> +
>  	reply.op		= BOOTREPLY;
>  	reply.htype		= m->htype;
>  	reply.hlen		= m->hlen;
> @@ -352,7 +364,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	reply.secs		= 0;
>  	reply.flags		= m->flags;
>  	reply.ciaddr		= m->ciaddr;
> -	reply.yiaddr		= c->ip4.addr;
> +	reply.yiaddr		= *addr;
>  	reply.siaddr		= 0;
>  	reply.giaddr		= m->giaddr;
>  	memcpy(&reply.chaddr,	m->chaddr,	sizeof(reply.chaddr));
> @@ -408,7 +420,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  
>  	info("    from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr)));
>  
> -	mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len));
> +	mask.s_addr = IN4_MASK(plen);
>  	memcpy(opts[1].s,  &mask,                sizeof(mask));
>  	memcpy(opts[3].s,  &c->ip4.guest_gw,     sizeof(c->ip4.guest_gw));
>  	memcpy(opts[54].s, &c->ip4.our_tap_addr, sizeof(c->ip4.our_tap_addr));
> @@ -416,7 +428,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	/* If the gateway is not on the assigned subnet, send an option 121
>  	 * (Classless Static Routing) adding a dummy route to it.
>  	 */
> -	if ((c->ip4.addr.s_addr & mask.s_addr)
> +	if ((addr->s_addr & mask.s_addr)
>  	    != (c->ip4.guest_gw.s_addr & mask.s_addr)) {
>  		/* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */
>  		opts[121].slen = 14;
> @@ -475,7 +487,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	if (m->flags & FLAG_BROADCAST)
>  		dst = in4addr_broadcast;
>  	else
> -		dst = c->ip4.addr;
> +		dst = *addr;
>  
>  	tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, &reply, dlen);
>  
> diff --git a/dhcpv6.c b/dhcpv6.c
> index 29c7e320..5fbba7cf 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -318,7 +318,7 @@ static bool dhcpv6_opt(struct iov_tail *data, uint16_t type)
>   *         false otherwise and @data is unmodified
>   */
>  static bool dhcpv6_ia_notonlink(struct iov_tail *data,
> -				struct in6_addr *la)
> +				const struct in6_addr *la)
>  {
>  	int ia_types[2] = { OPT_IA_NA, OPT_IA_TA };
>  	struct opt_ia_addr opt_addr_storage;
> @@ -567,6 +567,7 @@ 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;
>  	struct msg_hdr mh_storage;
>  	const struct msg_hdr *mh;
> @@ -574,6 +575,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  	const struct udphdr *uh;
>  	size_t mlen, n;
>  
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +
>  	uh = IOV_REMOVE_HEADER(data, uh_storage);
>  	if (!uh)
>  		return -1;
> @@ -627,7 +630,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  		if (mh->type == TYPE_CONFIRM && server_id)
>  			return -1;
>  
> -		if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) {
> +		if (a && dhcpv6_ia_notonlink(data, &a->addr.a6)) {
>  
>  			dhcpv6_send_ia_notonlink(c, saddr, data,
>  						 &client_id_base,
> @@ -682,7 +685,8 @@ 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);
> -	c->ip6.addr_seen = c->ip6.addr;
> +	if (a)
> +		c->ip6.addr_seen = a->addr.a6;
>  
>  	return 1;
>  }
> @@ -694,6 +698,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  void dhcpv6_init(const struct ctx *c)
>  {
>  	time_t y2k = 946684800; /* Epoch to 2000-01-01T00:00:00Z, no mktime() */
> +	const struct guest_addr *a;
>  	uint32_t duid_time;
>  
>  	duid_time = htonl(difftime(time(NULL), y2k));
> @@ -706,5 +711,7 @@ void dhcpv6_init(const struct ctx *c)
>  	memcpy(resp_not_on_link.server_id.duid_lladdr,
>  	       c->our_tap_mac, sizeof(c->our_tap_mac));
>  
> -	resp.ia_addr.addr	= c->ip6.addr;
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +	if (a)
> +		resp.ia_addr.addr = a->addr.a6;
>  }
> diff --git a/fwd.c b/fwd.c
> index 5679a3d6..574db1b3 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -243,6 +243,61 @@ void fwd_neigh_table_init(const struct ctx *c)
>  		fwd_neigh_table_update(c, &mga, c->our_tap_mac, true);
>  }
>  
> +/**
> + * fwd_set_addr() - Add or update an address in the unified address array
> + * @c:		Execution context
> + * @addr:	Address to add (IPv4-mapped or IPv6)
> + * @flags:	CONF_ADDR_* flags for this address
> + * @prefix_len:	Prefix length in IPv6 format (96-128 for IPv4-mapped)
> + *
> + * 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)
> +{
> +	struct guest_addr *a;
> +
> +	for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) {

No need for curly brackets.

> +		goto found;
> +	}
> +
> +	if (c->addr_count >= MAX_GUEST_ADDRS)
> +		return;
> +
> +	a = &c->addrs[c->addr_count++];
> +
> +found:
> +	a->addr = *addr;
> +	a->prefix_len = prefix_len;
> +	a->flags = flags;
> +}
> +
> +/**
> + * fwd_get_addr() - Get guest address entry matching criteria
> + * @c:		Execution context
> + * @af:		Address family (AF_INET, AF_INET6, or AF_UNSPEC for any)
> + * @global:	If true, match global only; if false, match link-local only
> + * @flags:	Required provenance flags (any-match), 0 for any provenance
> + *
> + * Return: first address entry matching criteria, or NULL
> + */
> +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af,
> +				      bool global, uint8_t flags)

I think I suggested this in an early revision but I didn't get a reply:
wouldn't it be convenient to have '0' defined as some kind of
CONF_ADDR_ANY value?

Callers mostly pass '0' there, and having a specific define for that
would make things clearer.

> +{
> +	const struct guest_addr *a;
> +
> +	for_each_addr(a, c->addrs, c->addr_count, af) {
> +		if (inany_is_linklocal(&a->addr) == global)
> +			continue;
> +		if (flags && !(a->flags & flags))
> +			continue;
> +		return a;
> +	}
> +
> +	return NULL;
> +}
> +
>  /* Forwarding table storage, generally accessed via pointers in struct ctx */
>  static struct fwd_table fwd_in;
>  static struct fwd_table fwd_out;
> @@ -799,8 +854,10 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini)
>   *         translation, false otherwise
>   */
>  static bool fwd_guest_accessible4(const struct ctx *c,
> -				    const struct in_addr *addr)
> +				  const struct in_addr *addr)
>  {
> +	const struct guest_addr *a;
> +
>  	if (IN4_IS_ADDR_LOOPBACK(addr))
>  		return false;
>  
> @@ -815,7 +872,8 @@ static bool fwd_guest_accessible4(const struct ctx *c,
>  	/* For IPv4, addr_seen is initialised to addr, so is always a valid
>  	 * address
>  	 */
> -	if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr) ||
> +	a = fwd_get_addr(c, AF_INET, true, 0);
> +	if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) ||
>  	    IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen))
>  		return false;
>  
> @@ -833,10 +891,13 @@ 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 guest_addr *a;
> +
>  	if (IN6_IS_ADDR_LOOPBACK(addr))
>  		return false;
>  
> -	if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr))
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +	if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6))
>  		return false;
>  
>  	/* For IPv6, addr_seen starts unspecified, because we don't know what LL
> @@ -881,16 +942,21 @@ static bool fwd_guest_accessible(const struct ctx *c,
>  static void nat_outbound(const struct ctx *c, const union inany_addr *addr,
>  			 union inany_addr *translated)
>  {
> -	if (inany_equals4(addr, &c->ip4.map_host_loopback))
> +	const struct guest_addr *ga;
> +
> +	if (inany_equals4(addr, &c->ip4.map_host_loopback)) {
>  		*translated = inany_loopback4;
> -	else if (inany_equals6(addr, &c->ip6.map_host_loopback))
> +	} else if (inany_equals6(addr, &c->ip6.map_host_loopback)) {
>  		*translated = inany_loopback6;
> -	else if (inany_equals4(addr, &c->ip4.map_guest_addr))
> -		*translated = inany_from_v4(c->ip4.addr);
> -	else if (inany_equals6(addr, &c->ip6.map_guest_addr))
> -		translated->a6 = c->ip6.addr;
> -	else
> +	} else if (inany_equals4(addr, &c->ip4.map_guest_addr)) {
> +		ga = fwd_get_addr(c, AF_INET, true, 0);
> +		*translated = ga ? ga->addr : inany_any4;
> +	} else if (inany_equals6(addr, &c->ip6.map_guest_addr)) {
> +		ga = fwd_get_addr(c, AF_INET6, true, 0);
> +		translated->a6 = ga ? ga->addr.a6 : in6addr_any;
> +	} else {
>  		*translated = *addr;
> +	}
>  }
>  
>  /**
> @@ -995,16 +1061,23 @@ bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
>  	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback) &&
>  		   inany_equals6(addr, &in6addr_loopback)) {
>  		translated->a6 = c->ip6.map_host_loopback;
> -	} else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) &&
> -		   inany_equals4(addr, &c->ip4.addr)) {
> -		*translated = inany_from_v4(c->ip4.map_guest_addr);
> -	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) &&
> -		   inany_equals6(addr, &c->ip6.addr)) {
> -		translated->a6 = c->ip6.map_guest_addr;
> -	} else if (fwd_guest_accessible(c, addr)) {
> -		*translated = *addr;
>  	} else {
> -		return false;
> +		const struct guest_addr *ga4, *ga6;
> +
> +		ga4 = fwd_get_addr(c, AF_INET, true, 0);
> +		ga6 = fwd_get_addr(c, AF_INET6, true, 0);
> +
> +		if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) &&
> +		    ga4 && inany_equals(addr, &ga4->addr)) {
> +			*translated = inany_from_v4(c->ip4.map_guest_addr);
> +		} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) &&
> +			   ga6 && inany_equals(addr, &ga6->addr)) {
> +			translated->a6 = c->ip6.map_guest_addr;
> +		} else if (fwd_guest_accessible(c, addr)) {
> +			*translated = *addr;
> +		} else {
> +			return false;
> +		}
>  	}
>  
>  	return true;
> diff --git a/fwd.h b/fwd.h
> index b60697d9..d001d8ae 100644
> --- a/fwd.h
> +++ b/fwd.h
> @@ -79,5 +79,9 @@ void fwd_neigh_table_free(const struct ctx *c,
>  void fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr,
>  		       uint8_t *mac);
>  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);
> +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af,
> +				      bool global, uint8_t flags);
>  
>  #endif /* FWD_H */
> diff --git a/inany.h b/inany.h
> index d0befb15..4f2331ee 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -104,6 +104,34 @@ static inline struct in_addr *inany_v4(const union inany_addr *addr)
>  	return (struct in_addr *)&addr->v4mapped.a4;
>  }
>  
> +/** inany_prefix_v4() - Extract IPv4 address and prefix length from inany
> + * @addr:	IPv4 (as mapped) or IPv6 address
> + * @prefix_len:	Prefix length in IPv6 format (96-128 for IPv4)
> + * @v4_plen:	Set to IPv4 prefix length (0-32) on success
> + *
> + * Return: IPv4 address if @addr is IPv4, NULL otherwise
> + */
> +static inline struct in_addr *inany_prefix_v4(const union inany_addr *addr,
> +					      int prefix_len, int *v4_plen)
> +{
> +	struct in_addr *v4 = inany_v4(addr);
> +
> +	if (!v4)
> +		return NULL;

We usually try to make early returns and returns in general a bit more
apparent, that is, we would usually have an extra newline here...

> +	*v4_plen = prefix_len - 96;

...and here too (but this isn't done consistently).

> +	return v4;
> +}
> +
> +/** inany_af - Get address family of IPv[46] address

inany_af() (it's a function)

> + * @addr:	IPv4 or IPv6 address
> + *
> + * Return: AF_INET for IPv4, AF_INET6 for IPv6
> + */
> +static inline sa_family_t inany_af(const union inany_addr *addr)
> +{
> +	return inany_v4(addr) ? AF_INET : AF_INET6;
> +}
> +
>  /** inany_default_prefix_len() - Get default prefix length for address
>   * @addr:	IPv4 or iPv6 address
>   *
> @@ -230,6 +258,20 @@ static inline bool inany_is_linklocal6(const union inany_addr *a)
>  	return IN6_IS_ADDR_LINKLOCAL(&a->a6);
>  }
>  
> +/** inany_is_linklocal() - Check if address is link-local
> + * @a:		IPv[46] address
> + *
> + * Return: true if @a is in 169.254.0.0/16 or fe80::/10
> + */
> +static inline bool inany_is_linklocal(const union inany_addr *a)
> +{
> +	const struct in_addr *v4 = inany_v4(a);
> +
> +	if (v4)
> +		return IN4_IS_ADDR_LINKLOCAL(v4);

Same here, extra newline for consistency.

> +	return IN6_IS_ADDR_LINKLOCAL(&a->a6);
> +}
> +
>  /** inany_is_multicast() - Check if address is multicast or broadcast
>   * @a:		IPv[46] address
>   *
> diff --git a/ip.h b/ip.h
> index aab9b86a..ac93c543 100644
> --- a/ip.h
> +++ b/ip.h
> @@ -19,6 +19,8 @@
>  	(ntohl(((struct in_addr *)(a))->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET)
>  #define IN4_IS_ADDR_MULTICAST(a) \
>  	(IN_MULTICAST(ntohl(((struct in_addr *)(a))->s_addr)))
> +#define IN4_MASK(prefix) \
> +	((prefix) <= 0 ? 0 : htonl(0xffffffff << (32 - (prefix))))
>  #define IN4_ARE_ADDR_EQUAL(a, b) \
>  	(((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr)
>  #define IN4ADDR_LOOPBACK_INIT \
> diff --git a/ndp.c b/ndp.c
> index 1f2bcb0c..ac1325e1 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -257,7 +257,6 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>  			.valid_lifetime		= ~0U,
>  			.pref_lifetime		= ~0U,
>  		},
> -		.prefix = c->ip6.addr,
>  		.source_ll = {
>  			.header = {
>  				.type		= OPT_SRC_L2_ADDR,
> @@ -265,8 +264,15 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>  			},
>  		},
>  	};
> +	const struct guest_addr *a;
>  	unsigned char *ptr = NULL;
>  
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +	if (!a)
> +		return;
> +
> +	ra.prefix = a->addr.a6;
> +
>  	ptr = &ra.var[0];
>  
>  	if (c->mtu) {
> @@ -460,6 +466,7 @@ first:
>   */
>  void ndp_send_init_req(const struct ctx *c)
>  {
> +	const struct guest_addr *a;
>  	struct ndp_ns ns = {
>  		.ih = {
>  			.icmp6_type		= NS,
> @@ -468,8 +475,14 @@ void ndp_send_init_req(const struct ctx *c)
>  			.icmp6_solicited	= 0, /* Reserved */
>  			.icmp6_override		= 0, /* Reserved */
>  		},
> -		.target_addr = c->ip6.addr
> +		.target_addr = IN6ADDR_ANY_INIT
>  	};
> +
> +	a = fwd_get_addr(c, AF_INET6, true, 0);
> +	if (!a)
> +		return;
> +
> +	ns.target_addr = a->addr.a6;
>  	debug("Sending initial NDP NS request for guest MAC address");
> -	ndp_send(c, &c->ip6.addr, &ns, sizeof(ns));
> +	ndp_send(c, &a->addr.a6, &ns, sizeof(ns));
>  }
> diff --git a/passt.h b/passt.h
> index afd8e9f0..47e25d5c 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -64,11 +64,27 @@ enum passt_modes {
>  	MODE_VU,
>  };
>  
> +/* Maximum number of addresses in context address array */
> +#define MAX_GUEST_ADDRS		32
> +
> +/**
> + * struct guest_addr - Unified IPv4/IPv6 address entry
> + * @addr:	IPv4 (as mapped) or IPv6 address
> + * @prefix_len:	Prefix length in IPv6/IPv4-mapped [0,128]/[96,128] format
> + * @flags:	CONF_ADDR_* flags
> + */
> +struct guest_addr {
> +	union inany_addr addr;
> +	uint8_t prefix_len;
> +	uint8_t flags;
> +#define CONF_ADDR_USER		BIT(0)		/* User set via -a */
> +#define CONF_ADDR_HOST		BIT(1)		/* From host interface */
> +#define CONF_ADDR_GENERATED	BIT(2)		/* Generated by PASST/PASTA */
> +};
> +
>  /**
>   * struct ip4_ctx - IPv4 execution context
> - * @addr:		IPv4 address assigned to guest
>   * @addr_seen:		Latest IPv4 address seen as source from tap
> - * @prefixlen:		IPv4 prefix length (netmask)
>   * @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,9 +103,7 @@ enum passt_modes {
>   */
>  struct ip4_ctx {
>  	/* PIF_TAP addresses */
> -	struct in_addr addr;
>  	struct in_addr addr_seen;
> -	int prefix_len;
>  	struct in_addr guest_gw;
>  	struct in_addr map_host_loopback;
>  	struct in_addr map_guest_addr;
> @@ -110,7 +124,6 @@ struct ip4_ctx {
>  
>  /**
>   * struct ip6_ctx - IPv6 execution context
> - * @addr:		IPv6 address assigned to guest
>   * @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
> @@ -132,7 +145,6 @@ struct ip4_ctx {
>   */
>  struct ip6_ctx {
>  	/* PIF_TAP addresses */
> -	struct in6_addr addr;
>  	struct in6_addr addr_seen;
>  	struct in6_addr addr_ll_seen;
>  	struct in6_addr guest_gw;
> @@ -192,6 +204,8 @@ struct ip6_ctx {
>   * @fqdn:		Guest FQDN
>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
>   * @ip6:		IPv6 configuration
> + * @addrs:		Unified address array for both IPv4 (mapped) and IPv6
> + * @addr_count:		Number of active entries in @addrs array
>   * @pasta_ifn:		Name of namespace interface for pasta
>   * @pasta_ifi:		Index of namespace interface for pasta
>   * @pasta_conf_ns:	Configure namespace after creating it
> @@ -276,6 +290,9 @@ struct ctx {
>  	int ifi6;
>  	struct ip6_ctx ip6;
>  
> +	struct guest_addr addrs[MAX_GUEST_ADDRS];
> +	int addr_count;
> +
>  	char pasta_ifn[IF_NAMESIZE];
>  	unsigned int pasta_ifi;
>  	int pasta_conf_ns;
> @@ -321,6 +338,37 @@ struct ctx {
>  
>  extern struct ctx passt_ctx;
>  
> +/**
> + * next_addr_idx_() - Find next address index matching family filter
> + * @addrs:	Array of guest addresses
> + * @count:	Number of addresses in array
> + * @i:		Starting index
> + * @af:		Address family filter: AF_INET, AF_INET6, or AF_UNSPEC for all
> + *
> + * Return: next matching index, or count if none found
> + */
> +static inline int next_addr_idx_(const struct guest_addr *addrs, int count,
> +				 int i, sa_family_t af)
> +{
> +	for (; i < count; i++) {
> +		if (af == AF_UNSPEC || af == inany_af(&addrs[i].addr))
> +			return i;
> +	}
> +	return i;
> +}
> +
> +/**
> + * for_each_addr() - Iterate over addresses in array
> + * @a:		Pointer variable for current entry (struct guest_addr *)
> + * @addrs:	Array of guest addresses (e.g., c->addrs)
> + * @count:	Number of addresses (e.g., c->addr_count)
> + * @af:		Address family filter: AF_INET, AF_INET6, or 0 for all
> + */
> +#define for_each_addr(a, addrs, count, af)				\
> +	for (int i_ = next_addr_idx_((addrs), (count), 0, (af));	\
> +	     i_ < (count) && ((a) = &(addrs)[i_], true);		\
> +	     i_ = next_addr_idx_((addrs), (count), i_ + 1, (af)))

If I understand correctly, next_addr_idx_() is a trick to avoid an
explicit if clause with a 'continue' statement here. I think it would
be more readable like that (assuming it's doable), though, and we
consistently do that in iterator macros.

> +
>  void proto_update_l2_buf(const unsigned char *eth_d);
>  
>  #endif /* PASST_H */
> diff --git a/pasta.c b/pasta.c
> index 10f3ec8d..24427ddb 100644
> --- a/pasta.c
> +++ b/pasta.c
> @@ -299,23 +299,32 @@ void pasta_start_ns(struct ctx *c, int argc, char *argv[])
>   *
>   * Return: 0 on success, negative error code on failure
>   */
> -static int pasta_conf_addrs(struct ctx *c, sa_family_t af, int ifi,
> +static int pasta_conf_addrs(const 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);
>  
> -	if (af == AF_INET && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr))
> +	a = fwd_get_addr(c, af, true, 0);
> +	if (!a)
> +		return 0;
> +
> +	if (af == AF_INET) {
> +		const struct in_addr *v4;
> +		int plen = 0;
> +
> +		v4 = inany_prefix_v4(&a->addr, a->prefix_len, &plen);
>  		return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET,
> -				   &c->ip4.addr, c->ip4.prefix_len);
> -	if (af == AF_INET6 && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr))
> -		return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6,
> -				   &c->ip6.addr, 64);
> +				   v4, plen);
> +	}
>  
> -	return 0;
> +	return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6,
> +			   &a->addr.a6, a->prefix_len);
>  }
>  
>  /**
> diff --git a/tap.c b/tap.c
> index dfa66c71..604b54ca 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1005,8 +1005,12 @@ resume:
>  				c->ip6.addr_seen = *saddr;
>  			}
>  
> -			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr))
> -				c->ip6.addr = *saddr;
> +			if (!fwd_get_addr(c, AF_INET6, true, 0) &&
> +			    !fwd_get_addr(c, AF_INET6, false, 0)) {
> +				union inany_addr addr = { .a6 = *saddr };
> +
> +				fwd_set_addr(c, &addr, CONF_ADDR_GENERATED, 64);
> +			}
>  		} else if (!c->ip6.addr_fixed &&
>  			   !IN6_IS_ADDR_UNSPECIFIED(saddr)) {
>  			c->ip6.addr_seen = *saddr;

-- 
Stefano


      reply	other threads:[~2026-10-21 22:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  1:57 Jon Maloy
2026-10-21 22:33 ` Stefano Brivio [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022003349.4dfcb6f1@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=jmaloy@redhat.com \
    --cc=passt-dev@passt.top \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).