Re: [PATCH v3 1/2] dhcpv6: Add --dhcpv6-opt with option type table and value parser

[Laurent Vivier <lvivier@redhat.com>]

On 8/24/26 14:13, Anshu Kumari wrote:
> Introduce the --dhcpv6-opt flag that allows setting arbitrary DHCPv6
> options from command-line in the form [--dhcpv6-opt CODE,VALUE].
> 
> Add a type lookup table mapping option codes to value types (IPv6,
> IPv6 list, integer, string, vendor class, length-prefixed string
> list) and dhcpv6_opt_parse() to convert CLI strings to binary wire
> format. If the same option code is given more than once, the
> last value wins.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>    - Use parse_unsigned(), parse_literal(), parse_eoi() from parse.c
>      instead of manual strtoul()/errno/strchr() in dhcpv6_opt_parse()
>      for UINT and VENDOR_CLASS cases.
>    - Store options as pre-parsed binary in a static array indexed by
>      option code, instead of storing raw strings.
>    - Add dhcpv6_opt_to_str() to render binary option values back to
>      printable strings for startup logging in conf_print().
> 
> v2:
>    - Renamed custom_v6opts to dhcpv6_opts, MAX_CUSTOM_DHCPV6_OPTS
>      to MAX_DHCPV6_OPTS.
>    - Dropped val/len from ctx struct.
>    - Moved dhcpv6_add_option() to conf.c as static
>      conf_dhcpv6_option().
>    - Made dhcpv6_opt_parse() non-static, declared in dhcpv6.h
>    - Omitted explicit [256] from dhcpv6_opt_types[].
>    - Moved chunk declaration into while block.
>    - Removed redundant !slen check in DHCPV6_OPT_STR case.
>    - All errors in dhcpv6_opt_parse() return -1, removed die()
>      calls.
> ---
>   conf.c   |  25 ++++-
>   dhcpv6.c | 326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   dhcpv6.h |   2 +
>   passt.1  |  31 ++++++
>   4 files changed, 382 insertions(+), 2 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index faf2681..17c7c0c 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -47,6 +47,7 @@
>   #include "lineread.h"
>   #include "isolation.h"
>   #include "log.h"
> +#include "dhcpv6.h"
>   #include "vhost_user.h"
>   #include "epoll_ctl.h"
>   #include "conf.h"
> @@ -632,7 +633,8 @@ static void usage(const char *name, FILE *f, int status)
>   		"  -S, --search LIST	Space-separated list, search domains\n"
>   		"    a single, empty option disables the DNS search list\n"
>   		"  -H, --hostname NAME 	Hostname to configure client with\n"
> -		"  --fqdn NAME		FQDN to configure client with\n");
> +		"  --fqdn NAME		FQDN to configure client with\n"
> +		"  --dhcpv6-opt CODE,VAL	Set DHCPv6 option CODE to VAL\n");
>   	if (strstr(name, "pasta"))
>   		FPRINTF(f, "    default: don't use any search list\n");
>   	else
> @@ -900,6 +902,9 @@ static void conf_print(const struct ctx *c)
>   		info("    our link-local: %s",
>   		     inet_ntop(AF_INET6, &c->ip6.our_tap_ll,
>   			       buf, sizeof(buf)));
> +		for (i = 1; i < UINT16_MAX; i++)
> +			if (dhcpv6_opt_to_str(i, buf, sizeof(buf)))
> +				info("    option %u: %s", i, buf);
>   
>   dns6:
>   		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> @@ -1348,6 +1353,7 @@ void conf(struct ctx *c, int argc, char **argv)
>   		{"stats", required_argument,		NULL,		31 },
>   		{"conf-path",	required_argument,	NULL,		'c' },
>   		{"chroot-fallback", no_argument,	NULL, 		32 },
> +		{"dhcpv6-opt", required_argument,	NULL,		35 },
>   		{ 0 },
>   	};
>   	const char *optstring = "+dqfel:hs:c:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:";
> @@ -1589,6 +1595,23 @@ void conf(struct ctx *c, int argc, char **argv)
>   		case 32:
>   			c->chroot_fallback = true;
>   			break;
> +		case 35: {
> +			unsigned long dhcpv6_opt_code;
> +			p = optarg;
> +
> +			if (!parse_unsigned(&p, 0, &dhcpv6_opt_code) ||
> +			    !parse_literal(&p, ",")) {
> +				die("--dhcpv6-opt requires CODE,VALUE format");
> +			}
> +
> +			if (dhcpv6_opt_code < 1) {
> +				die("Invalid DHCPv6 option code: %s",
> +				    optarg);
> +			}

dhcpv6_opt_code is unsigned, so "dhcpv6_opt_code < 1" means "dhcpv6_opt_code == 0".
You should check also dhcpv6_opt_code < UINT16_MAX because dhcpv6_set_opt() takes an 
uint16_t for code, and it can be silently truncated:

if (dhcpv6_opt_code == 0 || dhcpv6_opt_code > UINT16_MAX) {
	die("Invalid DHCPv6 option code: %s", optarg);
}

> +
> +			dhcpv6_set_opt(dhcpv6_opt_code, p);
> +			break;
> +		}
>   		case 'd':
>   			c->debug = 1;
>   			c->quiet = 0;
> diff --git a/dhcpv6.c b/dhcpv6.c
> index 29c7e32..3964dd9 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -25,12 +25,15 @@
>   #include <string.h>
>   #include <time.h>
>   #include <limits.h>
> +#include <errno.h>
>   
>   #include "packet.h"
>   #include "util.h"
>   #include "passt.h"
>   #include "tap.h"
>   #include "log.h"
> +#include "inany.h"
> +#include "parse.h"
>   
>   /**
>    * struct opt_hdr - DHCPv6 option header
> @@ -278,6 +281,328 @@ static struct resp_not_on_link_t {
>   	{ 0, },
>   };
>   
> +/**
> + * 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_UINT8:		Unsigned 8-bit integer
> + * @DHCPV6_OPT_UINT16:		Unsigned 16-bit integer
> + * @DHCPV6_OPT_UINT32:		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_UINT8,
> +	DHCPV6_OPT_UINT16,
> +	DHCPV6_OPT_UINT32,
> +	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[] = {
> +	[7]   = DHCPV6_OPT_UINT8,		/* Preference */
> +	[15]  = DHCPV6_OPT_LEN_STR_LIST,	/* User Class */
> +	[16]  = DHCPV6_OPT_VENDOR_CLASS,	/* Vendor Class */
> +	[17]  = DHCPV6_OPT_VENDOR_CLASS,	/* Vendor Opts */

