Date: Mon, 26 Jan 2004 13:22:12 -0800 From: Brooks Davis <brooks@one-eyed-alien.net> To: net@freebsd.org Subject: Re: review request: interface renaming patch Message-ID: <20040126212212.GA30225@Odin.AC.HMC.Edu> In-Reply-To: <20040123200238.GA3133@Odin.AC.HMC.Edu> References: <20040123200238.GA3133@Odin.AC.HMC.Edu>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040126212212.GA30225>