From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Bernhard Thaler <bernhard.thaler@wvnet.at>
Cc: kadlec@blackhole.kfki.hu, netfilter-devel@vger.kernel.org
Subject: Re: [PATCHv3 2/2 nf] netfilter: bridge: fix IPv6 packets not being bridged with CONFIG_IPV6=n
Date: Thu, 6 Aug 2015 12:39:11 +0200 [thread overview]
Message-ID: <20150806103911.GA18793@salvia> (raw)
In-Reply-To: <1438229221-25959-1-git-send-email-bernhard.thaler@wvnet.at>
Hi Bernhard,
On Thu, Jul 30, 2015 at 06:07:01AM +0200, Bernhard Thaler wrote:
> 230ac490f7fba introduced a dependency to CONFIG_IPV6 which breaks bridging
> of IPv6 packets on a bridge with CONFIG_IPV6=n. This is due to the default
> value 1 for sysctl entry /proc/sys/net/bridge/bridge-nf-call-ip6tables,
> manually setting it to 0 makes IPv6 packets traverse bridge again.
>
> Default /proc/sys/net/bridge/bridge-nf-call-ip6tables to 0 if
> CONFIG_IP6_NF_IPTABLES is not enabled as CONFIG_IP6_NF_IPTABLES depends on
> CONFIG_IPV6 as well and is needed for ip6tables to work correctly.
>
> Do not expose sysctl entry /proc/sys/net/bridge/bridge-nf-call-ip6tables
> and sysfs entry /sys/class/net/brXXX/bridge/nf_call_ip6tables if
> CONFIG_IP6_NF_IPTABLES is not enabled.
>
> Make br_netfilter_ipv6.o dependent on CONFIG_IP6_NF_IPTABLES instead of
> CONFIG_IPV6.
Could you send a patch in first place to
s/CONFIG_IPV6/CONFIG_IP6_NF_IPTABLES ?
More comments below.
> Set global brnf_call_* variables to defaults in br_nf_proc_register() and
> br_nf_ipv6_proc_register() at module init instead of at variable definition
> to avoid further #ifdef CONFIG_SYSCTL constructs.
>
> Define br_nf_ipv6_proc_register() and br_nf_ipv6_proc_unregister() to avoid
>
> Tested with a simple bridge with two interfaces and IPv6 packets trying
> to pass from host on left side to host on right side of the bridge.
>
> Fixes: 230ac490f7fba ("netfilter: bridge: split ipv6 code into separated file")
>
> Signed-off-by: Bernhard Thaler <bernhard.thaler@wvnet.at>
> ---
> ---
> NOTE:
> * dependency to CONFIG_IPV6 instead of CONFIG_IP6_NF_IPTABLES would be more
> "conservative" approach as br_netfilter_ipv6.o was introduced due to
> dependencies in br_validate_ipv6() to CONFIG_IPV6; but CONFIG_IP6_NF_IPTABLES
> will be needed for ip6tables so this dependency may be more "holistic"
> * CONFIG_IP6_NF_IPTABLES=n makes br_validate_ipv6() being imported from
> br_netfilter.h; it is used in br_netfilter_hooks.c within br_nf_forward_ip()
> and br_nf_dev_queue_xmit() and returns -1 which will lead to NF_DROP;
> After defaulting /proc/sys/net/bridge/bridge-nf-call-ip6tables to 0 with
> CONFIG_IP6_NF_IPTABLES=n these two functions me never see IPV6 packets and
> therefore this may not be a problem; I was not able to fully confirm this
>
> Patch history
>
> v3
> * fix checkpatch error in separate patch
> * changes to reduce #ifdef pollution
>
> v2
> * do not expose sysfs and sysctl if CONFIG_IP6_NF_IPTABLES=n
> * change dependency to CONFIG_IP6_NF_IPTABLES as suggested by Florian Westphal
> * removed changes to br_validate_ipv6() in br_netfilter.h as test show it may
> not be needed
>
> v1
> * sysfs and sysctl entry were exposed but not writeable if CONFIG_IPV6=n
> include/net/netfilter/br_netfilter.h | 21 +++++++++-
> net/bridge/Makefile | 2 +-
> net/bridge/br_netfilter_hooks.c | 70 ++++++++++++++++++++--------------
> net/bridge/br_netfilter_ipv6.c | 44 +++++++++++++++++++++
> net/bridge/br_sysfs_br.c | 4 ++
> 5 files changed, 111 insertions(+), 30 deletions(-)
>
> diff --git a/include/net/netfilter/br_netfilter.h b/include/net/netfilter/br_netfilter.h
> index bab824b..e2cb715 100644
> --- a/include/net/netfilter/br_netfilter.h
> +++ b/include/net/netfilter/br_netfilter.h
> @@ -44,11 +44,17 @@ static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
> struct net_device *setup_pre_routing(struct sk_buff *skb);
> void br_netfilter_enable(void);
>
> -#if IS_ENABLED(CONFIG_IPV6)
> +#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
> int br_validate_ipv6(struct sk_buff *skb);
> unsigned int br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops,
> struct sk_buff *skb,
> const struct nf_hook_state *state);
> +#ifdef CONFIG_SYSCTL
> +int brnf_sysctl_call_tables(struct ctl_table *ctl, int write,
> + void __user *buffer, size_t *lenp, loff_t *ppos);
> +int br_nf_ipv6_proc_register(void);
> +void br_nf_ipv6_proc_unregister(void);
> +#endif
> #else
> static inline int br_validate_ipv6(struct sk_buff *skb)
> {
> @@ -61,6 +67,19 @@ br_nf_pre_routing_ipv6(const struct nf_hook_ops *ops, struct sk_buff *skb,
> {
> return NF_DROP;
> }
> +
> +#ifdef CONFIG_SYSCTL
> +static inline int br_nf_ipv6_proc_register(void)
> +{
> + return 0;
> +}
> +
> +static inline void br_nf_ipv6_proc_unregister(void)
> +{
> +}
> #endif
> +#endif
> +
> +extern int brnf_call_ip6tables __read_mostly;
>
> #endif /* _BR_NETFILTER_H_ */
> diff --git a/net/bridge/Makefile b/net/bridge/Makefile
> index a1cda5d..3fd8beb 100644
> --- a/net/bridge/Makefile
> +++ b/net/bridge/Makefile
> @@ -13,7 +13,7 @@ bridge-$(CONFIG_SYSFS) += br_sysfs_if.o br_sysfs_br.o
> bridge-$(subst m,y,$(CONFIG_BRIDGE_NETFILTER)) += br_nf_core.o
>
> br_netfilter-y := br_netfilter_hooks.o
> -br_netfilter-$(subst m,y,$(CONFIG_IPV6)) += br_netfilter_ipv6.o
> +br_netfilter-$(subst m,y,$(CONFIG_IP6_NF_IPTABLES)) += br_netfilter_ipv6.o
> obj-$(CONFIG_BRIDGE_NETFILTER) += br_netfilter.o
>
> bridge-$(CONFIG_BRIDGE_IGMP_SNOOPING) += br_multicast.o br_mdb.o
> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index 624e1f2..b202cfe 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c
> @@ -42,24 +42,16 @@
> #include "br_private.h"
> #ifdef CONFIG_SYSCTL
> #include <linux/sysctl.h>
> -#endif
>
> -#ifdef CONFIG_SYSCTL
> static struct ctl_table_header *brnf_sysctl_header;
> -static int brnf_call_iptables __read_mostly = 1;
> -static int brnf_call_ip6tables __read_mostly = 1;
> -static int brnf_call_arptables __read_mostly = 1;
> +#endif
> +
> +static int brnf_call_iptables __read_mostly;
> +int brnf_call_ip6tables __read_mostly;
You can move brnf_call_ip6tables to br_netfilter_ipv6, then add to the
header file this:
#if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
extern int brnf_call_ip6tables;
static inline bool brnf_call_ip6tables_is_set(struct net_bridge *br)
{
return !brnf_call_ip6tables && !br->nf_call_ip6tables;
}
#else
static inline bool brnf_call_ip6tables_is_set(struct net_bridge *br)
{
return false;
}
#endif
from the br netfilter ipv6 header file.
next prev parent reply other threads:[~2015-08-06 10:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-30 4:07 [PATCHv3 2/2 nf] netfilter: bridge: fix IPv6 packets not being bridged with CONFIG_IPV6=n Bernhard Thaler
2015-08-06 10:39 ` Pablo Neira Ayuso [this message]
2015-08-09 22:50 ` Bernhard Thaler
2015-08-12 17:57 ` Pablo Neira Ayuso
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150806103911.GA18793@salvia \
--to=pablo@netfilter.org \
--cc=bernhard.thaler@wvnet.at \
--cc=kadlec@blackhole.kfki.hu \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).