From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PhoDx5eg; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 3EC635A0265 for ; Wed, 01 Jul 2026 02:07:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1782864475; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rrSWiIyDyS1FFLgIOkf08vlBlxSgVyec9zoR3+gl5Y0=; b=PhoDx5egXcwcQmvHNzNmPyA1O+S/8+el2LzoUf52GgcXXk0r/Q5FnTcHzQ9+3/1XrhMcIW YNoEj6I9EleQCy/Ab8KQDoWLP/sVcf8Jdsru0HSPbpvRkjWVcVrd3HwAPILEX7GKzNtJpS BBC1NLg0D20Bhtba+bXOXLjqaQ6JGY4= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-74-xstBnhcWNFa8Zpkruk4pqA-1; Tue, 30 Jun 2026 20:07:53 -0400 X-MC-Unique: xstBnhcWNFa8Zpkruk4pqA-1 X-Mimecast-MFC-AGG-ID: xstBnhcWNFa8Zpkruk4pqA_1782864472 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-493b54823bdso8573715e9.0 for ; Tue, 30 Jun 2026 17:07:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782864472; x=1783469272; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rrSWiIyDyS1FFLgIOkf08vlBlxSgVyec9zoR3+gl5Y0=; b=hAWhN6jo3AhBMjQ4/CN8Tuz8w7VSOx4CYxK7QoDkGg2QxlrF5uR2qWh78sqo4O0MRk 4IUgapSyVRUqzq3WRmINHKj8tSlYLNZJbSLTW1KZaKYhRW5PRGZ3BGwBLqPMZADhDNQY HT9NZsJxkiRduqrL5HTFtin6FoUSpqi0d10GUjbzeWMmQehLWw+3sTbZCMlappR2Frmw Au1VsVJM2hz5F4nYPivtM8JWn5R4SktMKRUy9vzLi6ASbJlL8w2eo5rUx5N3yjB0j3Fd tIZijfx6ntwj1i922bFi7u+ndS0fcFWtba8OYafGp+bpVj9gkWW2NdfdKHYB8dwWlqHr rkIA== X-Gm-Message-State: AOJu0YxWIEpanneTkdOSVepzoKLdJnLw+Jxpqom0eIYzWGDw4A2ajkvE pP/8N72xu4BvmCpBEK6d2yzinrwLtBdMVtC8I8SRrGPvnVAjru+txxFgf316/i48CwzlnciFuPb +LbedPal6hvDvbaOglcFt3kMpOmiRvFVbarv7fjZd2/uBJt/IPgAKPQ== X-Gm-Gg: AfdE7cmVqinlBj9K4YPqYkU0VbU+784t69zp3ZeFmm8dR3ElRAP5rsKfOy/+k3xwqlh 2stQerfSeaR600RM2zTKjS+g1YvwOa7vJ1PkqXlirmbzeDkHQePPuBfH9jSyt3RjxjFWYasMxSR talDqjAD6+gL+BdNk0I8Gsfkcpp7rcvz8e9l+3/0ACyQYzVkjen54K4VfuDxWp03uXg/ZzZ9RHg Th5+KhQfJTIulWKxAByaWaW+u1Rw6/m/4eIuFpbhAplwePVD2u1Bf+rwhnTNZstR4U070qu2WED On7WFZS9RIefkwCpFml5Rkqo4lNsHCkbN00U9M+cnwjIck589VdvNUWfrDLFoyV9i4OVGIsCdg0 IpdMYX8W3CFnZ10udRTUloQ== X-Received: by 2002:a05:600c:c0da:b0:492:3fb5:3a17 with SMTP id 5b1f17b1804b1-493bc525d29mr45508105e9.2.1782864472176; Tue, 30 Jun 2026 17:07:52 -0700 (PDT) X-Received: by 2002:a05:600c:c0da:b0:492:3fb5:3a17 with SMTP id 5b1f17b1804b1-493bc525d29mr45507825e9.2.1782864471656; Tue, 30 Jun 2026 17:07:51 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-493be4bfdd5sm56422125e9.1.2026.06.30.17.07.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jun 2026 17:07:51 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 10/12] conf: Use new parsing tools to handle -a option Message-ID: <20260701020750.765493ee@elisabeth> In-Reply-To: <20260626071003.3472194-11-david@gibson.dropbear.id.au> References: <20260626071003.3472194-1-david@gibson.dropbear.id.au> <20260626071003.3472194-11-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 01 Jul 2026 02:07:50 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: -7nv3s-78rK-vqYnnXMSpm-fDaYExbbMggYWHQ4BcQU_1782864472 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: GI3OFFGMDMRIJ3V6WGSDUT26GRC6QNH7 X-Message-ID-Hash: GI3OFFGMDMRIJ3V6WGSDUT26GRC6QNH7 X-MailFrom: sbrivio@redhat.com 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: On Fri, 26 Jun 2026 17:10:01 +1000 David Gibson wrote: > 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. > > 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. 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). 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. > > Cc: Jon Maloy > > 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(-) > > 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 += -DPESTO > pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c > pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h > -pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c > pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h > pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c > pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.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; I think parse_af would be more readable (and also more consistent with e.g. prefix_len). > const char *p = arg; > - uint8_t prefix_len; > bool is_prefix; > > - is_prefix = 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 = opt_n; > - else if (!is_prefix) > - prefix_len = inany_default_prefix_len(&addr); > + if (!parse_inany_(&p, &addr, &parseaf)) ...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. > + goto bad; > + a4 = inany_v4(&addr); > + > + if ((is_prefix = parse_literal(&p, "/"))) { The current return convention makes more sense here, but I wouldn't find: is_prefix = !parse_literal(&p, "/") outrageous, either. > + /* 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 == AF_INET) { > + if (prefix_len > 32) > + goto bad_prefix; > + prefix_len += 96; > + } else if (prefix_len > 128) { > + goto bad_prefix; > + } > + } else { > + /* Get prefix length from elsewhere */ > + if (opt_n && a4) > + prefix_len = opt_n; > + else > + 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", arg); > + if (!parse_eoi(p) || Note: with *p instead of !parse_eoi(), this would fit on one line. > + !inany_is_unicast(&addr) || > + inany_is_loopback(&addr)) > + goto bad; > > - if (inany_v4(&addr)) { > - c->ip4.addr = *inany_v4(&addr); > + if (a4) { > + c->ip4.addr = *a4; > c->ip4.prefix_len = prefix_len - 96; > c->ip4.addr_fixed = true; > - if (c->mode == MODE_PASTA) 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). > - c->ip4.no_copy_addrs = true; > + c->ip4.no_copy_addrs = true; > } else { > c->ip6.addr = addr.a6; > c->ip6.addr_fixed = true; > - if (c->mode == MODE_PASTA) > - c->ip6.no_copy_addrs = true; > + c->ip6.no_copy_addrs = true; > } > > return is_prefix; > + > +bad_prefix: > + die("Invalid prefix length: %s", arg); > +bad: > + die("Invalid guest address: %s", arg); > } > > /** > 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) > > 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] = { 0 }; > - size_t alen = strcspn(src, "/"); > - const char *pstr = &src[alen + 1]; > - const char *p = astr; > - 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; > - > - /* Read address */ > - if (inet_pton(AF_INET6, astr, dst)) { > - if (inany_v4(dst) && plen < 96) > - return 0; > - *prefix_len = plen; > - return 1; > - } > - > - if (parse_inany(&p, dst) && parse_eoi(p)) { > - if (plen > 32) > - return 0; > - *prefix_len = 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_addr *dst, in_port_t *port, > > 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); > > #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, struct in6_addr *abuf) > } > > /** > - * 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 address > + * family. So "::ffff:192.0.1.1", will set @parseaf to AF_INET6, despite 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; > > - if (parse_ipv6(cursor, &addr->a6)) > + if (parse_ipv6(cursor, &addr->a6)) { > + if (parseaf) > + *parseaf = AF_INET6; > return true; > + } > > if (parse_ipv4(cursor, &a4)) { > *addr = inany_from_v4(a4); > + if (parseaf) > + *parseaf = AF_INET; > return true; > } > > 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) > > #endif /* _PARSE_H */ -- Stefano