From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Gao feng <gaofeng@cn.fujitsu.com>
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
ebiederm@xmission.com, serge.hallyn@canonical.com,
dlezcano@fr.ibm.com
Subject: Re: [PATCH 02/12] netfilter: don't register sysctl when register proto
Date: Tue, 17 Apr 2012 13:26:34 +0200 [thread overview]
Message-ID: <20120417112634.GA2915@1984> (raw)
In-Reply-To: <4F8D4535.1040509@cn.fujitsu.com>
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@cn.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 netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-04-17 11:26 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-17 2:56 [PATCH 00/12] add namespace support for netfilter protos Gao feng
2012-04-17 2:56 ` [PATCH 01/12] netfilter: add struct netns_ct_proto to support netfilter namespace Gao feng
2012-04-17 8:54 ` Pablo Neira Ayuso
2012-04-17 2:56 ` [PATCH 02/12] netfilter: don't register sysctl when register proto Gao feng
2012-04-17 8:56 ` Pablo Neira Ayuso
2012-04-17 10:25 ` Gao feng
2012-04-17 11:26 ` Pablo Neira Ayuso [this message]
2012-04-17 2:56 ` [PATCH 03/12] netfilter: generic proto sysctl support for net namespace Gao feng
2012-04-17 8:58 ` Pablo Neira Ayuso
2012-04-17 10:22 ` Gao feng
2012-04-17 11:35 ` Pablo Neira Ayuso
2012-04-18 0:20 ` Gao feng
2012-04-17 2:56 ` [PATCH 04/12] netfilter: tcp " Gao feng
2012-04-17 2:56 ` [PATCH 05/12] netfilter: udp " Gao feng
2012-04-17 2:56 ` [PATCH 06/12] netfilter: icmp " Gao feng
2012-04-17 2:56 ` [PATCH 07/12] netfilter: icmpv6 proto sysctl support for net Gao feng
2012-04-17 2:56 ` [PATCH 08/12] netfilter: ipv4 sysctl support for net namespace Gao feng
2012-04-17 2:56 ` [PATCH 09/12] netfilter: ipv6 " Gao feng
2012-04-17 2:56 ` [PATCH 10/12] netfilter: sctp proto " Gao feng
2012-04-17 10:30 ` Gao feng
2012-04-17 11:29 ` Pablo Neira Ayuso
2012-04-17 2:56 ` [PATCH 11/12] netfilter: udplite proto sysctl support for net Gao feng
2012-04-17 2:56 ` [PATCH 12/12] netfilter: export necessary function for generic proto Gao feng
2012-04-17 9:01 ` Pablo Neira Ayuso
2012-04-17 8:52 ` [PATCH 00/12] add namespace support for netfilter protos Pablo Neira Ayuso
2012-04-17 10:12 ` Gao feng
2012-04-17 10:34 ` Jan Engelhardt
2012-04-17 10:59 ` Pablo Neira Ayuso
2012-04-17 14:35 ` Serge Hallyn
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=20120417112634.GA2915@1984 \
--to=pablo@netfilter.org \
--cc=dlezcano@fr.ibm.com \
--cc=ebiederm@xmission.com \
--cc=gaofeng@cn.fujitsu.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=serge.hallyn@canonical.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.