Now that the conntrack module contains code for ipv6, we can no longer
have it built-in while IPv6 itself is a loadable module:
net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
nf_conntrack_proto.c:(.text+0x88c): undefined reference to `nf_defrag_ipv6_enable'
net/netfilter/nf_conntrack_proto.o:(.rodata+0x178): undefined reference to `nf_conntrack_l4proto_icmpv6'
This adds a dependency on IPv6 that makes it possible to still build
the conntrack module with IPv6 disabled, but avoids the broken configuration.
Fixes: 66c524acfb51 ("netfilter: conntrack: remove l3proto abstraction")
Signed-off-by: Arnd Bergmann <[email protected]>
---
net/netfilter/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 74df382bf2ba..e42c38c99741 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -49,6 +49,7 @@ config NETFILTER_NETLINK_LOG
config NF_CONNTRACK
tristate "Netfilter connection tracking support"
default m if NETFILTER_ADVANCED=n
+ depends on IPV6 || !IPV6
select NF_DEFRAG_IPV4
select NF_DEFRAG_IPV6 if IPV6
help
--
2.9.0
Arnd Bergmann <[email protected]> wrote:
> Now that the conntrack module contains code for ipv6, we can no longer
> have it built-in while IPv6 itself is a loadable module:
>
> net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
> nf_conntrack_proto.c:(.text+0x88c): undefined reference to `nf_defrag_ipv6_enable'
AFAICS this is caused by
CONFIG_NF_CONNTRACK=y
CONFIG_IPV6=m
CONFIG_NF_DEFRAG_IPV6=m
This is exported via nf_defrag_ipv6.ko.
nf_defrag_ipv6 has an ipv6 dependency, but i think it might be avoidable
so this would work:
CONFIG_NF_CONNTRACK=y
CONFIG_NF_DEFRAG_IPV6=y
CONFIG_IPV6=m
On Fri, Jul 6, 2018 at 3:55 PM, Florian Westphal <[email protected]> wrote:
> Arnd Bergmann <[email protected]> wrote:
>> Now that the conntrack module contains code for ipv6, we can no longer
>> have it built-in while IPv6 itself is a loadable module:
>>
>> net/netfilter/nf_conntrack_proto.o: In function `nf_ct_netns_do_get':
>> nf_conntrack_proto.c:(.text+0x88c): undefined reference to `nf_defrag_ipv6_enable'
>
> AFAICS this is caused by
>
> CONFIG_NF_CONNTRACK=y
> CONFIG_IPV6=m
> CONFIG_NF_DEFRAG_IPV6=m
>
> This is exported via nf_defrag_ipv6.ko.
>
> nf_defrag_ipv6 has an ipv6 dependency, but i think it might be avoidable
> so this would work:
>
> CONFIG_NF_CONNTRACK=y
> CONFIG_NF_DEFRAG_IPV6=y
> CONFIG_IPV6=m
I've tried it like this now:
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 07516d5c2f80..18b9f8f37c97 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -5,10 +5,6 @@
menu "IPv6: Netfilter Configuration"
depends on INET && IPV6 && NETFILTER
-config NF_DEFRAG_IPV6
- tristate
- default n
-
config NF_SOCKET_IPV6
tristate "IPv6 socket lookup support"
help
@@ -352,3 +348,6 @@ endif # IP6_NF_IPTABLES
endmenu
+config NF_DEFRAG_IPV6
+ tristate
+ default n
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index e42c38c99741..51be519a3802 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -49,9 +49,8 @@ config NETFILTER_NETLINK_LOG
config NF_CONNTRACK
tristate "Netfilter connection tracking support"
default m if NETFILTER_ADVANCED=n
- depends on IPV6 || !IPV6
select NF_DEFRAG_IPV4
- select NF_DEFRAG_IPV6 if IPV6
+ select NF_DEFRAG_IPV6 if IPV6 != n
help
Connection tracking keeps a record of what packets have passed
through your machine, in order to figure out how they are related
and that resulted in a new build failure:
net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
reference to `nf_conntrack_l4proto_icmpv6'
net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
`ip6_expire_frag_queue'
net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
`ip6_expire_frag_queue'
I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
Arnd
Arnd Bergmann <[email protected]> wrote:
> and that resulted in a new build failure:
>
> net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
> reference to `nf_conntrack_l4proto_icmpv6'
> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
> `ip6_expire_frag_queue'
> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
> nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
> nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
> nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
> `ip6_expire_frag_queue'
>
> I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
Yes, not with current implementation, but I still don't think this
is unavoidable.
In case this is urgent I'm fine with the patch that adds the dependency,
otherwise I'd like to try and disentangle nf_conntrack_reasm and ipv6.
On Fri, Jul 6, 2018 at 5:00 PM, Florian Westphal <[email protected]> wrote:
> Arnd Bergmann <[email protected]> wrote:
>> and that resulted in a new build failure:
>>
>> net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
>> reference to `nf_conntrack_l4proto_icmpv6'
>> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
>> nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
>> `ip6_expire_frag_queue'
>> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
>> nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
>> nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
>> nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
>> net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
>> nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
>> `ip6_expire_frag_queue'
>>
>> I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
>
> Yes, not with current implementation, but I still don't think this
> is unavoidable.
>
> In case this is urgent I'm fine with the patch that adds the dependency,
> otherwise I'd like to try and disentangle nf_conntrack_reasm and ipv6.
I don't think any real users depend on it, but I hit the problem quite
frequently in randconfig testing on linux-next.
Arnd
Florian Westphal <[email protected]> wrote:
> Arnd Bergmann <[email protected]> wrote:
> > and that resulted in a new build failure:
> >
> > net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
> > reference to `nf_conntrack_l4proto_icmpv6'
> > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
> > `ip6_expire_frag_queue'
> > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
> > nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
> > nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
> > nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
> > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
> > `ip6_expire_frag_queue'
> >
> > I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
>
> Yes, not with current implementation, but I still don't think this
> is unavoidable.
>
> In case this is urgent I'm fine with the patch that adds the dependency,
> otherwise I'd like to try and disentangle nf_conntrack_reasm and ipv6.
This links fine for me with IPV6=m:
Pablo, do you think this is too ugly?
It requires some copypastry from ipv6 defrag into nfct ipv6 defrag
to avoid the link errors outlined above.
Rest of defrag uses generic inet_defrag routines.
diff --git a/net/ipv6/netfilter/Kconfig b/net/ipv6/netfilter/Kconfig
index 07516d5c2f80..339d0762b027 100644
--- a/net/ipv6/netfilter/Kconfig
+++ b/net/ipv6/netfilter/Kconfig
@@ -5,10 +5,6 @@
menu "IPv6: Netfilter Configuration"
depends on INET && IPV6 && NETFILTER
-config NF_DEFRAG_IPV6
- tristate
- default n
-
config NF_SOCKET_IPV6
tristate "IPv6 socket lookup support"
help
@@ -349,6 +345,7 @@ config IP6_NF_TARGET_NPT
endif # IP6_NF_NAT
endif # IP6_NF_IPTABLES
-
endmenu
+config NF_DEFRAG_IPV6
+ tristate
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 5e0332014c17..9973aadc6b71 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -142,6 +142,50 @@ static inline u8 ip6_frag_ecn(const struct ipv6hdr *ipv6h)
return 1 << (ipv6_get_dsfield(ipv6h) & INET_ECN_MASK);
}
+static void nfct_expire_frag_queue(struct net *net, struct frag_queue *fq)
+{
+ struct net_device *dev = NULL;
+ struct sk_buff *head;
+
+ rcu_read_lock();
+ spin_lock(&fq->q.lock);
+
+ if (fq->q.flags & INET_FRAG_COMPLETE)
+ goto out;
+
+ inet_frag_kill(&fq->q);
+
+ dev = dev_get_by_index_rcu(net, fq->iif);
+ if (!dev)
+ goto out;
+
+ __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
+ __IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMTIMEOUT);
+
+ /* Don't send error if the first segment did not arrive. */
+ head = fq->q.fragments;
+ if (!(fq->q.flags & INET_FRAG_FIRST_IN) || !head)
+ goto out;
+
+ /* But use as source device on which LAST ARRIVED
+ * segment was received. And do not use fq->dev
+ * pointer directly, device might already disappeared.
+ */
+ head->dev = dev;
+ skb_get(head);
+ spin_unlock(&fq->q.lock);
+
+ icmpv6_send(head, ICMPV6_TIME_EXCEED, ICMPV6_EXC_FRAGTIME, 0);
+ kfree_skb(head);
+ goto out_rcu_unlock;
+
+out:
+ spin_unlock(&fq->q.lock);
+out_rcu_unlock:
+ rcu_read_unlock();
+ inet_frag_put(&fq->q);
+}
+
static void nf_ct_frag6_expire(struct timer_list *t)
{
struct inet_frag_queue *frag = from_timer(frag, t, timer);
@@ -151,7 +195,7 @@ static void nf_ct_frag6_expire(struct timer_list *t)
fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, nf_frag.frags);
- ip6_expire_frag_queue(net, fq);
+ nfct_expire_frag_queue(net, fq);
}
/* Creation primitives. */
@@ -622,16 +666,55 @@ static struct pernet_operations nf_ct_net_ops = {
.exit = nf_ct_net_exit,
};
+static u32 ip6_key_hashfn(const void *data, u32 len, u32 seed)
+{
+ return jhash2(data,
+ sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
+}
+
+static u32 ip6_obj_hashfn(const void *data, u32 len, u32 seed)
+{
+ const struct inet_frag_queue *fq = data;
+
+ return jhash2((const u32 *)&fq->key.v6,
+ sizeof(struct frag_v6_compare_key) / sizeof(u32), seed);
+}
+
+static int ip6_obj_cmpfn(struct rhashtable_compare_arg *arg, const void *ptr)
+{
+ const struct frag_v6_compare_key *key = arg->key;
+ const struct inet_frag_queue *fq = ptr;
+
+ return !!memcmp(&fq->key, key, sizeof(*key));
+}
+
+static const struct rhashtable_params nfct_rhash_params = {
+ .head_offset = offsetof(struct inet_frag_queue, node),
+ .hashfn = ip6_key_hashfn,
+ .obj_hashfn = ip6_obj_hashfn,
+ .obj_cmpfn = ip6_obj_cmpfn,
+ .automatic_shrinking = true,
+};
+
+static void nfct_frag_init(struct inet_frag_queue *q, const void *a)
+{
+ struct frag_queue *fq = container_of(q, struct frag_queue, q);
+ const struct frag_v6_compare_key *key = a;
+
+ q->key.v6 = *key;
+ fq->ecn = 0;
+}
+
int nf_ct_frag6_init(void)
{
int ret = 0;
- nf_frags.constructor = ip6_frag_init;
+ nf_frags.constructor = nfct_frag_init;
nf_frags.destructor = NULL;
nf_frags.qsize = sizeof(struct frag_queue);
nf_frags.frag_expire = nf_ct_frag6_expire;
nf_frags.frags_cache_name = nf_frags_cache_name;
- nf_frags.rhash_params = ip6_rhash_params;
+ nf_frags.rhash_params = nfct_rhash_params;
ret = inet_frags_init(&nf_frags);
if (ret)
goto out;
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 6c65d756e603..e0ab50c58dc4 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -50,7 +50,7 @@ config NF_CONNTRACK
tristate "Netfilter connection tracking support"
default m if NETFILTER_ADVANCED=n
select NF_DEFRAG_IPV4
- select NF_DEFRAG_IPV6 if IPV6
+ select NF_DEFRAG_IPV6 if IPV6 != n
help
Connection tracking keeps a record of what packets have passed
through your machine, in order to figure out how they are related
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 0b3851e825fa..53bd1ed1228a 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -6,7 +6,7 @@ nf_conntrack-y := nf_conntrack_core.o nf_conntrack_standalone.o nf_conntrack_exp
nf_conntrack_proto_icmp.o \
nf_conntrack_extend.o nf_conntrack_acct.o nf_conntrack_seqadj.o
-nf_conntrack-$(CONFIG_IPV6) += nf_conntrack_proto_icmpv6.o
+nf_conntrack-$(subst m,y,$(CONFIG_IPV6)) += nf_conntrack_proto_icmpv6.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMEOUT) += nf_conntrack_timeout.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_TIMESTAMP) += nf_conntrack_timestamp.o
nf_conntrack-$(CONFIG_NF_CONNTRACK_EVENTS) += nf_conntrack_ecache.o
On Fri, Jul 06, 2018 at 08:35:48PM +0200, Florian Westphal wrote:
> Florian Westphal <[email protected]> wrote:
> > Arnd Bergmann <[email protected]> wrote:
> > > and that resulted in a new build failure:
> > >
> > > net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
> > > reference to `nf_conntrack_l4proto_icmpv6'
> > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > > nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
> > > `ip6_expire_frag_queue'
> > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
> > > nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
> > > nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
> > > nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
> > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > > nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
> > > `ip6_expire_frag_queue'
> > >
> > > I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
> >
> > Yes, not with current implementation, but I still don't think this
> > is unavoidable.
> >
> > In case this is urgent I'm fine with the patch that adds the dependency,
> > otherwise I'd like to try and disentangle nf_conntrack_reasm and ipv6.
>
> This links fine for me with IPV6=m:
>
> Pablo, do you think this is too ugly?
Well, yes...
> It requires some copypastry from ipv6 defrag into nfct ipv6 defrag
> to avoid the link errors outlined above.
This is a bit of a regression I think.
While I think a bit of demodularization is fine, if things like this
start to kick in, it's probably good if we go make a step back...
Let me know, thanks.
Pablo Neira Ayuso <[email protected]> wrote:
> On Fri, Jul 06, 2018 at 08:35:48PM +0200, Florian Westphal wrote:
> > Florian Westphal <[email protected]> wrote:
> > > Arnd Bergmann <[email protected]> wrote:
> > > > and that resulted in a new build failure:
> > > >
> > > > net/netfilter/nf_conntrack_proto.o:(.rodata+0x788): undefined
> > > > reference to `nf_conntrack_l4proto_icmpv6'
> > > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > > > nf_conntrack_reasm.c:(.text+0x2320): undefined reference to
> > > > `ip6_expire_frag_queue'
> > > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_init':
> > > > nf_conntrack_reasm.c:(.text+0x2384): undefined reference to `ip6_frag_init'
> > > > nf_conntrack_reasm.c:(.text+0x2394): undefined reference to `ip6_frag_init'
> > > > nf_conntrack_reasm.c:(.text+0x2398): undefined reference to `ip6_rhash_params'
> > > > net/ipv6/netfilter/nf_conntrack_reasm.o: In function `nf_ct_frag6_expire':
> > > > nf_conntrack_reasm.c:(.text+0x10bc): undefined reference to
> > > > `ip6_expire_frag_queue'
> > > >
> > > > I don't think we can get CONFIG_NF_DEFRAG_IPV6=y to work with IPV6=m.
> > >
> > > Yes, not with current implementation, but I still don't think this
> > > is unavoidable.
> > >
> > > In case this is urgent I'm fine with the patch that adds the dependency,
> > > otherwise I'd like to try and disentangle nf_conntrack_reasm and ipv6.
> >
> > This links fine for me with IPV6=m:
> >
> > Pablo, do you think this is too ugly?
>
> Well, yes...
>
> > It requires some copypastry from ipv6 defrag into nfct ipv6 defrag
> > to avoid the link errors outlined above.
>
> This is a bit of a regression I think.
nf_defrag_ipv6 has always depended on ipv6.
Previously dependency chain is
nf_conntrack_ipv6 -> ipv6
+-----> nf_defrag_ipv6 > ipv6
+-----> nf_conntrack
new dependency chain is:
nf_conntrack --> nf_defrag_ipv6 --> ipv6
The alternate fix is the one from Arnd to disallow
NF_CONNTRACK=y
IPV6=m
> While I think a bit of demodularization is fine, if things like this
> start to kick in, it's probably good if we go make a step back...
If you want to revert, ok. It will prevent all l4 unification patches
plus any attempt to get rid of the nf_hook for defrag too however so I
do not like it.
> Let me know, thanks.
Yet another alternative would be to remove the
nf_conntrack -> nf_defrag_ipv6
dependency, e.g. by adding a new rcu-protected function pointer.
nf_conntrack, when ipv6 conntrack is requested, would check if its null,
and, if so, issue request_module() for ipv6 defrag before failing the
request.
However, I find it even more ugly than my patch.