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=ZK/8FEgn; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 97B395A026D for ; Tue, 05 May 2026 16:41:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1777992079; bh=vIXq1FuodkTqZnhd85SUAEFJ29YTXv/ynaUjCmew+Jo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZK/8FEgnYrKZGNSJ6BMjPKE1CjhCamkUBXXfF22HpEOqHWDA0T4wevBxeIIM/qsBd QeaWxUhqIa08GUmzQK8Bw5+/vZbrgx3cUkKTfyLr1kQGo/gcDlOkTga5sKEKCMAzVR RfS/PB3sOOkBjuuqnxd7cZBiUGyU5F8SoLdFDCcuP+w2qZhny5+NsodI7w3uNnnCIt WN6nOZBZH+ZnFNFszQBPsyGoxf9ndu1IeDfmSdcWJOAYOdyekl/nkYT1Pvk6QDvKB4 J1NV5xoQ9L4tUPyYQWBoQfJ77hBOhebUu2kZOh/2rhD7EG/F1iMCjhiN52KGh6YuGk +z+DBKm9nUd9Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4g91RM6jHYz4wJ2; Wed, 06 May 2026 00:41:19 +1000 (AEST) Date: Wed, 6 May 2026 00:32:16 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 17/18] conf, fwd: Allow switching to new rules received from pesto Message-ID: References: <20260504231142.1118652-1-sbrivio@redhat.com> <20260504231142.1118652-18-sbrivio@redhat.com> <20260505120408.6f14cc15@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Awf8NdzqT0DmbWgZ" Content-Disposition: inline In-Reply-To: <20260505120408.6f14cc15@elisabeth> Message-ID-Hash: D6YBHTDCPSOLDM3JJN6L4527GAMVJVS5 X-Message-ID-Hash: D6YBHTDCPSOLDM3JJN6L4527GAMVJVS5 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: Laurent Vivier , 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: --Awf8NdzqT0DmbWgZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 05, 2026 at 12:04:09PM +0200, Stefano Brivio wrote: > On Tue, 5 May 2026 11:08:27 +0200 > Laurent Vivier wrote: >=20 > > On 5/5/26 01:11, Stefano Brivio wrote: > > > From: David Gibson > > >=20 > > > We can now receive updates to the forwarding rules from the pesto cli= ent > > > and store them in a "pending" copy of the forwarding tables. Impleme= nt > > > switching to using the new rules. > > >=20 > > > The logic is in a new fwd_listen_switch(). For now this closes all > > > listening sockets related to the old tables, swaps the active and pen= ding > > > tables, then listens based on the new tables. In future we look to i= mprove > > > this so that we don't temporarily stop listening on ports that both t= he > > > old and new tables specify. > > >=20 > > > Signed-off-by: David Gibson > > > Signed-off-by: Stefano Brivio > > > --- > > > conf.c | 5 ++--- > > > fwd.c | 34 ++++++++++++++++++++++++++++++++++ > > > fwd.h | 1 + > > > 3 files changed, 37 insertions(+), 3 deletions(-) > > >=20 > > > diff --git a/conf.c b/conf.c > > > index f035fd3..75b8291 100644 > > > --- a/conf.c > > > +++ b/conf.c > > > @@ -2159,15 +2159,14 @@ void conf_handler(struct ctx *c, uint32_t eve= nts) > > > fwd_rules_dump(info, fwd->rules, fwd->count, > > > " ", ""); > > > } > > > + > > > + fwd_listen_switch(c); > > > } > > > =20 > > > if (events & EPOLLHUP) { > > > debug("Configuration client hangup"); > > > - goto close; > > > } > > > =20 > > > - return; > > > - > > > close: > > > conf_close(c); > > > =20 > > > diff --git a/fwd.c b/fwd.c > > > index d93d2e5..35b9e2b 100644 > > > --- a/fwd.c > > > +++ b/fwd.c > > > @@ -534,6 +534,40 @@ int fwd_listen_init(const struct ctx *c) > > > return 0; > > > } > > > =20 > > > +/** > > > + * fwd_listen_switch() - Switch from current to pending rules table > > > + * @c: Execution context > > > + */ > > > +void fwd_listen_switch(struct ctx *c) > > > +{ > > > + struct fwd_table *tmp[PIF_NUM_TYPES]; > > > + unsigned i; > > > + > > > + /* Stop listening on the old tables */ > > > + for (i =3D 0; i < PIF_NUM_TYPES; i++) { > > > + struct fwd_table *fwd =3D c->fwd[i]; > > > + > > > + if (!fwd) > > > + continue; > > > + > > > + debug("Flushing %u old %s rules", fwd->count, pif_name(i)); > > > + fwd_listen_close(fwd); > > > + fwd->count =3D fwd->sock_count =3D 0; =20 > >=20 > > Perhaps we can reset fwd->count and fwd->sock_count in fwd_listen_close= () as after=20 > > fwd_listen_close() these values are wrong? >=20 > Right, while not strictly necessary it still looks like a good idea, > I'll change that. As noted elswhere this is correct as it is. > > > + } > > > + > > > + /* Swap active and pending tables */ > > > + static_assert(sizeof(tmp) =3D=3D sizeof(c->fwd) && > > > + sizeof(tmp) =3D=3D sizeof(c->fwd_pending), > > > + "Temporary has wrong size"); > > > + memcpy(&tmp, (void *)c->fwd, sizeof(tmp)); > > > + memcpy((void *)c->fwd, (void *)c->fwd_pending, sizeof(tmp)); > > > + memcpy((void *)c->fwd_pending, &tmp, sizeof(tmp)); =20 > >=20 > > I know we have the static_assert(), but with memcpy() we usually use th= e sizeof() of the=20 > > destination to avoid write overflow. >=20 > I'll change this as well. Sounds good. > > Why do we keep the old active table? Do we plan to have a "--restore" o= ption? >=20 > It's just to add and delete rules using a temporary table so that we > can abort cleanly and atomically on errors. >=20 > Are you asking why we don't wipe the old table afterwards? No > particular reason for that, even though I'm not sure if it's useful. Also looking ahead to listening socket continuity. In that case we need to keep the old table until we've stolen all the sockets we can re-use for the new table. *Then* we can fwd_listen_close() anything left over, and wipe the table. > Actually some kind of --restore option might be desirable, even though > we would probably need to re-validate all the rules, or keep a "dirty" > bit that's set on other types of changes and would tell us that the > previous table can't be used as it is anymore. >=20 > --=20 > Stefano >=20 >=20 > > > + > > > + /* Start listening on the new tables */ > > > + if (fwd_listen_init(c) < 0) > > > + err("Error switching to new forwarding rules"); > > > +} > > > + > > > /* See enum in kernel's include/net/tcp_states.h */ > > > #define UDP_LISTEN 0x07 > > > #define TCP_LISTEN 0x0a > > > diff --git a/fwd.h b/fwd.h > > > index ac24782..b60697d 100644 > > > --- a/fwd.h > > > +++ b/fwd.h > > > @@ -61,6 +61,7 @@ int fwd_listen_sync(const struct ctx *c, uint8_t pi= f, > > > const struct fwd_scan *tcp, const struct fwd_scan *udp); > > > void fwd_listen_close(const struct fwd_table *fwd); > > > int fwd_listen_init(const struct ctx *c); > > > +void fwd_listen_switch(struct ctx *c); > > > =20 > > > bool nat_inbound(const struct ctx *c, const union inany_addr *addr, > > > union inany_addr *translated); =20 > >=20 >=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 --Awf8NdzqT0DmbWgZ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmn5/2cACgkQzQJF27ox 2GdDIA/8DHIf/hIuNnLBSgT9qcKjR2Oi6gNXrYalTCWotUfS+90Mo6DK5PQlKzwS t/kaAmcrvt9JFDg472yZJMNsE+VcZ11Tn5T6RUEc/fWQ+ohOUaVtCxLHYOXYpZpV EgY9M5DoMbcODwbmC5frQslmeGDRxdDrd78GsKHkT1T9FM8OmgCP0cT0kaLdRos3 i52/AdkuS3va1B+BjUCy0w35P0f3Yoc6ZLIXk3/hqV1Bga7w3vSvd8Dyoj7vtmPs gQmfyZGgweTI6I50R/S+wc+CDTCin7QC0r4QoUdx1/dPk7mqT7r8qDcdjPuM84SW A1ugKpTLmeEtw1Lqs3DdwBv+pZRjJZKOVlwYhraBblapXRSzkmwlJOCVqARlaO4J Oh/fNyRDwWDYfoj8ZEjAapK3Elv+5f6Ei7OJsssdfeAFW6almaHTY2BkbCtMcnKw g5ojNOonyB/YthIFa6OvAGJ8E3Uya3GwL+lFTyThyz0aXl/l4seeXsRy0csjPsRA hRHBIpBYmr3QokpeUVU3ipu14/mhBXPQWZVzVzNmyUvN/aXDcNoyRwOva7b8kbAB 6bLFF7lpXMJzY6o/GrgVt7Sj83WZZFCdcR2p4LotUT/6Ve7TYZeZoS+T75wgidqw QNVdXvEHOc5fLsrB6XACSj7QTMdoHTGMAorcJNa1IsP0TS8CGVo= =8Tgu -----END PGP SIGNATURE----- --Awf8NdzqT0DmbWgZ--