public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options
@ 2026-06-01  7:37 Anshu Kumari
  2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

This series adds support for custom DHCP options in passt, enabling
network boot (PXE/UEFI HTTP Boot) and arbitrary DHCP option injection.

Two new command-line flags are introduced:

  --dhcp-boot URL    Sets the boot file URL (DHCP option 67 and the
                     legacy boot file field)

  --dhcp-opt CODE,VALUE
                     Sets any DHCP option by numeric code, with
                     type-aware parsing per RFC 2132

The DHCP reply path is extended with option overload support (RFC 2132
option 52), allowing options to overflow into the file and sname fields
when the standard options area is full.

Anshu Kumari (6):
  conf: Add --dhcp-opt command-line option
  conf: Add --dhcp-boot command-line option
  dhcp: Add option type table and value parser
  dhcp: Refactor fill_one() to operate on a generic buffer
  dhcp: Add option overload
  doc: Add --dhcp-boot and --dhcp-opt to man page

 conf.c  |  29 ++++-
 dhcp.c  | 322 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 dhcp.h  |   1 +
 passt.1 |  41 ++++++++
 passt.h |  16 +++
 5 files changed, 389 insertions(+), 20 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/6] conf: Add --dhcp-opt command-line option
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:00   ` David Gibson
  2026-06-11 23:04   ` Stefano Brivio
  2026-06-01  7:37 ` [PATCH v3 2/6] conf: Add --dhcp-boot " Anshu Kumari
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

Introduce the --dhcp-opt flag that allows setting arbitrary DHCP
options from command-line in the form of [Option CODE,VALUE].
This patch adds the option storage in struct ctx and CLI parsing;
the type-aware value parser and DHCP reply injection follow
in subsequent patches.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - Added dhcp_add_option() helper in dhcp.c for storing options with
    duplicate-code detection.
  - case 33 now calls dhcp_add_option() instead of inline storage.

v2:
  - Added kerneldoc for @custom_opts, @custom_opts.code, @custom_opts.str, and @custom_opts_count in struct ctx
  - Removed len and val[255] fields from struct (moved to patch 3)
  - Removed braces from case 33, moved declarations (optcode, comma, end) to function scope
  - Renamed code → optcode to follow function-scope convention.
---
 conf.c  | 24 +++++++++++++++++++++++-
 dhcp.c  | 38 ++++++++++++++++++++++++++++++++++++++
 dhcp.h  |  1 +
 passt.h | 12 ++++++++++++
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/conf.c b/conf.c
index 029b9c7..ce78af1 100644
--- a/conf.c
+++ b/conf.c
@@ -47,6 +47,7 @@
 #include "lineread.h"
 #include "isolation.h"
 #include "log.h"
+#include "dhcp.h"
 #include "vhost_user.h"
 #include "epoll_ctl.h"
 #include "conf.h"
@@ -616,7 +617,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"
+		"  --dhcp-opt CODE,VAL	Set DHCP option by code\n");
 	if (strstr(name, "pasta"))
 		FPRINTF(f, "    default: don't use any search list\n");
 	else
@@ -844,6 +846,10 @@ static void conf_print(const struct ctx *c)
 			info("    router: %s",
 			     inet_ntop(AF_INET, &c->ip4.guest_gw,
 				       buf, sizeof(buf)));
+			for (i = 0; i < c->custom_opts_count; i++)
+				info("    option %u: %s",
+				     c->custom_opts[i].code,
+				     c->custom_opts[i].str);
 		}
 
 		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
@@ -1233,6 +1239,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"migrate-no-linger", no_argument,	NULL,		30 },
 		{"stats", required_argument,		NULL,		31 },
 		{"conf-path",	required_argument,	NULL,		'c' },
+		{"dhcp-opt", required_argument,		NULL,		33 },
 		{ 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:";
@@ -1248,10 +1255,13 @@ void conf(struct ctx *c, int argc, char **argv)
 	uint8_t prefix_len_from_opt = 0;
 	unsigned int ifi4 = 0, ifi6 = 0;
 	const char *logfile = NULL;
+	unsigned long optcode;
 	char *runas = NULL;
 	size_t logsize = 0;
+	const char *comma;
 	long fd_tap_opt;
 	int name, ret;
+	char *end;
 	uid_t uid;
 	gid_t gid;
 	
@@ -1465,6 +1475,18 @@ void conf(struct ctx *c, int argc, char **argv)
 				die("Can't display statistics if not running in foreground");
 			c->stats = strtol(optarg, NULL, 0);
 			break;
+		case 33:
+			comma = strchr(optarg, ',');
+			if (!comma)
+				die("--dhcp-opt requires CODE,VALUE format");
+
+			optcode = strtoul(optarg, &end, 0);
+			if (end != comma || optcode < 1 || optcode > 254)
+				die("DHCP option code must be 1-254: %s",
+				    optarg);
+
+			dhcp_add_option(c, optcode, comma + 1);
+			break;
 		case 'd':
 			c->debug = 1;
 			c->quiet = 0;
diff --git a/dhcp.c b/dhcp.c
index 1ff8cba..c5fbf37 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -33,6 +33,44 @@
 #include "log.h"
 #include "dhcp.h"
 
+
+/**
+ * dhcp_add_option() - Add or update a custom DHCP option
+ * @c:		Execution context
+ * @code:	DHCP option code
+ * @val_str:	Value string from command line
+ *
+ * If @code was already added, the previous value is overwritten.
+ * Calls die() on any error.
+ *
+ * Return: 0 on success
+ */
+int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
+{
+	int idx;
+
+	for (idx = 0; idx < c->custom_opts_count; idx++) {
+		if (c->custom_opts[idx].code == code)
+			break;
+	}
+
+	if (idx == c->custom_opts_count) {
+		if (c->custom_opts_count >= MAX_CUSTOM_DHCP_OPTS)
+			die("Too many --dhcp-opt entries (max %d)",
+			    MAX_CUSTOM_DHCP_OPTS);
+		c->custom_opts_count++;
+	}
+
+	c->custom_opts[idx].code = code;
+
+	if (snprintf_check(c->custom_opts[idx].str,
+			   sizeof(c->custom_opts[0].str),
+			   "%s", val_str))
+		die("DHCP option value too long: %s", val_str);
+
+	return 0;
+}
+
 /**
  * struct opt - DHCP option
  * @sent:	Convenience flag, set while filling replies
diff --git a/dhcp.h b/dhcp.h
index cd50c99..9c8f1e3 100644
--- a/dhcp.h
+++ b/dhcp.h
@@ -8,5 +8,6 @@
 
 int dhcp(const struct ctx *c, struct iov_tail *data);
 void dhcp_init(void);
+int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str);
 
 #endif /* DHCP_H */
diff --git a/passt.h b/passt.h
index 1726965..3a0816f 100644
--- a/passt.h
+++ b/passt.h
@@ -182,6 +182,10 @@ struct ip6_ctx {
  * @dns_search:		DNS search list
  * @hostname:		Guest hostname
  * @fqdn:		Guest FQDN
+ * @custom_opts:	User-specified DHCP options from --dhcp-opt
+ * @custom_opts.code:	DHCP option code
+ * @custom_opts.str:	Original string value from command line
+ * @custom_opts_count:	Number of entries in @custom_opts
  * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
  * @ip6:		IPv6 configuration
  * @pasta_ifn:		Name of namespace interface for pasta
@@ -263,6 +267,14 @@ struct ctx {
 	char hostname[PASST_MAXDNAME];
 	char fqdn[PASST_MAXDNAME];
 
+#define MAX_CUSTOM_DHCP_OPTS	32
+
+	struct {
+		uint8_t code;
+		char str[256];
+	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
+	int custom_opts_count;
+
 	int ifi6;
 	struct ip6_ctx ip6;
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 2/6] conf: Add --dhcp-boot command-line option
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
  2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:02   ` David Gibson
  2026-06-11 23:05   ` Stefano Brivio
  2026-06-01  7:37 ` [PATCH v3 3/6] dhcp: Add option type table and value parser Anshu Kumari
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

Introduce the --dhcp-boot flag that sets the boot file URL for
network boot specially for ipxe. This patch adds the option
storage and CLI parsing.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - case 32 now calls dhcp_add_option(c, 67, optarg).
  - Handles duplicate codes: --dhcp-boot and --dhcp-opt 67 coexist
    correctly, last value wins.

v2:
  - Removed separate dhcp_boot[PATH_MAX] field — --dhcp-boot foo now stores into custom_opts[] as code 67 (same as --dhcp-opt 67,foo)

---
 conf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/conf.c b/conf.c
index ce78af1..f7281e2 100644
--- a/conf.c
+++ b/conf.c
@@ -618,6 +618,7 @@ static void usage(const char *name, FILE *f, int status)
 		"    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"
+		"  --dhcp-boot URL	Boot file URL for network boot\n"
 		"  --dhcp-opt CODE,VAL	Set DHCP option by code\n");
 	if (strstr(name, "pasta"))
 		FPRINTF(f, "    default: don't use any search list\n");
@@ -1239,6 +1240,7 @@ void conf(struct ctx *c, int argc, char **argv)
 		{"migrate-no-linger", no_argument,	NULL,		30 },
 		{"stats", required_argument,		NULL,		31 },
 		{"conf-path",	required_argument,	NULL,		'c' },
+		{"dhcp-boot", required_argument,	NULL,		32 },
 		{"dhcp-opt", required_argument,		NULL,		33 },
 		{ 0 },
 	};
@@ -1475,6 +1477,9 @@ void conf(struct ctx *c, int argc, char **argv)
 				die("Can't display statistics if not running in foreground");
 			c->stats = strtol(optarg, NULL, 0);
 			break;
