netdev - Re: [PATCH] net: Improve perf of bond/vlans modification

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 17 Aug 2021 12:19:42 +0000
From:   Gilad Naaman <gnaaman@...venets.com>
To:     Nikolay Aleksandrov <nikolay@...dia.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "luwei32@...wei.com" <luwei32@...wei.com>,
        "wangxiongfeng2@...wei.com" <wangxiongfeng2@...wei.com>,
        "ap420073@...il.com" <ap420073@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: Improve perf of bond/vlans modification


> On 17 Aug 2021, at 14:39, Nikolay Aleksandrov <nikolay@...dia.com> wrote:
> 
> On 17/08/2021 14:04, Gilad Naaman wrote:
>> When a bond have a massive amount of VLANs with IPv6 addresses,
>> performance of changing link state, attaching a VRF, changing an IPv6
>> address, etc. go down dramtically.
>> 
>> The source of most of the slow down is the `dev_addr_lists.c` module,
>> which mainatins a linked list of HW addresses.
>> When using IPv6, this list grows for each IPv6 address added on a
>> VLAN, since each IPv6 address has a multicast HW address associated with
>> it.
>> 
>> When performing any modification to the involved links, this list is
>> traversed many times, often for nothing, all while holding the RTNL
>> lock.
>> 
>> Instead, this patch adds an auxilliary rbtree which cuts down
>> traversal time significantly.
>> 
> [snip]
>> Cc: David S. Miller <davem@...emloft.net>
>> Cc: Jakub Kicinski <kuba@...nel.org>
>> Cc: Lu Wei <luwei32@...wei.com>
>> Cc: Xiongfeng Wang <wangxiongfeng2@...wei.com>
>> Cc: Taehee Yoo <ap420073@...il.com>
>> Signed-off-by: Gilad Naaman <gnaaman@...venets.com>
>> ---
> 
> Hi Gilad,
> Generally I like the idea, I have a similar hacky patch for the same reason but related to bridge
> static entries which in some cases get added to lower device addr lists causing soft lockups due
> to the list traversals.
> 
> The patch should be targeted at net-next, more comments below…

Hi Nikolay,
Thanks for the review, I will retarget the patch.

>> +	/* Auxiliary tree for faster lookup when modifying the structure */
>> +	struct rb_root		tree_root;
> 
> Why keep the list when now we have the rbtree ?
> 
I was about to say that the list is used in RCU contexts,
but in retrospect rbtree can also be used with RCU, so there’s no real reason to keep the list.

I’ll work on implementing this, but I fear I don’t have ability to test all downstream
users of this module.

>> +
>> +	/* Insert node to hash table for quicker lookups during modification */
> 
> hash table?
> 
Oops, my earlier implantation is showing.