| Message ID | 20180706130005.3640993-1-arnd@arndb.de |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: conntrack: add weak IPV6 dependency | expand |
Arnd Bergmann <arnd@arndb.de> 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 6, 2018 at 3:55 PM, Florian Westphal <fw@strlen.de> wrote: > Arnd Bergmann <arnd@arndb.de> 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Arnd Bergmann <arnd@arndb.de> 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 6, 2018 at 5:00 PM, Florian Westphal <fw@strlen.de> wrote: > Arnd Bergmann <arnd@arndb.de> 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Florian Westphal <fw@strlen.de> wrote: > Arnd Bergmann <arnd@arndb.de> 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 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 06, 2018 at 08:35:48PM +0200, Florian Westphal wrote: > Florian Westphal <fw@strlen.de> wrote: > > Arnd Bergmann <arnd@arndb.de> 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Fri, Jul 06, 2018 at 08:35:48PM +0200, Florian Westphal wrote: > > Florian Westphal <fw@strlen.de> wrote: > > > Arnd Bergmann <arnd@arndb.de> 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 <arnd@arndb.de> --- net/netfilter/Kconfig | 1 + 1 file changed, 1 insertion(+)