+		case 32:
+			dhcp_add_option(c, 67, optarg);
+			break;
 		case 33:
 			comma = strchr(optarg, ',');
 			if (!comma)
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 3/6] dhcp: Add option type table and value parser
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
  2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
  2026-06-01  7:37 ` [PATCH v3 2/6] conf: Add --dhcp-boot " Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:23   ` David Gibson
  2026-06-11 23:04   ` Stefano Brivio
  2026-06-01  7:37 ` [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

Add an RFC 2132 type lookup table mapping DHCP option codes to their
expected value formats, and a dhcp_opt_parse() function that converts
CLI string values into their binary wire representation.

Wire dhcp_opt_parse() into dhcp_add_option() so that values are
validated and encoded at configuration time.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32
    enums, removed dhcp_opt_int_width[] array.
  - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse
    both as list, error if >1 in single case.
  - Added errno = 0 before strtoul() and check after.
  - Fixed range check: 1ULL << (width * 8) for all widths including
    width==4.
  - strncpy → memcpy for DHCP_OPT_STR.
  - Moved enum to dhcp.c since not used in other files.
  - Removed options 55, 61 (client-only), 119 (DNS compression, use
    --dhcp-search instead), 33 (IP pairs not supported).
  - DHCP_OPT_PARSE_BUF 1024 → char tmp[256].
  - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate
    val[]/len.
  - Aligned array entries for readability.
  - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc.
  - Reject empty value strings before parsing
  - Reject leading/trailing/consecutive commas in IP list values.

v2:
  - Replaced struct lookup table + dhcp_opt_type_lookup() function with flat dhcp_opt_types[256] array indexed by code.
  - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table.
  - Dropped DHCP_OPT_ROUTES / option 121 entirely.
  - Added kerneldoc for enum dhcp_opt_type values.
  - Removed curly braces from switch cases, declarations before switch.
  - Added newlines before return statements.
  - Changed IP list delimiter from space to comma (--dhcp-opt 6,1.1.1.1,8.8.8.8).
  - Defined DHCP_OPT_PARSE_BUF constant for bare 1024.
  - Added len and val[255] fields to struct here (moved from patch 1).
  - Added kerneldoc for @custom_opts.len and @custom_opts.val.
  - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate val/len.
---
 dhcp.c  | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 passt.h |   4 ++
 2 files changed, 181 insertions(+), 3 deletions(-)

diff --git a/dhcp.c b/dhcp.c
index c5fbf37..07a42b9 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -23,6 +23,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <limits.h>
+#include <errno.h>
 
 #include "util.h"
 #include "ip.h"
@@ -33,6 +34,170 @@
 #include "log.h"
 #include "dhcp.h"
 
+/**
+ * enum dhcp_opt_type - DHCP option value types per RFC 2132
+ * @DHCP_OPT_NONE:	Unsupported or unknown option
+ * @DHCP_OPT_STR:	Variable-length string
+ * @DHCP_OPT_IPV4:	Single IPv4 address
+ * @DHCP_OPT_IPV4_LIST:	Multiple IPv4 addresses, comma-separated
+ * @DHCP_OPT_INT8:	Unsigned 8-bit integer
+ * @DHCP_OPT_INT16:	Unsigned 16-bit integer
+ * @DHCP_OPT_INT32:	Unsigned 32-bit integer
+ */
+enum dhcp_opt_type {
+	DHCP_OPT_NONE,
+	DHCP_OPT_STR,
+	DHCP_OPT_IPV4,
+	DHCP_OPT_IPV4_LIST,
+	DHCP_OPT_INT8,
+	DHCP_OPT_INT16,
+	DHCP_OPT_INT32,
+};
+
+/**
+ * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code
+ */
+static const enum dhcp_opt_type dhcp_opt_types[256] = {
+	[1]   = DHCP_OPT_IPV4,		/* Subnet Mask */
+	[2]   = DHCP_OPT_INT32,		/* Time Offset */
+	[3]   = DHCP_OPT_IPV4_LIST,	/* Router */
+	[4]   = DHCP_OPT_IPV4_LIST,	/* Time Server */
+	[5]   = DHCP_OPT_IPV4_LIST,	/* Name Server */
+	[6]   = DHCP_OPT_IPV4_LIST,	/* Domain Name Server */
+	[7]   = DHCP_OPT_IPV4_LIST,	/* Log Server */
+	[8]   = DHCP_OPT_IPV4_LIST,	/* Cookie Server */
+	[9]   = DHCP_OPT_IPV4_LIST,	/* LPR Server */
+	[10]  = DHCP_OPT_IPV4_LIST,	/* Impress Server */
+	[11]  = DHCP_OPT_IPV4_LIST,	/* Resource Location Server */
+	[12]  = DHCP_OPT_STR,		/* Host Name */
+	[13]  = DHCP_OPT_INT16,		/* Boot File Size */
+	[15]  = DHCP_OPT_STR,		/* Domain Name */
+	[16]  = DHCP_OPT_IPV4,		/* Swap Server */
+	[17]  = DHCP_OPT_STR,		/* Root Path */
+	[19]  = DHCP_OPT_INT8,		/* IP Forwarding */
+	[23]  = DHCP_OPT_INT8,		/* Default IP TTL */
+	[26]  = DHCP_OPT_INT16,		/* Interface MTU */
+	[28]  = DHCP_OPT_IPV4,		/* Broadcast Address */
+	[37]  = DHCP_OPT_INT8,		/* TCP Default TTL */
+	[38]  = DHCP_OPT_INT32,		/* TCP Keepalive Interval */
+	[40]  = DHCP_OPT_STR,		/* NIS Domain Name */
+	[41]  = DHCP_OPT_IPV4_LIST,	/* NIS Servers */
+	[42]  = DHCP_OPT_IPV4_LIST,	/* NTP Servers */
+	[44]  = DHCP_OPT_IPV4_LIST,	/* NetBIOS Name Server */
+	[50]  = DHCP_OPT_IPV4,		/* Requested IP Address */
+	[51]  = DHCP_OPT_INT32,		/* IP Address Lease Time */
+	[53]  = DHCP_OPT_INT8,		/* DHCP Message Type */
+	[54]  = DHCP_OPT_IPV4,		/* Server Identifier */
+	[57]  = DHCP_OPT_INT16,		/* Max DHCP Message Size */
+	[58]  = DHCP_OPT_INT32,		/* Renewal (T1) Time */
+	[59]  = DHCP_OPT_INT32,		/* Rebinding (T2) Time */
+	[60]  = DHCP_OPT_STR,		/* Vendor Class Identifier */
+	[66]  = DHCP_OPT_STR,		/* TFTP Server Name */
+	[67]  = DHCP_OPT_STR,		/* Bootfile Name */
+	[252] = DHCP_OPT_STR,		/* WPAD URL */
+};
+
+/**
+ * dhcp_opt_parse() - Parse a DHCP option value
+ * @code:	DHCP 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 dhcp_opt_parse(uint8_t code, const char *str,
+			  uint8_t *buf, size_t buf_len)
+{
+	enum dhcp_opt_type type = dhcp_opt_types[code];
+	char *tok, *saveptr, *end;
+	struct in_addr addr;
+	unsigned long val;
+	unsigned int i;
+	uint8_t width;
+	char tmp[256];
+	size_t slen;
+	int len;
+
+	if (!*str)
+		die("Empty value for DHCP option %u", code);
+
+	switch (type) {
+	case DHCP_OPT_NONE:
+		die("Unsupported DHCP option: %u,"
+		    " see passt(1) for supported codes", code);
+	case DHCP_OPT_IPV4:
+	case DHCP_OPT_IPV4_LIST:
+		len = 0;
+
+		/* Reject empty, leading/trailing, or consecutive commas */
+		if (!*str || *str == ',' || str[strlen(str) - 1] == ',' ||
+		    strstr(str, ",,"))
+			return -1;
+
+		if (snprintf_check(tmp, sizeof(tmp), "%s", str))
+			return -1;
+
+		for (tok = strtok_r(tmp, ",", &saveptr); tok;
+		     tok = strtok_r(NULL, ",", &saveptr)) {
+			if (inet_pton(AF_INET, tok, &addr) != 1)
+				return -1;
+
+			if (len + (int)sizeof(addr) > (int)buf_len)
+				return -1;
+
+			memcpy(buf + len, &addr, sizeof(addr));
+			len += sizeof(addr);
+
+			if (type == DHCP_OPT_IPV4)
+				break;
+		}
+
+		if (type == DHCP_OPT_IPV4 && strtok_r(NULL, ",", &saveptr))
+			return -1;
+
+		return len;
+	case DHCP_OPT_INT8:
+	case DHCP_OPT_INT16:
+	case DHCP_OPT_INT32:
+		if (type == DHCP_OPT_INT8)
+			width = 1;
+		else if (type == DHCP_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 DHCP_OPT_STR:
+		slen = strlen(str);
+
+		if (!slen || slen >= buf_len)
+			return -1;
+
+		memcpy(buf, str, slen);
+
+		return slen;
+	}
+
+	return -1;
+}
 
 /**
  * dhcp_add_option() - Add or update a custom DHCP option
@@ -40,14 +205,15 @@
  * @code:	DHCP option code
  * @val_str:	Value string from command line
  *
- * If @code was already added, the previous value is overwritten.
- * Calls die() on any error.
+ * Parses @val_str according to the type registered for @code in
+ * dhcp_opt_types[]. If @code was already added, the previous value
+ * is overwritten. Calls die() on any error.
  *
  * Return: 0 on success
  */
 int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
 {
-	int idx;
+	int idx, ret;
 
 	for (idx = 0; idx < c->custom_opts_count; idx++) {
 		if (c->custom_opts[idx].code == code)
@@ -61,7 +227,15 @@ int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
 		c->custom_opts_count++;
 	}
 
+	ret = dhcp_opt_parse(code, val_str,
+			     c->custom_opts[idx].val,
+			     sizeof(c->custom_opts[0].val));
+	if (ret < 0)
+		die("Invalid value for DHCP option %u: %s",
+		    code, val_str);
+
 	c->custom_opts[idx].code = code;
+	c->custom_opts[idx].len = ret;
 
 	if (snprintf_check(c->custom_opts[idx].str,
 			   sizeof(c->custom_opts[0].str),
diff --git a/passt.h b/passt.h
index 3a0816f..751fee3 100644
--- a/passt.h
+++ b/passt.h
@@ -184,6 +184,8 @@ struct ip6_ctx {
  * @fqdn:		Guest FQDN
  * @custom_opts:	User-specified DHCP options from --dhcp-opt
  * @custom_opts.code:	DHCP option code
+ * @custom_opts.len:	Length of binary value in @val
+ * @custom_opts.val:	Binary-encoded option value
  * @custom_opts.str:	Original string value from command line
  * @custom_opts_count:	Number of entries in @custom_opts
  * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
@@ -271,6 +273,8 @@ struct ctx {
 
 	struct {
 		uint8_t code;
+		uint8_t len;
+		uint8_t val[255];
 		char str[256];
 	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
 	int custom_opts_count;
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
                   ` (2 preceding siblings ...)
  2026-06-01  7:37 ` [PATCH v3 3/6] dhcp: Add option type table and value parser Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:25   ` David Gibson
  2026-06-01  7:37 ` [PATCH v3 5/6] dhcp: Add option overload Anshu Kumari
  2026-06-01  7:37 ` [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page Anshu Kumari
  5 siblings, 1 reply; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

Change fill_one() to accept a buffer pointer and capacity instead of
a struct msg pointer.  This is a pure refactor with no behavior change,
preparing for option overload support where fill_one() will also write
into the file and sname fields.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - Restored removed comments: "If we don't have space to write the
    option, then just skip" and "Move to option".

v2:
  - Renamed parameter cap → size.
---
 dhcp.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/dhcp.c b/dhcp.c
index 07a42b9..5c6a492 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -343,28 +343,29 @@ struct msg {
 } __attribute__((__packed__));
 
 /**
- * fill_one() - Fill a single option in message
- * @m:		Message to fill
+ * fill_one() - Fill a single option into a buffer
+ * @buf:	Buffer to write option
+ * @size:	Usable size of @buf (excluding end marker)
  * @o:		Option number
- * @offset:	Current offset within options field, updated on insertion
+ * @offset:	Current offset within @buf, updated on insertion
  *
- * Return: false if m has space to write the option, true otherwise
+ * Return: false if @buf has space to write the option, true otherwise
  */
-static bool fill_one(struct msg *m, int o, int *offset)
+static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
 {
 	size_t slen = opts[o].slen;
 
 	/* If we don't have space to write the option, then just skip */
-	if (*offset + 2 /* code and length of option */ + slen > OPT_MAX)
+	if (*offset + 2 + slen > size)
 		return true;
 
-	m->o[*offset] = o;
-	m->o[*offset + 1] = slen;
+	buf[*offset] = o;
+	buf[*offset + 1] = slen;
 
 	/* Move to option */
 	*offset += 2;
 
-	memcpy(&m->o[*offset], opts[o].s, slen);
+	memcpy(&buf[*offset], opts[o].s, slen);
 
 	opts[o].sent = 1;
 	*offset += slen;
@@ -389,19 +390,19 @@ static int fill(struct msg *m)
 	 * Put it there explicitly, unless requested via option 55.
 	 */
 	if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
-		if (fill_one(m, 53, &offset))
+		if (fill_one(m->o, OPT_MAX, 53, &offset))
 			 debug("DHCP: skipping option 53");
 
 	for (i = 0; i < opts[55].clen; i++) {
 		o = opts[55].c[i];
 		if (opts[o].slen != -1)
-			if (fill_one(m, o, &offset))
+			if (fill_one(m->o, OPT_MAX, o, &offset))
 				debug("DHCP: skipping option %i", o);
 	}
 
 	for (o = 0; o < 255; o++) {
 		if (opts[o].slen != -1 && !opts[o].sent)
-			if (fill_one(m, o, &offset))
+			if (fill_one(m->o, OPT_MAX, o, &offset))
 				debug("DHCP: skipping option %i", o);
 	}
 
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 5/6] dhcp: Add option overload
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
                   ` (3 preceding siblings ...)
  2026-06-01  7:37 ` [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:50   ` David Gibson
  2026-06-11 23:04   ` Stefano Brivio
  2026-06-01  7:37 ` [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page Anshu Kumari
  5 siblings, 2 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

A user can enter lots of options in command-line which may not fit in
existing buffer, So when the options field is full, overflow remaining
DHCP options into the file and sname fields per RFC 2132 option 52.

Also, when the file field is not used for overload, copy the boot
file URL there directly for legacy PXE clients.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - Added RFC 2132 Section 9.3 reference comment on overload
    constants.
  - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow().
  - Swapped overflow order: try sname (64 bytes) first, then file
    (128 bytes) — better packing and keeps file field available for
    boot file name.
  - Removed '&' from &reply.file.
  - Removed '+1' from memcpy — reply.file already zeroed.
  - opt_set_dns_search() max_len: OPT_MAX - 3 instead of
    sizeof(m->o).

v2:
  - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants
  - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */
  - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname
  - Removed dhcp_boot references — reply.file copy now reads from opts[67]
  - Used DHCP_OVERLOAD_FILE constant in reply.file guard
---
 dhcp.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 81 insertions(+), 10 deletions(-)

diff --git a/dhcp.c b/dhcp.c
index 5c6a492..a9d56fd 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -372,14 +372,64 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
 	return false;
 }
 
+/* RFC 2132, Section 9.3 - Option Overload*/
+#define DHCP_OVERLOAD_FILE	1
+#define DHCP_OVERLOAD_SNAME	2
+
 /**
- * fill() - Fill options in message
+ * fill_overflow() - Fill remaining options into sname and file fields
+ * @m:		Message whose sname/file fields may be used for overflow
+ *
+ * Try the smaller sname field first: small options go there, leaving
+ * the larger file field available for big options and for the boot
+ * file name (option 67) if set.
+ *
+ * Return: option 52 overload value: 0 if no overflow,
+ *         DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname,
+ *         or both OR'd together
+ */
+static int fill_overflow(struct msg *m)
+{
+	int sname_off = 0, file_off = 0, overload = 0;
+	int o;
+
+	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
+		if (opts[o].slen == -1 || opts[o].sent)
+			continue;
+		fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off);
+	}
+
+	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
+		if (opts[o].slen == -1 || opts[o].sent)
+			continue;
+		if (fill_one(m->file, sizeof(m->file) - 1, o, &file_off))
+			debug("DHCP: skipping option %i (overload full)", o);
+	}
+
+	if (sname_off) {
+		m->sname[sname_off] = 255;
+		overload |= DHCP_OVERLOAD_SNAME;
+	}
+
+	if (file_off) {
+		m->file[file_off] = 255;
+		overload |= DHCP_OVERLOAD_FILE;
+	}
+
+	return overload;
+}
+
+/**
+ * fill() - Fill options in message, with overload into file/sname if needed
  * @m:		Message to fill
+ * @overload:	Set to option 52 value (0 if none, 1/2/3 per RFC 2132)
  *
  * Return: current size of options field
  */
-static int fill(struct msg *m)
+static int fill(struct msg *m, int *overload)
 {
+	/* Reserve 3 bytes for option 52 (overload) if needed */
+	size_t size = OPT_MAX - 3;
 	int i, o, offset = 0;
 
 	for (o = 0; o < 255; o++)
@@ -390,20 +440,25 @@ static int fill(struct msg *m)
 	 * Put it there explicitly, unless requested via option 55.
 	 */
 	if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
-		if (fill_one(m->o, OPT_MAX, 53, &offset))
-			 debug("DHCP: skipping option 53");
+		fill_one(m->o, size, 53, &offset);
 
 	for (i = 0; i < opts[55].clen; i++) {
 		o = opts[55].c[i];
 		if (opts[o].slen != -1)
-			if (fill_one(m->o, OPT_MAX, o, &offset))
-				debug("DHCP: skipping option %i", o);
+			fill_one(m->o, size, o, &offset);
 	}
 
 	for (o = 0; o < 255; o++) {
 		if (opts[o].slen != -1 && !opts[o].sent)
-			if (fill_one(m->o, OPT_MAX, o, &offset))
-				debug("DHCP: skipping option %i", o);
+			fill_one(m->o, size, o, &offset);
+	}
+
+	*overload = fill_overflow(m);
+
+	if (*overload) {
+		m->o[offset++] = 52;
+		m->o[offset++] = 1;
+		m->o[offset++] = *overload;
 	}
 
 	m->o[offset++] = 255;
@@ -528,6 +583,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
 	struct msg const *m;
 	struct msg reply;
 	unsigned int i;
+	int overload;
 
 	eh = IOV_REMOVE_HEADER(data, eh_storage);
 	iph = IOV_PEEK_HEADER(data, iph_storage);
@@ -677,9 +733,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
 	}
 
 	if (!c->no_dhcp_dns_search)
-		opt_set_dns_search(c, sizeof(m->o));
+		opt_set_dns_search(c, OPT_MAX - 3);
+
+	for (i = 0; i < (unsigned int)c->custom_opts_count; i++) {
+		uint8_t code = c->custom_opts[i].code;
+
+		opts[code].slen = c->custom_opts[i].len;
+		memcpy(opts[code].s, c->custom_opts[i].val,
+		       c->custom_opts[i].len);
+	}
+
+	dlen = offsetof(struct msg, o) + fill(&reply, &overload);
 
-	dlen = offsetof(struct msg, o) + fill(&reply);
+	/* Copy boot file name into the file field for legacy PXE clients,
+	 * unless the file field is already used for option overload.
+	 */
+	if (!(overload & DHCP_OVERLOAD_FILE) &&
+	    opts[67].slen > 0 && (size_t)opts[67].slen < sizeof(reply.file))
+		memcpy(reply.file, opts[67].s, opts[67].slen);
 
 	if (m->flags & FLAG_BROADCAST)
 		dst = in4addr_broadcast;
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page
  2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
                   ` (4 preceding siblings ...)
  2026-06-01  7:37 ` [PATCH v3 5/6] dhcp: Add option overload Anshu Kumari
@ 2026-06-01  7:37 ` Anshu Kumari
  2026-06-02  2:54   ` David Gibson
  2026-06-11 23:05   ` Stefano Brivio
  5 siblings, 2 replies; 19+ messages in thread
From: Anshu Kumari @ 2026-06-01  7:37 UTC (permalink / raw)
  To: passt-dev, anskuma, sbrivio; +Cc: david, jmaloy, lvivier

Document the new --dhcp-boot and --dhcp-opt command-line options in
the passt(1) man page, including supported option codes grouped by
value type and usage examples.

Link: https://bugs.passt.top/show_bug.cgi?id=192
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v3:
  - Removed options 33, 55, 61, 119 from supported codes list
  - Added note: "If the same name option code is given more than once,
    the last value wins".

v2:
  - Updated --dhcp-boot description.
  - Highlighted cross-referenced options with \fB...\fR.
  - Updated IP list format from "space-separated within quotes" to "comma-separated".
  - option 121 dropped.
  - Added option 55 to string options list.
  - Removed --dhcp-boot override reference from --dhcp-opt description.
---
 passt.1 | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/passt.1 b/passt.1
index 908fd4a..57e2cc1 100644
--- a/passt.1
+++ b/passt.1
@@ -430,6 +430,47 @@ 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 \-\-dhcp-boot " " \fIurl
+Convenience shorthand for \fB\-\-dhcp-opt\fR 67,\fIurl\fR.
+Sets the boot file name (DHCP option 67) for network boot.
+For UEFI HTTP boot, also set the vendor class identifier using
+\fB\-\-dhcp-opt\fR 60,HTTPClient.
+
+.TP
+.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR
+Set a DHCP option by numeric code. The value format is determined automatically
+from the option code. Multiple IPv4 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. Options set with
+\fB\-\-dhcp-opt\fR override built-in values.
+Only the following option codes are supported (unsupported codes cause an error):
+.RS
+.TP
+.B IPv4 address options
+1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 (Requested IP),
+54 (Server Identifier)
+.TP
+.B IPv4 address list options (comma-separated)
+3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server),
+8 (Cookie Server), 9 (LPR Server), 10 (Impress Server),
+11 (Resource Location Server), 41 (NIS Servers),
+42 (NTP Servers), 44 (NetBIOS Name Server)
+.TP
+.B Integer options
+2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP Forwarding, 8-bit),
+23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit),
+37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit),
+51 (IP Address Lease Time, 32-bit),
+53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit),
+58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit)
+.TP
+.B String options
+12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name),
+60 (Vendor Class Identifier), 66 (TFTP Server Name),
+67 (Bootfile Name), 252 (WPAD URL)
+.RE
+
 .TP
 .BR \-t ", " \-\-tcp-ports " " \fIspec
 Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/6] conf: Add --dhcp-opt command-line option
  2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
@ 2026-06-02  2:00   ` David Gibson
  2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:00 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:51PM +0530, Anshu Kumari wrote:
> Introduce the --dhcp-opt flag that allows setting arbitrary DHCP
> options from command-line in the form of [Option CODE,VALUE].
> This patch adds the option storage in struct ctx and CLI parsing;
> the type-aware value parser and DHCP reply injection follow
> in subsequent patches.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

LGTM except for one remaining detail

[snip]
> @@ -1465,6 +1475,18 @@ void conf(struct ctx *c, int argc, char **argv)
>  				die("Can't display statistics if not running in foreground");
>  			c->stats = strtol(optarg, NULL, 0);
>  			break;
> +		case 33:
> +			comma = strchr(optarg, ',');
> +			if (!comma)
> +				die("--dhcp-opt requires CODE,VALUE format");
> +
> +			optcode = strtoul(optarg, &end, 0);

Sorry, missed it on the previous round of review.  This strtoul() also
needs the errno = 0 / check errno trick to catch the handful of error
cases that end != comma won't.

> +			if (end != comma || optcode < 1 || optcode > 254)
> +				die("DHCP option code must be 1-254: %s",
> +				    optarg);
> +
> +			dhcp_add_option(c, optcode, comma + 1);
> +			break;
>  		case 'd':
>  			c->debug = 1;
>  			c->quiet = 0;
> diff --git a/dhcp.c b/dhcp.c
> index 1ff8cba..c5fbf37 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -33,6 +33,44 @@
>  #include "log.h"
>  #include "dhcp.h"
>  
> +
> +/**
> + * dhcp_add_option() - Add or update a custom DHCP option
> + * @c:		Execution context
> + * @code:	DHCP option code
> + * @val_str:	Value string from command line
> + *
> + * If @code was already added, the previous value is overwritten.
> + * Calls die() on any error.
> + *
> + * Return: 0 on success
> + */
> +int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < c->custom_opts_count; idx++) {
> +		if (c->custom_opts[idx].code == code)
> +			break;
> +	}
> +
> +	if (idx == c->custom_opts_count) {
> +		if (c->custom_opts_count >= MAX_CUSTOM_DHCP_OPTS)
> +			die("Too many --dhcp-opt entries (max %d)",
> +			    MAX_CUSTOM_DHCP_OPTS);
> +		c->custom_opts_count++;
> +	}
> +
> +	c->custom_opts[idx].code = code;
> +
> +	if (snprintf_check(c->custom_opts[idx].str,
> +			   sizeof(c->custom_opts[0].str),
> +			   "%s", val_str))
> +		die("DHCP option value too long: %s", val_str);
> +
> +	return 0;
> +}
> +
>  /**
>   * struct opt - DHCP option
>   * @sent:	Convenience flag, set while filling replies
> diff --git a/dhcp.h b/dhcp.h
> index cd50c99..9c8f1e3 100644
> --- a/dhcp.h
> +++ b/dhcp.h
> @@ -8,5 +8,6 @@
>  
>  int dhcp(const struct ctx *c, struct iov_tail *data);
>  void dhcp_init(void);
> +int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str);
>  
>  #endif /* DHCP_H */
> diff --git a/passt.h b/passt.h
> index 1726965..3a0816f 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -182,6 +182,10 @@ struct ip6_ctx {
>   * @dns_search:		DNS search list
>   * @hostname:		Guest hostname
>   * @fqdn:		Guest FQDN
> + * @custom_opts:	User-specified DHCP options from --dhcp-opt
> + * @custom_opts.code:	DHCP option code
> + * @custom_opts.str:	Original string value from command line
> + * @custom_opts_count:	Number of entries in @custom_opts
>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
>   * @ip6:		IPv6 configuration
>   * @pasta_ifn:		Name of namespace interface for pasta
> @@ -263,6 +267,14 @@ struct ctx {
>  	char hostname[PASST_MAXDNAME];
>  	char fqdn[PASST_MAXDNAME];
>  
> +#define MAX_CUSTOM_DHCP_OPTS	32
> +
> +	struct {
> +		uint8_t code;
> +		char str[256];
> +	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
> +	int custom_opts_count;
> +
>  	int ifi6;
>  	struct ip6_ctx ip6;
>  
> -- 
> 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/6] conf: Add --dhcp-boot command-line option
  2026-06-01  7:37 ` [PATCH v3 2/6] conf: Add --dhcp-boot " Anshu Kumari
@ 2026-06-02  2:02   ` David Gibson
  2026-06-11 23:05   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:02 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:52PM +0530, Anshu Kumari wrote:
> Introduce the --dhcp-boot flag that sets the boot file URL for
> network boot specially for ipxe. This patch adds the option
> storage and CLI parsing.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v3:
>   - case 32 now calls dhcp_add_option(c, 67, optarg).
>   - Handles duplicate codes: --dhcp-boot and --dhcp-opt 67 coexist
>     correctly, last value wins.
> 
> v2:
>   - Removed separate dhcp_boot[PATH_MAX] field — --dhcp-boot foo now stores into custom_opts[] as code 67 (same as --dhcp-opt 67,foo)
> 
> ---
>  conf.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/conf.c b/conf.c
> index ce78af1..f7281e2 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -618,6 +618,7 @@ static void usage(const char *name, FILE *f, int status)
>  		"    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"
> +		"  --dhcp-boot URL	Boot file URL for network boot\n"
>  		"  --dhcp-opt CODE,VAL	Set DHCP option by code\n");
>  	if (strstr(name, "pasta"))
>  		FPRINTF(f, "    default: don't use any search list\n");
> @@ -1239,6 +1240,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"migrate-no-linger", no_argument,	NULL,		30 },
>  		{"stats", required_argument,		NULL,		31 },
>  		{"conf-path",	required_argument,	NULL,		'c' },
> +		{"dhcp-boot", required_argument,	NULL,		32 },
>  		{"dhcp-opt", required_argument,		NULL,		33 },
>  		{ 0 },
>  	};
> @@ -1475,6 +1477,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  				die("Can't display statistics if not running in foreground");
>  			c->stats = strtol(optarg, NULL, 0);
>  			break;
> +		case 32:
> +			dhcp_add_option(c, 67, optarg);
> +			break;
>  		case 33:
>  			comma = strchr(optarg, ',');
>  			if (!comma)
> -- 
> 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/6] dhcp: Add option type table and value parser
  2026-06-01  7:37 ` [PATCH v3 3/6] dhcp: Add option type table and value parser Anshu Kumari
@ 2026-06-02  2:23   ` David Gibson
  2026-06-11 23:04     ` Stefano Brivio
  2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 1 reply; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:23 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:53PM +0530, Anshu Kumari wrote:
> Add an RFC 2132 type lookup table mapping DHCP option codes to their
> expected value formats, and a dhcp_opt_parse() function that converts
> CLI string values into their binary wire representation.
> 
> Wire dhcp_opt_parse() into dhcp_add_option() so that values are
> validated and encoded at configuration time.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32
>     enums, removed dhcp_opt_int_width[] array.
>   - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse
>     both as list, error if >1 in single case.
>   - Added errno = 0 before strtoul() and check after.
>   - Fixed range check: 1ULL << (width * 8) for all widths including
>     width==4.
>   - strncpy → memcpy for DHCP_OPT_STR.
>   - Moved enum to dhcp.c since not used in other files.
>   - Removed options 55, 61 (client-only), 119 (DNS compression, use
>     --dhcp-search instead), 33 (IP pairs not supported).
>   - DHCP_OPT_PARSE_BUF 1024 → char tmp[256].
>   - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate
>     val[]/len.
>   - Aligned array entries for readability.
>   - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc.
>   - Reject empty value strings before parsing
>   - Reject leading/trailing/consecutive commas in IP list values.

Thanks for the detailed changelogs, by the way.  I know these are a
bunch of work to maintain, but they really help when reviewing.

> v2:
>   - Replaced struct lookup table + dhcp_opt_type_lookup() function with flat dhcp_opt_types[256] array indexed by code.
>   - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table.
>   - Dropped DHCP_OPT_ROUTES / option 121 entirely.
>   - Added kerneldoc for enum dhcp_opt_type values.
>   - Removed curly braces from switch cases, declarations before switch.
>   - Added newlines before return statements.
>   - Changed IP list delimiter from space to comma (--dhcp-opt 6,1.1.1.1,8.8.8.8).
>   - Defined DHCP_OPT_PARSE_BUF constant for bare 1024.
>   - Added len and val[255] fields to struct here (moved from patch 1).
>   - Added kerneldoc for @custom_opts.len and @custom_opts.val.
>   - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate val/len.
> ---
>  dhcp.c  | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  passt.h |   4 ++
>  2 files changed, 181 insertions(+), 3 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index c5fbf37..07a42b9 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -23,6 +23,7 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <limits.h>
> +#include <errno.h>
>  
>  #include "util.h"
>  #include "ip.h"
> @@ -33,6 +34,170 @@
>  #include "log.h"
>  #include "dhcp.h"
>  
> +/**
> + * enum dhcp_opt_type - DHCP option value types per RFC 2132
> + * @DHCP_OPT_NONE:	Unsupported or unknown option
> + * @DHCP_OPT_STR:	Variable-length string
> + * @DHCP_OPT_IPV4:	Single IPv4 address
> + * @DHCP_OPT_IPV4_LIST:	Multiple IPv4 addresses, comma-separated
> + * @DHCP_OPT_INT8:	Unsigned 8-bit integer
> + * @DHCP_OPT_INT16:	Unsigned 16-bit integer
> + * @DHCP_OPT_INT32:	Unsigned 32-bit integer
> + */
> +enum dhcp_opt_type {
> +	DHCP_OPT_NONE,
> +	DHCP_OPT_STR,
> +	DHCP_OPT_IPV4,
> +	DHCP_OPT_IPV4_LIST,
> +	DHCP_OPT_INT8,
> +	DHCP_OPT_INT16,
> +	DHCP_OPT_INT32,
> +};
> +
> +/**
> + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code
> + */
> +static const enum dhcp_opt_type dhcp_opt_types[256] = {
> +	[1]   = DHCP_OPT_IPV4,		/* Subnet Mask */
> +	[2]   = DHCP_OPT_INT32,		/* Time Offset */
> +	[3]   = DHCP_OPT_IPV4_LIST,	/* Router */

I'm still a bit unsure if we want to allow user modification of the
options, like this one, which we already manage ourselves.

> +	[4]   = DHCP_OPT_IPV4_LIST,	/* Time Server */
> +	[5]   = DHCP_OPT_IPV4_LIST,	/* Name Server */
> +	[6]   = DHCP_OPT_IPV4_LIST,	/* Domain Name Server */
> +	[7]   = DHCP_OPT_IPV4_LIST,	/* Log Server */
> +	[8]   = DHCP_OPT_IPV4_LIST,	/* Cookie Server */
> +	[9]   = DHCP_OPT_IPV4_LIST,	/* LPR Server */
> +	[10]  = DHCP_OPT_IPV4_LIST,	/* Impress Server */
> +	[11]  = DHCP_OPT_IPV4_LIST,	/* Resource Location Server */
> +	[12]  = DHCP_OPT_STR,		/* Host Name */
> +	[13]  = DHCP_OPT_INT16,		/* Boot File Size */
> +	[15]  = DHCP_OPT_STR,		/* Domain Name */
> +	[16]  = DHCP_OPT_IPV4,		/* Swap Server */
> +	[17]  = DHCP_OPT_STR,		/* Root Path */
> +	[19]  = DHCP_OPT_INT8,		/* IP Forwarding */
> +	[23]  = DHCP_OPT_INT8,		/* Default IP TTL */
> +	[26]  = DHCP_OPT_INT16,		/* Interface MTU */
> +	[28]  = DHCP_OPT_IPV4,		/* Broadcast Address */
> +	[37]  = DHCP_OPT_INT8,		/* TCP Default TTL */
> +	[38]  = DHCP_OPT_INT32,		/* TCP Keepalive Interval */
> +	[40]  = DHCP_OPT_STR,		/* NIS Domain Name */
> +	[41]  = DHCP_OPT_IPV4_LIST,	/* NIS Servers */
> +	[42]  = DHCP_OPT_IPV4_LIST,	/* NTP Servers */
> +	[44]  = DHCP_OPT_IPV4_LIST,	/* NetBIOS Name Server */
> +	[50]  = DHCP_OPT_IPV4,		/* Requested IP Address */
> +	[51]  = DHCP_OPT_INT32,		/* IP Address Lease Time */
> +	[53]  = DHCP_OPT_INT8,		/* DHCP Message Type */
> +	[54]  = DHCP_OPT_IPV4,		/* Server Identifier */
> +	[57]  = DHCP_OPT_INT16,		/* Max DHCP Message Size */
> +	[58]  = DHCP_OPT_INT32,		/* Renewal (T1) Time */
> +	[59]  = DHCP_OPT_INT32,		/* Rebinding (T2) Time */
> +	[60]  = DHCP_OPT_STR,		/* Vendor Class Identifier */
> +	[66]  = DHCP_OPT_STR,		/* TFTP Server Name */
> +	[67]  = DHCP_OPT_STR,		/* Bootfile Name */
> +	[252] = DHCP_OPT_STR,		/* WPAD URL */
> +};
> +
> +/**
> + * dhcp_opt_parse() - Parse a DHCP option value
> + * @code:	DHCP 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 dhcp_opt_parse(uint8_t code, const char *str,
> +			  uint8_t *buf, size_t buf_len)
> +{
> +	enum dhcp_opt_type type = dhcp_opt_types[code];
> +	char *tok, *saveptr, *end;
> +	struct in_addr addr;
> +	unsigned long val;
> +	unsigned int i;
> +	uint8_t width;
> +	char tmp[256];
> +	size_t slen;
> +	int len;
> +
> +	if (!*str)
> +		die("Empty value for DHCP option %u", code);
> +
> +	switch (type) {
> +	case DHCP_OPT_NONE:
> +		die("Unsupported DHCP option: %u,"
> +		    " see passt(1) for supported codes", code);
> +	case DHCP_OPT_IPV4:
> +	case DHCP_OPT_IPV4_LIST:
> +		len = 0;
> +
> +		/* Reject empty, leading/trailing, or consecutive commas */
> +		if (!*str || *str == ',' || str[strlen(str) - 1] == ',' ||
> +		    strstr(str, ",,"))
> +			return -1;
> +
> +		if (snprintf_check(tmp, sizeof(tmp), "%s", str))
> +			return -1;

The arbitrary 256 byte buffer limit here isn't great.  The string
encoding of an IPv4 address can be nearly 4 times as long as the
binary encoding, so we could potentially hit this with a longish, but
not ridiculous address list.

> +		for (tok = strtok_r(tmp, ",", &saveptr); tok;
> +		     tok = strtok_r(NULL, ",", &saveptr)) {

One way to avoid that would be to avoid using strtok_r() which relies
in in-place modifying the input.  Instead you'd need to repeatedly
find the length of the next chunk with strchr() or strcpsn(), then
extract each one into a tmp of length INET_ADDRSTRLEN to call
inet_pton().  On the plus side, that should naturally deal with the
case of extraneous commas (it would show up as an empth entry), rather
than requiring an explicit check at the top.


> +			if (inet_pton(AF_INET, tok, &addr) != 1)
> +				return -1;
> +
> +			if (len + (int)sizeof(addr) > (int)buf_len)
> +				return -1;

You could make this check before the inet_pton(), then do the
conversion directly into buf, avoiding the addr temporary.

> +
> +			memcpy(buf + len, &addr, sizeof(addr));
> +			len += sizeof(addr);
> +
> +			if (type == DHCP_OPT_IPV4)
> +				break;
> +		}
> +
> +		if (type == DHCP_OPT_IPV4 && strtok_r(NULL, ",", &saveptr))
> +			return -1;
> +
> +		return len;
> +	case DHCP_OPT_INT8:
> +	case DHCP_OPT_INT16:
> +	case DHCP_OPT_INT32:
> +		if (type == DHCP_OPT_INT8)
> +			width = 1;
> +		else if (type == DHCP_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 DHCP_OPT_STR:
> +		slen = strlen(str);
> +
> +		if (!slen || slen >= buf_len)
> +			return -1;
> +
> +		memcpy(buf, str, slen);

Do you need to include the terminating \0 here?  If so you'll need
slen + 1.

> +
> +		return slen;
> +	}
> +
> +	return -1;
> +}
>  
>  /**
>   * dhcp_add_option() - Add or update a custom DHCP option
> @@ -40,14 +205,15 @@
>   * @code:	DHCP option code
>   * @val_str:	Value string from command line
>   *
> - * If @code was already added, the previous value is overwritten.
> - * Calls die() on any error.
> + * Parses @val_str according to the type registered for @code in
> + * dhcp_opt_types[]. If @code was already added, the previous value
> + * is overwritten. Calls die() on any error.
>   *
>   * Return: 0 on success
>   */
>  int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
>  {
> -	int idx;
> +	int idx, ret;
>  
>  	for (idx = 0; idx < c->custom_opts_count; idx++) {
>  		if (c->custom_opts[idx].code == code)
> @@ -61,7 +227,15 @@ int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
>  		c->custom_opts_count++;
>  	}
>  
> +	ret = dhcp_opt_parse(code, val_str,
> +			     c->custom_opts[idx].val,
> +			     sizeof(c->custom_opts[0].val));

Now that this parsing and adding code is all in dhcp.c, could we parse
the options directly into the existing opts[] global, rather than
requiring both the string and parsed forms in c->custom_opts?

> +	if (ret < 0)
> +		die("Invalid value for DHCP option %u: %s",
> +		    code, val_str);
> +
>  	c->custom_opts[idx].code = code;
> +	c->custom_opts[idx].len = ret;
>  
>  	if (snprintf_check(c->custom_opts[idx].str,
>  			   sizeof(c->custom_opts[0].str),
> diff --git a/passt.h b/passt.h
> index 3a0816f..751fee3 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -184,6 +184,8 @@ struct ip6_ctx {
>   * @fqdn:		Guest FQDN
>   * @custom_opts:	User-specified DHCP options from --dhcp-opt
>   * @custom_opts.code:	DHCP option code
> + * @custom_opts.len:	Length of binary value in @val
> + * @custom_opts.val:	Binary-encoded option value
>   * @custom_opts.str:	Original string value from command line
>   * @custom_opts_count:	Number of entries in @custom_opts
>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
> @@ -271,6 +273,8 @@ struct ctx {
>  
>  	struct {
>  		uint8_t code;
> +		uint8_t len;
> +		uint8_t val[255];
>  		char str[256];
>  	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
>  	int custom_opts_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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer
  2026-06-01  7:37 ` [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari
@ 2026-06-02  2:25   ` David Gibson
  0 siblings, 0 replies; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:25 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:54PM +0530, Anshu Kumari wrote:
> Change fill_one() to accept a buffer pointer and capacity instead of
> a struct msg pointer.  This is a pure refactor with no behavior change,
> preparing for option overload support where fill_one() will also write
> into the file and sname fields.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v3:
>   - Restored removed comments: "If we don't have space to write the
>     option, then just skip" and "Move to option".
> 
> v2:
>   - Renamed parameter cap → size.
> ---
>  dhcp.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index 07a42b9..5c6a492 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -343,28 +343,29 @@ struct msg {
>  } __attribute__((__packed__));
>  
>  /**
> - * fill_one() - Fill a single option in message
> - * @m:		Message to fill
> + * fill_one() - Fill a single option into a buffer
> + * @buf:	Buffer to write option
> + * @size:	Usable size of @buf (excluding end marker)
>   * @o:		Option number
> - * @offset:	Current offset within options field, updated on insertion
> + * @offset:	Current offset within @buf, updated on insertion
>   *
> - * Return: false if m has space to write the option, true otherwise
> + * Return: false if @buf has space to write the option, true otherwise
>   */
> -static bool fill_one(struct msg *m, int o, int *offset)
> +static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
>  {
>  	size_t slen = opts[o].slen;
>  
>  	/* If we don't have space to write the option, then just skip */
> -	if (*offset + 2 /* code and length of option */ + slen > OPT_MAX)
> +	if (*offset + 2 + slen > size)
>  		return true;
>  
> -	m->o[*offset] = o;
> -	m->o[*offset + 1] = slen;
> +	buf[*offset] = o;
> +	buf[*offset + 1] = slen;
>  
>  	/* Move to option */
>  	*offset += 2;
>  
> -	memcpy(&m->o[*offset], opts[o].s, slen);
> +	memcpy(&buf[*offset], opts[o].s, slen);
>  
>  	opts[o].sent = 1;
>  	*offset += slen;
> @@ -389,19 +390,19 @@ static int fill(struct msg *m)
>  	 * Put it there explicitly, unless requested via option 55.
>  	 */
>  	if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
> -		if (fill_one(m, 53, &offset))
> +		if (fill_one(m->o, OPT_MAX, 53, &offset))
>  			 debug("DHCP: skipping option 53");
>  
>  	for (i = 0; i < opts[55].clen; i++) {
>  		o = opts[55].c[i];
>  		if (opts[o].slen != -1)
> -			if (fill_one(m, o, &offset))
> +			if (fill_one(m->o, OPT_MAX, o, &offset))
>  				debug("DHCP: skipping option %i", o);
>  	}
>  
>  	for (o = 0; o < 255; o++) {
>  		if (opts[o].slen != -1 && !opts[o].sent)
> -			if (fill_one(m, o, &offset))
> +			if (fill_one(m->o, OPT_MAX, o, &offset))
>  				debug("DHCP: skipping option %i", o);
>  	}
>  
> -- 
> 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 5/6] dhcp: Add option overload
  2026-06-01  7:37 ` [PATCH v3 5/6] dhcp: Add option overload Anshu Kumari
@ 2026-06-02  2:50   ` David Gibson
  2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:50 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:55PM +0530, Anshu Kumari wrote:
> A user can enter lots of options in command-line which may not fit in
> existing buffer, So when the options field is full, overflow remaining
> DHCP options into the file and sname fields per RFC 2132 option 52.
> 
> Also, when the file field is not used for overload, copy the boot
> file URL there directly for legacy PXE clients.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

For the code proper.  The commit message is perhaps buries the lede in
that in that this is also the patch that actually starts putting the
custom_opts into DHCP replies.  Mind you, that would change if we
parsed directly into opts[], which might be nicer anyway.


> ---
> v3:
>   - Added RFC 2132 Section 9.3 reference comment on overload
>     constants.
>   - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow().
>   - Swapped overflow order: try sname (64 bytes) first, then file
>     (128 bytes) — better packing and keeps file field available for
>     boot file name.
>   - Removed '&' from &reply.file.
>   - Removed '+1' from memcpy — reply.file already zeroed.
>   - opt_set_dns_search() max_len: OPT_MAX - 3 instead of
>     sizeof(m->o).
> 
> v2:
>   - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants
>   - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */
>   - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname
>   - Removed dhcp_boot references — reply.file copy now reads from opts[67]
>   - Used DHCP_OVERLOAD_FILE constant in reply.file guard
> ---
>  dhcp.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 10 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index 5c6a492..a9d56fd 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -372,14 +372,64 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
>  	return false;
>  }
>  
> +/* RFC 2132, Section 9.3 - Option Overload*/
> +#define DHCP_OVERLOAD_FILE	1
> +#define DHCP_OVERLOAD_SNAME	2
> +
>  /**
> - * fill() - Fill options in message
> + * fill_overflow() - Fill remaining options into sname and file fields
> + * @m:		Message whose sname/file fields may be used for overflow
> + *
> + * Try the smaller sname field first: small options go there, leaving
> + * the larger file field available for big options and for the boot
> + * file name (option 67) if set.
> + *
> + * Return: option 52 overload value: 0 if no overflow,
> + *         DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname,
> + *         or both OR'd together
> + */
> +static int fill_overflow(struct msg *m)
> +{
> +	int sname_off = 0, file_off = 0, overload = 0;
> +	int o;
> +
> +	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
> +		if (opts[o].slen == -1 || opts[o].sent)
> +			continue;
> +		fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off);
> +	}
> +
> +	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
> +		if (opts[o].slen == -1 || opts[o].sent)
> +			continue;
> +		if (fill_one(m->file, sizeof(m->file) - 1, o, &file_off))
> +			debug("DHCP: skipping option %i (overload full)", o);
> +	}
> +
> +	if (sname_off) {
> +		m->sname[sname_off] = 255;
> +		overload |= DHCP_OVERLOAD_SNAME;
> +	}
> +
> +	if (file_off) {
> +		m->file[file_off] = 255;
> +		overload |= DHCP_OVERLOAD_FILE;
> +	}
> +
> +	return overload;
> +}
> +
> +/**
> + * fill() - Fill options in message, with overload into file/sname if needed
>   * @m:		Message to fill
> + * @overload:	Set to option 52 value (0 if none, 1/2/3 per RFC 2132)
>   *
>   * Return: current size of options field
>   */
> -static int fill(struct msg *m)
> +static int fill(struct msg *m, int *overload)
>  {
> +	/* Reserve 3 bytes for option 52 (overload) if needed */
> +	size_t size = OPT_MAX - 3;
>  	int i, o, offset = 0;
>  
>  	for (o = 0; o < 255; o++)
> @@ -390,20 +440,25 @@ static int fill(struct msg *m)
>  	 * Put it there explicitly, unless requested via option 55.
>  	 */
>  	if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
> -		if (fill_one(m->o, OPT_MAX, 53, &offset))
> -			 debug("DHCP: skipping option 53");
> +		fill_one(m->o, size, 53, &offset);
>  
>  	for (i = 0; i < opts[55].clen; i++) {
>  		o = opts[55].c[i];
>  		if (opts[o].slen != -1)
> -			if (fill_one(m->o, OPT_MAX, o, &offset))
> -				debug("DHCP: skipping option %i", o);
> +			fill_one(m->o, size, o, &offset);
>  	}
>  
>  	for (o = 0; o < 255; o++) {
>  		if (opts[o].slen != -1 && !opts[o].sent)
> -			if (fill_one(m->o, OPT_MAX, o, &offset))
> -				debug("DHCP: skipping option %i", o);
> +			fill_one(m->o, size, o, &offset);
> +	}
> +
> +	*overload = fill_overflow(m);
> +
> +	if (*overload) {
> +		m->o[offset++] = 52;
> +		m->o[offset++] = 1;
> +		m->o[offset++] = *overload;
>  	}
>  
>  	m->o[offset++] = 255;
> @@ -528,6 +583,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	struct msg const *m;
>  	struct msg reply;
>  	unsigned int i;
> +	int overload;
>  
>  	eh = IOV_REMOVE_HEADER(data, eh_storage);
>  	iph = IOV_PEEK_HEADER(data, iph_storage);
> @@ -677,9 +733,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	}
>  
>  	if (!c->no_dhcp_dns_search)
> -		opt_set_dns_search(c, sizeof(m->o));
> +		opt_set_dns_search(c, OPT_MAX - 3);
> +
> +	for (i = 0; i < (unsigned int)c->custom_opts_count; i++) {
> +		uint8_t code = c->custom_opts[i].code;
> +
> +		opts[code].slen = c->custom_opts[i].len;
> +		memcpy(opts[code].s, c->custom_opts[i].val,
> +		       c->custom_opts[i].len);
> +	}
> +
> +	dlen = offsetof(struct msg, o) + fill(&reply, &overload);
>  
> -	dlen = offsetof(struct msg, o) + fill(&reply);
> +	/* Copy boot file name into the file field for legacy PXE clients,
> +	 * unless the file field is already used for option overload.
> +	 */
> +	if (!(overload & DHCP_OVERLOAD_FILE) &&
> +	    opts[67].slen > 0 && (size_t)opts[67].slen < sizeof(reply.file))
> +		memcpy(reply.file, opts[67].s, opts[67].slen);
>  
>  	if (m->flags & FLAG_BROADCAST)
>  		dst = in4addr_broadcast;
> -- 
> 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page
  2026-06-01  7:37 ` [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page Anshu Kumari
@ 2026-06-02  2:54   ` David Gibson
  2026-06-11 23:05   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: David Gibson @ 2026-06-02  2:54 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, sbrivio, jmaloy, lvivier

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

On Mon, Jun 01, 2026 at 01:07:56PM +0530, Anshu Kumari wrote:
> Document the new --dhcp-boot and --dhcp-opt command-line options in
> the passt(1) man page, including supported option codes grouped by
> value type and usage examples.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>

Note that as a general rule, I think it's preferable to include
manpage / doc updates with the patch that implements the code change.
That can get fuzzy when the implementation is across several patches
of course.  In any case, not worth reworking here.

These options should probably also be mentioned in usage() output
(though with a much briefer description).

> ---
> v3:
>   - Removed options 33, 55, 61, 119 from supported codes list
>   - Added note: "If the same name option code is given more than once,
>     the last value wins".
> 
> v2:
>   - Updated --dhcp-boot description.
>   - Highlighted cross-referenced options with \fB...\fR.
>   - Updated IP list format from "space-separated within quotes" to "comma-separated".
>   - option 121 dropped.
>   - Added option 55 to string options list.
>   - Removed --dhcp-boot override reference from --dhcp-opt description.
> ---
>  passt.1 | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/passt.1 b/passt.1
> index 908fd4a..57e2cc1 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -430,6 +430,47 @@ 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 \-\-dhcp-boot " " \fIurl
> +Convenience shorthand for \fB\-\-dhcp-opt\fR 67,\fIurl\fR.
> +Sets the boot file name (DHCP option 67) for network boot.
> +For UEFI HTTP boot, also set the vendor class identifier using
> +\fB\-\-dhcp-opt\fR 60,HTTPClient.
> +
> +.TP
> +.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR
> +Set a DHCP option by numeric code. The value format is determined automatically
> +from the option code. Multiple IPv4 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. Options set with
> +\fB\-\-dhcp-opt\fR override built-in values.
> +Only the following option codes are supported (unsupported codes cause an error):
> +.RS
> +.TP
> +.B IPv4 address options
> +1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 (Requested IP),
> +54 (Server Identifier)
> +.TP
> +.B IPv4 address list options (comma-separated)
> +3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server),
> +8 (Cookie Server), 9 (LPR Server), 10 (Impress Server),
> +11 (Resource Location Server), 41 (NIS Servers),
> +42 (NTP Servers), 44 (NetBIOS Name Server)
> +.TP
> +.B Integer options
> +2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP Forwarding, 8-bit),
> +23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit),
> +37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit),
> +51 (IP Address Lease Time, 32-bit),
> +53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit),
> +58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit)
> +.TP
> +.B String options
> +12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name),
> +60 (Vendor Class Identifier), 66 (TFTP Server Name),
> +67 (Bootfile Name), 252 (WPAD URL)
> +.RE
> +
>  .TP
>  .BR \-t ", " \-\-tcp-ports " " \fIspec
>  Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:
> -- 
> 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 --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 1/6] conf: Add --dhcp-opt command-line option
  2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
  2026-06-02  2:00   ` David Gibson
@ 2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:04 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, david, jmaloy, lvivier

Sorry for the late review.

I have a few remarks on top of the one from David (which is the only one
left that's really critical, I guess):

On Mon,  1 Jun 2026 13:07:51 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> Introduce the --dhcp-opt flag that allows setting arbitrary DHCP
> options from command-line in the form of [Option CODE,VALUE].
> This patch adds the option storage in struct ctx and CLI parsing;
> the type-aware value parser and DHCP reply injection follow
> in subsequent patches.

This split makes the patch smaller, but not necessarily easier to
review, or maybe actually harder:

- as David noted, it would be preferable to have man page changes
  together with functional changes they relate to, but not just for
  correctness: I personally use those during reviews to double check if
  the implementation corresponds to the intention.

  That is, in some sense, I use man pages as specification, which is
  particularly fitting when it comes to new command line options. So,
  from my side, while it's not really a blocker, my preference to have
  those man page changes together with the patch implementing the code
  changes is probably a bit stronger than the one expressed by David.

  It's not just for review right now, it also helps investigation later
  when somebody finds issues: having a more comprehensive description
  in this patch itself means not having to correlate multiple patches.

  Think of bisecting an issue and finding that this patch breaks
  something: git reaches this patch, and now you don't have the man
  page in the checked out tree at all...

- I've been asking myself: is dhcp_add_option() matter for conf.c,
  rather than for dhcp.c? That is, is it merely a matter of
  configuration parsing / handling, or a part of the DHCP
  implementation proper?

  I needed to reach 3/6 and actually grasp 3/6 to answer this
  question... which seems to be a good indication that this change
  belongs to the same patch as 3/6. And I had similar fundamental
  questions around this patch that I could only resolve by reading 3/6.

  For me, a more reasonable split would have been something like:

  - 1/4 dhcp: Refactor fill_one() to operate on a generic buffer

    it's a dependency for 3/4 but doesn't depend on others

  - 2/4 dhcp: Add option overload

    also a dependency of 3/4, it only depends on 1/4 anyway

  - 3/4 dhcp: Add --dhcp-opt with option table and value parser

    the main feature, with man page for --dhcp-opt, and a clear match
    between what you parse from conf() and how it's used

  - 4/4 conf: Add --dhcp-boot command-line option

    with its part of man page, as it depends on 3/4 but no other bits
    seem to depend on it

...I'm not sure how much effort that is at this point, but I think it
would be nice for the revision history (current review, doesn't matter
so much, as both David and myself are now familiar with it).

Some nits below:

> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - Added dhcp_add_option() helper in dhcp.c for storing options with
>     duplicate-code detection.
>   - case 33 now calls dhcp_add_option() instead of inline storage.
> 
> v2:
>   - Added kerneldoc for @custom_opts, @custom_opts.code, @custom_opts.str, and @custom_opts_count in struct ctx
>   - Removed len and val[255] fields from struct (moved to patch 3)
>   - Removed braces from case 33, moved declarations (optcode, comma, end) to function scope
>   - Renamed code → optcode to follow function-scope convention.
> ---
>  conf.c  | 24 +++++++++++++++++++++++-
>  dhcp.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  dhcp.h  |  1 +
>  passt.h | 12 ++++++++++++
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/conf.c b/conf.c
> index 029b9c7..ce78af1 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -47,6 +47,7 @@
>  #include "lineread.h"
>  #include "isolation.h"
>  #include "log.h"
> +#include "dhcp.h"
>  #include "vhost_user.h"
>  #include "epoll_ctl.h"
>  #include "conf.h"
> @@ -616,7 +617,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"
> +		"  --dhcp-opt CODE,VAL	Set DHCP option by code\n");

What's VAL? It becomes obvious after some thinking but perhaps this
would be clearer:

		"  --dhcp-opt CODE,VAL	Set DHCP option CODE to VAL\n");

>  	if (strstr(name, "pasta"))
>  		FPRINTF(f, "    default: don't use any search list\n");
>  	else
> @@ -844,6 +846,10 @@ static void conf_print(const struct ctx *c)
>  			info("    router: %s",
>  			     inet_ntop(AF_INET, &c->ip4.guest_gw,
>  				       buf, sizeof(buf)));
> +			for (i = 0; i < c->custom_opts_count; i++)
> +				info("    option %u: %s",
> +				     c->custom_opts[i].code,
> +				     c->custom_opts[i].str);
>  		}
>  
>  		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
> @@ -1233,6 +1239,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"migrate-no-linger", no_argument,	NULL,		30 },
>  		{"stats", required_argument,		NULL,		31 },
>  		{"conf-path",	required_argument,	NULL,		'c' },
> +		{"dhcp-opt", required_argument,		NULL,		33 },
>  		{ 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:";
> @@ -1248,10 +1255,13 @@ void conf(struct ctx *c, int argc, char **argv)
>  	uint8_t prefix_len_from_opt = 0;
>  	unsigned int ifi4 = 0, ifi6 = 0;
>  	const char *logfile = NULL;
> +	unsigned long optcode;
>  	char *runas = NULL;
>  	size_t logsize = 0;
> +	const char *comma;
>  	long fd_tap_opt;
>  	int name, ret;
> +	char *end;
>  	uid_t uid;
>  	gid_t gid;
>  	
> @@ -1465,6 +1475,18 @@ void conf(struct ctx *c, int argc, char **argv)
>  				die("Can't display statistics if not running in foreground");
>  			c->stats = strtol(optarg, NULL, 0);
>  			break;
> +		case 33:
> +			comma = strchr(optarg, ',');
> +			if (!comma)
> +				die("--dhcp-opt requires CODE,VALUE format");
> +
> +			optcode = strtoul(optarg, &end, 0);
> +			if (end != comma || optcode < 1 || optcode > 254)
> +				die("DHCP option code must be 1-254: %s",
> +				    optarg);
> +
> +			dhcp_add_option(c, optcode, comma + 1);
> +			break;
>  		case 'd':
>  			c->debug = 1;
>  			c->quiet = 0;
> diff --git a/dhcp.c b/dhcp.c
> index 1ff8cba..c5fbf37 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -33,6 +33,44 @@
>  #include "log.h"
>  #include "dhcp.h"
>  
> +
> +/**
> + * dhcp_add_option() - Add or update a custom DHCP option

It's not clear where it's added, which is rather fundamental (to a
reply message or to the configuration?)

"Set" can replace "Add or update", and "custom" is not really important
or well defined I think (what makes an option custom? The fact that
it's not assigned by IANA or the fact that it's specified by the user?
But then what's not custom...?).

So, maybe... "Set value for a DHCP option in configuration"?

And looking at this, I guess conf_dhcp_option() in conf.c would be a
better fit.

> + * @c:		Execution context
> + * @code:	DHCP option code
> + * @val_str:	Value string from command line

@str would be equally expressive I think, but I'm fine with both
(slightly less typing for 'str' though).

> + *
> + * If @code was already added, the previous value is overwritten.
> + * Calls die() on any error.
> + *
> + * Return: 0 on success
> + */
> +int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < c->custom_opts_count; idx++) {
> +		if (c->custom_opts[idx].code == code)
> +			break;
> +	}
> +
> +	if (idx == c->custom_opts_count) {
> +		if (c->custom_opts_count >= MAX_CUSTOM_DHCP_OPTS)
> +			die("Too many --dhcp-opt entries (max %d)",
> +			    MAX_CUSTOM_DHCP_OPTS);

What's the rationale of this, though? You now have option overload and
a ("working") table with 256 slots, one might actually try to set more
than 32 options I think. If not, showing the calculation you made for
MAX_CUSTOM_DHCP_OPTS in its #define directive would make that clear.

If you're trying to save some memory in struct ctx by storing only
32 entries in the configuration, note that, as it's now moving to the
data segment (see Jon's "[PATCH v3] util, passt: Close daemon-lifetime
fds on exit to avoid Coverity warning") as long as you have a block of
zeroed bytes that's large enough to matter, that's going to take no
memory at all.

> +		c->custom_opts_count++;
> +	}
> +
> +	c->custom_opts[idx].code = code;
> +
> +	if (snprintf_check(c->custom_opts[idx].str,
> +			   sizeof(c->custom_opts[0].str),
> +			   "%s", val_str))
> +		die("DHCP option value too long: %s", val_str);
> +
> +	return 0;
> +}
> +
>  /**
>   * struct opt - DHCP option
>   * @sent:	Convenience flag, set while filling replies
> diff --git a/dhcp.h b/dhcp.h
> index cd50c99..9c8f1e3 100644
> --- a/dhcp.h
> +++ b/dhcp.h
> @@ -8,5 +8,6 @@
>  
>  int dhcp(const struct ctx *c, struct iov_tail *data);
>  void dhcp_init(void);
> +int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str);
>  
>  #endif /* DHCP_H */
> diff --git a/passt.h b/passt.h
> index 1726965..3a0816f 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -182,6 +182,10 @@ struct ip6_ctx {
>   * @dns_search:		DNS search list
>   * @hostname:		Guest hostname
>   * @fqdn:		Guest FQDN
> + * @custom_opts:	User-specified DHCP options from --dhcp-opt

I think this should be called @dhcp_opts, because @custom_opts in
the... context of ctx isn't really clear. They're all custom anyway in
some sense.

> + * @custom_opts.code:	DHCP option code
> + * @custom_opts.str:	Original string value from command line

It's the only one, there isn't one that's original and one that isn't
(right?).

> + * @custom_opts_count:	Number of entries in @custom_opts

And if you allow for 256 items here, you would skip this and the whole
complexity.

>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
>   * @ip6:		IPv6 configuration
>   * @pasta_ifn:		Name of namespace interface for pasta
> @@ -263,6 +267,14 @@ struct ctx {
>  	char hostname[PASST_MAXDNAME];
>  	char fqdn[PASST_MAXDNAME];
>  
> +#define MAX_CUSTOM_DHCP_OPTS	32
> +
> +	struct {
> +		uint8_t code;
> +		char str[256];

This should be 255 characters long, otherwise it might suggest to
others that it's NULL-terminated, which is a rather dangerous
assumption.

> +	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
> +	int custom_opts_count;
> +
>  	int ifi6;
>  	struct ip6_ctx ip6;
>  

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/6] dhcp: Add option type table and value parser
  2026-06-02  2:23   ` David Gibson
@ 2026-06-11 23:04     ` Stefano Brivio
  0 siblings, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:04 UTC (permalink / raw)
  To: David Gibson; +Cc: Anshu Kumari, passt-dev, jmaloy, lvivier

On Tue, 2 Jun 2026 12:23:47 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Jun 01, 2026 at 01:07:53PM +0530, Anshu Kumari wrote:
>
> > [...]
> >
> > +/**
> > + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code
> > + */
> > +static const enum dhcp_opt_type dhcp_opt_types[256] = {
> > +	[1]   = DHCP_OPT_IPV4,		/* Subnet Mask */
> > +	[2]   = DHCP_OPT_INT32,		/* Time Offset */
> > +	[3]   = DHCP_OPT_IPV4_LIST,	/* Router */  
> 
> I'm still a bit unsure if we want to allow user modification of the
> options, like this one, which we already manage ourselves.

I think we don't lose any functionality if we allow users to overwrite
them, and we might gain some: for example one might want to directly
set a specific default route in the target network namespace, in pasta,
but change it as a DHCP client runs.

Not that I see any point in doing that, but it's harmless, so why not.
Maybe even just for testing, go figure.

> > [...]
> >
> > @@ -61,7 +227,15 @@ int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
> >  		c->custom_opts_count++;
> >  	}
> >  
> > +	ret = dhcp_opt_parse(code, val_str,
> > +			     c->custom_opts[idx].val,
> > +			     sizeof(c->custom_opts[0].val));  
> 
> Now that this parsing and adding code is all in dhcp.c, could we parse
> the options directly into the existing opts[] global, rather than
> requiring both the string and parsed forms in c->custom_opts?

I see one downside with that: the day we add the possibility for
pesto(1) to set and read out options, we would need to add an
implementation to decode them back, which could be rather complicated
if you think of stuff such as "compressed" (yes, that madness) domain
names.

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/6] dhcp: Add option type table and value parser
  2026-06-01  7:37 ` [PATCH v3 3/6] dhcp: Add option type table and value parser Anshu Kumari
  2026-06-02  2:23   ` David Gibson
@ 2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:04 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, david, jmaloy, lvivier

On Mon,  1 Jun 2026 13:07:53 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> Add an RFC 2132 type lookup table mapping DHCP option codes to their
> expected value formats, and a dhcp_opt_parse() function that converts
> CLI string values into their binary wire representation.
> 
> Wire dhcp_opt_parse() into dhcp_add_option() so that values are
> validated and encoded at configuration time.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - Replaced DHCP_OPT_INTEGER with separate DHCP_OPT_INT8/INT16/INT32
>     enums, removed dhcp_opt_int_width[] array.
>   - Shared logic between DHCP_OPT_IPV4 and DHCP_OPT_IPV4_LIST — parse
>     both as list, error if >1 in single case.
>   - Added errno = 0 before strtoul() and check after.
>   - Fixed range check: 1ULL << (width * 8) for all widths including
>     width==4.
>   - strncpy → memcpy for DHCP_OPT_STR.
>   - Moved enum to dhcp.c since not used in other files.
>   - Removed options 55, 61 (client-only), 119 (DNS compression, use
>     --dhcp-search instead), 33 (IP pairs not supported).
>   - DHCP_OPT_PARSE_BUF 1024 → char tmp[256].
>   - Upgraded dhcp_add_option() to call dhcp_opt_parse() and populate
>     val[]/len.
>   - Aligned array entries for readability.
>   - Added tab after @DHCP_OPT_IPV4_LIST: in kerneldoc.
>   - Reject empty value strings before parsing
>   - Reject leading/trailing/consecutive commas in IP list values.
> 
> v2:
>   - Replaced struct lookup table + dhcp_opt_type_lookup() function with flat dhcp_opt_types[256] array indexed by code.
>   - Consolidated DHCP_OPT_UINT8/UINT16/UINT32 into single DHCP_OPT_INTEGER with dhcp_opt_int_width[256] table.
>   - Dropped DHCP_OPT_ROUTES / option 121 entirely.
>   - Added kerneldoc for enum dhcp_opt_type values.
>   - Removed curly braces from switch cases, declarations before switch.
>   - Added newlines before return statements.
>   - Changed IP list delimiter from space to comma (--dhcp-opt 6,1.1.1.1,8.8.8.8).
>   - Defined DHCP_OPT_PARSE_BUF constant for bare 1024.
>   - Added len and val[255] fields to struct here (moved from patch 1).
>   - Added kerneldoc for @custom_opts.len and @custom_opts.val.
>   - Wired dhcp_opt_parse() into case 32 (--dhcp-boot) to populate val/len.
> ---
>  dhcp.c  | 180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  passt.h |   4 ++
>  2 files changed, 181 insertions(+), 3 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index c5fbf37..07a42b9 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -23,6 +23,7 @@
>  #include <unistd.h>
>  #include <string.h>
>  #include <limits.h>
> +#include <errno.h>
>  
>  #include "util.h"
>  #include "ip.h"
> @@ -33,6 +34,170 @@
>  #include "log.h"
>  #include "dhcp.h"
>  
> +/**
> + * enum dhcp_opt_type - DHCP option value types per RFC 2132
> + * @DHCP_OPT_NONE:	Unsupported or unknown option
> + * @DHCP_OPT_STR:	Variable-length string
> + * @DHCP_OPT_IPV4:	Single IPv4 address
> + * @DHCP_OPT_IPV4_LIST:	Multiple IPv4 addresses, comma-separated
> + * @DHCP_OPT_INT8:	Unsigned 8-bit integer
> + * @DHCP_OPT_INT16:	Unsigned 16-bit integer
> + * @DHCP_OPT_INT32:	Unsigned 32-bit integer
> + */
> +enum dhcp_opt_type {
> +	DHCP_OPT_NONE,
> +	DHCP_OPT_STR,
> +	DHCP_OPT_IPV4,
> +	DHCP_OPT_IPV4_LIST,
> +	DHCP_OPT_INT8,
> +	DHCP_OPT_INT16,
> +	DHCP_OPT_INT32,
> +};
> +
> +/**
> + * dhcp_opt_types - Maps option code to RFC 2132 value type, indexed by code
> + */
> +static const enum dhcp_opt_type dhcp_opt_types[256] = {
> +	[1]   = DHCP_OPT_IPV4,		/* Subnet Mask */
> +	[2]   = DHCP_OPT_INT32,		/* Time Offset */
> +	[3]   = DHCP_OPT_IPV4_LIST,	/* Router */
> +	[4]   = DHCP_OPT_IPV4_LIST,	/* Time Server */
> +	[5]   = DHCP_OPT_IPV4_LIST,	/* Name Server */
> +	[6]   = DHCP_OPT_IPV4_LIST,	/* Domain Name Server */
> +	[7]   = DHCP_OPT_IPV4_LIST,	/* Log Server */
> +	[8]   = DHCP_OPT_IPV4_LIST,	/* Cookie Server */
> +	[9]   = DHCP_OPT_IPV4_LIST,	/* LPR Server */
> +	[10]  = DHCP_OPT_IPV4_LIST,	/* Impress Server */
> +	[11]  = DHCP_OPT_IPV4_LIST,	/* Resource Location Server */
> +	[12]  = DHCP_OPT_STR,		/* Host Name */
> +	[13]  = DHCP_OPT_INT16,		/* Boot File Size */
> +	[15]  = DHCP_OPT_STR,		/* Domain Name */
> +	[16]  = DHCP_OPT_IPV4,		/* Swap Server */
> +	[17]  = DHCP_OPT_STR,		/* Root Path */
> +	[19]  = DHCP_OPT_INT8,		/* IP Forwarding */
> +	[23]  = DHCP_OPT_INT8,		/* Default IP TTL */
> +	[26]  = DHCP_OPT_INT16,		/* Interface MTU */
> +	[28]  = DHCP_OPT_IPV4,		/* Broadcast Address */
> +	[37]  = DHCP_OPT_INT8,		/* TCP Default TTL */
> +	[38]  = DHCP_OPT_INT32,		/* TCP Keepalive Interval */
> +	[40]  = DHCP_OPT_STR,		/* NIS Domain Name */
> +	[41]  = DHCP_OPT_IPV4_LIST,	/* NIS Servers */
> +	[42]  = DHCP_OPT_IPV4_LIST,	/* NTP Servers */
> +	[44]  = DHCP_OPT_IPV4_LIST,	/* NetBIOS Name Server */
> +	[50]  = DHCP_OPT_IPV4,		/* Requested IP Address */
> +	[51]  = DHCP_OPT_INT32,		/* IP Address Lease Time */
> +	[53]  = DHCP_OPT_INT8,		/* DHCP Message Type */
> +	[54]  = DHCP_OPT_IPV4,		/* Server Identifier */
> +	[57]  = DHCP_OPT_INT16,		/* Max DHCP Message Size */
> +	[58]  = DHCP_OPT_INT32,		/* Renewal (T1) Time */
> +	[59]  = DHCP_OPT_INT32,		/* Rebinding (T2) Time */
> +	[60]  = DHCP_OPT_STR,		/* Vendor Class Identifier */
> +	[66]  = DHCP_OPT_STR,		/* TFTP Server Name */
> +	[67]  = DHCP_OPT_STR,		/* Bootfile Name */
> +	[252] = DHCP_OPT_STR,		/* WPAD URL */
> +};
> +
> +/**
> + * dhcp_opt_parse() - Parse a DHCP option value
> + * @code:	DHCP 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 dhcp_opt_parse(uint8_t code, const char *str,
> +			  uint8_t *buf, size_t buf_len)
> +{
> +	enum dhcp_opt_type type = dhcp_opt_types[code];
> +	char *tok, *saveptr, *end;
> +	struct in_addr addr;
> +	unsigned long val;
> +	unsigned int i;
> +	uint8_t width;
> +	char tmp[256];
> +	size_t slen;
> +	int len;
> +
> +	if (!*str)
> +		die("Empty value for DHCP option %u", code);
> +
> +	switch (type) {
> +	case DHCP_OPT_NONE:
> +		die("Unsupported DHCP option: %u,"
> +		    " see passt(1) for supported codes", code);
> +	case DHCP_OPT_IPV4:
> +	case DHCP_OPT_IPV4_LIST:
> +		len = 0;
> +
> +		/* Reject empty, leading/trailing, or consecutive commas */
> +		if (!*str || *str == ',' || str[strlen(str) - 1] == ',' ||
> +		    strstr(str, ",,"))
> +			return -1;
> +
> +		if (snprintf_check(tmp, sizeof(tmp), "%s", str))
> +			return -1;
> +
> +		for (tok = strtok_r(tmp, ",", &saveptr); tok;
> +		     tok = strtok_r(NULL, ",", &saveptr)) {
> +			if (inet_pton(AF_INET, tok, &addr) != 1)
> +				return -1;
> +
> +			if (len + (int)sizeof(addr) > (int)buf_len)
> +				return -1;
> +
> +			memcpy(buf + len, &addr, sizeof(addr));
> +			len += sizeof(addr);
> +
> +			if (type == DHCP_OPT_IPV4)
> +				break;
> +		}
> +
> +		if (type == DHCP_OPT_IPV4 && strtok_r(NULL, ",", &saveptr))
> +			return -1;
> +
> +		return len;
> +	case DHCP_OPT_INT8:
> +	case DHCP_OPT_INT16:
> +	case DHCP_OPT_INT32:
> +		if (type == DHCP_OPT_INT8)
> +			width = 1;
> +		else if (type == DHCP_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 DHCP_OPT_STR:
> +		slen = strlen(str);
> +
> +		if (!slen || slen >= buf_len)
> +			return -1;
> +
> +		memcpy(buf, str, slen);
> +
> +		return slen;
> +	}
> +
> +	return -1;
> +}
>  
>  /**
>   * dhcp_add_option() - Add or update a custom DHCP option
> @@ -40,14 +205,15 @@
>   * @code:	DHCP option code
>   * @val_str:	Value string from command line
>   *
> - * If @code was already added, the previous value is overwritten.
> - * Calls die() on any error.
> + * Parses @val_str according to the type registered for @code in
> + * dhcp_opt_types[]. If @code was already added, the previous value
> + * is overwritten. Calls die() on any error.
>   *
>   * Return: 0 on success
>   */
>  int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
>  {
> -	int idx;
> +	int idx, ret;
>  
>  	for (idx = 0; idx < c->custom_opts_count; idx++) {
>  		if (c->custom_opts[idx].code == code)
> @@ -61,7 +227,15 @@ int dhcp_add_option(struct ctx *c, uint8_t code, const char *val_str)
>  		c->custom_opts_count++;
>  	}
>  
> +	ret = dhcp_opt_parse(code, val_str,
> +			     c->custom_opts[idx].val,
> +			     sizeof(c->custom_opts[0].val));
> +	if (ret < 0)
> +		die("Invalid value for DHCP option %u: %s",
> +		    code, val_str);
> +
>  	c->custom_opts[idx].code = code;
> +	c->custom_opts[idx].len = ret;
>  
>  	if (snprintf_check(c->custom_opts[idx].str,
>  			   sizeof(c->custom_opts[0].str),
> diff --git a/passt.h b/passt.h
> index 3a0816f..751fee3 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -184,6 +184,8 @@ struct ip6_ctx {
>   * @fqdn:		Guest FQDN
>   * @custom_opts:	User-specified DHCP options from --dhcp-opt
>   * @custom_opts.code:	DHCP option code
> + * @custom_opts.len:	Length of binary value in @val
> + * @custom_opts.val:	Binary-encoded option value

But we already have struct opt opts[255] in dhcp.c, why don't you use
that directly? It would have the advantage of clearly separating what
belongs to the configuration proper and what belongs to the specific
DHCP encoding.

Besides, I see the point in maintaining what was set via command line,
but not some temporary value that would just be copied to struct opt[].
This would be a third copy.

>   * @custom_opts.str:	Original string value from command line
>   * @custom_opts_count:	Number of entries in @custom_opts
>   * @ifi6:		Template interface for IPv6, -1: none, 0: IPv6 disabled
> @@ -271,6 +273,8 @@ struct ctx {
>  
>  	struct {
>  		uint8_t code;
> +		uint8_t len;
> +		uint8_t val[255];
>  		char str[256];
>  	} custom_opts[MAX_CUSTOM_DHCP_OPTS];
>  	int custom_opts_count;

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 5/6] dhcp: Add option overload
  2026-06-01  7:37 ` [PATCH v3 5/6] dhcp: Add option overload Anshu Kumari
  2026-06-02  2:50   ` David Gibson
@ 2026-06-11 23:04   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:04 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, david, jmaloy, lvivier

On Mon,  1 Jun 2026 13:07:55 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> A user can enter lots of options in command-line which may not fit in
> existing buffer, So when the options field is full, overflow remaining
> DHCP options into the file and sname fields per RFC 2132 option 52.

As David already pointed out, this hides the fact that this patch
actually implements the insertion of options in the actual DHCP
messages.

This makes it pretty hard / impossible to find things later if we need
to investigate or fix some issue.

Would it be doable to have a separate change just implementing option
overload? If not, I think it would be preferable to merge this together
with the current 3/6.

> Also, when the file field is not used for overload, copy the boot
> file URL there directly for legacy PXE clients.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - Added RFC 2132 Section 9.3 reference comment on overload
>     constants.
>   - Use ARRAY_SIZE(opts) instead of raw 255 in fill_overflow().
>   - Swapped overflow order: try sname (64 bytes) first, then file
>     (128 bytes) — better packing and keeps file field available for
>     boot file name.
>   - Removed '&' from &reply.file.
>   - Removed '+1' from memcpy — reply.file already zeroed.
>   - opt_set_dns_search() max_len: OPT_MAX - 3 instead of
>     sizeof(m->o).
> 
> v2:
>   - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants
>   - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */
>   - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname
>   - Removed dhcp_boot references — reply.file copy now reads from opts[67]
>   - Used DHCP_OVERLOAD_FILE constant in reply.file guard
> ---
>  dhcp.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 81 insertions(+), 10 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index 5c6a492..a9d56fd 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -372,14 +372,64 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset)
>  	return false;
>  }
>  
> +/* RFC 2132, Section 9.3 - Option Overload*/

Nit: missing whitespace before */

> +#define DHCP_OVERLOAD_FILE	1
> +#define DHCP_OVERLOAD_SNAME	2
> +
>  /**
> - * fill() - Fill options in message
> + * fill_overflow() - Fill remaining options into sname and file fields
> + * @m:		Message whose sname/file fields may be used for overflow
> + *
> + * Try the smaller sname field first: small options go there, leaving
> + * the larger file field available for big options and for the boot
> + * file name (option 67) if set.
> + *
> + * Return: option 52 overload value: 0 if no overflow,
> + *         DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname,
> + *         or both OR'd together
> + */
> +static int fill_overflow(struct msg *m)
> +{
> +	int sname_off = 0, file_off = 0, overload = 0;
> +	int o;
> +
> +	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
> +		if (opts[o].slen == -1 || opts[o].sent)
> +			continue;
> +		fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off);
> +	}
> +
> +	for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) {
> +		if (opts[o].slen == -1 || opts[o].sent)
> +			continue;
> +		if (fill_one(m->file, sizeof(m->file) - 1, o, &file_off))
> +			debug("DHCP: skipping option %i (overload full)", o);
> +	}
> +
> +	if (sname_off) {
> +		m->sname[sname_off] = 255;
> +		overload |= DHCP_OVERLOAD_SNAME;
> +	}
> +
> +	if (file_off) {
> +		m->file[file_off] = 255;
> +		overload |= DHCP_OVERLOAD_FILE;
> +	}
> +
> +	return overload;
> +}
> +
> +/**
> + * fill() - Fill options in message, with overload into file/sname if needed
>   * @m:		Message to fill
> + * @overload:	Set to option 52 value (0 if none, 1/2/3 per RFC 2132)

Sounds like an enum to me.

>   *
>   * Return: current size of options field
>   */
> -static int fill(struct msg *m)
> +static int fill(struct msg *m, int *overload)
>  {
> +	/* Reserve 3 bytes for option 52 (overload) if needed */
> +	size_t size = OPT_MAX - 3;

This isn't ideal in the sense that if we have, at the end of the
options, an option that's three or less bytes in total (one or zero
bytes of value), and we are at OPT_MAX - 3, we could encode it without
overload.

