Prevent EFAULT errors when checking link status, in bonding net driver.

Also some minor cleanups as well.

[This patch qualifies for the cavemen ugh-lympics, because the driver does
some really nasty things in interrupt context and this patch does
not correct that.  However, the patch is an incremental improvement
over the current code so it's still worth applying.  I'll fix it
further if IBM does not fix it first.  -jgarzik]

diff --git a/drivers/net/bonding.c b/drivers/net/bonding.c
index 2fd61abaad885aa73ebbbf5d85fce5233c5b5854..68042f989e991a1fcdc58d84a036cf60f27cc413 100644 (file)
--- a/drivers/net/bonding.c
+++ b/drivers/net/bonding.c
@@ -186,6 +186,11 @@
  *       also added text to distinguish type of load balancing (rr or xor)
  *     - change arp_ip_target module param from "1-12s" (array of 12 ptrs)
  *       to "s" (a single ptr)
+ *
+ * 2002/09/18 - Jay Vosburgh <fubar at us dot ibm dot com>
+ *     - Fixed up bond_check_dev_link() (and callers): removed some magic
+ *      numbers, banished local MII_ defines, wrapped ioctl calls to
+ *      prevent EFAULT errors
  */
 
 #include <linux/config.h>
@@ -228,15 +233,6 @@
 #define BOND_LINK_MON_INTERV   0
 #endif
 
-#undef  MII_LINK_UP
-#define MII_LINK_UP    0x04
-
-#undef  MII_ENDOF_NWAY
-#define MII_ENDOF_NWAY 0x20
-
-#undef  MII_LINK_READY
-#define MII_LINK_READY (MII_LINK_UP)
-
 #ifndef BOND_LINK_ARP_INTERV
 #define BOND_LINK_ARP_INTERV   0
 #endif
@@ -386,13 +382,25 @@ static slave_t *bond_detach_slave(bonding_t *bond, slave_t *slave)
        return slave;
 }
 
