Currently the sysctl of netfilter proto is not isolated, so when
changing proto's sysctl in container will cause the host's sysctl
be changed too. it's not expected.
This patch set adds the namespace support for netfilter protos.
impletement four pernet_operations to register sysctl and initial
pernet data for proto.
-ipv4_net_ops is used to register tcp4(compat),
udp4(compat),icmp(compat),ipv4(compat).
-ipv6_net_ops is used to register tcp6,udp6 and icmpv6.
-sctp_net_ops is used to register sctp4(compat) and sctp6.
-udplite_net_ops is used to register udplite4 and udplite6
extern l[3,4]proto (sysctl) register functions to make them support
namespace.
finailly add namespace support for cttimeout.
Changes from v2:
re-split patchset to make compilation success.
Gao feng (17):
netfilter: add struct nf_proto_net for register l4proto sysctl
netfilter: add namespace support for l4proto
netfilter: add namespace support for l3proto
netfilter: add namespace support for l4proto_generic
netfilter: add namespace support for l4proto_tcp
netfilter: add namespace support for l4proto_udp
netfilter: add namespace support for l4proto_icmp
netfilter: add namespace support for l4proto_icmpv6
netfilter: add namespace support for l3proto_ipv4
netfilter: add namespace support for l3proto_ipv6
netfilter: add namespace support for l4proto_sctp
netfilter: add namespace support for l4proto_udplite
netfilter: adjust l4proto_dccp to the nf_conntrack_l4proto_register
netfilter: adjust l4proto_gre4 to the nf_conntrack_l4proto_register
netfilter: cleanup sysctl for l4proto and l3proto
netfilter: add namespace support for cttimeout
netfilter: cttimeout use pernet data of l4proto
include/net/netfilter/nf_conntrack_l3proto.h | 11 +-
include/net/netfilter/nf_conntrack_l4proto.h | 32 ++-
include/net/netns/conntrack.h | 55 ++++
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 123 +++++---
net/ipv4/netfilter/nf_conntrack_proto_icmp.c | 53 +++-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 88 ++++--
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 37 ++-
net/netfilter/nf_conntrack_core.c | 8 +-
net/netfilter/nf_conntrack_proto.c | 385 ++++++++++++++---------
net/netfilter/nf_conntrack_proto_dccp.c | 140 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 69 ++++-
net/netfilter/nf_conntrack_proto_gre.c | 64 +++--
net/netfilter/nf_conntrack_proto_sctp.c | 156 +++++++---
net/netfilter/nf_conntrack_proto_tcp.c | 135 ++++++---
net/netfilter/nf_conntrack_proto_udp.c | 88 ++++--
net/netfilter/nf_conntrack_proto_udplite.c | 123 ++++++--
net/netfilter/nfnetlink_cttimeout.c | 13 +-
17 files changed, 1056 insertions(+), 524 deletions(-)
--
1.7.7.6
From: Gao feng <redacted>
the struct nf_proto_net stroes proto's ctl_table_header and ctl_table,
nf_ct_l4proto_(un)register_sysctl use it to register sysctl.
there are some changes for struct nf_conntrack_l4proto:
- add field compat to identify if this proto should do compat.
- the net_id field is used to store the pernet_operations id
that belones to l4proto.
- init_net will be used to initial the proto's pernet data
and add init_net for struct nf_conntrack_l3proto too.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l3proto.h | 3 +++
include/net/netfilter/nf_conntrack_l4proto.h | 6 ++++++
include/net/netns/conntrack.h | 12 ++++++++++++
3 files changed, 21 insertions(+), 0 deletions(-)
@@ -69,6 +69,9 @@ struct nf_conntrack_l3proto {structctl_table*ctl_table;#endif /* CONFIG_SYSCTL */+/* Init l3proto pernet data */+int(*init_net)(structnet*net);+/* Module (if any) which this is connected to. */structmodule*me;};
@@ -22,6 +22,8 @@ struct nf_conntrack_l4proto {/* L4 Protocol number. */u_int8_tl4proto;+u_int8_tcompat;+/* Try to fill in the third arg: dataoff is offset past network protocolhdr.Returntrueifpossible.*/bool(*pkt_to_tuple)(conststructsk_buff*skb,unsignedintdataoff,
@@ -103,6 +105,10 @@ struct nf_conntrack_l4proto {structctl_table*ctl_compat_table;#endif#endif+int*net_id;+/* Init l4proto pernet data */+int(*init_net)(structnet*net,u_int8_tcompat);+/* Protocol name */constchar*name;
-Add the struct net as param of nf_conntrack_l3proto_(un)register.
register or unregister the l3proto only when the net is init_net.
-The new struct nf_ip_net is used to store the sysctl header and data
of l3proto_ipv4,l4proto_tcp(6),l4proto_udp(6),l4proto_icmp(v6).
because the protos such tcp and tcp6 use the same data,so making
nf_ip_net as a field of netns_ct is the easiest way to manager it.
-nf_ct_l3proto_register_sysctl call init_net to initial the pernet data
of l3proto.
-nf_ct_l3proto_net is used to get the pernet data of l3proto.
-export nf_conntrack_l3proto_(un)register
-use init_net as param of nf_conntrack_l3proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l3proto.h | 6 +-
include/net/netns/conntrack.h | 8 ++
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 +-
net/netfilter/nf_conntrack_proto.c | 127 +++++++++++++++---------
5 files changed, 97 insertions(+), 56 deletions(-)
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
@@ -42,7 +47,7 @@ static int generic_print_tuple(struct seq_file *s,staticunsignedint*generic_get_timeouts(structnet*net){-return&nf_ct_generic_timeout;+return&(generic_pernet(net)->timeout);}/* Returns verdict for packet, or -1 for invalid. */
implement tcp_init_net to initial the pernet sysctl data
for tcp proto.
Because tcp_init_net is called by l4proto_tcp[4,6],so use
nf_proto_net.users to identify if the pernet data is initialized
when CONFIG_SYSCTL is not configured.
nf_tcp_net as a field of netns_ct,when proto is tcp,
return net->ct.proto.tcp in function nf_ct_l4proto_net.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netns/conntrack.h | 10 +++
net/netfilter/nf_conntrack_proto.c | 2 +
net/netfilter/nf_conntrack_proto_tcp.c | 114 ++++++++++++++++++++++++--------
3 files changed, 97 insertions(+), 29 deletions(-)
implement udp_init_net to initial the pernet sysctl data for
udp protos.
Because udp_init_net is called by l4proto_udp[4,6],so use
nf_proto_net.users to identify if the pernet data is initialized
when CONFIG_SYSCTL is not configured.
nf_udp_net as a field of netns_ct,when proto is udp,
return net->ct.proto.udp in function nf_ct_l4proto_net.
and move enum udp_conntrack to conntrack.h
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netns/conntrack.h | 12 ++++++
net/netfilter/nf_conntrack_proto.c | 2 +
net/netfilter/nf_conntrack_proto_udp.c | 65 ++++++++++++++++++++++++++-----
3 files changed, 68 insertions(+), 11 deletions(-)
@@ -73,7 +72,7 @@ static int udp_print_tuple(struct seq_file *s,staticunsignedint*udp_get_timeouts(structnet*net){-returnudp_timeouts;+returnudp_pernet(net)->timeouts;}/* Returns verdict for packet, and may modify conntracktype */
add pernet_operations ipv4_net_ops and register it when
module nf_conntrack_ipv4 is loaded.
move the l4proto_register and l3proto_register from module_init
function to ipv4_net_ops.init.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 122 ++++++++++++++++--------
1 files changed, 84 insertions(+), 38 deletions(-)
add pernet_operations sctp_net_ops and register it when
module nf_conntrack_proto_sctp is loaded.
move the l4proto_register from module_init function to
sctp_net_ops.init.
and implement sctp_init_net to initial the pernet sysctl
data for sctp[4,6] protos.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/netfilter/nf_conntrack_proto_sctp.c | 135 ++++++++++++++++++++++++-------
1 files changed, 106 insertions(+), 29 deletions(-)
@@ -281,7 +292,7 @@ static int sctp_new_state(enum ip_conntrack_dir dir,staticunsignedint*sctp_get_timeouts(structnet*net){-returnsctp_timeouts;+returnsctp_pernet(net)->timeouts;}/* Returns verdict for packet, or -NF_ACCEPT for invalid. */
From: Gao feng <redacted>
-nf_ct_(un)register_sysctl are changed to support net namespace,
use (un)register_net_sysctl_table replaces (un)register_sysctl_paths.
and in nf_ct_unregister_sysctl,kfree table only when users is 0.
-Add the struct net as param of nf_conntrack_l4proto_(un)register.
register or unregister the l4proto only when the net is init_net.
-nf_conntrack_l4proto_register call init_net to initial the pernet
data of l4proto.
-nf_ct_l4proto_net is used to get the pernet data of l4proto.
-use init_net as a param of nf_conntrack_l4proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 13 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 18 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 18 +-
net/netfilter/nf_conntrack_proto.c | 245 ++++++++++++++----------
net/netfilter/nf_conntrack_proto_dccp.c | 10 +-
net/netfilter/nf_conntrack_proto_gre.c | 6 +-
net/netfilter/nf_conntrack_proto_sctp.c | 10 +-
net/netfilter/nf_conntrack_proto_udplite.c | 10 +-
8 files changed, 191 insertions(+), 139 deletions(-)
@@ -243,137 +253,172 @@ void nf_conntrack_l3proto_unregister(struct nf_conntrack_l3proto *proto)}EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_unregister);-staticintnf_ct_l4proto_register_sysctl(structnf_conntrack_l4proto*l4proto)+staticstructnf_proto_net*nf_ct_l4proto_net(structnet*net,+structnf_conntrack_l4proto*l4proto){-interr=0;+if(l4proto->net_id)+returnnet_generic(net,*l4proto->net_id);+else+returnNULL;+}+intnf_ct_l4proto_register_sysctl(structnet*net,+structnf_conntrack_l4proto*l4proto)+{+interr=0;+structnf_proto_net*pn=nf_ct_l4proto_net(net,l4proto);+if(pn==NULL)+return0;#ifdef CONFIG_SYSCTL-if(l4proto->ctl_table!=NULL){-err=nf_ct_register_sysctl(l4proto->ctl_table_header,+if(pn->ctl_table!=NULL){+err=nf_ct_register_sysctl(net,+&pn->ctl_table_header,"net/netfilter",-l4proto->ctl_table,-l4proto->ctl_table_users);-if(err<0)+pn->ctl_table,+&pn->users);+if(err<0){+kfree(pn->ctl_table);+pn->ctl_table=NULL;gotoout;+}}#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT-if(l4proto->ctl_compat_table!=NULL){-err=nf_ct_register_sysctl(&l4proto->ctl_compat_table_header,+if(l4proto->compat&&pn->ctl_compat_table!=NULL){+err=nf_ct_register_sysctl(net,+&pn->ctl_compat_header,"net/ipv4/netfilter",-l4proto->ctl_compat_table,NULL);+pn->ctl_compat_table,+NULL);if(err==0)gotoout;-nf_ct_unregister_sysctl(l4proto->ctl_table_header,-l4proto->ctl_table,-l4proto->ctl_table_users);++kfree(pn->ctl_compat_table);+pn->ctl_compat_table=NULL;+nf_ct_unregister_sysctl(&pn->ctl_table_header,+&pn->ctl_table,+&pn->users);}#endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */out:#endif /* CONFIG_SYSCTL */returnerr;}+EXPORT_SYMBOL_GPL(nf_ct_l4proto_register_sysctl);-staticvoidnf_ct_l4proto_unregister_sysctl(structnf_conntrack_l4proto*l4proto)+voidnf_ct_l4proto_unregister_sysctl(structnet*net,+structnf_conntrack_l4proto*l4proto){+structnf_proto_net*pn=nf_ct_l4proto_net(net,l4proto);+if(pn==NULL)+return;#ifdef CONFIG_SYSCTL-if(l4proto->ctl_table_header!=NULL&&-*l4proto->ctl_table_header!=NULL)-nf_ct_unregister_sysctl(l4proto->ctl_table_header,-l4proto->ctl_table,-l4proto->ctl_table_users);+if(pn->ctl_table_header!=NULL)+nf_ct_unregister_sysctl(&pn->ctl_table_header,+&pn->ctl_table,+&pn->users);+#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT-if(l4proto->ctl_compat_table_header!=NULL)-nf_ct_unregister_sysctl(&l4proto->ctl_compat_table_header,-l4proto->ctl_compat_table,NULL);+if(l4proto->compat&&pn->ctl_compat_header!=NULL)+nf_ct_unregister_sysctl(&pn->ctl_compat_header,+&pn->ctl_compat_table,+NULL);#endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */+#else+pn->users--;#endif /* CONFIG_SYSCTL */}+EXPORT_SYMBOL_GPL(nf_ct_l4proto_unregister_sysctl);/* FIXME: Allow NULL functions and sub in pointers to generic forthem.--RR*/-intnf_conntrack_l4proto_register(structnf_conntrack_l4proto*l4proto)+intnf_conntrack_l4proto_register(structnet*net,+structnf_conntrack_l4proto*l4proto){intret=0;-if(l4proto->l3proto>=PF_MAX)-return-EBUSY;--if((l4proto->to_nlattr&&!l4proto->nlattr_size)-||(l4proto->tuple_to_nlattr&&!l4proto->nlattr_tuple_size))-return-EINVAL;--mutex_lock(&nf_ct_proto_mutex);-if(!nf_ct_protos[l4proto->l3proto]){-/* l3proto may be loaded latter. */-structnf_conntrack_l4proto__rcu**proto_array;-inti;--proto_array=kmalloc(MAX_NF_CT_PROTO*-sizeof(structnf_conntrack_l4proto*),-GFP_KERNEL);-if(proto_array==NULL){-ret=-ENOMEM;+if(net==&init_net){+if(l4proto->l3proto>=PF_MAX)+return-EBUSY;++if((l4proto->to_nlattr&&!l4proto->nlattr_size)+||(l4proto->tuple_to_nlattr&&!l4proto->nlattr_tuple_size))+return-EINVAL;++mutex_lock(&nf_ct_proto_mutex);+if(!nf_ct_protos[l4proto->l3proto]){+/* l3proto may be loaded latter. */+structnf_conntrack_l4proto__rcu**proto_array;+inti;++proto_array=kmalloc(MAX_NF_CT_PROTO*+sizeof(structnf_conntrack_l4proto*),+GFP_KERNEL);+if(proto_array==NULL){+ret=-ENOMEM;+gotoout_unlock;+}++for(i=0;i<MAX_NF_CT_PROTO;i++)+RCU_INIT_POINTER(proto_array[i],&nf_conntrack_l4proto_generic);++/* Before making proto_array visible to lockless readers,+*wemustmakesureitscontentiscommittedtomemory.+*/+smp_wmb();++nf_ct_protos[l4proto->l3proto]=proto_array;+}elseif(rcu_dereference_protected(+nf_ct_protos[l4proto->l3proto][l4proto->l4proto],+lockdep_is_held(&nf_ct_proto_mutex)+)!=&nf_conntrack_l4proto_generic){+ret=-EBUSY;gotoout_unlock;}-for(i=0;i<MAX_NF_CT_PROTO;i++)-RCU_INIT_POINTER(proto_array[i],&nf_conntrack_l4proto_generic);--/* Before making proto_array visible to lockless readers,-*wemustmakesureitscontentiscommittedtomemory.-*/-smp_wmb();--nf_ct_protos[l4proto->l3proto]=proto_array;-}elseif(rcu_dereference_protected(-nf_ct_protos[l4proto->l3proto][l4proto->l4proto],-lockdep_is_held(&nf_ct_proto_mutex)-)!=&nf_conntrack_l4proto_generic){-ret=-EBUSY;-gotoout_unlock;-}--ret=nf_ct_l4proto_register_sysctl(l4proto);-if(ret<0)-gotoout_unlock;--l4proto->nla_size=0;-if(l4proto->nlattr_size)-l4proto->nla_size+=l4proto->nlattr_size();-if(l4proto->nlattr_tuple_size)-l4proto->nla_size+=3*l4proto->nlattr_tuple_size();--rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],-l4proto);+l4proto->nla_size=0;+if(l4proto->nlattr_size)+l4proto->nla_size+=l4proto->nlattr_size();+if(l4proto->nlattr_tuple_size)+l4proto->nla_size+=3*l4proto->nlattr_tuple_size();+rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],+l4proto);out_unlock:-mutex_unlock(&nf_ct_proto_mutex);-returnret;+mutex_unlock(&nf_ct_proto_mutex);+if(ret<0)+returnret;+}+if(l4proto->init_net){+ret=l4proto->init_net(net,l4proto->compat);+if(ret<0)+returnret;+}+returnnf_ct_l4proto_register_sysctl(net,l4proto);}EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);-voidnf_conntrack_l4proto_unregister(structnf_conntrack_l4proto*l4proto)+voidnf_conntrack_l4proto_unregister(structnet*net,+structnf_conntrack_l4proto*l4proto){-structnet*net;--BUG_ON(l4proto->l3proto>=PF_MAX);--mutex_lock(&nf_ct_proto_mutex);-BUG_ON(rcu_dereference_protected(-nf_ct_protos[l4proto->l3proto][l4proto->l4proto],-lockdep_is_held(&nf_ct_proto_mutex)-)!=l4proto);-rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],-&nf_conntrack_l4proto_generic);-nf_ct_l4proto_unregister_sysctl(l4proto);-mutex_unlock(&nf_ct_proto_mutex);--synchronize_rcu();+if(net==&init_net){+BUG_ON(l4proto->l3proto>=PF_MAX);+mutex_lock(&nf_ct_proto_mutex);++BUG_ON(rcu_dereference_protected(+nf_ct_protos[l4proto->l3proto][l4proto->l4proto],+lockdep_is_held(&nf_ct_proto_mutex)+)!=l4proto);+rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],+&nf_conntrack_l4proto_generic);+mutex_unlock(&nf_ct_proto_mutex);++synchronize_rcu();+}+nf_ct_l4proto_unregister_sysctl(net,l4proto);/* Remove all contrack entries for this protocol */rtnl_lock();-for_each_net(net)-nf_ct_iterate_cleanup(net,kill_l4proto,l4proto);+nf_ct_iterate_cleanup(net,kill_l4proto,l4proto);rtnl_unlock();}EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_unregister);
@@ -383,7 +428,7 @@ int nf_conntrack_proto_init(void)unsignedinti;interr;-err=nf_ct_l4proto_register_sysctl(&nf_conntrack_l4proto_generic);+err=nf_ct_l4proto_register_sysctl(&init_net,&nf_conntrack_l4proto_generic);if(err<0)returnerr;
implement icmp_init_net is to initial the pernet data for
icmp proto.
beacuse nf_icmp_net is a field of netns_ct,so when proto is icmp,
return net->ct.proto.icmp in function nf_ct_l4proto_net.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netns/conntrack.h | 6 ++++
net/ipv4/netfilter/nf_conntrack_proto_icmp.c | 39 ++++++++++++++++++++++++--
net/netfilter/nf_conntrack_proto.c | 2 +
3 files changed, 44 insertions(+), 3 deletions(-)
@@ -77,7 +82,7 @@ static int icmp_print_tuple(struct seq_file *s,staticunsignedint*icmp_get_timeouts(structnet*net){-return&nf_ct_icmp_timeout;+return&icmp_pernet(net)->timeout;}/* Returns verdict for packet, or -1 for invalid. */
add pernet_operations ipv6_net_ops and register it when
module nf_conntrack_ipv6 is loaded.
move the l4proto_register and l3proto_register from module_init
function to ipv6_net_ops.init.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 88 ++++++++++++++++--------
1 files changed, 59 insertions(+), 29 deletions(-)
implement icmpv6_init_net is to initial the pernet data for
icmpv6 proto.
because nf_icmp_net is a field of netns_ct,so when proto is icmpv6,
return net->ct.proto.icmpv6 in function nf_ct_l4proto_net.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netns/conntrack.h | 1 +
net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 26 ++++++++++++++++++++++-
net/netfilter/nf_conntrack_proto.c | 2 +
3 files changed, 27 insertions(+), 2 deletions(-)
@@ -90,7 +95,7 @@ static int icmpv6_print_tuple(struct seq_file *s,staticunsignedint*icmpv6_get_timeouts(structnet*net){-return&nf_ct_icmpv6_timeout;+return&icmpv6_pernet(net)->timeout;}/* Returns verdict for packet, or -1 for invalid. */
move the nf_conntrack_l4proto_register from module_init to
dccp_net_ops.init,and change the struct dccp_net to adjust
to the nf_conntrack_l4proto_register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/netfilter/nf_conntrack_proto_dccp.c | 135 ++++++++++++++++---------------
1 files changed, 69 insertions(+), 66 deletions(-)
add pernet_operations udplite_net_ops and register it when
module nf_conntrack_proto_udplite is loaded.
move the l4proto_register from module_init function to
udplite_net_ops.init.
and implement udplite_init_net to initial the pernet sysctl
table for udplite[4,6] protos.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/netfilter/nf_conntrack_proto_udplite.c | 103 +++++++++++++++++++++++-----
1 files changed, 85 insertions(+), 18 deletions(-)
@@ -35,6 +35,17 @@ static unsigned int udplite_timeouts[UDPLITE_CT_MAX] = {[UDPLITE_CT_REPLIED]=180*HZ,};+staticintudplite_net_id__read_mostly;+structudplite_net{+structnf_proto_netpn;+unsignedinttimeouts[UDPLITE_CT_MAX];+};++staticinlinestructudplite_net*udplite_pernet(structnet*net)+{+returnnet_generic(net,udplite_net_id);+}+staticbooludplite_pkt_to_tuple(conststructsk_buff*skb,unsignedintdataoff,structnf_conntrack_tuple*tuple)
@@ -70,7 +81,7 @@ static int udplite_print_tuple(struct seq_file *s,staticunsignedint*udplite_get_timeouts(structnet*net){-returnudplite_timeouts;+returnudplite_pernet(net)->timeouts;}/* Returns verdict for packet, and may modify conntracktype */
move the nf_conntrack_l4proto_register from module_init to
proto_gre_net_ops.init.
and use gre_pernet to replace net_generic.
because gre proto has no sysctl,so only need to initial pernet data
for gre proto.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
net/netfilter/nf_conntrack_proto_gre.c | 56 ++++++++++++++++++++------------
1 files changed, 35 insertions(+), 21 deletions(-)
@@ -54,13 +54,20 @@ static unsigned int gre_timeouts[GRE_CT_MAX] = {staticintproto_gre_net_id__read_mostly;structnetns_proto_gre{+structnf_proto_netnf;rwlock_tkeymap_lock;structlist_headkeymap_list;+unsignedintgre_timeouts[GRE_CT_MAX];};+staticinlinestructnetns_proto_gre*gre_pernet(structnet*net)+{+returnnet_generic(net,proto_gre_net_id);+}+voidnf_ct_gre_keymap_flush(structnet*net){-structnetns_proto_gre*net_gre=net_generic(net,proto_gre_net_id);+structnetns_proto_gre*net_gre=gre_pernet(net);structnf_ct_gre_keymap*km,*tmp;write_lock_bh(&net_gre->keymap_lock);
@@ -85,7 +92,7 @@ static inline int gre_key_cmpfn(const struct nf_ct_gre_keymap *km,/* look up the source key for a given tuple */static__be16gre_keymap_lookup(structnet*net,structnf_conntrack_tuple*t){-structnetns_proto_gre*net_gre=net_generic(net,proto_gre_net_id);+structnetns_proto_gre*net_gre=gre_pernet(net);structnf_ct_gre_keymap*km;__be16key=0;
@@ -308,10 +308,11 @@ static int gre_timeout_nlattr_to_obj(struct nlattr *tb[],structnet*net,void*data){unsignedint*timeouts=data;+structnetns_proto_gre*net_gre=gre_pernet(net);/* set default timeouts for GRE. */-timeouts[GRE_CT_UNREPLIED]=gre_timeouts[GRE_CT_UNREPLIED];-timeouts[GRE_CT_REPLIED]=gre_timeouts[GRE_CT_REPLIED];+timeouts[GRE_CT_UNREPLIED]=net_gre->gre_timeouts[GRE_CT_UNREPLIED];+timeouts[GRE_CT_REPLIED]=net_gre->gre_timeouts[GRE_CT_REPLIED];if(tb[CTA_TIMEOUT_GRE_UNREPLIED]){timeouts[GRE_CT_UNREPLIED]=
@@ -566,11 +566,12 @@ static int sctp_timeout_nlattr_to_obj(struct nlattr *tb[],structnet*net,void*data){unsignedint*timeouts=data;+structsctp_net*sn=sctp_pernet(net);inti;/* set default SCTP timeouts. */for(i=0;i<SCTP_CONNTRACK_MAX;i++)-timeouts[i]=sctp_timeouts[i];+timeouts[i]=sn->timeouts[i];/* there's a 1:1 mapping between attributes and protocol states. */for(i=CTA_TIMEOUT_SCTP_UNSPEC+1;i<CTA_TIMEOUT_SCTP_MAX+1;i++){
@@ -160,10 +160,11 @@ static int udp_timeout_nlattr_to_obj(struct nlattr *tb[],structnet*net,void*data){unsignedint*timeouts=data;+structnf_udp_net*un=udp_pernet(net);/* set default timeouts for UDP. */-timeouts[UDP_CT_UNREPLIED]=udp_timeouts[UDP_CT_UNREPLIED];-timeouts[UDP_CT_REPLIED]=udp_timeouts[UDP_CT_REPLIED];+timeouts[UDP_CT_UNREPLIED]=un->timeouts[UDP_CT_UNREPLIED];+timeouts[UDP_CT_REPLIED]=un->timeouts[UDP_CT_REPLIED];if(tb[CTA_TIMEOUT_UDP_UNREPLIED]){timeouts[UDP_CT_UNREPLIED]=
@@ -176,10 +176,11 @@ static int udplite_timeout_nlattr_to_obj(struct nlattr *tb[],structnet*net,void*data){unsignedint*timeouts=data;+structudplite_net*un=udplite_pernet(net);/* set default timeouts for UDPlite. */-timeouts[UDPLITE_CT_UNREPLIED]=udplite_timeouts[UDPLITE_CT_UNREPLIED];-timeouts[UDPLITE_CT_REPLIED]=udplite_timeouts[UDPLITE_CT_REPLIED];+timeouts[UDPLITE_CT_UNREPLIED]=un->timeouts[UDPLITE_CT_UNREPLIED];+timeouts[UDPLITE_CT_REPLIED]=un->timeouts[UDPLITE_CT_REPLIED];if(tb[CTA_TIMEOUT_UDPLITE_UNREPLIED]){timeouts[UDPLITE_CT_UNREPLIED]=
Currently the sysctl of netfilter proto is not isolated, so when
changing proto's sysctl in container will cause the host's sysctl
be changed too. it's not expected.
This patch set adds the namespace support for netfilter protos.
ping
--
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
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:12:00
On Mon, May 14, 2012 at 04:52:11PM +0800, Gao feng wrote:
From: Gao feng <redacted>
the struct nf_proto_net stroes proto's ctl_table_header and ctl_table,
nf_ct_l4proto_(un)register_sysctl use it to register sysctl.
there are some changes for struct nf_conntrack_l4proto:
- add field compat to identify if this proto should do compat.
- the net_id field is used to store the pernet_operations id
that belones to l4proto.
- init_net will be used to initial the proto's pernet data
and add init_net for struct nf_conntrack_l3proto too.
This patchset looks bette but there are still things that we have to
resolve.
The first one (regarding this patch 1/17) changes in:
* include/net/netfilter/nf_conntrack_l4proto.h
* include/net/netns/conntrack.h
should be included in:
[PATCH] netfilter: add namespace support for l4proto
And changes in:
* include/net/netfilter/nf_conntrack_l3proto.h
should be included in:
[PATCH] netfilter: add namespace support for l3proto
I already told you. A patch that adds a structure without using it,
is not good. The structure has to go together with the code uses it.
More comments below.
@@ -69,6 +69,9 @@ struct nf_conntrack_l3proto {structctl_table*ctl_table;#endif /* CONFIG_SYSCTL */+/* Init l3proto pernet data */+int(*init_net)(structnet*net);+/* Module (if any) which this is connected to. */structmodule*me;};
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
You have to fix this.
quoted hunk
+
/* Try to fill in the third arg: dataoff is offset past network protocol
hdr. Return true if possible. */
bool (*pkt_to_tuple)(const struct sk_buff *skb, unsigned int dataoff,
@@ -103,6 +105,10 @@ struct nf_conntrack_l4proto { struct ctl_table *ctl_compat_table; #endif #endif+ int *net_id;+ /* Init l4proto pernet data */+ int (*init_net)(struct net *net, u_int8_t compat);+ /* Protocol name */ const char *name;
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:25:57
On Mon, May 14, 2012 at 04:52:12PM +0800, Gao feng wrote:
quoted hunk
From: Gao feng <redacted>
-nf_ct_(un)register_sysctl are changed to support net namespace,
use (un)register_net_sysctl_table replaces (un)register_sysctl_paths.
and in nf_ct_unregister_sysctl,kfree table only when users is 0.
-Add the struct net as param of nf_conntrack_l4proto_(un)register.
register or unregister the l4proto only when the net is init_net.
-nf_conntrack_l4proto_register call init_net to initial the pernet
data of l4proto.
-nf_ct_l4proto_net is used to get the pernet data of l4proto.
-use init_net as a param of nf_conntrack_l4proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 13 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 18 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 18 +-
net/netfilter/nf_conntrack_proto.c | 245 ++++++++++++++----------
net/netfilter/nf_conntrack_proto_dccp.c | 10 +-
net/netfilter/nf_conntrack_proto_gre.c | 6 +-
net/netfilter/nf_conntrack_proto_sctp.c | 10 +-
net/netfilter/nf_conntrack_proto_udplite.c | 10 +-
8 files changed, 191 insertions(+), 139 deletions(-)
Minor nitpick: you save this amount of edits in this function that
result from the extra tabbing by moving all ...
if (net == &init_net) {
... this code ...
}
into some new static int nf_conntrack_l4proto_register_net(...) that
will be called by nf_conntrack_l4proto_register.
It will result more maintainable code. We still stick to 80-chars
columns, saving that extra tabbing makes the code more readable.
- if (l4proto->l3proto >= PF_MAX)
- return -EBUSY;
-
- if ((l4proto->to_nlattr && !l4proto->nlattr_size)
- || (l4proto->tuple_to_nlattr && !l4proto->nlattr_tuple_size))
- return -EINVAL;
-
- mutex_lock(&nf_ct_proto_mutex);
- if (!nf_ct_protos[l4proto->l3proto]) {
- /* l3proto may be loaded latter. */
- struct nf_conntrack_l4proto __rcu **proto_array;
- int i;
-
- proto_array = kmalloc(MAX_NF_CT_PROTO *
- sizeof(struct nf_conntrack_l4proto *),
- GFP_KERNEL);
- if (proto_array == NULL) {
- ret = -ENOMEM;
+ if (net == &init_net) {
+ if (l4proto->l3proto >= PF_MAX)
+ return -EBUSY;
+
+ if ((l4proto->to_nlattr && !l4proto->nlattr_size)
+ || (l4proto->tuple_to_nlattr && !l4proto->nlattr_tuple_size))
+ return -EINVAL;
+
+ mutex_lock(&nf_ct_proto_mutex);
+ if (!nf_ct_protos[l4proto->l3proto]) {
+ /* l3proto may be loaded latter. */
+ struct nf_conntrack_l4proto __rcu **proto_array;
+ int i;
+
+ proto_array = kmalloc(MAX_NF_CT_PROTO *
+ sizeof(struct nf_conntrack_l4proto *),
+ GFP_KERNEL);
+ if (proto_array == NULL) {
+ ret = -ENOMEM;
+ goto out_unlock;
+ }
+
+ for (i = 0; i < MAX_NF_CT_PROTO; i++)
+ RCU_INIT_POINTER(proto_array[i], &nf_conntrack_l4proto_generic);
+
+ /* Before making proto_array visible to lockless readers,
+ * we must make sure its content is committed to memory.
+ */
+ smp_wmb();
+
+ nf_ct_protos[l4proto->l3proto] = proto_array;
+ } else if (rcu_dereference_protected(
+ nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
+ lockdep_is_held(&nf_ct_proto_mutex)
+ ) != &nf_conntrack_l4proto_generic) {
+ ret = -EBUSY;
goto out_unlock;
}
- for (i = 0; i < MAX_NF_CT_PROTO; i++)
- RCU_INIT_POINTER(proto_array[i], &nf_conntrack_l4proto_generic);
-
- /* Before making proto_array visible to lockless readers,
- * we must make sure its content is committed to memory.
- */
- smp_wmb();
-
- nf_ct_protos[l4proto->l3proto] = proto_array;
- } else if (rcu_dereference_protected(
- nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
- lockdep_is_held(&nf_ct_proto_mutex)
- ) != &nf_conntrack_l4proto_generic) {
- ret = -EBUSY;
- goto out_unlock;
- }
-
- ret = nf_ct_l4proto_register_sysctl(l4proto);
- if (ret < 0)
- goto out_unlock;
-
- l4proto->nla_size = 0;
- if (l4proto->nlattr_size)
- l4proto->nla_size += l4proto->nlattr_size();
- if (l4proto->nlattr_tuple_size)
- l4proto->nla_size += 3 * l4proto->nlattr_tuple_size();
-
- rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
- l4proto);
+ l4proto->nla_size = 0;
+ if (l4proto->nlattr_size)
+ l4proto->nla_size += l4proto->nlattr_size();
+ if (l4proto->nlattr_tuple_size)
+ l4proto->nla_size += 3 * l4proto->nlattr_tuple_size();
+ rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
+ l4proto);
out_unlock:
- mutex_unlock(&nf_ct_proto_mutex);
- return ret;
+ mutex_unlock(&nf_ct_proto_mutex);
+ if (ret < 0)
+ return ret;
+ }
+ if (l4proto->init_net) {
+ ret = l4proto->init_net(net, l4proto->compat);
+ if (ret < 0)
+ return ret;
+ }
+ return nf_ct_l4proto_register_sysctl(net, l4proto);
}
EXPORT_SYMBOL_GPL(nf_conntrack_l4proto_register);
-void nf_conntrack_l4proto_unregister(struct nf_conntrack_l4proto *l4proto)
+void nf_conntrack_l4proto_unregister(struct net *net,
+ struct nf_conntrack_l4proto *l4proto)
{
- struct net *net;
-
- BUG_ON(l4proto->l3proto >= PF_MAX);
-
- mutex_lock(&nf_ct_proto_mutex);
- BUG_ON(rcu_dereference_protected(
- nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
- lockdep_is_held(&nf_ct_proto_mutex)
- ) != l4proto);
- rcu_assign_pointer(nf_ct_protos[l4proto->l3proto][l4proto->l4proto],
- &nf_conntrack_l4proto_generic);
- nf_ct_l4proto_unregister_sysctl(l4proto);
- mutex_unlock(&nf_ct_proto_mutex);
-
- synchronize_rcu();
+ if (net == &init_net) {
@@ -299,23 +299,23 @@ static int __init nf_conntrack_proto_udplite_init(void){interr;-err=nf_conntrack_l4proto_register(&nf_conntrack_l4proto_udplite4);+err=nf_conntrack_l4proto_register(&init_net,&nf_conntrack_l4proto_udplite4);if(err<0)gotoerr1;-err=nf_conntrack_l4proto_register(&nf_conntrack_l4proto_udplite6);+err=nf_conntrack_l4proto_register(&init_net,&nf_conntrack_l4proto_udplite6);if(err<0)gotoerr2;return0;err2:-nf_conntrack_l4proto_unregister(&nf_conntrack_l4proto_udplite4);+nf_conntrack_l4proto_unregister(&init_net,&nf_conntrack_l4proto_udplite4);err1:returnerr;}staticvoid__exitnf_conntrack_proto_udplite_exit(void){-nf_conntrack_l4proto_unregister(&nf_conntrack_l4proto_udplite6);-nf_conntrack_l4proto_unregister(&nf_conntrack_l4proto_udplite4);+nf_conntrack_l4proto_unregister(&init_net,&nf_conntrack_l4proto_udplite6);+nf_conntrack_l4proto_unregister(&init_net,&nf_conntrack_l4proto_udplite4);}module_init(nf_conntrack_proto_udplite_init);
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:29:16
On Mon, May 14, 2012 at 04:52:13PM +0800, Gao feng wrote:
quoted hunk
-Add the struct net as param of nf_conntrack_l3proto_(un)register.
register or unregister the l3proto only when the net is init_net.
-The new struct nf_ip_net is used to store the sysctl header and data
of l3proto_ipv4,l4proto_tcp(6),l4proto_udp(6),l4proto_icmp(v6).
because the protos such tcp and tcp6 use the same data,so making
nf_ip_net as a field of netns_ct is the easiest way to manager it.
-nf_ct_l3proto_register_sysctl call init_net to initial the pernet data
of l3proto.
-nf_ct_l3proto_net is used to get the pernet data of l3proto.
-export nf_conntrack_l3proto_(un)register
-use init_net as param of nf_conntrack_l3proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l3proto.h | 6 +-
include/net/netns/conntrack.h | 8 ++
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 +-
net/netfilter/nf_conntrack_proto.c | 127 +++++++++++++++---------
5 files changed, 97 insertions(+), 56 deletions(-)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:32:32
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted hunk
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
@@ -42,7 +47,7 @@ static int generic_print_tuple(struct seq_file *s,staticunsignedint*generic_get_timeouts(structnet*net){-return&nf_ct_generic_timeout;+return&(generic_pernet(net)->timeout);}/* Returns verdict for packet, or -1 for invalid. */
Interesting. This structure is added in patch 1/17, then it's remove
in patch 15/17.
Probably I'm missing anything, but why are you doing it like that?
quoted hunk
int *net_id;
/* Init l4proto pernet data */
int (*init_net)(struct net *net, u_int8_t compat);
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:41:10
On Mon, May 14, 2012 at 04:52:26PM +0800, Gao feng wrote:
add struct net as a param of ctnl_timeout.nlattr_to_obj,
modify ctnl_timeout_parse_policy and cttimeout_new_timeout
to transmit struct net to nlattr_to_obj.
Please, merge your patch 16 and 17 into one single patch.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-23 10:43:12
On Mon, May 14, 2012 at 04:52:10PM +0800, Gao feng wrote:
Currently the sysctl of netfilter proto is not isolated, so when
changing proto's sysctl in container will cause the host's sysctl
be changed too. it's not expected.
This patch set adds the namespace support for netfilter protos.
impletement four pernet_operations to register sysctl and initial
pernet data for proto.
-ipv4_net_ops is used to register tcp4(compat),
udp4(compat),icmp(compat),ipv4(compat).
-ipv6_net_ops is used to register tcp6,udp6 and icmpv6.
-sctp_net_ops is used to register sctp4(compat) and sctp6.
-udplite_net_ops is used to register udplite4 and udplite6
extern l[3,4]proto (sysctl) register functions to make them support
namespace.
finailly add namespace support for cttimeout.
This requires another spin. It looks way better than previous version
but I don't want to take the patchset and then send another batch to
David to remove the .compat field, the unrequired export of couple of
symbols, and so on...
Thanks!
Interesting. This structure is added in patch 1/17, then it's remove
in patch 15/17.
Probably I'm missing anything, but why are you doing it like that?
This structure means ctl_table_header,ctl_table and so on?
I add this structure to struct nf_proto_net in patch 1/17,so those fields in
struct nf_conntrack_l4proto are useless,this patch is just some cleanup.
the same with nf_conntrack_l3proto.
--
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 Mon, May 14, 2012 at 04:52:26PM +0800, Gao feng wrote:
quoted
add struct net as a param of ctnl_timeout.nlattr_to_obj,
modify ctnl_timeout_parse_policy and cttimeout_new_timeout
to transmit struct net to nlattr_to_obj.
Please, merge your patch 16 and 17 into one single patch.
@@ -562,7 +562,8 @@ static int sctp_nlattr_size(void)#include<linux/netfilter/nfnetlink.h>#include<linux/netfilter/nfnetlink_cttimeout.h>-staticintsctp_timeout_nlattr_to_obj(structnlattr*tb[],void*data)+staticintsctp_timeout_nlattr_to_obj(structnlattr*tb[],+structnet*net,void*data)
The interface modification and the use of the new *net parameter
should go together, ie. merge patch 16 and 17 :-).
got it,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
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
...
}
if my understanding is right,my answer is yes we can ;)
--
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
Hi pablo:
于 2012年05月23日 18:12, Pablo Neira Ayuso 写道:
On Mon, May 14, 2012 at 04:52:11PM +0800, Gao feng wrote:
quoted
From: Gao feng <redacted>
the struct nf_proto_net stroes proto's ctl_table_header and ctl_table,
nf_ct_l4proto_(un)register_sysctl use it to register sysctl.
there are some changes for struct nf_conntrack_l4proto:
- add field compat to identify if this proto should do compat.
- the net_id field is used to store the pernet_operations id
that belones to l4proto.
- init_net will be used to initial the proto's pernet data
and add init_net for struct nf_conntrack_l3proto too.
This patchset looks bette but there are still things that we have to
resolve.
The first one (regarding this patch 1/17) changes in:
* include/net/netfilter/nf_conntrack_l4proto.h
* include/net/netns/conntrack.h
should be included in:
[PATCH] netfilter: add namespace support for l4proto
And changes in:
* include/net/netfilter/nf_conntrack_l3proto.h
should be included in:
[PATCH] netfilter: add namespace support for l3proto
I already told you. A patch that adds a structure without using it,
is not good. The structure has to go together with the code uses it.
It seams this patch should be merged to "netfilter: add namespace support for l4proto"
the struct nf_proto_net is first used there.
@@ -69,6 +69,9 @@ struct nf_conntrack_l3proto {structctl_table*ctl_table;#endif /* CONFIG_SYSCTL */+/* Init l3proto pernet data */+int(*init_net)(structnet*net);+/* Module (if any) which this is connected to. */structmodule*me;};
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
and beacuse protocols will have pernet ctl_compat_table and ctl_table,the .ctl_compat_table
field will be deleted in patch 15/17. so we should the new field compat.
actually, we don't need to pass compat value for generic_init_net,beacuse
we know l4proto_generic need compat. But consider there are l4proto_tcp(6), and in order to keep
code readable,I prefer to add compat field and pass it to init_net.
You have to fix this.
quoted
+
/* Try to fill in the third arg: dataoff is offset past network protocol
hdr. Return true if possible. */
bool (*pkt_to_tuple)(const struct sk_buff *skb, unsigned int dataoff,
@@ -103,6 +105,10 @@ struct nf_conntrack_l4proto { struct ctl_table *ctl_compat_table; #endif #endif+ int *net_id;+ /* Init l4proto pernet data */+ int (*init_net)(struct net *net, u_int8_t compat);+ /* Protocol name */ const char *name;
--
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
--
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 Mon, May 14, 2012 at 04:52:12PM +0800, Gao feng wrote:
quoted
From: Gao feng <redacted>
-nf_ct_(un)register_sysctl are changed to support net namespace,
use (un)register_net_sysctl_table replaces (un)register_sysctl_paths.
and in nf_ct_unregister_sysctl,kfree table only when users is 0.
-Add the struct net as param of nf_conntrack_l4proto_(un)register.
register or unregister the l4proto only when the net is init_net.
-nf_conntrack_l4proto_register call init_net to initial the pernet
data of l4proto.
-nf_ct_l4proto_net is used to get the pernet data of l4proto.
-use init_net as a param of nf_conntrack_l4proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 13 +-
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 18 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 18 +-
net/netfilter/nf_conntrack_proto.c | 245 ++++++++++++++----------
net/netfilter/nf_conntrack_proto_dccp.c | 10 +-
net/netfilter/nf_conntrack_proto_gre.c | 6 +-
net/netfilter/nf_conntrack_proto_sctp.c | 10 +-
net/netfilter/nf_conntrack_proto_udplite.c | 10 +-
8 files changed, 191 insertions(+), 139 deletions(-)
@@ -243,137 +253,172 @@ void nf_conntrack_l3proto_unregister(struct nf_conntrack_l3proto *proto) } EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_unregister);-static int nf_ct_l4proto_register_sysctl(struct nf_conntrack_l4proto *l4proto)+static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,+ struct nf_conntrack_l4proto *l4proto) {- int err = 0;+ if (l4proto->net_id)+ return net_generic(net, *l4proto->net_id);+ else+ return NULL;+}+int nf_ct_l4proto_register_sysctl(struct net *net,+ struct nf_conntrack_l4proto *l4proto)+{+ int err = 0;+ struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);+ if (pn == NULL)+ return 0; #ifdef CONFIG_SYSCTL- if (l4proto->ctl_table != NULL) {- err = nf_ct_register_sysctl(l4proto->ctl_table_header,+ if (pn->ctl_table != NULL) {+ err = nf_ct_register_sysctl(net,+ &pn->ctl_table_header, "net/netfilter",- l4proto->ctl_table,- l4proto->ctl_table_users);- if (err < 0)+ pn->ctl_table,+ &pn->users);+ if (err < 0) {+ kfree(pn->ctl_table);+ pn->ctl_table = NULL;
^^^^^^^^^^^
Do you really need to set this above to NULL? Is there any existing
bug trap? If not, it's superfluous, please, remove it.
yes,l4proto_tcp(udp,icmp)'s ctl_table is stored in netns_ct.proto,
so when we register l4proto_tcp's sysctl failed,ctl_table will still
point to the kfreed memory. this will cause panic the next
time we register l4proto_tcp's sysctl.
Minor nitpick: you save this amount of edits in this function that
result from the extra tabbing by moving all ...
if (net == &init_net) {
... this code ...
}
into some new static int nf_conntrack_l4proto_register_net(...) that
will be called by nf_conntrack_l4proto_register.
It will result more maintainable code. We still stick to 80-chars
columns, saving that extra tabbing makes the code more readable.
Yes,it will be more readable,I will do it.
--
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 Mon, May 14, 2012 at 04:52:13PM +0800, Gao feng wrote:
quoted
-Add the struct net as param of nf_conntrack_l3proto_(un)register.
register or unregister the l3proto only when the net is init_net.
-The new struct nf_ip_net is used to store the sysctl header and data
of l3proto_ipv4,l4proto_tcp(6),l4proto_udp(6),l4proto_icmp(v6).
because the protos such tcp and tcp6 use the same data,so making
nf_ip_net as a field of netns_ct is the easiest way to manager it.
-nf_ct_l3proto_register_sysctl call init_net to initial the pernet data
of l3proto.
-nf_ct_l3proto_net is used to get the pernet data of l3proto.
-export nf_conntrack_l3proto_(un)register
-use init_net as param of nf_conntrack_l3proto_(un)register.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l3proto.h | 6 +-
include/net/netns/conntrack.h | 8 ++
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 6 +-
net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 6 +-
net/netfilter/nf_conntrack_proto.c | 127 +++++++++++++++---------
5 files changed, 97 insertions(+), 56 deletions(-)
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-24 09:52:33
On Thu, May 24, 2012 at 09:13:36AM +0800, Gao feng wrote:
于 2012年05月23日 18:32, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
Yes, all protocol trackers included in nf_conntrack_proto_init:
err = nf_conntrack_proto_generic_init(net);
...
err = nf_conntrack_proto_tcp_init(net);
...
and so on.
...
}
if my understanding is right,my answer is yes we can ;)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Interesting. This structure is added in patch 1/17, then it's remove
in patch 15/17.
Probably I'm missing anything, but why are you doing it like that?
This structure means ctl_table_header,ctl_table and so on?
I add this structure to struct nf_proto_net in patch 1/17,so those fields in
struct nf_conntrack_l4proto are useless,this patch is just some cleanup.
the same with nf_conntrack_l3proto.
I see, then it's OK. Please, elaborate a bit more the patch
description to explain that this structure is not required anymore.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-24 09:59:12
On Thu, May 24, 2012 at 09:35:50AM +0800, Gao feng wrote:
Hi pablo:
于 2012年05月23日 18:12, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:11PM +0800, Gao feng wrote:
quoted
From: Gao feng <redacted>
the struct nf_proto_net stroes proto's ctl_table_header and ctl_table,
nf_ct_l4proto_(un)register_sysctl use it to register sysctl.
there are some changes for struct nf_conntrack_l4proto:
- add field compat to identify if this proto should do compat.
- the net_id field is used to store the pernet_operations id
that belones to l4proto.
- init_net will be used to initial the proto's pernet data
and add init_net for struct nf_conntrack_l3proto too.
This patchset looks bette but there are still things that we have to
resolve.
The first one (regarding this patch 1/17) changes in:
* include/net/netfilter/nf_conntrack_l4proto.h
* include/net/netns/conntrack.h
should be included in:
[PATCH] netfilter: add namespace support for l4proto
And changes in:
* include/net/netfilter/nf_conntrack_l3proto.h
should be included in:
[PATCH] netfilter: add namespace support for l3proto
I already told you. A patch that adds a structure without using it,
is not good. The structure has to go together with the code uses it.
It seams this patch should be merged to "netfilter: add namespace support for l4proto"
the struct nf_proto_net is first used there.
@@ -69,6 +69,9 @@ struct nf_conntrack_l3proto {structctl_table*ctl_table;#endif /* CONFIG_SYSCTL */+/* Init l3proto pernet data */+int(*init_net)(structnet*net);+/* Module (if any) which this is connected to. */structmodule*me;};
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
and beacuse protocols will have pernet ctl_compat_table and ctl_table,the .ctl_compat_table
field will be deleted in patch 15/17. so we should the new field compat.
actually, we don't need to pass compat value for generic_init_net,beacuse
we know l4proto_generic need compat. But consider there are l4proto_tcp(6), and in order to keep
code readable,I prefer to add compat field and pass it to init_net.
quoted
You have to fix this.
quoted
+
/* Try to fill in the third arg: dataoff is offset past network protocol
hdr. Return true if possible. */
bool (*pkt_to_tuple)(const struct sk_buff *skb, unsigned int dataoff,
@@ -103,6 +105,10 @@ struct nf_conntrack_l4proto { struct ctl_table *ctl_compat_table; #endif #endif+ int *net_id;+ /* Init l4proto pernet data */+ int (*init_net)(struct net *net, u_int8_t compat);+ /* Protocol name */ const char *name;
--
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
--
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
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-24 10:00:50
On Thu, May 24, 2012 at 09:52:51AM +0800, Gao feng wrote:
于 2012年05月23日 18:25, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:12PM +0800, Gao feng wrote:
quoted
From: Gao feng <redacted>
[...]
quoted
quoted
@@ -243,137 +253,172 @@ void nf_conntrack_l3proto_unregister(struct nf_conntrack_l3proto *proto) } EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_unregister);-static int nf_ct_l4proto_register_sysctl(struct nf_conntrack_l4proto *l4proto)+static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,+ struct nf_conntrack_l4proto *l4proto) {- int err = 0;+ if (l4proto->net_id)+ return net_generic(net, *l4proto->net_id);+ else+ return NULL;+}+int nf_ct_l4proto_register_sysctl(struct net *net,+ struct nf_conntrack_l4proto *l4proto)+{+ int err = 0;+ struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);+ if (pn == NULL)+ return 0; #ifdef CONFIG_SYSCTL- if (l4proto->ctl_table != NULL) {- err = nf_ct_register_sysctl(l4proto->ctl_table_header,+ if (pn->ctl_table != NULL) {+ err = nf_ct_register_sysctl(net,+ &pn->ctl_table_header, "net/netfilter",- l4proto->ctl_table,- l4proto->ctl_table_users);- if (err < 0)+ pn->ctl_table,+ &pn->users);+ if (err < 0) {+ kfree(pn->ctl_table);+ pn->ctl_table = NULL;
^^^^^^^^^^^
Do you really need to set this above to NULL? Is there any existing
bug trap? If not, it's superfluous, please, remove it.
yes,l4proto_tcp(udp,icmp)'s ctl_table is stored in netns_ct.proto,
so when we register l4proto_tcp's sysctl failed,ctl_table will still
point to the kfreed memory. this will cause panic the next
time we register l4proto_tcp's sysctl.
Because l3proto_ipv6 doesn't need sysctl,so l3proto_ipv6's nf_ip_net is NULL,
please see function nf_ct_l3proto_net above.
Then, please add a comment there to explain that some per-net protocol
information may missing since no sysctl is supported.
Yes, I will add a comment to make it more clearer ;)
--
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 Thu, May 24, 2012 at 09:13:36AM +0800, Gao feng wrote:
quoted
于 2012年05月23日 18:32, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
Yes, all protocol trackers included in nf_conntrack_proto_init:
err = nf_conntrack_proto_generic_init(net);
...
err = nf_conntrack_proto_tcp_init(net);
...
and so on.
sounds good,but the l4protos except l4proto_generic are enabled by
insmod modules(such as nf_conntrack_ipv4,nf_conntrack_proto_udplite).
So I think it makes no sense to init all protocol here, unless we decide
to put those protos into module nf_conntrack.
--
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 Thu, May 24, 2012 at 09:35:50AM +0800, Gao feng wrote:
quoted
Hi pablo:
于 2012年05月23日 18:12, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:11PM +0800, Gao feng wrote:
quoted
From: Gao feng <redacted>
the struct nf_proto_net stroes proto's ctl_table_header and ctl_table,
nf_ct_l4proto_(un)register_sysctl use it to register sysctl.
there are some changes for struct nf_conntrack_l4proto:
- add field compat to identify if this proto should do compat.
- the net_id field is used to store the pernet_operations id
that belones to l4proto.
- init_net will be used to initial the proto's pernet data
and add init_net for struct nf_conntrack_l3proto too.
This patchset looks bette but there are still things that we have to
resolve.
The first one (regarding this patch 1/17) changes in:
* include/net/netfilter/nf_conntrack_l4proto.h
* include/net/netns/conntrack.h
should be included in:
[PATCH] netfilter: add namespace support for l4proto
And changes in:
* include/net/netfilter/nf_conntrack_l3proto.h
should be included in:
[PATCH] netfilter: add namespace support for l3proto
I already told you. A patch that adds a structure without using it,
is not good. The structure has to go together with the code uses it.
It seams this patch should be merged to "netfilter: add namespace support for l4proto"
the struct nf_proto_net is first used there.
@@ -69,6 +69,9 @@ struct nf_conntrack_l3proto {structctl_table*ctl_table;#endif /* CONFIG_SYSCTL */+/* Init l3proto pernet data */+int(*init_net)(structnet*net);+/* Module (if any) which this is connected to. */structmodule*me;};
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-24 14:38:54
On Thu, May 24, 2012 at 06:54:42PM +0800, Gao feng wrote:
[...]
quoted
quoted
quoted
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
If this removes the .compat field that you added, then use two
init_net functions, yes.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-24 14:40:44
On Thu, May 24, 2012 at 07:07:36PM +0800, Gao feng wrote:
于 2012年05月24日 17:52, Pablo Neira Ayuso 写道:
quoted
On Thu, May 24, 2012 at 09:13:36AM +0800, Gao feng wrote:
quoted
于 2012年05月23日 18:32, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
Yes, all protocol trackers included in nf_conntrack_proto_init:
err = nf_conntrack_proto_generic_init(net);
...
err = nf_conntrack_proto_tcp_init(net);
...
and so on.
sounds good,but the l4protos except l4proto_generic are enabled by
insmod modules(such as nf_conntrack_ipv4,nf_conntrack_proto_udplite).
So I think it makes no sense to init all protocol here, unless we decide
to put those protos into module nf_conntrack.
Sorry, I meant to say all protocols that are built-in.
So, just put there those that are built-in, like TCP, UDP and generic
On Thu, May 24, 2012 at 06:54:42PM +0800, Gao feng wrote:
[...]
quoted
quoted
quoted
quoted
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
If this removes the .compat field that you added, then use two
init_net functions, yes.
Sorry I miss something.
nf_ct_l4proto_unregister_sysctl also uses .compat to identify if we
can unregister the compat sysctl.
if we register l4proto_tcp and l4proto_tcp6 both. without .compat,
when unregister l4proto_tcp6, the compat sysctl will be unregister too.
So maybe we have to use .compat.
--
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
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-25 02:54:51
On Fri, May 25, 2012 at 09:05:34AM +0800, Gao feng wrote:
于 2012年05月24日 22:38, Pablo Neira Ayuso 写道:
quoted
On Thu, May 24, 2012 at 06:54:42PM +0800, Gao feng wrote:
[...]
quoted
quoted
quoted
quoted
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
If this removes the .compat field that you added, then use two
init_net functions, yes.
Sorry I miss something.
nf_ct_l4proto_unregister_sysctl also uses .compat to identify if we
can unregister the compat sysctl.
if we register l4proto_tcp and l4proto_tcp6 both. without .compat,
when unregister l4proto_tcp6, the compat sysctl will be unregister too.
So maybe we have to use .compat.
Could you resolve this by checking pn->ctl_compat_header != NULL ?
--
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, May 25, 2012 at 09:05:34AM +0800, Gao feng wrote:
quoted
于 2012年05月24日 22:38, Pablo Neira Ayuso 写道:
quoted
On Thu, May 24, 2012 at 06:54:42PM +0800, Gao feng wrote:
[...]
quoted
quoted
quoted
quoted
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
If this removes the .compat field that you added, then use two
init_net functions, yes.
Sorry I miss something.
nf_ct_l4proto_unregister_sysctl also uses .compat to identify if we
can unregister the compat sysctl.
if we register l4proto_tcp and l4proto_tcp6 both. without .compat,
when unregister l4proto_tcp6, the compat sysctl will be unregister too.
So maybe we have to use .compat.
Could you resolve this by checking pn->ctl_compat_header != NULL ?
pn->ctl_table_header and ctl_compat_header is shared by l4proto_tcp and l4proto_tcp6.
if we both register l4proto_tcp and l4proto_tcp6, when unregister l4proto_tcp6
pn->ctl_compat_header must not be NULL.
--
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
You can make a generic function to set the ctl_data that you can
reuse for this code above and the one below.
Actually I want reuse this code too,
But Unfortunately the ctl_data has different order or different size.
ctl_compat_table[1].data = &tn->timeouts[TCP_CONNTRACK_SYN_SENT2]
but
ctl_table[1].data = &tn->timeouts[TCP_CONNTRACK_SYN_RECV];
I have bad experience with code that has lots of #ifdef's.
Please, split all *_init_net into smaller functions.
It did look ugly,I will try my best to make code clear. ;)
--
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, May 25, 2012 at 09:05:34AM +0800, Gao feng wrote:
quoted
于 2012年05月24日 22:38, Pablo Neira Ayuso 写道:
quoted
On Thu, May 24, 2012 at 06:54:42PM +0800, Gao feng wrote:
[...]
quoted
quoted
quoted
quoted
I don't see why we need this new field.
It seems to be set to 1 in each structure that has set:
.ctl_compat_table
to non-NULL. So, it's redundant.
Moreover, you already know from the protocol tracker itself if you
have to allocate the compat ctl table or not.
In other words: You set compat to 1 for nf_conntrack_l4proto_generic.
Then, you pass that compat value to generic_init_net via ->inet_net
again, but this information (that determines if the compat has to be
done or not) is already in the scope of the protocol tracker.
because some protocols such l4proto_tcp6 and l4proto_tcp use the same init_net
function. the l4proto_tcp6 doesn't need compat sysctl, so we should use this new
field to identify if we should kmemdup compat_sysctl_table.
Then, could you use two init_net functions? one for TCP for IPv4 and another
for TCP for IPv6?
Of cause, if you prefer to impletment it in this way.
If this removes the .compat field that you added, then use two
init_net functions, yes.
Sorry I miss something.
nf_ct_l4proto_unregister_sysctl also uses .compat to identify if we
can unregister the compat sysctl.
if we register l4proto_tcp and l4proto_tcp6 both. without .compat,
when unregister l4proto_tcp6, the compat sysctl will be unregister too.
So maybe we have to use .compat.
Could you resolve this by checking pn->ctl_compat_header != NULL ?
pn->ctl_table_header and ctl_compat_header is shared by l4proto_tcp and l4proto_tcp6.
if we both register l4proto_tcp and l4proto_tcp6, when unregister l4proto_tcp6
pn->ctl_compat_header must not be NULL.
Maybe we can resolve this by nf_conntrack_l4proto.l3proto == AF_INET && pn->ctl_compat_header != NULL
Because compat sysctl is registered by AF_INET's proto only.
--
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 Thu, May 24, 2012 at 07:07:36PM +0800, Gao feng wrote:
quoted
于 2012年05月24日 17:52, Pablo Neira Ayuso 写道:
quoted
On Thu, May 24, 2012 at 09:13:36AM +0800, Gao feng wrote:
quoted
于 2012年05月23日 18:32, Pablo Neira Ayuso 写道:
quoted
On Mon, May 14, 2012 at 04:52:14PM +0800, Gao feng wrote:
quoted
implement and export nf_conntrack_proto_generic_[init,fini],
nf_conntrack_[init,cleanup]_net call them to register or unregister
the sysctl of generic proto.
implement generic_net_init,it's used to initial the pernet
data for generic proto.
and use nf_generic_net.timeout to replace nf_ct_generic_timeout in
get_timeouts function.
Acked-by: Eric W. Biederman <redacted>
Signed-off-by: Gao feng <redacted>
---
include/net/netfilter/nf_conntrack_l4proto.h | 2 +
include/net/netns/conntrack.h | 6 +++
net/netfilter/nf_conntrack_core.c | 8 +++-
net/netfilter/nf_conntrack_proto.c | 21 +++++-----
net/netfilter/nf_conntrack_proto_generic.c | 55 ++++++++++++++++++++++++-
5 files changed, 76 insertions(+), 16 deletions(-)
@@ -1353,6 +1353,7 @@ static void nf_conntrack_cleanup_net(struct net *net)}nf_ct_free_hashtable(net->ct.hash,net->ct.htable_size);+nf_conntrack_proto_generic_fini(net);nf_conntrack_helper_fini(net);nf_conntrack_timeout_fini(net);nf_conntrack_ecache_fini(net);
@@ -1586,9 +1587,12 @@ static int nf_conntrack_init_net(struct net *net)ret=nf_conntrack_helper_init(net);if(ret<0)gotoerr_helper;-+ret=nf_conntrack_proto_generic_init(net);+if(ret<0)+gotoerr_generic;return0;-+err_generic:+nf_conntrack_helper_fini(net);err_helper:nf_conntrack_timeout_fini(net);err_timeout:
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
Yes, all protocol trackers included in nf_conntrack_proto_init:
err = nf_conntrack_proto_generic_init(net);
...
err = nf_conntrack_proto_tcp_init(net);
...
and so on.
sounds good,but the l4protos except l4proto_generic are enabled by
insmod modules(such as nf_conntrack_ipv4,nf_conntrack_proto_udplite).
So I think it makes no sense to init all protocol here, unless we decide
to put those protos into module nf_conntrack.
Sorry, I meant to say all protocols that are built-in.
So, just put there those that are built-in, like TCP, UDP and generic
AFAIK l4proto_generic is registered when install module nf_conntrack,
BUT l4proto_tcp,l4proto_udp,l4proto_icmp are registered when install module nf_conntrack_ipv4.
So we can only register generic proto here.
--
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
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2012-05-28 09:53:02
On Sat, May 26, 2012 at 10:28:33AM +0800, Gao feng wrote:
于 2012年05月25日 14:02, Gao feng 写道:
quoted
于 2012年05月25日 10:54, Pablo Neira Ayuso 写道:
[...]
quoted
quoted
Could you resolve this by checking pn->ctl_compat_header != NULL ?
pn->ctl_table_header and ctl_compat_header is shared by l4proto_tcp and l4proto_tcp6.
if we both register l4proto_tcp and l4proto_tcp6, when unregister l4proto_tcp6
pn->ctl_compat_header must not be NULL.
Maybe we can resolve this by nf_conntrack_l4proto.l3proto == AF_INET && pn->ctl_compat_header != NULL
Because compat sysctl is registered by AF_INET's proto only.
OK, as soon as it can remove the compat field, I prefer it.
--
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
I like that all protocols sysctl are registered by
nf_conntrack_proto_init. Can you keep using that?
you mean per-net's generic_proto sysctl are registered by
nf_conntrack_proto_init?
such as
int nf_conntrack_proto_init(struct net *net)
{
...
err = nf_ct_l4proto_register_sysctl(net, &nf_conntrack_l4proto_generic);
Yes, all protocol trackers included in nf_conntrack_proto_init:
err = nf_conntrack_proto_generic_init(net);
...
err = nf_conntrack_proto_tcp_init(net);
...
and so on.
sounds good,but the l4protos except l4proto_generic are enabled by
insmod modules(such as nf_conntrack_ipv4,nf_conntrack_proto_udplite).
So I think it makes no sense to init all protocol here, unless we decide
to put those protos into module nf_conntrack.
Sorry, I meant to say all protocols that are built-in.
So, just put there those that are built-in, like TCP, UDP and generic
AFAIK l4proto_generic is registered when install module nf_conntrack,
BUT l4proto_tcp,l4proto_udp,l4proto_icmp are registered when install module nf_conntrack_ipv4.
So we can only register generic proto here.