[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120417112634.GA2915@1984>
Date: Tue, 17 Apr 2012 13:26:34 +0200
From: Pablo Neira Ayuso <pablo@...filter.org>
To: Gao feng <gaofeng@...fujitsu.com>
Cc: netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
ebiederm@...ssion.com, serge.hallyn@...onical.com,
dlezcano@...ibm.com
Subject: Re: [PATCH 02/12] netfilter: don't register sysctl when register
proto
On Tue, Apr 17, 2012 at 06:25:57PM +0800, Gao feng wrote:
> 于 2012年04月17日 16:56, Pablo Neira Ayuso 写道:
> > On Tue, Apr 17, 2012 at 10:56:13AM +0800, Gao feng wrote:
> >> delete nf_ct_l[3,4]proto_register_sysctl when register l[3,4]proto.
> >> and add nf_ct_register_net_sysctl,nf_ct_unregister_net_sysctl to
> >> register the sysctl for net namespace.
> >>
> >> Signed-off-by: Gao feng <gaofeng@...fujitsu.com>
> >> ---
> >> net/netfilter/nf_conntrack_proto.c | 109 +++++-------------------------------
> >> 1 files changed, 15 insertions(+), 94 deletions(-)
> >>
> >> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >> index be3da2c..207cdd8 100644
> >> --- a/net/netfilter/nf_conntrack_proto.c
> >> +++ b/net/netfilter/nf_conntrack_proto.c
> >> @@ -35,12 +35,15 @@ EXPORT_SYMBOL_GPL(nf_ct_l3protos);
> >> static DEFINE_MUTEX(nf_ct_proto_mutex);
> >>
> >> #ifdef CONFIG_SYSCTL
> >> -static int
> >> -nf_ct_register_sysctl(struct ctl_table_header **header, struct ctl_path *path,
> >> - struct ctl_table *table, unsigned int *users)
> >> +int
> >> +nf_ct_register_net_sysctl(struct net *net,
> >> + struct ctl_table_header **header,
> >> + struct ctl_path *path,
> >> + struct ctl_table *table,
> >> + unsigned int *users)
> >
> > Please, don't rename this function. Just add the *net parameter
> > instead.
> >
>
> OK,i will modify it.
>
> >> {
> >> if (*header == NULL) {
> >> - *header = register_sysctl_paths(path, table);
> >> + *header = register_net_sysctl_table(net, path, table);
> >> if (*header == NULL)
> >> return -ENOMEM;
> >> }
> >> @@ -48,17 +51,21 @@ nf_ct_register_sysctl(struct ctl_table_header **header, struct ctl_path *path,
> >> (*users)++;
> >> return 0;
> >> }
> >> +EXPORT_SYMBOL_GPL(nf_ct_register_net_sysctl);
> >>
> >> -static void
> >> -nf_ct_unregister_sysctl(struct ctl_table_header **header,
> >> - struct ctl_table *table, unsigned int *users)
> >> +void
> >> +nf_ct_unregister_net_sysctl(struct ctl_table_header **header,
> >> + struct ctl_table *table,
> >> + unsigned int *users)
> >> {
> >> if (users != NULL && --*users > 0)
> >> return;
> >>
> >> unregister_sysctl_table(*header);
> >> + kfree(table);
> >> *header = NULL;
> >> }
> >> +EXPORT_SYMBOL_GPL(nf_ct_unregister_net_sysctl);
> >> #endif
> >>
> >> struct nf_conntrack_l4proto *
> >> @@ -161,29 +168,6 @@ static int kill_l4proto(struct nf_conn *i, void *data)
> >> nf_ct_l3num(i) == l4proto->l3proto;
> >> }
> >>
> >> -static int nf_ct_l3proto_register_sysctl(struct nf_conntrack_l3proto *l3proto)
> >> -{
> >> - int err = 0;
> >> -
> >> -#ifdef CONFIG_SYSCTL
> >> - if (l3proto->ctl_table != NULL) {
> >> - err = nf_ct_register_sysctl(&l3proto->ctl_table_header,
> >> - l3proto->ctl_table_path,
> >> - l3proto->ctl_table, NULL);
> >> - }
> >> -#endif
> >> - return err;
> >> -}
> >> -
> >> -static void nf_ct_l3proto_unregister_sysctl(struct nf_conntrack_l3proto *l3proto)
> >> -{
> >> -#ifdef CONFIG_SYSCTL
> >> - if (l3proto->ctl_table_header != NULL)
> >> - nf_ct_unregister_sysctl(&l3proto->ctl_table_header,
> >> - l3proto->ctl_table, NULL);
> >> -#endif
> >> -}
> >> -
> >> int nf_conntrack_l3proto_register(struct nf_conntrack_l3proto *proto)
> >> {
> >> int ret = 0;
> >> @@ -203,10 +187,6 @@ int nf_conntrack_l3proto_register(struct nf_conntrack_l3proto *proto)
> >> goto out_unlock;
> >> }
> >>
> >> - ret = nf_ct_l3proto_register_sysctl(proto);
> >> - if (ret < 0)
> >> - goto out_unlock;
> >> -
> >> if (proto->nlattr_tuple_size)
> >> proto->nla_size = 3 * proto->nlattr_tuple_size();
> >>
> >> @@ -230,7 +210,6 @@ void nf_conntrack_l3proto_unregister(struct nf_conntrack_l3proto *proto)
> >> ) != proto);
> >> rcu_assign_pointer(nf_ct_l3protos[proto->l3proto],
> >> &nf_conntrack_l3proto_generic);
> >> - nf_ct_l3proto_unregister_sysctl(proto);
> >> mutex_unlock(&nf_ct_proto_mutex);
> >>
> >> synchronize_rcu();
> >> @@ -243,52 +222,6 @@ 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)
> >> -{
> >> - int err = 0;
> >> -
> >> -#ifdef CONFIG_SYSCTL
> >> - if (l4proto->ctl_table != NULL) {
> >> - err = nf_ct_register_sysctl(l4proto->ctl_table_header,
> >> - nf_net_netfilter_sysctl_path,
> >> - l4proto->ctl_table,
> >> - l4proto->ctl_table_users);
> >> - if (err < 0)
> >> - goto out;
> >> - }
> >> -#ifdef CONFIG_NF_CONNTRACK_PROC_COMPAT
> >> - if (l4proto->ctl_compat_table != NULL) {
> >> - err = nf_ct_register_sysctl(&l4proto->ctl_compat_table_header,
> >> - nf_net_ipv4_netfilter_sysctl_path,
> >> - l4proto->ctl_compat_table, NULL);
> >> - if (err == 0)
> >> - goto out;
> >> - nf_ct_unregister_sysctl(l4proto->ctl_table_header,
> >> - l4proto->ctl_table,
> >> - l4proto->ctl_table_users);
> >> - }
> >> -#endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
> >> -out:
> >> -#endif /* CONFIG_SYSCTL */
> >> - return err;
> >> -}
> >> -
> >> -static void nf_ct_l4proto_unregister_sysctl(struct nf_conntrack_l4proto *l4proto)
> >> -{
> >> -#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);
> >> -#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);
> >> -#endif /* CONFIG_NF_CONNTRACK_PROC_COMPAT */
> >> -#endif /* CONFIG_SYSCTL */
> >> -}
> >> -
> >
> > Where did this function go?
>
>
> nf_ct_l4proto_unregister_sysctl just register sysctl,and we move this logic
> to the pernet_operations.init, so this function has no use.
I think I prefer if you add struct net *net to all those functions to
reduce the amount of changes in the patch.
Have a look per-net helper registration in this patch:
http://patchwork.ozlabs.org/patch/152096/
We needed to add a new sysctl to disable helper assignment. I made it
in a way that it supports per-net.
I'm pointing to that patch as example because I think it's similar to the
protocol registration. Before, the helper registration was not made
per-net at all.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists