From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=Rmt6XMIj; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 387D45A061B for ; Wed, 01 Jul 2026 05:04:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782875034; bh=dSiFW1psDZs3ODqp8FGtHg2BdHt6jWFMce7qLxk7jJg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Rmt6XMIjJuKEMTviZdmsqfKJ4mFFOKONsMtquCmn5MfuqNEClqq6QUKIBGyhT0yrS qVq7TaryV3KgCIkCB0RQxHP3gqWdRz2U3/2jevaVrTJ0/1uueSTCBO5dj0XsyO66FM Tk4NT0vajeYI6sfJUF7KTbYXKL9pL3+a8ZmO1jojWL3oMTES9PAVvx8KA43gfrZ/Ye T1z+73hSzHlB2kSfoqC6i/hNo+0DZNm4KqA+Jd+grHA9uIiFIy/Cu8Bg++FHm/r0Eh l5eIGH/xLUc1s0fX+mk9lOXHcgtgkdiG2S9pP4qHe5bxrvPwydStC/bE1B4KksawtA uyev8ydRlG3og== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gqlGL419rz4wbn; Wed, 01 Jul 2026 13:03:54 +1000 (AEST) Date: Wed, 1 Jul 2026 11:55:12 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 10/12] conf: Use new parsing tools to handle -a option Message-ID: References: <20260626071003.3472194-1-david@gibson.dropbear.id.au> <20260626071003.3472194-11-david@gibson.dropbear.id.au> <20260701020750.765493ee@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="vgDznxyD0+PAAgxg" Content-Disposition: inline In-Reply-To: <20260701020750.765493ee@elisabeth> Message-ID-Hash: FW43LBV5QEYHOCGBBTUWCQPDAAAUA4Y6 X-Message-ID-Hash: FW43LBV5QEYHOCGBBTUWCQPDAAAUA4Y6 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Jon Maloy X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --vgDznxyD0+PAAgxg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 01, 2026 at 02:07:50AM +0200, Stefano Brivio wrote: > On Fri, 26 Jun 2026 17:10:01 +1000 > David Gibson wrote: >=20 > > The -a command line option can take either an address prefix, or a bare > > address. Current parsing of this is pretty awkward, using the special > > purpose helper inany_prefix_pton(). With the new incremental parsing > > helpers this can be done more naturally. Rework it to use them. > >=20 > > This does requiring extending parse_inany() to parse_inany_() which also > > reports the format of the address as parse, as opposed to the family of > > the resulting address. This is so that ::ffff:192.0.1.1/112 will be > > correctly interpreted the same as 192.0.1.1/16, rather than the > > nonsensical 192.0.0.1/112. >=20 > By the way, as far as I know, ::ffff:192.0.1.1/112 is not a valid > address, because IPv4-mapped addresses must always have /96 as prefix > length (see RFC 6890, Table 20). AFAICT the /96 there is just indicating the size of the v4-mapped block, not saying you can't have longer prefixes within that block. > RFC 6052 adds some madness (e.g. 2001:db8:122:344::192.0.2.33 from > Table 1) on top, but as far as I understand you can't use that for > prefixes. >=20 > >=20 > > Cc: Jon Maloy > >=20 > > Signed-off-by: David Gibson > > --- > > Makefile | 1 - > > conf.c | 63 +++++++++++++++++++++++++++++++++++--------------------- > > inany.c | 50 -------------------------------------------- > > inany.h | 2 -- > > parse.c | 17 ++++++++++++--- > > parse.h | 5 ++++- > > 6 files changed, 58 insertions(+), 80 deletions(-) > >=20 > > diff --git a/Makefile b/Makefile > > index e2b22ddf..5757aeff 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -223,7 +223,6 @@ passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST= _REPAIR_HEADERS) seccomp_repa > > pesto.cppcheck: BASE_CPPFLAGS +=3D -DPESTO > > pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DunusedFunction:bitmap= =2Ec > > pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DunusedFunction:inany.h > > -pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DunusedFunction:inany.c > > pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DunusedFunction:ip.h > > pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DunusedFunction:serial= ise.c > > pesto.cppcheck: CPPCHECK_FLAGS +=3D --suppress=3DstaticFunction:fwd_ru= le.c > > diff --git a/conf.c b/conf.c > > index 3614776c..ff7ca5c7 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -1176,43 +1176,60 @@ int conf_tap_fd(const char *arg) > > */ > > static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n) > > { > > + unsigned long prefix_len; > > + const struct in_addr *a4; > > union inany_addr addr; > > + sa_family_t parseaf; >=20 > I think parse_af would be more readable (and also more consistent with > e.g. prefix_len). Sure. > > const char *p =3D arg; > > - uint8_t prefix_len; > > bool is_prefix; > > =20 > > - is_prefix =3D inany_prefix_pton(arg, &addr, &prefix_len); > > - > > - if (is_prefix && opt_n) > > - die("Redundant prefix length specification"); > > - > > - if (!is_prefix && > > - !(parse_inany(&p, &addr) && parse_eoi(p))) > > - die("Invalid address: %s", arg); > > - > > - if (opt_n && inany_v4(&addr)) > > - prefix_len =3D opt_n; > > - else if (!is_prefix) > > - prefix_len =3D inany_default_prefix_len(&addr); > > + if (!parse_inany_(&p, &addr, &parseaf)) >=20 > ...just call it af? It doesn't matter so much in the following code > where that comes from. Starting from here, we know it's the address > family we're using. Eh.. I don't like that, because just "af" suggests it's the address family of the address itself. That's not the case - it's very specifically the address family of the *string* not the resulting address. > > + goto bad; > > + a4 =3D inany_v4(&addr); > > + > > + if ((is_prefix =3D parse_literal(&p, "/"))) { >=20 > The current return convention makes more sense here, but I wouldn't > find: >=20 > is_prefix =3D !parse_literal(&p, "/") >=20 > outrageous, either. >=20 > > + /* Prefix length included in -a option */ > > + if (!parse_unsigned(&p, 10, &prefix_len)) > > + goto bad; > > + if (opt_n) > > + die("Redundant prefix length specification"); > > + if (parseaf =3D=3D AF_INET) { > > + if (prefix_len > 32) > > + goto bad_prefix; > > + prefix_len +=3D 96; > > + } else if (prefix_len > 128) { > > + goto bad_prefix; > > + } > > + } else { > > + /* Get prefix length from elsewhere */ > > + if (opt_n && a4) > > + prefix_len =3D opt_n; > > + else > > + prefix_len =3D inany_default_prefix_len(&addr); > > + } > > =20 > > - if (inany_is_unspecified(&addr) || inany_is_multicast(&addr) || > > - inany_is_loopback(&addr) || IN6_IS_ADDR_V4COMPAT(&addr.a6)) > > - die("Invalid address: %s", arg); > > + if (!parse_eoi(p) || >=20 > Note: with *p instead of !parse_eoi(), this would fit on one line. >=20 > > + !inany_is_unicast(&addr) || > > + inany_is_loopback(&addr)) > > + goto bad; > > =20 > > - if (inany_v4(&addr)) { > > - c->ip4.addr =3D *inany_v4(&addr); > > + if (a4) { > > + c->ip4.addr =3D *a4; > > c->ip4.prefix_len =3D prefix_len - 96; > > c->ip4.addr_fixed =3D true; > > - if (c->mode =3D=3D MODE_PASTA) >=20 > Why does this (and the same condition just below) go away here? I mean, > it's not fundamental in any case, but changing it here makes it look > like it has something to do with this change (and I guess it's not the > case). Good point, that's an unrelated change. I split it into its own patch. >=20 > > - c->ip4.no_copy_addrs =3D true; > > + c->ip4.no_copy_addrs =3D true; > > } else { > > c->ip6.addr =3D addr.a6; > > c->ip6.addr_fixed =3D true; > > - if (c->mode =3D=3D MODE_PASTA) > > - c->ip6.no_copy_addrs =3D true; > > + c->ip6.no_copy_addrs =3D true; > > } > > =20 > > return is_prefix; > > + > > +bad_prefix: > > + die("Invalid prefix length: %s", arg); > > +bad: > > + die("Invalid guest address: %s", arg); > > } > > =20 > > /** > > diff --git a/inany.c b/inany.c > > index 154f08b5..120c9387 100644 > > --- a/inany.c > > +++ b/inany.c > > @@ -70,53 +70,3 @@ const char *inany_ntop(const union inany_addr *src, = char *dst, socklen_t size) > > =20 > > return inet_ntop(AF_INET6, &src->a6, dst, size); > > } > > - > > -/** > > - * 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: 1 on success, 0 if no parseable address or prefix is found > > - */ > > -int inany_prefix_pton(const char *src, union inany_addr *dst, > > - uint8_t *prefix_len) > > -{ > > - char astr[INANY_ADDRSTRLEN] =3D { 0 }; > > - size_t alen =3D strcspn(src, "/"); > > - const char *pstr =3D &src[alen + 1]; > > - const char *p =3D astr; > > - unsigned long plen; > > - char *end; > > - > > - if (alen >=3D INANY_ADDRSTRLEN) > > - return 0; > > - > > - if (src[alen] !=3D '/') > > - return 0; > > - > > - strncpy(astr, src, alen); > > - > > - /* Read prefix length */ > > - errno =3D 0; > > - plen =3D strtoul(pstr, &end, 10); > > - if (errno || *end || plen > 128) > > - return 0; > > - > > - /* Read address */ > > - if (inet_pton(AF_INET6, astr, dst)) { > > - if (inany_v4(dst) && plen < 96) > > - return 0; > > - *prefix_len =3D plen; > > - return 1; > > - } > > - > > - if (parse_inany(&p, dst) && parse_eoi(p)) { > > - if (plen > 32) > > - return 0; > > - *prefix_len =3D plen + 96; > > - return 1; > > - } > > - > > - return 0; > > -} > > diff --git a/inany.h b/inany.h > > index 93d98368..5b176ccf 100644 > > --- a/inany.h > > +++ b/inany.h > > @@ -303,7 +303,5 @@ static inline int inany_from_sockaddr(union inany_a= ddr *dst, in_port_t *port, > > =20 > > bool inany_matches(const union inany_addr *a, const union inany_addr *= b); > > const char *inany_ntop(const union inany_addr *src, char *dst, socklen= _t size); > > -int inany_prefix_pton(const char *src, union inany_addr *dst, > > - uint8_t *prefix_len); > > =20 > > #endif /* INANY_H */ > > diff --git a/parse.c b/parse.c > > index 0349c5dc..3e0dbd45 100644 > > --- a/parse.c > > +++ b/parse.c > > @@ -184,18 +184,29 @@ static bool parse_ipv6(const char **cursor, struc= t in6_addr *abuf) > > } > > =20 > > /** > > - * parse_inany() - Parse an IPv4 or IPv6 address from a string > > + * parse_inany_() - Parse an IPv4 or IPv6 address from a string > > * @addr: On success, updated with parsed address > > + * @parseaf: On success, updated with the format of the parsed address > > + * > > + * @parseaf is updated to reflect the string format, not the final add= ress > > + * family. So "::ffff:192.0.1.1", will set @parseaf to AF_INET6, desp= ite being > > + * a IPv4-mapped address. > > */ > > -bool parse_inany(const char **cursor, union inany_addr *addr) > > +bool parse_inany_(const char **cursor, union inany_addr *addr, > > + sa_family_t *parseaf) > > { > > struct in_addr a4; > > =20 > > - if (parse_ipv6(cursor, &addr->a6)) > > + if (parse_ipv6(cursor, &addr->a6)) { > > + if (parseaf) > > + *parseaf =3D AF_INET6; > > return true; > > + } > > =20 > > if (parse_ipv4(cursor, &a4)) { > > *addr =3D inany_from_v4(a4); > > + if (parseaf) > > + *parseaf =3D AF_INET; > > return true; > > } > > =20 > > diff --git a/parse.h b/parse.h > > index 2820a065..08b038cf 100644 > > --- a/parse.h > > +++ b/parse.h > > @@ -27,6 +27,9 @@ bool parse_eoi(const char *cursor); > > bool parse_unsigned(const char **cursor, int base, unsigned long *valp= ); > > bool parse_port_range(const char **cursor, struct port_range *range); > > bool parse_ipv4(const char **cursor, struct in_addr *abuf); > > -bool parse_inany(const char **cursor, union inany_addr *addr); > > +bool parse_inany_(const char **cursor, union inany_addr *addr, > > + sa_family_t *parseaf); > > + > > +#define parse_inany(cursor, addr) parse_inany_((cursor), (addr), NULL) > > =20 > > #endif /* _PARSE_H */ >=20 > --=20 > Stefano >=20 --=20 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 --vgDznxyD0+PAAgxg Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpEc3YACgkQzQJF27ox 2Ge74g/+PbBZH/RPmVNLqzaUrH6FlrHsWk1zJY4XV6YtkCAzG6TiYwnXXX/Wbjcg Cue6UqXA7PxrI8se65WT7thhXwF4X4erV7IGNBLA0b1UAKEJdy1GtCMApXLvNf8L kQt5NhEzw9g4+/GOW094m3ysO+cIClZcFuA8H9g4ptckrW8MXbywYcjHnY//CItT u8RqYqjBGrtnVEIgZidAQKI6YROagcnvUgy+HLndDvl4EOMHEU7cfuMIHJywA/va /1wnmOcVgQOoPc63+734DUDP9w4DvUURE3OZkrA94jg4dI3JlS3I7LlEnpaTESSC hmvgz+DnmQEcIUeGmdxw+LYxL72TltGB8YOQ+g/xlUwhKMRKLtzg/YYxUK6lVeGR sNwnSRFPaZjpnqhkK6LmxRlmK2qG7L5iK5Q9u+GM41OINTqYRmxh3qob/9xLCB09 CkqBr0pgglIiBWHPNK82JoPUcA08tQGZPQ+c2ugZ+K6WX2cPFJYwkUdZsmmNBpCQ oDhXudL+ETbpx/FgVQWYsMi8aL6Dqs/29cKq0LQjPvF4kz48L1r+HShrBZY6J/KA mBSAF6YG5NiYZ5z6qawRBAn/Vfs3sV3kOesO50kR7GcvymwbkcrcoTioCF+WfWX4 ysbYd0bPSPbuNyiTfZwevdrPCXkBaig7qd6dobEH62v42YZb3Bg= =zG/b -----END PGP SIGNATURE----- --vgDznxyD0+PAAgxg--