From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top
Subject: Re: [PATCH v3 07/25] fwd: Store forwarding tables indexed by (origin) pif
Date: Wed, 25 Mar 2026 01:54:24 +0100 (CET) [thread overview]
Message-ID: <20260325015423.4c217023@elisabeth> (raw)
In-Reply-To: <20260323073732.3158468-8-david@gibson.dropbear.id.au>
On Mon, 23 Mar 2026 18:37:14 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> Currently we store the inbound (PIF_HOST) and outbound (PIF_SPLICE)
> forwarding tables in separate fields of struct ctx. In a number of places
> this requires somewhat awkward if or switch constructs to select the
> right table for updates. Conceptually simplify that by using an index of
> forwarding tables by pif, which as a bonus keeps track generically which
> pifs have implemented forwarding tables so far.
>
> For now this doesn't simplify a lot textually, because many places that
> need this also have other special cases to apply by pif. It does simplify
> a few crucial places though, and we expect it will become more useful as
> the flexibility of the forwarding table is improved.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> conf.c | 58 +++++++++++++++++++++++++++++++-------------------
> flow.c | 22 +++++++------------
> fwd.c | 65 ++++++++++++++++++++++++++++++---------------------------
> fwd.h | 4 ++--
> passt.h | 6 ++----
> 5 files changed, 83 insertions(+), 72 deletions(-)
>
> diff --git a/conf.c b/conf.c
> index b1ebb4a4..6ca61b74 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1252,11 +1252,17 @@ dns6:
> }
> }
>
> - info("Inbound forwarding:");
> - fwd_rules_print(&c->fwd_in);
> - if (c->mode == MODE_PASTA) {
> - info("Outbound forwarding:");
> - fwd_rules_print(&c->fwd_out);
> + for (i = 0; i < PIF_NUM_TYPES; i++) {
> + const char *dir = "Outbound";
> +
> + if (!c->fwd[i])
> + continue;
> +
> + if (i == PIF_HOST)
> + dir = "Inbound";
> +
> + info("%s forwarding rules (%s):", dir, pif_name(i));
> + fwd_rules_print(c->fwd[i]);
> }
> }
>
> @@ -2154,18 +2160,24 @@ void conf(struct ctx *c, int argc, char **argv)
>
> /* Forwarding options can be parsed now, after IPv4/IPv6 settings */
> fwd_probe_ephemeral();
> + fwd_rule_init(c);
> optind = 0;
> do {
> name = getopt_long(argc, argv, optstring, options, NULL);
>
> - if (name == 't')
> - conf_ports(c, name, optarg, &c->fwd_in, &tcp_in_mode);
> - else if (name == 'u')
> - conf_ports(c, name, optarg, &c->fwd_in, &udp_in_mode);
> - else if (name == 'T')
> - conf_ports(c, name, optarg, &c->fwd_out, &tcp_out_mode);
> - else if (name == 'U')
> - conf_ports(c, name, optarg, &c->fwd_out, &udp_out_mode);
> + if (name == 't') {
> + conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> + &tcp_in_mode);
> + } else if (name == 'u') {
> + conf_ports(c, name, optarg, c->fwd[PIF_HOST],
> + &udp_in_mode);
> + } else if (name == 'T') {
> + conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> + &tcp_out_mode);
> + } else if (name == 'U') {
> + conf_ports(c, name, optarg, c->fwd[PIF_SPLICE],
> + &udp_out_mode);
> + }
> } while (name != -1);
>
> if (c->mode == MODE_PASTA)
> @@ -2224,20 +2236,24 @@ void conf(struct ctx *c, int argc, char **argv)
> udp_out_mode = fwd_default;
>
> if (tcp_in_mode == FWD_MODE_AUTO) {
> - conf_ports_range_except(c, 't', "auto", &c->fwd_in, NULL, NULL,
> - 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> + conf_ports_range_except(c, 't', "auto", c->fwd[PIF_HOST],
> + NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
> + FWD_SCAN);
> }
> if (tcp_out_mode == FWD_MODE_AUTO) {
> - conf_ports_range_except(c, 'T', "auto", &c->fwd_out, NULL, "lo",
> - 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> + conf_ports_range_except(c, 'T', "auto", c->fwd[PIF_SPLICE],
> + NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
> + FWD_SCAN);
> }
> if (udp_in_mode == FWD_MODE_AUTO) {
> - conf_ports_range_except(c, 'u', "auto", &c->fwd_in, NULL, NULL,
> - 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> + conf_ports_range_except(c, 'u', "auto", c->fwd[PIF_HOST],
> + NULL, NULL, 1, NUM_PORTS - 1, NULL, 1,
> + FWD_SCAN);
> }
> if (udp_out_mode == FWD_MODE_AUTO) {
> - conf_ports_range_except(c, 'U', "auto", &c->fwd_out, NULL, "lo",
> - 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN);
> + conf_ports_range_except(c, 'U', "auto", c->fwd[PIF_SPLICE],
> + NULL, "lo", 1, NUM_PORTS - 1, NULL, 1,
> + FWD_SCAN);
> }
>
> if (!c->quiet)
> diff --git a/flow.c b/flow.c
> index c84857b2..2972ab87 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -503,10 +503,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
> {
> char estr[INANY_ADDRSTRLEN], ostr[INANY_ADDRSTRLEN];
> struct flow_common *f = &flow->f;
> + const struct fwd_table *fwd = c->fwd[f->pif[INISIDE]];
> const struct flowside *ini = &f->side[INISIDE];
> struct flowside *tgt = &f->side[TGTSIDE];
> const struct fwd_rule *rule = NULL;
> - const struct fwd_table *fwd;
> uint8_t tgtpif = PIF_NONE;
>
> assert(flow_new_entry == flow && f->state == FLOW_STATE_INI);
> @@ -514,6 +514,11 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
> assert(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE);
> assert(flow->f.state == FLOW_STATE_INI);
>
> + if (fwd) {
> + if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> + goto norule;
> + }
> +
> switch (f->pif[INISIDE]) {
> case PIF_TAP:
> memcpy(f->tap_omac, MAC_UNDEF, ETH_ALEN);
> @@ -521,20 +526,10 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow,
> break;
>
> case PIF_SPLICE:
> - fwd = &c->fwd_out;
> -
> - if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> - goto norule;
> -
Coverity Scan doesn't like this:
/home/sbrivio/passt/flow.c:528:3:
Type: Explicit null dereferenced (FORWARD_NULL)
/home/sbrivio/passt/flow.c:508:2:
1. assign_zero: Assigning: "rule" = "NULL".
/home/sbrivio/passt/flow.c:511:2:
2. path: Condition "flow_new_entry == flow", taking true branch.
/home/sbrivio/passt/flow.c:511:2:
3. path: Condition "f->state == FLOW_STATE_INI", taking true branch.
/home/sbrivio/passt/flow.c:512:2:
4. path: Condition "f->type == FLOW_TYPE_NONE", taking true branch.
/home/sbrivio/passt/flow.c:513:2:
5. path: Condition "f->pif[0] != PIF_NONE", taking true branch.
/home/sbrivio/passt/flow.c:513:2:
6. path: Condition "f->pif[1] == PIF_NONE", taking true branch.
/home/sbrivio/passt/flow.c:514:2:
7. path: Condition "flow->f.state == FLOW_STATE_INI", taking true branch.
/home/sbrivio/passt/flow.c:516:2:
8. path: Condition "fwd", taking false branch.
/home/sbrivio/passt/flow.c:521:2:
9. path: Switch case value "PIF_SPLICE".
/home/sbrivio/passt/flow.c:528:3:
10. var_deref_model: Passing null pointer "rule" to "fwd_nat_from_splice", which dereferences it.
/home/sbrivio/passt/fwd.c:1095:2:
10.1. path: Condition "!inany_is_loopback(&ini->eaddr)", taking false branch.
/home/sbrivio/passt/fwd.c:1095:2:
10.2. path: Condition "!inany_is_loopback(&ini->oaddr)", taking false branch.
/home/sbrivio/passt/fwd.c:1112:2:
10.3. path: Condition "proto == IPPROTO_UDP", taking true branch.
/home/sbrivio/passt/fwd.c:1116:2:
10.4. dereference: Dereferencing pointer "rule".
> tgtpif = fwd_nat_from_splice(rule, proto, ini, tgt);
> break;
>
> case PIF_HOST:
> - fwd = &c->fwd_in;
> -
> - if (!(rule = fwd_rule_search(fwd, ini, proto, rule_hint)))
> - goto norule;
> -
...and not this either:
/home/sbrivio/passt/flow.c:532:3:
Type: Explicit null dereferenced (FORWARD_NULL)
/home/sbrivio/passt/flow.c:508:2:
1. assign_zero: Assigning: "rule" = "NULL".
/home/sbrivio/passt/flow.c:511:2:
2. path: Condition "flow_new_entry == flow", taking true branch.
/home/sbrivio/passt/flow.c:511:2:
3. path: Condition "f->state == FLOW_STATE_INI", taking true branch.
/home/sbrivio/passt/flow.c:512:2:
4. path: Condition "f->type == FLOW_TYPE_NONE", taking true branch.
/home/sbrivio/passt/flow.c:513:2:
5. path: Condition "f->pif[0] != PIF_NONE", taking true branch.
/home/sbrivio/passt/flow.c:513:2:
6. path: Condition "f->pif[1] == PIF_NONE", taking true branch.
/home/sbrivio/passt/flow.c:514:2:
7. path: Condition "flow->f.state == FLOW_STATE_INI", taking true branch.
/home/sbrivio/passt/flow.c:516:2:
8. path: Condition "fwd", taking false branch.
/home/sbrivio/passt/flow.c:521:2:
9. path: Switch case value "PIF_HOST".
/home/sbrivio/passt/flow.c:532:3:
10. var_deref_model: Passing null pointer "rule" to "fwd_nat_from_host", which dereferences it.
/home/sbrivio/passt/fwd.c:1173:2:
10.1. dereference: Dereferencing pointer "rule".
I haven't checked why.
> tgtpif = fwd_nat_from_host(c, rule, proto, ini, tgt);
> fwd_neigh_mac_get(c, &tgt->oaddr, f->tap_omac);
> break;
>
> [...]
--
Stefano
next prev parent reply other threads:[~2026-03-25 0:54 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 7:37 [PATCH v3 00/25] RFC: Read-only dynamic update implementation David Gibson
2026-03-23 7:37 ` [PATCH v3 01/25] conf: runas can be const David Gibson
2026-03-23 7:37 ` [PATCH v3 02/25] vhost_user: Fix assorted minor cppcheck warnings David Gibson
2026-03-23 7:37 ` [PATCH v3 03/25] serialise: Split functions user for serialisation from util.c David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-25 1:50 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 04/25] serialise: Add helpers for serialising unsigned integers David Gibson
2026-03-23 7:37 ` [PATCH v3 05/25] fwd: Move selecting correct scan bitmap into fwd_sync_one() David Gibson
2026-03-23 7:37 ` [PATCH v3 06/25] fwd: Look up rule index in fwd_sync_one() David Gibson
2026-03-23 7:37 ` [PATCH v3 07/25] fwd: Store forwarding tables indexed by (origin) pif David Gibson
2026-03-25 0:54 ` Stefano Brivio [this message]
2026-03-25 4:04 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 08/25] fwd: Allow FWD_DUAL_STACK_ANY flag to be passed directly to fwd_rule_add() David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-25 4:07 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 09/25] fwd, conf: Expose ephemeral ports as bitmap rather than function David Gibson
2026-03-23 7:37 ` [PATCH v3 10/25] conf: Don't bother complaining about overlapping excluded ranges David Gibson
2026-03-23 7:37 ` [PATCH v3 11/25] conf: Move check for mapping port 0 to caller David Gibson
2026-03-23 7:37 ` [PATCH v3 12/25] conf: Move check for disabled interfaces earlier David Gibson
2026-03-23 7:37 ` [PATCH v3 13/25] pesto: Introduce stub configuration interface and tool David Gibson
2026-03-25 0:54 ` Stefano Brivio
2026-03-23 7:37 ` [PATCH v3 14/25] pesto: Add command line option parsing and debug messages David Gibson
2026-03-25 0:55 ` Stefano Brivio
2026-03-25 4:27 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 15/25] pesto: Expose list of pifs to pesto David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:34 ` David Gibson
2026-03-25 8:18 ` Stefano Brivio
2026-03-25 8:31 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 16/25] ip: Prepare ip.[ch] for sharing with pesto tool David Gibson
2026-03-23 7:37 ` [PATCH v3 17/25] inany: Prepare inany.[ch] " David Gibson
2026-03-23 7:37 ` [PATCH v3 18/25] fwd: Split forwading rule specification from its implementation state David Gibson
2026-03-23 7:37 ` [PATCH v3 19/25] ip: Define a bound for the string returned by ipproto_name() David Gibson
2026-03-23 7:37 ` [PATCH v3 20/25] fwd_rule: Move forwarding rule text formatting to common code David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:42 ` David Gibson
2026-03-25 8:18 ` Stefano Brivio
2026-03-25 23:54 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 21/25] pesto: Read current ruleset from passt/pasta and display it David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 4:43 ` David Gibson
2026-03-23 7:37 ` [PATCH v3 22/25] conf: Move port parsing functions to own file, ports.c David Gibson
2026-03-23 7:37 ` [PATCH v3 23/25] conf, fwd, ports, util: Move things around for pesto David Gibson
2026-03-23 7:37 ` [PATCH v3 24/25] pesto, conf: Parse, send and receive new rules David Gibson
2026-03-23 7:37 ` [PATCH v3 25/25] conf, fwd: Allow switching to new rules received from pesto David Gibson
2026-03-23 8:38 ` [PATCH v3 00/25] RFC: Read-only dynamic update implementation David Gibson
2026-03-25 0:56 ` Stefano Brivio
2026-03-25 1:00 ` Stefano Brivio
2026-03-25 4:44 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260325015423.4c217023@elisabeth \
--to=sbrivio@redhat.com \
--cc=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).