I don't see a real issue in keeping this for the sake of simplicity,
especially because that's such an unlikely corner case, and the
consequences of this non-ideal implementation are really minor if any,
but maybe (I haven't really thought it through) to always keep three
bytes free, and then add another loop checking if we have a single
option that's three bytes long or less?

>  	int i, o, offset = 0;
>  
>  	for (o = 0; o < 255; o++)
> @@ -390,20 +440,25 @@ static int fill(struct msg *m)
>  	 * Put it there explicitly, unless requested via option 55.
>  	 */
>  	if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen))
> -		if (fill_one(m->o, OPT_MAX, 53, &offset))
> -			 debug("DHCP: skipping option 53");
> +		fill_one(m->o, size, 53, &offset);
>  
>  	for (i = 0; i < opts[55].clen; i++) {
>  		o = opts[55].c[i];
>  		if (opts[o].slen != -1)
> -			if (fill_one(m->o, OPT_MAX, o, &offset))
> -				debug("DHCP: skipping option %i", o);
> +			fill_one(m->o, size, o, &offset);
>  	}
>  
>  	for (o = 0; o < 255; o++) {
>  		if (opts[o].slen != -1 && !opts[o].sent)
> -			if (fill_one(m->o, OPT_MAX, o, &offset))
> -				debug("DHCP: skipping option %i", o);
> +			fill_one(m->o, size, o, &offset);
> +	}
> +
> +	*overload = fill_overflow(m);
> +
> +	if (*overload) {
> +		m->o[offset++] = 52;
> +		m->o[offset++] = 1;
> +		m->o[offset++] = *overload;
>  	}
>  
>  	m->o[offset++] = 255;
> @@ -528,6 +583,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	struct msg const *m;
>  	struct msg reply;
>  	unsigned int i;
> +	int overload;
>  
>  	eh = IOV_REMOVE_HEADER(data, eh_storage);
>  	iph = IOV_PEEK_HEADER(data, iph_storage);
> @@ -677,9 +733,24 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>  	}
>  
>  	if (!c->no_dhcp_dns_search)
> -		opt_set_dns_search(c, sizeof(m->o));
> +		opt_set_dns_search(c, OPT_MAX - 3);
> +
> +	for (i = 0; i < (unsigned int)c->custom_opts_count; i++) {
> +		uint8_t code = c->custom_opts[i].code;
> +
> +		opts[code].slen = c->custom_opts[i].len;
> +		memcpy(opts[code].s, c->custom_opts[i].val,
> +		       c->custom_opts[i].len);
> +	}
> +
> +	dlen = offsetof(struct msg, o) + fill(&reply, &overload);
>  
> -	dlen = offsetof(struct msg, o) + fill(&reply);
> +	/* Copy boot file name into the file field for legacy PXE clients,
> +	 * unless the file field is already used for option overload.
> +	 */

