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=202512 header.b=neHZJxz5; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id A08615A0623 for ; Tue, 13 Jan 2026 11:04:40 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1768298676; bh=TkcTSWpk5tiyrC+8ZYhIQftOq5y02uNOqwhy+Op2v7o=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=neHZJxz5rfCU35+3BXJn4eBH7JZqqSKvjkSMKY2cwTVd5BGA23jsi97TQmVungTjU b18pJUBemNor2D5G+4a6sIsirN/KYINIHM4HAgULj64fLgYXwfqgFG1OZbpMnZ6aAG noZQbF6+lOc3hE3/nJMXArI8dgUYk36LGCaVOC/B6cYu3W6OvQ9S41eQ5tn5Pgm+C6 7bBytVm75wCQylBVfw2DZ4Otc92rXA2SV4WiXimQXRxqgW2f428MPUz4frxDb1/Uke +grcZoryQJIJVjKX+20zNHKP6z0s1Rv/Gnt8TQLfMIPRqgtyLzXZWagSOK/jcZXCgo dA9h60ffqtmCQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dr4bm6Bb9z4wB9; Tue, 13 Jan 2026 21:04:36 +1100 (AEDT) Date: Tue, 13 Jan 2026 16:41:13 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 09/14] conf, fwd: Check forwarding table for conflicting rules Message-ID: References: <20260108022948.2657573-1-david@gibson.dropbear.id.au> <20260108022948.2657573-10-david@gibson.dropbear.id.au> <20260113002646.5fcf58b2@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DXQBWMwKMd3SRyGY" Content-Disposition: inline In-Reply-To: <20260113002646.5fcf58b2@elisabeth> Message-ID-Hash: SAQMKI2HS66NSPNLI466YOAHWGIT2S7W X-Message-ID-Hash: SAQMKI2HS66NSPNLI466YOAHWGIT2S7W 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 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: --DXQBWMwKMd3SRyGY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 13, 2026 at 12:26:46AM +0100, Stefano Brivio wrote: > On Thu, 8 Jan 2026 13:29:43 +1100 > David Gibson wrote: >=20 > > It's possible for a user to supply conflicting forwarding parameters, e= =2Eg. > > $ pasta -t 80:8080 -t 127.0.0.1/80:8888 > >=20 > > We give a warning in this case, but it's based on the legacy > > forwarding bitmaps. This is too strict, because it will also warn on > > cases that shouldn't conflict because they use different addresses, > > e.g. > > $ pasta -t 192.0.2.1/80:8080 127.0.0.1/80:8888 > >=20 > > Theoretically, it's also too loose because it won't take into account > > auto-scan forwarding rules. We can't hit that in practice now, > > because we only ever have one auto-scan rule and nothing else, but we > > want to remove that restriction in future. > >=20 > > Replace the bitmap based check with a check based on actually scanning > > the forwarding rules for conflicts. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 5 ----- > > fwd.c | 21 ++++++++++++++++++++- > > inany.c | 19 +++++++++++++++++++ > > inany.h | 1 + > > 4 files changed, 40 insertions(+), 6 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 725cf88b..e8d6d5d9 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -172,11 +172,6 @@ static void conf_ports_range_except(const struct c= tx *c, char optname, > > for (i =3D base; i <=3D last; i++) { > > if (exclude && bitmap_isset(exclude, i)) > > break; > > - > > - if (bitmap_isset(fwd->map, i)) { > > - warn( > > -"Altering mapping of already mapped port number: %s", optarg); > > - } > > } > > =20 > > if ((optname =3D=3D 'T' || optname =3D=3D 'U') && c->no_bindtodevice= ) { > > diff --git a/fwd.c b/fwd.c > > index 70ef73a3..5208155b 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -348,7 +348,7 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t fl= ags, > > const uint8_t allowed_flags =3D FWD_WEAK | FWD_SCAN; > > unsigned num =3D (unsigned)last - first + 1; > > struct fwd_rule *new; > > - unsigned port; > > + unsigned i, port; > > =20 > > ASSERT(!(flags & ~allowed_flags)); > > =20 > > @@ -357,6 +357,25 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t f= lags, > > if ((fwd->listen_sock_count + num) > ARRAY_SIZE(fwd->listen_socks)) > > die("Too many listening sockets"); > > =20 > > + /* Check for any conflicting entries */ > > + for (i =3D 0; i < fwd->count; i++) { > > + char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN]; > > + struct fwd_rule *rule =3D &fwd->rules[i]; > > + > > + if (!inany_matches(addr, fwd_rule_addr(rule))) > > + /* Non-conflicting addresses */ > > + continue; > > + > > + if (last < rule->first || rule->last < first) > > + /* Port ranges don't overlap */ > > + continue; > > + > > + die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u", > > + inany_ntop(addr, newstr, sizeof(newstr)), first, last, > > + inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)), > > + rule->first, rule->last); >=20 > Same as comments to earlier places in fwd_rule_add(): we'll eventually > trigger this from a client so we should eventually report failure > rather than quitting. Right, and in the same way I'm planning to change all the die()s to return errors in a later patch. >=20 > > + } > > + > > new =3D &fwd->rules[fwd->count++]; > > new->flags =3D flags; > > =20 > > diff --git a/inany.c b/inany.c > > index 87a4d8b6..a8c44237 100644 > > --- a/inany.c > > +++ b/inany.c > > @@ -21,6 +21,25 @@ > > const union inany_addr inany_loopback4 =3D INANY_INIT4(IN4ADDR_LOOPBAC= K_INIT); > > const union inany_addr inany_any4 =3D INANY_INIT4(IN4ADDR_ANY_INIT); > > =20 > > +/** inany_matches - Do two addresses match >=20 > Nit: "Do [...] match?" Fixed. > > + * @a, @b: IPv[46] addresses (NULL for 0.0.0.0 & ::) > > + * > > + * Return: true if they match, false otherwise > > + * > > + * Addresses match themselves, but also with unspecified addresses of = the same > > + * family. > > + */ > > +bool inany_matches(const union inany_addr *a, const union inany_addr *= b) > > +{ > > + if (!a || !b) > > + return true; > > + > > + if (inany_is_unspecified(a) || inany_is_unspecified(b)) > > + return !!inany_v4(a) =3D=3D !!inany_v4(b); > > + > > + return inany_equals(a, b); > > +} > > + > > /** inany_ntop - Convert an IPv[46] address to text format > > * @src: IPv[46] address (NULL for unspecified) > > * @dst: output buffer, minimum INANY_ADDRSTRLEN bytes > > diff --git a/inany.h b/inany.h > > index 61b36fb4..b02c2891 100644 > > --- a/inany.h > > +++ b/inany.h > > @@ -293,6 +293,7 @@ static inline void inany_siphash_feed(struct siphas= h_state *state, > > =20 > > #define INANY_ADDRSTRLEN MAX(INET_ADDRSTRLEN, INET6_ADDRSTRLEN) > > =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_pton(const char *src, union inany_addr *dst); > > =20 >=20 > I reviewed up to 10/14 only (I have no comments on that one), I still > need a bit of time for the remaining four patches. >=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 --DXQBWMwKMd3SRyGY Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmll2vgACgkQzQJF27ox 2Gf2ZRAAkEOnltqdoxpobl8cK31yWI6139kXVr/mcKJo3/jh0WlAUvYUO76HS6UN iLcoc6d4vG6yY0kDL7S/p2/5UoWeq0KXXhV22CKBwwzLpg5AVES4THOG6jQ+K1D7 5d5or7xyEAw5QVGyJZx6pBiUnmKjYvQ9dE/2dxRAY5XezXB2eQvOPU8es4jMhuxa i74WGiS4lVnWlsbjNyeaaFInI5vkTD7R21YHhbt+L5EuULvtw2hocy2pNeT84RiN LGjsTyl7rhQmY6Usj6RPBu1ef/GSYInLat42CbUE1XIgAltAoOlJs95EV7wVDl+M mechVd8G/Liv715yKD88N9T4MfjziA7sS2p1TD4yYCUfUGkQtcSsDepVq4rlMGbG mm7bHTpkn6PbVLVeLWtj69Ltjjtjc1+ed6mUNX4Crl31e9lcFc1NaYCxshx69Qzx nI8PFBJBerebU8BqIhs+Qa/73tSc18shyLw0eex4NNC7c7HcLQWC5iG/vMdTVi2A QcMXkPmSV4T1KwO0v91qluOKT6jFN3egjDilKXMNjDrpzSfUjoSX6yW34XDKuzpe ZkSJanUL0qpPQioNJj1oQHBZUogAMzXAaVX1fhtk5TES+JXpjI5b0+/w+0FWtwWc +tbfwh2K6FXAhb6CDz8cJmWZ2Z759Jc/uLN+GJdrtSwxDn2u+L4= =4UpX -----END PGP SIGNATURE----- --DXQBWMwKMd3SRyGY--