Re: [PATCH v13] passt, pasta: Introduce unified multi-address data structures

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 27 Oct 2026 19:14:59 -0400
Jon Maloy <jmaloy@redhat.com> wrote:

> [...]
>
> +++ 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, CONF_ADDR_ANY);
> +
>  	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,
> @@ -667,6 +670,14 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>  	default:
>  		return -1;
>  	}
> +
> +	/* No address: INFORMATION_REQUEST can still be answered. For other

Nit: it's called Information-request in the RFC (our define can't be
called that way, but this is a comment about the message type).

> +	 * types RFC 8415 requires a NoAddrsAvail reply, but silently dropping

Nit: RFC 8415 was obsoleted by RFC 9915.

> +	 * is acceptable here as this is a transient state.
> +	 */
> +	if (!a && mh->type != TYPE_INFORMATION_REQUEST)
> +		return 1;

Partially pre-existing problem with Information-request: we're always
including the IA_NA option, but we shouldn't, see Appendix B. of RFC
9915:

   +=========+========+========+=====+=====+===+====+=====+=====+=====+
   |         | Client | Server |IA_NA|IA_PD|ORO|Pref|Elap.|Relay|Auth.|
   |         | ID     | ID     |     |     |   |    |Time |Msg. |     |
   +=========+========+========+=====+=====+===+====+=====+=====+=====+
   [...]
   +=========+--------+--------+-----+-----+---+----+-----+-----+-----+
   | Inform. |   *    | (see   |     |     | * |    |  *  |     |     |
   |         |        | note)  |     |     |   |    |     |     |     |
   +=========+--------+--------+-----+-----+---+----+-----+-----+-----+

...making things conceptually worse, until this patch, the address in
the IA_NA option was always considered valid.  Now we're sending an
option that shouldn't be there (pre-existing) with a previous address
that doesn't exist anymore (because it was set in resp.ia_na), if any,
and an all-zero address otherwise.

This new part of the old issue is just conceptual because we always have
an IPv6 address to offer if DHCPv6 is enabled, so in practice this
patch doesn't make things worse.  Except for those two nits (which I
can fix on merge) I wouldn't see a problem merging the patch like this.

If we want to solve that issue before proceeding, though:

- DHCPv6 doesn't have padding options, so we can't just zero it out. We
  would need to actually remove it

- to do so, instead of a pre-formed IA_NA option, we would need to do
  something similar to what dhcpv6_dns_fill(), appending it only as
  needed and increasing the size of the message ('n') as needed.

I haven't tried to do that.  It doesn't look exceedingly complicated,
but, if you think it is, then let's merge it this way for the moment
being. Or maybe skip the Information-request reply if (!a), which should
be slightly better than the alternative.  Let me know what you think.

-- 
Stefano