Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[David Miller]

Any reason you're not sending this to the netfilter developer list
mentioned in MAINTAINERS, or it's chief maintainer Patrick McHardy?

NETFILTER/IPTABLES/IPCHAINS
P:	Rusty Russell
P:	Marc Boucher
P:	James Morris
P:	Harald Welte
P:	Jozsef Kadlecsik
P:	Patrick McHardy
M:	kaber@trash.net
L:	netfilter-devel@lists.netfilter.org
L:	netfilter@lists.netfilter.org (subscribers-only)
L:	coreteam@netfilter.org
W:	http://www.netfilter.org/
W:	http://www.iptables.org/
S:	Supported

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Sam Ravnborg]

On Tue, Jul 24, 2007 at 08:36:33AM +0300, Al Boldi wrote:
> 
> Replaces NF_CONNTRACK_ENABLED with NF_CONNTRACK and selects it for 
> NF_CONNTRACK_IPV4 and NF_CONNTRACK_IPV6
> 
> This exposes IPv4/6 connection tracking options for easier Kconfig setup.
> 
> Signed-off-by: Al Boldi <a1426z@gawab.com>
> Cc: David Miller <davem@davemloft.net>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
> --- a/net/netfilter/Kconfig	2007-07-09 06:38:52.000000000 +0300
> +++ b/net/netfilter/Kconfig	2007-07-24 08:28:06.000000000 +0300
> @@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
>  	  and is also scheduled to replace the old syslog-based ipt_LOG
>  	  and ip6t_LOG modules.
>  
> -# Rename this to NF_CONNTRACK in a 2.6.25
> -config NF_CONNTRACK_ENABLED
> +config NF_CONNTRACK
>  	tristate "Netfilter connection tracking support"
>  	help
>  	  Connection tracking keeps a record of what packets have passed
> @@ -40,10 +39,6 @@ config NF_CONNTRACK_ENABLED
>  
>  	  To compile it as a module, choose M here.  If unsure, say N.
>  
> -config NF_CONNTRACK
> -	tristate
> -	default NF_CONNTRACK_ENABLED
> -
>  config NF_CT_ACCT
>  	bool "Connection tracking flow accounting"
>  	depends on NF_CONNTRACK
> --- a/net/ipv4/netfilter/Kconfig	2007-07-09 06:38:50.000000000 +0300
> +++ b/net/ipv4/netfilter/Kconfig	2007-07-24 08:27:39.000000000 +0300
> @@ -7,7 +7,7 @@ menu "IP: Netfilter Configuration"
>  
>  config NF_CONNTRACK_IPV4
>  	tristate "IPv4 connection tracking support (required for NAT)"
> -	depends on NF_CONNTRACK
> +	select NF_CONNTRACK
>  	---help---
>  	  Connection tracking keeps a record of what packets have passed
>  	  through your machine, in order to figure out how they are related
> --- a/net/ipv6/netfilter/Kconfig	2007-07-09 06:38:51.000000000 +0300
> +++ b/net/ipv6/netfilter/Kconfig	2007-07-24 08:27:54.000000000 +0300
> @@ -7,7 +7,8 @@ menu "IPv6: Netfilter Configuration (EXP
>  
>  config NF_CONNTRACK_IPV6
>  	tristate "IPv6 connection tracking support (EXPERIMENTAL)"
> -	depends on INET && IPV6 && EXPERIMENTAL && NF_CONNTRACK
> +	depends on INET && IPV6 && EXPERIMENTAL
> +	select NF_CONNTRACK
>  	---help---
>  	  Connection tracking keeps a record of what packets have passed
>  	  through your machine, in order to figure out how they are related
> 
This change looks wrong.
Due to the reverse nature of "select" kconfig cannot fulfill the dependencies
of selected symbols. So as a rule of thumb select should only select
symbols with no menu and no dependencies to avoid some of the
problems that have popped up during the last months.

	Sam

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Patrick McHardy]

Sam Ravnborg wrote:
> On Tue, Jul 24, 2007 at 08:36:33AM +0300, Al Boldi wrote:
> 
>>Replaces NF_CONNTRACK_ENABLED with NF_CONNTRACK and selects it for 
>>NF_CONNTRACK_IPV4 and NF_CONNTRACK_IPV6
>>
>>This exposes IPv4/6 connection tracking options for easier Kconfig setup.
>>
>>Signed-off-by: Al Boldi <a1426z@gawab.com>
>>Cc: David Miller <davem@davemloft.net>
>>Cc: Sam Ravnborg <sam@ravnborg.org>
>>Cc: Andrew Morton <akpm@linux-foundation.org>
>>---
>>--- a/net/netfilter/Kconfig	2007-07-09 06:38:52.000000000 +0300
>>+++ b/net/netfilter/Kconfig	2007-07-24 08:28:06.000000000 +0300
>>@@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
>> 	  and is also scheduled to replace the old syslog-based ipt_LOG
>> 	  and ip6t_LOG modules.
>> 
>>-# Rename this to NF_CONNTRACK in a 2.6.25
>>-config NF_CONNTRACK_ENABLED
>>+config NF_CONNTRACK


We kept this mainly for an easier upgrade. As the comment states, it
should go in 2.6.25, at which time all people having reconfigured
their kernel at least once since ip_conntrack was removed will have
the NF_CONNTRACK option set to the same value as NF_CONNTRACK_ENABLED.

>>--- a/net/ipv4/netfilter/Kconfig	2007-07-09 06:38:50.000000000 +0300
>>+++ b/net/ipv4/netfilter/Kconfig	2007-07-24 08:27:39.000000000 +0300
>>@@ -7,7 +7,7 @@ menu "IP: Netfilter Configuration"
>> 
>> config NF_CONNTRACK_IPV4
>> 	tristate "IPv4 connection tracking support (required for NAT)"
>>-	depends on NF_CONNTRACK
>>+	select NF_CONNTRACK
>> 	---help---
>> 	  Connection tracking keeps a record of what packets have passed
>> 	  through your machine, in order to figure out how they are related
>>--- a/net/ipv6/netfilter/Kconfig	2007-07-09 06:38:51.000000000 +0300
>>+++ b/net/ipv6/netfilter/Kconfig	2007-07-24 08:27:54.000000000 +0300
>>@@ -7,7 +7,8 @@ menu "IPv6: Netfilter Configuration (EXP
>> 
>> config NF_CONNTRACK_IPV6
>> 	tristate "IPv6 connection tracking support (EXPERIMENTAL)"
>>-	depends on INET && IPV6 && EXPERIMENTAL && NF_CONNTRACK
>>+	depends on INET && IPV6 && EXPERIMENTAL
>>+	select NF_CONNTRACK
>> 	---help---
>> 	  Connection tracking keeps a record of what packets have passed
>> 	  through your machine, in order to figure out how they are related
>>
> 
> This change looks wrong.
> Due to the reverse nature of "select" kconfig cannot fulfill the dependencies
> of selected symbols. So as a rule of thumb select should only select
> symbols with no menu and no dependencies to avoid some of the
> problems that have popped up during the last months.


In this case it looks OK since the dependencies of IPv4 connection
tracking are (besides NF_CONNTRACK) are superset of those of
nf_conntrack.

But I vaguely recall having tried this myself and it broke somewhere,
maybe it was because of the NF_CONNTRACK_ENABLED option, I can't
recall anymore. Al, if this also works without removal of
NF_CONNTRACK_ENABLED, please resend without that part.

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Al Boldi]

Patrick McHardy wrote:
> Sam Ravnborg wrote:
> > On Tue, Jul 24, 2007 at 08:36:33AM +0300, Al Boldi wrote:
> >>Replaces NF_CONNTRACK_ENABLED with NF_CONNTRACK and selects it for
> >>NF_CONNTRACK_IPV4 and NF_CONNTRACK_IPV6
> >>
> >>This exposes IPv4/6 connection tracking options for easier Kconfig
> >> setup.
> >>
> >>Signed-off-by: Al Boldi <a1426z@gawab.com>
> >>Cc: David Miller <davem@davemloft.net>
> >>Cc: Sam Ravnborg <sam@ravnborg.org>
> >>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>---
> >>--- a/net/netfilter/Kconfig	2007-07-09 06:38:52.000000000 +0300
> >>+++ b/net/netfilter/Kconfig	2007-07-24 08:28:06.000000000 +0300
> >>@@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
> >> 	  and is also scheduled to replace the old syslog-based ipt_LOG
> >> 	  and ip6t_LOG modules.
> >>
> >>-# Rename this to NF_CONNTRACK in a 2.6.25
> >>-config NF_CONNTRACK_ENABLED
> >>+config NF_CONNTRACK
>
> We kept this mainly for an easier upgrade. As the comment states, it
> should go in 2.6.25, at which time all people having reconfigured
> their kernel at least once since ip_conntrack was removed will have
> the NF_CONNTRACK option set to the same value as NF_CONNTRACK_ENABLED.
>
> >>--- a/net/ipv4/netfilter/Kconfig	2007-07-09 06:38:50.000000000 +0300
> >>+++ b/net/ipv4/netfilter/Kconfig	2007-07-24 08:27:39.000000000 +0300
> >>@@ -7,7 +7,7 @@ menu "IP: Netfilter Configuration"
> >>
> >> config NF_CONNTRACK_IPV4
> >> 	tristate "IPv4 connection tracking support (required for NAT)"
> >>-	depends on NF_CONNTRACK
> >>+	select NF_CONNTRACK
> >> 	---help---
> >> 	  Connection tracking keeps a record of what packets have passed
> >> 	  through your machine, in order to figure out how they are related
> >>--- a/net/ipv6/netfilter/Kconfig	2007-07-09 06:38:51.000000000 +0300
> >>+++ b/net/ipv6/netfilter/Kconfig	2007-07-24 08:27:54.000000000 +0300
> >>@@ -7,7 +7,8 @@ menu "IPv6: Netfilter Configuration (EXP
> >>
> >> config NF_CONNTRACK_IPV6
> >> 	tristate "IPv6 connection tracking support (EXPERIMENTAL)"
> >>-	depends on INET && IPV6 && EXPERIMENTAL && NF_CONNTRACK
> >>+	depends on INET && IPV6 && EXPERIMENTAL
> >>+	select NF_CONNTRACK
> >> 	---help---
> >> 	  Connection tracking keeps a record of what packets have passed
> >> 	  through your machine, in order to figure out how they are related
> >
> > This change looks wrong.
> > Due to the reverse nature of "select" kconfig cannot fulfill the
> > dependencies of selected symbols. So as a rule of thumb select should
> > only select symbols with no menu and no dependencies to avoid some of
> > the
> > problems that have popped up during the last months.
>
> In this case it looks OK since the dependencies of IPv4 connection
> tracking are (besides NF_CONNTRACK) are superset of those of
> nf_conntrack.
>
> But I vaguely recall having tried this myself and it broke somewhere,
> maybe it was because of the NF_CONNTRACK_ENABLED option, I can't
> recall anymore. Al, if this also works without removal of
> NF_CONNTRACK_ENABLED, please resend without that part.

It doesn't.  But how about this, if you really can't live without 
NF_CONNTRACK_ENBLED:

==================
--- Kconfig.old	2007-07-09 06:38:52.000000000 +0300
+++ Kconfig	2007-07-24 20:24:27.000000000 +0300
@@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
 	  and is also scheduled to replace the old syslog-based ipt_LOG
 	  and ip6t_LOG modules.
 
-# Rename this to NF_CONNTRACK in a 2.6.25
-config NF_CONNTRACK_ENABLED
+config NF_CONNTRACK
 	tristate "Netfilter connection tracking support"
 	help
 	  Connection tracking keeps a record of what packets have passed
@@ -40,9 +39,9 @@ config NF_CONNTRACK_ENABLED
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
-config NF_CONNTRACK
+config NF_CONNTRACK_ENABLED
 	tristate
-	default NF_CONNTRACK_ENABLED
+	default NF_CONNTRACK
 
 config NF_CT_ACCT
 	bool "Connection tracking flow accounting"
==================


Thanks!

--
Al

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Patrick McHardy]

Al Boldi wrote:
> Patrick McHardy wrote:
> 
>>But I vaguely recall having tried this myself and it broke somewhere,
>>maybe it was because of the NF_CONNTRACK_ENABLED option, I can't
>>recall anymore. Al, if this also works without removal of
>>NF_CONNTRACK_ENABLED, please resend without that part.
> 
> 
> It doesn't.  But how about this, if you really can't live without 
> NF_CONNTRACK_ENBLED:
> 
> ==================
> --- Kconfig.old	2007-07-09 06:38:52.000000000 +0300
> +++ Kconfig	2007-07-24 20:24:27.000000000 +0300
> @@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
>  	  and is also scheduled to replace the old syslog-based ipt_LOG
>  	  and ip6t_LOG modules.
>  
> -# Rename this to NF_CONNTRACK in a 2.6.25
> -config NF_CONNTRACK_ENABLED
> +config NF_CONNTRACK
>  	tristate "Netfilter connection tracking support"
>  	help
>  	  Connection tracking keeps a record of what packets have passed
> @@ -40,9 +39,9 @@ config NF_CONNTRACK_ENABLED
>  
>  	  To compile it as a module, choose M here.  If unsure, say N.
>  
> -config NF_CONNTRACK
> +config NF_CONNTRACK_ENABLED
>  	tristate
> -	default NF_CONNTRACK_ENABLED
> +	default NF_CONNTRACK
>  
>  config NF_CT_ACCT
>  	bool "Connection tracking flow accounting"


That defeats the only purpose why we kept it. How about we change this
once we remove it, in 2.6.25?

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Al Boldi]

Patrick McHardy wrote:
> Al Boldi wrote:
> > Patrick McHardy wrote:
> >>But I vaguely recall having tried this myself and it broke somewhere,
> >>maybe it was because of the NF_CONNTRACK_ENABLED option, I can't
> >>recall anymore. Al, if this also works without removal of
> >>NF_CONNTRACK_ENABLED, please resend without that part.
> >
> > It doesn't.  But how about this, if you really can't live without
> > NF_CONNTRACK_ENBLED:
> >
> > ==================
> > --- Kconfig.old	2007-07-09 06:38:52.000000000 +0300
> > +++ Kconfig	2007-07-24 20:24:27.000000000 +0300
> > @@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
> >  	  and is also scheduled to replace the old syslog-based ipt_LOG
> >  	  and ip6t_LOG modules.
> >
> > -# Rename this to NF_CONNTRACK in a 2.6.25
> > -config NF_CONNTRACK_ENABLED
> > +config NF_CONNTRACK
> >  	tristate "Netfilter connection tracking support"
> >  	help
> >  	  Connection tracking keeps a record of what packets have passed
> > @@ -40,9 +39,9 @@ config NF_CONNTRACK_ENABLED
> >
> >  	  To compile it as a module, choose M here.  If unsure, say N.
> >
> > -config NF_CONNTRACK
> > +config NF_CONNTRACK_ENABLED
> >  	tristate
> > -	default NF_CONNTRACK_ENABLED
> > +	default NF_CONNTRACK
> >
> >  config NF_CT_ACCT
> >  	bool "Connection tracking flow accounting"
>
> That defeats the only purpose why we kept it.

I'm not sure how this would defeat the only purpose.  Isn't the purpose of 
this to alias NF_CONNTRACK_ENABLED to NF_CONNTRACK?  And as such would yield 
the same result.

Also, we could leave this as is, and select NF_CONNTRACK_ENABLED instead of 
NF_CONNTRACK.


Thanks!

--
Al

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Patrick McHardy]

Al Boldi wrote:
> Patrick McHardy wrote:
> 
>>Al Boldi wrote:
>>
>>>Patrick McHardy wrote:
>>>
>>>>But I vaguely recall having tried this myself and it broke somewhere,
>>>>maybe it was because of the NF_CONNTRACK_ENABLED option, I can't
>>>>recall anymore. Al, if this also works without removal of
>>>>NF_CONNTRACK_ENABLED, please resend without that part.
>>>
>>>It doesn't.  But how about this, if you really can't live without
>>>NF_CONNTRACK_ENBLED:
>>>
>>>==================
>>>--- Kconfig.old	2007-07-09 06:38:52.000000000 +0300
>>>+++ Kconfig	2007-07-24 20:24:27.000000000 +0300
>>>@@ -25,8 +25,7 @@ config NETFILTER_NETLINK_LOG
>>> 	  and is also scheduled to replace the old syslog-based ipt_LOG
>>> 	  and ip6t_LOG modules.
>>>
>>>-# Rename this to NF_CONNTRACK in a 2.6.25
>>>-config NF_CONNTRACK_ENABLED
>>>+config NF_CONNTRACK
>>> 	tristate "Netfilter connection tracking support"
>>> 	help
>>> 	  Connection tracking keeps a record of what packets have passed
>>>@@ -40,9 +39,9 @@ config NF_CONNTRACK_ENABLED
>>>
>>> 	  To compile it as a module, choose M here.  If unsure, say N.
>>>
>>>-config NF_CONNTRACK
>>>+config NF_CONNTRACK_ENABLED
>>> 	tristate
>>>-	default NF_CONNTRACK_ENABLED
>>>+	default NF_CONNTRACK
>>>
>>> config NF_CT_ACCT
>>> 	bool "Connection tracking flow accounting"
>>
>>That defeats the only purpose why we kept it.
> 
> 
> I'm not sure how this would defeat the only purpose.  Isn't the purpose of 
> this to alias NF_CONNTRACK_ENABLED to NF_CONNTRACK?  And as such would yield 
> the same result.


The purpose is to avoid forcing people a second time to reconfigure
the conntrack options since we've completed nf_conntrack and removed
ip_conntrack. Previously NF_CONNTRACK was a bool (selecting the new
implementation) and NF_CONNTRACK_ENABLED specified whether to build
either nf_conntrack or ip_conntrack modular/static/not at all. So
old configs only have the information whether to build modular in
NF_CONNTRACK_ENABLED, but NF_CONNTRACK is what actually controls it.
With your change, old configs will still build nf_conntrack properly,
but they will always choose static linking.

> Also, we could leave this as is, and select NF_CONNTRACK_ENABLED instead of 
> NF_CONNTRACK.

I guess so, and that would have to select NF_CONNTRACK.

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Al Boldi]

Patrick McHardy wrote:
> Al Boldi wrote:
> > Also, we could leave this as is, and select NF_CONNTRACK_ENABLED instead
> > of NF_CONNTRACK.
>
> I guess so, and that would have to select NF_CONNTRACK.

Should I resend, or can you take care of it?


Thanks!

--
Al

Re: [PATCH] Netfilter Kconfig: Expose IPv4/6 connection tracking options by selecting NF_CONNTRACK

[Patrick McHardy]

Al Boldi wrote:
> Patrick McHardy wrote:
> 
>>Al Boldi wrote:
>>
>>>Also, we could leave this as is, and select NF_CONNTRACK_ENABLED instead
>>>of NF_CONNTRACK.
>>
>>I guess so, and that would have to select NF_CONNTRACK.
> 
> 
> Should I resend, or can you take care of it?


Please resend after testing.