public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Laurent Vivier <lvivier@redhat.com>
To: Jon Maloy <jmaloy@redhat.com>,
	sbrivio@redhat.com, dgibson@redhat.com,
	david@gibson.dropbear.id.au, passt-dev@passt.top
Subject: Re: [PATCH] conf: Support CIDR notation for -a/--address option
Date: Wed, 17 Dec 2025 09:50:23 +0100	[thread overview]
Message-ID: <37df2e6f-3eeb-4c34-8025-2515fc5a6a87@redhat.com> (raw)
In-Reply-To: <20251216230408.100976-1-jmaloy@redhat.com>

On 12/17/25 00:04, Jon Maloy wrote:
> We 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.
> 
> For IPv4, the prefix is stored in ip4.prefix_len when provided. The
> CIDR prefix and -n/--netmask options are mutually exclusive - using
> both results in an error.
> 
> 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>
> ---
>   conf.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 84 insertions(+), 15 deletions(-)

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Note if you provide two IPv4 addresses with prefix (-a XXX/YY -a XXX/YY) the error 
reported will be "Can't use both -n and CIDR prefix" that is not the correct error.

Thanks,
Laurent>
> diff --git a/conf.c b/conf.c
> index 2942c8c..0f66c77 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;
>   	}
> @@ -690,6 +690,56 @@ static int conf_ip4_prefix(const char *arg)
>   	return len;
>   }
>   
> +/**
> + * conf_addr_prefix() - Parse address with optional /prefix notation
> + * @arg:	Address string, optionally with /prefix
> + * @addr4:	Output for IPv4 address
> + * @addr6:	Output for IPv6 address
> + * @prefix:	Output for prefix length (0 if not specified)
> + *
> + * Return: AF_INET for IPv4, AF_INET6 for IPv6, -1 on error
> + */
> +static int conf_addr_prefix(const char *arg, struct in_addr *addr4,
> +			    struct in6_addr *addr6, int *prefix)
> +{
> +	char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> +	char *slash;
> +
> +	*prefix = 0;
> +
> +	if (snprintf(buf, sizeof(buf), "%s", arg) >= (int)sizeof(buf))
> +		return -1;
> +
> +	/* Check for /prefix suffix */
> +	slash = strchr(buf, '/');
> +	if (slash) {
> +		unsigned long len;
> +		char *end;
> +
> +		*slash = '\0';
> +		errno = 0;
> +		len = strtoul(slash + 1, &end, 10);
> +		if (errno || *end)
> +			return -1;
> +
> +		*prefix = len;
> +	}
> +
> +	if (inet_pton(AF_INET6, buf, addr6) == 1) {
> +		if (*prefix > 128)
> +			return -1;
> +		return AF_INET6;
> +	}
> +
> +	if (inet_pton(AF_INET, buf, addr4) == 1) {
> +		if (*prefix > 32)
> +			return -1;
> +		return AF_INET;
> +	}
> +
> +	return -1;
> +}
> +
>   /**
>    * conf_ip4() - Verify or detect IPv4 support, get relevant addresses
>    * @ifi:	Host interface to attempt (0 to determine one)
> @@ -896,7 +946,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[/PREFIX]\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"
> @@ -1501,6 +1551,7 @@ void conf(struct ctx *c, int argc, char **argv)
>   	bool copy_addrs_opt = false, copy_routes_opt = false;
>   	enum fwd_ports_mode fwd_default = FWD_NONE;
>   	bool v4_only = false, v6_only = false;
> +	bool prefix_explicit = false;
>   	unsigned dns4_idx = 0, dns6_idx = 0;
>   	unsigned long max_mtu = IP_MAX_MTU;
>   	struct fqdn *dnss = c->dns_search;
> @@ -1808,23 +1859,38 @@ 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)) {
> +		case 'a': {
> +			struct in6_addr addr6;
> +			struct in_addr addr4;
> +			int prefix = 0;
> +			int af;
> +
> +			af = conf_addr_prefix(optarg, &addr4, &addr6, &prefix);
> +
> +			if (af == AF_INET6 &&
> +			    !IN6_IS_ADDR_UNSPECIFIED(&addr6)	&&
> +			    !IN6_IS_ADDR_LOOPBACK(&addr6)	&&
> +			    !IN6_IS_ADDR_V4MAPPED(&addr6)	&&
> +			    !IN6_IS_ADDR_V4COMPAT(&addr6)	&&
> +			    !IN6_IS_ADDR_MULTICAST(&addr6)) {
> +				c->ip6.addr = addr6;
>   				if (c->mode == MODE_PASTA)
>   					c->ip6.no_copy_addrs = true;
>   				break;
>   			}
>   
> -			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 (af == AF_INET &&
> +			    !IN4_IS_ADDR_UNSPECIFIED(&addr4)	&&
> +			    !IN4_IS_ADDR_BROADCAST(&addr4)	&&
> +			    !IN4_IS_ADDR_LOOPBACK(&addr4)	&&
> +			    !IN4_IS_ADDR_MULTICAST(&addr4)) {
> +				c->ip4.addr = addr4;
> +				if (prefix) {
> +					if (prefix_explicit)
> +						die("Can't use both -n and CIDR prefix");
> +					c->ip4.prefix_len = prefix;
> +					prefix_explicit = true;
> +				}
>   				if (c->mode == MODE_PASTA)
>   					c->ip4.no_copy_addrs = true;
>   				break;
> @@ -1832,11 +1898,14 @@ void conf(struct ctx *c, int argc, char **argv)
>   
>   			die("Invalid address: %s", optarg);
>   			break;
> +		}
>   		case 'n':
> +			if (prefix_explicit)
> +				die("Can't use both -n and CIDR prefix");
>   			c->ip4.prefix_len = conf_ip4_prefix(optarg);
>   			if (c->ip4.prefix_len < 0)
>   				die("Invalid netmask: %s", optarg);
> -
> +			prefix_explicit = true;
>   			break;
>   		case 'M':
>   			parse_mac(c->our_tap_mac, optarg);


      reply	other threads:[~2025-12-17  8:50 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-16 23:04 Jon Maloy
2025-12-17  8:50 ` Laurent Vivier [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=37df2e6f-3eeb-4c34-8025-2515fc5a6a87@redhat.com \
    --to=lvivier@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=dgibson@redhat.com \
    --cc=jmaloy@redhat.com \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).