Re: [PATCH v3 1/6] conf: Add --dhcp-opt command-line option

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

[-- 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 --]