| Message ID | 20240827145242.3094777-2-leitao@debian.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netfilter: Make IP_NF_IPTABLES_LEGACY selectable | expand |
On Tue, 27 Aug 2024 07:52:40 -0700 Breno Leitao wrote:
> +++ b/tools/testing/selftests/net/config
You gotta check all the configs, net is now fine, but bpf still breaks.
There may be more configs we don't use in CI.
BTW I'm not saying anything about the change itself. There's a non-zero
chance that netfilter maintainers made the option hidden on purpose..
Hello Jakub, On Wed, Aug 28, 2024 at 07:42:40AM -0700, Jakub Kicinski wrote: > On Tue, 27 Aug 2024 07:52:40 -0700 Breno Leitao wrote: > > +++ b/tools/testing/selftests/net/config > > You gotta check all the configs, net is now fine, but bpf still breaks. > There may be more configs we don't use in CI. Sure, how can I find which configs I should care about? > BTW I'm not saying anything about the change itself. There's a non-zero > chance that netfilter maintainers made the option hidden on purpose.. Right, but it seems there was a plan to have it enabled in the future, as least that is what I read in a9525c7f6219c ("netfilter: xtables: allow xtables-nft only builds") In the future the _LEGACY symbol will become visible and the select statements will be turned into 'depends on', but for now be on safe side so "make oldconfig" won't break things. Also, this was discussed in the thread below, and it seems it is fine to make the symbols visible: https://lore.kernel.org/all/20240822132022.GA25665@breakpoint.cc/ Thanks for the review, --breno
On Wed, 28 Aug 2024 08:05:09 -0700 Breno Leitao wrote: > On Wed, Aug 28, 2024 at 07:42:40AM -0700, Jakub Kicinski wrote: > > On Tue, 27 Aug 2024 07:52:40 -0700 Breno Leitao wrote: > > > +++ b/tools/testing/selftests/net/config > > > > You gotta check all the configs, net is now fine, but bpf still breaks. > > There may be more configs we don't use in CI. > > Sure, how can I find which configs I should care about? There are various configs in the tree. Grep for the configs you convert from select to depends on, they will all need updating.
Hello Jakub, On Wed, Aug 28, 2024 at 11:41:23AM -0700, Jakub Kicinski wrote: > On Wed, 28 Aug 2024 08:05:09 -0700 Breno Leitao wrote: > > On Wed, Aug 28, 2024 at 07:42:40AM -0700, Jakub Kicinski wrote: > > > On Tue, 27 Aug 2024 07:52:40 -0700 Breno Leitao wrote: > > > > +++ b/tools/testing/selftests/net/config > > > > > > You gotta check all the configs, net is now fine, but bpf still breaks. > > > There may be more configs we don't use in CI. > > > > Sure, how can I find which configs I should care about? > > There are various configs in the tree. Grep for the configs you convert > from select to depends on, they will all need updating. I am looking at all files that depend on these Kconfig options, and there are a lot of tests. Thinking more about the problem, it doesn't seem to be a good idea to change dependency from all NF modules to NF_IPTABLES_LEGACY. In other words, the `s/selects/depends on/` is the part that is causing all this hassle, and it seems unnecessary. That said, I would suggest we do not change the dependency, and keep the "select NF_IPTABLES_LEGACY", and keep NF_IPTABLES_LEGACY user selectable. This will make the patch safer, while fixing the problem.
On Thu, 29 Aug 2024 03:08:01 -0700 Breno Leitao wrote: > > There are various configs in the tree. Grep for the configs you convert > > from select to depends on, they will all need updating. > > I am looking at all files that depend on these Kconfig options, and > there are a lot of tests. > > Thinking more about the problem, it doesn't seem to be a good idea to > change dependency from all NF modules to NF_IPTABLES_LEGACY. In other > words, the `s/selects/depends on/` is the part that is causing all this > hassle, and it seems unnecessary. > > That said, I would suggest we do not change the dependency, and keep the > "select NF_IPTABLES_LEGACY", and keep NF_IPTABLES_LEGACY user selectable. Good idea, sounds much simpler!
On Thu, Aug 29, 2024 at 07:53:03AM -0700, Jakub Kicinski wrote: > On Thu, 29 Aug 2024 03:08:01 -0700 Breno Leitao wrote: > > > There are various configs in the tree. Grep for the configs you convert > > > from select to depends on, they will all need updating. > > > > I am looking at all files that depend on these Kconfig options, and > > there are a lot of tests. > > > > Thinking more about the problem, it doesn't seem to be a good idea to > > change dependency from all NF modules to NF_IPTABLES_LEGACY. In other > > words, the `s/selects/depends on/` is the part that is causing all this > > hassle, and it seems unnecessary. > > > > That said, I would suggest we do not change the dependency, and keep the > > "select NF_IPTABLES_LEGACY", and keep NF_IPTABLES_LEGACY user selectable. > > Good idea, sounds much simpler! Thanks, I will submit the patch soon. --breno
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig index 1b991b889506..a06c1903183f 100644 --- a/net/ipv4/netfilter/Kconfig +++ b/net/ipv4/netfilter/Kconfig @@ -12,7 +12,12 @@ config NF_DEFRAG_IPV4 # old sockopt interface and eval loop config IP_NF_IPTABLES_LEGACY - tristate + tristate "Legacy IP tables support" + default n + select NETFILTER_XTABLES + help + iptables is a general, extensible packet identification legacy framework. + This is not needed if you are using iptables over nftables (iptables-nft). config NF_SOCKET_IPV4 tristate "IPv4 socket lookup support" @@ -177,7 +182,7 @@ config IP_NF_MATCH_TTL config IP_NF_FILTER tristate "Packet filtering" default m if NETFILTER_ADVANCED=n - select IP_NF_IPTABLES_LEGACY + depends on IP_NF_IPTABLES_LEGACY help Packet filtering defines a table `filter', which has a series of rules for simple packet filtering at local input, forwarding and @@ -217,7 +222,7 @@ config IP_NF_NAT default m if NETFILTER_ADVANCED=n select NF_NAT select NETFILTER_XT_NAT - select IP_NF_IPTABLES_LEGACY + depends on IP_NF_IPTABLES_LEGACY help This enables the `nat' table in iptables. This allows masquerading, port forwarding and other forms of full Network Address Port @@ -258,7 +263,7 @@ endif # IP_NF_NAT config IP_NF_MANGLE tristate "Packet mangling" default m if NETFILTER_ADVANCED=n - select IP_NF_IPTABLES_LEGACY + depends on IP_NF_IPTABLES_LEGACY help This option adds a `mangle' table to iptables: see the man page for iptables(8). This table is used for various packet alterations @@ -293,7 +298,7 @@ config IP_NF_TARGET_TTL # raw + specific targets config IP_NF_RAW tristate 'raw table support (required for NOTRACK/TRACE)' - select IP_NF_IPTABLES_LEGACY + depends on IP_NF_IPTABLES_LEGACY help This option adds a `raw' table to iptables. This table is the very first in the netfilter framework and hooks in at the PREROUTING @@ -305,9 +310,7 @@ config IP_NF_RAW # security table for MAC policy config IP_NF_SECURITY tristate "Security table" - depends on SECURITY - depends on NETFILTER_ADVANCED - select IP_NF_IPTABLES_LEGACY + depends on SECURITY && NETFILTER_ADVANCED && IP_NF_IPTABLES_LEGACY help This option adds a `security' table to iptables, for use with Mandatory Access Control (MAC) policy. diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config index 5b9baf708950..90e997cfa12e 100644 --- a/tools/testing/selftests/net/config +++ b/tools/testing/selftests/net/config @@ -28,6 +28,7 @@ CONFIG_NET_FOU=y CONFIG_NET_FOU_IP_TUNNELS=y CONFIG_NETFILTER=y CONFIG_NETFILTER_ADVANCED=y +CONFIG_NETFILTER_XT_TARGET_HL=m CONFIG_NF_CONNTRACK=m CONFIG_IPV6_MROUTE=y CONFIG_IPV6_SIT=y @@ -35,6 +36,11 @@ CONFIG_IP_DCCP=m CONFIG_NF_NAT=m CONFIG_IP6_NF_IPTABLES=m CONFIG_IP_NF_IPTABLES=m +CONFIG_IP_NF_IPTABLES_LEGACY=m +CONFIG_IP_NF_FILTER=m +CONFIG_IP_NF_TARGET_REJECT=m +CONFIG_IP_NF_TARGET_MASQUERADE=m +CONFIG_IP_NF_MANGLE=m CONFIG_IP6_NF_NAT=m CONFIG_IP6_NF_RAW=m CONFIG_IP_NF_NAT=m @@ -54,6 +60,7 @@ CONFIG_MPTCP=y CONFIG_NF_TABLES=m CONFIG_NF_TABLES_IPV6=y CONFIG_NF_TABLES_IPV4=y +CONFIG_NF_REJECT_IPV4=y CONFIG_NFT_NAT=m CONFIG_NETFILTER_XT_MATCH_LENGTH=m CONFIG_NET_ACT_CSUM=m @@ -106,4 +113,5 @@ CONFIG_CRYPTO_ARIA=y CONFIG_XFRM_INTERFACE=m CONFIG_XFRM_USER=m CONFIG_IP_NF_MATCH_RPFILTER=m +CONFIG_IP_NF_TARGET_MASQUERADE=m CONFIG_IP6_NF_MATCH_RPFILTER=m
This option makes IP_NF_IPTABLES_LEGACY user selectable, giving users the option to configure iptables without enabling any other config. Suggested-by: Florian Westphal <fw@strlen.de> Signed-off-by: Breno Leitao <leitao@debian.org> --- net/ipv4/netfilter/Kconfig | 19 +++++++++++-------- tools/testing/selftests/net/config | 8 ++++++++ 2 files changed, 19 insertions(+), 8 deletions(-)