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=ZF02oKhT; 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 7C6DB5A0271 for ; Tue, 13 Jan 2026 00:26:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1768260376; 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=o8a2jdtTw5Zm+pkL8eZuh/4uATaJ2eNn3RgoJ49p8oM=; b=ZF02oKhTLUhkoPRDVpdGMKBl0CCw3AY34Oldd9xkiOpz0aqxiLZRf/1f5EyFp4++7YhLbT b9+ugbX/NkikKKMVbPmw1utjAm5AzhkTAV/MtxdEoXfK54KkhoSV2geyYZmslcVMgRpUsh ErrtWU95Le+SYZ4aAzWrfOJTfAMo1js= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-269-TJsfljHTMreLhtqZcziDjw-1; Mon, 12 Jan 2026 18:26:14 -0500 X-MC-Unique: TJsfljHTMreLhtqZcziDjw-1 X-Mimecast-MFC-AGG-ID: TJsfljHTMreLhtqZcziDjw_1768260373 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-430f8866932so5883768f8f.1 for ; Mon, 12 Jan 2026 15:26:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768260373; x=1768865173; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=o8a2jdtTw5Zm+pkL8eZuh/4uATaJ2eNn3RgoJ49p8oM=; b=LPP4lUHJLfyiNWXS8+LzhPQSiTTXvrqmflHFAb+uqJIKknij50hKsfGItbCQOL7/73 lByJ6FevPU5iOYDjfwGnvnkt3vetVJG0F+hulTXGMH6s1jDwsGTVYcdBJHK4udUf2AlU 2ZYMN/dPi8gp4aX9kQzjCiO5IRkcabiUuaXuzHR6kxTwyz3RNd0IwMaifhTpkIIBWqaj 20rDMVbznrkQJr8vyS5FlDFbGq0kRUx5qCgVA2lzRILyrlXjhZH9A0V0rht1IwaPwtcM VeZobv2IJ/jrGrzyAnjqVBHIL+McHqBZkLCNsP2CiSzKjBmIciGo4jAxxe1htGqD3eWV SR0Q== X-Gm-Message-State: AOJu0Yw5UhFiocXqng3L2meLZmoFYJ1MjpGLAAgp7J3TqAImB6DILdVs qi7YMqxWNqBNBQOzbRRpEWVyIXb/wB2vH5kEkJrOLjYDEar97tJp8HXKAY5S3xTStpkUxW2tD8z Hv4h4Pxy4p3UAJF4BLBM6HDvyebyaY6RBYpVFxCdZQFWsi6Q68DMiyqHdA03SPg== X-Gm-Gg: AY/fxX6jYiIPfKRXlJwOY5rF3HlDpOwP3MOO0I3wvp/nDjgkcGulp4GzYuFlxW33KOq cbQTvMDbKxg3+1JH91TcGX1rsfRDpJxtDOTw5Dmuf1CfJxru5+PfzbHrLHEabs8FE8BXO6vvesl 4+m+cFYt0xdoNSnnY+BlUygnHpUUbofSRdUkYqhYuDuMozOMiTyOpchUNvJYBr1NdWaUSpIOVos 1RObax1dr0toWSUMcE4dZ4EULK9AzSNc5UKMt0H1bmmTQDRlun3palUhoj5pCDzYFeZHEakKLfu GbZc0d4ms11kN/rpiEJghBX9J8LMXkfmUDSThc0XLS5XmslB80nmWajCiCFaSsY0bdSVfsPrPDQ YEYa/pBI8AMURDbY2jYoXJocopgNbhcYehZD24Q== X-Received: by 2002:a5d:5885:0:b0:431:864:d492 with SMTP id ffacd0b85a97d-432c3798314mr22009288f8f.36.1768260372921; Mon, 12 Jan 2026 15:26:12 -0800 (PST) X-Google-Smtp-Source: AGHT+IHvGcz/Q+sSJHvNThBBrbnWRr0p/mCAlCYPEhlXyY/T2mc6U70b0RxV/4R6bbQWJBLwHz8ExA== X-Received: by 2002:a5d:5885:0:b0:431:864:d492 with SMTP id ffacd0b85a97d-432c3798314mr22009264f8f.36.1768260372292; Mon, 12 Jan 2026 15:26:12 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-432bd0e180csm39938503f8f.10.2026.01.12.15.26.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Jan 2026 15:26:11 -0800 (PST) Date: Tue, 13 Jan 2026 00:26:10 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 02/14] conf, fwd: Keep a table of our port forwarding configuration Message-ID: <20260113002610.5168fd6a@elisabeth> In-Reply-To: <20260108022948.2657573-3-david@gibson.dropbear.id.au> References: <20260108022948.2657573-1-david@gibson.dropbear.id.au> <20260108022948.2657573-3-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 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: wx4th2NqgZud4RNtANvGzhtNDN2G5oaqVyEhX8uET2s_1768260373 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: UX5T6UELRPAPLC6X4CW24CMG7MCH7MPX X-Message-ID-Hash: UX5T6UELRPAPLC6X4CW24CMG7MCH7MPX 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, 8 Jan 2026 13:29:36 +1100 David Gibson wrote: > At present, we set up forwarding as we parse the -t, and -u options, not > keeping a persistent data structure with all the details. We do have > some information in struct fwd_ports, but it doesn't capture all the nuance > that the options do. > > As a first step to generalising our forwarding model, add a table of all > the forwarding rules to struct fwd_ports. For now it covers only explicit > forwards, not automatic, and we don't do anything with it other than print > some additional debug information. We'll do more with it in future > patches. > > Signed-off-by: David Gibson > --- > conf.c | 80 ++++++++++++++++++++++++++++++------------------- > fwd.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > fwd.h | 35 +++++++++++++++++++++- > 3 files changed, 179 insertions(+), 31 deletions(-) > > diff --git a/conf.c b/conf.c > index c936664b..127e69f5 100644 > --- a/conf.c > +++ b/conf.c > @@ -137,7 +137,7 @@ static int parse_port_range(const char *s, char **endptr, > * @last: Last port to forward > * @exclude: Bitmap of ports to exclude > * @to: Port to translate @first to when forwarding > - * @weak: Ignore errors, as long as at least one port is mapped > + * @flags: Flags for forwarding entries > */ > static void conf_ports_range_except(const struct ctx *c, char optname, > const char *optarg, struct fwd_ports *fwd, > @@ -145,10 +145,11 @@ static void conf_ports_range_except(const struct ctx *c, char optname, > const char *ifname, > uint16_t first, uint16_t last, > const uint8_t *exclude, uint16_t to, > - bool weak) > + uint8_t flags) > { > + unsigned delta = to - first; > bool bound_one = false; > - unsigned i; > + unsigned base, i; > int fd; > > if (first == 0) { > @@ -172,37 +173,45 @@ static void conf_ports_range_except(const struct ctx *c, char optname, > } > } > > - for (i = first; i <= last; i++) { > - if (bitmap_isset(exclude, i)) > + for (base = first; base <= last; base++) { > + if (bitmap_isset(exclude, base)) > continue; > > - if (bitmap_isset(fwd->map, i)) { > - warn( > -"Altering mapping of already mapped port number: %s", optarg); > - } > + for (i = base; i <= last; i++) { This nested loop is a bit convoluted, but I don't have better ideas and I suppose it goes away later (I just reached 10/14 so far). > + if (bitmap_isset(exclude, i)) > + break; > > - bitmap_set(fwd->map, i); > - fwd->delta[i] = to - first; > + if (bitmap_isset(fwd->map, i)) { > + warn( > +"Altering mapping of already mapped port number: %s", optarg); > + } > > - if (optname == 't') > - fd = tcp_listen(c, PIF_HOST, addr, ifname, i); > - else if (optname == 'u') > - fd = udp_listen(c, PIF_HOST, addr, ifname, i); > - else > - /* No way to check in advance for -T and -U */ > - fd = 0; > + if (optname == 't') > + fd = tcp_listen(c, PIF_HOST, addr, ifname, i); > + else if (optname == 'u') > + fd = udp_listen(c, PIF_HOST, addr, ifname, i); > + else > + /* No way to check in advance for -T and -U */ > + fd = 0; > + > + if (fd == -ENFILE || fd == -EMFILE) { > + die( > +"Can't open enough sockets for port specifier: %s", > + optarg); > + } > > - if (fd == -ENFILE || fd == -EMFILE) { > - die("Can't open enough sockets for port specifier: %s", > - optarg); > + if (fd >= 0) { > + bound_one = true; > + } else if (!(flags & FWD_WEAK)) { > + die( > +"Failed to bind port %u (%s) for option '-%c %s'", > + i, strerror_(-fd), optname, optarg); > + } > } > > - if (fd >= 0) { > - bound_one = true; > - } else if (!weak) { > - die("Failed to bind port %u (%s) for option '-%c %s'", > - i, strerror_(-fd), optname, optarg); > - } > + fwd_rule_add(fwd, flags, addr, ifname, base, i - 1, > + base + delta); > + base = i - 1; > } > > if (!bound_one) > @@ -272,7 +281,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > conf_ports_range_except(c, optname, optarg, fwd, > NULL, NULL, > 1, NUM_PORTS - 1, exclude, > - 1, true); > + 1, FWD_WEAK); > return; > } > > @@ -357,7 +366,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > conf_ports_range_except(c, optname, optarg, fwd, > addr, ifname, > 1, NUM_PORTS - 1, exclude, > - 1, true); > + 1, FWD_WEAK); > return; > } > > @@ -390,7 +399,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, > addr, ifname, > orig_range.first, orig_range.last, > exclude, > - mapped_range.first, false); > + mapped_range.first, 0); > } while ((p = next_chunk(p, ','))); > > return; > @@ -1225,6 +1234,17 @@ dns6: > info(" %s", c->dns_search[i].n); > } > } > + > + info("Inbound TCP forwarding:"); > + fwd_rules_print(&c->tcp.fwd_in); > + info("Inbound UDP forwarding:"); > + fwd_rules_print(&c->udp.fwd_in); > + if (c->mode == MODE_PASTA) { > + info("Outbound TCP forwarding:"); > + fwd_rules_print(&c->tcp.fwd_out); > + info("Outbound UDP forwarding:"); > + fwd_rules_print(&c->udp.fwd_out); > + } > } > > /** > diff --git a/fwd.c b/fwd.c > index 961c533f..ad398e54 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -13,6 +13,7 @@ > * Author: David Gibson > */ > > +#include > #include > #include > #include > @@ -22,6 +23,8 @@ > > #include "util.h" > #include "ip.h" > +#include "siphash.h" > +#include "inany.h" > #include "fwd.h" > #include "passt.h" > #include "lineread.h" > @@ -300,6 +303,19 @@ parse_err: > warn("Unable to parse %s", PORT_RANGE_SYSCTL); > } > > +/** > + * fwd_rule_addr() - Return match address for a rule > + * @rule: Forwarding rule > + * > + * Return: Rule's match address, or NULL, if it matches any address What about: matching address for rule, NULL if it matches all addresses > + */ > +static const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) > +{ > + if (rule->flags & FWD_DUAL_STACK_ANY) > + return NULL; Nit: usual newline here. > + return &rule->addr; > +} > + > /** > * fwd_port_is_ephemeral() - Is port number ephemeral? > * @port: Port number > @@ -313,6 +329,85 @@ bool fwd_port_is_ephemeral(in_port_t port) > return (port >= fwd_ephemeral_min) && (port <= fwd_ephemeral_max); > } > > +/** > + * fwd_rule_add() - Add a rule to a forwarding table > + * @fwd: Table to add to > + * @flags: Flags for this entry > + * @addr: Our address to forward (NULL for both 0.0.0.0 and ::) > + * @ifname: Only forward from this interface name, if non-empty > + * @first: First port number to forward > + * @last: Last port number to forward > + * @to: First port of target port range to map to > + */ > +void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags, > + const union inany_addr *addr, const char *ifname, > + in_port_t first, in_port_t last, in_port_t to) > +{ > + /* Flags which can be set from the caller */ > + const uint8_t allowed_flags = FWD_WEAK; > + struct fwd_rule *new; > + unsigned port; > + > + ASSERT(!(flags & ~allowed_flags)); > + > + if (fwd->count >= ARRAY_SIZE(fwd->rules)) > + die("Too many port forwarding ranges"); > + > + new = &fwd->rules[fwd->count++]; > + new->flags = flags; > + > + if (addr) { > + new->addr = *addr; > + } else { > + new->addr = inany_any6; > + new->flags |= FWD_DUAL_STACK_ANY; > + } > + > + memset(new->ifname, 0, sizeof(new->ifname)); > + if (ifname) > + strncpy(new->ifname, ifname, sizeof(new->ifname)); Coverity Scan complains because we copy exactly up to sizeof(new->ifname) bytes, and the terminator might be missing: /home/sbrivio/passt/fwd.c:368:3: Type: Buffer not null terminated (BUFFER_SIZE) /home/sbrivio/passt/fwd.c:351:2: 1. path: Condition "!(flags & -3 /* ~allowed_flags */)", taking true branch. /home/sbrivio/passt/fwd.c:353:2: 2. path: Condition "fwd->count >= 128U /* (int)(sizeof (fwd->rules) / sizeof (fwd->rules[0])) */", taking false branch. /home/sbrivio/passt/fwd.c:359:2: 3. path: Condition "addr", taking true branch. /home/sbrivio/passt/fwd.c:361:2: 4. path: Falling through to end of if statement. /home/sbrivio/passt/fwd.c:367:2: 5. path: Condition "ifname", taking true branch. /home/sbrivio/passt/fwd.c:368:3: 6. buffer_size_warning: Calling "strncpy" with a maximum size argument of 16 bytes on destination array "new->ifname" of size 16 bytes might leave the destination string unterminated. /home/sbrivio/passt/fwd.c:370:2: > + > + ASSERT(first <= last); > + new->first = first; > + new->last = last; > + > + new->to = to; > + > + for (port = new->first; port <= new->last; port++) { > + /* Fill in the legacy data structures to match the table */ > + bitmap_set(fwd->map, port); > + fwd->delta[port] = new->to - new->first; > + } > +} > + > +/** > + * fwd_rules_print() - Print forwarding rules for debugging > + * @fwd: Table to print > + */ > +void fwd_rules_print(const struct fwd_ports *fwd) > +{ > + unsigned i; > + > + for (i = 0; i < fwd->count; i++) { > + const struct fwd_rule *rule = &fwd->rules[i]; > + const char *weak = rule->flags & FWD_WEAK ? " WEAK" : ""; Should we print " might fail" or " can fail" instead of " WEAK"? This is for users. > + const char *percent = *rule->ifname ? "%" : ""; > + char addr[INANY_ADDRSTRLEN]; > + > + inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); > + > + if (rule->first == rule->last) { > + info(" [%s]%s%s:%hu => %hu %s", > + addr, percent, rule->ifname, > + rule->first, rule->to, weak); > + } else { > + info(" [%s]%s%s:%hu-%hu => %hu-%hu %s", > + addr, percent, rule->ifname, rule->first, rule->last, > + rule->to, rule->last - rule->first + rule->to, weak); > + } > + } > +} > + > /* See enum in kernel's include/net/tcp_states.h */ > #define UDP_LISTEN 0x07 > #define TCP_LISTEN 0x0a > diff --git a/fwd.h b/fwd.h > index 934bab33..3dfc7959 100644 > --- a/fwd.h > +++ b/fwd.h > @@ -16,6 +16,30 @@ struct flowside; > void fwd_probe_ephemeral(void); > bool fwd_port_is_ephemeral(in_port_t port); > > +/** > + * struct fwd_rule - Forwarding rule governing a range of ports > + * @addr: Address to forward from > + * @ifname: Interface to forward from > + * @first: First port number to forward > + * @last: Last port number to forward > + * @to: Port number to forward port @first to. > + * @flags: Flag mask > + * FWD_DUAL_STACK_ANY - match any IPv4 or IPv6 address (@addr should be ::) > + * FWD_WEAK - Don't give an error if binds fail for some forwards Usually we document bit flags inline as we define them (see struct tcp_tap_conn), I guess it's a bit easier to follow, but this has the advantage of looking less cramped (unless we add comment lines before the #defines, which would also be reasonable I think). Not a strong preference either way. > + * > + * FIXME: @addr and @ifname currently ignored for outbound tables > + */ > +struct fwd_rule { > + union inany_addr addr; > + char ifname[IFNAMSIZ]; > + in_port_t first, last, to; Having one line for each helps (me at least) visualising the size of the struct. Not a strong preference. > +#define FWD_DUAL_STACK_ANY BIT(0) > +#define FWD_WEAK BIT(1) > + uint8_t flags; > +}; > + > +#define MAX_FWD_RULES 128 > + > /** > * union fwd_listen_ref - information about a single listening socket > * @port: Bound port number of the socket > @@ -44,6 +68,8 @@ enum fwd_ports_mode { > * @mode: Overall forwarding mode (all, none, auto, specific ports) > * @scan4: /proc/net fd to scan for IPv4 ports when in AUTO mode > * @scan6: /proc/net fd to scan for IPv6 ports when in AUTO mode > + * @count: Number of forwarding rules > + * @rules: Array of forwarding rules > * @map: Bitmap describing which ports are forwarded > * @delta: Offset between the original destination and mapped port number > */ > @@ -51,14 +77,21 @@ struct fwd_ports { > enum fwd_ports_mode mode; > int scan4; > int scan6; > + unsigned count; > + struct fwd_rule rules[MAX_FWD_RULES]; > uint8_t map[PORT_BITMAP_SIZE]; > in_port_t delta[NUM_PORTS]; > }; > > #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ > > +void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags, > + const union inany_addr *addr, const char *ifname, > + in_port_t first, in_port_t last, in_port_t to); > +void fwd_rules_print(const struct fwd_ports *fwd); > + > void fwd_scan_ports_init(struct ctx *c); > -void fwd_scan_ports_timer(struct ctx *c, const struct timespec *now); > +void fwd_scan_ports_timer(struct ctx * c, const struct timespec *now); > > bool nat_inbound(const struct ctx *c, const union inany_addr *addr, > union inany_addr *translated); -- Stefano