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

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

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

On Sat, Mar 21, 2026 at 08:43:31PM -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
> ---
>  conf.c    |  9 ++++-----
>  dhcp.c    | 21 +++++++++++++--------
>  dhcp.h    |  2 +-
>  fwd.c     |  9 +++++++++
>  migrate.c |  5 +++++
>  passt.h   |  1 +
>  6 files changed, 33 insertions(+), 14 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 320a9e4..512fa38 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"
> @@ -1182,11 +1183,9 @@ 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) {
> -			uint32_t mask;
> -
> -			mask = IN4_MASK(inany_prefix_len(a));
> +		a = fwd_get_addr(c, AF_INET, CONF_ADDR_DHCP, 0);
> +		if (a) {
> +			uint32_t mask = IN4_MASK(inany_prefix_len(a));
>  
>  			info("DHCP:");
>  			inany_ntop(&a->addr, buf, sizeof(buf));
> diff --git a/dhcp.c b/dhcp.c
> index 6d9def5..cdb3afb 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"
>  
>  /**
> @@ -300,23 +302,22 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
>   *
>   * Return: 0 if it's not a DHCP message, 1 if handled, -1 on failure
>   */
> -int dhcp(const struct ctx *c, struct iov_tail *data)
> +int dhcp(struct ctx *c, struct iov_tail *data)

I don't see why the const needs to be dropped.

>  {
> -	const struct guest_addr *a = fwd_get_addr(c, AF_INET, 0, 0);
> +	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 udphdr uh_storage;
>  	struct ethhdr eh_storage;
>  	struct iphdr iph_storage;
> -	struct udphdr uh_storage;

Re-ordering uh_storage seems an unrelated change.

> +	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;
> +	int i;

As does this.

>  
>  	eh = IOV_REMOVE_HEADER(data, eh_storage);
>  	iph = IOV_PEEK_HEADER(data, iph_storage);
> @@ -346,7 +347,11 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	    m->op != BOOTREQUEST)
>  		return -1;
>  
> -	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/dhcp.h b/dhcp.h
> index cd50c99..7326c7d 100644
> --- a/dhcp.h
> +++ b/dhcp.h
> @@ -6,7 +6,7 @@
>  #ifndef DHCP_H
>  #define DHCP_H
>  
> -int dhcp(const struct ctx *c, struct iov_tail *data);
> +int dhcp(struct ctx *c, struct iov_tail *data);
>  void dhcp_init(void);
>  
>  #endif /* DHCP_H */
> diff --git a/fwd.c b/fwd.c
> index b3f5dc0..e1c85dd 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -295,6 +295,15 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr,
>  		return;
>  	}
>  
> +	/* Determine advertisement eligibility */
> +	if ((flags & (CONF_ADDR_HOST | CONF_ADDR_USER)) &&
> +	    !(flags & CONF_ADDR_LINKLOCAL)) {
> +		if (inany_v4(addr)) {
> +			if (!c->no_dhcp)
> +				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 7b9a2f6..1d1e0e6 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 - Wire format for a single address entry
> @@ -83,6 +84,8 @@ static uint8_t flags_to_wire(uint8_t flags)
>  		wire |= MIGRATE_ADDR_LINKLOCAL;
>  	if (flags & CONF_ADDR_OBSERVED)
>  		wire |= MIGRATE_ADDR_OBSERVED;
> +	if (flags & CONF_ADDR_DHCP)
> +		wire |= MIGRATE_ADDR_DHCP;
>  
>  	return wire;
>  }
> @@ -105,6 +108,8 @@ static uint8_t flags_from_wire(uint8_t wire)
>  		flags |= CONF_ADDR_LINKLOCAL;
>  	if (wire & MIGRATE_ADDR_OBSERVED)
>  		flags |= CONF_ADDR_OBSERVED;
> +	if (wire & MIGRATE_ADDR_DHCP)
> +		flags |= CONF_ADDR_DHCP;
>  
>  	return flags;
>  }
> diff --git a/passt.h b/passt.h
> index db2f10d..5ea1715 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -75,6 +75,7 @@ enum passt_modes {
>  #define CONF_ADDR_HOST		BIT(1)		/* From host interface */
>  #define CONF_ADDR_LINKLOCAL	BIT(2)		/* Link-local address */
>  #define CONF_ADDR_OBSERVED	BIT(3)		/* Seen in guest traffic */
> +#define CONF_ADDR_DHCP		BIT(4)		/* Advertise via DHCP (IPv4) */
>  
>  /**
>   * struct guest_addr - Unified IPv4/IPv6 address entry
> -- 
> 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 --]