netfilter: nf_nat: remove bogus direction check

Jakub reports spurious failures of the 'conntrack_reverse_clash.sh'
selftest.  A bogus test makes nat core resort to port rewrite even
though there is no need for this.

When the test is made, nf_nat_used_tuple() would already have caused us
to return if no other CPU had added a colliding entry.
Moreover, nf_nat_used_tuple() would have ignored the colliding entry if
their origin tuples had been the same.

All that is left to check is if the colliding entry in the hash table
is subject to NAT, and, if its not, if our entry matches in the reverse
direction, e.g. hash table has

addr1:1234 -> addr2:80, and we want to commit
addr2:80   -> addr1:1234.

Because we already checked that neither the new nor the committed entry is
subject to NAT we only have to check origin vs. reply tuple:
for non-nat entries, the reply tuple is always the inverted original.

Just in case there are more problems extend the error reporting
in the selftest while at it and dump conntrack table/stats on error.

Reported-by: Jakub Kicinski <kuba@kernel.org>
Closes: https://lore.kernel.org/netdev/20251206175135.4a56591b@kernel.org/
Fixes: d8f84a9bc7 ("netfilter: nf_nat: don't try nat source port reallocation for reverse dir clash")
Signed-off-by: Florian Westphal <fw@strlen.de>
This commit is contained in:
Florian Westphal
2025-12-08 16:00:34 +01:00
parent 99c6931fe1
commit 5ec8ca26fe
3 changed files with 12 additions and 17 deletions

View File

@@ -33,9 +33,14 @@ static void die(const char *e)
exit(111);
}
static void die_port(uint16_t got, uint16_t want)
static void die_port(const struct sockaddr_in *sin, uint16_t want)
{
fprintf(stderr, "Port number changed, wanted %d got %d\n", want, ntohs(got));
uint16_t got = ntohs(sin->sin_port);
char str[INET_ADDRSTRLEN];
inet_ntop(AF_INET, &sin->sin_addr, str, sizeof(str));
fprintf(stderr, "Port number changed, wanted %d got %d from %s\n", want, got, str);
exit(1);
}
@@ -100,7 +105,7 @@ int main(int argc, char *argv[])
die("child recvfrom");
if (peer.sin_port != htons(PORT))
die_port(peer.sin_port, PORT);
die_port(&peer, PORT);
} else {
if (sendto(s2, buf, LEN, 0, (struct sockaddr *)&sa1, sizeof(sa1)) != LEN)
continue;
@@ -109,7 +114,7 @@ int main(int argc, char *argv[])
die("parent recvfrom");
if (peer.sin_port != htons((PORT + 1)))
die_port(peer.sin_port, PORT + 1);
die_port(&peer, PORT + 1);
}
}

View File

@@ -45,6 +45,8 @@ if ip netns exec "$ns0" ./conntrack_reverse_clash; then
echo "PASS: No SNAT performed for null bindings"
else
echo "ERROR: SNAT performed without any matching snat rule"
ip netns exec "$ns0" conntrack -L
ip netns exec "$ns0" conntrack -S
exit 1
fi