[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 28 Jan 2014 16:13:44 +0800
From: Wang Weidong <wangweidong1@...wei.com>
To: Neil Horman <nhorman@...driver.com>
CC: <davem@...emloft.net>, <vyasevich@...il.com>,
<dborkman@...hat.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH 0/2] sctp: fix a problem with net_namespace
On 2014/1/27 19:49, Neil Horman wrote:
> On Mon, Jan 27, 2014 at 11:49:01AM +0800, Wang Weidong wrote:
>> fix a problem with net_namespace, and optimize
>> the sctp_sysctl_net_register.
>>
>> Wang Weidong (2):
>> sctp: fix a missed .data initialization
>> sctp: optimize the sctp_sysctl_net_register
>>
>> net/sctp/sysctl.c | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> --
>> 1.7.12
>>
>>
>>
> I don't see that either of these patches are needed. In sctp_init_net, the
> sctp_hmac_alg pointer gets initalized before calling sctp_sysctl_net_register,
> and sctp_proc_do_hmac_alg is written to specifically expect NULL values, so this
> code may change behavior regarding default cookie selection.
>
Hi Neil,
Here, I think the sctp_proc_do_hmac_alg will be called only when we change the
/proc/sys/net/cookie_hmac_alg. So add the .data won't effect the default value.
and the data isn't equal to the "cookie_hmac_alg"?
> This was coded so that poniters to entires in the string table could be used,
> rather than needing to allocate or maintain character buffers. That said, it
> does look like that for loop in sctp_sysctl_register_table might compute an odd
> offset when cloning the table. I think the right fix for that is likely to just
> move the sysctl value initalization in sctp_init_net to below the sysctl
> register function.
>
> Neil
>
I found the problem is that:
I use "ip netns add netns1/netns2"
In any netns(netns1 or netns2 or init_net) when I change the value of the entry
such as "addip_enable" "max_autoclose" which after the cookie_hmac_alg (contain it),
and the other netns will be effected.
In sctp_sysctl_net_register, kmemdup does cloning the table. The offset of netns1 and
init_net's clt_table.data is the same as two netns offset. So the for(){...} would do
add the offset for every clt_table.data.
The code:
for (i = 0; table[i].data; i++)
table[i].data += (char *)(&net->sctp) - (char *)&init_net.sctp;
And I add a pr_info into the for(){...} in sctp_sysctl_net_register and only print 7 times for each ns.
7 is the index of "cookie_preserve_enable" which before the "cookie_hmac_alg".
As the "cookie_hmac_alg" data is NULL, so we can't add offset to the rest, and all the netns use the same
address of clt_table entry after the "cookie_hmac_alg".
So I think only "move the sysctl value initalization in sctp_init_net to below the sysctl
register function" won't solve the problem, because the problem is at the for() {...}.
Is there something wrong?
The next patch is that, the sctp_net_table is for init_net, So when load the sctp module, we needn't
to do the cloning tables for init_net again. And I found the ipv4 do it in the same way.
I have a doubt : how can I rmmod the sctp? only add '-f' ? If I do "rmmod -f sctp", I will get the
log by dmesg: "Disabling lock debugging due to kernel taint"
Regards,
Wang
>
>
--
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