Re: [PATCH v5 10/13] migrate: Update protocol to v3 for multi-address support

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

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

On Sun, Feb 22, 2026 at 12:44:42PM -0500, Jon Maloy wrote:
> We update the migration protocol to version 3 to support distributing
> multiple addresses from the unified address array. The new protocol
> migrates all address entries in the array, along with their prefix
> lengths and flags, and leaves it to the receiver to filter which
> ones he wants to apply.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v4: - Broke out as separate commit
>     - Made number of transferable addresses variable
> ---
>  migrate.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 123 insertions(+)
> 
> diff --git a/migrate.c b/migrate.c
> index 1990067..c2c2403 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -122,6 +122,108 @@ static int seen_addrs_target_v2(struct ctx *c,
>  	return 0;
>  }
>  
> +/**
> + * addrs_source_v3() - Send all addresses with flags from source
> + * @c:		Execution context
> + * @stage:	Migration stage, unused
> + * @fd:		File descriptor for state transfer
> + *
> + * Send all address entries with their flags. The receiver can filter
> + * based on flags as needed. This provides forward compatibility if
> + * future versions need different address types.
> + *
> + * Return: 0 on success, positive error code on failure
> + */
> +/* cppcheck-suppress [constParameterCallback, unmatchedSuppression] */
> +static int addrs_source_v3(struct ctx *c,
> +			   const struct migrate_stage *stage, int fd)
> +{
> +	uint8_t addr_count = c->addr_count;
> +
> +	(void)stage;
> +
> +	/* Send count, then all addresses with flags, then MAC */
> +	if (write_all_buf(fd, &addr_count, sizeof(addr_count)))
> +		return errno;
> +
> +	if (addr_count && write_all_buf(fd, c->addrs,
> +					addr_count * sizeof(c->addrs[0])))

Writing the raw internal structures is kind of risky - it means
migration will be broken by any change in the addrs entry format -
including any alterations to the flags bit assignments.  Mind you,
it's probably not the only place in the current migration format with
that problem.

> +		return errno;
> +
> +	if (write_all_buf(fd, c->guest_mac, ETH_ALEN))
> +		return errno;
> +
> +	return 0;
> +}
> +
> +/**
> + * migrate_merge_addr() - Merge migrated address into local array
> + * @c:		Execution context
> + * @addr:	Address entry from migration source
> + *
> + * If the address already exists locally, merge the flags (preserving
> + * local flags and adding migrated ones). Otherwise add as new entry.
> + */
> +static void migrate_merge_addr(struct ctx *c,
> +			       const struct inany_addr_entry *addr)
> +{
> +	struct inany_addr_entry *e;
> +
> +	if (inany_is_unspecified(&addr->addr))
> +		return;

That would be a bug on the migration origin side, wouldn't it?

> +
> +	/* Check if address already exists, merge flags */
> +	for_each_addr(e, c, 0) {
> +		if (inany_equals(&e->addr, &addr->addr)) {

Related to comments earlier in the series, should we be checking the
whole prefix for equality?

> +			e->flags |= addr->flags;
> +			return;
> +		}
> +	}
> +
> +	/* Add new entry if there's room */
> +	if (c->addr_count < INANY_MAX_ADDRS)
> +		c->addrs[c->addr_count++] = *addr;

We probably shouldn't continue silently if there isn't room - it's
quite likely the migrated guest won't work properly if that's the
case.

> +}
> +
> +/**
> + * addrs_target_v3() - Receive addresses on target
> + * @c:		Execution context
> + * @stage:	Migration stage, unused
> + * @fd:		File descriptor for state transfer
> + *
> + * Receive all address entries and merge only observed addresses into local
> + * array. Source sends all addresses for forward compatibility, but target
> + * only applies those marked as observed by guest traffic.
> + *
> + * Return: 0 on success, positive error code on failure
> + */
> +static int addrs_target_v3(struct ctx *c,
> +			   const struct migrate_stage *stage, int fd)
> +{
> +	struct inany_addr_entry addrs[INANY_MAX_ADDRS];
> +	uint8_t addr_count, i;
> +
> +	(void)stage;
> +
> +	if (read_all_buf(fd, &addr_count, sizeof(addr_count)))
> +		return errno;
> +
> +	if (addr_count > INANY_MAX_ADDRS)
> +		addr_count = INANY_MAX_ADDRS;
> +
> +	if (addr_count && read_all_buf(fd, addrs, addr_count * sizeof(addrs[0])))
> +		return errno;

Reading and processing the addresses from the stream one by one would
be slightly more elegant, and importantly it decouples the max number
of addresses at the origin from that at the target.

> +
> +	if (read_all_buf(fd, c->guest_mac, ETH_ALEN))
> +		return errno;
> +
> +	for (i = 0; i < addr_count; i++)
> +		if (addrs[i].flags & CONF_ADDR_OBSERVED)
> +			migrate_merge_addr(c, &addrs[i]);

Can you re-use your addr_set function from earlier in the series here?
It seems like the logic is pretty similar.

> +
> +	return 0;
> +}
> +
>  /* Stages for version 2 */
>  static const struct migrate_stage stages_v2[] = {
>  	{
> @@ -142,8 +244,29 @@ static const struct migrate_stage stages_v2[] = {
>  	{ 0 },
>  };
>  
> +/* Stages for version 3 (multiple observed IPv4 addresses) */
> +static const struct migrate_stage stages_v3[] = {
> +	{
> +		.name = "addresses",
> +		.source = addrs_source_v3,
> +		.target = addrs_target_v3,
> +	},
> +	{
> +		.name = "prepare flows",
> +		.source = flow_migrate_source_pre,
> +		.target = NULL,
> +	},
> +	{
> +		.name = "transfer flows",
> +		.source = flow_migrate_source,
> +		.target = flow_migrate_target,
> +	},
> +	{ 0 },
> +};
> +
>  /* Supported encoding versions, from latest (most preferred) to oldest */
>  static const struct migrate_version versions[] = {
> +	{ 3,	stages_v3, },
>  	{ 2,	stages_v2, },
>  	/* v1 was released, but not widely used.  It had bad endianness for the
>  	 * MSS and omitted timestamps, which meant it usually wouldn't work.
> -- 
> 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 --]