See below, option 17 cannot be decoded with DHCPV6_OPT_VENDOR_CLASS

> +	[31]  = DHCPV6_OPT_IPV6_LIST,		/* SNTP Servers */
> +	[32]  = DHCPV6_OPT_UINT32,		/* Information Refresh Time */
> +	[59]  = DHCPV6_OPT_STR,			/* Boot File URL */
> +	[60]  = DHCPV6_OPT_LEN_STR_LIST,	/* Boot File Params */
> +	[82]  = DHCPV6_OPT_UINT32,		/* SOL_MAX_RT */
> +	[83]  = DHCPV6_OPT_UINT32,		/* 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)
> +{
> +	enum dhcpv6_opt_type type;
> +	unsigned long val;
> +	uint8_t width;
> +	size_t slen;
> +	int len;
> +
> +	if (!*str)
> +		return -1;
> +
> +	if (code >= ARRAY_SIZE(dhcpv6_opt_types))
> +		return -1;
> +
> +	type = dhcpv6_opt_types[code];
> +
> +	switch (type) {
> +	case DHCPV6_OPT_NONE:
> +		return -1;
> +	case DHCPV6_OPT_IPV6:
> +	case DHCPV6_OPT_IPV6_LIST: {
> +		union inany_addr addr;
> +		sa_family_t af;
> +
> +		len = 0;
> +
> +		do {
> +			if (len + sizeof(struct in6_addr) > buf_len)
> +				return -1;
> +
> +			if (!parse_inany_(&str, &addr, &af) ||
> +			    af != AF_INET6)
> +				return -1;
> +
> +			memcpy(buf + len, &addr.a6, sizeof(struct in6_addr));
> +			len += sizeof(struct in6_addr);
> +
> +			if (type == DHCPV6_OPT_IPV6)
> +				break;
> +		} while (parse_literal(&str, ","));
> +
> +		if (!len || !parse_eoi(str))
> +			return -1;
> +
> +		return len;
> +	}
> +	case DHCPV6_OPT_UINT8:
> +	case DHCPV6_OPT_UINT16:
> +	case DHCPV6_OPT_UINT32:
> +		if (type == DHCPV6_OPT_UINT8)
> +			width = 1;
> +		else if (type == DHCPV6_OPT_UINT16)
> +			width = 2;
> +		else
> +			width = 4;
> +
> +		if (buf_len < width)
> +			return -1;
> +
> +		if (!parse_unsigned(&str, 0, &val) ||
> +		    !parse_eoi(str) ||
> +		    val >= (1ULL << (width * 8)))
> +			return -1;
> +
> +		if (type == DHCPV6_OPT_UINT16)
> +			*(uint16_t *)buf = htons(val);
> +		else if (type == DHCPV6_OPT_UINT32)
> +			*(uint32_t *)buf = htonl(val);

I think they could be alignment problem here as buf is uint8_t *.
You should use something like:

uint16_t v16 = htons(val);  memcpy(buf, &v16, sizeof(v16));

and

uint32_t v32 = htonl(val);  memcpy(buf, &v32, sizeof(v32));

> +		else
> +			buf[0] = val;
> +
> +		return width;
> +	case DHCPV6_OPT_STR:
> +		slen = strlen(str);
> +
> +		if (slen >= buf_len)

off by one, should be "if (slen > buf_len)" (as the option string is not NUL terminated).

> +			return -1;
> +
> +		memcpy(buf, str, slen);
> +
> +		return slen;
> +	case DHCPV6_OPT_VENDOR_CLASS: {
> +		uint16_t slen_net;
> +		uint32_t ent;
> +
> +		if (!parse_unsigned(&str, 0, &val) ||
> +		    !parse_literal(&str, ":") ||
> +		    val > UINT32_MAX)
> +			return -1;
> +
> +		slen = strlen(str);
> +		if (!slen)
> +			return -1;
> +
> +		len = sizeof(uint32_t) + sizeof(uint16_t) + slen;

Why to add sizeof(uint32_t) + sizeof(uint16_t)?

For option 16, it's length of vendor-class-data/opaque-data (so without the length of 
enterprise-number and vendor-class-len)
> +		if ((size_t)len > buf_len)
> +			return -1;
> +
> +		ent = htonl(val);
> +		memcpy(buf, &ent, sizeof(ent));

So I guess this is "enterprise-number"

> +
> +		slen_net = htons(slen);
> +		memcpy(buf + sizeof(uint32_t), &slen_net, sizeof(slen_net));

For option 16, it's vendor-class-len, but for option 17 we have here sub-opt-code, so it 
doesn't work. I think you cannot use DHCPV6_OPT_VENDOR_CLASS with option 17.

> +
> +		memcpy(buf + sizeof(uint32_t) + sizeof(uint16_t),
> +		       str, slen);
> +
> +		return len;
> +	}
> +	case DHCPV6_OPT_LEN_STR_LIST:
> +		len = 0;
> +
> +		while (*str) {
> +			uint16_t slen_net;
> +
> +			slen = strcspn(str, ",");
> +			if (!slen)
> +				return -1;
> +
> +			if (len + (int)(sizeof(uint16_t) + slen) > (int)buf_len)
> +				return -1;
> +
> +			slen_net = htons(slen);
> +			memcpy(buf + len, &slen_net, sizeof(slen_net));
> +			len += sizeof(uint16_t);
> +
> +			memcpy(buf + len, str, slen);
> +			len += slen;
> +
> +			str += slen;
> +			if (*str == ',')
> +				str++;
> +		}
> +
> +		if (!len)
> +			return -1;
> +
> +		return len;
> +	}
> +
> +	return -1;
> +}
> +
> +/**
> + * struct dhcpv6_user_opts - User-specified DHCPv6 options from --dhcpv6-opt
> + * @val:	Binary option value in wire format
> + * @len:	Length of @val in bytes, 0 if not set
> + */
> +static struct {
> +	uint8_t val[UINT16_MAX];
> +	int len;
> +} dhcpv6_user_opts[ARRAY_SIZE(dhcpv6_opt_types)];
> +
> +/**
> + * dhcpv6_opt_to_str() - Render a binary DHCPv6 option value to printable string
> + * @code:	DHCPv6 option code
> + * @buf:	Output string buffer
> + * @buf_len:	Size of output buffer
> + *
> + * Return: pointer to @buf if option is set, NULL otherwise
> + */
> +const char *dhcpv6_opt_to_str(uint16_t code, char *buf, size_t buf_len)
> +{
> +	enum dhcpv6_opt_type type;
> +	unsigned int i;
> +	uint16_t slen;
> +	int off = 0;
> +
> +	if (code >= ARRAY_SIZE(dhcpv6_user_opts) ||
> +		!dhcpv6_user_opts[code].len)
> +		return NULL;
> +
> +	type = dhcpv6_opt_types[code];
> +
> +	switch (type) {
> +	case DHCPV6_OPT_IPV6:
> +	case DHCPV6_OPT_IPV6_LIST:

I think buf from conf_print() is too small to contains a list of in6 addr (char 
buf[INANY_ADDRSTRLEN]), it can contain only one.

> +		for (i = 0; i + sizeof(struct in6_addr) <=
> +		     (unsigned int)dhcpv6_user_opts[code].len;
> +		     i += sizeof(struct in6_addr)) {
> +			if (off)
> +				off += snprintf(buf + off, buf_len - off, ",");

It's simpler to write:

if (off) {
	if (off + 1 >= (int)buf_len)
		return NULL;
	buf[off++] = ',';
}

> +			inet_ntop(AF_INET6, dhcpv6_user_opts[code].val + i,
> +				  buf + off, buf_len - off);

Return error of inet_ntop() is silently ignored, if buf - off is not big enough, nothing 
is written.

if (!inet_ntop(AF_INET6, dhcpv6_user_opts[code].val + i, buf + off, buf_len - off))
	return NULL;
> +			off = strlen(buf);

off += strlen(buff + off) is more efficient.

> +		}
> +		return buf;
> +	case DHCPV6_OPT_UINT8:
> +	case DHCPV6_OPT_UINT16:
> +	case DHCPV6_OPT_UINT32: {
> +		unsigned int val = 0;
> +		int j;
> +
> +		for (j = 0; j < dhcpv6_user_opts[code].len; j++)
> +			val = (val << 8) | dhcpv6_user_opts[code].val[j];
> +
> +		(void)snprintf(buf, buf_len, "%u", val);
> +		return buf;
> +	}
> +	case DHCPV6_OPT_STR:
> +		(void)snprintf(buf, buf_len, "%.*s",
> +			 dhcpv6_user_opts[code].len,
> +			 dhcpv6_user_opts[code].val);
> +		return buf;
> +	case DHCPV6_OPT_VENDOR_CLASS: {
> +		uint32_t ent;
> +		int doff;
> +
> +		if (dhcpv6_user_opts[code].len <
> +		    (int)(sizeof(uint32_t) + sizeof(uint16_t)))
> +			return NULL;
> +
> +		memcpy(&ent, dhcpv6_user_opts[code].val, sizeof(ent));
> +		memcpy(&slen, dhcpv6_user_opts[code].val + sizeof(uint32_t),
> +		       sizeof(slen));
> +		doff = sizeof(uint32_t) + sizeof(uint16_t);
> +
> +		(void)snprintf(buf, buf_len, "%u:%.*s",
> +			 ntohl(ent), ntohs(slen),
> +			 dhcpv6_user_opts[code].val + doff);
> +		return buf;
> +	}
> +	case DHCPV6_OPT_LEN_STR_LIST:
> +		off = 0;
> +		i = 0;
> +
> +		while (i + sizeof(uint16_t) <=
> +		       (unsigned int)dhcpv6_user_opts[code].len) {
> +			memcpy(&slen, dhcpv6_user_opts[code].val + i,
> +			       sizeof(slen));
> +			slen = ntohs(slen);
> +			i += sizeof(uint16_t);
> +
> +			if (i + slen > (unsigned int)dhcpv6_user_opts[code].len)
> +				break;
> +
> +			if (off)
> +				off += snprintf(buf + off, buf_len - off, ",");

As previously:

if (off) {
	if (off + 1 >= (int)buf_len)
		return NULL;
	buf[off++] = ',';
}

> +			off += snprintf(buf + off, buf_len - off, "%.*s",
> +					slen, dhcpv6_user_opts[code].val + i);

we should check buf_len is big enough to store slen.

if (off + slen >= (int)buf_len)
	return NULL;

and as we have the size, mempcpy() seems to be a better choice than snprintf() (and we 
have  checked it fits in buf):

memcpy(buf + off, dhcpv6_user_opts[code].val + i, slen);
off += slen;
buf[off] = '\0';

> +			i += slen;
> +		}
> +		return buf;
> +	default:
> +		return NULL;
> +	}
> +}
> +
> +/**
> + * dhcpv6_set_opt() - Parse and store a user-specified DHCPv6 option
> + * @code:	DHCPv6 option code
> + * @val_str:	Value string from command line
> + */
> +void dhcpv6_set_opt(uint16_t code, const char *val_str)
> +{
> +	int ret;
> +
> +	if (code >= ARRAY_SIZE(dhcpv6_user_opts))
> +		die("DHCPv6 option code %u out of supported range", code);
> +
> +	ret = dhcpv6_opt_parse(code, val_str,
> +			       dhcpv6_user_opts[code].val,
> +			       sizeof(dhcpv6_user_opts[code].val));
> +	if (ret < 0)
> +		die("Invalid value for DHCPv6 option %u: %s", code, val_str);
> +
> +	dhcpv6_user_opts[code].len = ret;
> +}
> +
>   /**
>    * dhcpv6_opt() - Get option from DHCPv6 message
>    * @data:	Buffer with options, set to matching option on return
> @@ -678,7 +1003,6 @@ int dhcpv6(struct ctx *c, struct iov_tail *data,
>   	    sizeof(struct opt_hdr) + ntohs(client_id->l);
>   	n = dhcpv6_dns_fill(c, (char *)&resp, n);
>   	n = dhcpv6_client_fqdn_fill(data, c, (char *)&resp, n);
> -

Seems to be unrelated.

>   	resp.hdr.xid = mh->xid;
>   
>   	tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n);
> diff --git a/dhcpv6.h b/dhcpv6.h
> index 1015a1a..973c477 100644
> --- a/dhcpv6.h
> +++ b/dhcpv6.h
> @@ -9,5 +9,7 @@
>   int dhcpv6(struct ctx *c, struct iov_tail *data,
>   	   const struct in6_addr *saddr, const struct in6_addr *daddr);
>   void dhcpv6_init(const struct ctx *c);
> +void dhcpv6_set_opt(uint16_t code, const char *val_str);
> +const char *dhcpv6_opt_to_str(uint16_t code, char *buf, size_t buf_len);
>   
>   #endif /* DHCPV6_H */
> diff --git a/passt.1 b/passt.1
> index 53e072a..156aa8d 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -440,6 +440,37 @@ Send \fIname\fR as DHCP option 12 (hostname).
>   FQDN to configure the client with.
>   Send \fIname\fR as Client FQDN: DHCP option 81 and DHCPv6 option 39.
>   
> +.TP
> +.BR \-\-dhcpv6-opt " " \fICODE\fR,\fIVALUE\fR
> +Set DHCPv6 option \fICODE\fR to \fIVALUE\fR.  The value format depends
> +on the option type and is determined 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
>   Configure TCP port forwarding to guest or namespace. \fIspec\fR can be either: