From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=VXU+4JtW; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id B01D55A0262 for ; Sat, 04 Jul 2026 17:27:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1783178831; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HdVL7M7qhUWD7mzBAlDfZIBXRzkQbTSe633gGs1G19o=; b=VXU+4JtWRFBQPsT/gjWP0XfTFa3e35kSdFSBwIVBTBx25gQxRrJS001oYlYHm8cNv9pBI7 u0d6yykv+qFH7nKdhNlZay15schLVLROwDQvqSJjvbEOp3VjgXYVQRu7FAeIwid5dX4v1t i4iHVgo1AXtzeXbxXWl8/Tfo/SsLzTk= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-602--BhqyvCTN0W2CvL6IkgICA-1; Sat, 04 Jul 2026 11:27:10 -0400 X-MC-Unique: -BhqyvCTN0W2CvL6IkgICA-1 X-Mimecast-MFC-AGG-ID: -BhqyvCTN0W2CvL6IkgICA_1783178829 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4925bf70f5bso13493585e9.1 for ; Sat, 04 Jul 2026 08:27:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783178829; x=1783783629; h=date:content-transfer-encoding:content-type:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=gKm2wdJLMhAMt/2phG5U4j2OaQ/EmOdmwwiucXgEuSI=; b=ZgD08rB7ksYzuRBxvNQ7ypsSqnU7hMIjKgNkDpglfX/IEGPqQ0OLlsnvuPP5LgfdkM N13mda+b5g5+pZd2guvzB0sUkneZJ0zM1Y3wbbRRQp/w5L21o+1eDsB7MW26mfwm4Ojg 51GaQdGCTKCY+bJrfe17U7p+n9o95m0PctnKz6zctV3LF3jL/gs9scTvURGYpfIOY6S1 mBq5/skaJkR5e9DHGFAHDOXyUtgoh9sSKyMMnKB9grpM2/b0ttvxqgsDifgBmlfr/vPf aSuOjemOi1EtACNEDjK8sgxNN9YvKdmX5Og8RpkawtvvZ+uxB9BlSmlhraNdpDiakQQs q/wQ== X-Gm-Message-State: AOJu0Yx73taAuHvow+dLPGWXVhaCJO41ZvLOQmpUwUBPo+cDACUjDOdk sgHTj1mi8feZ10B5tSG4VYieS7st9PID+SmvFNyIaZpe5QSKgJRntR/0AYc5KbttZwF3xQz/iuM zEnukZtfShUEvRvtUUldjTZJ+Zik724YxeEkfBKdlTEL3qNRkbxvipMxUvVDzlA== X-Gm-Gg: AfdE7cmA7djUgS5uC8c6DSHKVNHI2Yup8LwrlzSL7XW0hmHoW9AN50gFDarE6UkPqb/ 1TwB4iaFNGWLZxN0Et9aLTnTPBgKnhfLEuWaaXJrSgYMA/BBvS8mxVWb2yOBkV30kGXXc7t/8By ON2faQHo8Uqa0oRi+zY5tV4l7QsvGbRpfex+4CL0RBKJm4dgKiW7j865jtNHvOApzLlkXFiKUD/ rUWz0CMzMlbHF91LKtSFj3lM23e6jDhJZ6afgsP0FONgjrRZijCJt8/T8fBeVxJgQSdvtvW0Txq m7BcPZ4ML63dzd6oLkYvLZl497WLJFwRo1AqK/Uc90yKquG/E16CXlgzSyjLJy9NRplOj3sQeQb T8UINgtlNCTGISROyL3ST9Q== X-Received: by 2002:a05:600c:811a:b0:493:bc92:ba9a with SMTP id 5b1f17b1804b1-493d11d7e5bmr35799635e9.13.1783178828329; Sat, 04 Jul 2026 08:27:08 -0700 (PDT) X-Received: by 2002:a05:600c:811a:b0:493:bc92:ba9a with SMTP id 5b1f17b1804b1-493d11d7e5bmr35799325e9.13.1783178827712; Sat, 04 Jul 2026 08:27:07 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-493be4a343csm224051595e9.0.2026.07.04.08.27.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 04 Jul 2026 08:27:06 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 1/2] fwd_rule: Parse target addresses for forwarding rules Message-ID: <20260704172705.71da2cf3@elisabeth> In-Reply-To: <20260702073215.751291-2-david@gibson.dropbear.id.au> References: <20260702073215.751291-1-david@gibson.dropbear.id.au> <20260702073215.751291-2-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Sat, 04 Jul 2026 17:27:06 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: DueHajX0J6r_JJKiKAm1Oap0g-hCnMMc_8dZrVta734_1783178829 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 6KF7CHWKVLN677J6T7XQ5LVCXOHYZ7T3 X-Message-ID-Hash: 6KF7CHWKVLN677J6T7XQ5LVCXOHYZ7T3 X-MailFrom: sbrivio@redhat.com 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: On Thu, 2 Jul 2026 17:32:14 +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. >=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 ne= w > parsing helpers] > Signed-off-by: David Gibson > --- > fwd_rule.c | 90 +++++++++++++++++++++++++++++++++++++++++++++--------- > passt.1 | 31 +++++++++++++------ > 2 files changed, 98 insertions(+), 23 deletions(-) >=20 > diff --git a/fwd_rule.c b/fwd_rule.c > index ef35e1b4..bed29ed9 100644 > --- a/fwd_rule.c > +++ b/fwd_rule.c > @@ -378,14 +378,17 @@ int fwd_rule_add(struct fwd_table *fwd, const struc= t fwd_rule *new) > * @first:=09First port to forward > * @last:=09Last port to forward > * @exclude:=09Bitmap of ports to exclude (may be NULL) > - * @to:=09=09Port to translate @first to when forwarding > + * @tgt_addr:=09Destination address on the target side > + * @tgt_first:=09Destination port to use for @first on the target side > * @flags:=09Flags for forwarding entries > */ > static void fwd_rule_range_except(struct fwd_table *fwd, bool del, > =09=09=09=09 uint8_t proto, const union inany_addr *addr, > =09=09=09=09 const char *ifname, > =09=09=09=09 uint16_t first, uint16_t last, > -=09=09=09=09 const uint8_t *exclude, uint16_t to, > +=09=09=09=09 const uint8_t *exclude, > +=09=09=09=09 const union inany_addr *tgt_addr, > +=09=09=09=09 uint16_t tgt_first, > =09=09=09=09 uint8_t flags) > { > =09struct fwd_rule rule =3D { > @@ -394,10 +397,31 @@ static void fwd_rule_range_except(struct fwd_table = *fwd, bool del, > =09=09.proto =3D proto, > =09=09.flags =3D flags, > =09}; > +=09unsigned delta =3D tgt_first - first; > =09char rulestr[FWD_RULE_STRLEN]; > -=09unsigned delta =3D to - first; > =09unsigned base, i; > =20 > +=09if (tgt_addr && !inany_is_unspecified(tgt_addr)) { > +=09=09char astr[INANY_ADDRSTRLEN]; > + > +=09=09if (!inany_is_unicast(tgt_addr)) { > +=09=09=09die("Target address %s is not unicast", > +=09=09=09 inany_ntop(tgt_addr, astr, sizeof(astr))); > +=09=09} Nit: an extra newline here would help readability. > +=09=09if (!addr || !!inany_v4(addr) !=3D !!inany_v4(tgt_addr)) { I wonder: if !addr, shouldn't we replace it, for the moment, with inany_any4 or inany_any6 depending on !!inany_v4(tgt_addr)? I see that an empty address already works with -4 or -6, and I understand that, by doing this, we'll have a change in behaviour once forwarding between IP versions is implemented. But it will take a while before we get there, and, meanwhile, I guess almost all users will just want to do stuff like -t 8080:192.0.2.1/80, just to hit: Forwarding between IP versions (* =3D> 192.0.2.1) not implemented which isn't obvious if you aren't familiar with the implementation. I can also picture a constant flow of incoming tickets as a result. The change in behaviour once we implement forwarding between IP versions, by the way, looks a bit like an extension rather than a real change that could reasonably cause trouble to anybody. > +=09=09=09char bstr[INANY_ADDRSTRLEN]; > + > +=09=09=09die( > +"Forwarding between IP versions (%s =3D> %s) not implemented", > +=09=09=09 inany_ntop(addr, bstr, sizeof(bstr)), > +=09=09=09 inany_ntop(tgt_addr, astr, sizeof(astr))); > +=09=09} About both validations: shouldn't they live in fwd_rule_add(), where we already have this kind of stuff? Or at least be duplicated there? Now, I tried dropping those in pesto (#ifndef PESTO ...) to see what happens, and I couldn't really spot anything really bad, not with multicast and not with mixing IPv4 and IPv6: 14 20.647728 fe80::1 =E2=86=92 ::ffff:127.0.0.1 82 TCP 57812 =E2= =86=92 5201 [SYN] Seq=3D0 Win=3D65535 Len=3D0 MSS=3D61440 WS=3D256=20 but still I wonder if we shouldn't make it a bit more robust. > + > +=09=09info("Target address: %s", > +=09=09 inany_ntop(tgt_addr, astr, sizeof(astr))); > +=09=09die("Target address remapping not yet implemented"); > +=09} > + > =09if (!addr) > =09=09rule.flags |=3D FWD_DUAL_STACK_ANY; > =09if (ifname) { > @@ -458,19 +482,31 @@ enum fwd_port_chunk_kind { > * @cursor:=09Parsing point (see parse.c) > * @kindp:=09Updated with kind of chunk we parsed > * @lrange:=09Updated with listening port range (for INCLUDE & EXCLUDE) > + * @taddr:=09Updated with target address (for INCLUDE & ALL) > * @trange:=09Updated with target port range (for INCLUDE) > */ > static bool parse_port_chunk(const char **cursor, > =09=09=09 enum fwd_port_chunk_kind *kindp, > =09=09=09 struct port_range *lrange, > +=09=09=09 union inany_addr *taddr, > =09=09=09 struct port_range *trange) > { > =09struct port_range lr =3D { 0 }, tr =3D { 0 }; > +=09union inany_addr taddr_tmp =3D inany_any6; > =09enum fwd_port_chunk_kind kind; > =09const char *p =3D *cursor; > =20 > =09if (parse_literal(&p, "all")) { > +=09=09const char *tgtspec =3D p; > + > =09=09kind =3D CHUNK_ALL; > +=09=09if (p =3D tgtspec, > +=09=09 parse_literal(&p, ":")=09=09&& > +=09=09 parse_inany(&p, &taddr_tmp)) { > +=09=09=09/* Target address */ > +=09=09} else { > +=09=09=09p =3D tgtspec; > +=09=09} > =09} else if (parse_literal(&p, "auto")) { > =09=09kind =3D CHUNK_AUTO; > =09} else if (parse_literal(&p, "~")) { > @@ -478,12 +514,29 @@ static bool parse_port_chunk(const char **cursor, > =09=09if (!parse_port_range(&p, &lr)) > =09=09=09return false; > =09} else if (parse_port_range(&p, &lr)) { > -=09=09kind =3D CHUNK_INCLUDE; > +=09=09const char *tgtspec =3D p; > =20 > -=09=09if (parse_literal(&p, ":")) { > -=09=09=09if (!parse_port_range(&p, &tr)) > -=09=09=09=09return false; > +=09=09kind =3D CHUNK_INCLUDE; > +=09=09if (p =3D tgtspec, > +=09=09 parse_literal(&p, ":")=09=09&& > +=09=09 parse_inany(&p, &taddr_tmp)=09=09&& > +=09=09 parse_literal(&p, "/")=09=09&& > +=09=09 parse_port_range(&p, &tr)) { > +=09=09=09/* Target address & range */ > +=09=09} else if (p =3D tgtspec, > +=09=09=09 parse_literal(&p, ":")=09&& > +=09=09=09 parse_inany(&p, &taddr_tmp)) { > +=09=09=09/* Target address only */ > +=09=09=09tr =3D lr; > +=09=09} else if (p =3D tgtspec, > +=09=09=09 parse_literal(&p, ":")=09&& > +=09=09=09 parse_port_range(&p, &tr)) { > +=09=09=09/* Target range only */ > +=09=09=09taddr_tmp =3D inany_any6; > =09=09} else { > +=09=09=09p =3D tgtspec; > +=09=09=09/* No target specification */ > +=09=09=09taddr_tmp =3D inany_any6; > =09=09=09tr =3D lr; > =09=09} > =09} else { > @@ -492,6 +545,8 @@ static bool parse_port_chunk(const char **cursor, > =20 > =09*kindp =3D kind; > =09*lrange =3D lr; > +=09if (taddr) > +=09=09*taddr =3D taddr_tmp; > =09if (trange) > =09=09*trange =3D tr; > =09*cursor =3D p; > @@ -551,6 +606,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fw= d, bool del, uint8_t proto, > =09=09=09=09 const char *spec) > { > =09uint8_t exclude[PORT_BITMAP_SIZE] =3D { 0 }; > +=09union inany_addr all_taddr =3D inany_any6; > =09enum fwd_port_chunk_kind kind; > =09struct port_range lrange; > =09bool exclude_only =3D true; > @@ -561,7 +617,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fw= d, bool del, uint8_t proto, > =09/* Consider excluded ranges and "auto" in the first pass */ > =09p =3D spec; > =09do { > -=09=09if (!parse_port_chunk(&p, &kind, &lrange, NULL)) > +=09=09if (!parse_port_chunk(&p, &kind, &lrange, NULL, NULL)) > =09=09=09goto bad; > =20 > =09=09switch (kind) { > @@ -586,14 +642,19 @@ static void fwd_rule_parse_ports(struct fwd_table *= fwd, bool del, uint8_t proto, > =09p =3D spec; > =09do { > =09=09struct port_range trange; > +=09=09union inany_addr taddr; > =20 > -=09=09if (!parse_port_chunk(&p, &kind, &lrange, &trange)) > +=09=09if (!parse_port_chunk(&p, &kind, &lrange, &taddr, &trange)) > =09=09=09goto bad; > =20 > =09=09switch (kind) { > -=09=09case CHUNK_AUTO:=09/* already handled */ > -=09=09case CHUNK_EXCLUDE:=09/* already handled */ > -=09=09case CHUNK_ALL:=09=09/* handled later */ > +=09=09case CHUNK_AUTO: > +=09=09case CHUNK_EXCLUDE: > +=09=09=09continue; /* already handled */ > + > +=09=09case CHUNK_ALL: > +=09=09=09/* Save the address to use later */ > +=09=09=09all_taddr =3D taddr; > =09=09=09continue; > =20 > =09=09case CHUNK_INCLUDE: > @@ -604,7 +665,8 @@ static void fwd_rule_parse_ports(struct fwd_table *fw= d, bool del, uint8_t proto, > =20 > =09=09=09fwd_rule_range_except(fwd, del, proto, addr, ifname, > =09=09=09=09=09 lrange.first, lrange.last, > -=09=09=09=09=09 exclude, trange.first, flags); > +=09=09=09=09=09 exclude, &taddr, trange.first, > +=09=09=09=09=09 flags); > =09=09=09break; > =09=09default: > =09=09=09goto bad; > @@ -620,7 +682,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fw= d, bool del, uint8_t proto, > =20 > =09=09fwd_rule_range_except(fwd, del, proto, addr, ifname, > =09=09=09=09 1, NUM_PORTS - 1, exclude, > -=09=09=09=09 1, flags | FWD_WEAK); > +=09=09=09=09 &all_taddr, 1, flags | FWD_WEAK); > =09} > =09return; > bad: > diff --git a/passt.1 b/passt.1 > index c3722ef9..9ece0e0c 100644 > --- a/passt.1 > +++ b/passt.1 > @@ -449,12 +449,15 @@ interface name (since Linux 5.7) can be specified. > =20 > \fIports\fR is a comma-separated list of entries which may be any of: > .TP > -\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]= ] > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR[\fItoaddr\fR\fB/\fR]\fItofirst\fR= [\fB-\fR\fItolast\fR]] > +.TP > +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItoaddr\fR] > Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR > -(inclusive) to ports between \fItofirst\fR and \fItolast\fR. If > -\fItofirst\fR and \fItolast\fR are omitted, assume the same as > -\fIfirst\fR and \fIlast\fR. If \fIlast\fR is omitted, assume the same > -as \fIfirst\fR. > +(inclusive) to ports between \fItofirst\fR and \fItolast\fR to address > +\fItoaddr\fR. If \fItoaddr\fR is omitted, automatically determine the > +guest or namespace address. If \fItofirst\fR and \fItolast\fR are > +omitted, assume the same as \fIfirst\fR and \fIlast\fR. If \fIlast\fR > +is omitted, assume the same as \fIfirst\fR. > =20 > .TP > \fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR] > @@ -462,11 +465,13 @@ Exclude range. Don't forward port numbers between = \fIfirst\fR and > \fIlast\fR. This takes precedences over include ranges. > =20 > .TP > -.BR all > +.BR all\fR[\fB:\fItoaddr\fR] > Forward all unbound, non-ephemeral ports, not covered by exclude > -ranges above, as permitted by current capabilities. For low (< 1024) > -ports, see \fBNOTES\fR. No failures are reported for unavailable > -ports, unless no ports could be forwarded at all. > +ranges above, as permitted by current capabilities, to the > +corresponding ports on address \fItoaddr\fR. If \fItoaddr\fR is > +omitted, automatically determine the guest or namespace address. For > +low (< 1024) ports, see \fBNOTES\fR. No failures are reported for > +unavailable ports, unless no ports could be forwarded at all. > =20 > .TP > .BR auto > @@ -516,6 +521,14 @@ Forward local port 22, bound to 192.0.2.1 and interf= ace eth0, to port 22 > -t %eth0/22 > Forward local port 22, bound to any address on interface eth0, to port 2= 2 > .TP > +-t 0.0.0.0/5000:192.0.2.5/6000 > +Forward local port 5000, bound to any IPv4 address, to port 6000 on addr= ess 192.0.2.5. > +.TP > +-t 127.0.0.6/all:192.0.2.6 > +For the local address 127.0.0.6 forward all unbound, non-ephemeral I think this is a bit difficult to follow without a comma, it should be: For the local address 127.0.0.6, forward all unbound, [...] also for consistency with the existing: For the local address 192.0.2.1, forward ports between 20 and 24 [...] > +ports as permitted by current capabilities to the corresponding port > +on 192.0.2.6. > +.TP > -t 2000-5000,~3000-3010 > Forward local ports between 2000 and 5000, except for those between 3000= and > 3010 the rest and 2/2 look good to me, no further comments. --=20 Stefano