Hi Jeff & Dave,
The trivial patch below implement SIOCSIFNAME wilcard
support. With the appropriate version of nameif (please ask me), you
can do binding of this kind :
------------------------------------------------
# AMD Lance cards (pcnet32)
amd* 00:10:83:*
# Intersil PrismGT cards (prism54)
wlan* 00:04:E2:*
------------------------------------------------
Of course, this make sense mostly for wireless cards. There is
currently lot's od debate about the use of 'ethX' and 'wlanX' names
for wireless driver, and some confusion resulting from that. Each
driver use a different naming style, and some have module parameters
to change the name prefix. Therefore, I think most distro and driver
authors would benefit from such a feature.
It would also enable to emulate the naming conventions of
*BSD, but I don't know if I should list that as an argument ;-)
I've run this patch for a while without any adverse
effect. Please forward upstream ;-)
Jean
P.S. : Don't miss the second part of the change, we absolutely need to
return the new name to user space.
-------------------------------------------------------------
diff -u -p linux/net/core/dev.j1.c linux/net/core/dev.c
--- linux/net/core/dev.j1.c Wed Dec 3 18:39:09 2003
+++ linux/net/core/dev.c Wed Dec 3 18:54:36 2003
@@ -2344,15 +2344,29 @@ static int dev_ifsioc(struct ifreq *ifr,
if (dev->flags & IFF_UP)
return -EBUSY;
ifr->ifr_newname[IFNAMSIZ-1] = '\0';
- if (__dev_get_by_name(ifr->ifr_newname))
- return -EEXIST;
- err = class_device_rename(&dev->class_dev,
- ifr->ifr_newname);
- if (!err) {
+ /* Check if name contains a wildcard */
+ if (strchr(ifr->ifr_newname, '%')) {
+ int ret;
+ /* Find a free name based on format.
+ * dev_alloc_name() replaces "%d" with at max
+ * 2 digits, so no name overflow. - Jean II */
+ ret = dev_alloc_name(dev, ifr->ifr_newname);
+ if (ret < 0)
+ return ret;
+ /* Copy the new name back to caller. */
+ strncpy(ifr->ifr_newname, dev->name, IFNAMSIZ);
+ } else {
+ if (__dev_get_by_name(ifr->ifr_newname))
+ return -EEXIST;
strlcpy(dev->name, ifr->ifr_newname, IFNAMSIZ);
-
+ }
+ err = class_device_rename(&dev->class_dev, dev->name);
+ if (!err) {
notifier_call_chain(&netdev_chain,
NETDEV_CHANGENAME, dev);
+ } else {
+ /* Restore original name */
+ strlcpy(dev->name, ifr->ifr_name, IFNAMSIZ);
}
return err;
@@ -2485,6 +2499,7 @@ int dev_ioctl(unsigned int cmd, void *ar
* - require strict serialization.
* - return a value
*/
+ case SIOCSIFNAME:
case SIOCGMIIPHY:
case SIOCGMIIREG:
if (!capable(CAP_NET_ADMIN))
@@ -2518,7 +2533,6 @@ int dev_ioctl(unsigned int cmd, void *ar
case SIOCDELMULTI:
case SIOCSIFHWBROADCAST:
case SIOCSIFTXQLEN:
- case SIOCSIFNAME:
case SIOCSMIIREG:
case SIOCBONDENSLAVE:
case SIOCBONDRELEASE:
On Tue, Jan 13, 2004 at 02:22:04PM -0800, Stephen Hemminger wrote:
> Here is an enhanced version of the previous patch.
> It adds both the wildcard support that Jean did, and validation of network
> device names.
Excellent. Your patch looks much cleaner.
> It doesn't check the error return from class_device_rename because
> that will not fail unless object or name are null.
I was not 100% sure if this would remain true.
Thanks !
Jean
Here is an enhanced version of the previous patch.
It adds both the wildcard support that Jean did, and validation of network
device names.
It doesn't check the error return from class_device_rename because
that will not fail unless object or name are null.
diff -Nru a/net/core/dev.c b/net/core/dev.c
--- a/net/core/dev.c Tue Jan 13 14:20:51 2004
+++ b/net/core/dev.c Tue Jan 13 14:20:51 2004
@@ -621,6 +621,21 @@
}
/**
+ * dev_valid_name - check if name is okay for network device
+ * @name: name string
+ *
+ * Network device names need to be valid file names to
+ * to allow sysfs to work
+ */
+int dev_valid_name(const char *name)
+{
+ return !(*name == '\0'
+ || !strcmp(name, ".")
+ || !strcmp(name, "..")
+ || strchr(name, '/'));
+}
+
+/**
* dev_alloc_name - allocate a name for a device
* @dev: device
* @name: name format string
@@ -660,6 +675,41 @@
return -ENFILE; /* Over 100 of the things .. bail out! */
}
+
+/**
+ * dev_change_name - change name of a device
+ * @dev: device
+ * @name: name (or format string) must be at least IFNAMSIZ
+ *
+ * Change name of a device, can pass format strings "eth%d".
+ * for wildcarding.
+ */
+int dev_change_name(struct net_device *dev, char *newname)
+{
+ ASSERT_RTNL();
+
+ if (dev->flags & IFF_UP)
+ return -EBUSY;
+
+ if (!dev_valid_name(newname))
+ return -EINVAL;
+
+ if (strchr(newname, '%')) {
+ int err = dev_alloc_name(dev, newname);
+ if (err)
+ return err;
+ strcpy(newname, dev->name);
+ }
+ else if (__dev_get_by_name(newname))
+ return -EEXIST;
+ else
+ strlcpy(dev->name, newname, IFNAMSIZ);
+
+ class_device_rename(&dev->class_dev, dev->name);
+ notifier_call_chain(&netdev_chain, NETDEV_CHANGENAME, dev);
+ return 0;
+}
+
/**
* dev_alloc - allocate a network device and name
* @name: name format string
@@ -2359,20 +2409,8 @@
return 0;
case SIOCSIFNAME:
- if (dev->flags & IFF_UP)
- return -EBUSY;
ifr->ifr_newname[IFNAMSIZ-1] = '\0';
- if (__dev_get_by_name(ifr->ifr_newname))
- return -EEXIST;
- err = class_device_rename(&dev->class_dev,
- ifr->ifr_newname);
- if (!err) {
- strlcpy(dev->name, ifr->ifr_newname, IFNAMSIZ);
-
- notifier_call_chain(&netdev_chain,
- NETDEV_CHANGENAME, dev);
- }
- return err;
+ return dev_change_name(dev, ifr->ifr_newname);
/*
* Unknown or private ioctl
@@ -2505,6 +2543,7 @@
*/
case SIOCGMIIPHY:
case SIOCGMIIREG:
+ case SIOCSIFNAME:
if (!capable(CAP_NET_ADMIN))
return -EPERM;
dev_load(ifr.ifr_name);
@@ -2536,7 +2575,6 @@
case SIOCDELMULTI:
case SIOCSIFHWBROADCAST:
case SIOCSIFTXQLEN:
- case SIOCSIFNAME:
case SIOCSMIIREG:
case SIOCBONDENSLAVE:
case SIOCBONDRELEASE:
@@ -2685,6 +2723,11 @@
ret = -EIO;
goto out_err;
}
+ }
+
+ if (!dev_valid_name(dev->name)) {
+ ret = -EINVAL;
+ goto out_err;
}
dev->ifindex = dev_new_index();
On Tue, 13 Jan 2004 14:22:04 -0800
Stephen Hemminger <[email protected]> wrote:
> Here is an enhanced version of the previous patch.
> It adds both the wildcard support that Jean did, and validation of network
> device names.
>
> It doesn't check the error return from class_device_rename because
> that will not fail unless object or name are null.
This is really cool, nice work guys.
Patch applied, thanks.
On Wed, Jan 14, 2004 at 04:13:24PM -0800, Stephen Hemminger wrote:
> Bug: dev_alloc_name returns the number of the slot used, so comparison needs
> to be < 0.
>
> diff -Nru a/net/core/dev.c b/net/core/dev.c
> --- a/net/core/dev.c Wed Jan 14 16:09:02 2004
> +++ b/net/core/dev.c Wed Jan 14 16:09:02 2004
> @@ -718,7 +718,7 @@
>
> if (strchr(newname, '%')) {
> int err = dev_alloc_name(dev, newname);
> - if (err)
> + if (err < 0)
> return err;
> strcpy(newname, dev->name);
> }
Doh ! I feel stupid.
Jean
Bug: dev_alloc_name returns the number of the slot used, so comparison needs
to be < 0.
diff -Nru a/net/core/dev.c b/net/core/dev.c
--- a/net/core/dev.c Wed Jan 14 16:09:02 2004
+++ b/net/core/dev.c Wed Jan 14 16:09:02 2004
@@ -718,7 +718,7 @@
if (strchr(newname, '%')) {
int err = dev_alloc_name(dev, newname);
- if (err)
+ if (err < 0)
return err;
strcpy(newname, dev->name);
}
On Wed, 14 Jan 2004 16:13:24 -0800
Stephen Hemminger <[email protected]> wrote:
> Bug: dev_alloc_name returns the number of the slot used, so comparison needs
> to be < 0.
Applied, thanks Stephen.