Re: [PATCH v7 13/13] ndp: Support advertising multiple prefixes in Router Advertisements

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

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

On Sun, Apr 12, 2026 at 08:53:19PM -0400, Jon Maloy wrote:
> We extend NDP to advertise all suitable IPv6 prefixes in Router
> Advertisements, per RFC 4861. Observed and link-local addresses,
> plus addresses with a prefix length != 64, are excluded.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v6: -Adapted to previous changes in series
> 
> v7: -Adapted to previous changes in series
>     -Use struct initializer for source link-layer address option
>     -Other minor fixes based on feedback from Stefano
> ---
>  conf.c    |  19 ++++++---
>  fwd.c     |   4 ++
>  migrate.c |   5 +++
>  ndp.c     | 123 +++++++++++++++++++++++++++++++++++++-----------------
>  passt.h   |   3 +-
>  5 files changed, 108 insertions(+), 46 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 7c705de..97c66c9 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1216,7 +1216,7 @@ static void conf_print(const struct ctx *c)
>  	}
>  
>  	if (c->ifi6) {
> -		bool has_dhcpv6 = false;
> +		bool has_ndp = false, has_dhcpv6 = false;

Nit: has_slaac might be a better name, since NDP has a number of
functions.

>  		const char *head;
>  
>  		if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback))
> @@ -1225,18 +1225,25 @@ static void conf_print(const struct ctx *c)
>  				       buf, sizeof(buf)));
>  
>  		for_each_addr(a, c->addrs, c->addr_count, AF_INET6) {
> +			if (a->flags & CONF_ADDR_SLAAC)
> +				has_ndp = true;
>  			if (a->flags & CONF_ADDR_DHCPV6)
>  				has_dhcpv6 = true;
>  		}
>  
> -		if (c->no_ndp && !has_dhcpv6)
> +		if (!has_ndp && !has_dhcpv6)
>  			goto dns6;
>  
> -		a = fwd_get_addr(c, AF_INET6, 0, CONF_ADDR_LINKLOCAL);
> -		if (!c->no_ndp && a) {
> +		if (has_ndp) {
>  			info("NDP:");
> -			info("    assign: %s",
> -			     inany_ntop(&a->addr, buf, sizeof(buf)));
> +			head = "assign: ";
> +			for_each_addr(a, c->addrs, c->addr_count, AF_INET6) {

Nit: AF_INET6 is redundant with the CONF_ADDR_SLAAC check.

> +				if (!(a->flags & CONF_ADDR_SLAAC))
> +					continue;
> +				inany_ntop(&a->addr, buf, sizeof(buf));
> +				info("    %s: %s/%d", head, buf, a->prefix_len);
> +				head = "        ";
> +			}
>  		}
>  
>  		if (has_dhcpv6) {
> diff --git a/fwd.c b/fwd.c
> index 2b444fb..2bc8e33 100644
> --- a/fwd.c
> +++ b/fwd.c
> @@ -302,6 +302,10 @@ void fwd_set_addr(struct ctx *c, const union inany_addr *addr,
>  	} else if (!(flags & CONF_ADDR_LINKLOCAL)) {
>  		if (!c->no_dhcpv6)
>  			flags |= CONF_ADDR_DHCPV6;
> +
> +		/* NDP/RA only if prefix is /64 */
> +		if (!c->no_ndp && prefix_len == 64)
> +			flags |= CONF_ADDR_SLAAC;
>  	}
>  
>  	/* Add to head or tail, depending on flag */
> diff --git a/migrate.c b/migrate.c
> index adcbc63..f019924 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -54,6 +54,7 @@ struct migrate_seen_addrs_v2 {
>  #define MIGRATE_ADDR_OBSERVED	BIT(3)
>  #define MIGRATE_ADDR_DHCP	BIT(4)
>  #define MIGRATE_ADDR_DHCPV6	BIT(5)
> +#define MIGRATE_ADDR_SLAAC	BIT(6)

Same comments again about migration protocol.

>  
>  /**
>   * struct migrate_addr_v3 - Migration format for a single address entry
> @@ -89,6 +90,8 @@ static uint8_t flags_to_migration(uint8_t flags)
>  		migration |= MIGRATE_ADDR_DHCP;
>  	if (flags & CONF_ADDR_DHCPV6)
>  		migration |= MIGRATE_ADDR_DHCPV6;
> +	if (flags & CONF_ADDR_SLAAC)
> +		migration |= MIGRATE_ADDR_SLAAC;
>  
>  	return migration;
>  }
> @@ -115,6 +118,8 @@ static uint8_t flags_from_migration(uint8_t migration)
>  		flags |= CONF_ADDR_DHCP;
>  	if (migration & MIGRATE_ADDR_DHCPV6)
>  		flags |= CONF_ADDR_DHCPV6;
> +	if (migration & MIGRATE_ADDR_SLAAC)
> +		flags |= CONF_ADDR_SLAAC;
>  
>  	return flags;
>  }
> diff --git a/ndp.c b/ndp.c
> index 3750fc5..bb8374a 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -32,6 +32,8 @@
>  #include "passt.h"
>  #include "tap.h"
>  #include "log.h"
> +#include "fwd.h"
> +#include "conf.h"
>  
>  #define	RT_LIFETIME	65535
>  
> @@ -99,6 +101,16 @@ struct opt_prefix_info {
>  	uint32_t reserved;
>  } __attribute__((packed));
>  
> +/**
> + * struct ndp_prefix - Prefix Information option with prefix
> + * @info:		Prefix Information option header
> + * @prefix:		IPv6 prefix
> + */
> +struct ndp_prefix {
> +	struct opt_prefix_info info;
> +	struct in6_addr prefix;
> +} __attribute__((__packed__));
> +
>  /**
>   * struct opt_mtu - Maximum transmission unit (MTU) option
>   * @header:		Option header
> @@ -140,27 +152,23 @@ struct opt_dnssl {
>  } __attribute__((packed));
>  
>  /**
> - * struct ndp_ra - NDP Router Advertisement (RA) message
> + * struct ndp_ra_hdr - NDP Router Advertisement fixed header
>   * @ih:			ICMPv6 header
>   * @reachable:		Reachability time, after confirmation (ms)
>   * @retrans:		Time between retransmitted NS messages (ms)
> - * @prefix_info:	Prefix Information option
> - * @prefix:		IPv6 prefix
> - * @mtu:		MTU option
> - * @source_ll:		Target link-layer address
> - * @var:		Variable fields
>   */
> -struct ndp_ra {
> +struct ndp_ra_hdr {
>  	struct icmp6hdr ih;
>  	uint32_t reachable;
>  	uint32_t retrans;
> -	struct opt_prefix_info prefix_info;
> -	struct in6_addr prefix;
> -	struct opt_l2_addr source_ll;
> +} __attribute__((__packed__));
>  
> -	unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) +
> -			  sizeof(struct opt_dnssl)];
> -} __attribute__((packed, aligned(__alignof__(struct in6_addr))));
> +/* Maximum RA message size: hdr + prefixes + source_ll + mtu + rdnss + dnssl */
> +#define NDP_RA_MAX_SIZE	(sizeof(struct ndp_ra_hdr) + \
> +			 MAX_GUEST_ADDRS * sizeof(struct ndp_prefix) + \
> +			 sizeof(struct opt_l2_addr) + \
> +			 sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) + \
> +			 sizeof(struct opt_dnssl))
>  
>  /**
>   * struct ndp_ns - NDP Neighbor Solicitation (NS) message
> @@ -231,6 +239,42 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr)
>  		ndp_na(c, &in6addr_ll_all_nodes, addr);
>  }
>  
> +/**
> + * ndp_prefix_fill() - Fill prefix options for all suitable addresses
> + * @c:		Execution context
> + * @buf:	Buffer to write prefix options into
> + *
> + * Fills buffer with Prefix Information options for all non-linklocal,
> + * non-observed addresses with prefix_len == 64
> + *
> + * Return: number of bytes written
> + */
> +static size_t ndp_prefix_fill(const struct ctx *c, unsigned char *buf)
> +{
> +	const struct guest_addr *a;
> +	struct ndp_prefix *p;
> +	size_t offset = 0;
> +
> +	for_each_addr(a, c->addrs, c->addr_count, AF_INET6) {
> +		if (!(a->flags & CONF_ADDR_SLAAC))
> +			continue;
> +
> +		p = (struct ndp_prefix *)(buf + offset);
> +		p->info.header.type = OPT_PREFIX_INFO;
> +		p->info.header.len = 4;  /* 4 * 8 = 32 bytes */
> +		p->info.prefix_len = 64;
> +		p->info.prefix_flags = 0xc0;  /* L, A flags */
> +		p->info.valid_lifetime = ~0U;
> +		p->info.pref_lifetime = ~0U;
> +		p->info.reserved = 0;
> +		p->prefix = a->addr.a6;
> +
> +		offset += sizeof(struct ndp_prefix);
> +	}
> +
> +	return offset;
> +}
> +
>  /**
>   * ndp_ra() - Send an NDP Router Advertisement (RA) message
>   * @c:		Execution context
> @@ -238,7 +282,15 @@ void ndp_unsolicited_na(const struct ctx *c, const struct in6_addr *addr)
>   */
>  static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>  {
> -	struct ndp_ra ra = {
> +	unsigned char buf[NDP_RA_MAX_SIZE]
> +		__attribute__((__aligned__(__alignof__(struct in6_addr))));
> +	struct ndp_ra_hdr *hdr = (struct ndp_ra_hdr *)buf;
> +	struct opt_l2_addr *source_ll;
> +	unsigned char *ptr;
> +	size_t prefix_len;
> +
> +	/* Build RA header */
> +	*hdr = (struct ndp_ra_hdr){
>  		.ih = {
>  			.icmp6_type		= RA,
>  			.icmp6_code		= 0,
> @@ -247,31 +299,26 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>  			.icmp6_rt_lifetime	= htons_constant(RT_LIFETIME),
>  			.icmp6_addrconf_managed	= 1,
>  		},
> -		.prefix_info = {
> -			.header = {
> -				.type		= OPT_PREFIX_INFO,
> -				.len		= 4,
> -			},
> -			.prefix_len		= 64,
> -			.prefix_flags		= 0xc0,	/* prefix flags: L, A */
> -			.valid_lifetime		= ~0U,
> -			.pref_lifetime		= ~0U,
> -		},
> -		.source_ll = {
> -			.header = {
> -				.type		= OPT_SRC_L2_ADDR,
> -				.len		= 1,
> -			},
> -		},
>  	};
> -	const struct guest_addr *a = fwd_get_addr(c, AF_INET6, 0, 0);
> -	unsigned char *ptr = NULL;
>  
> -	ASSERT(a);
> -
> -	ra.prefix = a->addr.a6;
> +	/* Fill prefix options */
> +	prefix_len = ndp_prefix_fill(c, (unsigned char *)(hdr + 1));
> +	if (prefix_len == 0) {
> +		/* No suitable prefixes to advertise */
> +		return;

Do we want this?  I think it's still valid to present other options
via NDP, even if we have no prefixes to advertise.  And, I think we
want to do so for at least the MTU.

> +	}
>  
> -	ptr = &ra.var[0];
> +	/* Add source link-layer address option */
> +	ptr = (unsigned char *)(hdr + 1) + prefix_len;
> +	source_ll = (struct opt_l2_addr *)ptr;
> +	*source_ll = (struct opt_l2_addr) {
> +		.header = {
> +			.type	= OPT_SRC_L2_ADDR,
> +			.len	= 1,
> +		},
> +	};
> +	memcpy(source_ll->mac, c->our_tap_mac, ETH_ALEN);
> +	ptr += sizeof(struct opt_l2_addr);
>  
>  	if (c->mtu) {
>  		struct opt_mtu *mtu = (struct opt_mtu *)ptr;
> @@ -345,10 +392,8 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>  		}
>  	}
>  
> -	memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
> -
>  	/* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */
> -	ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);
> +	ndp_send(c, dst, buf, ptr - buf);
>  }
>  
>  /**
> diff --git a/passt.h b/passt.h
> index 028eb7c..2c633e4 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -84,7 +84,8 @@ struct guest_addr {
>  #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) */
> -#define CONF_ADDR_DHCPV6	BIT(6)		/* Advertise via DHCPv6 (IPv6) */
> +#define CONF_ADDR_DHCPV6	BIT(6)		/* Advertise via DHCPv6 */

This change belongs in the previous patch.

> +#define CONF_ADDR_SLAAC		BIT(7)		/* Advertise via NDP/RA (/64) */
>  };
>  
>  /**
> -- 
> 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 --]