ip: defer lookup interface index

The ip command would always lookup the network device index
even when not necessary. This slows down operations like creating
lots of VLAN's.

David reported the original issue, this is an alternative patch
that solves it in a slightly more general method.

Using iproute2 to create a bridge and add 4094 vlans to it can take from
2 to 3 *minutes*. The reason is the extraneous call to ll_name_to_index.
ll_name_to_index results in an ioctl(SIOCGIFINDEX) call which in turn
invokes dev_load. If the index does not exist, which it won't when
creating a new link, dev_load calls modprobe twice -- once for
netdev-NAME and again for NAME. This is unnecessary overhead for each
link create.

When ip link is invoked for a new device, there is no reason to
call ll_name_to_index for the new device. With this patch, creating
a bridge and adding 4094 vlans takes less than 3 *seconds*.

	old:
	# time ip -batch ip-vlan.batch
	real    3m13.727s
	user    0m0.076s
	sys     0m1.959s

	new:
	# time ip -batch ip-vlan.batch
	real    0m3.222s
	user    0m0.044s
	sys     0m1.777s

Reported-off-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/ip/ip_common.h b/ip/ip_common.h
index 1b89795caa58fa26e5e48795094a40f086a51da8..49eb7d7bed401120d8f765d881d4589e0c73f555 100644 (file)
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -36,7 +36,7 @@ int print_addrlabel(const struct sockaddr_nl *who,
 int print_neigh(const struct sockaddr_nl *who,
                struct nlmsghdr *n, void *arg);
 int ipaddr_list_link(int argc, char **argv);
-void ipaddr_get_vf_rate(int, int *, int *, int);
+void ipaddr_get_vf_rate(int, int *, int *, const char *);
 void iplink_usage(void) __attribute__((noreturn));
 
 void iproute_reset_filter(int ifindex);
@@ -145,7 +145,7 @@ int lwt_parse_encap(struct rtattr *rta, size_t len, int *argcp, char ***argvp);
 void lwt_print_encap(FILE *fp, struct rtattr *encap_type, struct rtattr *encap);
 
 /* iplink_xdp.c */
-int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
+int xdp_parse(int *argc, char ***argv, struct iplink_req *req, const char *ifname,
              bool generic, bool drv, bool offload);
 void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
 

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 75539e057f6a7b11e70749192c4dd4689730bf1d..00da14c6f97c853f5b622e997b0e004065334eca 100644 (file)
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1967,14 +1967,20 @@ ipaddr_loop_each_vf(struct rtattr *tb[], int vfnum, int *min, int *max)
        exit(1);
 }
 
