On Wed, 24 Jul 2024 16:20:57 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Currently, we start by handling the common case, where we don't translate the destination address, then we modify the tgt side for the special cases. In the process we do comparisons on the tentatively set fields in tgt, which obscures the fact that tgt should be an essentially pure function of ini, and risks people examining fields of tgt that are not yet initialized. To make this clearer, do all our tests on 'ini', constructing tgt from scratch on that basis. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- fwd.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/fwd.c b/fwd.c index 8c1f3d91..bd16ac42 100644 --- a/fwd.c +++ b/fwd.c @@ -169,22 +169,22 @@ void fwd_scan_ports_init(struct ctx *c) uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt) { - tgt->eaddr = ini->faddr; - tgt->eport = ini->fport; - - if (proto == IPPROTO_UDP && tgt->eport == 53 && - inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) {These blocks are all one-liners now and curly brackets could go away, I guess?+ if (proto == IPPROTO_UDP && ini->fport == 53 && + inany_equals4(&ini->faddr, &c->ip4.dns_match)) { tgt->eaddr = inany_from_v4(c->ip4.dns_host); - } else if (proto == IPPROTO_UDP && tgt->eport == 53 && - inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) { + } else if (proto == IPPROTO_UDP && ini->fport == 53 && + inany_equals6(&ini->faddr, &c->ip6.dns_match)) { tgt->eaddr.a6 = c->ip6.dns_host; - } else if (!c->no_map_gw) { - if (inany_equals4(&tgt->eaddr, &c->ip4.gw)) - tgt->eaddr = inany_loopback4; - else if (inany_equals6(&tgt->eaddr, &c->ip6.gw)) + } else if (!c->no_map_gw && inany_equals4(&ini->faddr, &c->ip4.gw)) { + tgt->eaddr = inany_loopback4; + } else if (!c->no_map_gw && inany_equals6(&ini->faddr, &c->ip6.gw)) { tgt->eaddr = inany_loopback6;Resulting excess tab here.+ } else { + tgt->eaddr = ini->faddr; } + tgt->eport = ini->fport; + /* The relevant addr_out controls the host side source address. This * may be unspecified, which allows the kernel to pick an address. */-- Stefano