Re: [PATCH 1/3] fwd_rule: Parse target adddresses for forwarding rules

Nit, in the title: addresses. On Wed, 1 Jul 2026 17:08:09 +1000 David Gibson wrote:

Extend the parsing of forwarding rules (-[tu]) to allow the destination address on the target side to be specified. For now just parse them, and give an error if we try to create rules with a specified target address. We'll implement the actual forwarding logic in another patch.

Format (for either command line or pesto): -t 2222:192.0.2.1/2222

This should work along with all the other bits, that is, say: -t 192.0.2.1%eth0/2222-2225:192.0.2.2/22-25

FIXME: Ban for -[TU] for now FIXME: Check interaction with splice handling

Signed-off-by: Stefano Brivio [dwg: Syntax from Stefano's earlier draft, largely rewritten on top of new parsing helpers] Signed-off-by: David Gibson --- fwd_rule.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/fwd_rule.c b/fwd_rule.c index ca409eaf..e8abc884 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) * @first: First port to forward * @last: Last port to forward * @exclude: Bitmap of ports to exclude (may be NULL) - * @to: Port to translate @first to when forwarding + * @tgt_addr: Destination address on the target side + * @tgt_first: Destination port to use for @first on the traget side

Nit: target.

* @flags: Flags for forwarding entries */ static void fwd_rule_range_except(struct fwd_table *fwd, bool del, uint8_t proto, const union inany_addr *addr, const char *ifname, uint16_t first, uint16_t last, - const uint8_t *exclude, uint16_t to, + const uint8_t *exclude, + const union inany_addr *tgt_addr, + uint16_t tgt_first, uint8_t flags) { struct fwd_rule rule = { @@ -395,9 +398,17 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del, .flags = flags, }; char rulestr[FWD_RULE_STRLEN]; - unsigned delta = to - first; + unsigned delta = tgt_first - first;

Nit: should be moved above now.

unsigned base, i;

+ if (tgt_addr && !inany_is_unspecified(tgt_addr)) { + char astr[INANY_ADDRSTRLEN]; + + info("Target address: %s", + inany_ntop(tgt_addr, astr, sizeof(astr))); + die("Target address remapping not yet implemented"); + } + if (!addr) rule.flags |= FWD_DUAL_STACK_ANY; if (ifname) { @@ -458,14 +469,17 @@ enum fwd_port_chunk_kind { * @cursor: Parsing point (see parse.c) * @kindp: Updated with kind of chunk we parsed * @lrange: Updated with listening port range (for INCLUDE & EXCLUDE) + * @taddr: Updated with target address (for INCLUDE) * @trange: Updated with target port range (for INCLUDE) */ static bool parse_port_chunk(const char **cursor, enum fwd_port_chunk_kind *kindp, struct port_range *lrange, + union inany_addr *taddr, struct port_range *trange) { struct port_range lr = { 0 }, tr = { 0 }; + union inany_addr taddr_tmp = inany_any6; enum fwd_port_chunk_kind kind; const char *p = *cursor;

@@ -481,6 +495,12 @@ static bool parse_port_chunk(const char **cursor, kind = CHUNK_INCLUDE;

if (parse_literal(&p, ":")) { + const char *tgtspec = p; + + if (!parse_inany(&p, &taddr_tmp) || + !parse_literal(&p, "/"))

This is to support ":/PORT" together with ":ADDR/PORT", right? I think it's nice to have for users, but it looks inconsistent here, so maybe you could add a comment before that, say: /* Accept :/PORT as well as :ADDR/PORT */

+ p = tgtspec; /* No target address, backtrack */ + if (!parse_port_range(&p, &tr)) return false; } else { @@ -492,6 +512,8 @@ static bool parse_port_chunk(const char **cursor,

*kindp = kind; *lrange = lr; + if (taddr) + *taddr = taddr_tmp; if (trange) *trange = tr; *cursor = p; @@ -561,7 +583,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, /* Consider excluded ranges and "auto" in the first pass */ p = spec; do { - if (!parse_port_chunk(&p, &kind, &lrange, NULL)) + if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL)) goto bad;

switch (kind) { @@ -586,8 +608,9 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, p = spec; do { struct port_range trange; + union inany_addr taddr;

- if (!parse_port_chunk(&p, &kind, &lrange, &trange)) + if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange)) goto bad;

switch (kind) { @@ -604,7 +627,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,

fwd_rule_range_except(fwd, del, proto, addr, ifname, lrange.first, lrange.last, - exclude, trange.first, flags); + exclude, &taddr, trange.first, + flags); break; default: goto bad; @@ -620,7 +644,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto,

fwd_rule_range_except(fwd, del, proto, addr, ifname, 1, NUM_PORTS - 1, exclude, - 1, flags | FWD_WEAK); + NULL, 1, flags | FWD_WEAK); } return; bad:

The rest of the series looks good to me, but I didn't test things at all. By the way, man page and usage changes for 3/3 are missing. I haven't seen anything preventing mapping between IPv4 and IPv6, but I guess I missed something. I actually think it would be a nice feature but I guess it needs some more effort to properly support it. -- Stefano