search

libremesh / lime-packages

LibreMesh packages configuring OpenWrt for wireless mesh networking
https://libremesh.org/
GNU Affero General Public License v3.0
276 stars 95 forks source link

Port lime-proto-bmx7 to fw4 and nftables #1020

Closed pony1k closed 1 year ago

pony1k commented 1 year ago

This should make lime-proto-bmx7 compatible to firewall4 and nftables. The ebtables rules are now added with an init.d script, as well as the iptables rules for the mtu fix in case there is no firewall installed.

pony1k commented 1 year ago

This change is necessary because the scripts in /etc/firewall.lime.d are no longer run since 8aa007e456d3cc98fd45997689624dbbeade3c7b. It was tested with firewall4 installed on archer c7 and at least one other device.

ilario commented 1 year ago

Looks good to me! :D Let's see if @G10h4ck or @dangowrt or @aparcar have comments.

pony1k commented 1 year ago

What you think about keeping /etc/firewall.lime.d/bmx7-not-over-bat0-ebtables permanently (instead of generating and deleting it in the bmx7 proto) use an include section in /etc/config/firewall with option fw4_compatible 1, and then use option enabled 0 vs. option enabled 1 to toggle whether the user wants bmx7-over-batadv or not?

For anygw, we use scripts in /etc/init.d now. I think it would be nice to consistently use the same method everywhere. It would also mean that we undo some changes in https://github.com/libremesh/lime-packages/commit/8aa007e456d3cc98fd45997689624dbbeade3c7b. This is why I decided to make an init.d script.

Or even go one step further and use nftables directly, you can set option type 'nftables' for an include in /etc/config/firewall, see https://github.com/openwrt/firewall4/blob/master/tests/06_includes/01_nft_includes for inspiration.

I like this idea, and I was thinking about proposing this too. But, since nftable rules and xtable rules don't mix well and we also have some anygw rules in the same chain, we need to change this everywhere in one commit. I would volunteer for that. @G10h4ck , @aparcar , what do you think?

dangowrt commented 1 year ago

I think the decision to use an init script was made without considering that nftables may be flushed and restarted every time when a new interface is added, PPPoE internet WAN comes up or is re-connected every 24 hours on a gateway and many other situations. I guess there was simply no awareness that include sections in /etc/config/firewall now require option fw4_compatible 1 to still work, and that's the whole reason why we have an init script there now...

tl;dr: Yes, we should undo the change in https://github.com/libremesh/lime-packages/commit/8aa007e456d3cc98fd45997689624dbbeade3c7b which moved the script to be an init script (unfortunately without a meaningful commit description hinting why this was done), as this solution will have several problems on nodes which use dynamic interfaces (such as PPPoE WAN on a gateway which is removed and re-added every time the PPPoE link goes down/up, ie. every 24h with most ISPs)

pony1k commented 1 year ago

Closing, because making /etc/init.d scripts for nftable rules is not a good approach.