-void ipaddr_get_vf_rate(int vfnum, int *min, int *max, int idx)
+void ipaddr_get_vf_rate(int vfnum, int *min, int *max, const char *dev)
 {
        struct nlmsg_chain linfo = { NULL, NULL};
        struct rtattr *tb[IFLA_MAX+1];
        struct ifinfomsg *ifi;
        struct nlmsg_list *l;
        struct nlmsghdr *n;
-       int len;
+       int idx, len;
+
+       idx = ll_name_to_index(dev);
+       if (idx == 0) {
+               fprintf(stderr, "Device %s does not exist\n", dev);
+               exit(1);
+       }
 
        if (rtnl_wilddump_request(&rth, AF_UNSPEC, RTM_GETLINK) < 0) {
                perror("Cannot send dump request");

diff --git a/ip/iplink.c b/ip/iplink.c
index 22afe0221f3c8a109d2809ce4bb33cff9d6ec125..9ff5f692a1d4baeb7aa4e2b7d28ba18c077baeba 100644 (file)
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -242,9 +242,10 @@ static int iplink_have_newlink(void)
 }
 #endif /* ! IPLINK_IOCTL_COMPAT */
 
-static int nl_get_ll_addr_len(unsigned int dev_index)
+static int nl_get_ll_addr_len(const char *ifname)
 {
        int len;
+       int dev_index = ll_name_to_index(ifname);
        struct iplink_req req = {
                .n = {
                        .nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)),
@@ -259,6 +260,9 @@ static int nl_get_ll_addr_len(unsigned int dev_index)
        struct nlmsghdr *answer;
        struct rtattr *tb[IFLA_MAX+1];
 
+       if (dev_index == 0)
+               return -1;
+
        if (rtnl_talk(&rth, &req.n, &answer) < 0)
                return -1;
 
@@ -337,7 +341,7 @@ static void iplink_parse_vf_vlan_info(int vf, int *argcp, char ***argvp,
 }
 
 static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
-                          struct iplink_req *req, int dev_index)
+                          struct iplink_req *req, const char *dev)
 {
        char new_rate_api = 0, count = 0, override_legacy_rate = 0;
        struct ifla_vf_rate tivt;
@@ -373,7 +377,7 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
                NEXT_ARG();
                if (matches(*argv, "mac") == 0) {
                        struct ifla_vf_mac ivm = { 0 };
-                       int halen = nl_get_ll_addr_len(dev_index);
+                       int halen = nl_get_ll_addr_len(dev);
 
                        NEXT_ARG();
                        ivm.vf = vf;
@@ -542,7 +546,7 @@ static int iplink_parse_vf(int vf, int *argcp, char ***argvp,
                int tmin, tmax;
 
                if (tivt.min_tx_rate == -1 || tivt.max_tx_rate == -1) {
-                       ipaddr_get_vf_rate(tivt.vf, &tmin, &tmax, dev_index);
+                       ipaddr_get_vf_rate(tivt.vf, &tmin, &tmax, dev);
                        if (tivt.min_tx_rate == -1)
                                tivt.min_tx_rate = tmin;
                        if (tivt.max_tx_rate == -1)
@@ -583,7 +587,6 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
        int vf = -1;
        int numtxqueues = -1;
        int numrxqueues = -1;
-       int dev_index = 0;
        int link_netnsid = -1;
        int index = 0;
        int group = -1;
@@ -605,10 +608,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
                        if (check_ifname(*argv))
                                invarg("\"name\" not a valid ifname", *argv);
                        name = *argv;
-                       if (!dev) {
+                       if (!dev)
                                dev = name;
-                               dev_index = ll_name_to_index(dev);
-                       }
                } else if (strcmp(*argv, "index") == 0) {
                        NEXT_ARG();
                        if (index)
@@ -660,7 +661,7 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
                        bool offload = strcmp(*argv, "xdpoffload") == 0;
 
                        NEXT_ARG();
-                       if (xdp_parse(&argc, &argv, req, dev_index,
+                       if (xdp_parse(&argc, &argv, req, dev,
                                      generic, drv, offload))
                                exit(-1);
 
@@ -750,10 +751,10 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
 
                        vflist = addattr_nest(&req->n, sizeof(*req),
                                              IFLA_VFINFO_LIST);
-                       if (dev_index == 0)
+                       if (!dev)
                                missarg("dev");
 
-                       len = iplink_parse_vf(vf, &argc, &argv, req, dev_index);
+                       len = iplink_parse_vf(vf, &argc, &argv, req, dev);
                        if (len < 0)
                                return -1;
                        addattr_nest_end(&req->n, vflist);
@@ -916,7 +917,6 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
                        if (check_ifname(*argv))
                                invarg("\"dev\" not a valid ifname", *argv);
                        dev = *argv;
-                       dev_index = ll_name_to_index(dev);
                }
                argc--; argv++;
        }
@@ -931,8 +931,8 @@ int iplink_parse(int argc, char **argv, struct iplink_req *req, char **type)
        else if (!strcmp(name, dev))
                name = dev;
 
-       if (dev_index && addr_len) {
-               int halen = nl_get_ll_addr_len(dev_index);
+       if (dev && addr_len) {
+               int halen = nl_get_ll_addr_len(dev);
 
                if (halen >= 0 && halen != addr_len) {
                        fprintf(stderr,

diff --git a/ip/iplink_xdp.c b/ip/iplink_xdp.c
index 83826358aa227a5563ecf859d2bd61f5d79ea12b..dd4fd1fd3a3b1955e3e2dad81e59c1e8894897da 100644 (file)
--- a/ip/iplink_xdp.c
+++ b/ip/iplink_xdp.c
@@ -48,8 +48,8 @@ static int xdp_delete(struct xdp_req *xdp)
        return 0;
 }
 
-int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
-             bool generic, bool drv, bool offload)
+int xdp_parse(int *argc, char ***argv, struct iplink_req *req,
+             const char *ifname, bool generic, bool drv, bool offload)
 {
        struct bpf_cfg_in cfg = {
                .type = BPF_PROG_TYPE_XDP,
@@ -61,6 +61,8 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, __u32 ifindex,
        };
 
        if (offload) {
+               int ifindex = ll_name_to_index(ifname);
+
                if (!ifindex)
                        incomplete_command();
                cfg.ifindex = ifindex;