Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luci-mod-firewall: Expand on naming of forwarding rule inside the zone #6537

Merged

Conversation

jonas2515
Copy link
Contributor

Apparently the "Forward" entry of the individual firewall zones controls forwarding within the zone (between the individual interfaces) only, and not the forwarding of packets from the zone to other zones. This is quite confusing, as the meaning is different from the global "Forward" option above, which does control forwarding between zones.

Quote from user jow on the forum:

The per-zone forward controls forwarding traffic among the ifaces of this
zone. Traffic from/to other zones is handled by the global forward policy,
or individual forwardings or rules.

See https://forum.openwrt.org/t/likely-bug-in-openwrt-firewall-rule-generation/18152

Let's try to be a bit more concise with the naming here and rename this entry to "Intra-Zone Forward", which hopefully makes the difference clear.

@jonas2515 jonas2515 marked this pull request as draft August 23, 2023 12:13
@jonas2515
Copy link
Contributor Author

Marked as draft because I haven't tested this...

Also I'm not too happy with the new name for the entry, better suggestions are welcome!

Maybe it would make more sense to rename the global option to "Forwarding between zones" and this one to "Forwarding within zone"?

@systemcrash
Copy link
Contributor

Intra and Inter have worked fine for aeons. Please rebase your changes onto current master.

@jow-
Copy link
Contributor

jow- commented Dec 6, 2023

Please spell it Intra zone forwarding or Intra zone forward to keep casing somewhat consistent with the other option labels.

@jonas2515 jonas2515 force-pushed the better-explain-forwarding-rule branch from fe78038 to 825595a Compare December 6, 2023 09:53
@jonas2515
Copy link
Contributor Author

Thanks for the reviews, rebased and renamed to "Intra zone forward".

@jonas2515 jonas2515 changed the title Draft: luci-mod-firewall: Expand on naming of forwarding rule inside the zone luci-mod-firewall: Expand on naming of forwarding rule inside the zone Dec 6, 2023
@jonas2515 jonas2515 marked this pull request as ready for review December 6, 2023 09:54
@systemcrash systemcrash force-pushed the better-explain-forwarding-rule branch from 825595a to d38edeb Compare December 28, 2023 23:19
@systemcrash
Copy link
Contributor

@jonas2515 Please fix your signed-off-by in the commit.

@jonas2515 jonas2515 force-pushed the better-explain-forwarding-rule branch from d38edeb to 7dd50c4 Compare December 29, 2023 01:37
@jonas2515
Copy link
Contributor Author

Ah sure, added the signed-off-by!

Apparently the "Forward" entry of the individual firewall zones controls
forwarding within the zone (between the individual interfaces) only, and not
the forwarding of packets from the zone to other zones. This is quite
confusing, as the meaning is different from the global "Forward" option
above, which does control forwarding between zones.

Quote from user jow on the forum:
> The per-zone forward controls forwarding traffic among the ifaces of this
> zone. Traffic from/to other zones is handled by the global forward policy,
> or individual forwardings or rules.

See https://forum.openwrt.org/t/likely-bug-in-openwrt-firewall-rule-generation/18152

Let's try to be a bit more concise with the naming here and rename this
entry to "Intra zone forward", which hopefully makes the difference clear.

Signed-off-by: Jonas Dreßler <verdre@v0yd.nl>
@systemcrash systemcrash force-pushed the better-explain-forwarding-rule branch from 7dd50c4 to 785da07 Compare December 30, 2023 02:51
@systemcrash systemcrash merged commit c74c861 into openwrt:master Dec 30, 2023
2 checks passed
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.

3 participants