From owner-freebsd-net@FreeBSD.ORG Mon Jan 26 13:22:27 2004 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D9EDB16A4D0 for ; Mon, 26 Jan 2004 13:22:27 -0800 (PST) Received: from odin.ac.hmc.edu (Odin.AC.HMC.Edu [134.173.32.75]) by mx1.FreeBSD.org (Postfix) with ESMTP id D3DF743D1D for ; Mon, 26 Jan 2004 13:22:18 -0800 (PST) (envelope-from brdavis@odin.ac.hmc.edu) Received: from odin.ac.hmc.edu (IDENT:brdavis@localhost.localdomain [127.0.0.1]) by odin.ac.hmc.edu (8.12.10/8.12.3) with ESMTP id i0QLMDaT000975; Mon, 26 Jan 2004 13:22:13 -0800 Received: (from brdavis@localhost) by odin.ac.hmc.edu (8.12.10/8.12.3/Submit) id i0QLMDKv000972; Mon, 26 Jan 2004 13:22:13 -0800 Date: Mon, 26 Jan 2004 13:22:12 -0800 From: Brooks Davis To: net@freebsd.org Message-ID: <20040126212212.GA30225@Odin.AC.HMC.Edu> References: <20040123200238.GA3133@Odin.AC.HMC.Edu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040123200238.GA3133@Odin.AC.HMC.Edu> User-Agent: Mutt/1.5.4i X-Virus-Scanned: by amavisd-milter (http://amavis.org/) on odin.ac.hmc.edu Subject: Re: review request: interface renaming patch X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Jan 2004 21:22:28 -0000 On Fri, Jan 23, 2004 at 12:02:38PM -0800, Brooks Davis wrote: > The patch is split into cleanups that apply to the tree regardless of > this functional change and the actual functional changes. You will need > to use "patch -p2" to apply the patch due to they way I generated it > from my perforce trees. Here's a new version of the patch. Following a suggestion from Vincent Jardin, I announce the departure of the interface before renaming it and announce its arrival afterwards. I've also added some documentation for SIOCSIFNAME to ifnet.9. -- Brooks *** Cleanup diffs *** --- ../freebsd/sbin/ifconfig/ifconfig.c Wed Oct 29 10:24:27 2003 +++ ../cleanup/sbin/ifconfig/ifconfig.c Fri Jan 23 10:44:54 2004 @@ -113,7 +113,7 @@ struct sockaddr_in netmask; struct netrange at_nr; /* AppleTalk net range */ -char name[32]; +char name[IFNAMSIZ]; int flags; int setaddr; int setipdst; @@ -596,8 +596,9 @@ addrcount++; next += nextifm->ifm_msglen; } - strncpy(name, sdl->sdl_data, sdl->sdl_nlen); - name[sdl->sdl_nlen] = '\0'; + strlcpy(name, sdl->sdl_data, + sizeof(name) <= sdl->sdl_nlen ? + sizeof(name) : sdl->sdl_nlen + 1); if (all || namesonly) { if (uponly) --- ../freebsd/sbin/ifconfig/ifconfig.h Thu Oct 9 18:23:30 2003 +++ ../cleanup/sbin/ifconfig/ifconfig.h Fri Jan 23 10:44:54 2004 @@ -36,7 +36,7 @@ extern struct ifreq ifr; -extern char name[32]; /* name of interface */ +extern char name[IFNAMSIZ]; /* name of interface */ extern int allmedia; extern int supmedia; struct afswtch; --- ../freebsd/sys/net/if.c Fri Jan 23 09:26:48 2004 +++ ../cleanup/sys/net/if.c Fri Jan 23 10:21:15 2004 @@ -410,36 +410,34 @@ * create a Link Level name for this device */ namelen = strlen(ifp->if_xname); -#define _offsetof(t, m) ((int)((caddr_t)&((t *)0)->m)) - masklen = _offsetof(struct sockaddr_dl, sdl_data[0]) + namelen; + masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + namelen; socksize = masklen + ifp->if_addrlen; #define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1))) if (socksize < sizeof(*sdl)) socksize = sizeof(*sdl); socksize = ROUNDUP(socksize); +#undef ROUNDUP ifasize = sizeof(*ifa) + 2 * socksize; - ifa = (struct ifaddr *)malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO); - if (ifa) { - IFA_LOCK_INIT(ifa); - sdl = (struct sockaddr_dl *)(ifa + 1); - sdl->sdl_len = socksize; - sdl->sdl_family = AF_LINK; - bcopy(ifp->if_xname, sdl->sdl_data, namelen); - sdl->sdl_nlen = namelen; - sdl->sdl_index = ifp->if_index; - sdl->sdl_type = ifp->if_type; - ifaddr_byindex(ifp->if_index) = ifa; - ifa->ifa_ifp = ifp; - ifa->ifa_rtrequest = link_rtrequest; - ifa->ifa_addr = (struct sockaddr *)sdl; - sdl = (struct sockaddr_dl *)(socksize + (caddr_t)sdl); - ifa->ifa_netmask = (struct sockaddr *)sdl; - sdl->sdl_len = masklen; - while (namelen != 0) - sdl->sdl_data[--namelen] = 0xff; - ifa->ifa_refcnt = 1; - TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link); - } + ifa = malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO); + IFA_LOCK_INIT(ifa); + sdl = (struct sockaddr_dl *)(ifa + 1); + sdl->sdl_len = socksize; + sdl->sdl_family = AF_LINK; + bcopy(ifp->if_xname, sdl->sdl_data, namelen); + sdl->sdl_nlen = namelen; + sdl->sdl_index = ifp->if_index; + sdl->sdl_type = ifp->if_type; + ifaddr_byindex(ifp->if_index) = ifa; + ifa->ifa_ifp = ifp; + ifa->ifa_rtrequest = link_rtrequest; + ifa->ifa_addr = (struct sockaddr *)sdl; + sdl = (struct sockaddr_dl *)(socksize + (caddr_t)sdl); + ifa->ifa_netmask = (struct sockaddr *)sdl; + sdl->sdl_len = masklen; + while (namelen != 0) + sdl->sdl_data[--namelen] = 0xff; + ifa->ifa_refcnt = 1; + TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link); ifp->if_broadcastaddr = 0; /* reliably crash if used uninitialized */ if (domains) *** Functional diffs *** --- ../cleanup/sbin/ifconfig/ifconfig.8 Fri Jan 23 09:36:11 2004 +++ sbin/ifconfig/ifconfig.8 Fri Jan 23 10:58:58 2004 @@ -322,6 +322,9 @@ and 802.11g .Pq Dq 11g operating modes. +.It Cm name Ar name +Set the interface name to +.Ar name . .It Cm rxcsum , txcsum If the driver supports user-configurable checksum offloading, enable receive (or transmit) checksum offloading on the interface. @@ -353,7 +356,10 @@ If the interface is given without a unit number, try to create a new device with an arbitrary unit number. If creation of an arbitrary device is successful, the new device name is -printed to standard output. +printed to standard output unless the interface is renamed or destroyed +in the same +.Nm +invocation. .It Cm destroy Destroy the specified network pseudo-device. .It Cm plumb --- ../cleanup/sbin/ifconfig/ifconfig.c Fri Jan 23 10:44:54 2004 +++ sbin/ifconfig/ifconfig.c Fri Jan 23 10:58:58 2004 @@ -129,6 +129,7 @@ int supmedia = 0; int listcloners = 0; +int printname = 0; /* Print the name of the created interface. */ #ifdef INET6 char addr_buf[MAXHOSTNAMELEN *2 + 1]; /*for getnameinfo()*/ @@ -172,6 +173,7 @@ c_func setifipdst; c_func setifflags, setifmetric, setifmtu, setifcap; c_func clone_destroy; +c_func setifname; void clone_create(void); @@ -286,6 +288,7 @@ { "compress", IFF_LINK0, setifflags }, { "noicmp", IFF_LINK1, setifflags }, { "mtu", NEXTARG, setifmtu }, + { "name", NEXTARG, setifname }, { 0, 0, setifaddr }, { 0, 0, setifdstaddr }, }; @@ -525,7 +528,7 @@ clone_create(); argc--, argv++; if (argc == 0) - exit(0); + goto end; } ifindex = if_nametoindex(name); if (ifindex == 0) @@ -629,6 +632,9 @@ if (namesonly && need_nl > 0) putchar('\n'); +end: + if (printname) + printf("%s\n", name); exit (0); } @@ -1037,6 +1043,30 @@ warn("ioctl (set mtu)"); } +void +setifname(const char *val, int dummy __unused, int s, + const struct afswtch *afp) +{ + char *newname; + + newname = strdup(val); + + ifr.ifr_data = newname; + if (ioctl(s, SIOCSIFNAME, (caddr_t)&ifr) < 0) { + warn("ioctl (set name)"); + free(newname); + return; + } + strlcpy(name, newname, sizeof(name)); + free(newname); + + /* + * Even if we just created the interface, we don't need to print + * its name because we just nailed it down separately. + */ + printname = 0; +} + #define IFFBITS \ "\020\1UP\2BROADCAST\3DEBUG\4LOOPBACK\5POINTOPOINT\6SMART\7RUNNING" \ "\10NOARP\11PROMISC\12ALLMULTI\13OACTIVE\14SIMPLEX\15LINK0\16LINK1\17LINK2" \ @@ -1883,8 +1913,13 @@ if (ioctl(s, SIOCIFCREATE, &ifr) < 0) err(1, "SIOCIFCREATE"); + /* + * If we get a different name back then we put in, we probably + * want to print it out, but we might change our mind later so + * we just signal our intrest and leave the printout for later. + */ if (strcmp(name, ifr.ifr_name) != 0) { - printf("%s\n", ifr.ifr_name); + printname = 1; strlcpy(name, ifr.ifr_name, sizeof(name)); } @@ -1898,4 +1933,9 @@ (void) strncpy(ifr.ifr_name, name, sizeof(ifr.ifr_name)); if (ioctl(s, SIOCIFDESTROY, &ifr) < 0) err(1, "SIOCIFDESTROY"); + /* + * If we create and destroy an interface in the same command, + * there isn't any reason to print it's name. + */ + printname = 0; } --- ../cleanup/share/man/man9/ifnet.9 Fri Jan 23 10:08:43 2004 +++ share/man/man9/ifnet.9 Mon Jan 26 13:11:57 2004 @@ -950,6 +950,13 @@ Get interface configuration. (No call-down to driver.) .Pp +.It Dv SIOCSIFNAME +Set the interface name. +.Dv RTM_IFANNOUCNE departure and arrival messages are sent so that +routing code that relies on the interface name will update its interface +list. +Caller must have appropriate privilege. +(No call-down to driver.) .It Dv SIOCGIFCAP .It Dv SIOCGIFFLAGS .It Dv SIOCGIFMETRIC --- ../cleanup/sys/net/if.c Fri Jan 23 10:21:15 2004 +++ sys/net/if.c Fri Jan 23 20:52:02 2004 @@ -410,7 +410,11 @@ * create a Link Level name for this device */ namelen = strlen(ifp->if_xname); - masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + namelen; + /* + * Always save enough space for any possiable name so we can do + * a rename in place later. + */ + masklen = offsetof(struct sockaddr_dl, sdl_data[0]) + IFNAMSIZ; socksize = masklen + ifp->if_addrlen; #define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1))) if (socksize < sizeof(*sdl)) @@ -733,17 +737,16 @@ int bytoff, bitoff; int unit; - ifc = if_clone_lookup(name, &unit); - if (ifc == NULL) - return (EINVAL); - - if (unit < ifc->ifc_minifs) - return (EINVAL); - ifp = ifunit(name); if (ifp == NULL) return (ENXIO); + unit = ifp->if_dunit; + + ifc = if_clone_lookup(ifp->if_dname, NULL); + if (ifc == NULL) + return (EINVAL); + if (ifc->ifc_destroy == NULL) return (EOPNOTSUPP); @@ -1228,25 +1231,11 @@ struct ifnet * ifunit(const char *name) { - char namebuf[IFNAMSIZ + sizeof("net")]; /* XXX net_cdevsw.d_name */ struct ifnet *ifp; - dev_t dev; - - /* - * Now search all the interfaces for this name/number - */ - /* - * XXX - * Devices should really be known as /dev/fooN, not /dev/net/fooN. - */ - snprintf(namebuf, sizeof(namebuf), "%s/%s", net_cdevsw.d_name, name); IFNET_RLOCK(); TAILQ_FOREACH(ifp, &ifnet, if_link) { - dev = ifdev_byindex(ifp->if_index); - if (strcmp(devtoname(dev), namebuf) == 0) - break; - if (dev_named(dev, name)) + if (strncmp(name, ifp->if_xname, IFNAMSIZ) == 0) break; } IFNET_RUNLOCK(); @@ -1289,6 +1278,10 @@ struct ifstat *ifs; int error = 0; int new_flags; + size_t namelen, onamelen; + char new_name[IFNAMSIZ]; + struct ifaddr *ifa; + struct sockaddr_dl *sdl; ifr = (struct ifreq *)data; switch (cmd) { @@ -1370,6 +1363,46 @@ error = mac_ioctl_ifnet_set(td->td_ucred, ifr, ifp); break; #endif + + case SIOCSIFNAME: + error = suser(td); + if (error) + return (error); + error = copyinstr(ifr->ifr_data, new_name, IFNAMSIZ, NULL); + if (error) + return (error); + if (ifunit(new_name) != NULL) + return (EEXIST); + + /* Announce the departure of the interface. */ + rt_ifannouncemsg(ifp, IFAN_DEPARTURE); + + strlcpy(ifp->if_xname, new_name, sizeof(ifp->if_xname)); + ifa = TAILQ_FIRST(&ifp->if_addrhead); + IFA_LOCK(ifa); + sdl = (struct sockaddr_dl *)ifa->ifa_addr; + namelen = strlen(new_name); + onamelen = sdl->sdl_nlen; + /* + * Move the address if needed. This is safe because we + * allocate space for a name of length IFNAMSIZ when we + * create this in if_attach(). + */ + if (namelen != onamelen) { + bcopy(sdl->sdl_data + onamelen, + sdl->sdl_data + namelen, sdl->sdl_alen); + } + bcopy(new_name, sdl->sdl_data, namelen); + sdl->sdl_nlen = namelen; + sdl = (struct sockaddr_dl *)ifa->ifa_netmask; + bzero(sdl->sdl_data, onamelen); + while (namelen != 0) + sdl->sdl_data[--namelen] = 0xff; + IFA_UNLOCK(ifa); + + /* Announce the return of the interface. */ + rt_ifannouncemsg(ifp, IFAN_ARRIVAL); + break; case SIOCSIFMETRIC: error = suser(td); --- ../cleanup/sys/sys/sockio.h Fri Jan 23 09:38:05 2004 +++ sys/sys/sockio.h Mon Dec 8 12:03:32 2003 @@ -82,6 +82,7 @@ #define SIOCGIFINDEX _IOWR('i', 32, struct ifreq) /* get IF index */ #define SIOCGIFMAC _IOWR('i', 38, struct ifreq) /* get IF MAC label */ #define SIOCSIFMAC _IOW('i', 39, struct ifreq) /* set IF MAC label */ +#define SIOCSIFNAME _IOW('i', 40, struct ifreq) /* set IF name */ #define SIOCADDMULTI _IOW('i', 49, struct ifreq) /* add m'cast addr */ #define SIOCDELMULTI _IOW('i', 50, struct ifreq) /* del m'cast addr */ -- Any statement of the form "X is the one, true Y" is FALSE. PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4