On Thu, Jul 02, 2026 at 06:58:23AM +0200, Stefano Brivio wrote: > Nit, in the title: addresses. Received this moments before I was going to send out a v2 :) Fixed the title. > 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. Fixed. > > > * @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. Fixed. > > 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? No, this is checking for the case where we didn't get a target address at all. De Morgan's law might make it clearer: !( && 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. Ugh, yeah, I belatedly realised that. > 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. Oh sod, good point. I forgot to add that check. I think we need it for now, because I'm pretty sure it won't work end to end yet. -- 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