On 27.08.25 11:05, Florian Westphal wrote:
If the connection isn't subject to snat, why to we need to mangle the
source address in the first place?
It is not limited to SNAT/MASQUERADE.
DNAT also affects which source address should be used, depending on the packet
direction.
With DNAT, the *destination* of the original direction is changed.
In the reply direction, this becomes the *source* address.
So reply packets of a DNAT connection are effectively subject to source address
translation. If icmp_ndo_send doesn’t account for this, rate limiting breaks,
which is exactly the problem this function was meant to solve.
Don't understand this either. Why these checks?
AFAICS you can keep the original check in place, and then:
replace this
orig_ip = ip_hdr(skb_in)->saddr;
- ip_hdr(skb_in)->saddr = ct->tuplehash[0].tuple.src.u3.ip;
... with ...
You are right: the code can be simplified. I'm not sure show this slipped through.
I will send an updated patch with this change — thanks for the suggestion.
However, the old check (IPS_SRC_NAT only) cannot be kept, because:
- Reply packets of a DNAT connection also need handling.
- Reply packets of a pure SNAT connection don’t need it, but replacing the
address is a no-op in that case (tuple == skb address).
To avoid unnecessary translations, I suggested the direction-specific checks.
Another option is to simplify them to:
if (!(ct->status & IPS_NAT_MASK)) { … }
This ensures we only ever touch connections with NAT, while keeping the code
straightforward.
Without dnat, the reply tuple saddr == original tuple daddr.
With dnat, its the dnat targets' address (i.e., the real destination
the client is talking to).
Yes, exactly.
If you are worried about "dnat to", then please update the commit
message, which only mentions masquerade/snat.
Correct — the change not only fixes SNAT-in-reply handling, but also adds
proper handling for DNAT in the reply direction, which was missing entirely.
I will update the commit message to reflect this.
Best regards,
Fabian
