Re: [PATCH v3 07/25] fwd: Store forwarding tables indexed by (origin) pif

On Wed, Mar 25, 2026 at 01:54:24AM +0100, Stefano Brivio wrote:

On Mon, 23 Mar 2026 18:37:14 +1100 David Gibson 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

[snip]

/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;

Oops. That's because we're relying on the fact that we _do_ have populated tables for HOST and SPLICE. That's constructed a very long way away, so of course coverity can't figure that out. I've fixed it with assert()s for now, and those should go away if/when we implement forwarding tables for TAP. -- 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