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=202602 header.b=YNCrXimR; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 88F485A0265 for ; Thu, 16 Apr 2026 03:22:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1776302552; bh=1R6dlg0BdAJdKVvf0ob6Pp2Mol5PDo/8PcE+YlZH6Xo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YNCrXimRL1khKdJRDKKcPtIEA7YLLdEX7sDMrSqsHmvsqociyLIHmogs/ikSfQrtN OGXtCYjpDXFngNotAefX4JCBlotlz2+t0dHmFAVF1v4tEVs9nFyxytdMC4FbKCONDj ftZ+AJqFeAI15N09+7UR/dFH/LXMXjxluUpjTPDXcOLIIppWeqgtpJzRn0opYWlsB8 44vMuNEFNBYcNsVmiMSRH888zKGtO0OYN0v5D8LoJOJCNJ4A9gmVY1dBDiWmrYo6xt U9Qe3En1IUZoHrWuFePCeSHQ3wCmvA5iH2yNow497WBOd6jjGn15v07YDEsKck2RNG lsvZ28K2MWvwQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fx0cS0CWqz4wTL; Thu, 16 Apr 2026 11:22:32 +1000 (AEST) Date: Thu, 16 Apr 2026 11:19:53 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 10/23] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller Message-ID: References: <20260410010309.736855-1-david@gibson.dropbear.id.au> <20260410010309.736855-11-david@gibson.dropbear.id.au> <20260416000427.64745c9c@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="4stACy+5CtETpq2e" Content-Disposition: inline In-Reply-To: <20260416000427.64745c9c@elisabeth> Message-ID-Hash: HW2JLZ4W3QWWEKPU3VR6PQ36MSXZKPTD X-Message-ID-Hash: HW2JLZ4W3QWWEKPU3VR6PQ36MSXZKPTD 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: --4stACy+5CtETpq2e Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 16, 2026 at 12:04:28AM +0200, Stefano Brivio wrote: > On Fri, 10 Apr 2026 11:02:56 +1000 > David Gibson wrote: >=20 > > Amongst other checks, fwd_rule_add() checks that the newly added rule > > doesn't conflict with any existing rules. However, unlike the other th= ings > > we verify, this isn't really required for safe operation. Rule conflic= ts > > are a useful thing for the user to know about, but the forwarding logic > > is perfectly sound with conflicting rules (the first one will win). > >=20 > > In order to support dynamic rule updates, we want fwd_rule_add() to bec= ome > > a more low-level function, only checking the things it really needs to. > > So, move rule conflict checking to its caller via new helpers in > > fwd_rule.c. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 5 +++++ > > fwd.c | 26 +------------------------- > > fwd_rule.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ > > fwd_rule.h | 2 ++ > > 4 files changed, 53 insertions(+), 25 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index 027bbac9..b871646f 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct = ctx *c, char optname, > > =20 > > if (c->ifi4) { > > rulev.addr =3D inany_loopback4; > > + fwd_rule_conflict_check(&rulev, > > + fwd->rules, fwd->count); > > fwd_rule_add(fwd, &rulev); > > } > > if (c->ifi6) { > > rulev.addr =3D inany_loopback6; > > + fwd_rule_conflict_check(&rulev, > > + fwd->rules, fwd->count); > > fwd_rule_add(fwd, &rulev); > > } > > } else { > > + fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); > > fwd_rule_add(fwd, &rule); > > } > > base =3D i - 1; > > diff --git a/fwd.c b/fwd.c > > index c05107d1..c9637525 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const stru= ct fwd_rule *new) > > /* Flags which can be set from the caller */ > > const uint8_t allowed_flags =3D FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_= ANY; > > unsigned num =3D (unsigned)new->last - new->first + 1; > > - unsigned i, port; > > + unsigned port; > > =20 > > assert(!(new->flags & ~allowed_flags)); > > /* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */ > > @@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const str= uct fwd_rule *new) > > if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->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]; > > - const struct fwd_rule *rule =3D &fwd->rules[i]; > > - > > - if (new->proto !=3D rule->proto) > > - /* Non-conflicting protocols */ > > - continue; > > - > > - if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule))) > > - /* Non-conflicting addresses */ > > - continue; > > - > > - if (new->last < rule->first || rule->last < new->first) > > - /* Port ranges don't overlap */ > > - continue; > > - > > - die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u", > > - inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)), > > - new->first, new->last, > > - inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)), > > - rule->first, rule->last); > > - } > > - > > fwd->rulesocks[fwd->count] =3D &fwd->socks[fwd->sock_count]; > > for (port =3D new->first; port <=3D new->last; port++) > > fwd->rulesocks[fwd->count][port - new->first] =3D -1; > > diff --git a/fwd_rule.c b/fwd_rule.c > > index a034d5d1..5bc94efe 100644 > > --- a/fwd_rule.c > > +++ b/fwd_rule.c > > @@ -93,3 +93,48 @@ void fwd_rules_info(const struct fwd_rule *rules, si= ze_t count) > > info(" %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf))); > > } > > } > > + > > +/** > > + * fwd_rule_conflicts() - Test if two rules conflict with each other > > + * @a, @b: Rules to test > > + */ > > +static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct = fwd_rule *b) > > +{ > > + if (a->proto !=3D b->proto) > > + /* Non-conflicting protocols */ > > + return false; > > + > > + if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b))) > > + /* Non-conflicting addresses */ > > + return false; > > + > > + assert(a->first <=3D a->last && b->first <=3D b->last); >=20 > I expected this assert() to be gone by the end of the series, like the > ones dropped in 11/23, but it's still there. Is this something that the > client can't specifically trigger for some reason? Oops, yeah, that's a mistake. What I had in mind is that the rule is first checked for internal consistency (including first <=3D last), and only then do we conflict check. But then I changed things so that's no longer the case. I'll fix it with followup patches in the next spin. --=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 --4stACy+5CtETpq2e Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmngOTQACgkQzQJF27ox 2GfZGRAAhraRN3hotVsCaBMLEt5f9kEsTRYEsoLvT6divKTueFFtUJbSFyAMlT6m HgnCIVQttSTzYG4govEdDpWNzflBkOYEEJODRYBzQ9QwFl8ENcOl2cJ195avhssW U/5bMXfG/9A2GT3Q7ZkIScVTs6DPRJwew1s8GQHX/+cxQs/l0MW0tRcm58/1YwlP A1Ayp/7rqAO7Cm8+o2bB7sSXAywbXIiVzJMp75V29g7RHlm41hZPRdAgYtpbiHsC eTWERHZZkeNb7/ngwzc6hI0qHpWzQZaZkxVnglzJU07G6fEvjdwYC9BgLmI0eK3c PiHUgGLD5PsR1xuNYzaITqBW7vMvfyrSdNgoaTYLY6JFwkH02+UdAusf7Rm3QxKf YlbmhANM28Ryask70x8QT4p+T1y9oDQJh3w4BVBy44yZslMEMIfuxT6FqV+eosK0 MTbx6R+lkhPeLDkcB16PZBDsmn/53R3jZMudYgufP6UzhAsxAwV7TpXFlB/e5dKz fAHdPy8fB+I8eE+Xl+OQZ9dX5QY1CdmQXu7zD5I058w+xoN/xem+CLh3s0SeR3/K mBOgX8raZMQkV3MfGrBFExaJ+UTCZjQNk9FERydbPshhZ6ALCXG/SasVae2LThbq Jc8l2136lIg9FHZBGyviBYfUNjkpsB+LZUDUxwlPp0L/XDVirC0= =dbDf -----END PGP SIGNATURE----- --4stACy+5CtETpq2e--