* [PATCH iproute2-next] Improve batch times by caching link lookups
@ 2019-01-07 20:41 David Ahern
2019-01-07 20:57 ` Eric Dumazet
2019-01-07 21:31 ` Stephen Hemminger
0 siblings, 2 replies; 7+ messages in thread
From: David Ahern @ 2019-01-07 20:41 UTC (permalink / raw)
To: stephen; +Cc: netdev, David Ahern
From: David Ahern <dsahern@gmail.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).
Signed-off-by: David Ahern <dsahern@gmail.com>
---
lib/ll_map.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)
diff --git a/lib/ll_map.c b/lib/ll_map.c
index 1ab8ef0758ac..4fbd9fa70dd7 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -192,6 +192,46 @@ int ll_index_to_flags(unsigned idx)
return im ? im->flags : -1;
}
+static int ll_link_get(const char *name)
+{
+ struct {
+ struct nlmsghdr n;
+ struct ifinfomsg ifm;
+ char buf[1024];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETLINK,
+ };
+ __u32 filt_mask = RTEXT_FILTER_VF | RTEXT_FILTER_SKIP_STATS;
+ struct rtnl_handle rth = {};
+ struct nlmsghdr *answer;
+ int rc = 0;
+
+ if (rtnl_open(&rth, 0) < 0)
+ return 0;
+
+ addattr32(&req.n, sizeof(req), IFLA_EXT_MASK, filt_mask);
+ addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name,
+ strlen(name) + 1);
+
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
+ goto out;
+
+ /* add entry to cache */
+ rc = ll_remember_index(answer, NULL);
+ if (!rc) {
+ struct ifinfomsg *ifm = NLMSG_DATA(answer);
+
+ rc = ifm->ifi_index;
+ }
+
+ free(answer);
+out:
+ rtnl_close(&rth);
+ return rc;
+}
+
unsigned ll_name_to_index(const char *name)
{
const struct ll_cache *im;
@@ -204,7 +244,9 @@ unsigned ll_name_to_index(const char *name)
if (im)
return im->index;
- idx = if_nametoindex(name);
+ idx = ll_link_get(name);
+ if (idx == 0)
+ idx = if_nametoindex(name);
if (idx == 0)
idx = ll_idx_a2n(name);
return idx;
--
2.11.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 20:41 [PATCH iproute2-next] Improve batch times by caching link lookups David Ahern
@ 2019-01-07 20:57 ` Eric Dumazet
2019-01-07 20:58 ` David Ahern
2019-01-07 21:31 ` Stephen Hemminger
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-01-07 20:57 UTC (permalink / raw)
To: David Ahern, stephen; +Cc: netdev, David Ahern
On 01/07/2019 12:41 PM, David Ahern wrote:
> From: David Ahern <dsahern@gmail.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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 20:57 ` Eric Dumazet
@ 2019-01-07 20:58 ` David Ahern
2019-01-07 21:06 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2019-01-07 20:58 UTC (permalink / raw)
To: Eric Dumazet, David Ahern, stephen; +Cc: netdev
On 1/7/19 1:57 PM, Eric Dumazet wrote:
>
>
> On 01/07/2019 12:41 PM, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 20:58 ` David Ahern
@ 2019-01-07 21:06 ` Eric Dumazet
2019-01-07 21:16 ` David Ahern
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-01-07 21:06 UTC (permalink / raw)
To: David Ahern, David Ahern, stephen; +Cc: netdev
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@gmail.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() ?
If this is the case, maybe if_nametoindex() should be fixed, instead of carrying this locally in iproute2.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 21:06 ` Eric Dumazet
@ 2019-01-07 21:16 ` David Ahern
0 siblings, 0 replies; 7+ messages in thread
From: David Ahern @ 2019-01-07 21:16 UTC (permalink / raw)
To: Eric Dumazet, David Ahern, stephen; +Cc: netdev
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@gmail.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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 20:41 [PATCH iproute2-next] Improve batch times by caching link lookups David Ahern
2019-01-07 20:57 ` Eric Dumazet
@ 2019-01-07 21:31 ` Stephen Hemminger
2019-01-07 21:52 ` David Ahern
1 sibling, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-01-07 21:31 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, David Ahern
On Mon, 7 Jan 2019 12:41:30 -0800
David Ahern <dsahern@kernel.org> wrote:
> From: David Ahern <dsahern@gmail.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).
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
What if a ip command in the batch does a rename?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iproute2-next] Improve batch times by caching link lookups
2019-01-07 21:31 ` Stephen Hemminger
@ 2019-01-07 21:52 ` David Ahern
0 siblings, 0 replies; 7+ messages in thread
From: David Ahern @ 2019-01-07 21:52 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern; +Cc: netdev
On 1/7/19 2:31 PM, Stephen Hemminger wrote:
>
> What if a ip command in the batch does a rename?
>
Simplest action (meaning least overhead for non-batch) is to drop any
entry from the cache. If a later command needs it, the cache entry can
be re-created.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-07 21:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-07 20:41 [PATCH iproute2-next] Improve batch times by caching link lookups David Ahern
2019-01-07 20:57 ` Eric Dumazet
2019-01-07 20:58 ` David Ahern
2019-01-07 21:06 ` Eric Dumazet
2019-01-07 21:16 ` David Ahern
2019-01-07 21:31 ` Stephen Hemminger
2019-01-07 21:52 ` David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.