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=ZjmS6Uez; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id CCDE15A0271 for ; Thu, 26 Mar 2026 01:00:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774483222; bh=ZU4zN6Hd5IQZoW91+j1DEkOdZ2q5cE4PgQNNcAUnkSQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZjmS6UezmU+jdio4xufBLL5zIm9MhC61Us0WFS20h2q5NsCKjhBlKohC/7snixV/L AiEkSziNaciVuI4ZaxSHiFnuYEM5WZgBQ7Z8DnHH3hRsS6t/AnyBqLKxYEb3s9ODia ++rLrBCM73lHI6pqvZDZgSjRRdJxVrzCfXVnmM/M5S1MaHnVNU4iWC1yGcVDe6HGA5 EySDgTNHBlXA5f+WlTUcv6y9o4eBu/adNZkGOlsE72aPBAIWqiDLlhKM9vHyugA4ah g+a7APKVv3+uz58+QcrkG/Doy+ADvOgvkw4koY+F7wXUS+z6Xosrm3n45IyLIO7zhZ iZ6IuBxco/Bnw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fh3nL28KSz4wSK; Thu, 26 Mar 2026 11:00:22 +1100 (AEDT) Date: Thu, 26 Mar 2026 10:54:04 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 20/25] fwd_rule: Move forwarding rule text formatting to common code Message-ID: References: <20260323073732.3158468-1-david@gibson.dropbear.id.au> <20260323073732.3158468-21-david@gibson.dropbear.id.au> <20260325015633.655d7386@elisabeth> <20260325091806.3f389776@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ZsyQeQZLVh8s2RD5" Content-Disposition: inline In-Reply-To: <20260325091806.3f389776@elisabeth> Message-ID-Hash: HTAHS37XS227IW25TEOLP24RO2EUX3GR X-Message-ID-Hash: HTAHS37XS227IW25TEOLP24RO2EUX3GR 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: --ZsyQeQZLVh8s2RD5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 25, 2026 at 09:18:07AM +0100, Stefano Brivio wrote: > On Wed, 25 Mar 2026 15:42:00 +1100 > David Gibson wrote: >=20 > > On Wed, Mar 25, 2026 at 01:56:34AM +0100, Stefano Brivio wrote: > > > On Mon, 23 Mar 2026 18:37:27 +1100 > > > David Gibson wrote: > > > =20 > > > > Move the logic for formatting forwarding rules into strings from > > > > fwd_rules_print() into fwd_rule.c where it can be shared with pesto. > > > > We also make the function explicitly construct a string, rather than > > > > directly printing with info(), for greater flexibility. > > > >=20 > > > > Signed-off-by: David Gibson > > > > --- > > > > Makefile | 11 ++++---- > > > > fwd.c | 40 +++--------------------------- > > > > fwd_rule.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++= ++++ > > > > fwd_rule.h | 11 ++++++++ > > > > 4 files changed, 94 insertions(+), 41 deletions(-) > > > > create mode 100644 fwd_rule.c > > > >=20 > > > > diff --git a/Makefile b/Makefile > > > > index bc325482..44c396e7 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -38,13 +38,14 @@ FLAGS +=3D -DVERSION=3D\"$(VERSION)\" > > > > FLAGS +=3D -DDUAL_STACK_SOCKETS=3D$(DUAL_STACK_SOCKETS) > > > > =20 > > > > PASST_SRCS =3D arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epol= l_ctl.c \ > > > > - flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c linerea= d.c \ > > > > - log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pc= ap.c \ > > > > - pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp= _vu.c \ > > > > - udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c > > > > + flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c isolatio= n.c \ > > > > + lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c= \ > > > > + pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c \ > > > > + tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user= =2Ec \ > > > > + virtio.c vu_common.c > > > > QRAP_SRCS =3D qrap.c > > > > PASST_REPAIR_SRCS =3D passt-repair.c > > > > -PESTO_SRCS =3D pesto.c inany.c ip.c serialise.c > > > > +PESTO_SRCS =3D pesto.c fwd_rule.c inany.c ip.c serialise.c > > > > SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_S= RCS) > > > > =20 > > > > MANPAGES =3D passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1 > > > > diff --git a/fwd.c b/fwd.c > > > > index a32d0a20..20409c62 100644 > > > > --- a/fwd.c > > > > +++ b/fwd.c > > > > @@ -304,20 +304,6 @@ parse_err: > > > > warn("Unable to parse %s", PORT_RANGE_SYSCTL); > > > > } > > > > =20 > > > > -/** > > > > - * fwd_rule_addr() - Return match address for a rule > > > > - * @rule: Forwarding rule > > > > - * > > > > - * Return: matching address for rule, NULL if it matches all addre= sses > > > > - */ > > > > -static const union inany_addr *fwd_rule_addr(const struct fwd_rule= *rule) > > > > -{ > > > > - if (rule->flags & FWD_DUAL_STACK_ANY) > > > > - return NULL; > > > > - > > > > - return &rule->addr; > > > > -} > > > > - > > > > /** > > > > * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap > > > > * @map: Bitmap to update > > > > @@ -497,28 +483,10 @@ void fwd_rules_print(const struct fwd_table *= fwd) > > > > unsigned i; > > > > =20 > > > > for (i =3D 0; i < fwd->count; i++) { > > > > - const struct fwd_rule *rule =3D &fwd->rules[i].rule; > > > > - const char *percent =3D *rule->ifname ? "%" : ""; > > > > - const char *weak =3D "", *scan =3D ""; > > > > - char addr[INANY_ADDRSTRLEN]; > > > > - > > > > - inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); > > > > - if (rule->flags & FWD_WEAK) > > > > - weak =3D " (best effort)"; > > > > - if (rule->flags & FWD_SCAN) > > > > - scan =3D " (auto-scan)"; > > > > - > > > > - if (rule->first =3D=3D rule->last) { > > > > - info(" %s [%s]%s%s:%hu =3D> %hu %s%s", > > > > - ipproto_name(rule->proto), addr, percent, > > > > - rule->ifname, rule->first, rule->to, weak, scan); > > > > - } else { > > > > - info(" %s [%s]%s%s:%hu-%hu =3D> %hu-%hu %s%s", > > > > - ipproto_name(rule->proto), addr, percent, > > > > - rule->ifname, rule->first, rule->last, > > > > - rule->to, rule->last - rule->first + rule->to, > > > > - weak, scan); > > > > - } > > > > + char rulestr[FWD_RULE_STRLEN]; > > > > + > > > > + info(" %s", fwd_rule_ntop(&fwd->rules[i].rule, > > > > + rulestr, sizeof(rulestr))); > > > > } > > > > } > > > > =20 > > > > diff --git a/fwd_rule.c b/fwd_rule.c > > > > new file mode 100644 > > > > index 00000000..dfbdf683 > > > > --- /dev/null > > > > +++ b/fwd_rule.c > > > > @@ -0,0 +1,73 @@ > > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > > + > > > > +/* PASST - Plug A Simple Socket Transport > > > > + * for qemu/UNIX domain socket mode > > > > + * > > > > + * PASTA - Pack A Subtle Tap Abstraction > > > > + * for network namespace/tap device mode > > > > + * > > > > + * PESTO - Programmable Extensible Socket Translation Orchestrator > > > > + * front-end for passt(1) and pasta(1) forwarding configuration > > > > + * > > > > + * fwd_rule.c - Helpers for working with forwarding rule specifica= tions > > > > + * > > > > + * Copyright Red Hat > > > > + * Author: David Gibson > > > > + */ > > > > + > > > > +#include > > > > + > > > > +#include "fwd_rule.h" > > > > + > > > > +/** > > > > + * fwd_rule_addr() - Return match address for a rule > > > > + * @rule: Forwarding rule > > > > + * > > > > + * Return: matching address for rule, NULL if it matches all addre= sses > > > > + */ > > > > +const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) > > > > +{ > > > > + if (rule->flags & FWD_DUAL_STACK_ANY) > > > > + return NULL; > > > > + > > > > + return &rule->addr; > > > > +} > > > > + > > > > +/** > > > > + * fwd_rule_ntop() - Format forwarding rule as a string > > > > + * @rule: Rule to format > > > > + * @dst: Buffer to store output (should have FWD_RULE_STRLEN bytes) > > > > + * @size: Size of @dst > > > > + */ > > > > +const char *fwd_rule_ntop(const struct fwd_rule *rule, char *dst, = size_t size) > > > > +{ > > > > + const char *percent =3D *rule->ifname ? "%" : ""; > > > > + const char *weak =3D "", *scan =3D ""; > > > > + char addr[INANY_ADDRSTRLEN]; > > > > + int len; > > > > + > > > > + inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); > > > > + if (rule->flags & FWD_WEAK) > > > > + weak =3D " (best effort)"; > > > > + if (rule->flags & FWD_SCAN) > > > > + scan =3D " (auto-scan)"; > > > > + > > > > + if (rule->first =3D=3D rule->last) { > > > > + len =3D snprintf(dst, size, > > > > + "%s [%s]%s%s:%hu =3D> %hu %s%s", > > > > + ipproto_name(rule->proto), addr, percent, > > > > + rule->ifname, rule->first, rule->to, weak, scan); > > > > + } else { > > > > + in_port_t tolast =3D rule->last - rule->first + rule->to; > > > > + len =3D snprintf(dst, size, > > > > + "%s [%s]%s%s:%hu-%hu =3D> %hu-%hu %s%s", > > > > + ipproto_name(rule->proto), addr, percent, > > > > + rule->ifname, rule->first, rule->last, > > > > + rule->to, tolast, weak, scan); > > > > + } > > > > + > > > > + if (len < 0 || (size_t)len >=3D size) > > > > + return NULL; > > > > + > > > > + return dst; > > > > +} > > > > diff --git a/fwd_rule.h b/fwd_rule.h > > > > index 84ec5cbe..59db0e95 100644 > > > > --- a/fwd_rule.h > > > > +++ b/fwd_rule.h > > > > @@ -42,4 +42,15 @@ struct fwd_rule { > > > > uint8_t flags; > > > > }; > > > > =20 > > > > +const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); > > > > + > > > > +#define FWD_RULE_STRLEN \ > > > > + (IPPROTO_STRLEN - 1 \ > > > > + + INANY_ADDRSTRLEN - 1 \ > > > > + + IFNAMSIZ - 1 \ > > > > + + sizeof(" (best effort)") - 1 \ > > > > + + sizeof(" (auto-scan)") - 1 \ > > > > + + 15) =20 > > >=20 > > > I'm not quite sure about the reason for the 15 here, is there some > > > constant we can use perhaps? =20 > >=20 > > It's the spacing and punctuation around the other displayed > > parameters, roughly the number of non-format characters in > > "%s [%s]%s%s:%hu-%hu =3D> %hu-%hu %s%s" > > (plus one for the \0). I don't love it either, but I wasn't sure how > > to make it less obscure. Is > > sizeof(" []%:- =3D> - ") > > an improvement? I'm not sure. >=20 > I think it definitely is. I can't see a better explanation than that > sizeof(), and it also looks like a good way to double check the number > of additional characters. Ok, done. > If we want to do overdo with robustness, we could perhaps think of > separately defining the format string and using it here for the > calculation as well, which makes future (albeit unlikely) updates of > the format string safer. Eh, it's a tradeoff with two poor options. Put the format next to the printf and we have the current trouble. Or, put the fomat next to the length calculation, but then it's more awkward looking at the printf to check if the parameters are correct. > But, in that case, dropping characters for each format specifier is > something we would have to do manually anyway... or not do at all and > rely on the size of the format string as it is as an upper bound, > perhaps. Right. Or a snprintf wrapper macro + external tool that derives a bound from the format string and builds a header with it. Sounds nice, but more work than I care to do at this time :). --=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 --ZsyQeQZLVh8s2RD5 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnEdZAACgkQzQJF27ox 2GcVMA/+Pd1auRx8JOH5PsswLRLN2yGSszOawrEHLSFUIloUNYULaBLC9ou4TQUv Kvw8Ykb4OiL073aQQQAOJf/6Pkgak3EIy9O9e1Ty1chsthiAWISkUHT6vI69fBBy XjSDt6QOhJnX6Cw4Avgp3Kx4Qp7DQF4NDIQCc/gJqMFc3NLD7WC7OWR7bLPGmj8h uuM1UZ1VX5VOnQ3S+vVQMjPPR/xXjjpbDTQlgZb3AJCn2f2DCS71HIdy16/BZzqD TjfwYLAZPRQZfgyMWs2sZtDpgL+cvI6ILExAz9JtaOPsyTDEKQsOOmmgU0oEzUpb 4yVLMHfOCj6XVznbjh5u55pYoIEiN4RvtSzhb0eB+/NJp9VgiDC93jwE8W+s87JD vU1+RwO1SD6VyWhHcB7OpCf9WPhfx9h0QGNSLLKM8t0irdb6K0VMZ1vUPpT8ISDw 4F9CPJYGFwrmkbI6zsnH3MmG+R8LuOc3MGTceudPEBXilcmoi0WCX/mromYwZmNu a0BYHZ/Hd8AhuDy5/1lQroEpNNmIw+DEmJ/BRYCE0Lt15oXP+vkkRXGxvceDSymM 5xkSOwdYI2LoEfOveJ+gdws8h0LroLf+wmdiWrTQ4fxhrWQh10szDa9BjyRnwz16 afrRBen+eqXnklAA4h2RUXg7rkeFe6N6m02U+0vKuDJ7P5fruqU= =veNA -----END PGP SIGNATURE----- --ZsyQeQZLVh8s2RD5--