diff mbox series

[iproute2-next,2/3] ip link: Drop cache entry on name changes

Message ID 20190107225552.8441-3-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>

If a link's name is changed remove any entry from the link cache.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 ip/ip_common.h    |  3 ++-
 ip/iplink.c       | 10 ++++++++--
 ip/iplink_vxcan.c |  3 ++-
 ip/link_veth.c    |  3 ++-
 4 files changed, 14 insertions(+), 5 deletions(-)

Comments

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

> +int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type,
> +		 bool *name_change);

Not a real fan of adding another by reference return value flag.
It makes the logic flow more complex.

Is there another way? Caching in general is dicey anyway.
David Ahern Jan. 8, 2019, 12:26 a.m. UTC | #2 
On 1/7/19 5:06 PM, Stephen Hemminger wrote:
> On Mon,  7 Jan 2019 14:55:51 -0800
> David Ahern <dsahern@kernel.org> wrote:
> 
>> +int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type,
>> +		 bool *name_change);
> 
> Not a real fan of adding another by reference return value flag.
> It makes the logic flow more complex.
> 
> Is there another way? Caching in general is dicey anyway.
> 

The details of the link change are buried inside of the req argument. It
does not make sense to decode the message to see if the devices
referenced by ifi_index == IFLA_NAME.

The alternative is to just negate caching of ifi_index regardless of
what the request is doing.

If the caching only gained 5 or 10% we would not be having this
discussion - I would not have sent patches. The gain here is significant
and the caching aligns with the whole intent of batch files.
diff mbox series

Patch

diff --git a/ip/ip_common.h b/ip/ip_common.h
index d67575c63c24..ae2b2fb4e314 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -123,7 +123,8 @@  struct link_util {
 
 struct link_util *get_link_kind(const char *kind);
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type);
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type,
+		 bool *name_change);
 
 /* iplink_bridge.c */
 void br_dump_bridge_id(const struct ifla_bridge_id *id, char *buf, size_t len);
diff --git a/ip/iplink.c b/ip/iplink.c
index b5519201fef7..65838e2b3fe6 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -573,7 +573,8 @@  static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
 	return 0;
 }
 
-int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
+int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type,
+		 bool *name_change)
 {
 	char *name = NULL;
 	char *dev = NULL;
@@ -943,6 +944,8 @@  int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 		dev = name;
 	else if (!strcmp(name, dev))
 		name = dev;
+	else if (name_change)
+		*name_change = true;
 
 	if (dev && addr_len) {
 		int halen = nl_get_ll_addr_len(dev);
@@ -1030,9 +1033,10 @@  static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 		.n.nlmsg_type = cmd,
 		.i.ifi_family = preferred_family,
 	};
+	bool name_change = false;
 	int ret;
 
-	ret = iplink_parse(argc, argv, &req, &type);
+	ret = iplink_parse(argc, argv, &req, &type, &name_change);
 	if (ret < 0)
 		return ret;
 
@@ -1082,6 +1086,8 @@  static int iplink_modify(int cmd, unsigned int flags, int argc, char **argv)
 
 	if (rtnl_talk(&rth, &req.n, NULL) < 0)
 		return -2;
+	else if (name_change)
+		ll_drop_by_index(req.i.ifi_index);
 
 	return 0;
 }
diff --git a/ip/iplink_vxcan.c b/ip/iplink_vxcan.c
index 8b08c9a70c65..bdd372037836 100644
--- a/ip/iplink_vxcan.c
+++ b/ip/iplink_vxcan.c
@@ -56,7 +56,8 @@  static int vxcan_parse_opt(struct link_util *lu, int argc, char **argv,
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type,
+			   NULL);
 	if (err < 0)
 		return err;
 
diff --git a/ip/link_veth.c b/ip/link_veth.c
index 33e8f2b102e7..ef0bc1948323 100644
--- a/ip/link_veth.c
+++ b/ip/link_veth.c
@@ -54,7 +54,8 @@  static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
 
 	n->nlmsg_len += sizeof(struct ifinfomsg);
 
-	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type);
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)n, &type,
+			   NULL);
 	if (err < 0)
 		return err;