Re: [PATCH v3 20/25] fwd_rule: Move forwarding rule text formatting to common code

[Stefano Brivio <sbrivio@redhat.com>]

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?

> +const char *fwd_rule_ntop(const struct fwd_rule *rule, char *dst, size_t size);
> +
>  #endif /* FWD_RULE_H */

-- 
Stefano