Iptables compat fixes #147
Conversation
| @@ -33,8 +33,10 @@ | |||
| [ -z "$IP_BINARY" ] && IP_BINARY=$(command -v ip) | |||
| [ -z "$IPTABLES" ] && IPTABLES=iptables_wrapper | |||
| [ -z "$IPTABLES_BINARY" ] && IPTABLES_BINARY=$(command -v iptables) | |||
| [ -z "$IPTABLES_BINARY" ] && IPTABLES_BINARY=$(command -v iptables-nft) | |||
There was a problem hiding this comment.
Please switch the order, so that iptables-nft is evaluated first.
Otherwise the first line, with iptables, populates the IPTABLES_BINARY variable, and the added second line is worthless.
Or did I understood the the logic wrong?
(There will likely be iptables present for the near future, due to scrambled dependencies...)
The same comment for the ip6tables below.
There was a problem hiding this comment.
Hmm, I'm not sure that is the right thing to do in general, though? Having both binaries available is really a weird setup, and I'm not sure we can assume any order in general.
Maybe a better solution for openwrt would be to explicitly set IP{,6}TABLES_BINARY in /etc/sqm/sqm.conf for the platform?
|
Iptables will likely "in general" be there for the near future. Iptables is so tightly/widely in the package dependencies that it will take time to sort out... My logic starts from the perspective that in master the presence of iptables-nft likely means that firewall4 and nftables is in use, so iptables-nft should be preferred (and thus evaluated first). As the fallback option, mainly for 21.02 etc., the old iptables can be evaluated. |
|
Hmm, just checked on my own machine, and that has both binaries as well, both provided by the 'iptables' package. So presence of the binaries is definitely not a good indicator for which is in use :/ So that leaves platform-specific configuration. I think we could just stick a |
For systems switching to nftables, the iptables-nft binary can be used as a compatibility layer for applications relying on iptables. Not every distribution (notably OpenWrt) installs a symlink from iptables-nft to iptables, though, so add a fallback check for iptables-nft if we can't find iptables itself. Both binaries can be present on the same system simultaneously, so we can't really infer anything from their presence. In the interest of not breaking existing setups, prefer the regular 'iptables' binary over 'iptables-nft'. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Add a new function to explicitly check if we found an iptables binary on startup; and call this from simple.qos so we can error out when this happens. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
OpenWrt is moving to using nftables by default, but the 'iptables' binary is likely to stick around for a while. So add a platform-specific preference for using iptables-nft (if it exists). We'll still fall back to 'iptables' if 'iptables-nft' is not installed on the system, and we'll still get the warning if neither exists and simple.qos is used. Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
tohojo
force-pushed
the
iptables-compat
branch
from
1bdb2aa
to
350432d
Compare
|
LGTM. |
|
So can we merge this? |
|
Hmm, yeah, not sure why we never got around to merging it... |
A couple of fixes to be more compatible with iptables-nft, and to allow us to lift the hard package dependency on iptables in the openwrt package...