netdev - Re: [PATCH iproute2-next] Improve batch times by caching link lookups

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:   Mon, 7 Jan 2019 14:16:06 -0700
From:   David Ahern <dsahern@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        David Ahern <dsahern@...nel.org>, stephen@...workplumber.org
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next] Improve batch times by caching link lookups

On 1/7/19 2:06 PM, Eric Dumazet wrote:
> 
> 
> On 01/07/2019 12:58 PM, David Ahern wrote:
>> On 1/7/19 1:57 PM, Eric Dumazet wrote:
>>>
>>>
>>> On 01/07/2019 12:41 PM, David Ahern wrote:
>>>> From: David Ahern <dsahern@...il.com>
>>>>
>>>> ip route uses ll_name_to_index to convert the user given device name to an
>>>> index. At the moment ll_name_to_index uses if_nametoindex which is ioctl
>>>> based and does not cache the result. When using a batch file this means
>>>> the same device lookups can be done repeatedly adding unnecessary overhead
>>>> (socket + ioctl call for each device lookup).
>>>>
>>>> Add a new function, ll_link_get, to send a netlink based RTM_GETLINK. If
>>>> successful, cache the result in idx_head and name_head so future lookups
>>>> can re-use the entry.
>>>>
>>>> With this change the time to install routes via a batch file is reduced
>>>> from 30.7 seconds to 17.6 seconds (720,022 routes with 2 ecmp nexthops
>>>> where the nexthop device is given).
>>>>
>>>
>>> What time increase if we have 10,000 devices, and install one route ?
>>>
>>> Caching 10,000 devices would be quite a waste.
>>>
>>
>> It only lookups up the device if ll_name_to_index is invoked for the
>> device.
>>
>> This is NOT a link dump and cache; it only adds lookups to the cache.
>>
> 
> I see, maybe split this in two patches to avoid the confusion.
> 
> Why is the RTNETLINK approach better than ioctl() ?

I could very well cache the if_nametoindex result but that is only name
<--> idx. To avoid making assumptions on how the ll_map code is used in
the future, RTM_GETLINK is used to leverage ll_remember_index and have
the cache entries always be consistently filled.

> 
> If this is the case, maybe if_nametoindex() should be fixed, instead of carrying this locally in iproute2.
> 

if_nametoindex has to do socket() - ioctl() - close() for every device
lookup. It's not a bug and caching the socket descriptor makes
assumptions that libc should not be making.