Skip to content

redirect: preserve destination ipsets in NAT reflection - #104

Open
TMelik wants to merge 1 commit into
openwrt:masterfrom
TMelik:fix/reflection-destination-ipset
Open

TMelik wants to merge 1 commit into
openwrt:masterfrom
TMelik:fix/reflection-destination-ipset

Conversation

@TMelik

@TMelik TMelik commented Sep 14, 2026

Copy link
Copy Markdown

Problem

I have many DNAT port-forwardings, each with its own ipset containing two public IP addresses (I use ipset because the advanced settings allow only one external IP address). NAT reflection zones work fine with a single external IP address, but when I use ipset instead of the external IP address to specify multiple public IP addresses, NAT reflection zones stop working. More precisely, it works only for the first DNAT port forwarding; for all the others below, NAT reflection doesn't work and redirects to the host of the first DNAT rule.

Change

Keep the destination ipset match in reflection DNAT rules so that each port forwarding matches its own public IP addresses. If an external IP address (src_dip) is also configured, both conditions must match. Reflection SNAT continues to match the internal destination address and port.

This applies to non-inverted destination address/network sets, including combinations with destination ports. Other set matches keep their existing behavior.

Validation

Extend tests/03_rules/07_redirect with two forwards sharing TCP/UDP port 443, each with its own destination ipset and internal host. The public addresses in the sets are absent from WAN. The expected output checks that reflection DNAT keeps each set and reflection SNAT matches the translated address and port without the set.

  • The updated test fails on the base commit c2ae8c8940a89407da32fbd662d4010ee2c9bbe6: the four new reflection DNAT rules use the WAN address instead of their ipsets.
  • With the fix, the updated test and all 38 test files pass.
  • git diff --check passes.
./run_tests.sh tests/03_rules/07_redirect
./run_tests.sh

Tests were run with ucode b885dd0fe1e974551fb1233ca5f3aa74d80b74d9. These checks validate generated rules; they do not test packet delivery.

@brada4

brada4 commented Sep 14, 2026

Copy link
Copy Markdown

While there is nothing outright wrong with technical patched files the AI prose maker got it too radical, you should clarify in human language whether it pertains DNAT or REDIRECT
On the same note your friend refers to upstream, to tha best of my knowledge this repo is the upstream of upstreams for firewall4

@TMelik

TMelik commented Sep 14, 2026

Copy link
Copy Markdown
Author

Yes, sorry, I'll try to explain it in my own words.

I have many DNAT port-forwardings, each with its own ipset containing two public IP addresses (I use ipset because the advanced settings allow only one external IP address). NAT reflection zones work fine with a single external IP address, but when I use ipset instead of the external IP address to specify multiple public IP addresses, NAT reflection zones stop working. More precisely, it works only for the first DNAT port forwarding; for all the others below, NAT reflection doesn't work and redirects to the host of the first DNAT rule.

While there is nothing outright wrong with technical patched files the AI prose maker got it too radical, you should clarify in human language whether it pertains DNAT or REDIRECT On the same note your friend refers to upstream, to tha best of my knowledge this repo is the upstream of upstreams for firewall4

@brada4

brada4 commented Sep 14, 2026

Copy link
Copy Markdown

That is much better than 1000-word salad ;-)

Use commit --amend and push --force to update commit message with human text.

I think you can add new config section to test7 exactly passing new code path / fixed issue....

DNAT port forwards can use destination ipsets to select multiple public
IP addresses. Reflection drops the ipset match and, when src_dip is unset,
uses the WAN addresses instead. Forwards sharing a port then overlap and
send LAN traffic to the host from the first rule. Public addresses found
only in the ipset do not work with reflection at all.

Keep supported destination ipset matches in reflection DNAT rules. Use
the set to select public addresses, retaining src_dip as an additional
constraint when configured. Reflection SNAT still matches the internal
destination address and port.

Extend 07_redirect with two forwards sharing TCP/UDP port 443, each using
its own destination ipset and internal host. Check the generated DNAT and
reflection rules, including SNAT without the public-address ipset.

Signed-off-by: Tigran Melikyan <opensource@cyberhayq.am>
@TMelik
TMelik force-pushed the fix/reflection-destination-ipset branch from a13efe5 to 577846f Compare September 15, 2026 07:31
@brada4

brada4 commented Sep 15, 2026

Copy link
Copy Markdown

@jow- hello

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants