[PATCH] ila: add NETFILTER dependency

[PATCH] ila: add NETFILTER dependency

[Arnd Bergmann]

The recently added generic ILA translation facility fails to
build when CONFIG_NETFILTER is disabled:

net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
 static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {

This adds an explicit Kconfig dependency to avoid that case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")

diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 983bb999738c..bb7dabe2ebbf 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -94,6 +94,7 @@ config IPV6_MIP6
 
 config IPV6_ILA
 	tristate "IPv6: Identifier Locator Addressing (ILA)"
+	depends on NETFILTER
 	select LWTUNNEL
 	---help---
 	  Support for IPv6 Identifier Locator Addressing (ILA).

[PATCH] ila: add NETFILTER dependency

[Arnd Bergmann]

The recently added generic ILA translation facility fails to
build when CONFIG_NETFILTER is disabled:

net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
 static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {

This adds an explicit Kconfig dependency to avoid that case.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")

diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 983bb999738c..bb7dabe2ebbf 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -94,6 +94,7 @@ config IPV6_MIP6
 
 config IPV6_ILA
 	tristate "IPv6: Identifier Locator Addressing (ILA)"
+	depends on NETFILTER
 	select LWTUNNEL
 	---help---
 	  Support for IPv6 Identifier Locator Addressing (ILA).

Re: [PATCH] ila: add NETFILTER dependency

[Pablo Neira Ayuso]

On Fri, Dec 18, 2015 at 03:37:37PM +0100, Arnd Bergmann wrote:
> The recently added generic ILA translation facility fails to
> build when CONFIG_NETFILTER is disabled:
> 
> net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
>  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> 
> This adds an explicit Kconfig dependency to avoid that case.

I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
a symptom that there is something that doesn't belong there.

I overlook this new hook on priority -1, how does this integrate into
our infrastructure?

And mainly, isn't there any better way to integrate this into the
stack?

And why didn't you Cc netfilter-devel for code that involves
Netfilter?

We have to evaluate how this integrates into what we have, if it
breaks when it interacts with other components that we have.

I'm very sorry to say, but none of this has happened so far.

[PATCH] ila: add NETFILTER dependency

[Pablo Neira Ayuso]

On Fri, Dec 18, 2015 at 03:37:37PM +0100, Arnd Bergmann wrote:
> The recently added generic ILA translation facility fails to
> build when CONFIG_NETFILTER is disabled:
> 
> net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
>  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> 
> This adds an explicit Kconfig dependency to avoid that case.

I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
a symptom that there is something that doesn't belong there.

I overlook this new hook on priority -1, how does this integrate into
our infrastructure?

And mainly, isn't there any better way to integrate this into the
stack?

And why didn't you Cc netfilter-devel for code that involves
Netfilter?

We have to evaluate how this integrates into what we have, if it
breaks when it interacts with other components that we have.

I'm very sorry to say, but none of this has happened so far.

Re: [PATCH] ila: add NETFILTER dependency

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Dec 18, 2015 at 03:37:37PM +0100, Arnd Bergmann wrote:
> > The recently added generic ILA translation facility fails to
> > build when CONFIG_NETFILTER is disabled:
> > 
> > net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> > net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
> >  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> > 
> > This adds an explicit Kconfig dependency to avoid that case.
> 
> I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
> a symptom that there is something that doesn't belong there.
> 
> I overlook this new hook on priority -1, how does this integrate into
> our infrastructure?

Looks problematic since address changes post ipv6 dnat translations,
its certainly unexpected for nft since we have magic address mangling
after -2 and 0 priroized tables...

However ... how is ILA supposed to work?

ila_xlat_outgoing has no callers, so it appears we only do this
stateless nat on ingress...?

[PATCH] ila: add NETFILTER dependency

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Dec 18, 2015 at 03:37:37PM +0100, Arnd Bergmann wrote:
> > The recently added generic ILA translation facility fails to
> > build when CONFIG_NETFILTER is disabled:
> > 
> > net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> > net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
> >  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> > 
> > This adds an explicit Kconfig dependency to avoid that case.
> 
> I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
> a symptom that there is something that doesn't belong there.
> 
> I overlook this new hook on priority -1, how does this integrate into
> our infrastructure?

Looks problematic since address changes post ipv6 dnat translations,
its certainly unexpected for nft since we have magic address mangling
after -2 and 0 priroized tables...

However ... how is ILA supposed to work?

ila_xlat_outgoing has no callers, so it appears we only do this
stateless nat on ingress...?

Re: [PATCH] ila: add NETFILTER dependency

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 18 Dec 2015 15:37:37 +0100

> The recently added generic ILA translation facility fails to
> build when CONFIG_NETFILTER is disabled:
> 
> net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
>  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> 
> This adds an explicit Kconfig dependency to avoid that case.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")

Applied, thanks.

[PATCH] ila: add NETFILTER dependency

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 18 Dec 2015 15:37:37 +0100

> The recently added generic ILA translation facility fails to
> build when CONFIG_NETFILTER is disabled:
> 
> net/ipv6/ila/ila_xlat.c:229:20: warning: 'struct nf_hook_state' declared inside parameter list
> net/ipv6/ila/ila_xlat.c:235:27: error: array type has incomplete element type 'struct nf_hook_ops'
>  static struct nf_hook_ops ila_nf_hook_ops[] __read_mostly = {
> 
> This adds an explicit Kconfig dependency to avoid that case.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 7f00feaf1076 ("ila: Add generic ILA translation facility")

Applied, thanks.

Re: [PATCH] ila: add NETFILTER dependency

[Pablo Neira Ayuso]

On Fri, Dec 18, 2015 at 07:09:31PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
> > a symptom that there is something that doesn't belong there.
> > 
> > I overlook this new hook on priority -1, how does this integrate into
> > our infrastructure?
> 
> Looks problematic since address changes post ipv6 dnat translations,
> its certainly unexpected for nft since we have magic address mangling
> after -2 and 0 priroized tables...

David indicated that this should be sort of transparent and integrated
into separated infrastructure.

The existing hook will break IPv6 conntrack and NAT for us, and the
extra hook is suboptimal as it

I'd suggest you add a static key and specific hook before netfilter to
deal with this.

[PATCH] ila: add NETFILTER dependency

[Pablo Neira Ayuso]

On Fri, Dec 18, 2015 at 07:09:31PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > I'm afraid this extra Kconfig dependency that Arnd adds to fix this is
> > a symptom that there is something that doesn't belong there.
> > 
> > I overlook this new hook on priority -1, how does this integrate into
> > our infrastructure?
> 
> Looks problematic since address changes post ipv6 dnat translations,
> its certainly unexpected for nft since we have magic address mangling
> after -2 and 0 priroized tables...

David indicated that this should be sort of transparent and integrated
into separated infrastructure.

The existing hook will break IPv6 conntrack and NAT for us, and the
extra hook is suboptimal as it

I'd suggest you add a static key and specific hook before netfilter to
deal with this.