于 2012年09月26日 17:58, Pablo Neira Ayuso 写道:
> On Wed, Sep 26, 2012 at 05:42:31PM +0800, Gao feng wrote:>> Hi Pablo:>>>> 于 2012年09月26日 17:26, Pablo Neira Ayuso 写道:>>> On Wed, Sep 26, 2012 at 04:41:21PM +0800, Gao feng wrote:>>>> use proper netlink_dump_control.done and .module to avoid panic.>>>>>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>>>>> --->>>> net/netfilter/nf_conntrack_netlink.c | 8 ++++++++>>>> 1 files changed, 8 insertions(+), 0 deletions(-)>>>>>>>> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c>>>> index 9807f32..509a257 100644>>>> --- a/net/netfilter/nf_conntrack_netlink.c>>>> +++ b/net/netfilter/nf_conntrack_netlink.c>>>> @@ -706,6 +706,7 @@ static int ctnetlink_done(struct netlink_callback *cb)>>>> nf_ct_put((struct nf_conn *)cb->args[1]);>>>> if (cb->data)>>>> kfree(cb->data);>>>> + netlink_dump_done(cb);>>>>>> I think you can call netlink_dump_done from af_netlink.c:>>>>>> static int netlink_dump(struct sock *sk) >>> ...>>> if (cb->done) {>>> cb->done(cb);>>> netlink_dump_done(...);>>> }>>>>>> Thus, you don't need to change netlink_dump_control in every netlink>>> subsystem.>>>> because cb->done is called by netlink_sock_destruct too,it's very usefully>> when userspace program only send dump request to kernel without reading>> data from kernel.> > Then add that to netlink_sock_destruct as well. If possible, I prefer> if this remains in the netlink core to avoid leaking module refcount> if you forget to call netlink_dump_done.
make sense,I will update it in next version.
Thanks!
> >>>>>>> return 0;>>>> }>>>> >>>> @@ -1022,6 +1023,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,>>>> struct netlink_dump_control c = {>>>> .dump = ctnetlink_dump_table,>>>> .done = ctnetlink_done,>>>> + .module = THIS_MODULE,>>>>>> You can do something similar to:>>>>>> 9f00d97 netlink: hide struct module parameter in netlink_kernel_create>>>>>> by definiting netlink_dump_start as static inline and using>>> THIS_MODULE from there.>>>>>> If I'm not missing anything, with those two changes, you will not need>>> to modify any caller and it will result one single patch.>>>>>>> You can see the patch [11/11], THIS_MODULE in infiniband/core/cma.c>> means module rdma_cm,but we call netlink_dump_start in infiniband/core/netlink.c> > You can still use __netlink_dump_start for that case, which allows you> to specify a custom struct module * parameter. But for most cases,> netlink_dump_start (which hides THIS_MODULE) should be fine.>
I don't know how to deal with module_put in this way.
and I think my way is simple enough.
--
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

于 2012年09月26日 23:04, Pablo Neira Ayuso 写道:
> On Wed, Sep 26, 2012 at 08:35:53PM +0800, Gao feng wrote:>> 于 2012年09月26日 17:58, Pablo Neira Ayuso 写道:>>> On Wed, Sep 26, 2012 at 05:42:31PM +0800, Gao feng wrote:>>>> Hi Pablo:>>>>>>>> 于 2012年09月26日 17:26, Pablo Neira Ayuso 写道:>>>>> On Wed, Sep 26, 2012 at 04:41:21PM +0800, Gao feng wrote:>>>>>> use proper netlink_dump_control.done and .module to avoid panic.>>>>>>>>>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>>>>>>> --->>>>>> net/netfilter/nf_conntrack_netlink.c | 8 ++++++++>>>>>> 1 files changed, 8 insertions(+), 0 deletions(-)>>>>>>>>>>>> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c>>>>>> index 9807f32..509a257 100644>>>>>> --- a/net/netfilter/nf_conntrack_netlink.c>>>>>> +++ b/net/netfilter/nf_conntrack_netlink.c>>>>>> @@ -706,6 +706,7 @@ static int ctnetlink_done(struct netlink_callback *cb)>>>>>> nf_ct_put((struct nf_conn *)cb->args[1]);>>>>>> if (cb->data)>>>>>> kfree(cb->data);>>>>>> + netlink_dump_done(cb);>>>>>>>>>> I think you can call netlink_dump_done from af_netlink.c:>>>>>>>>>> static int netlink_dump(struct sock *sk) >>>>> ...>>>>> if (cb->done) {>>>>> cb->done(cb);>>>>> netlink_dump_done(...);>>>>> }>>>>>>>>>> Thus, you don't need to change netlink_dump_control in every netlink>>>>> subsystem.>>>>>>>> because cb->done is called by netlink_sock_destruct too,it's very usefully>>>> when userspace program only send dump request to kernel without reading>>>> data from kernel.>>>>>> Then add that to netlink_sock_destruct as well. If possible, I prefer>>> if this remains in the netlink core to avoid leaking module refcount>>> if you forget to call netlink_dump_done.>>>> make sense, I will update it in next version.>> Thanks!> > Great. Remove also netlink_dump_done and just use module_put instead> after cb->done. That new function you added is so small that there is> no way to justify its addition.> >>>>>>>>>>>>>> return 0;>>>>>> }>>>>>> >>>>>> @@ -1022,6 +1023,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,>>>>>> struct netlink_dump_control c = {>>>>>> .dump = ctnetlink_dump_table,>>>>>> .done = ctnetlink_done,>>>>>> + .module = THIS_MODULE,>>>>>>>>>> You can do something similar to:>>>>>>>>>> 9f00d97 netlink: hide struct module parameter in netlink_kernel_create>>>>>>>>>> by definiting netlink_dump_start as static inline and using>>>>> THIS_MODULE from there.>>>>>>>>>> If I'm not missing anything, with those two changes, you will not need>>>>> to modify any caller and it will result one single patch.>>>>>>>>>>>>> You can see the patch [11/11], THIS_MODULE in infiniband/core/cma.c>>>> means module rdma_cm,but we call netlink_dump_start in infiniband/core/netlink.c>>>>>> You can still use __netlink_dump_start for that case, which allows you>>> to specify a custom struct module * parameter. But for most cases,>>> netlink_dump_start (which hides THIS_MODULE) should be fine.>>>> I don't know how to deal with module_put in this way.>> and I think my way is simple enough.> > Not sure what problem with module_put you're refering to.>
forget this,I'm wrong.
> I think you can make this patchset way smaller with the change I'm> proposing.>
I know,but I choose simple and directviewing, not smaller.
I think these two function will make people confuse.
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