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]

Message-Id: <20150304.001640.1653151786060996185.davem@davemloft.net>
Date:	Wed, 04 Mar 2015 00:16:40 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	alexander.h.duyck@...hat.com
Cc:	netdev@...r.kernel.org
Subject: Re: [net-next PATCH 0/6] ipv4/fib_trie: Cleanups to prepare for
 introduction of key vector

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;

Also, two comments:

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

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.