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=gz0qLF0X; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 61D895A026D for ; Fri, 15 May 2026 01:28:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1778801324; 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=TAt2BWJp54X6P7Ovk5/d7EGG5zBh6OaajUVP4kORUVk=; b=gz0qLF0XuD5PaLrNk74jDI58DV+dEmEetSJE7VwAx4yCfNjIvxNJvMWfrfvv7VuBhQOCY+ XCeM5QGEgZAhtVdZ7B0id351m+e1H5DTDXPNH+UiCM9NvTNVDP8e5pf5hMaHPXOuFvFjmS O11peEjD7sULxf+/Pcqdk/sMmlpoI+4= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-349-31cYQsqFP_Wlu_dkBnEYqg-1; Thu, 14 May 2026 19:28:42 -0400 X-MC-Unique: 31cYQsqFP_Wlu_dkBnEYqg-1 X-Mimecast-MFC-AGG-ID: 31cYQsqFP_Wlu_dkBnEYqg_1778801321 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-44dad1b938fso5079235f8f.1 for ; Thu, 14 May 2026 16:28:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1778801320; x=1779406120; h=date:content-transfer-encoding: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; bh=TAt2BWJp54X6P7Ovk5/d7EGG5zBh6OaajUVP4kORUVk=; b=TOBo9WGqfhWGrw2bUcDOCmwHaDG+F7pJHg/pSjayksqphE7PNa0dK6o6kAdCnKYEbE V4q5Uu5hPWOthSNOATMVsKlOJsIjaFj5p+lu+iOU14N5dYNs76nBR4jR0+rVu3fdjHbh A/47KgAFHYVCa8TYHoDKScYQzT0UafnViGZNaHN1Jfg3sNvJcTZOtkscyiixhNrydTBH o1eKXcCmy5lDyQbj35WN42+Tq53bbGlIIBg4AB/T6CXnbv/W1BF+M4+UkIzMS6KQ8esP 2pa2O80jedJvCFr7mf6n/gbS5YFsCn+J6U9rMwwpCBPWSBGyi8H/WQzxHSksm31gR4Gz eB2Q== X-Forwarded-Encrypted: i=1; AFNElJ+fvyes+/SxLb78MkjX41dhWf8iqS8OXI+jJll6MsL96PW5JnjLx1EhnF/+PxQh0j67a7S1PhLcl7w=@passt.top X-Gm-Message-State: AOJu0YxEZiqrJcQ3kXPzZUGI6P5NSKKJ1xcvMXGQE3Aw90z4MqnfyfBr ElDUKb9qLcu//fWSSwhMMPwfs4Y/v8V2B4X1pygmToXHdsHjqQM15ch+w6Bgxvnn4C0hXz4Uf06 o7rHVR3DdX7ol2w8u5cEaSe3MhIzcXcs8AAsVuojwhCh/HYF7U1H1TQ== X-Gm-Gg: Acq92OGYjqNdKg1b6+83DP+o2NB5SGniIW7fY3xwZX/TkrCHSHZo1qMmXHmCr/xNoU8 i6yXZrUKAhKOiZrhuiTwGtduJYZSKWD/zISVZGD6NX9345gpIXDM804IgcsWxcmbqQBWTGv0yy9 YgXB3ZZcrrBXqZAUzG/RhsfFhflgeFs9CvcGRU2FEsT0ifdlQNLlPOMuCc/HK76RSl82gp9IU/C uCz2iwKa417+qsvPnyLZNaMuUQZSf+f3O2APaMu5OfDsaGuIFYMWW1WNDZ/RiiukG9TROnr+DVH dAJSVCtVZVHYuSQCroUt5NKbaDGGSsa0fm7q8OvyymNaB4jF69k6VFw7XDaxItRqFpggKK8axxk wzriax/cvg8S9aGUUlxfZh/Q9hVQ3Jsld X-Received: by 2002:a05:6000:4287:b0:43e:b0b0:629a with SMTP id ffacd0b85a97d-45e5c5ccc4bmr1506261f8f.34.1778801319784; Thu, 14 May 2026 16:28:39 -0700 (PDT) X-Received: by 2002:a05:6000:4287:b0:43e:b0b0:629a with SMTP id ffacd0b85a97d-45e5c5ccc4bmr1506220f8f.34.1778801319239; Thu, 14 May 2026 16:28:39 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45d9e767d16sm10737846f8f.6.2026.05.14.16.28.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 May 2026 16:28:38 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH HACK] fwd, fwd_rule: Implement configurable destination address mapping Message-ID: <20260515012835.087c0ae2@elisabeth> In-Reply-To: References: <20260507055036.2110260-1-sbrivio@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Fri, 15 May 2026 01:28:36 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: X8HhyeQfHFn_l6hIpqOaYPbiHCKfvKP3JI7Z_bMyA_I_1778801321 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 4CQYIX6WS2SMXWM6F7NZIDTACPJU4G6F X-Message-ID-Hash: 4CQYIX6WS2SMXWM6F7NZIDTACPJU4G6F 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: Paul Holzinger , passt-dev@passt.top, Jon Maloy 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, 14 May 2026 14:54:33 +1000 David Gibson wrote: > On Thu, May 07, 2026 at 07:50:36AM +0200, Stefano Brivio wrote: > > This isn't complete, it's rather a quick hack to enable early > > integration testing. > > > > Add a 'daddr' field to forwarding rules, and some rudimentary parsing. > > > > 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 > > > > Signed-off-by: Stefano Brivio > > Very nice for a quick hack, and it gets surprisingly far with not much > code at all. > > > > --- > > fwd.c | 4 +++- > > fwd_rule.c | 46 +++++++++++++++++++++++++++++++++++----------- > > fwd_rule.h | 2 ++ > > 3 files changed, 40 insertions(+), 12 deletions(-) > > > > diff --git a/fwd.c b/fwd.c > > index d224c0a..75db350 100644 > > --- a/fwd.c > > +++ b/fwd.c > > @@ -1095,7 +1095,9 @@ uint8_t fwd_nat_from_host(const struct ctx *c, > > } > > tgt->oport = ini->eport; > > > > - if (inany_v4(&tgt->oaddr)) { > > + if (!inany_is_unspecified(&rule->daddr)) { > > + tgt->eaddr = rule->daddr; > > Longer term, there are at least two options for what we want to do > if the rule doesn't specify a specific destination address: > * Use the observed guest address (what we do now) I would keep that as default for now. > * Use the host side destination address (potentially useful if we > have multiple containers each assigned different host addresses) This could be implemented on top of Jon's "multiple addresses" series I think, even in terms of using multiple observed guest addresses, if useful. > So we'll probably want to allow for some new rule flags to cover > options like this. Rather than flags, I've been suggesting address pointers / references. I'm not sure if flags are generic enough. At some point, we might want a syntax to refer to "the observed address of container B". Even if we have just container A at the moment, maybe we could start implementing something going in that direction. > > + } else if (inany_v4(&tgt->oaddr)) { > > tgt->eaddr = inany_from_v4(c->ip4.addr_seen); > > } else { > > if (inany_is_linklocal6(&tgt->oaddr)) > > diff --git a/fwd_rule.c b/fwd_rule.c > > index 5fc04d7..5bce2fb 100644 > > --- a/fwd_rule.c > > +++ b/fwd_rule.c > > @@ -465,6 +465,7 @@ static int parse_keyword(const char *s, const char **endptr, const char *kw) > > */ > > static void fwd_rule_range_except(struct fwd_table *fwd, bool del, > > uint8_t proto, const union inany_addr *addr, > > + const union inany_addr *daddr, > > const char *ifname, > > uint16_t first, uint16_t last, > > const uint8_t *exclude, uint16_t to, > > @@ -472,6 +473,7 @@ static void fwd_rule_range_except(struct fwd_table *fwd, bool del, > > { > > struct fwd_rule rule = { > > .addr = addr ? *addr : inany_any6, > > + .daddr = daddr ? *daddr : inany_any6, > > .ifname = { 0 }, > > .proto = proto, > > .flags = flags, > > @@ -544,13 +546,13 @@ fail: > > */ > > static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > const union inany_addr *addr, > > - const char *ifname, > > - const char *spec) > > + const char *ifname, char *spec) > > { > > + union inany_addr daddr_buf = inany_any6, *daddr = &daddr_buf; > > uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; > > bool exclude_only = true; > > - const char *p, *ep; > > uint8_t flags = 0; > > + char *p, *ep; > > unsigned i; > > > > if (!strcmp(spec, "all")) { > > @@ -568,7 +570,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > continue; > > } > > > > - if (parse_keyword(p, &p, "auto") == 0) { > > + if (parse_keyword(p, (const char **)&p, "auto") == 0) { > > if (p != ep) /* Garbage after the keyword */ > > goto bad; > > > > @@ -586,7 +588,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > goto bad; > > p++; > > > > - if (parse_port_range(p, &p, &xrange)) > > + if (parse_port_range(p, (const char **)&p, &xrange)) > > goto bad; > > if (p != ep) /* Garbage after the range */ > > goto bad; > > @@ -599,7 +601,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > /* Exclude ephemeral ports */ > > fwd_port_map_ephemeral(exclude); > > > > - fwd_rule_range_except(fwd, del, proto, addr, ifname, > > + fwd_rule_range_except(fwd, del, proto, addr, NULL, ifname, > > 1, NUM_PORTS - 1, exclude, > > 1, flags | FWD_WEAK); > > return; > > @@ -613,11 +615,32 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > /* Already parsed */ > > continue; > > > > - if (parse_port_range(p, &p, &orig_range)) > > + if (parse_port_range(p, (const char **)&p, &orig_range)) > > goto bad; > > > > - if (*p == ':') { /* There's a range to map to as well */ > > - if (parse_port_range(p + 1, &p, &mapped_range)) > > + if (*p == ':') { > > + /* There's a range or address to map to as well */ > > + char *addr_end = strchr(p, '/'); > > + > > + if (addr_end) { > > + *addr_end = '\0'; > > + > > + if (*p == '[' && p[strlen(p) - 1] == ']') { > > + p[strlen(p) - 1] = '\0'; > > + p++; > > + } > > + > > + if (!inany_pton(p + 1, daddr)) > > + die("Bad forwarding address '%s'", p); > > + > > + p = addr_end; > > + } else { > > + daddr = NULL; > > + } > > We probably want to factor some of this address parsing out into a > helper, since we're now doing it twice. Yes, definitely, and it should have its own small buffer. The in-place parsing we have is a remnant of my original implementation (originally intended for much simpler cases) but that's what forced me to essentially copy and paste this if I wanted to implement this in a few minutes. > > + > > + > > + if (parse_port_range(p + 1, (const char **)&p, > > + &mapped_range)) > > goto bad; > > if ((mapped_range.last - mapped_range.first) != > > (orig_range.last - orig_range.first)) > > @@ -629,7 +652,7 @@ static void fwd_rule_parse_ports(struct fwd_table *fwd, bool del, uint8_t proto, > > if (p != ep) /* Garbage after the ranges */ > > goto bad; > > > > - fwd_rule_range_except(fwd, del, proto, addr, ifname, > > + fwd_rule_range_except(fwd, del, proto, addr, daddr, ifname, > > orig_range.first, orig_range.last, > > exclude, > > mapped_range.first, flags); > > @@ -675,7 +698,8 @@ void fwd_rule_parse(char optname, bool del, const char *optarg, > > > > strncpy(buf, optarg, sizeof(buf) - 1); > > > > - if ((spec = strchr(buf, '/'))) { > > + if ((spec = strchr(buf, '/')) && > > + strchr(spec, ':') == strchr(buf, ':')) { > > strchr() for ':' needs a lot of caution, since it can appear within > IPv6 addresses. Absolutely. This patch actually works only for IPv4. See also: https://github.com/containers/container-libs/pull/755#issuecomment-4408900718 We probably need separate helpers extracting different parts to keep this sane (the LLM-sourced workaround shown there is probably correct but pretty hard to audit and hard to reason about). > > *spec = 0; > > spec++; > > > > diff --git a/fwd_rule.h b/fwd_rule.h > > index ae9a3cb..3a2a809 100644 > > --- a/fwd_rule.h > > +++ b/fwd_rule.h > > @@ -35,6 +35,7 @@ > > /** > > * struct fwd_rule - Forwarding rule governing a range of ports > > * @addr: Address to forward from > > + * @daddr: Optional address to set as destination when forwarding > > * @ifname: Interface to forward from > > * @first: First port number to forward > > * @last: Last port number to forward > > @@ -47,6 +48,7 @@ > > */ > > struct fwd_rule { > > union inany_addr addr; > > + union inany_addr daddr; > > We probably want to rethink the names here. Both of these are > destination addresses, just one is host side the other is guest side > (or, in general, they're destination addresses for two different > pifs). "initiating" vs. "target" are probably the terms to use, since > we already use that in the flow table. I guess we'll want four addresses anyway, eventually, but that probably needs the implementation of a separate address list with pointers that I've been suggesting in the past to keep memory usage reasonable. For the moment, there are two ways to distinguish these two destination addresses, I think: 1. outside / inside 2. initiating / target I was thinking that 1. is simpler to grasp as it refers to stable concepts (host, container / guest) rather than how a connection came to be (and client / server are much less universal than that). Going with that convention means that things will be swapped in terms of 2., though, for outbound forwarding. I still think it's preferable. > At some point I'm pretty sure we'll also want to put a target pif in > the table. Not sure if we want to introduce that initially, or > whether just an address makes sense for the first cut. I don't think it's really important for the table itself (I would add it if it doesn't add cachelines, and otherwise skip it or add it commented out), but we should make sure the command line extensions we're introducing will allow for that, or backwards compatibility will be complicated to achieve later. > > char ifname[IFNAMSIZ]; > > in_port_t first; > > in_port_t last; > > -- > > 2.43.0 Note that I'm not working on this right now as it's not immediately helpful: https://github.com/containers/container-libs/pull/755#issuecomment-4421500913 so I don't plan to post further versions of this, at least not any time soon. Feel free to pick it up from here, if it makes sense. -- Stefano