From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 20/25] fwd_rule: Move forwarding rule text formatting to common code
Date: Wed, 25 Mar 2026 15:42:00 +1100 [thread overview]
Message-ID: <acNnmDxwLg34_wWh@zatzit> (raw)
In-Reply-To: <20260325015633.655d7386@elisabeth>
[-- Attachment #1: Type: text/plain, Size: 8032 bytes --]
On Wed, Mar 25, 2026 at 01:56:34AM +0100, Stefano Brivio wrote:
> On Mon, 23 Mar 2026 18:37:27 +1100
> David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> > ---
> > 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 <david@gibson.dropbear.id.au>
> > + */
> > +
> > +#include <stdio.h>
> > +
> > +#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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2026-03-25 4:44 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 7:37 [PATCH v3 00/25] RFC: Read-only dynamic update implementation David Gibson
2026-03-23 7:37 ` [PATCH v3 01/25] conf: runas can be const David Gibson
2026-03-23 7:37 ` [PATCH v3 02/25] vhost_user: Fix assorted minor cppcheck warnings David Gibson
2026-03-23 7:37 ` [PATCH v3 03/25] serialise: Split functions user for serialisation from util.c David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-25 1:50 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 04/25] serialise: Add helpers for serialising unsigned integers David Gibson
2026-03-23 7:37 ` [PATCH v3 05/25] fwd: Move selecting correct scan bitmap into fwd_sync_one() David Gibson
2026-03-23 7:37 ` [PATCH v3 06/25] fwd: Look up rule index in fwd_sync_one() David Gibson
2026-03-23 7:37 ` [PATCH v3 07/25] fwd: Store forwarding tables indexed by (origin) pif David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-25 4:04 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 08/25] fwd: Allow FWD_DUAL_STACK_ANY flag to be passed directly to fwd_rule_add() David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-25 4:07 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 09/25] fwd, conf: Expose ephemeral ports as bitmap rather than function David Gibson
2026-03-23 7:37 ` [PATCH v3 10/25] conf: Don't bother complaining about overlapping excluded ranges David Gibson
2026-03-23 7:37 ` [PATCH v3 11/25] conf: Move check for mapping port 0 to caller David Gibson
2026-03-23 7:37 ` [PATCH v3 12/25] conf: Move check for disabled interfaces earlier David Gibson
2026-03-23 7:37 ` [PATCH v3 13/25] pesto: Introduce stub configuration interface and tool David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-23 7:37 ` [PATCH v3 14/25] pesto: Add command line option parsing and debug messages David Gibson
2026-03-25 0:55 ` Stefano Brivio
2026-03-25 4:27 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 15/25] pesto: Expose list of pifs to pesto David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:34 ` David Gibson
2026-03-25 8:18 ` Stefano Brivio
2026-03-25 8:31 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 16/25] ip: Prepare ip.[ch] for sharing with pesto tool David Gibson
2026-03-23 7:37 ` [PATCH v3 17/25] inany: Prepare inany.[ch] " David Gibson
2026-03-23 7:37 ` [PATCH v3 18/25] fwd: Split forwading rule specification from its implementation state David Gibson
2026-03-23 7:37 ` [PATCH v3 19/25] ip: Define a bound for the string returned by ipproto_name() David Gibson
2026-03-23 7:37 ` [PATCH v3 20/25] fwd_rule: Move forwarding rule text formatting to common code David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:42 ` David Gibson [this message]
2026-03-25 8:18 ` Stefano Brivio
2026-03-25 23:54 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 21/25] pesto: Read current ruleset from passt/pasta and display it David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:43 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 22/25] conf: Move port parsing functions to own file, ports.c David Gibson
2026-03-23 7:37 ` [PATCH v3 23/25] conf, fwd, ports, util: Move things around for pesto David Gibson
2026-03-23 7:37 ` [PATCH v3 24/25] pesto, conf: Parse, send and receive new rules David Gibson
2026-03-23 7:37 ` [PATCH v3 25/25] conf, fwd: Allow switching to new rules received from pesto David Gibson
2026-03-23 8:38 ` [PATCH v3 00/25] RFC: Read-only dynamic update implementation David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 1:00 ` Stefano Brivio
2026-03-25 4:44 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=acNnmDxwLg34_wWh@zatzit \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).