Subject: Re: Updating arp(8) to use getifaddrs (cf. bin/8566)
To: Rafal Boni <rafal@attbi.com>
From: Luke Mewburn <lukem@netbsd.org>
List: tech-net
Date: 11/07/2002 19:52:43
On Thu, Nov 07, 2002 at 02:12:04AM -0500, Rafal Boni wrote:
  | Folks:	
  | 	Many moons ago, when I added the ability for arp(8) to print the
  | 	names of the interfaces particular addresses were associated with,
  | 	I did it the hard way (but probably the only way that would have
  | 	worked accross the NetBSD versions I was then interested in) by
  | 	using the SIOCGIFCONF and walking the twisty maze of packed data
  | 	returned by it.
  | 	
  | 	When I made the changes, I promised to update them to use the
  | 	getifaddrs() interface; I finally got around to doing that just
  | 	now in a fit of insomnia.  
  | 
  | 	The change is appended (as expected, it's mostly code removal),
  | 	but I have a portability question -- is conversion this going
  | 	to be more of a portability issue due to the non-standardized
  | 	nature of getifaddrs(3), or should I "Just Do it" (yes to both
  | 	is acceptable as well 8-).

"Just do it".

getifaddrs(3) is more portable/scalable than SIOCGIFCONF, in that you
can always provide a version of getifaddrs(3) that uses whatever
underlying operating system method (e.g, use SIOCGIFCONF, frob the
routing socket, grovel kmem, etc...) to get this information and
return it in a single consistent format.

The problem with SIOCGIFCONF as used by most programs is that it
doesn't cater for when the number of interfaces * sizeof(struct ifreq)
(== 32) exceeds the buffer size passed in, so you might get truncated
results.  There is a way to detect and work around this, except non
of the programs in usr.sbin/* that use SIOCGIFCONF do this, and won't
work correctly if the buffer size is exceeded.
(I checked various programs in usr.sbin which use SIOCGIFCONF, and
they support between 10, 16, 32, 256 (or 1 case, 512) addresses.)

IIRC, SIOCGIFCONF also can't support sockaddr_in6 addresses, or any
other sockaddr > 16 bytes long...


I see that a bunch of other programs (mainly in usr.sbin) haven't been
converted yet from SIOCGIFCONF to the getifaddrs(3), so if you're
feeling enthusiastic ... :-)



  | Thanks,
  | --rafal
  | 
  | Index: arp.c
  | ===================================================================
  | RCS file: /cvsroot/basesrc/usr.sbin/arp/arp.c,v
  | retrieving revision 1.34
  | diff -b -u -p -r1.34 arp.c
  | --- arp.c	2002/03/02 03:45:07	1.34
  | +++ arp.c	2002/11/07 07:01:23
  | @@ -82,6 +82,7 @@ __RCSID("$NetBSD: arp.c,v 1.34 2002/03/0
  |  #include <stdlib.h>
  |  #include <string.h>
  |  #include <unistd.h>
  | +#include <ifaddrs.h>
  |  
  |  int	delete __P((const char *, const char *));
  |  void	dump __P((u_long));
  | @@ -101,11 +102,8 @@ void	usage __P((void));
  |  static int pid;
  |  static int aflag, nflag, vflag;
  |  static int s = -1;
  | -static int is = -1;
  | +static struct ifaddrs* ifaddrs = NULL;
  |  
  | -static struct ifconf ifc;
  | -static char ifconfbuf[8192];
  | -
  |  int
  |  main(argc, argv)
  |  	int argc;
  | @@ -663,48 +661,24 @@ getifname(ifindex, ifname)
  |  	u_int16_t ifindex;
  |  	char* ifname;
  |  {
  | -	int i, idx, siz;
  | -	char ifrbuf[8192];
  | -	struct ifreq ifreq, *ifr;
  | +	int i;
  | +	struct ifaddrs *addr;
  |  	const struct sockaddr_dl *sdl = NULL;
  | -
  | -	if (is < 0) {
  | -		is = socket(PF_INET, SOCK_DGRAM, 0);
  | -		if (is < 0)
  | -			err(1, "socket");
  |  
  | -		ifc.ifc_len = sizeof(ifconfbuf);
  | -		ifc.ifc_buf = ifconfbuf;
  | -
  | -		if (ioctl(is, SIOCGIFCONF, &ifc) < 0) {
  | -			close(is);
  | -			err(1, "SIOCGIFCONF");
  | -			is = -1;
  | -		}
  | +	if (ifaddrs == NULL) {
  | +		i = getifaddrs(&ifaddrs);
  | +		if (i != 0)
  | +			err(1, "getifaddrs");
  |  	}
  | -
  | -	ifr = ifc.ifc_req;
  | -	ifreq.ifr_name[0] = '\0';
  | -	for (i = 0, idx = 0; i < ifc.ifc_len; ) {
  | -		ifr = (struct ifreq *)((caddr_t)ifc.ifc_req + i);
  | -		siz = sizeof(ifr->ifr_name) +
  | -			(ifr->ifr_addr.sa_len > sizeof(struct sockaddr)
  | -				? ifr->ifr_addr.sa_len
  | -				: sizeof(struct sockaddr));
  | -		i += siz;
  | -		/* avoid alignment issue */
  | -		if (sizeof(ifrbuf) < siz)
  | -			errx(1, "ifr too big");
  | -
  | -		memcpy(ifrbuf, ifr, siz);
  | -		ifr = (struct ifreq *)ifrbuf;
  |  
  | -		if (ifr->ifr_addr.sa_family != AF_LINK)
  | +	for (addr = ifaddrs; addr; addr = addr->ifa_next) {
  | +		if (addr->ifa_addr == NULL || 
  | +		    addr->ifa_addr->sa_family != AF_LINK)
  |  			continue;
  |  
  | -		sdl = (const struct sockaddr_dl *) &ifr->ifr_addr;
  | +		sdl = (const struct sockaddr_dl *) addr->ifa_addr;
  |  		if (sdl && sdl->sdl_index == ifindex) {
  | -			(void) strncpy(ifname, ifr->ifr_name, IFNAMSIZ);
  | +			(void) strncpy(ifname, addr->ifa_name, IFNAMSIZ);
  |  			return 0;
  |  		}
  |  	}
  | ----
  | Rafal Boni                                                     rafal@attbi.com
  |   We are all worms.  But I do believe I am a glowworm.  -- Winston Churchill