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: > > > 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. > > > > 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 > > > > diff --git a/Makefile b/Makefile > > index bc325482..44c396e7 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -38,13 +38,14 @@ FLAGS += -DVERSION=\"$(VERSION)\" > > FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) > > > > PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ > > - flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.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.c virtio.c vu_common.c > > + flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c isolation.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.c \ > > + virtio.c vu_common.c > > QRAP_SRCS = qrap.c > > PASST_REPAIR_SRCS = passt-repair.c > > -PESTO_SRCS = pesto.c inany.c ip.c serialise.c > > +PESTO_SRCS = pesto.c fwd_rule.c inany.c ip.c serialise.c > > SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS) > > > > MANPAGES = 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); > > } > > > > -/** > > - * fwd_rule_addr() - Return match address for a rule > > - * @rule: Forwarding rule > > - * > > - * Return: matching address for rule, NULL if it matches all addresses > > - */ > > -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; > > > > for (i = 0; i < fwd->count; i++) { > > - const struct fwd_rule *rule = &fwd->rules[i].rule; > > - const char *percent = *rule->ifname ? "%" : ""; > > - const char *weak = "", *scan = ""; > > - char addr[INANY_ADDRSTRLEN]; > > - > > - inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); > > - if (rule->flags & FWD_WEAK) > > - weak = " (best effort)"; > > - if (rule->flags & FWD_SCAN) > > - scan = " (auto-scan)"; > > - > > - if (rule->first == rule->last) { > > - info(" %s [%s]%s%s:%hu => %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 => %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))); > > } > > } > > > > 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 specifications > > + * > > + * 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 addresses > > + */ > > +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 = *rule->ifname ? "%" : ""; > > + const char *weak = "", *scan = ""; > > + char addr[INANY_ADDRSTRLEN]; > > + int len; > > + > > + inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); > > + if (rule->flags & FWD_WEAK) > > + weak = " (best effort)"; > > + if (rule->flags & FWD_SCAN) > > + scan = " (auto-scan)"; > > + > > + if (rule->first == rule->last) { > > + len = snprintf(dst, size, > > + "%s [%s]%s%s:%hu => %hu %s%s", > > + ipproto_name(rule->proto), addr, percent, > > + rule->ifname, rule->first, rule->to, weak, scan); > > + } else { > > + in_port_t tolast = rule->last - rule->first + rule->to; > > + len = snprintf(dst, size, > > + "%s [%s]%s%s:%hu-%hu => %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 >= 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; > > }; > > > > +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) > > I'm not quite sure about the reason for the 15 here, is there some > constant we can use perhaps? 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 => %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(" []%:- => - ") an improvement? I'm not sure. Oh... that made me realise I'm missing 4 * (UINT16_STRLEN - 1). I've fixed that now. > > > +const char *fwd_rule_ntop(const struct fwd_rule *rule, char *dst, size_t size); > > + > > #endif /* FWD_RULE_H */ > > -- > Stefano > -- 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