Date: Wed, 26 Mar 2014 14:24:31 -0700 From: Nick Rogers <ncrogers@gmail.com> To: freebsd-hackers@freebsd.org Subject: arp(8) performance - use if_nameindex() instead of if_indextoname() Message-ID: <CAKOb=YaNqG3GBAomp%2Bm6ab%2B8vrdJ2s%2BApGXYM3GHHTYz83_XqA@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
This is in reference to an old thread in this list referenced here: http://www.mail-archive.com/freebsd-hackers@freebsd.org/msg157105.html Running arp -na from a shell can still take a very long time to complete on systems with thousands of VLAN interfaces and ARP entries. This is because if_indextoname() is an extremely heavy (slow) operation, and it is called for every ARP entry in src/usr.sbin/arp/arp.c. The following commit addressed a similar issue when many aliases were configured on a given interface, however as Adrian Chadd pointed out, does not help for cases where the interface name alternates between entries. This is exactly what happens when many VLANs interfaces exist, and each has only one IP alias. http://svnweb.freebsd.org/base?view=revision&revision=209063 I propose that instead of calling if_indextoname() for every entry, we leverage if_nameindex() to obtain a cache of if_index to if_name mappings, before printing all the entries. This results in a considerable performance improvement for my situation, and also handles the case that was "fixed" in the commit I just mentioned. I took a shot at this and came up with the following diff against HEAD. I used routed's route6d.c as a reference, which is the only thing I could find utilizing if_nameindex(). I am currently using this in production environments with great success. Index: usr.sbin/arp/arp.c =================================================================== --- arp.c (revision 263777) +++ arp.c (working copy) @@ -104,6 +104,8 @@ static time_t expire_time; static int flags, doing_proxy, proxy_only; +struct if_nameindex *ifnameindex; + /* which function we're supposed to do */ #define F_GET 1 #define F_SET 2 @@ -204,6 +206,9 @@ break; } +if (ifnameindex != NULL) + if_freenameindex(ifnameindex); + return (rtn); } @@ -557,13 +562,15 @@ /* * Display an arp entry */ -static char lifname[IF_NAMESIZE]; -static int64_t lifindex = -1; static void print_entry(struct sockaddr_dl *sdl, struct sockaddr_inarp *addr, struct rt_msghdr *rtm) { + + if (ifnameindex == NULL) + ifnameindex = if_nameindex(); + const char *host; struct hostent *hp; struct iso88025_sockaddr_dl_data *trld; @@ -595,12 +602,15 @@ } } else printf("(incomplete)"); - if (sdl->sdl_index != lifindex && - if_indextoname(sdl->sdl_index, lifname) != NULL) { - lifindex = sdl->sdl_index; - printf(" on %s", lifname); - } else if (sdl->sdl_index == lifindex) - printf(" on %s", lifname); + + struct if_nameindex *p; + for (p = ifnameindex; p && ifnameindex->if_index && ifnameindex->if_name; p++) { + if (p->if_index == sdl->sdl_index) { + printf(" on %s", p->if_name); + break; + } + } + if (rtm->rtm_rmx.rmx_expire == 0) printf(" permanent"); else { The following illustrates the performance improvement: [root@vm ~/arp]# ifconfig -a | grep vlan | grep interface | wc -l 1500 [root@vm ~/arp]# arp -na | wc -l 1503 [root@vm ~/arp]# time /usr/sbin/arp.old -na > /dev/null real 0m5.529s user 0m0.813s sys 0m4.231s [root@vm ~/arp]# time /usr/sbin/arp -na > /dev/null real 0m0.011s user 0m0.008s sys 0m0.002s [root@vm ~/arp]# I realize this may not be the cleanest way of implementing if_nameindex within arp.c. I'm hoping Max Laier or someone else can help me out (again) and get an adequate fix committed. Thanks! -Nick Rogers
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAKOb=YaNqG3GBAomp%2Bm6ab%2B8vrdJ2s%2BApGXYM3GHHTYz83_XqA>