But RFC 2132, section 9.5, says:

   This option is used to identify a bootfile when the 'file' field in
   the DHCP header has been used for DHCP options.

and I think we should interpret that "when" as "if and only if" by
default (that's the least surprising way from a linguistic
perspective), so, as a consequence, we should never duplicate that in
the replies we send.

That is, if we can insert it in 'file', we should do that, and we
should use option 67 _only if_ we can't. Not always. Not if it's
already in 'file'.

Now, we don't know if we need option overload until we're done
inserting options, 

> +	if (!(overload & DHCP_OVERLOAD_FILE) &&
> +	    opts[67].slen > 0 && (size_t)opts[67].slen < sizeof(reply.file))
> +		memcpy(reply.file, opts[67].s, opts[67].slen);
>  
>  	if (m->flags & FLAG_BROADCAST)
>  		dst = in4addr_broadcast;

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page
  2026-06-01  7:37 ` [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page Anshu Kumari
  2026-06-02  2:54   ` David Gibson
@ 2026-06-11 23:05   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:05 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, david, jmaloy, lvivier

On Mon,  1 Jun 2026 13:07:56 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> Document the new --dhcp-boot and --dhcp-opt command-line options in
> the passt(1) man page, including supported option codes grouped by
> value type and usage examples.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - Removed options 33, 55, 61, 119 from supported codes list
>   - Added note: "If the same name option code is given more than once,
>     the last value wins".
> 
> v2:
>   - Updated --dhcp-boot description.
>   - Highlighted cross-referenced options with \fB...\fR.
>   - Updated IP list format from "space-separated within quotes" to "comma-separated".
>   - option 121 dropped.
>   - Added option 55 to string options list.
>   - Removed --dhcp-boot override reference from --dhcp-opt description.
> ---
>  passt.1 | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/passt.1 b/passt.1
> index 908fd4a..57e2cc1 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -430,6 +430,47 @@ 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 \-\-dhcp-boot " " \fIurl
> +Convenience shorthand for \fB\-\-dhcp-opt\fR 67,\fIurl\fR.
> +Sets the boot file name (DHCP option 67) for network boot.
> +For UEFI HTTP boot, also set the vendor class identifier using
> +\fB\-\-dhcp-opt\fR 60,HTTPClient.

