Open lathiat opened 2 years ago
I guess deleting the
nft_list['status'] == 0 and
in https://github.com/sosreport/sos/blob/main/sos/report/plugins/firewall_tables.py#L80 will resolve this use case, am I right? We need to carefully review the change for possible negative consequences (all the if-then-else clauses is like a minefield walk).
The "iptables-save" command seems to successfully avoid loading any kernel module not in use, seems like we would be better off using it and avoiding all the complicated logic? This command is already used in the network namespace plugin also.
Actually, if nft
is present, then iptables-save
will load that kernel module - https://github.com/sosreport/sos/pull/2703, and is why we made some changes to the networking
plugin logic around it.
So, we're probably going to unfortunately end up with yet another conditional fork based on the presence of nft
.
I was affected by this bug too :)
I guess updating the condition to if nft_list['status'] != 0 or table in nft_ip_tables['ip']:
should do the trick, wdyt @pmoravec ?
It is tricky to eval and I am unsure in one situation. Since nft_list['status'] != 0
can have three reasons:
nft
command is not present (and status
is 126 or 127, per https://github.com/sosreport/sos/blob/0097a99fdf0501dde8ba334df34b1d4a8a3650d5/sos/report/plugins/__init__.py#L2386-L2387) and then we should collect any table
from ip_tables_names
nft
command itself was executed but returned nonzero status - then again it is safe to collect default tables.status
is None
(https://github.com/sosreport/sos/blob/0097a99fdf0501dde8ba334df34b1d4a8a3650d5/sos/report/plugins/__init__.py#L2513) - but then iptables-save
would load that kernel modules and we should not call it..?If my "iptables-save
would load kmods" understanding is right, then the condition should be if not nft_list['status'] or table in nft_ip_tables['ip']:
.
If my understanding is wrong, then Nadia's suggestion seems right to me.
We don't even necessarily need to use iptables-save
, we can leave the existing collect_iptable
usage, but we need to make sure it is called if nft is not installed, or some error happened and nft_ip_tables
was not properly filled.
My suggestion was more python-based than linux-based, so I definitely trust your opinion more here :)
Based on #2724 it started to use nft to determine which tables (raw/filter/mangle/nat) exist and should be exported to avoid loading the relevant kernel module if it's not in use. However if the 'nft' tool is not installed, it fails to collect any of the tables even if they are in use.
The "iptables-save" command seems to successfully avoid loading any kernel module not in use, seems like we would be better off using it and avoiding all the complicated logic? This command is already used in the network namespace plugin also.