Date: Fri, 23 Jan 2004 12:02:38 -0800 From: Brooks Davis <brooks@one-eyed-alien.net> To: net@freebsd.org Subject: review request: interface renaming patch Message-ID: <20040123200238.GA3133@Odin.AC.HMC.Edu>
next in thread | raw e-mail | index | archive | help
The following patch implements network interface renaming via:
ifconfig <if> name <new name>
The mechanism is to change if_xname in the ifp and then to adjust the
link level address sockaddr_dl appropriatly. One question I do have is
if locking the ifa is sufficent or if we need to force the user to down
the interface before renaming it. I'm not sure where all the places
that use the sockaddr_dl are.
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
=66rom my perforce trees.
Please let me know about both problem with the patch it self and any
edge cases where chaning the interface name will cause problems (for
instance, I just noticed a minor problem in if_clone_create where you
could end up with duplicate interfaces names.)
Thanks,
Brooks
--=20
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
*** 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 */
=20
-char name[32];
+char name[IFNAMSIZ];
int flags;
int setaddr;
int setipdst;
@@ -596,8 +596,9 @@
addrcount++;
next +=3D nextifm->ifm_msglen;
}
- strncpy(name, sdl->sdl_data, sdl->sdl_nlen);
- name[sdl->sdl_nlen] =3D '\0';
+ strlcpy(name, sdl->sdl_data,
+ sizeof(name) <=3D sdl->sdl_nlen ?
+ sizeof(name) : sdl->sdl_nlen + 1);
=20
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 @@
=20
extern struct ifreq ifr;
=20
-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 =3D strlen(ifp->if_xname);
-#define _offsetof(t, m) ((int)((caddr_t)&((t *)0)->m))
- masklen =3D _offsetof(struct sockaddr_dl, sdl_data[0]) + namelen;
+ masklen =3D offsetof(struct sockaddr_dl, sdl_data[0]) + namelen;
socksize =3D masklen + ifp->if_addrlen;
#define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1)))
if (socksize < sizeof(*sdl))
socksize =3D sizeof(*sdl);
socksize =3D ROUNDUP(socksize);
+#undef ROUNDUP
ifasize =3D sizeof(*ifa) + 2 * socksize;
- ifa =3D (struct ifaddr *)malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO);
- if (ifa) {
- IFA_LOCK_INIT(ifa);
- sdl =3D (struct sockaddr_dl *)(ifa + 1);
- sdl->sdl_len =3D socksize;
- sdl->sdl_family =3D AF_LINK;
- bcopy(ifp->if_xname, sdl->sdl_data, namelen);
- sdl->sdl_nlen =3D namelen;
- sdl->sdl_index =3D ifp->if_index;
- sdl->sdl_type =3D ifp->if_type;
- ifaddr_byindex(ifp->if_index) =3D ifa;
- ifa->ifa_ifp =3D ifp;
- ifa->ifa_rtrequest =3D link_rtrequest;
- ifa->ifa_addr =3D (struct sockaddr *)sdl;
- sdl =3D (struct sockaddr_dl *)(socksize + (caddr_t)sdl);
- ifa->ifa_netmask =3D (struct sockaddr *)sdl;
- sdl->sdl_len =3D masklen;
- while (namelen !=3D 0)
- sdl->sdl_data[--namelen] =3D 0xff;
- ifa->ifa_refcnt =3D 1;
- TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link);
- }
+ ifa =3D malloc(ifasize, M_IFADDR, M_WAITOK | M_ZERO);
+ IFA_LOCK_INIT(ifa);
+ sdl =3D (struct sockaddr_dl *)(ifa + 1);
+ sdl->sdl_len =3D socksize;
+ sdl->sdl_family =3D AF_LINK;
+ bcopy(ifp->if_xname, sdl->sdl_data, namelen);
+ sdl->sdl_nlen =3D namelen;
+ sdl->sdl_index =3D ifp->if_index;
+ sdl->sdl_type =3D ifp->if_type;
+ ifaddr_byindex(ifp->if_index) =3D ifa;
+ ifa->ifa_ifp =3D ifp;
+ ifa->ifa_rtrequest =3D link_rtrequest;
+ ifa->ifa_addr =3D (struct sockaddr *)sdl;
+ sdl =3D (struct sockaddr_dl *)(socksize + (caddr_t)sdl);
+ ifa->ifa_netmask =3D (struct sockaddr *)sdl;
+ sdl->sdl_len =3D masklen;
+ while (namelen !=3D 0)
+ sdl->sdl_data[--namelen] =3D 0xff;
+ ifa->ifa_refcnt =3D 1;
+ TAILQ_INSERT_HEAD(&ifp->if_addrhead, ifa, ifa_link);
ifp->if_broadcastaddr =3D 0; /* reliably crash if used uninitialized */
=20
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 @@
=20
int supmedia =3D 0;
int listcloners =3D 0;
+int printname =3D 0; /* Print the name of the created interface. */
=20
#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;
=20
=20
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 =3D=3D 0)
- exit(0);
+ goto end;
}
ifindex =3D if_nametoindex(name);
if (ifindex =3D=3D 0)
@@ -629,6 +632,9 @@
=20
if (namesonly && need_nl > 0)
putchar('\n');
+end:
+ if (printname)
+ printf("%s\n", name);
=20
exit (0);
}
@@ -1037,6 +1043,30 @@
warn("ioctl (set mtu)");
}
=20
+void
+setifname(const char *val, int dummy __unused, int s,=20
+ const struct afswtch *afp)
+{
+ char *newname;
+
+ newname =3D strdup(val);
+
+ ifr.ifr_data =3D 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 =3D 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");
=20
+ /*
+ * 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) !=3D 0) {
- printf("%s\n", ifr.ifr_name);
+ printname =3D 1;
strlcpy(name, ifr.ifr_name, sizeof(name));
}
=20
@@ -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 =3D 0;
}
--- ../cleanup/sys/net/if.c Fri Jan 23 10:21:15 2004
+++ sys/net/if.c Fri Jan 23 10:24:19 2004
@@ -410,7 +410,11 @@
* create a Link Level name for this device
*/
namelen =3D strlen(ifp->if_xname);
- masklen =3D 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 =3D offsetof(struct sockaddr_dl, sdl_data[0]) + IFNAMSIZ;
socksize =3D 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;
=20
- ifc =3D if_clone_lookup(name, &unit);
- if (ifc =3D=3D NULL)
- return (EINVAL);
-
- if (unit < ifc->ifc_minifs)
- return (EINVAL);
-
ifp =3D ifunit(name);
if (ifp =3D=3D NULL)
return (ENXIO);
=20
+ unit =3D ifp->if_dunit;
+
+ ifc =3D if_clone_lookup(ifp->if_dname, NULL);
+ if (ifc =3D=3D NULL)
+ return (EINVAL);
+
if (ifc->ifc_destroy =3D=3D NULL)
return (EOPNOTSUPP);
=20
@@ -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
- */
=20
- /*
- * 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 =3D ifdev_byindex(ifp->if_index);
- if (strcmp(devtoname(dev), namebuf) =3D=3D 0)
- break;
- if (dev_named(dev, name))
+ if (strncmp(name, ifp->if_xname, IFNAMSIZ) =3D=3D 0)
break;
}
IFNET_RUNLOCK();
@@ -1289,6 +1278,10 @@
struct ifstat *ifs;
int error =3D 0;
int new_flags;
+ size_t namelen, onamelen;
+ char new_name[IFNAMSIZ];
+ struct ifaddr *ifa;
+ struct sockaddr_dl *sdl;
=20
ifr =3D (struct ifreq *)data;
switch (cmd) {
@@ -1370,6 +1363,39 @@
error =3D mac_ioctl_ifnet_set(td->td_ucred, ifr, ifp);
break;
#endif
+
+ case SIOCSIFNAME:
+ error =3D suser(td);
+ if (error)
+ return (error);
+ error =3D copyinstr(ifr->ifr_data, new_name, IFNAMSIZ, NULL);
+ if (error)
+ return (error);
+ if (ifunit(new_name) !=3D NULL)
+ return (EEXIST);
+ strlcpy(ifp->if_xname, new_name, sizeof(ifp->if_xname));
+ ifa =3D TAILQ_FIRST(&ifp->if_addrhead);
+ IFA_LOCK(ifa);
+ sdl =3D (struct sockaddr_dl *)ifa->ifa_addr;
+ namelen =3D strlen(new_name);
+ onamelen =3D 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 !=3D onamelen) {
+ bcopy(sdl->sdl_data + onamelen,
+ sdl->sdl_data + namelen, sdl->sdl_alen);
+ }
+ bcopy(new_name, sdl->sdl_data, namelen);
+ sdl->sdl_nlen =3D namelen;
+ sdl =3D (struct sockaddr_dl *)ifa->ifa_netmask;
+ bzero(sdl->sdl_data, onamelen);
+ while (namelen !=3D 0)
+ sdl->sdl_data[--namelen] =3D 0xff;
+ IFA_UNLOCK(ifa);
+ break;
=20
case SIOCSIFMETRIC:
error =3D 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 */
=20
#define SIOCADDMULTI _IOW('i', 49, struct ifreq) /* add m'cast addr */
#define SIOCDELMULTI _IOW('i', 50, struct ifreq) /* del m'cast addr */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040123200238.GA3133>