This is ambiguous. The man page is written in an imperative style in
the sense of what the tool is supposed to do, but this becomes a
recommendation to the user instead (this sentence says "set" and not
"sets" but that's very easy to miss, and ambiguous nevertheless).

This would be clearer I think:

For UEFI HTTP boot, the vendor class identifier also needs to be set
using ...

> +
> +.TP
> +.BR \-\-dhcp-opt " " \fICODE\fR,\fIVALUE\fR
> +Set a DHCP option by numeric code. The value format is determined automatically
> +from the option code. Multiple IPv4 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. Options set with
> +\fB\-\-dhcp-opt\fR override built-in values.
> +Only the following option codes are supported (unsupported codes cause an error):

By the way, eventually, I think it would be nice to support those as
hex encoding, maybe with a --dhcp-opt-raw parameter taking a length
value (also comma-separated, say, CODE,LENGTH,VALUE).

I guess it's beyond the scope of this series though.

> +.RS
> +.TP
> +.B IPv4 address options
> +1 (Subnet Mask), 16 (Swap Server), 28 (Broadcast Address), 50 (Requested IP),
> +54 (Server Identifier)
> +.TP
> +.B IPv4 address list options (comma-separated)
> +3 (Router), 4 (Time Server), 5 (Name Server), 6 (DNS), 7 (Log Server),
> +8 (Cookie Server), 9 (LPR Server), 10 (Impress Server),
> +11 (Resource Location Server), 41 (NIS Servers),
> +42 (NTP Servers), 44 (NetBIOS Name Server)
> +.TP
> +.B Integer options
> +2 (Time Offset, 32-bit), 13 (Boot File Size, 16-bit), 19 (IP Forwarding, 8-bit),
> +23 (Default IP TTL, 8-bit), 26 (Interface MTU, 16-bit),
> +37 (TCP Default TTL, 8-bit), 38 (TCP Keepalive Interval, 32-bit),
> +51 (IP Address Lease Time, 32-bit),
> +53 (DHCP Message Type, 8-bit), 57 (Max DHCP Message Size, 16-bit),
> +58 (Renewal Time, 32-bit), 59 (Rebinding Time, 32-bit)
> +.TP
> +.B String options
> +12 (Host Name), 15 (Domain Name), 17 (Root Path), 40 (NIS Domain Name),
> +60 (Vendor Class Identifier), 66 (TFTP Server Name),
> +67 (Bootfile Name), 252 (WPAD URL)
> +.RE
> +
>  .TP
>  .BR \-t ", " \-\-tcp-ports " " \fIspec
>  Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of:

The rest looks good to me.

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/6] conf: Add --dhcp-boot command-line option
  2026-06-01  7:37 ` [PATCH v3 2/6] conf: Add --dhcp-boot " Anshu Kumari
  2026-06-02  2:02   ` David Gibson
