On 16.07.2015 13:17, Pablo Neira Ayuso wrote: > On Thu, Jul 16, 2015 at 02:34:44AM +0200, Bernhard Thaler wrote: > [...] >> * checkpatch.pl throws error "ERROR: do not initialise statics to 0 or NULL" >> but left for consistency with similar declarations > > I'd appreciate if you can address this in first place, so we don't have > to undo what we've just done for the ip6tables LOCs. > OK, will do so. >> * 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" > > I'm fine if you want to restrict this to ip6tables. This may break > out-of-tree modules since they may be relying on these hooks, but we > don't care about those. > Wasn't aware of that. Don't really want to break functionality for anybody...but if you are fine with the change as it is I leave it as it is. > I think this s/CONFIG_IPV6/CONFIG_IP6_NF_IPTABLES should come in a > separated patch, just after the one that removes the checkpatch > errors I'd suggest. > OK will do so. >> * 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 > > It would be great if you can reduce #ifdef pollution. Now that we have > a br_netfilter_ipv6.c file, I think it should be possible to move the > IPv6 specific code there, at least the /proc code chunk. > > Could you have a look into this? Thanks! > I don't like the #ifdef pollution as well. But I do not find a good solution to avoid it for brnf_call_ip6tables definition. I had a look into putting the /proc entry code into br_netfilter_ipv6.c but I may need some pointers/help how to do it: - br_netfilter_init() calls register_net_sysctl() with brnf_table[] as argument to setup the sysctl entries - brnf_table[] is initialized with static number of elements in br_netfilter_hooks.c, with the current code I see no way how to set this single ip6tables array element in br_netfilter_ipv6.c Currently I see not other solution than using the #ifdef in the definition of ip6tables sysctl entry in brnf_table[]. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
