Commit eda607d8 authored by Jay Vosburgh's avatar Jay Vosburgh Committed by Jeff Garzik

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]
parent 95dad872
...@@ -186,6 +186,11 @@ ...@@ -186,6 +186,11 @@
* also added text to distinguish type of load balancing (rr or xor) * 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) * - change arp_ip_target module param from "1-12s" (array of 12 ptrs)
* to "s" (a single ptr) * 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> #include <linux/config.h>
...@@ -228,15 +233,6 @@ ...@@ -228,15 +233,6 @@
#define BOND_LINK_MON_INTERV 0 #define BOND_LINK_MON_INTERV 0
#endif #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 #ifndef BOND_LINK_ARP_INTERV
#define BOND_LINK_ARP_INTERV 0 #define BOND_LINK_ARP_INTERV 0
#endif #endif
...@@ -387,12 +383,24 @@ static slave_t *bond_detach_slave(bonding_t *bond, slave_t *slave) ...@@ -387,12 +383,24 @@ static slave_t *bond_detach_slave(bonding_t *bond, slave_t *slave)
} }
/* /*
* if <dev> supports MII link status reporting, check its link * Less bad way to call ioctl from within the kernel; this needs to be
* and report it as a bit field in a short int : * done some other way to get the call out of interrupt context.
* - 0x04 means link is up, * Needs "ioctl" variable to be supplied by calling context.
* - 0x20 means end of autonegociation */
* If the device doesn't support MII, then we only report 0x24, #define IOCTL(dev, arg, cmd) ({ \
* meaning that the link is up and running since we can't check it. 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 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) static u16 bond_check_dev_link(struct net_device *dev)
{ {
...@@ -401,7 +409,8 @@ 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 mii_ioctl_data *mii;
struct ethtool_value etool; 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 */ /* TODO: set pointer to correct ioctl on a per team member */
/* bases to make this more efficient. that is, once */ /* bases to make this more efficient. that is, once */
/* we determine the correct ioctl, we will always */ /* we determine the correct ioctl, we will always */
...@@ -415,9 +424,9 @@ static u16 bond_check_dev_link(struct net_device *dev) ...@@ -415,9 +424,9 @@ static u16 bond_check_dev_link(struct net_device *dev)
/* effect... */ /* effect... */
etool.cmd = ETHTOOL_GLINK; etool.cmd = ETHTOOL_GLINK;
ifr.ifr_data = (char*)&etool; ifr.ifr_data = (char*)&etool;
if (ioctl(dev, &ifr, SIOCETHTOOL) == 0) { if (IOCTL(dev, &ifr, SIOCETHTOOL) == 0) {
if (etool.data == 1) { if (etool.data == 1) {
return(MII_LINK_READY); return BMSR_LSTATUS;
} }
else { else {
return(0); return(0);
...@@ -431,21 +440,17 @@ static u16 bond_check_dev_link(struct net_device *dev) ...@@ -431,21 +440,17 @@ static u16 bond_check_dev_link(struct net_device *dev)
/* Yes, the mii is overlaid on the ifreq.ifr_ifru */ /* Yes, the mii is overlaid on the ifreq.ifr_ifru */
mii = (struct mii_ioctl_data *)&ifr.ifr_data; mii = (struct mii_ioctl_data *)&ifr.ifr_data;
if (ioctl(dev, &ifr, SIOCGMIIPHY) != 0) { if (IOCTL(dev, &ifr, SIOCGMIIPHY) != 0) {
return MII_LINK_READY; /* can't tell */ return BMSR_LSTATUS; /* can't tell */
} }
mii->reg_num = 1; mii->reg_num = MII_BMSR;
if (ioctl(dev, &ifr, SIOCGMIIREG) == 0) { if (IOCTL(dev, &ifr, SIOCGMIIREG) == 0) {
/* return mii->val_out & BMSR_LSTATUS;
* mii->val_out contains MII reg 1, BMSR
* 0x0004 means link established
*/
return mii->val_out;
} }
} }
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) static u16 bond_check_mii_link(bonding_t *bond)
...@@ -459,7 +464,7 @@ 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(&bond->ptrlock);
read_unlock_irqrestore(&bond->lock, flags); 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) static int bond_open(struct net_device *dev)
...@@ -797,8 +802,8 @@ static int bond_enslave(struct net_device *master_dev, ...@@ -797,8 +802,8 @@ static int bond_enslave(struct net_device *master_dev,
new_slave->link_failure_count = 0; new_slave->link_failure_count = 0;
/* check for initial state */ /* check for initial state */
if ((miimon <= 0) || ((bond_check_dev_link(slave_dev) & MII_LINK_READY) if ((miimon <= 0) ||
== MII_LINK_READY)) { (bond_check_dev_link(slave_dev) == BMSR_LSTATUS)) {
#ifdef BONDING_DEBUG #ifdef BONDING_DEBUG
printk(KERN_CRIT "Initial state of slave_dev is BOND_LINK_UP\n"); printk(KERN_CRIT "Initial state of slave_dev is BOND_LINK_UP\n");
#endif #endif
...@@ -1220,7 +1225,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1220,7 +1225,7 @@ static void bond_mii_monitor(struct net_device *master)
switch (slave->link) { switch (slave->link) {
case BOND_LINK_UP: /* the link was up */ 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 /* link stays up, tell that this one
is immediately available */ is immediately available */
if (IS_UP(dev) && (mindelay > -2)) { if (IS_UP(dev) && (mindelay > -2)) {
...@@ -1256,7 +1261,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1256,7 +1261,7 @@ static void bond_mii_monitor(struct net_device *master)
ensure proper action to be taken ensure proper action to be taken
*/ */
case BOND_LINK_FAIL: /* the link has just gone down */ 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 */ /* link stays down */
if (slave->delay <= 0) { if (slave->delay <= 0) {
/* link down for too long time */ /* link down for too long time */
...@@ -1285,7 +1290,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1285,7 +1290,7 @@ static void bond_mii_monitor(struct net_device *master)
} else { } else {
slave->delay--; slave->delay--;
} }
} else if ((link_state & MII_LINK_READY) == MII_LINK_READY) { } else {
/* link up again */ /* link up again */
slave->link = BOND_LINK_UP; slave->link = BOND_LINK_UP;
printk(KERN_INFO printk(KERN_INFO
...@@ -1304,7 +1309,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1304,7 +1309,7 @@ static void bond_mii_monitor(struct net_device *master)
} }
break; break;
case BOND_LINK_DOWN: /* the link was down */ 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 */ /* the link stays down, nothing more to do */
break; break;
} else { /* link going up */ } else { /* link going up */
...@@ -1326,7 +1331,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1326,7 +1331,7 @@ static void bond_mii_monitor(struct net_device *master)
case there's something to do. case there's something to do.
*/ */
case BOND_LINK_BACK: /* the link has just come back */ 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 */ /* link down again */
slave->link = BOND_LINK_DOWN; slave->link = BOND_LINK_DOWN;
printk(KERN_INFO printk(KERN_INFO
...@@ -1335,8 +1340,7 @@ static void bond_mii_monitor(struct net_device *master) ...@@ -1335,8 +1340,7 @@ static void bond_mii_monitor(struct net_device *master)
master->name, master->name,
(updelay - slave->delay) * miimon, (updelay - slave->delay) * miimon,
dev->name); dev->name);
} } else {
else if ((link_state & MII_LINK_READY) == MII_LINK_READY) {
/* link stays up */ /* link stays up */
if (slave->delay == 0) { if (slave->delay == 0) {
/* now the link has been up for long time enough */ /* 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) ...@@ -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, "MII Status: ");
len += sprintf(buf + len, 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", len += sprintf(buf + len, "MII Polling Interval (ms): %d\n",
miimon); miimon);
len += sprintf(buf + len, "Up Delay (ms): %d\n", updelay); len += sprintf(buf + len, "Up Delay (ms): %d\n", updelay);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment