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=cxX5byVe; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A7C285A0269 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=nDXygo0eIalTWKLO0TMi1pr/XTkpB+KtGX3S1bKmCQo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cxX5byVeZn1yORpYfdpuO++KvIiSC2T2/Kn41THdOcvnx7kTY3eXSj3UUej5Ae5MV W9YsHAmqzVvQYghNLRPWKGYnVGJa8UkyV9MBxG/P/2/kdrmbkm50+rfIFlfrYHQZxP C1BJ0diBk5gQ3dMeCKdD3Bbo+GVLDBDJLRZdAvWcIrnOmQ42jmIpPRXOQsPoslu160 fn0vzbZJqi8z1msz4eLIkAg3YHGZvJEvlTSUJTTpLEN09VPelnWEZsTqb0WUYdhcDO dyn3Z23osxzez+XVLtO9oj+kV5bVpsMR1VOUG75Z9pog9W0ZUdRPnAgYo7S/4B70ix O9UyZ2hbOOGrw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fx0cS0LNtz4wTJ; Thu, 16 Apr 2026 11:22:32 +1000 (AEST) Date: Thu, 16 Apr 2026 11:21:53 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 11/23] fwd: Improve error handling in fwd_rule_add() Message-ID: References: <20260410010309.736855-1-david@gibson.dropbear.id.au> <20260410010309.736855-12-david@gibson.dropbear.id.au> <20260416000432.49626da5@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="YQOX1oooN/D8v2EY" Content-Disposition: inline In-Reply-To: <20260416000432.49626da5@elisabeth> Message-ID-Hash: GIKOF2FSSP32VA644PROOE6RII3LAKBX X-Message-ID-Hash: GIKOF2FSSP32VA644PROOE6RII3LAKBX 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: --YQOX1oooN/D8v2EY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 16, 2026 at 12:04:33AM +0200, Stefano Brivio wrote: > On Fri, 10 Apr 2026 11:02:57 +1000 > David Gibson wrote: >=20 > > fwd_rule_add() sanity checks the given rule, however all errors are fat= al: > > either they're assert()s in the case of things that callers should have > > already verified, or die()s if we run out of space for the new rule. > >=20 > > This won't suffice any more when we allow rule updates from a > > configuration client. We don't want to trust the input we get from > > the client any more than we have to. > >=20 > > Replace the assert()s and die()s with a return value. Also include war= n()s > > so that the user gets a more specific idea of the problem in the logs or > > stderr. > >=20 > > Signed-off-by: David Gibson > > --- > > conf.c | 15 ++++++++++++--- > > fwd.c | 41 +++++++++++++++++++++++++++++++---------- > > fwd.h | 2 +- > > fwd_rule.c | 3 +-- > > fwd_rule.h | 1 + > > 5 files changed, 46 insertions(+), 16 deletions(-) > >=20 > > diff --git a/conf.c b/conf.c > > index b871646f..5c913820 100644 > > --- a/conf.c > > +++ b/conf.c > > @@ -157,6 +157,7 @@ static void conf_ports_range_except(const struct ct= x *c, char optname, > > .proto =3D proto, > > .flags =3D flags, > > }; > > + char rulestr[FWD_RULE_STRLEN]; > > unsigned delta =3D to - first; > > unsigned base, i; > > =20 > > @@ -207,20 +208,28 @@ static void conf_ports_range_except(const struct = ctx *c, char optname, > > rulev.addr =3D inany_loopback4; > > fwd_rule_conflict_check(&rulev, > > fwd->rules, fwd->count); > > - fwd_rule_add(fwd, &rulev); > > + if (fwd_rule_add(fwd, &rulev) < 0) > > + goto fail; > > } > > if (c->ifi6) { > > rulev.addr =3D inany_loopback6; > > fwd_rule_conflict_check(&rulev, > > fwd->rules, fwd->count); > > - fwd_rule_add(fwd, &rulev); > > + if (fwd_rule_add(fwd, &rulev) < 0) > > + goto fail; > > } > > } else { > > fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); > > - fwd_rule_add(fwd, &rule); > > + if (fwd_rule_add(fwd, &rule) < 0) > > + goto fail; > > } > > base =3D i - 1; > > } > > + return; > > + > > +fail: > > + die("Unable to add rule %s", > > + fwd_rule_fmt(&rule, rulestr, sizeof(rulestr))); >=20 > Similar to my comment to 10/23: I expected this die() to be gone by the > end of the series, but it's still there. Is it just a placeholder die() > that will go once pesto(1) is added? No, this one's intended to stay in pesto, at least to begin with. This is in the parsing path, not the rule insertion path. So once pesto is involved this will die() *in pesto*, which is correct[0]. When we receive new rules from pesto in passt we bypass this giong direct to fwd_rule_add(). --=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 --YQOX1oooN/D8v2EY Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmngObAACgkQzQJF27ox 2GeHFw//bF9r7+RO2+SKUthm0FO9rygkfdsxVcCbKrTqZIATPx0H4H6XUv488VN0 NXWS/e7LTZ4U8O+WxfS9WBiAxU8I5Txc5u3OahaFq6NeMHBYICnTTgutsx+1Db1w ZLj6amWJ0/+MALoVzC54F03VjmKG8wp7qo/xIOkvmN9gBpfO97TeIXptudkDPkLK 235Wc/awgfsUiGFZndG7omzgksKf/dR8ohVBn2+uPb9XekLeS28WEmq1jWkMykKh 57sQMTwmDCt3p+NtXa9TnAF0n7UAHmNgQ8RH2vuB9t25A/Fy7Sjze7PN54JW+V4Q 8uv9i0D1L3Nm8D8BW+qayzurQv/goV9KejpNnbOHq8MuLR3JzQLOZAMMWcgHH4Ri EcAqBeCd0pOjSckmh5m0dwY8dS8I8bMEl6TCVeqYBH9yxruCzF2Fu1o9vTugcnQH +Ti3toj4WcYRao+m7VhTWgUwKs+AxxlUsdt9cvXTHHcl/b9fEquYn1S8FfR4QEIV 7KkKJgLGatuh6+FqTMMKA0lAHlZFoTkNUi485oC1rMiMVRg/pj+TqG5deLOzaTwy Xc8ekWeXdphF9GK00OUweaSwVZ/VrEq0qOWjnb6AL9bdFX33D8xAF5DSZoaaY0hv JgBz6U45Mfw/lP6+gUC/1lCgHgbIgErwHGzkLUwnE9z/Qzj3opw= =S6C1 -----END PGP SIGNATURE----- --YQOX1oooN/D8v2EY--