+/*
+ * Less bad way to call ioctl from within the kernel; this needs to be
+ * done some other way to get the call out of interrupt context.
+ * Needs "ioctl" variable to be supplied by calling context.
+ */
+#define IOCTL(dev, arg, cmd) ({                \
+       int ret;                        \
+       mm_segment_t fs = get_fs();     \
+       set_fs(get_ds());               \
+       ret = ioctl(dev, arg, cmd);     \
+       set_fs(fs);                     \
+       ret; })
+
 /* 
- * if <dev> supports MII link status reporting, check its link
- * and report it as a bit field in a short int :
- *   - 0x04 means link is up,
- *   - 0x20 means end of autonegociation
- * If the device doesn't support MII, then we only report 0x24,
- * meaning that the link is up and running since we can't check it.
+ * if <dev> supports MII link status reporting, check its link status.
+ *
+ * Return either BMSR_LSTATUS, meaning that the link is up (or we
+ * can't tell and just pretend it is), or 0, meaning that the link is
+ * down.
  */
 static u16 bond_check_dev_link(struct net_device *dev)
 {
@@ -401,7 +409,8 @@ static u16 bond_check_dev_link(struct net_device *dev)
        struct mii_ioctl_data *mii;
        struct ethtool_value etool;
 
-       if ((ioctl = dev->do_ioctl) != NULL)  { /* ioctl to access MII */
+       ioctl = dev->do_ioctl;
+       if (ioctl) {
                /* TODO: set pointer to correct ioctl on a per team member */
                /*       bases to make this more efficient. that is, once  */
                /*       we determine the correct ioctl, we will always    */
@@ -415,9 +424,9 @@ static u16 bond_check_dev_link(struct net_device *dev)
                /* effect...                                               */
                etool.cmd = ETHTOOL_GLINK;
                ifr.ifr_data = (char*)&etool;
-               if (ioctl(dev, &ifr, SIOCETHTOOL) == 0) {
+               if (IOCTL(dev, &ifr, SIOCETHTOOL) == 0) {
                        if (etool.data == 1) {
-                               return(MII_LINK_READY);
+                               return BMSR_LSTATUS;
                        } 
                        else { 
                                return(0);
@@ -431,21 +440,17 @@ static u16 bond_check_dev_link(struct net_device *dev)
 
                /* Yes, the mii is overlaid on the ifreq.ifr_ifru */
                mii = (struct mii_ioctl_data *)&ifr.ifr_data;
-               if (ioctl(dev, &ifr, SIOCGMIIPHY) != 0) {
-                       return MII_LINK_READY;   /* can't tell */
+               if (IOCTL(dev, &ifr, SIOCGMIIPHY) != 0) {
+                       return BMSR_LSTATUS;     /* can't tell */
                }
 
-               mii->reg_num = 1;
-               if (ioctl(dev, &ifr, SIOCGMIIREG) == 0) {
-                       /*
-                        * mii->val_out contains MII reg 1, BMSR
-                        * 0x0004 means link established
-                        */
-                       return mii->val_out;
+               mii->reg_num = MII_BMSR;
+               if (IOCTL(dev, &ifr, SIOCGMIIREG) == 0) {
+                       return mii->val_out & BMSR_LSTATUS;
                }
 
        }
-       return MII_LINK_READY;  /* spoof link up ( we can't check it) */
+       return BMSR_LSTATUS;  /* spoof link up ( we can't check it) */
 }
 
 static u16 bond_check_mii_link(bonding_t *bond)
@@ -459,7 +464,7 @@ static u16 bond_check_mii_link(bonding_t *bond)
        read_unlock(&bond->ptrlock);
        read_unlock_irqrestore(&bond->lock, flags);
 
-       return (has_active_interface ? MII_LINK_READY : 0);
+       return (has_active_interface ? BMSR_LSTATUS : 0);
 }
 
 static int bond_open(struct net_device *dev)
@@ -797,8 +802,8 @@ static int bond_enslave(struct net_device *master_dev,
        new_slave->link_failure_count = 0;
 
        /* check for initial state */
-       if ((miimon <= 0) || ((bond_check_dev_link(slave_dev) & MII_LINK_READY)
-                == MII_LINK_READY)) {
+       if ((miimon <= 0) ||
+           (bond_check_dev_link(slave_dev) == BMSR_LSTATUS)) {
 #ifdef BONDING_DEBUG
                printk(KERN_CRIT "Initial state of slave_dev is BOND_LINK_UP\n");
 #endif
@@ -1220,7 +1225,7 @@ static void bond_mii_monitor(struct net_device *master)
 
                switch (slave->link) {
                case BOND_LINK_UP:      /* the link was up */
-                       if ((link_state & MII_LINK_UP) == MII_LINK_UP) {
+                       if (link_state == BMSR_LSTATUS) {
                                /* link stays up, tell that this one
                                   is immediately available */
                                if (IS_UP(dev) && (mindelay > -2)) {
@@ -1256,7 +1261,7 @@ static void bond_mii_monitor(struct net_device *master)
                           ensure proper action to be taken
                        */
                case BOND_LINK_FAIL:    /* the link has just gone down */
-                       if ((link_state & MII_LINK_UP) == 0) {
+                       if (link_state != BMSR_LSTATUS) {
                                /* link stays down */
                                if (slave->delay <= 0) {
                                        /* link down for too long time */
@@ -1285,7 +1290,7 @@ static void bond_mii_monitor(struct net_device *master)
                                } else {
                                        slave->delay--;
                                }
-                       } else if ((link_state & MII_LINK_READY) == MII_LINK_READY) {
+                       } else {
                                /* link up again */
                                slave->link  = BOND_LINK_UP;
                                printk(KERN_INFO
@@ -1304,7 +1309,7 @@ static void bond_mii_monitor(struct net_device *master)
                        }
                        break;
                case BOND_LINK_DOWN:    /* the link was down */
-                       if ((link_state & MII_LINK_READY) != MII_LINK_READY) {
+                       if (link_state != BMSR_LSTATUS) {
                                /* the link stays down, nothing more to do */
                                break;
                        } else {        /* link going up */
@@ -1326,7 +1331,7 @@ static void bond_mii_monitor(struct net_device *master)
                           case there's something to do.
                        */
                case BOND_LINK_BACK:    /* the link has just come back */
-                       if ((link_state & MII_LINK_UP) == 0) {
+                       if (link_state != BMSR_LSTATUS) {
                                /* link down again */
                                slave->link  = BOND_LINK_DOWN;
                                printk(KERN_INFO
@@ -1335,8 +1340,7 @@ static void bond_mii_monitor(struct net_device *master)
                                        master->name,
                                        (updelay - slave->delay) * miimon,
                                        dev->name);
-                       }
-                       else if ((link_state & MII_LINK_READY) == MII_LINK_READY) {
+                       } else {
                                /* link stays up */
                                if (slave->delay == 0) {
                                        /* now the link has been up for long time enough */
@@ -2110,7 +2114,7 @@ static int bond_get_info(char *buf, char **start, off_t offset, int length)
 
                len += sprintf(buf + len, "MII Status: ");
                len += sprintf(buf + len, 
-                               link == MII_LINK_READY ? "up\n" : "down\n");
+                               link == BMSR_LSTATUS ? "up\n" : "down\n");
                len += sprintf(buf + len, "MII Polling Interval (ms): %d\n", 
                                miimon);
                len += sprintf(buf + len, "Up Delay (ms): %d\n", updelay);