Re: [PATCH v9] conf: Support CIDR notation for -a/--address option

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

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

On Sun, Feb 08, 2026 at 07:37:27PM -0500, Jon Maloy wrote:
> Extend the -a/--address option to accept addresses in CIDR notation
> (e.g., 192.168.1.1/24 or 2001:db8::1/64) as an alternative to using
> separate -a and -n options.
> 
> We add a new inany_prefix_pton() helper function that:
> - Parses address strings with a compulsory /prefix_len suffix
> - Validates prefix length based on address family (0-32 for IPv4,
>   0-128 for IPv6), including handling of IPv4-to-IPv6 mapping case.
> 
> For IPv4, the prefix length is stored in ip4.prefix_len when provided.
> Mixing -n and CIDR notation results in an error to catch likely user
> mistakes.
> 
> Also fix a bug in conf_ip4_prefix() that was incorrectly using the
> global 'optarg' instead of its 'arg' parameter.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

I have some comments below, but none of them are showstoppers, so:

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

> 
> ---
> v3: Fixes after feedback from Laurent, David and Stefano
>     Notably, updated man page for the -a option
> 
> v4: Fixes based on feedback from David G:
>   - Handling prefix length adjustment when IPv4-to-IPv6 mapping
>   - Removed redundant !IN6_IS_ADDR_V4MAPPED(&addr.a6) test
>   - Simplified tests of acceptable address types
>   - Merged documentation and code commits
>   - Some documentation text clarifications
> 
> v5: - Moved address/prefix parsing into a refactored
>       inany_prefix_pton() function.
>     - inany_prefix_pton() now only caluclates IPv6 style
>       prefix lengths
>     - Stricter distinction between error causes.
>     - Some refactoring of the 'case a:' branch in conf()
>     - Some small fixes in passt.1
> 
> v6: - Refactored inany_prefix_pton() and conf()::'case -a'
>       code after input from David Gibson.
> 
> v7: - More refactoring after input from David Gibson.
>     - I kept the return values 1 and 0. This is consistent
>       with the return values of inet_pton() and inany_pton().

Ah, good point, my mistake.

> v8: - Changed condition for updating ipv4 prefix length
> 
> v9: - Made char *src and char *pstr in inany_prefix_pton() const
>     - Updated logics in conf.c:: case 'a' and case 'n' to
>       be clearer (I think)
> ---
>  conf.c  | 72 +++++++++++++++++++++++++++++++++++++--------------------
>  inany.c | 51 ++++++++++++++++++++++++++++++++++++++++
>  inany.h | 15 ++++++++++++
>  ip.c    | 21 +++++++++++++++++
>  ip.h    |  2 ++
>  passt.1 | 17 ++++++++++----
>  6 files changed, 148 insertions(+), 30 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 2942c8c..46cfb6e 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -682,7 +682,7 @@ static int conf_ip4_prefix(const char *arg)
>  			return -1;
>  	} else {
>  		errno = 0;
> -		len = strtoul(optarg, NULL, 0);
> +		len = strtoul(arg, NULL, 0);
>  		if (len > 32 || errno)
>  			return -1;
>  	}
> @@ -896,7 +896,7 @@ static void usage(const char *name, FILE *f, int status)
>  		"    a zero value disables assignment\n"
>  		"    default: 65520: maximum 802.3 MTU minus 802.3 header\n"
>  		"                    length, rounded to 32 bits (IPv4 words)\n"
> -		"  -a, --address ADDR	Assign IPv4 or IPv6 address ADDR\n"
> +		"  -a, --address ADDR	Assign IPv4 or IPv6 address ADDR[/PREFIXLEN]\n"
>  		"    can be specified zero to two times (for IPv4 and IPv6)\n"
>  		"    default: use addresses from interface with default route\n"
>  		"  -n, --netmask MASK	Assign IPv4 MASK, dot-decimal or bits\n"
> @@ -1505,6 +1505,8 @@ void conf(struct ctx *c, int argc, char **argv)
>  	unsigned long max_mtu = IP_MAX_MTU;
>  	struct fqdn *dnss = c->dns_search;
>  	unsigned int ifi4 = 0, ifi6 = 0;
> +	bool prefix_from_cidr = false;
> +	uint8_t prefix_from_opt = 0;

I see why, but having two variables with very similarly formatted
names, but different types and semantics is potentially confusing.

Using '0' as a nil value for the prefix length also makes me nervous,
since 0 length prefixes are in general valid, even if they're not
useful for the -a option.

