diff mbox series

[iproute2-next,3/3] Improve batch times by caching link lookups

Message ID 20190107225552.8441-4-dsahern@kernel.org
State Changes Requested
Delegated to: David Ahern
Headers show
Series Improve batch times by caching link lookups | expand

Commit Message

David Ahern Jan. 7, 2019, 10:55 p.m. UTC 
From: David Ahern <dsahern@gmail.com>

ip route (for example) 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 + close 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. Update ll_name_to_index to use ll_link_get over
if_nametoindex.

With this change the time to install 720,022 routes with 2 ecmp nexthops
where the nexthop device is given is reduced from 30.7 seconds to 17.6
seconds.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 lib/ll_map.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

Comments

Stephen Hemminger Jan. 8, 2019, 12:05 a.m. UTC | #1 
On Mon,  7 Jan 2019 14:55:52 -0800
David Ahern <dsahern@kernel.org> wrote:

> +	idx = ll_link_get(name);
> +	if (idx == 0)
> +		idx = if_nametoindex(name);

What is advantage of using netlink, other than not having to open
a socket.
David Ahern Jan. 8, 2019, 12:28 a.m. UTC | #2 
On 1/7/19 5:05 PM, Stephen Hemminger wrote:
> On Mon,  7 Jan 2019 14:55:52 -0800
> David Ahern <dsahern@kernel.org> wrote:
> 
>> +	idx = ll_link_get(name);
>> +	if (idx == 0)
>> +		idx = if_nametoindex(name);
> 
> What is advantage of using netlink, other than not having to open
> a socket.
> 

As mentioned in my response to Eric, use of netlink is to keep the cache
entries in sync. The existing cache is populated by netlink messages;
this function should follow suit. if_nametoindex does not allow setting
the flags or type fields.
diff mbox series

Patch

diff --git a/lib/ll_map.c b/lib/ll_map.c
index 8e8a0b1e9c9d..46c78df08c11 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;