Re: [PATCH 2/3] dhcpv6: Add option type table and value parser

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

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

On Thu, Jun 04, 2026 at 04:21:49PM +0530, Anshu Kumari wrote:
> Add a type table mapping DHCPv6 option codes to value types,
> and a parser that encodes CLI strings into the correct binary wire
> format for each type.
> 
> Supported types: plain string, IPv6 address, IPv6 address list,
> 8/16/32-bit integers, vendor class (enterprise number + length-prefixed
> data, per RFC 8415 Section 21.16), status code (uint16 + message),
> and length-prefixed string lists.
> 
> Modify dhcpv6_add_option() to call the parser for validation and binary
> encoding at configuration time, storing both the raw string and the
> encoded value.
> 
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

Many comments below, but all of them minor.

> ---
>  dhcpv6.c | 228 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  passt.1  |  28 ++++++-
>  passt.h  |   4 +
>  3 files changed, 255 insertions(+), 5 deletions(-)
> 
> diff --git a/dhcpv6.c b/dhcpv6.c
> index a0fb77c..85e2926 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -25,6 +25,7 @@
>  #include <string.h>
>  #include <time.h>
>  #include <limits.h>
> +#include <errno.h>
>  
>  #include "packet.h"
>  #include "util.h"
> @@ -32,20 +33,233 @@
>  #include "tap.h"
>  #include "log.h"
>  
> +/**
> + * enum dhcpv6_opt_type - DHCPv6 option value types
> + * @DHCPV6_OPT_NONE:		Unsupported or unknown option
> + * @DHCPV6_OPT_STR:		Variable-length string
> + * @DHCPV6_OPT_IPV6:		Single IPv6 address
> + * @DHCPV6_OPT_IPV6_LIST:	Multiple IPv6 addresses, comma-separated
> + * @DHCPV6_OPT_INT8:		Unsigned 8-bit integer
> + * @DHCPV6_OPT_INT16:		Unsigned 16-bit integer
> + * @DHCPV6_OPT_INT32:		Unsigned 32-bit integer
> + * @DHCPV6_OPT_VENDOR_CLASS:	Enterprise number + length-prefixed data
> + * @DHCPV6_OPT_LEN_STR_LIST:	Length-prefixed string list
> + */
> +enum dhcpv6_opt_type {
> +	DHCPV6_OPT_NONE,
> +	DHCPV6_OPT_STR,
> +	DHCPV6_OPT_IPV6,
> +	DHCPV6_OPT_IPV6_LIST,
> +	DHCPV6_OPT_INT8,
> +	DHCPV6_OPT_INT16,
> +	DHCPV6_OPT_INT32,
> +	DHCPV6_OPT_VENDOR_CLASS,
> +	DHCPV6_OPT_LEN_STR_LIST,
> +};
> +
> +/**
> + * dhcpv6_opt_types - Maps DHCPv6 option code to value type, indexed by code
> + * RFC 8415 Options: 7, 15, 16, 17, 32, 82, 83
> + * RFC 5970 Options: 59, 60
> + * RFC 4075 Options: 31
> + */
> +static const enum dhcpv6_opt_type dhcpv6_opt_types[256] = {

Nit: AFAICT when you look up a value here, you already check against
the array size.  So, I think you could not specify the array size
explicitly here, and the compiler should infer it from initializer,
omitting all the empty space past option 83.

> +	[7]   = DHCPV6_OPT_INT8,		/* Preference */
> +	[15]  = DHCPV6_OPT_LEN_STR_LIST,	/* User Class */
> +	[16]  = DHCPV6_OPT_VENDOR_CLASS,	/* Vendor Class */
> +	[17]  = DHCPV6_OPT_VENDOR_CLASS,	/* Vendor Opts */
> +	[31]  = DHCPV6_OPT_IPV6_LIST,		/* SNTP Servers */
> +	[32]  = DHCPV6_OPT_INT32,		/* Information Refresh Time */
> +	[59]  = DHCPV6_OPT_STR,			/* Boot File URL */
> +	[60]  = DHCPV6_OPT_LEN_STR_LIST,	/* Boot File Params */
> +	[82]  = DHCPV6_OPT_INT32,		/* SOL_MAX_RT */
> +	[83]  = DHCPV6_OPT_INT32,		/* INF_MAX_RT */
> +};
> +
> +/**
> + * dhcpv6_opt_parse() - Parse a DHCPv6 option value string into binary
> + * @code:	DHCPv6 option code
> + * @str:	Value string from command line
> + * @buf:	Output buffer for binary value
> + * @buf_len:	Size of output buffer
> + *
> + * Return: number of bytes written to @buf, or -1 on error
> + */
> +static int dhcpv6_opt_parse(uint16_t code, const char *str,
> +			    uint8_t *buf, size_t buf_len)
> +{
> +	char chunk[INET6_ADDRSTRLEN];
> +	enum dhcpv6_opt_type type;
> +	unsigned long val;
> +	const char *colon;
> +	unsigned int i;
> +	uint8_t width;
> +	size_t slen;
> +	char *end;
> +	int len;
> +
> +	if (!*str)
> +		die("Empty value for DHCPv6 option %u", code);

Nit: I can imagine an option for which an empty string is a valid and
meaningful value, though I don't know if there exist any in practice.

> +
> +	if (code >= ARRAY_SIZE(dhcpv6_opt_types))
> +		die("Unsupported DHCPv6 option: %u,"
> +		    " see passt(1) for supported codes", code);
> +
> +	type = dhcpv6_opt_types[code];
> +
> +	switch (type) {
> +	case DHCPV6_OPT_NONE:
> +		die("Unsupported DHCPv6 option: %u,"
> +		    " see passt(1) for supported codes", code);
> +	case DHCPV6_OPT_IPV6:
> +	case DHCPV6_OPT_IPV6_LIST:
> +		len = 0;
> +
> +		while (*str) {

Nit: 'chunk' can be moved into this while block.

> +			slen = strcspn(str, ",");
> +			if (!slen || slen >= sizeof(chunk))

Nit: I don't think you need the explicit !slen check - inet_pton()
should fail on an empty string anyway.

> +				return -1;
> +
> +			if (len + (int)sizeof(struct in6_addr) > (int)buf_len)
> +				return -1;
> +
> +			memcpy(chunk, str, slen);
> +			chunk[slen] = '\0';
> +
> +			if (inet_pton(AF_INET6, chunk,
> +				      buf + len) != 1)
> +				return -1;
> +
> +			len += sizeof(struct in6_addr);
> +
> +			if (type == DHCPV6_OPT_IPV6) {
> +				if (str[slen] == ',')
> +					return -1;
> +				break;
> +			}
> +
> +			str += slen;
> +			if (*str == ',')
> +				str++;
> +		}
> +
> +		if (!len)
> +			return -1;
> +
> +		return len;
> +	case DHCPV6_OPT_INT8:
> +	case DHCPV6_OPT_INT16:
> +	case DHCPV6_OPT_INT32:
> +		if (type == DHCPV6_OPT_INT8)
> +			width = 1;
> +		else if (type == DHCPV6_OPT_INT16)
> +			width = 2;
> +		else
> +			width = 4;
> +
> +		errno = 0;
> +		val = strtoul(str, &end, 0);
> +
> +		if (*end || errno)
> +			return -1;
> +
> +		if (buf_len < width)
> +			return -1;
> +
> +		if (val >= (1ULL << (width * 8)))
> +			return -1;
> +
> +		for (i = width; i > 0; i--) {
> +			buf[i - 1] = val & 0xff;
> +			val >>= 8;
> +		}
> +
> +		return width;
> +	case DHCPV6_OPT_STR:
> +		slen = strlen(str);
> +
> +		if (!slen || slen >= buf_len)

Nit: Again, I'm not sure an empty string option is necessarily
impossible, and in any case you already checked for that case.

> +			return -1;
> +
> +		memcpy(buf, str, slen);
> +
> +		return slen;
> +	case DHCPV6_OPT_VENDOR_CLASS:
> +		colon = strchr(str, ':');
> +		if (!colon)
> +			return -1;
> +
> +		errno = 0;
> +		val = strtoul(str, &end, 0);
> +		if (end != colon || errno || val > UINT32_MAX)
> +			return -1;
> +
> +		slen = strlen(colon + 1);
> +		if (!slen)
> +			return -1;
> +
> +		len = sizeof(uint32_t) + sizeof(uint16_t) + slen;
> +		if ((size_t)len > buf_len)
> +			return -1;
> +
> +		uint32_t ent = htonl(val);

Nit: Although they are permitted in C11, by convention we avoid inline
declarations in passt.

> +
> +		memcpy(buf, &ent, sizeof(ent));
> +
> +		buf[4] = slen >> 8;
> +		buf[5] = slen & 0xff;
> +
> +		memcpy(buf + sizeof(uint32_t) + sizeof(uint16_t),
> +		       colon + 1, slen);

For this type of structured option value, it might be nicer to declare
a structure and cast the buf pointer into that.  This approach is also
fine, though.

> +
> +		return len;
> +	case DHCPV6_OPT_LEN_STR_LIST:
> +		len = 0;
> +
> +		while (*str) {
> +			slen = strcspn(str, ",");
> +			if (!slen)
> +				return -1;
> +
> +			if (len + (int)(sizeof(uint16_t) + slen) > (int)buf_len)
> +				return -1;
> +
> +			buf[len]     = slen >> 8;
> +			buf[len + 1] = slen & 0xff;
> +			len += sizeof(uint16_t);
> +
> +			memcpy(buf + len, str, slen);
> +			len += slen;
> +
> +			str += slen;
> +			if (*str == ',')
> +				str++;
> +		}
> +
> +		if (!len)
> +			return -1;
> +
> +		return len;
> +	}
> +
> +	return -1;
> +}
> +
>  /**
>   * dhcpv6_add_option() - Add or update a custom DHCPv6 option
>   * @c:		Execution context
>   * @code:	DHCPv6 option code
>   * @val_str:	Value string from command line
>   *
> - * Stores the option code and raw string value. Binary encoding and
> - * injection into DHCPv6 replies are handled by later patches.
> + * Parses @val_str according to the type registered for @code in
> + * dhcpv6_opt_types[]. If @code was already added, the previous value
> + * is overwritten.
>   *
>   * Return: 0 on success
>   */
>  int dhcpv6_add_option(struct ctx *c, uint16_t code, const char *val_str)
>  {
> -	int idx;
> +	int idx, ret;
>  
>  	for (idx = 0; idx < c->custom_v6opts_count; idx++) {
>  		if (c->custom_v6opts[idx].code == code)
> @@ -59,7 +273,15 @@ int dhcpv6_add_option(struct ctx *c, uint16_t code, const char *val_str)
>  		c->custom_v6opts_count++;
>  	}
>  
> +	ret = dhcpv6_opt_parse(code, val_str,
> +			       c->custom_v6opts[idx].val,
> +			       sizeof(c->custom_v6opts[0].val));

Unlike IPV4 DHCP, we don't appear to have an existing array of DHCP
option values in the DHCPv6 code we could reuse.  Nonetheless, it
seems like it would be nicer to have that table of values in something
local to the DHCPv6 code, rather than in the global context structure.

> +	if (ret < 0)
> +		die("Invalid value for DHCPv6 option %u: %s",
> +		    code, val_str);
> +
>  	c->custom_v6opts[idx].code = code;
> +	c->custom_v6opts[idx].len = ret;
>  
>  	if (snprintf_check(c->custom_v6opts[idx].str,
>  			   sizeof(c->custom_v6opts[0].str),
> diff --git a/passt.1 b/passt.1
> index 9c25214..3c4f3ac 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -433,8 +433,32 @@ Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39.
>  .TP
>  .BR \-\-dhcpv6-opt " " \fICODE\fR,\fIVALUE\fR
>  Set a DHCPv6 option by numeric code. The value format is determined
> -automatically from the option code. This option can be specified multiple
> -times. If the same option code is given more than once, the last value wins.
> +automatically from the option code. Multiple IPv6 addresses are
> +comma-separated. This option can be specified multiple times. If the same
> +option code is given more than once, the last value wins.
> +.RS
> +.TP
> +.B String options
> +59 (Boot File URL, RFC 5970)
> +.TP
> +.B Length-prefixed string list options (comma-separated entries)
> +15 (User Class, RFC 8415), 60 (Boot File Params, RFC 5970).
> +Each comma-separated entry is encoded with a 2-byte length prefix.
> +Example: \fB\-\-dhcpv6-opt 15,class1,class2\fR.
> +.TP
> +.B Vendor class options (ENTERPRISE:DATA format)
> +16 (Vendor Class, RFC 8415), 17 (Vendor-specific Info, RFC 8415).
> +VALUE is \fIENTERPRISE\fR:\fIDATA\fR where \fIENTERPRISE\fR is the IANA
> +Private Enterprise Number and \fIDATA\fR is the vendor class string.
> +Example: \fB\-\-dhcpv6-opt 16,0:HTTPClient\fR for UEFI HTTP Boot.
> +.TP
> +.B IPv6 address list options (comma-separated)
> +31 (SNTP Servers)
> +.TP
> +.B Integer options
> +7 (Preference, 8-bit), 32 (Information Refresh Time, 32-bit),
> +82 (SOL_MAX_RT, 32-bit), 83 (INF_MAX_RT, 32-bit)
> +.RE
>  
>  .TP
>  .BR \-t ", " \-\-tcp-ports " " \fIspec
> diff --git a/passt.h b/passt.h
> index 91509df..21f43d8 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -184,6 +184,8 @@ struct ip6_ctx {
>   * @fqdn:		Guest FQDN
>   * @custom_v6opts:	User-specified DHCPv6 options from --dhcpv6-opt
>   * @custom_v6opts.code:	DHCPv6 option code
> + * @custom_v6opts.len: Length of binary value in @val
> + * @custom_v6opts.val: Binary-encoded option value
>   * @custom_v6opts.str:	Original string value from command line
>   * @custom_v6opts_count:Number of entries in @custom_v6opts
>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
> @@ -271,6 +273,8 @@ struct ctx {
>  
>  	struct {
>  		uint16_t code;
> +		uint16_t len;
> +		uint8_t val[255];
>  		char str[256];
>  	} custom_v6opts[MAX_CUSTOM_DHCPV6_OPTS];
>  	int custom_v6opts_count;
> -- 
> 2.54.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 --]