>  	const char *logfile = NULL;
>  	size_t logsize = 0;
>  	char *runas = NULL;
> @@ -1808,36 +1810,56 @@ void conf(struct ctx *c, int argc, char **argv)
>  			c->mtu = mtu;
>  			break;
>  		}
> -		case 'a':
> -			if (inet_pton(AF_INET6, optarg, &c->ip6.addr)	&&
> -			    !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)	&&
> -			    !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr)		&&
> -			    !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr)		&&
> -			    !IN6_IS_ADDR_V4COMPAT(&c->ip6.addr)		&&
> -			    !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) {
> -				if (c->mode == MODE_PASTA)
> -					c->ip6.no_copy_addrs = true;
> -				break;
> -			}
> +		case 'a': {
> +			union inany_addr addr;
> +			uint8_t prefix_len;
> +
> +			prefix_from_cidr =
> +				inany_prefix_pton(optarg, &addr, &prefix_len);
> +
> +			if (prefix_from_cidr && prefix_from_opt)
> +				die("Can't mix CIDR with -n");
> +
> +			if (!prefix_from_cidr && !inany_pton(optarg, &addr))
> +				die("Invalid address: %s", optarg);
> +
> +			if (prefix_from_opt && inany_v4(&addr))
> +				prefix_len = prefix_from_opt;
> +			else if (!prefix_from_cidr)
> +				prefix_len = inany_default_prefix_len(&addr);
> +
> +			if (inany_is_unspecified(&addr) ||
> +			    inany_is_multicast(&addr) ||
> +			    inany_is_loopback(&addr) ||
> +			    IN6_IS_ADDR_V4COMPAT(&addr.a6))
> +				die("Invalid address: %s", optarg);
>  
> -			if (inet_pton(AF_INET, optarg, &c->ip4.addr)	&&
> -			    !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr)	&&
> -			    !IN4_IS_ADDR_BROADCAST(&c->ip4.addr)	&&
> -			    !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr)		&&
> -			    !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) {
> +			if (inany_v4(&addr)) {
> +				c->ip4.addr = *inany_v4(&addr);
> +				c->ip4.prefix_len = prefix_len - 96;
>  				if (c->mode == MODE_PASTA)
>  					c->ip4.no_copy_addrs = true;
> -				break;
> +			} else {
> +				c->ip6.addr = addr.a6;
> +				if (c->mode == MODE_PASTA)
> +					c->ip6.no_copy_addrs = true;
>  			}
> -
> -			die("Invalid address: %s", optarg);
>  			break;
> -		case 'n':
> -			c->ip4.prefix_len = conf_ip4_prefix(optarg);
> -			if (c->ip4.prefix_len < 0)
> -				die("Invalid netmask: %s", optarg);
> +		}
> +		case 'n': {
> +			int plen;
> +
> +			if (prefix_from_cidr)
> +				die("Can't use both -n and CIDR prefix length");
>  
> +			plen = conf_ip4_prefix(optarg);
> +			if (plen < 0)
> +				die("Invalid prefix length: %s", optarg);
> +
> +			prefix_from_opt = plen + 96;

Since it's only used for IPv4 adddresses, prefix_from_opt could encode
in IPv4-style (0..32, rather than 0..128).  I don't have a strong
preference, though.

> +			c->ip4.prefix_len = plen;
>  			break;
> +		}
>  		case 'M':
>  			parse_mac(c->our_tap_mac, optarg);
>  			break;
> diff --git a/inany.c b/inany.c
> index 7680439..df6a126 100644
> --- a/inany.c
> +++ b/inany.c
> @@ -11,6 +11,7 @@
>  #include <assert.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
> +#include <errno.h>
>  
>  #include "util.h"
>  #include "ip.h"
> @@ -57,3 +58,53 @@ int inany_pton(const char *src, union inany_addr *dst)
>  
>  	return 0;
>  }
> +
> +/**
> + * inany_prefix_pton() - Parse an IPv[46] address with prefix length
> + * @src:	IPv[46] address and prefix length string in CIDR format
> + * @dst:	Output buffer, filled with parsed address
> + * @prefix_len:	Prefix length, to be filled in IPv6 format
> + *
> + * Return: on success, 1, if no parseable address or prefix is found, 0
> + */
> +int inany_prefix_pton(const char *src, union inany_addr *dst,
> +		      uint8_t *prefix_len)
> +{
> +	char astr[INANY_ADDRSTRLEN] = {0,};

I don't believe this needs an initializer.

> +	size_t alen = strcspn(src, "/");
> +	const char *pstr = &src[alen + 1];
> +	unsigned long plen;
> +	char *end;
> +
> +	if (alen >= INANY_ADDRSTRLEN)
> +		return 0;
> +
> +	if (src[alen] != '/')
> +		return 0;
> +
> +	strncpy(astr, src, alen);
> +
> +	/* Read prefix length */
> +	errno = 0;
> +	plen = strtoul(pstr, &end, 10);
> +	if (errno || *end || plen > 128)
> +		return 0;
> +
> +	*prefix_len = plen;

It would be slightly cleaner not to clobber any of the output
parameters on failure, but it's not a big deal.

> +
> +	/* Read address */
> +	if (inet_pton(AF_INET6, astr, dst)) {
> +		if (inany_v4(dst) && *prefix_len < 96)
> +			return 0;
> +		return 1;
> +	}
> +
> +	if (inany_pton(astr, dst)) {
> +		if (*prefix_len > 32)
> +			return 0;
> +		*prefix_len += 96;
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> diff --git a/inany.h b/inany.h
> index 61b36fb..c7d6027 100644
> --- a/inany.h
> +++ b/inany.h
> @@ -96,6 +96,19 @@ static inline struct in_addr *inany_v4(const union inany_addr *addr)
>  	return (struct in_addr *)&addr->v4mapped.a4;
>  }
>  
> +/** inany_default_prefix_len() - Get default prefix length for address
> + * @addr:	IPv4 or iPv6 address
> + *
> + * Return: Class-based prefix length for IPv4 (in IPv6 format: 104-128),
> + *         or 64 for IPv6
> + */
> +static inline int inany_default_prefix_len(const union inany_addr *addr)
> +{
> +	const struct in_addr *v4 = inany_v4(addr);
> +
> +	return v4 ? ip4_class_prefix_len(v4) + 96 : 64;
> +}
> +
>  /** inany_equals - Compare two IPv[46] addresses
>   * @a, @b:	IPv[46] addresses
>   *
> @@ -295,5 +308,7 @@ static inline void inany_siphash_feed(struct siphash_state *state,
>  
>  const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size);
>  int inany_pton(const char *src, union inany_addr *dst);
> +int inany_prefix_pton(const char *src, union inany_addr *dst,
> +		      uint8_t *prefix_len);
>  
>  #endif /* INANY_H */
> diff --git a/ip.c b/ip.c
> index 9a7f4c5..40dc24e 100644
> --- a/ip.c
> +++ b/ip.c
> @@ -13,6 +13,8 @@
>   */
>  
>  #include <stddef.h>
> +#include <netinet/in.h>
> +
>  #include "util.h"
>  #include "ip.h"
>  
> @@ -67,3 +69,22 @@ found:
>  	*proto = nh;
>  	return true;
>  }
> +
> +/**
> + * ip4_class_prefix_len() - Get class based prefix length for IPv4 address
> + * @addr:	IPv4 address
> + *
> + * Return: prefix length based on address class, or 32 for other
> + */
> +int ip4_class_prefix_len(const struct in_addr *addr)
> +{
> +	in_addr_t a = ntohl(addr->s_addr);
> +
> +	if (IN_CLASSA(a))
> +		return 32 - IN_CLASSA_NSHIFT;
> +	if (IN_CLASSB(a))
> +		return 32 - IN_CLASSB_NSHIFT;
> +	if (IN_CLASSC(a))
> +		return 32 - IN_CLASSC_NSHIFT;
> +	return 32;
> +}
> diff --git a/ip.h b/ip.h
> index 5830b92..bd28640 100644
> --- a/ip.h
> +++ b/ip.h
> @@ -135,4 +135,6 @@ static const struct in_addr in4addr_broadcast = { 0xffffffff };
>  #define IPV6_MIN_MTU		1280
>  #endif
>  
> +int ip4_class_prefix_len(const struct in_addr *addr);
> +
>  #endif /* IP_H */
> diff --git a/passt.1 b/passt.1
> index db0d662..2c3eae9 100644
> --- a/passt.1
> +++ b/passt.1
> @@ -156,10 +156,14 @@ By default, the advertised MTU is 65520 bytes, that is, the maximum 802.3 MTU
>  minus the length of a 802.3 header, rounded to 32 bits (IPv4 words).
>  
>  .TP
> -.BR \-a ", " \-\-address " " \fIaddr
> +.BR \-a ", " \-\-address " " \fIaddr\fR[/\fIprefix_len\fR]
>  Assign IPv4 \fIaddr\fR via DHCP (\fByiaddr\fR), or \fIaddr\fR via DHCPv6 (option
>  5) and an \fIaddr\fR-based prefix via NDP Router Advertisement (option type 3)
>  for an IPv6 \fIaddr\fR.
> +An optional /\fIprefix_len\fR (0-32 for IPv4, 0-128 for IPv6) can be
> +appended in CIDR notation (e.g. 192.0.2.1/24). This is an alternative to
> +using the \fB-n\fR, \fB--netmask\fR option. Mixing CIDR notation with
> +\fB-n\fR results in an error.
>  This option can be specified zero (for defaults) to two times (once for IPv4,
>  once for IPv6).
>  By default, assigned IPv4 and IPv6 addresses are taken from the host interfaces
> @@ -172,10 +176,13 @@ is assigned for IPv4, and no additional address will be assigned for IPv6.
>  .TP
>  .BR \-n ", " \-\-netmask " " \fImask
>  Assign IPv4 netmask \fImask\fR, expressed as dot-decimal or number of bits, via
> -DHCP (option 1).
> -By default, the netmask associated to the host address matching the assigned one
> -is used. If there's no matching address on the host, the netmask is determined
> -according to the CIDR block of the assigned address (RFC 4632).
> +DHCP (option 1). Alternatively, the prefix length can be specified using CIDR
> +notation with the \fB-a\fR, \fB--address\fR option (e.g. \fB-a\fR 192.0.2.1/24).
> +Mixing \fB-n\fR with CIDR notation results in an error.
> +If no address is indicated, the netmask associated with the adopted host address,
> +if any, is used. If an address is indicated, but without a prefix length, the
> +netmask is determined based on the corresponding network class. In all other
> +cases, the netmask is determined by using the indicated prefix length.
>  
>  .TP
>  .BR \-M ", " \-\-mac-addr " " \fIaddr
> -- 
> 2.52.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 --]