On Wed, Jun 27, 2012 at 09:38:44AM +0800, Gao feng wrote:
> 于 2012年06月26日 22:36, Pablo Neira Ayuso 写道:
> > On Tue, Jun 26, 2012 at 11:40:14AM +0800, Gao feng wrote:
> >> Hi Pablo:
> >>
> >> 于 2012年06月25日 19:12, Pablo Neira Ayuso 写道:
> >>> On Thu, Jun 21, 2012 at 10:36:38PM +0800, Gao feng wrote:
> >>>> before commit 2c352f444ccfa966a1aa4fd8e9ee29381c467448
> >>>> (netfilter: nf_conntrack: prepare namespace support for
> >>>> l4 protocol trackers), we register sysctl before register
> >>>> protos, so if sysctl is registered faild, the protos will
> >>>> not be registered.
> >>>>
> >>>> but now, we register protos first, and when register
> >>>> sysctl failed, we can use protos too, it's different
> >>>> from before.
> >>>
> >>> No, this has to be an all-or-nothing game. If one fails, everything
> >>> else that you've registered has to be unregistered.
> >>
> >> indeed,this is an all-or-nothing game right now,please look at the ipv4_net_init,
> >> when we register nf_conntrack_l3proto_ipv4 failed,we will unregister the already
> >> registered l4protoes, and in nf_conntrack_l4proto_unregister,we will call
> >> nf_ct_l4proto_unregister_sysctl to free the sysctl table.
> >
> > I see proto->init_net allocates in->ctl_table, then
> > nf_ct_l3proto_register_sysctl release it if it fails. I got confused
> > because I did not see where that memory was being freed. Then, it's
> > good.
> >
> > Still one more thing:
> >
> >>>> so change to register sysctl before register protos.
> >>>>
> >>>> Signed-off-by: Gao feng <gaofeng@xxxxxxxxxxxxxx>
> >>>> ---
> >>>> net/netfilter/nf_conntrack_proto.c | 36 +++++++++++++++++++++++-------------
> >>>> 1 files changed, 23 insertions(+), 13 deletions(-)
> >>>>
> >>>> diff --git a/net/netfilter/nf_conntrack_proto.c b/net/netfilter/nf_conntrack_proto.c
> >>>> index 1ea9194..9bd88aa 100644
> >>>> --- a/net/netfilter/nf_conntrack_proto.c
> >>>> +++ b/net/netfilter/nf_conntrack_proto.c
> >>>> @@ -253,18 +253,23 @@ int nf_conntrack_l3proto_register(struct net *net,
> >>>> {
> >>>> int ret = 0;
> >>>>
> >>>> - if (net == &init_net)
> >>>> - ret = nf_conntrack_l3proto_register_net(proto);
> >>>> + if (proto->init_net) {
> >
> > I think proto->init_net has to be mandatory since all protocol support
> > pernet already. We can add BUG_ON at the beginning of the function if
> > proto->init_net is not defined.
> >
>
> we can add BUG_ON at nf_conntrack_l4proto_register,because all of the l4protoes
> have the init_net function.
>
> BUT nf_conntrack_l3proto_ipv6 doesn't have init_net function,because this proto
> doesn't have pernet data, and nf_conntrack_l3proto_ipv4 has pernet data only when
> CONFIG_NF_CONNTRACK_PROC_COMPAT is configured.
OK, thanks for the clarification.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
