[PATCH] fix bonding crash, remove old ABI support
authorJay Vosburgh <fubar@us.ibm.com>
Mon, 26 Sep 2005 23:11:50 +0000 (16:11 -0700)
committerJeff Garzik <jgarzik@pobox.com>
Tue, 4 Oct 2005 02:15:00 +0000 (22:15 -0400)
David S. Miller <davem@davemloft.net> wrote:
>I think removing support for older ifenslave binaries is
>the least painful solution to this problem.

This patch removes backwards compatibility for old ifenslave
binaries (ifenslave prior to verison 1.0.0).

I did not similarly modify ifenslave itself; with sysfs on the
horizon, I don't see that as being worthwhile.

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Signed-off-by: Jeff Garzik <jgarzik@pobox.com>
drivers/net/bonding/bond_main.c
drivers/net/bonding/bonding.h

index bf81cd4..fd62e43 100644 (file)
  *       * Added xmit_hash_policy_layer34()
  *     - Modified by Jay Vosburgh <fubar@us.ibm.com> to also support mode 4.
  *       Set version to 2.6.3.
+ * 2005/09/26 - Jay Vosburgh <fubar@us.ibm.com>
+ *     - Removed backwards compatibility for old ifenslaves.  Version 2.6.4.
  */
 
 //#define BONDING_DEBUG 1
@@ -595,14 +597,7 @@ static int arp_ip_count    = 0;
 static int bond_mode   = BOND_MODE_ROUNDROBIN;
 static int xmit_hashtype= BOND_XMIT_POLICY_LAYER2;
 static int lacp_fast   = 0;
-static int app_abi_ver = 0;
-static int orig_app_abi_ver = -1; /* This is used to save the first ABI version
-                                  * we receive from the application. Once set,
-                                  * it won't be changed, and the module will
-                                  * refuse to enslave/release interfaces if the
-                                  * command comes from an application using
-                                  * another ABI version.
-                                  */
+
 struct bond_parm_tbl {
        char *modename;
        int mode;
@@ -1702,51 +1697,29 @@ static int bond_enslave(struct net_device *bond_dev, struct net_device *slave_de
                }
        }
 
-       if (app_abi_ver >= 1) {
-               /* The application is using an ABI, which requires the
-                * slave interface to be closed.
-                */
-               if ((slave_dev->flags & IFF_UP)) {
-                       printk(KERN_ERR DRV_NAME
-                              ": Error: %s is up\n",
-                              slave_dev->name);
-                       res = -EPERM;
-                       goto err_undo_flags;
-               }
-
-               if (slave_dev->set_mac_address == NULL) {
-                       printk(KERN_ERR DRV_NAME
-                              ": Error: The slave device you specified does "
-                              "not support setting the MAC address.\n");
-                       printk(KERN_ERR
-                              "Your kernel likely does not support slave "
-                              "devices.\n");
+       /*
+        * Old ifenslave binaries are no longer supported.  These can
+        * be identified with moderate accurary by the state of the slave:
+        * the current ifenslave will set the interface down prior to
+        * enslaving it; the old ifenslave will not.
+        */
+       if ((slave_dev->flags & IFF_UP)) {
+               printk(KERN_ERR DRV_NAME ": %s is up. "
+                      "This may be due to an out of date ifenslave.\n",
+                      slave_dev->name);
+               res = -EPERM;
+               goto err_undo_flags;
+       }
 
-                       res = -EOPNOTSUPP;
-                       goto err_undo_flags;
-               }
-       } else {
-               /* The application is not using an ABI, which requires the
-                * slave interface to be open.
-                */
-               if (!(slave_dev->flags & IFF_UP)) {
-                       printk(KERN_ERR DRV_NAME
-                              ": Error: %s is not running\n",
-                              slave_dev->name);
-                       res = -EINVAL;
-                       goto err_undo_flags;
-               }
+       if (slave_dev->set_mac_address == NULL) {
+               printk(KERN_ERR DRV_NAME
+                      ": Error: The slave device you specified does "
+                      "not support setting the MAC address.\n");
+               printk(KERN_ERR
+                      "Your kernel likely does not support slave devices.\n");
 
-               if ((bond->params.mode == BOND_MODE_8023AD) ||
-                   (bond->params.mode == BOND_MODE_TLB)    ||
-                   (bond->params.mode == BOND_MODE_ALB)) {
-                       printk(KERN_ERR DRV_NAME
-                              ": Error: to use %s mode, you must upgrade "
-                              "ifenslave.\n",
-                              bond_mode_name(bond->params.mode));
-                       res = -EOPNOTSUPP;
-                       goto err_undo_flags;
-               }
+               res = -EOPNOTSUPP;
+               goto err_undo_flags;
        }
 
        new_slave = kmalloc(sizeof(struct slave), GFP_KERNEL);
@@ -1762,41 +1735,36 @@ static int bond_enslave(struct net_device *bond_dev, struct net_device *slave_de
         */
        new_slave->original_flags = slave_dev->flags;
 
-       if (app_abi_ver >= 1) {
-               /* save slave's original ("permanent") mac address for
-                * modes that needs it, and for restoring it upon release,
-                * and then set it to the master's address
-                */
-               memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);
+       /*
+        * Save slave's original ("permanent") mac address for modes
+        * that need it, and for restoring it upon release, and then
+        * set it to the master's address
+        */
+       memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);
 
-               /* set slave to master's mac address
-                * The application already set the master's
-                * mac address to that of the first slave
-                */
-               memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
-               addr.sa_family = slave_dev->type;
-               res = dev_set_mac_address(slave_dev, &addr);
-               if (res) {
-                       dprintk("Error %d calling set_mac_address\n", res);
-                       goto err_free;
-               }
+       /*
+        * Set slave to master's mac address.  The application already
+        * set the master's mac address to that of the first slave
+        */
+       memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
+       addr.sa_family = slave_dev->type;
+       res = dev_set_mac_address(slave_dev, &addr);
+       if (res) {
+               dprintk("Error %d calling set_mac_address\n", res);
+               goto err_free;
+       }
 
-               /* open the slave since the application closed it */
-               res = dev_open(slave_dev);
-               if (res) {
-                       dprintk("Openning slave %s failed\n", slave_dev->name);
-                       goto err_restore_mac;
-               }
+       /* open the slave since the application closed it */
+       res = dev_open(slave_dev);
+       if (res) {
+               dprintk("Openning slave %s failed\n", slave_dev->name);
+               goto err_restore_mac;
        }
 
        res = netdev_set_master(slave_dev, bond_dev);
        if (res) {
                dprintk("Error %d calling netdev_set_master\n", res);
-               if (app_abi_ver < 1) {
-                       goto err_free;
-               } else {
-                       goto err_close;
-               }
+               goto err_close;
        }
 
        new_slave->dev = slave_dev;
@@ -1997,39 +1965,6 @@ static int bond_enslave(struct net_device *bond_dev, struct net_device *slave_de
 
        write_unlock_bh(&bond->lock);
 
-       if (app_abi_ver < 1) {
-               /*
-                * !!! This is to support old versions of ifenslave.
-                * We can remove this in 2.5 because our ifenslave takes
-                * care of this for us.
-                * We check to see if the master has a mac address yet.
-                * If not, we'll give it the mac address of our slave device.
-                */
-               int ndx = 0;
-
-               for (ndx = 0; ndx < bond_dev->addr_len; ndx++) {
-                       dprintk("Checking ndx=%d of bond_dev->dev_addr\n",
-                               ndx);
-                       if (bond_dev->dev_addr[ndx] != 0) {
-                               dprintk("Found non-zero byte at ndx=%d\n",
-                                       ndx);
-                               break;
-                       }
-               }
-
-               if (ndx == bond_dev->addr_len) {
-                       /*
-                        * We got all the way through the address and it was
-                        * all 0's.
-                        */
-                       dprintk("%s doesn't have a MAC address yet.  \n",
-                               bond_dev->name);
-                       dprintk("Going to give assign it from %s.\n",
-                               slave_dev->name);
-                       bond_sethwaddr(bond_dev, slave_dev);
-               }
-       }
-
        printk(KERN_INFO DRV_NAME
               ": %s: enslaving %s as a%s interface with a%s link.\n",
               bond_dev->name, slave_dev->name,
@@ -2227,12 +2162,10 @@ static int bond_release(struct net_device *bond_dev, struct net_device *slave_de
        /* close slave before restoring its mac address */
        dev_close(slave_dev);
 
-       if (app_abi_ver >= 1) {
-               /* restore original ("permanent") mac address */
-               memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
-               addr.sa_family = slave_dev->type;
-               dev_set_mac_address(slave_dev, &addr);
-       }
+       /* restore original ("permanent") mac address */
+       memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+       addr.sa_family = slave_dev->type;
+       dev_set_mac_address(slave_dev, &addr);
 
        /* restore the original state of the
         * IFF_NOARP flag that might have been
@@ -2320,12 +2253,10 @@ static int bond_release_all(struct net_device *bond_dev)
                /* close slave before restoring its mac address */
                dev_close(slave_dev);
 
-               if (app_abi_ver >= 1) {
-                       /* restore original ("permanent") mac address*/
-                       memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
-                       addr.sa_family = slave_dev->type;
-                       dev_set_mac_address(slave_dev, &addr);
-               }
+               /* restore original ("permanent") mac address*/
+               memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+               addr.sa_family = slave_dev->type;
+               dev_set_mac_address(slave_dev, &addr);
 
                /* restore the original state of the IFF_NOARP flag that might have
                 * been set by bond_set_slave_inactive_flags()
@@ -2423,57 +2354,6 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi
        return res;
 }
 
-static int bond_ethtool_ioctl(struct net_device *bond_dev, struct ifreq *ifr)
-{
-       struct ethtool_drvinfo info;
-       void __user *addr = ifr->ifr_data;
-       uint32_t cmd;
-
-       if (get_user(cmd, (uint32_t __user *)addr)) {
-               return -EFAULT;
-       }
-
-       switch (cmd) {
-       case ETHTOOL_GDRVINFO:
-               if (copy_from_user(&info, addr, sizeof(info))) {
-                       return -EFAULT;
-               }
-
-               if (strcmp(info.driver, "ifenslave") == 0) {
-                       int new_abi_ver;
-                       char *endptr;
-
-                       new_abi_ver = simple_strtoul(info.fw_version,
-                                                    &endptr, 0);
-                       if (*endptr) {
-                               printk(KERN_ERR DRV_NAME
-                                      ": Error: got invalid ABI "
-                                      "version from application\n");
-
-                               return -EINVAL;
-                       }
-
-                       if (orig_app_abi_ver == -1) {
-                               orig_app_abi_ver  = new_abi_ver;
-                       }
-
-                       app_abi_ver = new_abi_ver;
-               }
-
-               strncpy(info.driver,  DRV_NAME, 32);
-               strncpy(info.version, DRV_VERSION, 32);
-               snprintf(info.fw_version, 32, "%d", BOND_ABI_VERSION);
-
-               if (copy_to_user(addr, &info, sizeof(info))) {
-                       return -EFAULT;
-               }
-
-               return 0;
-       default:
-               return -EOPNOTSUPP;
-       }
-}
-
 static int bond_info_query(struct net_device *bond_dev, struct ifbond *info)
 {
        struct bonding *bond = bond_dev->priv;
@@ -3442,16 +3322,11 @@ static void bond_info_show_slave(struct seq_file *seq, const struct slave *slave
        seq_printf(seq, "Link Failure Count: %d\n",
                   slave->link_failure_count);
 
-       if (app_abi_ver >= 1) {
-               seq_printf(seq,
-                          "Permanent HW addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
-                          slave->perm_hwaddr[0],
-                          slave->perm_hwaddr[1],
-                          slave->perm_hwaddr[2],
-                          slave->perm_hwaddr[3],
-                          slave->perm_hwaddr[4],
-                          slave->perm_hwaddr[5]);
-       }
+       seq_printf(seq,
+                  "Permanent HW addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
+                  slave->perm_hwaddr[0], slave->perm_hwaddr[1],
+                  slave->perm_hwaddr[2], slave->perm_hwaddr[3],
+                  slave->perm_hwaddr[4], slave->perm_hwaddr[5]);
 
        if (bond->params.mode == BOND_MODE_8023AD) {
                const struct aggregator *agg
@@ -4010,15 +3885,12 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
        struct ifslave k_sinfo;
        struct ifslave __user *u_sinfo = NULL;
        struct mii_ioctl_data *mii = NULL;
-       int prev_abi_ver = orig_app_abi_ver;
        int res = 0;
 
        dprintk("bond_ioctl: master=%s, cmd=%d\n",
                bond_dev->name, cmd);
 
        switch (cmd) {
-       case SIOCETHTOOL:
-               return bond_ethtool_ioctl(bond_dev, ifr);
        case SIOCGMIIPHY:
                mii = if_mii(ifr);
                if (!mii) {
@@ -4090,21 +3962,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
                return -EPERM;
        }
 
-       if (orig_app_abi_ver == -1) {
-               /* no orig_app_abi_ver was provided yet, so we'll use the
-                * current one from now on, even if it's 0
-                */
-               orig_app_abi_ver = app_abi_ver;
-
-       } else if (orig_app_abi_ver != app_abi_ver) {
-               printk(KERN_ERR DRV_NAME
-                      ": Error: already using ifenslave ABI version %d; to "
-                      "upgrade ifenslave to version %d, you must first "
-                      "reload bonding.\n",
-                      orig_app_abi_ver, app_abi_ver);
-               return -EINVAL;
-       }
-
        slave_dev = dev_get_by_name(ifr->ifr_slave);
 
        dprintk("slave_dev=%p: \n", slave_dev);
@@ -4137,14 +3994,6 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
                dev_put(slave_dev);
        }
 
-       if (res < 0) {
-               /* The ioctl failed, so there's no point in changing the
-                * orig_app_abi_ver. We'll restore it's value just in case
-                * we've changed it earlier in this function.
-                */
-               orig_app_abi_ver = prev_abi_ver;
-       }
-
        return res;
 }
 
@@ -4578,9 +4427,18 @@ static inline void bond_set_mode_ops(struct bonding *bond, int mode)
        }
 }
 
+static void bond_ethtool_get_drvinfo(struct net_device *bond_dev,
+                                   struct ethtool_drvinfo *drvinfo)
+{
+       strncpy(drvinfo->driver, DRV_NAME, 32);
+       strncpy(drvinfo->version, DRV_VERSION, 32);
+       snprintf(drvinfo->fw_version, 32, "%d", BOND_ABI_VERSION);
+}
+
 static struct ethtool_ops bond_ethtool_ops = {
        .get_tx_csum            = ethtool_op_get_tx_csum,
        .get_sg                 = ethtool_op_get_sg,
+       .get_drvinfo            = bond_ethtool_get_drvinfo,
 };
 
 /*
index 3881969..bbf9da8 100644 (file)
@@ -40,8 +40,8 @@
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
-#define DRV_VERSION    "2.6.3"
-#define DRV_RELDATE    "June 8, 2005"
+#define DRV_VERSION    "2.6.4"
+#define DRV_RELDATE    "September 26, 2005"
 #define DRV_NAME       "bonding"
 #define DRV_DESCRIPTION        "Ethernet Channel Bonding Driver"