@ 2026-06-11 23:05   ` Stefano Brivio
  1 sibling, 0 replies; 19+ messages in thread
From: Stefano Brivio @ 2026-06-11 23:05 UTC (permalink / raw)
  To: Anshu Kumari; +Cc: passt-dev, david, jmaloy, lvivier

On Mon,  1 Jun 2026 13:07:52 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> Introduce the --dhcp-boot flag that sets the boot file URL for
> network boot specially for ipxe. This patch adds the option

...meaning in a special way for iPXE (but what is special about it? I'm
not sure if I'm missing something) or especially for iPXE usage (but
then it's "especially")?

> storage and CLI parsing.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=192
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v3:
>   - case 32 now calls dhcp_add_option(c, 67, optarg).
>   - Handles duplicate codes: --dhcp-boot and --dhcp-opt 67 coexist
>     correctly, last value wins.
> 
> v2:
>   - Removed separate dhcp_boot[PATH_MAX] field — --dhcp-boot foo now stores into custom_opts[] as code 67 (same as --dhcp-opt 67,foo)
> 
> ---
>  conf.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/conf.c b/conf.c
> index ce78af1..f7281e2 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -618,6 +618,7 @@ static void usage(const char *name, FILE *f, int status)
>  		"    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"
> +		"  --dhcp-boot URL	Boot file URL for network boot\n"
>  		"  --dhcp-opt CODE,VAL	Set DHCP option by code\n");
>  	if (strstr(name, "pasta"))
>  		FPRINTF(f, "    default: don't use any search list\n");
> @@ -1239,6 +1240,7 @@ void conf(struct ctx *c, int argc, char **argv)
>  		{"migrate-no-linger", no_argument,	NULL,		30 },
>  		{"stats", required_argument,		NULL,		31 },
>  		{"conf-path",	required_argument,	NULL,		'c' },
> +		{"dhcp-boot", required_argument,	NULL,		32 },
>  		{"dhcp-opt", required_argument,		NULL,		33 },
>  		{ 0 },
>  	};
> @@ -1475,6 +1477,9 @@ void conf(struct ctx *c, int argc, char **argv)
>  				die("Can't display statistics if not running in foreground");
>  			c->stats = strtol(optarg, NULL, 0);
>  			break;
> +		case 32:
> +			dhcp_add_option(c, 67, optarg);
> +			break;
>  		case 33:
>  			comma = strchr(optarg, ',');
>  			if (!comma)

-- 
Stefano


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-06-11 23:05 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-01  7:37 [PATCH v3 0/6] Add --dhcp-boot and --dhcp-opt options Anshu Kumari
2026-06-01  7:37 ` [PATCH v3 1/6] conf: Add --dhcp-opt command-line option Anshu Kumari
2026-06-02  2:00   ` David Gibson
2026-06-11 23:04   ` Stefano Brivio
2026-06-01  7:37 ` [PATCH v3 2/6] conf: Add --dhcp-boot " Anshu Kumari
2026-06-02  2:02   ` David Gibson
2026-06-11 23:05   ` Stefano Brivio
2026-06-01  7:37 ` [PATCH v3 3/6] dhcp: Add option type table and value parser Anshu Kumari
2026-06-02  2:23   ` David Gibson
2026-06-11 23:04     ` Stefano Brivio
2026-06-11 23:04   ` Stefano Brivio
2026-06-01  7:37 ` [PATCH v3 4/6] dhcp: Refactor fill_one() to operate on a generic buffer Anshu Kumari
2026-06-02  2:25   ` David Gibson
2026-06-01  7:37 ` [PATCH v3 5/6] dhcp: Add option overload Anshu Kumari
2026-06-02  2:50   ` David Gibson
2026-06-11 23:04   ` Stefano Brivio
2026-06-01  7:37 ` [PATCH v3 6/6] doc: Add --dhcp-boot and --dhcp-opt to man page Anshu Kumari
2026-06-02  2:54   ` David Gibson
2026-06-11 23:05   ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).