netdev - Re: [IPV4] ROUTE: ip_rt_dump() is unecessary slow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47831A37.5000607@cosmosbay.com>
Date:	Tue, 08 Jan 2008 07:37:43 +0100
From:	Eric Dumazet <dada1@...mosbay.com>
To:	David Miller <davem@...emloft.net>,
	Dipankar Sarma <dipankar@...ibm.com>
CC:	netdev@...r.kernel.org
Subject: Re: [IPV4] ROUTE: ip_rt_dump() is unecessary slow

David Miller a écrit :
> From: Eric Dumazet <dada1@...mosbay.com>
> Date: Tue, 08 Jan 2008 07:11:30 +0100
> 
>> @@ -288,15 +288,15 @@ static struct rtable *rt_cache_get_first(struct seq_file *seq)
>>  
>>  static struct rtable *rt_cache_get_next(struct seq_file *seq, struct rtable *r)
>>  {
>> -	struct rt_cache_iter_state *st = rcu_dereference(seq->private);
>> +	struct rt_cache_iter_state *st = seq->private;
>>  
> 
> Can you explain to me why this rcu_dereference() can be removed?

Very good question, but honestly I really dont see why it was there at the 
first place :

"struct seq_file" is private to this thread, so seq.private is
also private and cannot change while this thread runs rt_cache_get_next().

Reading it once (as guaranted bu rcu_dereference()) or several time if 
compiler really is dumb enough wont change the result...

> 
> The rest of your patch is OK and once I understand the above
> I'll add it to net-2.6, thanks!
> 

Maybe we can first have an Ack from Dipankar Sarma, I can repost the patch
with a more precise ChangeLog and wait his answer ?

[NET] ROUTE: fix rcu_dereference() uses in /proc/net/rt_cache

In rt_cache_get_next(), no need to guard seq->private by a rcu_dereference()
since seq is private to the thread running this function. Reading seq.private
once (as guaranted bu rcu_dereference()) or several time if compiler really is 
dumb enough wont change the result.

But we miss real spots where rcu_dereference() are needed, both in 
rt_cache_get_first() and rt_cache_get_next()

Signed-off-by: Eric Dumazet <dada1@...mosbay.com>

View attachment "route_rcu.patch" of type "text/plain" (1012 bytes)