From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=Y6RBAs9S; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 1586C5A0262 for ; Thu, 02 Jul 2026 07:21:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782969696; bh=qSGHZKBpCnAm0fPAOsk0Mvf4V8/1rd/yjbg7VoducGQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Y6RBAs9SYvlNYHqDckVa5D9C3pPG/BbBNNJzP3rVjY+ps4edTRnFDkkRUPIwrFqB9 mVCz/Keq5CVdfSoTqcQL+7kWM5V5oNtQQCfcgw7Ccc6XymDUmHy9k0N/FSz1yOMHKC 6zAPQEaqAFGEgJJzYN4eSB57pFgfhiO1n+HWhKHTxlWdcLZr2PEaZrG0V69ewkb/Xq 9BzlhrjPWjxzIg22IKuhDQqmiuFUNuAPIgvZHrFtjRK93DSD6bD41tPetYEPYUZ3Gh V2ESU37xc8aXoeH24gHv3Y1kEtdWxnS+V8t/fD7H4cFvfrDpSSUCL0AvirEfD0B4UM gdS+7y6vN0Cmg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4grQGm46Flz58mk; Thu, 02 Jul 2026 15:21:36 +1000 (AEST) Date: Thu, 2 Jul 2026 15:21:29 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] fwd_rule: Parse target adddresses for forwarding rules Message-ID: References: <20260701070811.1944139-1-david@gibson.dropbear.id.au> <20260701070811.1944139-2-david@gibson.dropbear.id.au> <20260702065821.745bbd05@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="tdnKl9cc+aVR/rpj" Content-Disposition: inline In-Reply-To: <20260702065821.745bbd05@elisabeth> Message-ID-Hash: JZ6Q5UOIDRU5NMWILZEJIM3OBBETJ2KO X-Message-ID-Hash: JZ6Q5UOIDRU5NMWILZEJIM3OBBETJ2KO X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --tdnKl9cc+aVR/rpj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: >=20 > > Extend the parsing of forwarding rules (-[tu]) to allow the destination > > address on the target side to be specified. For now just parse them, a= nd > > give an error if we try to create rules with a specified target address. > > We'll implement the actual forwarding logic in another patch. > >=20 > > Format (for either command line or pesto): > > -t 2222:192.0.2.1/2222 > >=20 > > 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 > >=20 > > FIXME: Ban for -[TU] for now > > FIXME: Check interaction with splice handling > >=20 > > 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(-) > >=20 > > 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 str= uct 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 >=20 > Nit: target. Fixed. >=20 > > * @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 =3D { > > @@ -395,9 +398,17 @@ static void fwd_rule_range_except(struct fwd_table= *fwd, bool del, > > .flags =3D flags, > > }; > > char rulestr[FWD_RULE_STRLEN]; > > - unsigned delta =3D to - first; > > + unsigned delta =3D tgt_first - first; >=20 > Nit: should be moved above now. Fixed. > > unsigned base, i; > > =20 > > + 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 |=3D 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 =3D { 0 }, tr =3D { 0 }; > > + union inany_addr taddr_tmp =3D inany_any6; > > enum fwd_port_chunk_kind kind; > > const char *p =3D *cursor; > > =20 > > @@ -481,6 +495,12 @@ static bool parse_port_chunk(const char **cursor, > > kind =3D CHUNK_INCLUDE; > > =20 > > if (parse_literal(&p, ":")) { > > + const char *tgtspec =3D p; > > + > > + if (!parse_inany(&p, &taddr_tmp) || > > + !parse_literal(&p, "/")) >=20 > 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: >=20 > /* Accept :/PORT as well as :ADDR/PORT */ >=20 > > + p =3D 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, > > =20 > > *kindp =3D kind; > > *lrange =3D lr; > > + if (taddr) > > + *taddr =3D taddr_tmp; > > if (trange) > > *trange =3D tr; > > *cursor =3D 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 =3D spec; > > do { > > - if (!parse_port_chunk(&p, &kind, &lrange, NULL)) > > + if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL)) > > goto bad; > > =20 > > switch (kind) { > > @@ -586,8 +608,9 @@ static void fwd_rule_parse_ports(struct fwd_table *= fwd, bool del, uint8_t proto, > > p =3D spec; > > do { > > struct port_range trange; > > + union inany_addr taddr; > > =20 > > - if (!parse_port_chunk(&p, &kind, &lrange, &trange)) > > + if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange)) > > goto bad; > > =20 > > switch (kind) { > > @@ -604,7 +627,8 @@ static void fwd_rule_parse_ports(struct fwd_table *= fwd, bool del, uint8_t proto, > > =20 > > 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, > > =20 > > 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: >=20 > 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. --=20 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 --tdnKl9cc+aVR/rpj Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpF9U0ACgkQzQJF27ox 2GcVOg/8DD8boyv7RxhmYSEtljgrdHUnLRPlx5iWNOXpxJCx3k66sHwPkFP4w+Vv qhjsMm7mtyY3Xq3sngwtfSir5ubKwVmSEuQpFOPxBduwIYCIA2FSBfd1iE83c6Hk GuEZfwym6QHXEVW79nqVlWCAlMqbo3u+Za3+7wXTCsilIXO4Kb9sSSwtPQi5bczG Mw0MOOnPzudffRs+BC7rAeVJ9vofnMySpG+cw5XOfM394rJA/edZzTDpTXuDRwhN 3vucowVF1S2QQ9uDoqEvpdQyHG5WbSbI3JCAN1swC99+ivDwEBQFUZX1GGOcS9+C EUxsQeIUeBRbN/MeLCxOwokaoS52rCkFj7spIs3wloRVKPeOf/GrUxrERo5CILFB FNOQGqF9hyqLcHtWJtOuWQWGfE4zKPRT/e+hwEBKxDS8DGC9h3asbUtR/MZYxdyy J0jhsDfJGnVA3CQXSSPFy+UbXsjGFIvQkghMt8rMq+ZiPYpTjb8EsM12fn3rY2wR mwEn51iUoswP8MBCqDSG+Vy5ZXMU1gleicUz2SfnyJpYYbHCuTzmMtkATtQQBJv5 kZ3XgwvSq69k8N3t46sUxYjbuinnp/Vie/lfM3k9nEnt0zbetqCH1WEvWfH23dNG xftXnnbRdGzkyDS2bi+Q8cMp8Kfbwi+ma+WC7miS3QH6Z40pvcc= =5WHN -----END PGP SIGNATURE----- --tdnKl9cc+aVR/rpj--