netdev - Re: [net-next PATCH 0/6] ipv4/fib_trie: Cleanups to prepare for introduction of key vector

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]

Date:	Wed, 04 Mar 2015 06:51:27 -0800
From:	Alexander Duyck <alexander.h.duyck@...hat.com>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org
Subject: Re: [net-next PATCH 0/6] ipv4/fib_trie: Cleanups to prepare for introduction
 of key vector


On 03/03/2015 09:16 PM, David Miller wrote:
> From: Alexander Duyck <alexander.h.duyck@...hat.com>
> Date: Mon, 02 Mar 2015 13:32:16 -0800
>
>> This patch series is meant to mostly just clean up the fib_trie to prepare
>> it for the introduction of the key_vector.  As such there are a number of
>> minor clean-ups such as reformatting the tnode to match the format once the
>> key vector is introduced, some optimizations to drop the need for a leaf
>> parent pointer, and some changes to remove duplication of effort such as
>> the 2 look-ups that were essentially being done per node insertion.
> This doesn't compile with trie stats enabled, I see something sneaking
> in from the main/local table collapsing patch :-)
>
> net/ipv4/fib_trie.c: In function ‘__trie_free_rcu’:
> net/ipv4/fib_trie.c:1601:36: error: ‘struct fib_table’ has no member named ‘data’
>    struct trie *t = (struct trie *)tb->data;

Ugh, sorry about that I will fix it this morning.

> Also, two comments:
>
> 1) When you go "if (idx >> n->bits)", can n->bits be == 32?  If so, this
>     expression is undefined.

If bits can be 32 then idx should be an unsigned long which is 64 bits.  
I will double check to make sure that is still the case.

The general idea is if a long is 32 bits then we likely cannot allocate 
a 32 bit tnode since that would more than consume all of the available 
memory in the system.  I can look into adding a comment to that effect 
somewhere.

> 2) In your simplification of fib_find_node(), don't keep storing over
>     and over again into the on-stack variable '*tp', instead just maintain
>     a one behind pointer in a local variable for the parent, and store it
>     one time as your exit the function.
>
> Thanks.

Okay I will update it.  As I recall that is what the compiler ends up 
doing anyway since fib_find_node ends up being inlined leaving the 
parent as just a local variable.

- Alex
--
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