Re: [PATCH v7 11/13] dhcp: Select address for DHCP distribution

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

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

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 <jmaloy@redhat.com>
> 
> ---
> 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

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