Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH iproute2 v1] Add mdb command to bridge
From: Cong Wang @ 2012-11-30 14:50 UTC (permalink / raw)
  To: Thomas Graf
  Cc: netdev, bridge, Herbert Xu, Stephen Hemminger, David S. Miller,
	Jesper Dangaard Brouer
In-Reply-To: <20121130105449.GE30697@casper.infradead.org>

On Fri, 2012-11-30 at 10:54 +0000, Thomas Graf wrote:
> 
> You shouldn't need to duplicate the attribute ids and struct br_port_msg
> in iproute2. Just put these definitions in a uapi kernel header and
> include the header from iproute2.

Right, I was confused by include/linux/rtnetlink.h, thought iproute2 has
its own copy of these headers, but they should be exported from kernel.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next v1 1/2] bridge: export port_no and port_id via IFA_INFO_DATA
From: Cong Wang @ 2012-11-30 14:51 UTC (permalink / raw)
  To: Thomas Graf
  Cc: netdev, bridge, Herbert Xu, Jesper Dangaard Brouer,
	Stephen Hemminger, David S. Miller
In-Reply-To: <20121130101814.GD30697@casper.infradead.org>

On Fri, 2012-11-30 at 10:18 +0000, Thomas Graf wrote:
> On 11/30/12 at 05:58pm, Cong Wang wrote:
> > @@ -168,6 +172,8 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
> >  	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
> >  	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
> >  	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
> > +	[IFLA_BRPORT_NO]	= { .type = NLA_U16 },
> > +	[IFLA_BRPORT_ID]	= { .type = NLA_U16 },
> >  };
> >  
> >  /* Change the state of the port and notify spanning tree */
> 
> I missed this in the first round. Not much of a point in adding policy
> entries if the attributes are read-only as in this case.

Yeah, remove these lines now.

^ permalink raw reply

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Eric Dumazet @ 2012-11-30 14:52 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert
In-Reply-To: <1354269846.11754.381.camel@localhost>

On Fri, 2012-11-30 at 11:04 +0100, Jesper Dangaard Brouer wrote:
> So, let me instead show, with tests, that the evictor strategy is
> broken, while keeping the original default thresh settings:
> 
> # grep . /proc/sys/net/ipv4/ipfrag_*_thresh
> /proc/sys/net/ipv4/ipfrag_high_thresh:262144
> /proc/sys/net/ipv4/ipfrag_low_thresh:196608
> 
> Test purpose, I will on a single 10G link demonstrate, that starting
> several "N" netperf UDP fragmentation flows, will hurt performance, and
> then claim this is caused by the bad evictor strategy.
> 
> Test setup:
>  - Disable Ethernet flow control
>  - netperf packet size 65507
>  - Run netserver on one NUMA node
>  - Start netperf clients against a NIC on the other NUMA node
>  - (The NUMA imbalance helps the effect occur at lower N) 
> 
> Result: N=1  8040 Mbit/s
> Result: N=2  9584 Mbit/s (4739+4845)
> Result: N=3  4055 Mbit/s (1436+1371+1248)
> Result: N=4  2247 Mbit/s (1538+29+54+626)
> Result: N=5   879 Mbit/s (78+152+226+125+298)
> Result: N=6   293 Mbit/s (85+55+32+57+46+18)
> Result: N=7   354 Mbit/s (70+47+33+80+20+72+32)
> 
> Can we, now, agree that the current evictor strategy is broken?!?

Your setup is broken for sure. I dont know how you expect that many
datagrams being correctly reassembled with ipfrag_high_thresh=262144 

No matter strategy is implemented, an attacker knows it and can send
frags so that regular workload is denied. Kernel cant decide which
packets are more likely to be completed.

BTW, install fq_codel at the sender side, so that frags are nicely
interleaved. Because on real networks, frags of an UDP datagram rarely
come to destination in a single train with no alien packets inside the
train.

You focus on a particular lab setup and particular workload, you
should consider that if an application _really_ depends on frags, then
the receiver is likely to be a target for various frag attacks.

^ permalink raw reply

* [PATCH 0/2] smsc75xx enhancements
From: Steve Glendinning @ 2012-11-30 14:52 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning

This patchset, as requestes, expands the macros used in
smsc75xx to make the flow control clearer.

I've left the fix in a separate patch, so the macro-
expanding patch should have zero functional change.

Steve Glendinning (2):
  smsc75xx: don't call usbnet_resume if usbnet_suspend fails
  smsc75xx: expand check_ macros

 drivers/net/usb/smsc75xx.c |  735 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 576 insertions(+), 159 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH 1/2] smsc75xx: don't call usbnet_resume if usbnet_suspend fails
From: Steve Glendinning @ 2012-11-30 14:52 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <1354287164-9884-1-git-send-email-steve.glendinning@shawell.net>

If usbnet_suspend returns an error we don't want to call
usbnet_resume to clean up, but instead just return the error.

If usbnet_suspend *does* succeed, and we have a problem further
on, the desired behaviour is still to call usbnet_resume
to clean up before returning.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc75xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 1823806..86d9249 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -1411,7 +1411,7 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 	int ret;
 
 	ret = usbnet_suspend(intf, message);
-	check_warn_goto_done(ret, "usbnet_suspend error\n");
+	check_warn_return(ret, "usbnet_suspend error\n");
 
 	if (pdata->suspend_flags) {
 		netdev_warn(dev->net, "error during last resume\n");
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/2] smsc75xx: expand check_ macros
From: Steve Glendinning @ 2012-11-30 14:52 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <1354287164-9884-1-git-send-email-steve.glendinning@shawell.net>

These macros, while reducing the amount of code, hide flow control
and make the code more confusing to follow and review.  This patch
expands them.  It should have no functional effect on the driver.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc75xx.c |  735 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 576 insertions(+), 159 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 86d9249..1cbd936 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -64,15 +64,6 @@
 #define SUSPEND_ALLMODES		(SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
 					 SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
 
-#define check_warn(ret, fmt, args...) \
-	({ if (ret < 0) netdev_warn(dev->net, fmt, ##args); })
-
-#define check_warn_return(ret, fmt, args...) \
-	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); return ret; } })
-
-#define check_warn_goto_done(ret, fmt, args...) \
-	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); goto done; } })
-
 struct smsc75xx_priv {
 	struct usbnet *dev;
 	u32 rfe_ctl;
@@ -182,7 +173,10 @@ static __must_check int __smsc75xx_phy_wait_not_busy(struct usbnet *dev,
 
 	do {
 		ret = __smsc75xx_read_reg(dev, MII_ACCESS, &val, in_pm);
-		check_warn_return(ret, "Error reading MII_ACCESS\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading MII_ACCESS\n");
+			return ret;
+		}
 
 		if (!(val & MII_ACCESS_BUSY))
 			return 0;
@@ -202,7 +196,10 @@ static int __smsc75xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
 
 	/* confirm MII not busy */
 	ret = __smsc75xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "MII is busy in smsc75xx_mdio_read\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "MII is busy in smsc75xx_mdio_read\n");
+		goto done;
+	}
 
 	/* set the address, index & direction (read from PHY) */
 	phy_id &= dev->mii.phy_id_mask;
@@ -211,13 +208,22 @@ static int __smsc75xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
 		| ((idx << MII_ACCESS_REG_ADDR_SHIFT) & MII_ACCESS_REG_ADDR)
 		| MII_ACCESS_READ | MII_ACCESS_BUSY;
 	ret = __smsc75xx_write_reg(dev, MII_ACCESS, addr, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_ACCESS\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_ACCESS\n");
+		goto done;
+	}
 
 	ret = __smsc75xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "Timed out reading MII reg %02X\n", idx);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Timed out reading MII reg %02X\n", idx);
+		goto done;
+	}
 
 	ret = __smsc75xx_read_reg(dev, MII_DATA, &val, in_pm);
-	check_warn_goto_done(ret, "Error reading MII_DATA\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_DATA\n");
+		goto done;
+	}
 
 	ret = (u16)(val & 0xFFFF);
 
@@ -237,11 +243,17 @@ static void __smsc75xx_mdio_write(struct net_device *netdev, int phy_id,
 
 	/* confirm MII not busy */
 	ret = __smsc75xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "MII is busy in smsc75xx_mdio_write\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "MII is busy in smsc75xx_mdio_write\n");
+		goto done;
+	}
 
 	val = regval;
 	ret = __smsc75xx_write_reg(dev, MII_DATA, val, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_DATA\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_DATA\n");
+		goto done;
+	}
 
 	/* set the address, index & direction (write to PHY) */
 	phy_id &= dev->mii.phy_id_mask;
@@ -250,10 +262,16 @@ static void __smsc75xx_mdio_write(struct net_device *netdev, int phy_id,
 		| ((idx << MII_ACCESS_REG_ADDR_SHIFT) & MII_ACCESS_REG_ADDR)
 		| MII_ACCESS_WRITE | MII_ACCESS_BUSY;
 	ret = __smsc75xx_write_reg(dev, MII_ACCESS, addr, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_ACCESS\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_ACCESS\n");
+		goto done;
+	}
 
 	ret = __smsc75xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "Timed out writing MII reg %02X\n", idx);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Timed out writing MII reg %02X\n", idx);
+		goto done;
+	}
 
 done:
 	mutex_unlock(&dev->phy_mutex);
@@ -290,7 +308,10 @@ static int smsc75xx_wait_eeprom(struct usbnet *dev)
 
 	do {
 		ret = smsc75xx_read_reg(dev, E2P_CMD, &val);
-		check_warn_return(ret, "Error reading E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_CMD\n");
+			return ret;
+		}
 
 		if (!(val & E2P_CMD_BUSY) || (val & E2P_CMD_TIMEOUT))
 			break;
@@ -313,7 +334,10 @@ static int smsc75xx_eeprom_confirm_not_busy(struct usbnet *dev)
 
 	do {
 		ret = smsc75xx_read_reg(dev, E2P_CMD, &val);
-		check_warn_return(ret, "Error reading E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_CMD\n");
+			return ret;
+		}
 
 		if (!(val & E2P_CMD_BUSY))
 			return 0;
@@ -341,14 +365,20 @@ static int smsc75xx_read_eeprom(struct usbnet *dev, u32 offset, u32 length,
 	for (i = 0; i < length; i++) {
 		val = E2P_CMD_BUSY | E2P_CMD_READ | (offset & E2P_CMD_ADDR);
 		ret = smsc75xx_write_reg(dev, E2P_CMD, val);
-		check_warn_return(ret, "Error writing E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_CMD\n");
+			return ret;
+		}
 
 		ret = smsc75xx_wait_eeprom(dev);
 		if (ret < 0)
 			return ret;
 
 		ret = smsc75xx_read_reg(dev, E2P_DATA, &val);
-		check_warn_return(ret, "Error reading E2P_DATA\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_DATA\n");
+			return ret;
+		}
 
 		data[i] = val & 0xFF;
 		offset++;
@@ -373,7 +403,10 @@ static int smsc75xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 	/* Issue write/erase enable command */
 	val = E2P_CMD_BUSY | E2P_CMD_EWEN;
 	ret = smsc75xx_write_reg(dev, E2P_CMD, val);
-	check_warn_return(ret, "Error writing E2P_CMD\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing E2P_CMD\n");
+		return ret;
+	}
 
 	ret = smsc75xx_wait_eeprom(dev);
 	if (ret < 0)
@@ -384,12 +417,18 @@ static int smsc75xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 		/* Fill data register */
 		val = data[i];
 		ret = smsc75xx_write_reg(dev, E2P_DATA, val);
-		check_warn_return(ret, "Error writing E2P_DATA\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_DATA\n");
+			return ret;
+		}
 
 		/* Send "write" command */
 		val = E2P_CMD_BUSY | E2P_CMD_WRITE | (offset & E2P_CMD_ADDR);
 		ret = smsc75xx_write_reg(dev, E2P_CMD, val);
-		check_warn_return(ret, "Error writing E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_CMD\n");
+			return ret;
+		}
 
 		ret = smsc75xx_wait_eeprom(dev);
 		if (ret < 0)
@@ -408,7 +447,10 @@ static int smsc75xx_dataport_wait_not_busy(struct usbnet *dev)
 	for (i = 0; i < 100; i++) {
 		u32 dp_sel;
 		ret = smsc75xx_read_reg(dev, DP_SEL, &dp_sel);
-		check_warn_return(ret, "Error reading DP_SEL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading DP_SEL\n");
+			return ret;
+		}
 
 		if (dp_sel & DP_SEL_DPRDY)
 			return 0;
@@ -431,28 +473,49 @@ static int smsc75xx_dataport_write(struct usbnet *dev, u32 ram_select, u32 addr,
 	mutex_lock(&pdata->dataport_mutex);
 
 	ret = smsc75xx_dataport_wait_not_busy(dev);
-	check_warn_goto_done(ret, "smsc75xx_dataport_write busy on entry\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "smsc75xx_dataport_write busy on entry\n");
+		goto done;
+	}
 
 	ret = smsc75xx_read_reg(dev, DP_SEL, &dp_sel);
-	check_warn_goto_done(ret, "Error reading DP_SEL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading DP_SEL\n");
+		goto done;
+	}
 
 	dp_sel &= ~DP_SEL_RSEL;
 	dp_sel |= ram_select;
 	ret = smsc75xx_write_reg(dev, DP_SEL, dp_sel);
-	check_warn_goto_done(ret, "Error writing DP_SEL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing DP_SEL\n");
+		goto done;
+	}
 
 	for (i = 0; i < length; i++) {
 		ret = smsc75xx_write_reg(dev, DP_ADDR, addr + i);
-		check_warn_goto_done(ret, "Error writing DP_ADDR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing DP_ADDR\n");
+			goto done;
+		}
 
 		ret = smsc75xx_write_reg(dev, DP_DATA, buf[i]);
-		check_warn_goto_done(ret, "Error writing DP_DATA\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing DP_DATA\n");
+			goto done;
+		}
 
 		ret = smsc75xx_write_reg(dev, DP_CMD, DP_CMD_WRITE);
-		check_warn_goto_done(ret, "Error writing DP_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing DP_CMD\n");
+			goto done;
+		}
 
 		ret = smsc75xx_dataport_wait_not_busy(dev);
-		check_warn_goto_done(ret, "smsc75xx_dataport_write timeout\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "smsc75xx_dataport_write timeout\n");
+			goto done;
+		}
 	}
 
 done:
@@ -480,7 +543,8 @@ static void smsc75xx_deferred_multicast_write(struct work_struct *param)
 		DP_SEL_VHF_HASH_LEN, pdata->multicast_hash_table);
 
 	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
-	check_warn(ret, "Error writing RFE_CRL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error writing RFE_CRL\n");
 }
 
 static void smsc75xx_set_multicast(struct net_device *netdev)
@@ -554,10 +618,16 @@ static int smsc75xx_update_flowcontrol(struct usbnet *dev, u8 duplex,
 	}
 
 	ret = smsc75xx_write_reg(dev, FLOW, flow);
-	check_warn_return(ret, "Error writing FLOW\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing FLOW\n");
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, FCT_FLOW, fct_flow);
-	check_warn_return(ret, "Error writing FCT_FLOW\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing FCT_FLOW\n");
+		return ret;
+	}
 
 	return 0;
 }
@@ -574,7 +644,10 @@ static int smsc75xx_link_reset(struct usbnet *dev)
 		PHY_INT_SRC_CLEAR_ALL);
 
 	ret = smsc75xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL);
-	check_warn_return(ret, "Error writing INT_STS\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing INT_STS\n");
+		return ret;
+	}
 
 	mii_check_media(mii, 1, 1);
 	mii_ethtool_gset(&dev->mii, &ecmd);
@@ -658,9 +731,10 @@ static int smsc75xx_ethtool_set_wol(struct net_device *net,
 	pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
 
 	ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
-	check_warn_return(ret, "device_set_wakeup_enable error %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "device_set_wakeup_enable error %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 static const struct ethtool_ops smsc75xx_ethtool_ops = {
@@ -713,19 +787,29 @@ static int smsc75xx_set_mac_address(struct usbnet *dev)
 	u32 addr_hi = dev->net->dev_addr[4] | dev->net->dev_addr[5] << 8;
 
 	int ret = smsc75xx_write_reg(dev, RX_ADDRH, addr_hi);
-	check_warn_return(ret, "Failed to write RX_ADDRH: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write RX_ADDRH: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, RX_ADDRL, addr_lo);
-	check_warn_return(ret, "Failed to write RX_ADDRL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write RX_ADDRL: %d\n", ret);
+		return ret;
+	}
 
 	addr_hi |= ADDR_FILTX_FB_VALID;
 	ret = smsc75xx_write_reg(dev, ADDR_FILTX, addr_hi);
-	check_warn_return(ret, "Failed to write ADDR_FILTX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write ADDR_FILTX: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, ADDR_FILTX + 4, addr_lo);
-	check_warn_return(ret, "Failed to write ADDR_FILTX+4: %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "Failed to write ADDR_FILTX+4: %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 static int smsc75xx_phy_initialize(struct usbnet *dev)
@@ -747,7 +831,10 @@ static int smsc75xx_phy_initialize(struct usbnet *dev)
 	do {
 		msleep(10);
 		bmcr = smsc75xx_mdio_read(dev->net, dev->mii.phy_id, MII_BMCR);
-		check_warn_return(bmcr, "Error reading MII_BMCR\n");
+		if (bmcr < 0) {
+			netdev_warn(dev->net, "Error reading MII_BMCR\n");
+			return bmcr;
+		}
 		timeout++;
 	} while ((bmcr & BMCR_RESET) && (timeout < 100));
 
@@ -764,7 +851,11 @@ static int smsc75xx_phy_initialize(struct usbnet *dev)
 
 	/* read and write to clear phy interrupt status */
 	ret = smsc75xx_mdio_read(dev->net, dev->mii.phy_id, PHY_INT_SRC);
-	check_warn_return(ret, "Error reading PHY_INT_SRC\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+		return ret;
+	}
+
 	smsc75xx_mdio_write(dev->net, dev->mii.phy_id, PHY_INT_SRC, 0xffff);
 
 	smsc75xx_mdio_write(dev->net, dev->mii.phy_id, PHY_INT_MASK,
@@ -782,14 +873,20 @@ static int smsc75xx_set_rx_max_frame_length(struct usbnet *dev, int size)
 	bool rxenabled;
 
 	ret = smsc75xx_read_reg(dev, MAC_RX, &buf);
-	check_warn_return(ret, "Failed to read MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_RX: %d\n", ret);
+		return ret;
+	}
 
 	rxenabled = ((buf & MAC_RX_RXEN) != 0);
 
 	if (rxenabled) {
 		buf &= ~MAC_RX_RXEN;
 		ret = smsc75xx_write_reg(dev, MAC_RX, buf);
-		check_warn_return(ret, "Failed to write MAC_RX: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to write MAC_RX: %d\n", ret);
+			return ret;
+		}
 	}
 
 	/* add 4 to size for FCS */
@@ -797,12 +894,18 @@ static int smsc75xx_set_rx_max_frame_length(struct usbnet *dev, int size)
 	buf |= (((size + 4) << MAC_RX_MAX_SIZE_SHIFT) & MAC_RX_MAX_SIZE);
 
 	ret = smsc75xx_write_reg(dev, MAC_RX, buf);
-	check_warn_return(ret, "Failed to write MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_RX: %d\n", ret);
+		return ret;
+	}
 
 	if (rxenabled) {
 		buf |= MAC_RX_RXEN;
 		ret = smsc75xx_write_reg(dev, MAC_RX, buf);
-		check_warn_return(ret, "Failed to write MAC_RX: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to write MAC_RX: %d\n", ret);
+			return ret;
+		}
 	}
 
 	return 0;
@@ -813,7 +916,10 @@ static int smsc75xx_change_mtu(struct net_device *netdev, int new_mtu)
 	struct usbnet *dev = netdev_priv(netdev);
 
 	int ret = smsc75xx_set_rx_max_frame_length(dev, new_mtu);
-	check_warn_return(ret, "Failed to set mac rx frame length\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to set mac rx frame length\n");
+		return ret;
+	}
 
 	return usbnet_change_mtu(netdev, new_mtu);
 }
@@ -838,9 +944,10 @@ static int smsc75xx_set_features(struct net_device *netdev,
 	/* it's racing here! */
 
 	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
-	check_warn_return(ret, "Error writing RFE_CTL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error writing RFE_CTL\n");
 
-	return 0;
+	return ret;
 }
 
 static int smsc75xx_wait_ready(struct usbnet *dev, int in_pm)
@@ -853,7 +960,10 @@ static int smsc75xx_wait_ready(struct usbnet *dev, int in_pm)
 
 		ret = __smsc75xx_read_reg(dev, PMT_CTL, &buf, in_pm);
 
-		check_warn_return(ret, "Failed to read PMT_CTL: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read PMT_CTL: %d\n", ret);
+			return ret;
+		}
 
 		if (buf & PMT_CTL_DEV_RDY)
 			return 0;
@@ -875,21 +985,33 @@ static int smsc75xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net, "entering smsc75xx_reset\n");
 
 	ret = smsc75xx_wait_ready(dev, 0);
-	check_warn_return(ret, "device not ready in smsc75xx_reset\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "device not ready in smsc75xx_reset\n");
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	buf |= HW_CFG_LRST;
 
 	ret = smsc75xx_write_reg(dev, HW_CFG, buf);
-	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	timeout = 0;
 	do {
 		msleep(10);
 		ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-		check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+			return ret;
+		}
 		timeout++;
 	} while ((buf & HW_CFG_LRST) && (timeout < 100));
 
@@ -901,18 +1023,27 @@ static int smsc75xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net, "Lite reset complete, resetting PHY\n");
 
 	ret = smsc75xx_read_reg(dev, PMT_CTL, &buf);
-	check_warn_return(ret, "Failed to read PMT_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read PMT_CTL: %d\n", ret);
+		return ret;
+	}
 
 	buf |= PMT_CTL_PHY_RST;
 
 	ret = smsc75xx_write_reg(dev, PMT_CTL, buf);
-	check_warn_return(ret, "Failed to write PMT_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write PMT_CTL: %d\n", ret);
+		return ret;
+	}
 
 	timeout = 0;
 	do {
 		msleep(10);
 		ret = smsc75xx_read_reg(dev, PMT_CTL, &buf);
-		check_warn_return(ret, "Failed to read PMT_CTL: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read PMT_CTL: %d\n", ret);
+			return ret;
+		}
 		timeout++;
 	} while ((buf & PMT_CTL_PHY_RST) && (timeout < 100));
 
@@ -926,13 +1057,19 @@ static int smsc75xx_reset(struct usbnet *dev)
 	smsc75xx_init_mac_address(dev);
 
 	ret = smsc75xx_set_mac_address(dev);
-	check_warn_return(ret, "Failed to set mac address\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to set mac address\n");
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "MAC Address: %pM\n",
 		  dev->net->dev_addr);
 
 	ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG : 0x%08x\n",
 		  buf);
@@ -940,10 +1077,16 @@ static int smsc75xx_reset(struct usbnet *dev)
 	buf |= HW_CFG_BIR;
 
 	ret = smsc75xx_write_reg(dev, HW_CFG, buf);
-	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net,  "Failed to write HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG after writing HW_CFG_BIR: 0x%08x\n",
 		  buf);
@@ -963,36 +1106,57 @@ static int smsc75xx_reset(struct usbnet *dev)
 		  (ulong)dev->rx_urb_size);
 
 	ret = smsc75xx_write_reg(dev, BURST_CAP, buf);
-	check_warn_return(ret, "Failed to write BURST_CAP: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write BURST_CAP: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, BURST_CAP, &buf);
-	check_warn_return(ret, "Failed to read BURST_CAP: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read BURST_CAP: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BURST_CAP after writing: 0x%08x\n", buf);
 
 	ret = smsc75xx_write_reg(dev, BULK_IN_DLY, DEFAULT_BULK_IN_DELAY);
-	check_warn_return(ret, "Failed to write BULK_IN_DLY: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write BULK_IN_DLY: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, BULK_IN_DLY, &buf);
-	check_warn_return(ret, "Failed to read BULK_IN_DLY: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read BULK_IN_DLY: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BULK_IN_DLY after writing: 0x%08x\n", buf);
 
 	if (turbo_mode) {
 		ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-		check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+			return ret;
+		}
 
 		netif_dbg(dev, ifup, dev->net, "HW_CFG: 0x%08x\n", buf);
 
 		buf |= (HW_CFG_MEF | HW_CFG_BCE);
 
 		ret = smsc75xx_write_reg(dev, HW_CFG, buf);
-		check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to write HW_CFG: %d\n", ret);
+			return ret;
+		}
 
 		ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
-		check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+			return ret;
+		}
 
 		netif_dbg(dev, ifup, dev->net, "HW_CFG: 0x%08x\n", buf);
 	}
@@ -1000,58 +1164,92 @@ static int smsc75xx_reset(struct usbnet *dev)
 	/* set FIFO sizes */
 	buf = (MAX_RX_FIFO_SIZE - 512) / 512;
 	ret = smsc75xx_write_reg(dev, FCT_RX_FIFO_END, buf);
-	check_warn_return(ret, "Failed to write FCT_RX_FIFO_END: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FCT_RX_FIFO_END: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "FCT_RX_FIFO_END set to 0x%08x\n", buf);
 
 	buf = (MAX_TX_FIFO_SIZE - 512) / 512;
 	ret = smsc75xx_write_reg(dev, FCT_TX_FIFO_END, buf);
-	check_warn_return(ret, "Failed to write FCT_TX_FIFO_END: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FCT_TX_FIFO_END: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "FCT_TX_FIFO_END set to 0x%08x\n", buf);
 
 	ret = smsc75xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL);
-	check_warn_return(ret, "Failed to write INT_STS: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write INT_STS: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, ID_REV, &buf);
-	check_warn_return(ret, "Failed to read ID_REV: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "ID_REV = 0x%08x\n", buf);
 
 	ret = smsc75xx_read_reg(dev, E2P_CMD, &buf);
-	check_warn_return(ret, "Failed to read E2P_CMD: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read E2P_CMD: %d\n", ret);
+		return ret;
+	}
 
 	/* only set default GPIO/LED settings if no EEPROM is detected */
 	if (!(buf & E2P_CMD_LOADED)) {
 		ret = smsc75xx_read_reg(dev, LED_GPIO_CFG, &buf);
-		check_warn_return(ret, "Failed to read LED_GPIO_CFG: %d\n",
-				  ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read LED_GPIO_CFG: %d\n", ret);
+			return ret;
+		}
 
 		buf &= ~(LED_GPIO_CFG_LED2_FUN_SEL | LED_GPIO_CFG_LED10_FUN_SEL);
 		buf |= LED_GPIO_CFG_LEDGPIO_EN | LED_GPIO_CFG_LED2_FUN_SEL;
 
 		ret = smsc75xx_write_reg(dev, LED_GPIO_CFG, buf);
-		check_warn_return(ret, "Failed to write LED_GPIO_CFG: %d\n",
-				  ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to write LED_GPIO_CFG: %d\n", ret);
+			return ret;
+		}
 	}
 
 	ret = smsc75xx_write_reg(dev, FLOW, 0);
-	check_warn_return(ret, "Failed to write FLOW: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FLOW: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, FCT_FLOW, 0);
-	check_warn_return(ret, "Failed to write FCT_FLOW: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FCT_FLOW: %d\n", ret);
+		return ret;
+	}
 
 	/* Don't need rfe_ctl_lock during initialisation */
 	ret = smsc75xx_read_reg(dev, RFE_CTL, &pdata->rfe_ctl);
-	check_warn_return(ret, "Failed to read RFE_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read RFE_CTL: %d\n", ret);
+		return ret;
+	}
 
 	pdata->rfe_ctl |= RFE_CTL_AB | RFE_CTL_DPF;
 
 	ret = smsc75xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
-	check_warn_return(ret, "Failed to write RFE_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write RFE_CTL: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, RFE_CTL, &pdata->rfe_ctl);
-	check_warn_return(ret, "Failed to read RFE_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read RFE_CTL: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "RFE_CTL set to 0x%08x\n",
 		  pdata->rfe_ctl);
@@ -1062,65 +1260,107 @@ static int smsc75xx_reset(struct usbnet *dev)
 	smsc75xx_set_multicast(dev->net);
 
 	ret = smsc75xx_phy_initialize(dev);
-	check_warn_return(ret, "Failed to initialize PHY: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to initialize PHY: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, INT_EP_CTL, &buf);
-	check_warn_return(ret, "Failed to read INT_EP_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read INT_EP_CTL: %d\n", ret);
+		return ret;
+	}
 
 	/* enable PHY interrupts */
 	buf |= INT_ENP_PHY_INT;
 
 	ret = smsc75xx_write_reg(dev, INT_EP_CTL, buf);
-	check_warn_return(ret, "Failed to write INT_EP_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write INT_EP_CTL: %d\n", ret);
+		return ret;
+	}
 
 	/* allow mac to detect speed and duplex from phy */
 	ret = smsc75xx_read_reg(dev, MAC_CR, &buf);
-	check_warn_return(ret, "Failed to read MAC_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_CR: %d\n", ret);
+		return ret;
+	}
 
 	buf |= (MAC_CR_ADD | MAC_CR_ASD);
 	ret = smsc75xx_write_reg(dev, MAC_CR, buf);
-	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_CR: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, MAC_TX, &buf);
-	check_warn_return(ret, "Failed to read MAC_TX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_TX: %d\n", ret);
+		return ret;
+	}
 
 	buf |= MAC_TX_TXEN;
 
 	ret = smsc75xx_write_reg(dev, MAC_TX, buf);
-	check_warn_return(ret, "Failed to write MAC_TX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_TX: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "MAC_TX set to 0x%08x\n", buf);
 
 	ret = smsc75xx_read_reg(dev, FCT_TX_CTL, &buf);
-	check_warn_return(ret, "Failed to read FCT_TX_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read FCT_TX_CTL: %d\n", ret);
+		return ret;
+	}
 
 	buf |= FCT_TX_CTL_EN;
 
 	ret = smsc75xx_write_reg(dev, FCT_TX_CTL, buf);
-	check_warn_return(ret, "Failed to write FCT_TX_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FCT_TX_CTL: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "FCT_TX_CTL set to 0x%08x\n", buf);
 
 	ret = smsc75xx_set_rx_max_frame_length(dev, 1514);
-	check_warn_return(ret, "Failed to set max rx frame length\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to set max rx frame length\n");
+		return ret;
+	}
 
 	ret = smsc75xx_read_reg(dev, MAC_RX, &buf);
-	check_warn_return(ret, "Failed to read MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_RX: %d\n", ret);
+		return ret;
+	}
 
 	buf |= MAC_RX_RXEN;
 
 	ret = smsc75xx_write_reg(dev, MAC_RX, buf);
-	check_warn_return(ret, "Failed to write MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_RX: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "MAC_RX set to 0x%08x\n", buf);
 
 	ret = smsc75xx_read_reg(dev, FCT_RX_CTL, &buf);
-	check_warn_return(ret, "Failed to read FCT_RX_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read FCT_RX_CTL: %d\n", ret);
+		return ret;
+	}
 
 	buf |= FCT_RX_CTL_EN;
 
 	ret = smsc75xx_write_reg(dev, FCT_RX_CTL, buf);
-	check_warn_return(ret, "Failed to write FCT_RX_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FCT_RX_CTL: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "FCT_RX_CTL set to 0x%08x\n", buf);
 
@@ -1149,7 +1389,10 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	printk(KERN_INFO SMSC_CHIPNAME " v" SMSC_DRIVER_VERSION "\n");
 
 	ret = usbnet_get_endpoints(dev, intf);
-	check_warn_return(ret, "usbnet_get_endpoints failed: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "usbnet_get_endpoints failed: %d\n", ret);
+		return ret;
+	}
 
 	dev->data[0] = (unsigned long)kzalloc(sizeof(struct smsc75xx_priv),
 		GFP_KERNEL);
@@ -1181,7 +1424,10 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	/* Init all registers */
 	ret = smsc75xx_reset(dev);
-	check_warn_return(ret, "smsc75xx_reset error %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "smsc75xx_reset error %d\n", ret);
+		return ret;
+	}
 
 	dev->net->netdev_ops = &smsc75xx_netdev_ops;
 	dev->net->ethtool_ops = &smsc75xx_ethtool_ops;
@@ -1215,19 +1461,34 @@ static int smsc75xx_write_wuff(struct usbnet *dev, int filter, u32 wuf_cfg,
 	int ret;
 
 	ret = smsc75xx_write_reg(dev, cfg_base, wuf_cfg);
-	check_warn_return(ret, "Error writing WUF_CFGX\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUF_CFGX\n");
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, mask_base, wuf_mask1);
-	check_warn_return(ret, "Error writing WUF_MASKX\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUF_MASKX\n");
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, mask_base + 4, 0);
-	check_warn_return(ret, "Error writing WUF_MASKX\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUF_MASKX\n");
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, mask_base + 8, 0);
-	check_warn_return(ret, "Error writing WUF_MASKX\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUF_MASKX\n");
+		return ret;
+	}
 
 	ret = smsc75xx_write_reg(dev, mask_base + 12, 0);
-	check_warn_return(ret, "Error writing WUF_MASKX\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUF_MASKX\n");
+		return ret;
+	}
 
 	return 0;
 }
@@ -1239,13 +1500,19 @@ static int smsc75xx_enter_suspend0(struct usbnet *dev)
 	int ret;
 
 	ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-	check_warn_return(ret, "Error reading PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PMT_CTL\n");
+		return ret;
+	}
 
 	val &= (~(PMT_CTL_SUS_MODE | PMT_CTL_PHY_RST));
 	val |= PMT_CTL_SUS_MODE_0 | PMT_CTL_WOL_EN | PMT_CTL_WUPS;
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	pdata->suspend_flags |= SUSPEND_SUSPEND0;
 
@@ -1259,20 +1526,29 @@ static int smsc75xx_enter_suspend1(struct usbnet *dev)
 	int ret;
 
 	ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-	check_warn_return(ret, "Error reading PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PMT_CTL\n");
+		return ret;
+	}
 
 	val &= ~(PMT_CTL_SUS_MODE | PMT_CTL_WUPS | PMT_CTL_PHY_RST);
 	val |= PMT_CTL_SUS_MODE_1;
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	/* clear wol status, enable energy detection */
 	val &= ~PMT_CTL_WUPS;
 	val |= (PMT_CTL_WUPS_ED | PMT_CTL_ED_EN);
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	pdata->suspend_flags |= SUSPEND_SUSPEND1;
 
@@ -1286,13 +1562,19 @@ static int smsc75xx_enter_suspend2(struct usbnet *dev)
 	int ret;
 
 	ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-	check_warn_return(ret, "Error reading PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PMT_CTL\n");
+		return ret;
+	}
 
 	val &= ~(PMT_CTL_SUS_MODE | PMT_CTL_WUPS | PMT_CTL_PHY_RST);
 	val |= PMT_CTL_SUS_MODE_2;
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	pdata->suspend_flags |= SUSPEND_SUSPEND2;
 
@@ -1306,7 +1588,10 @@ static int smsc75xx_enter_suspend3(struct usbnet *dev)
 	int ret;
 
 	ret = smsc75xx_read_reg_nopm(dev, FCT_RX_CTL, &val);
-	check_warn_return(ret, "Error reading FCT_RX_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading FCT_RX_CTL\n");
+		return ret;
+	}
 
 	if (val & FCT_RX_CTL_RXUSED) {
 		netdev_dbg(dev->net, "rx fifo not empty in autosuspend\n");
@@ -1314,20 +1599,29 @@ static int smsc75xx_enter_suspend3(struct usbnet *dev)
 	}
 
 	ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-	check_warn_return(ret, "Error reading PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PMT_CTL\n");
+		return ret;
+	}
 
 	val &= ~(PMT_CTL_SUS_MODE | PMT_CTL_WUPS | PMT_CTL_PHY_RST);
 	val |= PMT_CTL_SUS_MODE_3 | PMT_CTL_RES_CLR_WKP_EN;
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	/* clear wol status */
 	val &= ~PMT_CTL_WUPS;
 	val |= PMT_CTL_WUPS_WOL;
 
 	ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-	check_warn_return(ret, "Error writing PMT_CTL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PMT_CTL\n");
+		return ret;
+	}
 
 	pdata->suspend_flags |= SUSPEND_SUSPEND3;
 
@@ -1343,11 +1637,17 @@ static int smsc75xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
 
 	/* read to clear */
 	ret = smsc75xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_SRC);
-	check_warn_return(ret, "Error reading PHY_INT_SRC\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+		return ret;
+	}
 
 	/* enable interrupt source */
 	ret = smsc75xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_MASK);
-	check_warn_return(ret, "Error reading PHY_INT_MASK\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_MASK\n");
+		return ret;
+	}
 
 	ret |= mask;
 
@@ -1363,10 +1663,16 @@ static int smsc75xx_link_ok_nopm(struct usbnet *dev)
 
 	/* first, a dummy read, needed to latch some MII phys */
 	ret = smsc75xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
-	check_warn_return(ret, "Error reading MII_BMSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_BMSR\n");
+		return ret;
+	}
 
 	ret = smsc75xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
-	check_warn_return(ret, "Error reading MII_BMSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_BMSR\n");
+		return ret;
+	}
 
 	return !!(ret & BMSR_LSTATUS);
 }
@@ -1388,7 +1694,10 @@ static int smsc75xx_autosuspend(struct usbnet *dev, u32 link_up)
 		/* enable PHY wakeup events for if cable is attached */
 		ret = smsc75xx_enable_phy_wakeup_interrupts(dev,
 			PHY_INT_MASK_ANEG_COMP);
-		check_warn_return(ret, "error enabling PHY wakeup ints\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+			return ret;
+		}
 
 		netdev_info(dev->net, "entering SUSPEND1 mode\n");
 		return smsc75xx_enter_suspend1(dev);
@@ -1397,7 +1706,10 @@ static int smsc75xx_autosuspend(struct usbnet *dev, u32 link_up)
 	/* enable PHY wakeup events so we remote wakeup if cable is pulled */
 	ret = smsc75xx_enable_phy_wakeup_interrupts(dev,
 		PHY_INT_MASK_LINK_DOWN);
-	check_warn_return(ret, "error enabling PHY wakeup ints\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+		return ret;
+	}
 
 	netdev_dbg(dev->net, "autosuspend entering SUSPEND3\n");
 	return smsc75xx_enter_suspend3(dev);
@@ -1411,7 +1723,10 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 	int ret;
 
 	ret = usbnet_suspend(intf, message);
-	check_warn_return(ret, "usbnet_suspend error\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "usbnet_suspend error\n");
+		return ret;
+	}
 
 	if (pdata->suspend_flags) {
 		netdev_warn(dev->net, "error during last resume\n");
@@ -1436,20 +1751,32 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		/* disable energy detect (link up) & wake up events */
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val &= ~(WUCSR_MPEN | WUCSR_WUEN);
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 
 		ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-		check_warn_goto_done(ret, "Error reading PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PMT_CTL\n");
+			goto done;
+		}
 
 		val &= ~(PMT_CTL_ED_EN | PMT_CTL_WOL_EN);
 
 		ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-		check_warn_goto_done(ret, "Error writing PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PMT_CTL\n");
+			goto done;
+		}
 
 		ret = smsc75xx_enter_suspend2(dev);
 		goto done;
@@ -1458,7 +1785,10 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 	if (pdata->wolopts & WAKE_PHY) {
 		ret = smsc75xx_enable_phy_wakeup_interrupts(dev,
 			(PHY_INT_MASK_ANEG_COMP | PHY_INT_MASK_LINK_DOWN));
-		check_warn_goto_done(ret, "error enabling PHY wakeup ints\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+			goto done;
+		}
 
 		/* if link is down then configure EDPD and enter SUSPEND1,
 		 * otherwise enter SUSPEND0 below
@@ -1470,7 +1800,10 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 			/* enable energy detect power-down mode */
 			ret = smsc75xx_mdio_read_nopm(dev->net, mii->phy_id,
 				PHY_MODE_CTRL_STS);
-			check_warn_goto_done(ret, "Error reading PHY_MODE_CTRL_STS\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error reading PHY_MODE_CTRL_STS\n");
+				goto done;
+			}
 
 			ret |= MODE_CTRL_STS_EDPWRDOWN;
 
@@ -1489,7 +1822,10 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 		/* disable all filters */
 		for (i = 0; i < WUF_NUM; i++) {
 			ret = smsc75xx_write_reg_nopm(dev, WUF_CFGX + i * 4, 0);
-			check_warn_goto_done(ret, "Error writing WUF_CFGX\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing WUF_CFGX\n");
+				goto done;
+			}
 		}
 
 		if (pdata->wolopts & WAKE_MCAST) {
@@ -1499,7 +1835,10 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 			val = WUF_CFGX_EN | WUF_CFGX_ATYPE_MULTICAST
 				| smsc_crc(mcast, 3);
 			ret = smsc75xx_write_wuff(dev, filter++, val, 0x0007);
-			check_warn_goto_done(ret, "Error writing wakeup filter\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing wakeup filter\n");
+				goto done;
+			}
 		}
 
 		if (pdata->wolopts & WAKE_ARP) {
@@ -1509,102 +1848,159 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
 			val = WUF_CFGX_EN | WUF_CFGX_ATYPE_ALL | (0x0C << 16)
 				| smsc_crc(arp, 2);
 			ret = smsc75xx_write_wuff(dev, filter++, val, 0x0003);
-			check_warn_goto_done(ret, "Error writing wakeup filter\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing wakeup filter\n");
+				goto done;
+			}
 		}
 
 		/* clear any pending pattern match packet status */
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_WUFR;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 
 		netdev_info(dev->net, "enabling packet match detection\n");
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_WUEN;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	} else {
 		netdev_info(dev->net, "disabling packet match detection\n");
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val &= ~WUCSR_WUEN;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	/* disable magic, bcast & unicast wakeup sources */
 	ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-	check_warn_goto_done(ret, "Error reading WUCSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading WUCSR\n");
+		goto done;
+	}
 
 	val &= ~(WUCSR_MPEN | WUCSR_BCST_EN | WUCSR_PFDA_EN);
 
 	ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-	check_warn_goto_done(ret, "Error writing WUCSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUCSR\n");
+		goto done;
+	}
 
 	if (pdata->wolopts & WAKE_PHY) {
 		netdev_info(dev->net, "enabling PHY wakeup\n");
 
 		ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-		check_warn_goto_done(ret, "Error reading PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PMT_CTL\n");
+			goto done;
+		}
 
 		/* clear wol status, enable energy detection */
 		val &= ~PMT_CTL_WUPS;
 		val |= (PMT_CTL_WUPS_ED | PMT_CTL_ED_EN);
 
 		ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-		check_warn_goto_done(ret, "Error writing PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PMT_CTL\n");
+			goto done;
+		}
 	}
 
 	if (pdata->wolopts & WAKE_MAGIC) {
 		netdev_info(dev->net, "enabling magic packet wakeup\n");
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		/* clear any pending magic packet status */
 		val |= WUCSR_MPR | WUCSR_MPEN;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	if (pdata->wolopts & WAKE_BCAST) {
 		netdev_info(dev->net, "enabling broadcast detection\n");
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_BCAST_FR | WUCSR_BCST_EN;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	if (pdata->wolopts & WAKE_UCAST) {
 		netdev_info(dev->net, "enabling unicast detection\n");
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_WUFR | WUCSR_PFDA_EN;
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	/* enable receiver to enable frame reception */
 	ret = smsc75xx_read_reg_nopm(dev, MAC_RX, &val);
-	check_warn_goto_done(ret, "Failed to read MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_RX: %d\n", ret);
+		goto done;
+	}
 
 	val |= MAC_RX_RXEN;
 
 	ret = smsc75xx_write_reg_nopm(dev, MAC_RX, val);
-	check_warn_goto_done(ret, "Failed to write MAC_RX: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_RX: %d\n", ret);
+		goto done;
+	}
 
 	/* some wol options are enabled, so enter SUSPEND0 */
 	netdev_info(dev->net, "entering SUSPEND0 mode\n");
@@ -1632,39 +2028,60 @@ static int smsc75xx_resume(struct usb_interface *intf)
 	if (suspend_flags & SUSPEND_ALLMODES) {
 		/* Disable wakeup sources */
 		ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_return(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			return ret;
+		}
 
 		val &= ~(WUCSR_WUEN | WUCSR_MPEN | WUCSR_PFDA_EN
 			| WUCSR_BCST_EN);
 
 		ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_return(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			return ret;
+		}
 
 		/* clear wake-up status */
 		ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-		check_warn_return(ret, "Error reading PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PMT_CTL\n");
+			return ret;
+		}
 
 		val &= ~PMT_CTL_WOL_EN;
 		val |= PMT_CTL_WUPS;
 
 		ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-		check_warn_return(ret, "Error writing PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PMT_CTL\n");
+			return ret;
+		}
 	}
 
 	if (suspend_flags & SUSPEND_SUSPEND2) {
 		netdev_info(dev->net, "resuming from SUSPEND2\n");
 
 		ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
-		check_warn_return(ret, "Error reading PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PMT_CTL\n");
+			return ret;
+		}
 
 		val |= PMT_CTL_PHY_PWRUP;
 
 		ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
-		check_warn_return(ret, "Error writing PMT_CTL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PMT_CTL\n");
+			return ret;
+		}
 	}
 
 	ret = smsc75xx_wait_ready(dev, 1);
-	check_warn_return(ret, "device not ready in smsc75xx_resume\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "device not ready in smsc75xx_resume\n");
+		return ret;
+	}
 
 	return usbnet_resume(intf);
 }
-- 
1.7.10.4

^ permalink raw reply related

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Eric Dumazet @ 2012-11-30 14:57 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert, David Laight
In-Reply-To: <1354276891.11754.424.camel@localhost>

On Fri, 2012-11-30 at 13:01 +0100, Jesper Dangaard Brouer wrote:

> As the existing entries in the frag queues, are still being allowed
> packets through (even when the memory limit is exceeded).   In
> worst-case, as DaveM explained, this can be as much as 100KBytes per
> entry (for 64K fragments).

Some NIC uses a plain page to hold an ethernet frame.

Consider an UDP packet using 512 bytes frags, the resulting packet after
reassembly can use 128 pages. Thats 512 KB of memory on x86.

^ permalink raw reply

* Re: [PATCH net-next v1 2/2] bridge: export multicast database via netlink
From: Cong Wang @ 2012-11-30 15:00 UTC (permalink / raw)
  To: Thomas Graf
  Cc: netdev, bridge, Herbert Xu, Stephen Hemminger, David S. Miller,
	Jesper Dangaard Brouer
In-Reply-To: <20121130112618.GF30697@casper.infradead.org>

On Fri, 2012-11-30 at 11:26 +0000, Thomas Graf wrote:
> On 11/30/12 at 05:58pm, Cong Wang wrote:
> > +
> > +	nest = nla_nest_start(skb, MDBA_ROUTER);
> > +	if (nest == NULL)
> > +		return -EMSGSIZE;
> > +
> > +	hlist_for_each_entry_rcu(p, n, &br->router_list, rlist) {
> > +		if (p && nla_put_u16(skb, MDBA_BRPORT_NO, p->port_no))
> > +			goto fail;
> > +	}
> 
> port_no 0 is reserved, right?
> 
> We can reduce message size here and make it easier to extend by
> using p->port_no as the attribute id by doing something like this:
> 
> hlist_for_each_entry_rcu(p, n, &br->router_list, rlist)
> 	if (nla_put_flag(skb, p->port_no))
> 		goto fail;
> 
> This will result in an empty attribute body for now and if you ever
> need to include more data you can simply start putting attributes
> insde that empty body and old users will continue to function.


I don't understand this. nla_put_flag() is used to put a flag (only one
bit set) into a netlink message, so why should we use it to put
p->port_no here? And why port_no 0 matters here?

[...]

> > +
> > +	cb->seq = mdb->seq;
> 
> I'm not sure how this is supposed to worl. cb->seq may not change
> throughout the complete dump process or the dump will be interrupted
> and the user is required to restart.
> 
> Each bridge will have its own mdb resulting in a differing seq.
> 

So I should use net->dev_base_seq + mdb->seq ?


All of the rest suggestions are taken by me.

Thanks for your detailed review!

^ permalink raw reply

* Re: linux-next: Tree for Nov 29 (netlabel)
From: Paul Moore @ 2012-11-30 15:19 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, linux-next, linux-kernel,
	netdev@vger.kernel.org
In-Reply-To: <50B7F846.70202@infradead.org>

On Thursday, November 29, 2012 04:05:26 PM Randy Dunlap wrote:
> On 11/28/2012 10:40 PM, Stephen Rothwell wrote:
> > Hi all,
> 
> > Changes since 20121128:
> (on i386:)

If I had to guess it looks like CONFIG_NETLABEL needs to be dependent on 
CONFIG_INET.  While the net/ Kconfig only pulls in the net/netlabel Kconfig if 
CONFIG_INET is defined, I'm guessing that without the explicit dependency 
there is nothing preventing someone from arriving at a bad configuration as we 
see here.

Let me test this out to make sure my reasoning is right and if it is I'll post 
a patch to netdev later today.

Thanks for catching this.

> net/built-in.o: In function `netlbl_cfg_cipsov4_add':
> (.text+0x61757): undefined reference to `cipso_v4_doi_add'
> net/built-in.o: In function `netlbl_cfg_cipsov4_del':
> (.text+0x6177d): undefined reference to `cipso_v4_doi_remove'
> net/built-in.o: In function `netlbl_cfg_cipsov4_map_add':
> (.text+0x617ae): undefined reference to `cipso_v4_doi_getdef'
> net/built-in.o: In function `netlbl_cfg_cipsov4_map_add':
> (.text+0x61a49): undefined reference to `cipso_v4_doi_putdef'
> net/built-in.o: In function `netlbl_sock_setattr':
> (.text+0x6218c): undefined reference to `cipso_v4_sock_setattr'
> net/built-in.o: In function `netlbl_sock_delattr':
> (.text+0x6220b): undefined reference to `cipso_v4_sock_delattr'
> net/built-in.o: In function `netlbl_sock_getattr':
> (.text+0x62238): undefined reference to `cipso_v4_sock_getattr'
> net/built-in.o: In function `netlbl_conn_setattr':
> (.text+0x622de): undefined reference to `cipso_v4_sock_setattr'
> net/built-in.o: In function `netlbl_conn_setattr':
> (.text+0x62303): undefined reference to `cipso_v4_sock_delattr'
> net/built-in.o: In function `netlbl_req_setattr':
> (.text+0x62429): undefined reference to `cipso_v4_req_setattr'
> net/built-in.o: In function `netlbl_req_setattr':
> (.text+0x6244e): undefined reference to `cipso_v4_req_delattr'
> net/built-in.o: In function `netlbl_req_delattr':
> (.text+0x624ba): undefined reference to `cipso_v4_req_delattr'
> net/built-in.o: In function `netlbl_skbuff_setattr':
> (.text+0x62551): undefined reference to `cipso_v4_skbuff_setattr'
> net/built-in.o: In function `netlbl_skbuff_setattr':
> (.text+0x62576): undefined reference to `cipso_v4_skbuff_delattr'
> net/built-in.o: In function `netlbl_skbuff_getattr':
> (.text+0x62619): undefined reference to `cipso_v4_skbuff_getattr'
> net/built-in.o: In function `netlbl_skbuff_err':
> (.text+0x62685): undefined reference to `cipso_v4_error'
> net/built-in.o: In function `netlbl_cache_invalidate':
> (.text+0x626ab): undefined reference to `cipso_v4_cache_invalidate'
> net/built-in.o: In function `netlbl_cache_add':
> (.text+0x626ec): undefined reference to `cipso_v4_cache_add'
> net/built-in.o: In function `netlbl_domhsh_remove_entry':
> (.text+0x63294): undefined reference to `cipso_v4_doi_putdef'
> net/built-in.o: In function `netlbl_domhsh_remove_entry':
> (.text+0x632eb): undefined reference to `cipso_v4_doi_putdef'
> net/built-in.o: In function `netlbl_domhsh_remove_af4':
> (.text+0x6349b): undefined reference to `cipso_v4_doi_putdef'
> net/built-in.o: In function `netlbl_mgmt_add_common.clone.1':
> netlabel_mgmt.c:(.text+0x64a87): undefined reference to
> `cipso_v4_doi_getdef' netlabel_mgmt.c:(.text+0x64c83): undefined reference
> to `cipso_v4_doi_putdef' net/built-in.o: In function
> `netlbl_cipsov4_listall':
> netlabel_cipso_v4.c:(.text+0x66e52): undefined reference to
> `cipso_v4_doi_walk' net/built-in.o: In function `netlbl_cipsov4_list':
> netlabel_cipso_v4.c:(.text+0x67199): undefined reference to
> `cipso_v4_doi_getdef' net/built-in.o: In function `netlbl_cipsov4_remove':
> netlabel_cipso_v4.c:(.text+0x6771b): undefined reference to
> `cipso_v4_doi_remove' net/built-in.o: In function
> `netlbl_cipsov4_add_pass':
> netlabel_cipso_v4.c:(.text+0x67a4b): undefined reference to
> `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x67a76): undefined reference
> to `cipso_v4_doi_free' net/built-in.o: In function
> `netlbl_cipsov4_add_local':
> netlabel_cipso_v4.c:(.text+0x67b9a): undefined reference to
> `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x67bc5): undefined reference
> to `cipso_v4_doi_free' net/built-in.o: In function
> `netlbl_cipsov4_add_std':
> netlabel_cipso_v4.c:(.text+0x68535): undefined reference to
> `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x68575): undefined reference
> to `cipso_v4_doi_free'
> 
> 
> Full randconfig file is attached.
-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH] sctp: fix CONFIG_SCTP_DBG_MSG=y null pointer dereference in sctp_v6_get_dst()
From: Neil Horman @ 2012-11-30 15:20 UTC (permalink / raw)
  To: Tommi Rantala
  Cc: linux-sctp, netdev, Vlad Yasevich, Sridhar Samudrala,
	David S. Miller, Dave Jones
In-Reply-To: <1354267062-15888-1-git-send-email-tt.rantala@gmail.com>

On Fri, Nov 30, 2012 at 11:17:42AM +0200, Tommi Rantala wrote:
> Trinity (the syscall fuzzer) triggered the following BUG, reproducible
> only when the kernel is configured with CONFIG_SCTP_DBG_MSG=y.
> 
> When CONFIG_SCTP_DBG_MSG is not set, the null pointer is never
> dereferenced.
> 
> ---[ end trace a4de0bfcb38a3642 ]---
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000100
> IP: [<ffffffff8136796e>] ip6_string+0x1e/0xa0
> PGD 4eead067 PUD 4e472067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP
> Modules linked in:
> CPU 3
> Pid: 21324, comm: trinity-child11 Tainted: G        W    3.7.0-rc7+ #61 ASUSTeK Computer INC. EB1012/EB1012
> RIP: 0010:[<ffffffff8136796e>]  [<ffffffff8136796e>] ip6_string+0x1e/0xa0
> RSP: 0018:ffff88004e4637a0  EFLAGS: 00010046
> RAX: ffff88004e4637da RBX: ffff88004e4637da RCX: 0000000000000000
> RDX: ffffffff8246e92a RSI: 0000000000000100 RDI: ffff88004e4637da
> RBP: ffff88004e4637a8 R08: 000000000000ffff R09: 000000000000ffff
> R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff8289d600
> R13: ffffffff8289d230 R14: ffffffff8246e928 R15: ffffffff8289d600
> FS:  00007fed95153700(0000) GS:ffff88005fd80000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000100 CR3: 000000004eeac000 CR4: 00000000000007e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process trinity-child11 (pid: 21324, threadinfo ffff88004e462000, task ffff8800524b0000)
> Stack:
>  ffff88004e4637da ffff88004e463828 ffffffff81368eee 000000004e4637d8
>  ffffffff0000ffff ffff88000000ffff 0000000000000000 000000004e4637f8
>  ffffffff826285d8 ffff88004e4637f8 0000000000000000 ffff8800524b06b0
> Call Trace:
>  [<ffffffff81368eee>] ip6_addr_string.isra.11+0x3e/0xa0
>  [<ffffffff81369183>] pointer.isra.12+0x233/0x2d0
>  [<ffffffff810a413a>] ? vprintk_emit+0x1ba/0x450
>  [<ffffffff8110953d>] ? trace_hardirqs_on_caller+0x10d/0x1a0
>  [<ffffffff81369757>] vsnprintf+0x187/0x5d0
>  [<ffffffff81369c62>] vscnprintf+0x12/0x30
>  [<ffffffff810a4028>] vprintk_emit+0xa8/0x450
>  [<ffffffff81e5cb00>] printk+0x49/0x4b
>  [<ffffffff81d17221>] sctp_v6_get_dst+0x731/0x780
>  [<ffffffff81d16e15>] ? sctp_v6_get_dst+0x325/0x780
>  [<ffffffff81d00a96>] sctp_transport_route+0x46/0x120
>  [<ffffffff81cff0f1>] sctp_assoc_add_peer+0x161/0x350
>  [<ffffffff81d0fd8d>] sctp_sendmsg+0x6cd/0xcb0
>  [<ffffffff81b55bf0>] ? inet_create+0x670/0x670
>  [<ffffffff81b55cfb>] inet_sendmsg+0x10b/0x220
>  [<ffffffff81b55bf0>] ? inet_create+0x670/0x670
>  [<ffffffff81a72a64>] ? sock_update_classid+0xa4/0x2b0
>  [<ffffffff81a72ab0>] ? sock_update_classid+0xf0/0x2b0
>  [<ffffffff81a6ac1c>] sock_sendmsg+0xdc/0xf0
>  [<ffffffff8118e9e5>] ? might_fault+0x85/0x90
>  [<ffffffff8118e99c>] ? might_fault+0x3c/0x90
>  [<ffffffff81a6e12a>] sys_sendto+0xfa/0x130
>  [<ffffffff810a9887>] ? do_setitimer+0x197/0x380
>  [<ffffffff81e960d5>] ? sysret_check+0x22/0x5d
>  [<ffffffff81e960a9>] system_call_fastpath+0x16/0x1b
> Code: 01 eb 89 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 f8 31 c9 48 89 e5 53 eb 12 0f 1f 40 00 48 83 c1 01 48 83 c0 04 48 83 f9 08 74 70 <0f> b6 3c 4e 89 fb 83 e7 0f c0 eb 04 41 89 d8 41 83 e0 0f 0f b6
> RIP  [<ffffffff8136796e>] ip6_string+0x1e/0xa0
>  RSP <ffff88004e4637a0>
> CR2: 0000000000000100
> ---[ end trace a4de0bfcb38a3643 ]---
> 
> Signed-off-by: Tommi Rantala <tt.rantala@gmail.com>
> ---
>  net/sctp/ipv6.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index ea14cb4..f3f0f4d 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -345,7 +345,7 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
>  	}
>  
>  out:
> -	if (!IS_ERR(dst)) {
> +	if (!IS_ERR_OR_NULL(dst)) {
>  		struct rt6_info *rt;
>  		rt = (struct rt6_info *)dst;
>  		t->dst = dst;
> -- 
> 1.7.9.5
> 
> 
Acked-by; Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH] smsc: RFC: Workaround for problems with lan8710 phy auto MDI-X
From: Jiri Kosina @ 2012-11-30 15:23 UTC (permalink / raw)
  To: Peter Turczak
  Cc: David Miller, Otavio Salvador, Javier Martinez Canillas,
	Christian Hohnstaedt, netdev, linux-kernel
In-Reply-To: <8D7E9026-6276-452B-9E0C-AEB8CF38C9FD@netconsequence.de>

On Fri, 30 Nov 2012, Peter Turczak wrote:

> while debugging network outages on a customers hardware I found, that 
> the MDI-X function of the lan8710 phy seemed to cause trouble. When 
> connecting to almost any kind of 100/1000MBit switch, the link would 
> seem to come up and data where sent out to the network. But all incoming 
> packets got lost somehow. This is quite bad, as the system runs from 
> nfsroot while booting up during development.
> 
> When I disabled the auto MDI-X function of the phy the problem went away.
> 
> Signed-off-by: Peter Turczak <pt@netconsequence.de>
> ---
>  drivers/net/phy/Kconfig |   10 ++++++++++
>  drivers/net/phy/smsc.c  |   15 +++++++++++++++
>  include/linux/smscphy.h |    5 +++++
>  3 files changed, 30 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 961f0b2..341f5aa 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -60,6 +60,16 @@ config SMSC_PHY
>  	---help---
>  	  Currently supports the LAN83C185, LAN8187 and LAN8700 PHYs
>  
> +config SMSC_PHY_DISABLE_AUTOX
> +	bool "Disable MDI-X upon start"
> +	depends on SMSC_PHY
> +	---help---
> +	  When you experience problems estabishing a stable connection
> +	  to a network and you have e.g. a LAN8710 ethernet phy
> +	  this option might help you out.
> +
> +	  In doubt, say N
> +

I am not sure whether compile-time option for something like this is 
appropriate. Kernel module parameter, perhaps?

Of course it'd be far better if faulty hardware can be autodetected in 
runtime.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH net-next v1 2/2] bridge: export multicast database via netlink
From: Thomas Graf @ 2012-11-30 15:27 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, bridge, Herbert Xu, Stephen Hemminger, David S. Miller,
	Jesper Dangaard Brouer
In-Reply-To: <1354287652.19865.10.camel@cr0>

On 11/30/12 at 11:00pm, Cong Wang wrote:
> I don't understand this. nla_put_flag() is used to put a flag (only one
> bit set) into a netlink message, so why should we use it to put
> p->port_no here? And why port_no 0 matters here?

nla_put_flag() will simply add a netlink attribute with no payload,
i.e. just the header. Assuming that port_no == 0 is invalid the
port_no can be used as attribute id as both are 16bit integers.

It will look like this:

MDBA_ROUTERS = {
  {
    .nla_len = 4,
    .nla_type = <port_no_1>,
  },
  {
    .nla_len = 4,
    .nla_type = <port_no_2>,
  }
  [...]
}

If you ever need to extend this you can just add payload to the
per port attribute and nothing will break.

> So I should use net->dev_base_seq + mdb->seq ?

No you can't, mdb->seq is not stable throughout a dump. What you
can do is save mdb->seq in cb->args[] and in case you continue
dumping from the same mdb in the next call to your dump function
you check if it changed and bump cb->seq if it did to trigger an
interrupt.

^ permalink raw reply

* Re: linux-next: Tree for Nov 29 (netlabel)
From: Paul Moore @ 2012-11-30 15:31 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, linux-next, linux-kernel,
	netdev@vger.kernel.org
In-Reply-To: <3694027.VYznNcdp7C@sifl>

On Friday, November 30, 2012 10:19:16 AM Paul Moore wrote:
> On Thursday, November 29, 2012 04:05:26 PM Randy Dunlap wrote:
> > On 11/28/2012 10:40 PM, Stephen Rothwell wrote:
> > > Hi all,
> > 
> > > Changes since 20121128:
> > (on i386:)
> 
> If I had to guess it looks like CONFIG_NETLABEL needs to be dependent on
> CONFIG_INET.  While the net/ Kconfig only pulls in the net/netlabel Kconfig
> if CONFIG_INET is defined, I'm guessing that without the explicit
> dependency there is nothing preventing someone from arriving at a bad
> configuration as we see here.
> 
> Let me test this out to make sure my reasoning is right and if it is I'll
> post a patch to netdev later today.
> 
> Thanks for catching this.

Hmmm.  The existing logic in net/Kconfig seems to disable CONFIG_NETLABEL at 
build time whenever CONFIG_INET is disabled in my .config file.  The only way 
I can recreate what you are seeing here is if I move the NetLabel include 
outside of the INET conditional in net/Kconfig.

Regardless, adding an explicit dependency on INET to NETLABEL shouldn't hurt 
anything so I'll go ahead and post the patch to netdev.  Hopefully someone who 
understands Kconfig better than I do can help shed some light on this.

> > net/built-in.o: In function `netlbl_cfg_cipsov4_add':
> > (.text+0x61757): undefined reference to `cipso_v4_doi_add'
> > net/built-in.o: In function `netlbl_cfg_cipsov4_del':
> > (.text+0x6177d): undefined reference to `cipso_v4_doi_remove'
> > net/built-in.o: In function `netlbl_cfg_cipsov4_map_add':
> > (.text+0x617ae): undefined reference to `cipso_v4_doi_getdef'
> > net/built-in.o: In function `netlbl_cfg_cipsov4_map_add':
> > (.text+0x61a49): undefined reference to `cipso_v4_doi_putdef'
> > net/built-in.o: In function `netlbl_sock_setattr':
> > (.text+0x6218c): undefined reference to `cipso_v4_sock_setattr'
> > net/built-in.o: In function `netlbl_sock_delattr':
> > (.text+0x6220b): undefined reference to `cipso_v4_sock_delattr'
> > net/built-in.o: In function `netlbl_sock_getattr':
> > (.text+0x62238): undefined reference to `cipso_v4_sock_getattr'
> > net/built-in.o: In function `netlbl_conn_setattr':
> > (.text+0x622de): undefined reference to `cipso_v4_sock_setattr'
> > net/built-in.o: In function `netlbl_conn_setattr':
> > (.text+0x62303): undefined reference to `cipso_v4_sock_delattr'
> > net/built-in.o: In function `netlbl_req_setattr':
> > (.text+0x62429): undefined reference to `cipso_v4_req_setattr'
> > net/built-in.o: In function `netlbl_req_setattr':
> > (.text+0x6244e): undefined reference to `cipso_v4_req_delattr'
> > net/built-in.o: In function `netlbl_req_delattr':
> > (.text+0x624ba): undefined reference to `cipso_v4_req_delattr'
> > net/built-in.o: In function `netlbl_skbuff_setattr':
> > (.text+0x62551): undefined reference to `cipso_v4_skbuff_setattr'
> > net/built-in.o: In function `netlbl_skbuff_setattr':
> > (.text+0x62576): undefined reference to `cipso_v4_skbuff_delattr'
> > net/built-in.o: In function `netlbl_skbuff_getattr':
> > (.text+0x62619): undefined reference to `cipso_v4_skbuff_getattr'
> > net/built-in.o: In function `netlbl_skbuff_err':
> > (.text+0x62685): undefined reference to `cipso_v4_error'
> > net/built-in.o: In function `netlbl_cache_invalidate':
> > (.text+0x626ab): undefined reference to `cipso_v4_cache_invalidate'
> > net/built-in.o: In function `netlbl_cache_add':
> > (.text+0x626ec): undefined reference to `cipso_v4_cache_add'
> > net/built-in.o: In function `netlbl_domhsh_remove_entry':
> > (.text+0x63294): undefined reference to `cipso_v4_doi_putdef'
> > net/built-in.o: In function `netlbl_domhsh_remove_entry':
> > (.text+0x632eb): undefined reference to `cipso_v4_doi_putdef'
> > net/built-in.o: In function `netlbl_domhsh_remove_af4':
> > (.text+0x6349b): undefined reference to `cipso_v4_doi_putdef'
> > net/built-in.o: In function `netlbl_mgmt_add_common.clone.1':
> > netlabel_mgmt.c:(.text+0x64a87): undefined reference to
> > `cipso_v4_doi_getdef' netlabel_mgmt.c:(.text+0x64c83): undefined reference
> > to `cipso_v4_doi_putdef' net/built-in.o: In function
> > `netlbl_cipsov4_listall':
> > netlabel_cipso_v4.c:(.text+0x66e52): undefined reference to
> > `cipso_v4_doi_walk' net/built-in.o: In function `netlbl_cipsov4_list':
> > netlabel_cipso_v4.c:(.text+0x67199): undefined reference to
> > `cipso_v4_doi_getdef' net/built-in.o: In function `netlbl_cipsov4_remove':
> > netlabel_cipso_v4.c:(.text+0x6771b): undefined reference to
> > `cipso_v4_doi_remove' net/built-in.o: In function
> > `netlbl_cipsov4_add_pass':
> > netlabel_cipso_v4.c:(.text+0x67a4b): undefined reference to
> > `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x67a76): undefined
> > reference
> > to `cipso_v4_doi_free' net/built-in.o: In function
> > `netlbl_cipsov4_add_local':
> > netlabel_cipso_v4.c:(.text+0x67b9a): undefined reference to
> > `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x67bc5): undefined
> > reference
> > to `cipso_v4_doi_free' net/built-in.o: In function
> > `netlbl_cipsov4_add_std':
> > netlabel_cipso_v4.c:(.text+0x68535): undefined reference to
> > `cipso_v4_doi_add' netlabel_cipso_v4.c:(.text+0x68575): undefined
> > reference
> > to `cipso_v4_doi_free'
> > 
> > 
> > Full randconfig file is attached.
-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* [PATCH] net: make CONFIG_NETLABEL explicitly dependent on CONFIG_INET
From: Paul Moore @ 2012-11-30 15:35 UTC (permalink / raw)
  To: netdev; +Cc: rdunlap

This should resolve an issue found during testing with random
configuration files.

  net/built-in.o: In function `netlbl_cfg_cipsov4_add':
  (.text+0x61757): undefined reference to `cipso_v4_doi_add'
  net/built-in.o: In function `netlbl_cfg_cipsov4_del':
  (.text+0x6177d): undefined reference to `cipso_v4_doi_remove'
  [... and so on ...]


Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 net/netlabel/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netlabel/Kconfig b/net/netlabel/Kconfig
index 56958c8..f272826 100644
--- a/net/netlabel/Kconfig
+++ b/net/netlabel/Kconfig
@@ -4,7 +4,7 @@
 
 config NETLABEL
 	bool "NetLabel subsystem support"
-	depends on SECURITY
+	depends on SECURITY && INET
 	default n
 	---help---
 	  NetLabel provides support for explicit network packet labeling

^ permalink raw reply related

* Re: [net-next PATCH V2 1/9] net: frag evictor, avoid killing warm frag queues
From: Jesper Dangaard Brouer @ 2012-11-30 15:45 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, fw, netdev, pablo, tgraf, amwang, kaber, paulmck,
	herbert
In-Reply-To: <1354287134.3299.67.camel@edumazet-glaptop>

On Fri, 2012-11-30 at 06:52 -0800, Eric Dumazet wrote:
> On Fri, 2012-11-30 at 11:04 +0100, Jesper Dangaard Brouer wrote:
> > So, let me instead show, with tests, that the evictor strategy is
> > broken, while keeping the original default thresh settings:
> > 
> > # grep . /proc/sys/net/ipv4/ipfrag_*_thresh
> > /proc/sys/net/ipv4/ipfrag_high_thresh:262144
> > /proc/sys/net/ipv4/ipfrag_low_thresh:196608
> > 
> > Test purpose, I will on a single 10G link demonstrate, that starting
> > several "N" netperf UDP fragmentation flows, will hurt performance, and
> > then claim this is caused by the bad evictor strategy.
> > 
> > Test setup:
> >  - Disable Ethernet flow control
> >  - netperf packet size 65507
> >  - Run netserver on one NUMA node
> >  - Start netperf clients against a NIC on the other NUMA node
> >  - (The NUMA imbalance helps the effect occur at lower N) 
> > 
> > Result: N=1  8040 Mbit/s
> > Result: N=2  9584 Mbit/s (4739+4845)
> > Result: N=3  4055 Mbit/s (1436+1371+1248)
> > Result: N=4  2247 Mbit/s (1538+29+54+626)
> > Result: N=5   879 Mbit/s (78+152+226+125+298)
> > Result: N=6   293 Mbit/s (85+55+32+57+46+18)
> > Result: N=7   354 Mbit/s (70+47+33+80+20+72+32)
> > 
> > Can we, now, agree that the current evictor strategy is broken?!?
> 
> Your setup is broken for sure. 

No, its not.

> I dont know how you expect that many
> datagrams being correctly reassembled with ipfrag_high_thresh=262144 

That's my point... I'm showing that its not possible, with out current
implementation!


> No matter strategy is implemented, an attacker knows it and can send
> frags so that regular workload is denied. Kernel cant decide which
> packets are more likely to be completed.

Our current evictor implementation will allow the attacker to kill ALL
frag traffic to the machine (and cause high CPU load, with CPUs
spinning).
My implementation will guarantee that some fragments will be, allowed to
finish and complete.  A far better choice, than our current situation.


> BTW, install fq_codel at the sender side, so that frags are nicely
> interleaved. Because on real networks, frags of an UDP datagram rarely
> come to destination in a single train with no alien packets inside the
> train.

You are arguing in my favor.  I have taken great care that my test will
interleave UDP datagrams (by CPU pinning netperf clients on the sender
side).  If I just naively start all netperf client on one CPU on the
sender, then I will have packet trains... and the result will be:
 Packet trains result N=5  8884 Mbit/s (1775+1775+1790+1789+1755)
That test would be "broken for sure".

^ permalink raw reply

* Re: [PATCH net-next v1 1/2] bridge: export port_no and port_id via IFA_INFO_DATA
From: Stephen Hemminger @ 2012-11-30 15:52 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, bridge, Herbert Xu, David S. Miller, Thomas Graf,
	Jesper Dangaard Brouer
In-Reply-To: <1354269514-1863-1-git-send-email-amwang@redhat.com>

On Fri, 30 Nov 2012 17:58:32 +0800
Cong Wang <amwang@redhat.com> wrote:

> port_no will be used to get ifindex of a port in user-space,
> export it togather with port_id.
> 
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Thomas Graf <tgraf@suug.ch>
> Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> Acked-by: Thomas Graf <tgraf@suug.ch>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  include/uapi/linux/if_link.h |    2 ++
>  net/bridge/br_netlink.c      |    8 +++++++-
>  2 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index bb58aeb..9cd91a9 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -218,6 +218,8 @@ enum {
>  	IFLA_BRPORT_MODE,	/* mode (hairpin)          */
>  	IFLA_BRPORT_GUARD,	/* bpdu guard              */
>  	IFLA_BRPORT_PROTECT,	/* root port protection    */
> +	IFLA_BRPORT_NO,		/* port no                 */
> +	IFLA_BRPORT_ID,		/* port id                 */
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 65429b9..7b7414e 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -28,6 +28,8 @@ static inline size_t br_port_info_size(void)
>  		+ nla_total_size(1)	/* IFLA_BRPORT_MODE */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
> +		+ nla_total_size(2)	/* IFLA_BRPORT_NO */
> +		+ nla_total_size(2)	/* IFLA_BRPORT_ID */
>  		+ 0;
>  }
>  
> @@ -53,7 +55,9 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>  	    nla_put_u32(skb, IFLA_BRPORT_COST, p->path_cost) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_MODE, mode) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_GUARD, !!(p->flags & BR_BPDU_GUARD)) ||
> -	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)))
> +	    nla_put_u8(skb, IFLA_BRPORT_PROTECT, !!(p->flags & BR_ROOT_BLOCK)) ||
> +	    nla_put_u16(skb, IFLA_BRPORT_NO, p->port_no) ||
> +	    nla_put_u16(skb, IFLA_BRPORT_ID, p->port_id))
>  		return -EMSGSIZE;
>  
>  	return 0;
> @@ -168,6 +172,8 @@ static const struct nla_policy ifla_brport_policy[IFLA_BRPORT_MAX + 1] = {
>  	[IFLA_BRPORT_MODE]	= { .type = NLA_U8 },
>  	[IFLA_BRPORT_GUARD]	= { .type = NLA_U8 },
>  	[IFLA_BRPORT_PROTECT]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_NO]	= { .type = NLA_U16 },
> +	[IFLA_BRPORT_ID]	= { .type = NLA_U16 },
>  };
>  
>  /* Change the state of the port and notify spanning tree */

I don't think these are necessary. The device is already available and the relationship
can be determined from other messages. This is what RSTP daemon does.

^ permalink raw reply

* [PATCH 0/5] smsc95xx enhancements
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning

This patchset is a resubmission of several patches plus two new
patches, including the expansion of cpp macros at the request of
davem.

Steve Glendinning (5):
  smsc95xx: fix suspend buffer overflow
  smsc95xx: fix error handling in suspend failure case
  smsc95xx: don't enable remote wakeup directly
  smsc95xx: fix smsc_crc return type
  smsc95xx: expand check_ macros

 drivers/net/usb/smsc95xx.c |  566 ++++++++++++++++++++++++++++++++------------
 1 file changed, 415 insertions(+), 151 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH 1/5] smsc95xx: fix suspend buffer overflow
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning, Bjorn Mork, Joe Perches
In-Reply-To: <1354290952-27109-1-git-send-email-steve.glendinning@shawell.net>

This patch fixes a buffer overflow introduced by bbd9f9e, where
the filter_mask array is accessed beyond its bounds.

Updated to also add a check for kzalloc failure, as reported by
Bjorn Mork and Joe Perches.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Cc: Bjorn Mork <bjorn@mork.no>
Cc: Joe Perches <joe@perches.com>
---
 drivers/net/usb/smsc95xx.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 79d495d..c397b3a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1281,7 +1281,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	}
 
 	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
-		u32 *filter_mask = kzalloc(32, GFP_KERNEL);
+		u32 *filter_mask = kzalloc(sizeof(u32) * 32, GFP_KERNEL);
 		u32 command[2];
 		u32 offset[2];
 		u32 crc[4];
@@ -1290,6 +1290,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 			LAN9500A_WUFF_NUM : LAN9500_WUFF_NUM;
 		int i, filter = 0;
 
+		if (!filter_mask) {
+			netdev_warn(dev->net, "Unable to allocate filter_mask\n");
+			return -ENOMEM;
+		}
+
 		memset(command, 0, sizeof(command));
 		memset(offset, 0, sizeof(offset));
 		memset(crc, 0, sizeof(crc));
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/5] smsc95xx: fix error handling in suspend failure case
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <1354290952-27109-1-git-send-email-steve.glendinning@shawell.net>

This patch ensures that if we fail to suspend the LAN9500 device
we call usbnet_resume before returning failure, instead of
leaving the usbnet driver in an unusable state.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |   50 +++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index c397b3a..ffeaf13 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1248,35 +1248,37 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		/* disable energy detect (link up) & wake up events */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_return(ret, "Error reading WUCSR\n");
+		check_warn_goto_done(ret, "Error reading WUCSR\n");
 
 		val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_return(ret, "Error writing WUCSR\n");
+		check_warn_goto_done(ret, "Error writing WUCSR\n");
 
 		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		check_warn_return(ret, "Error reading PM_CTRL\n");
+		check_warn_goto_done(ret, "Error reading PM_CTRL\n");
 
 		val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		check_warn_return(ret, "Error writing PM_CTRL\n");
+		check_warn_goto_done(ret, "Error writing PM_CTRL\n");
 
-		return smsc95xx_enter_suspend2(dev);
+		ret = smsc95xx_enter_suspend2(dev);
+		goto done;
 	}
 
 	if (pdata->wolopts & WAKE_PHY) {
 		ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
 			(PHY_INT_MASK_ANEG_COMP_ | PHY_INT_MASK_LINK_DOWN_));
-		check_warn_return(ret, "error enabling PHY wakeup ints\n");
+		check_warn_goto_done(ret, "error enabling PHY wakeup ints\n");
 
 		/* if link is down then configure EDPD and enter SUSPEND1,
 		 * otherwise enter SUSPEND0 below
 		 */
 		if (!link_up) {
 			netdev_info(dev->net, "entering SUSPEND1 mode\n");
-			return smsc95xx_enter_suspend1(dev);
+			ret = smsc95xx_enter_suspend1(dev);
+			goto done;
 		}
 	}
 
@@ -1292,7 +1294,8 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		if (!filter_mask) {
 			netdev_warn(dev->net, "Unable to allocate filter_mask\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto done;
 		}
 
 		memset(command, 0, sizeof(command));
@@ -1354,49 +1357,49 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
 			if (ret < 0)
 				kfree(filter_mask);
-			check_warn_return(ret, "Error writing WUFF\n");
+			check_warn_goto_done(ret, "Error writing WUFF\n");
 		}
 		kfree(filter_mask);
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
-			check_warn_return(ret, "Error writing WUFF\n");
+			check_warn_goto_done(ret, "Error writing WUFF\n");
 		}
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
-			check_warn_return(ret, "Error writing WUFF\n");
+			check_warn_goto_done(ret, "Error writing WUFF\n");
 		}
 
 		for (i = 0; i < (wuff_filter_count / 2); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
-			check_warn_return(ret, "Error writing WUFF\n");
+			check_warn_goto_done(ret, "Error writing WUFF\n");
 		}
 
 		/* clear any pending pattern match packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_return(ret, "Error reading WUCSR\n");
+		check_warn_goto_done(ret, "Error reading WUCSR\n");
 
 		val |= WUCSR_WUFR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_return(ret, "Error writing WUCSR\n");
+		check_warn_goto_done(ret, "Error writing WUCSR\n");
 	}
 
 	if (pdata->wolopts & WAKE_MAGIC) {
 		/* clear any pending magic packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_return(ret, "Error reading WUCSR\n");
+		check_warn_goto_done(ret, "Error reading WUCSR\n");
 
 		val |= WUCSR_MPR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_return(ret, "Error writing WUCSR\n");
+		check_warn_goto_done(ret, "Error writing WUCSR\n");
 	}
 
 	/* enable/disable wakeup sources */
 	ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-	check_warn_return(ret, "Error reading WUCSR\n");
+	check_warn_goto_done(ret, "Error reading WUCSR\n");
 
 	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
 		netdev_info(dev->net, "enabling pattern match wakeup\n");
@@ -1415,11 +1418,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	}
 
 	ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-	check_warn_return(ret, "Error writing WUCSR\n");
+	check_warn_goto_done(ret, "Error writing WUCSR\n");
 
 	/* enable wol wakeup source */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL\n");
+	check_warn_goto_done(ret, "Error reading PM_CTRL\n");
 
 	val |= PM_CTL_WOL_EN_;
 
@@ -1428,14 +1431,19 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		val |= PM_CTL_ED_EN_;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	check_warn_goto_done(ret, "Error writing PM_CTRL\n");
 
 	/* enable receiver to enable frame reception */
 	smsc95xx_start_rx_path(dev, 1);
 
 	/* some wol options are enabled, so enter SUSPEND0 */
 	netdev_info(dev->net, "entering SUSPEND0 mode\n");
-	return smsc95xx_enter_suspend0(dev);
+	ret = smsc95xx_enter_suspend0(dev);
+
+done:
+	if (ret)
+		usbnet_resume(intf);
+	return ret;
 }
 
 static int smsc95xx_resume(struct usb_interface *intf)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/5] smsc95xx: don't enable remote wakeup directly
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning, Bjorn Mork
In-Reply-To: <1354290952-27109-1-git-send-email-steve.glendinning@shawell.net>

As pointed out by Bjorn Mork, the generic "usb" driver sets this
for us so no need to directly set it in this driver.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Cc: Bjorn Mork <bjorn@mork.no>
---
 drivers/net/usb/smsc95xx.c |   30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index ffeaf13..064df1a 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -154,25 +154,6 @@ static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
 {
 	return __smsc95xx_write_reg(dev, index, data, 0);
 }
-static int smsc95xx_set_feature(struct usbnet *dev, u32 feature)
-{
-	if (WARN_ON_ONCE(!dev))
-		return -EINVAL;
-
-	return usbnet_write_cmd_nopm(dev, USB_REQ_SET_FEATURE,
-				     USB_RECIP_DEVICE, feature, 0,
-				     NULL, 0);
-}
-
-static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
-{
-	if (WARN_ON_ONCE(!dev))
-		return -EINVAL;
-
-	return usbnet_write_cmd_nopm(dev, USB_REQ_CLEAR_FEATURE,
-				     USB_RECIP_DEVICE, feature,
-				     0, NULL, 0);
-}
 
 /* Loop until the read is completed with timeout
  * called with phy_mutex held */
@@ -685,8 +666,13 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
 {
 	struct usbnet *dev = netdev_priv(net);
 	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	int ret;
 
 	pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
+
+	ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
+	check_warn_return(ret, "device_set_wakeup_enable error %d\n", ret);
+
 	return 0;
 }
 
@@ -1160,8 +1146,6 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
 	check_warn_return(ret, "Error reading PM_CTRL\n");
 
-	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
 	return 0;
 }
 
@@ -1204,8 +1188,6 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
 	check_warn_return(ret, "Error writing PM_CTRL\n");
 
-	smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
 	return 0;
 }
 
@@ -1456,8 +1438,6 @@ static int smsc95xx_resume(struct usb_interface *intf)
 	BUG_ON(!dev);
 
 	if (pdata->wolopts) {
-		smsc95xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
-
 		/* clear wake-up sources */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
 		check_warn_return(ret, "Error reading WUCSR\n");
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 4/5] smsc95xx: fix smsc_crc return type
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning, Joe Perches
In-Reply-To: <1354290952-27109-1-git-send-email-steve.glendinning@shawell.net>

This patch fixes a bug introduced in bbd9f9e which could prevent
some wakeups from working correctly if multiple wol options were
selected.

This helper function calculates a 16-bit crc and shifts it into
either the high or low 16 bits of a u32 so the caller can or it
directly into place.  The function previously had a u16 return
type so would always have returned zero when filter was odd.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Reported-by: Bjorn Mork <bjorn@mork.no>
Cc: Joe Perches <joe@perches.com>
---
 drivers/net/usb/smsc95xx.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 064df1a..b9eb490 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1074,9 +1074,10 @@ static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
 	}
 }
 
-static u16 smsc_crc(const u8 *buffer, size_t len, int filter)
+static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
 {
-	return bitrev16(crc16(0xFFFF, buffer, len)) << ((filter % 2) * 16);
+	u32 crc = bitrev16(crc16(0xFFFF, buffer, len));
+	return crc << ((filter % 2) * 16);
 }
 
 static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 5/5] smsc95xx: expand check_ macros
From: Steve Glendinning @ 2012-11-30 15:55 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <1354290952-27109-1-git-send-email-steve.glendinning@shawell.net>

These macros, while reducing the amount of code, hide flow control
and make the code more confusing to follow and review.  This patch
expands them.  It should have no functional effect on the driver.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
---
 drivers/net/usb/smsc95xx.c |  512 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 391 insertions(+), 121 deletions(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index b9eb490..f7e1e18 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -55,15 +55,6 @@
 #define FEATURE_PHY_NLP_CROSSOVER	(0x02)
 #define FEATURE_AUTOSUSPEND		(0x04)
 
-#define check_warn(ret, fmt, args...) \
-	({ if (ret < 0) netdev_warn(dev->net, fmt, ##args); })
-
-#define check_warn_return(ret, fmt, args...) \
-	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); return ret; } })
-
-#define check_warn_goto_done(ret, fmt, args...) \
-	({ if (ret < 0) { netdev_warn(dev->net, fmt, ##args); goto done; } })
-
 struct smsc95xx_priv {
 	u32 mac_cr;
 	u32 hash_hi;
@@ -166,7 +157,11 @@ static int __must_check __smsc95xx_phy_wait_not_busy(struct usbnet *dev,
 
 	do {
 		ret = __smsc95xx_read_reg(dev, MII_ADDR, &val, in_pm);
-		check_warn_return(ret, "Error reading MII_ACCESS\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading MII_ACCESS\n");
+			return ret;
+		}
+
 		if (!(val & MII_BUSY_))
 			return 0;
 	} while (!time_after(jiffies, start_time + HZ));
@@ -185,20 +180,32 @@ static int __smsc95xx_mdio_read(struct net_device *netdev, int phy_id, int idx,
 
 	/* confirm MII not busy */
 	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_read\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "MII is busy in smsc95xx_mdio_read\n");
+		goto done;
+	}
 
 	/* set the address, index & direction (read from PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_READ_ | MII_BUSY_;
 	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_ADDR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_ADDR\n");
+		goto done;
+	}
 
 	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "Timed out reading MII reg %02X\n", idx);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Timed out reading MII reg %02X\n", idx);
+		goto done;
+	}
 
 	ret = __smsc95xx_read_reg(dev, MII_DATA, &val, in_pm);
-	check_warn_goto_done(ret, "Error reading MII_DATA\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_DATA\n");
+		goto done;
+	}
 
 	ret = (u16)(val & 0xFFFF);
 
@@ -218,21 +225,33 @@ static void __smsc95xx_mdio_write(struct net_device *netdev, int phy_id,
 
 	/* confirm MII not busy */
 	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "MII is busy in smsc95xx_mdio_write\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "MII is busy in smsc95xx_mdio_write\n");
+		goto done;
+	}
 
 	val = regval;
 	ret = __smsc95xx_write_reg(dev, MII_DATA, val, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_DATA\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_DATA\n");
+		goto done;
+	}
 
 	/* set the address, index & direction (write to PHY) */
 	phy_id &= dev->mii.phy_id_mask;
 	idx &= dev->mii.reg_num_mask;
 	addr = (phy_id << 11) | (idx << 6) | MII_WRITE_ | MII_BUSY_;
 	ret = __smsc95xx_write_reg(dev, MII_ADDR, addr, in_pm);
-	check_warn_goto_done(ret, "Error writing MII_ADDR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MII_ADDR\n");
+		goto done;
+	}
 
 	ret = __smsc95xx_phy_wait_not_busy(dev, in_pm);
-	check_warn_goto_done(ret, "Timed out writing MII reg %02X\n", idx);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Timed out writing MII reg %02X\n", idx);
+		goto done;
+	}
 
 done:
 	mutex_unlock(&dev->phy_mutex);
@@ -269,7 +288,11 @@ static int __must_check smsc95xx_wait_eeprom(struct usbnet *dev)
 
 	do {
 		ret = smsc95xx_read_reg(dev, E2P_CMD, &val);
-		check_warn_return(ret, "Error reading E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_CMD\n");
+			return ret;
+		}
+
 		if (!(val & E2P_CMD_BUSY_) || (val & E2P_CMD_TIMEOUT_))
 			break;
 		udelay(40);
@@ -291,7 +314,10 @@ static int __must_check smsc95xx_eeprom_confirm_not_busy(struct usbnet *dev)
 
 	do {
 		ret = smsc95xx_read_reg(dev, E2P_CMD, &val);
-		check_warn_return(ret, "Error reading E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_CMD\n");
+			return ret;
+		}
 
 		if (!(val & E2P_CMD_BUSY_))
 			return 0;
@@ -319,14 +345,20 @@ static int smsc95xx_read_eeprom(struct usbnet *dev, u32 offset, u32 length,
 	for (i = 0; i < length; i++) {
 		val = E2P_CMD_BUSY_ | E2P_CMD_READ_ | (offset & E2P_CMD_ADDR_);
 		ret = smsc95xx_write_reg(dev, E2P_CMD, val);
-		check_warn_return(ret, "Error writing E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_CMD\n");
+			return ret;
+		}
 
 		ret = smsc95xx_wait_eeprom(dev);
 		if (ret < 0)
 			return ret;
 
 		ret = smsc95xx_read_reg(dev, E2P_DATA, &val);
-		check_warn_return(ret, "Error reading E2P_DATA\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading E2P_DATA\n");
+			return ret;
+		}
 
 		data[i] = val & 0xFF;
 		offset++;
@@ -351,7 +383,10 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 	/* Issue write/erase enable command */
 	val = E2P_CMD_BUSY_ | E2P_CMD_EWEN_;
 	ret = smsc95xx_write_reg(dev, E2P_CMD, val);
-	check_warn_return(ret, "Error writing E2P_DATA\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing E2P_DATA\n");
+		return ret;
+	}
 
 	ret = smsc95xx_wait_eeprom(dev);
 	if (ret < 0)
@@ -362,12 +397,18 @@ static int smsc95xx_write_eeprom(struct usbnet *dev, u32 offset, u32 length,
 		/* Fill data register */
 		val = data[i];
 		ret = smsc95xx_write_reg(dev, E2P_DATA, val);
-		check_warn_return(ret, "Error writing E2P_DATA\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_DATA\n");
+			return ret;
+		}
 
 		/* Send "write" command */
 		val = E2P_CMD_BUSY_ | E2P_CMD_WRITE_ | (offset & E2P_CMD_ADDR_);
 		ret = smsc95xx_write_reg(dev, E2P_CMD, val);
-		check_warn_return(ret, "Error writing E2P_CMD\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing E2P_CMD\n");
+			return ret;
+		}
 
 		ret = smsc95xx_wait_eeprom(dev);
 		if (ret < 0)
@@ -450,13 +491,16 @@ static void smsc95xx_set_multicast(struct net_device *netdev)
 
 	/* Initiate async writes, as we can't wait for completion here */
 	ret = smsc95xx_write_reg_async(dev, HASHH, &pdata->hash_hi);
-	check_warn(ret, "failed to initiate async write to HASHH\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "failed to initiate async write to HASHH\n");
 
 	ret = smsc95xx_write_reg_async(dev, HASHL, &pdata->hash_lo);
-	check_warn(ret, "failed to initiate async write to HASHL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "failed to initiate async write to HASHL\n");
 
 	ret = smsc95xx_write_reg_async(dev, MAC_CR, &pdata->mac_cr);
-	check_warn(ret, "failed to initiate async write to MAC_CR\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "failed to initiate async write to MAC_CR\n");
 }
 
 static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
@@ -465,7 +509,10 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
 	u32 flow, afc_cfg = 0;
 
 	int ret = smsc95xx_read_reg(dev, AFC_CFG, &afc_cfg);
-	check_warn_return(ret, "Error reading AFC_CFG\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading AFC_CFG\n");
+		return ret;
+	}
 
 	if (duplex == DUPLEX_FULL) {
 		u8 cap = mii_resolve_flowctrl_fdx(lcladv, rmtadv);
@@ -490,12 +537,16 @@ static int smsc95xx_phy_update_flowcontrol(struct usbnet *dev, u8 duplex,
 	}
 
 	ret = smsc95xx_write_reg(dev, FLOW, flow);
-	check_warn_return(ret, "Error writing FLOW\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing FLOW\n");
+		return ret;
+	}
 
 	ret = smsc95xx_write_reg(dev, AFC_CFG, afc_cfg);
-	check_warn_return(ret, "Error writing AFC_CFG\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error writing AFC_CFG\n");
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_link_reset(struct usbnet *dev)
@@ -509,10 +560,16 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 
 	/* clear interrupt status */
 	ret = smsc95xx_mdio_read(dev->net, mii->phy_id, PHY_INT_SRC);
-	check_warn_return(ret, "Error reading PHY_INT_SRC\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+		return ret;
+	}
 
 	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
-	check_warn_return(ret, "Error writing INT_STS\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing INT_STS\n");
+		return ret;
+	}
 
 	mii_check_media(mii, 1, 1);
 	mii_ethtool_gset(&dev->mii, &ecmd);
@@ -534,12 +591,16 @@ static int smsc95xx_link_reset(struct usbnet *dev)
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
 	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
-	check_warn_return(ret, "Error writing MAC_CR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing MAC_CR\n");
+		return ret;
+	}
 
 	ret = smsc95xx_phy_update_flowcontrol(dev, ecmd.duplex, lcladv, rmtadv);
-	check_warn_return(ret, "Error updating PHY flow control\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error updating PHY flow control\n");
 
-	return 0;
+	return ret;
 }
 
 static void smsc95xx_status(struct usbnet *dev, struct urb *urb)
@@ -573,7 +634,10 @@ static int smsc95xx_set_features(struct net_device *netdev,
 	int ret;
 
 	ret = smsc95xx_read_reg(dev, COE_CR, &read_buf);
-	check_warn_return(ret, "Failed to read COE_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read COE_CR: %d\n", ret);
+		return ret;
+	}
 
 	if (features & NETIF_F_HW_CSUM)
 		read_buf |= Tx_COE_EN_;
@@ -586,7 +650,10 @@ static int smsc95xx_set_features(struct net_device *netdev,
 		read_buf &= ~Rx_COE_EN_;
 
 	ret = smsc95xx_write_reg(dev, COE_CR, read_buf);
-	check_warn_return(ret, "Failed to write COE_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write COE_CR: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, hw, dev->net, "COE_CR = 0x%08x\n", read_buf);
 	return 0;
@@ -671,9 +738,10 @@ static int smsc95xx_ethtool_set_wol(struct net_device *net,
 	pdata->wolopts = wolinfo->wolopts & SUPPORTED_WAKE;
 
 	ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);
-	check_warn_return(ret, "device_set_wakeup_enable error %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "device_set_wakeup_enable error %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 static const struct ethtool_ops smsc95xx_ethtool_ops = {
@@ -728,12 +796,16 @@ static int smsc95xx_set_mac_address(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_write_reg(dev, ADDRL, addr_lo);
-	check_warn_return(ret, "Failed to write ADDRL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write ADDRL: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_write_reg(dev, ADDRH, addr_hi);
-	check_warn_return(ret, "Failed to write ADDRH: %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "Failed to write ADDRH: %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 /* starts the TX path */
@@ -749,13 +821,17 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
 	ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
-	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write MAC_CR: %d\n", ret);
+		return ret;
+	}
 
 	/* Enable Tx at SCSRs */
 	ret = smsc95xx_write_reg(dev, TX_CFG, TX_CFG_ON_);
-	check_warn_return(ret, "Failed to write TX_CFG: %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "Failed to write TX_CFG: %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 /* Starts the Receive path */
@@ -770,9 +846,10 @@ static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
 	spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
 
 	ret = __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
-	check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
+	if (ret < 0)
+		netdev_warn(dev->net, "Failed to write MAC_CR: %d\n", ret);
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_phy_initialize(struct usbnet *dev)
@@ -807,7 +884,10 @@ static int smsc95xx_phy_initialize(struct usbnet *dev)
 
 	/* read to clear */
 	ret = smsc95xx_mdio_read(dev->net, dev->mii.phy_id, PHY_INT_SRC);
-	check_warn_return(ret, "Failed to read PHY_INT_SRC during init\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read PHY_INT_SRC during init\n");
+		return ret;
+	}
 
 	smsc95xx_mdio_write(dev->net, dev->mii.phy_id, PHY_INT_MASK,
 		PHY_INT_MASK_DEFAULT_);
@@ -826,13 +906,19 @@ static int smsc95xx_reset(struct usbnet *dev)
 	netif_dbg(dev, ifup, dev->net, "entering smsc95xx_reset\n");
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, HW_CFG_LRST_);
-	check_warn_return(ret, "Failed to write HW_CFG_LRST_ bit in HW_CFG\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write HW_CFG_LRST_ bit in HW_CFG\n");
+		return ret;
+	}
 
 	timeout = 0;
 	do {
 		msleep(10);
 		ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
-		check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+			return ret;
+		}
 		timeout++;
 	} while ((read_buf & HW_CFG_LRST_) && (timeout < 100));
 
@@ -842,13 +928,19 @@ static int smsc95xx_reset(struct usbnet *dev)
 	}
 
 	ret = smsc95xx_write_reg(dev, PM_CTRL, PM_CTL_PHY_RST_);
-	check_warn_return(ret, "Failed to write PM_CTRL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write PM_CTRL: %d\n", ret);
+		return ret;
+	}
 
 	timeout = 0;
 	do {
 		msleep(10);
 		ret = smsc95xx_read_reg(dev, PM_CTRL, &read_buf);
-		check_warn_return(ret, "Failed to read PM_CTRL: %d\n", ret);
+		if (ret < 0) {
+			netdev_warn(dev->net, "Failed to read PM_CTRL: %d\n", ret);
+			return ret;
+		}
 		timeout++;
 	} while ((read_buf & PM_CTL_PHY_RST_) && (timeout < 100));
 
@@ -865,7 +957,10 @@ static int smsc95xx_reset(struct usbnet *dev)
 		  dev->net->dev_addr);
 
 	ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG : 0x%08x\n",
 		  read_buf);
@@ -873,10 +968,17 @@ static int smsc95xx_reset(struct usbnet *dev)
 	read_buf |= HW_CFG_BIR_;
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
-	check_warn_return(ret, "Failed to write HW_CFG_BIR_ bit in HW_CFG\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write HW_CFG_BIR_ bit in HW_CFG\n");
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
+
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG after writing HW_CFG_BIR_: 0x%08x\n",
 		  read_buf);
@@ -896,27 +998,42 @@ static int smsc95xx_reset(struct usbnet *dev)
 		  (ulong)dev->rx_urb_size);
 
 	ret = smsc95xx_write_reg(dev, BURST_CAP, burst_cap);
-	check_warn_return(ret, "Failed to write BURST_CAP: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write BURST_CAP: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, BURST_CAP, &read_buf);
-	check_warn_return(ret, "Failed to read BURST_CAP: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read BURST_CAP: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BURST_CAP after writing: 0x%08x\n",
 		  read_buf);
 
 	ret = smsc95xx_write_reg(dev, BULK_IN_DLY, DEFAULT_BULK_IN_DELAY);
-	check_warn_return(ret, "Failed to write BULK_IN_DLY: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write BULK_IN_DLY: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, BULK_IN_DLY, &read_buf);
-	check_warn_return(ret, "Failed to read BULK_IN_DLY: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read BULK_IN_DLY: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from BULK_IN_DLY after writing: 0x%08x\n",
 		  read_buf);
 
 	ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "Read Value from HW_CFG: 0x%08x\n",
 		  read_buf);
@@ -930,66 +1047,111 @@ static int smsc95xx_reset(struct usbnet *dev)
 	read_buf |= NET_IP_ALIGN << 9;
 
 	ret = smsc95xx_write_reg(dev, HW_CFG, read_buf);
-	check_warn_return(ret, "Failed to write HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, HW_CFG, &read_buf);
-	check_warn_return(ret, "Failed to read HW_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read HW_CFG: %d\n", ret);
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net,
 		  "Read Value from HW_CFG after writing: 0x%08x\n", read_buf);
 
 	ret = smsc95xx_write_reg(dev, INT_STS, INT_STS_CLEAR_ALL_);
-	check_warn_return(ret, "Failed to write INT_STS: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write INT_STS: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, ID_REV, &read_buf);
-	check_warn_return(ret, "Failed to read ID_REV: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
+		return ret;
+	}
 	netif_dbg(dev, ifup, dev->net, "ID_REV = 0x%08x\n", read_buf);
 
 	/* Configure GPIO pins as LED outputs */
 	write_buf = LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
 		LED_GPIO_CFG_FDX_LED;
 	ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, write_buf);
-	check_warn_return(ret, "Failed to write LED_GPIO_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write LED_GPIO_CFG: %d\n", ret);
+		return ret;
+	}
 
 	/* Init Tx */
 	ret = smsc95xx_write_reg(dev, FLOW, 0);
-	check_warn_return(ret, "Failed to write FLOW: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write FLOW: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_write_reg(dev, AFC_CFG, AFC_CFG_DEFAULT);
-	check_warn_return(ret, "Failed to write AFC_CFG: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write AFC_CFG: %d\n", ret);
+		return ret;
+	}
 
 	/* Don't need mac_cr_lock during initialisation */
 	ret = smsc95xx_read_reg(dev, MAC_CR, &pdata->mac_cr);
-	check_warn_return(ret, "Failed to read MAC_CR: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read MAC_CR: %d\n", ret);
+		return ret;
+	}
 
 	/* Init Rx */
 	/* Set Vlan */
 	ret = smsc95xx_write_reg(dev, VLAN1, (u32)ETH_P_8021Q);
-	check_warn_return(ret, "Failed to write VLAN1: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write VLAN1: %d\n", ret);
+		return ret;
+	}
 
 	/* Enable or disable checksum offload engines */
 	ret = smsc95xx_set_features(dev->net, dev->net->features);
-	check_warn_return(ret, "Failed to set checksum offload features\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to set checksum offload features\n");
+		return ret;
+	}
 
 	smsc95xx_set_multicast(dev->net);
 
 	ret = smsc95xx_phy_initialize(dev);
-	check_warn_return(ret, "Failed to init PHY\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to init PHY\n");
+		return ret;
+	}
 
 	ret = smsc95xx_read_reg(dev, INT_EP_CTL, &read_buf);
-	check_warn_return(ret, "Failed to read INT_EP_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read INT_EP_CTL: %d\n", ret);
+		return ret;
+	}
 
 	/* enable PHY interrupts */
 	read_buf |= INT_EP_CTL_PHY_INT_;
 
 	ret = smsc95xx_write_reg(dev, INT_EP_CTL, read_buf);
-	check_warn_return(ret, "Failed to write INT_EP_CTL: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to write INT_EP_CTL: %d\n", ret);
+		return ret;
+	}
 
 	ret = smsc95xx_start_tx_path(dev);
-	check_warn_return(ret, "Failed to start TX path\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to start TX path\n");
+		return ret;
+	}
 
 	ret = smsc95xx_start_rx_path(dev, 0);
-	check_warn_return(ret, "Failed to start RX path\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to start RX path\n");
+		return ret;
+	}
 
 	netif_dbg(dev, ifup, dev->net, "smsc95xx_reset, return 0\n");
 	return 0;
@@ -1017,7 +1179,10 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	printk(KERN_INFO SMSC_CHIPNAME " v" SMSC_DRIVER_VERSION "\n");
 
 	ret = usbnet_get_endpoints(dev, intf);
-	check_warn_return(ret, "usbnet_get_endpoints failed: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "usbnet_get_endpoints failed: %d\n", ret);
+		return ret;
+	}
 
 	dev->data[0] = (unsigned long)kzalloc(sizeof(struct smsc95xx_priv),
 		GFP_KERNEL);
@@ -1044,7 +1209,10 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	/* detect device revision as different features may be available */
 	ret = smsc95xx_read_reg(dev, ID_REV, &val);
-	check_warn_return(ret, "Failed to read ID_REV: %d\n", ret);
+	if (ret < 0) {
+		netdev_warn(dev->net, "Failed to read ID_REV: %d\n", ret);
+		return ret;
+	}
 	val >>= 16;
 
 	if ((val == ID_REV_CHIP_ID_9500A_) || (val == ID_REV_CHIP_ID_9530_) ||
@@ -1089,11 +1257,17 @@ static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)
 
 	/* read to clear */
 	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_SRC);
-	check_warn_return(ret, "Error reading PHY_INT_SRC\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_SRC\n");
+		return ret;
+	}
 
 	/* enable interrupt source */
 	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_INT_MASK);
-	check_warn_return(ret, "Error reading PHY_INT_MASK\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_INT_MASK\n");
+		return ret;
+	}
 
 	ret |= mask;
 
@@ -1109,10 +1283,16 @@ static int smsc95xx_link_ok_nopm(struct usbnet *dev)
 
 	/* first, a dummy read, needed to latch some MII phys */
 	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
-	check_warn_return(ret, "Error reading MII_BMSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_BMSR\n");
+		return ret;
+	}
 
 	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, MII_BMSR);
-	check_warn_return(ret, "Error reading MII_BMSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading MII_BMSR\n");
+		return ret;
+	}
 
 	return !!(ret & BMSR_LSTATUS);
 }
@@ -1124,13 +1304,19 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		return ret;
+	}
 
 	val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
 	val |= PM_CTL_SUS_MODE_0;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		return ret;
+	}
 
 	/* clear wol status */
 	val &= ~PM_CTL_WUPS_;
@@ -1141,13 +1327,17 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
 		val |= PM_CTL_WUPS_ED_;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		return ret;
+	}
 
 	/* read back PM_CTRL */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error reading PM_CTRL\n");
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_enter_suspend1(struct usbnet *dev)
@@ -1166,7 +1356,10 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 	/* enable energy detect power-down mode */
 	ret = smsc95xx_mdio_read_nopm(dev->net, mii->phy_id, PHY_MODE_CTRL_STS);
-	check_warn_return(ret, "Error reading PHY_MODE_CTRL_STS\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PHY_MODE_CTRL_STS\n");
+		return ret;
+	}
 
 	ret |= MODE_CTRL_STS_EDPWRDOWN_;
 
@@ -1174,22 +1367,29 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 	/* enter SUSPEND1 mode */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		return ret;
+	}
 
 	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
 	val |= PM_CTL_SUS_MODE_1;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		return ret;
+	}
 
 	/* clear wol status, enable energy detection */
 	val &= ~PM_CTL_WUPS_;
 	val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_enter_suspend2(struct usbnet *dev)
@@ -1198,15 +1398,19 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_return(ret, "Error reading PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		return ret;
+	}
 
 	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
 	val |= PM_CTL_SUS_MODE_2;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_return(ret, "Error writing PM_CTRL\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
-	return 0;
+	return ret;
 }
 
 static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
@@ -1217,7 +1421,10 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	int ret;
 
 	ret = usbnet_suspend(intf, message);
-	check_warn_return(ret, "usbnet_suspend error\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "usbnet_suspend error\n");
+		return ret;
+	}
 
 	/* determine if link is up using only _nopm functions */
 	link_up = smsc95xx_link_ok_nopm(dev);
@@ -1231,20 +1438,32 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		/* disable energy detect (link up) & wake up events */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 
 		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		check_warn_goto_done(ret, "Error reading PM_CTRL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PM_CTRL\n");
+			goto done;
+		}
 
 		val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		check_warn_goto_done(ret, "Error writing PM_CTRL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PM_CTRL\n");
+			goto done;
+		}
 
 		ret = smsc95xx_enter_suspend2(dev);
 		goto done;
@@ -1253,7 +1472,10 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	if (pdata->wolopts & WAKE_PHY) {
 		ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
 			(PHY_INT_MASK_ANEG_COMP_ | PHY_INT_MASK_LINK_DOWN_));
-		check_warn_goto_done(ret, "error enabling PHY wakeup ints\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
+			goto done;
+		}
 
 		/* if link is down then configure EDPD and enter SUSPEND1,
 		 * otherwise enter SUSPEND0 below
@@ -1338,51 +1560,77 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		for (i = 0; i < (wuff_filter_count * 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
-			if (ret < 0)
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing WUFF\n");
 				kfree(filter_mask);
-			check_warn_goto_done(ret, "Error writing WUFF\n");
+				goto done;
+			}
 		}
 		kfree(filter_mask);
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
-			check_warn_goto_done(ret, "Error writing WUFF\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing WUFF\n");
+				goto done;
+			}
 		}
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
-			check_warn_goto_done(ret, "Error writing WUFF\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing WUFF\n");
+				goto done;
+			}
 		}
 
 		for (i = 0; i < (wuff_filter_count / 2); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
-			check_warn_goto_done(ret, "Error writing WUFF\n");
+			if (ret < 0) {
+				netdev_warn(dev->net, "Error writing WUFF\n");
+				goto done;
+			}
 		}
 
 		/* clear any pending pattern match packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_WUFR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	if (pdata->wolopts & WAKE_MAGIC) {
 		/* clear any pending magic packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_goto_done(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			goto done;
+		}
 
 		val |= WUCSR_MPR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_goto_done(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			goto done;
+		}
 	}
 
 	/* enable/disable wakeup sources */
 	ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-	check_warn_goto_done(ret, "Error reading WUCSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading WUCSR\n");
+		goto done;
+	}
 
 	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
 		netdev_info(dev->net, "enabling pattern match wakeup\n");
@@ -1401,11 +1649,17 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	}
 
 	ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-	check_warn_goto_done(ret, "Error writing WUCSR\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing WUCSR\n");
+		goto done;
+	}
 
 	/* enable wol wakeup source */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	check_warn_goto_done(ret, "Error reading PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		goto done;
+	}
 
 	val |= PM_CTL_WOL_EN_;
 
@@ -1414,7 +1668,10 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		val |= PM_CTL_ED_EN_;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	check_warn_goto_done(ret, "Error writing PM_CTRL\n");
+	if (ret < 0) {
+		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		goto done;
+	}
 
 	/* enable receiver to enable frame reception */
 	smsc95xx_start_rx_path(dev, 1);
@@ -1441,28 +1698,41 @@ static int smsc95xx_resume(struct usb_interface *intf)
 	if (pdata->wolopts) {
 		/* clear wake-up sources */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		check_warn_return(ret, "Error reading WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading WUCSR\n");
+			return ret;
+		}
 
 		val &= ~(WUCSR_WAKE_EN_ | WUCSR_MPEN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		check_warn_return(ret, "Error writing WUCSR\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing WUCSR\n");
+			return ret;
+		}
 
 		/* clear wake-up status */
 		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		check_warn_return(ret, "Error reading PM_CTRL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error reading PM_CTRL\n");
+			return ret;
+		}
 
 		val &= ~PM_CTL_WOL_EN_;
 		val |= PM_CTL_WUPS_;
 
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		check_warn_return(ret, "Error writing PM_CTRL\n");
+		if (ret < 0) {
+			netdev_warn(dev->net, "Error writing PM_CTRL\n");
+			return ret;
+		}
 	}
 
 	ret = usbnet_resume(intf);
-	check_warn_return(ret, "usbnet_resume error\n");
+	if (ret < 0)
+		netdev_warn(dev->net, "usbnet_resume error\n");
 
-	return 0;
+	return ret;
 }
 
 static void smsc95xx_rx_csum_offload(struct sk_buff *skb)
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 4/5] smsc95xx: fix smsc_crc return type
From: Joe Perches @ 2012-11-30 15:59 UTC (permalink / raw)
  To: Steve Glendinning; +Cc: netdev
In-Reply-To: <1354290952-27109-5-git-send-email-steve.glendinning@shawell.net>

On Fri, 2012-11-30 at 15:55 +0000, Steve Glendinning wrote:
> This patch fixes a bug introduced in bbd9f9e which could prevent
> some wakeups from working correctly if multiple wol options were
> selected.
> 
> This helper function calculates a 16-bit crc and shifts it into
> either the high or low 16 bits of a u32 so the caller can or it
> directly into place.  The function previously had a u16 return
> type so would always have returned zero when filter was odd.
> 
> Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
> Reported-by: Bjorn Mork <bjorn@mork.no>
> Cc: Joe Perches <joe@perches.com>
> ---
>  drivers/net/usb/smsc95xx.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 064df1a..b9eb490 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -1074,9 +1074,10 @@ static void smsc95xx_unbind(struct usbnet *dev, struct usb_interface *intf)
>  	}
>  }
>  
> -static u16 smsc_crc(const u8 *buffer, size_t len, int filter)
> +static u32 smsc_crc(const u8 *buffer, size_t len, int filter)
>  {
> -	return bitrev16(crc16(0xFFFF, buffer, len)) << ((filter % 2) * 16);
> +	u32 crc = bitrev16(crc16(0xFFFF, buffer, len));
> +	return crc << ((filter % 2) * 16);
>  }
>  
>  static int smsc95xx_enable_phy_wakeup_interrupts(struct usbnet *dev, u16 mask)

Having filter as an argument to this function really
just confuses things.

The shift should be done when the result is or'd onto the
control variable.

^ permalink raw reply

* Re: [PATCH] sctp: verify length provided in heartbeat information parameter
From: Neil Horman @ 2012-11-30 16:01 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, netdev, linux-sctp
In-Reply-To: <20121130121627.GG30697@casper.infradead.org>

On Fri, Nov 30, 2012 at 12:16:27PM +0000, Thomas Graf wrote:
> If the variable parameter length provided in the mandatory
> heartbeat information parameter exceeds the calculated payload
> length the packet has been corrupted. Reply with a parameter
> length protocol violation message.
> 
> Signed-off-by: Thomas Graf <tgraf@suug.ch>
> ---
>  net/sctp/sm_statefuns.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b6adef8..e92079d 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1055,6 +1055,7 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>  				    void *arg,
>  				    sctp_cmd_seq_t *commands)
>  {
> +	sctp_paramhdr_t *param_hdr;
>  	struct sctp_chunk *chunk = arg;
>  	struct sctp_chunk *reply;
>  	size_t paylen = 0;
> @@ -1072,12 +1073,17 @@ sctp_disposition_t sctp_sf_beat_8_3(struct net *net,
>  	 * Information field copied from the received HEARTBEAT chunk.
>  	 */
>  	chunk->subh.hb_hdr = (sctp_heartbeathdr_t *) chunk->skb->data;
> +	param_hdr = (sctp_paramhdr_t *) chunk->subh.hb_hdr;
>  	paylen = ntohs(chunk->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
> +
> +	if (ntohs(param_hdr->length) > paylen)
> +		return sctp_sf_violation_paramlen(net, ep, asoc, type, arg,
> +						  param_hdr, commands);
> +
>  	if (!pskb_pull(chunk->skb, paylen))
>  		goto nomem;
>  
> -	reply = sctp_make_heartbeat_ack(asoc, chunk,
> -					chunk->subh.hb_hdr, paylen);
> +	reply = sctp_make_heartbeat_ack(asoc, chunk, param_hdr, paylen);
>  	if (!reply)
>  		goto nomem;
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Looks good, thanks Thomas.
Acked-by: Neil Horman <nhorman@tuxdriver.com>

^ permalink raw reply

* Re: [PATCH 1/5] smsc95xx: fix suspend buffer overflow
From: Joe Perches @ 2012-11-30 16:03 UTC (permalink / raw)
  To: Steve Glendinning; +Cc: netdev, Bjorn Mork
In-Reply-To: <1354290952-27109-2-git-send-email-steve.glendinning@shawell.net>

On Fri, 2012-11-30 at 15:55 +0000, Steve Glendinning wrote:
> This patch fixes a buffer overflow introduced by bbd9f9e, where
> the filter_mask array is accessed beyond its bounds.
[]
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
[]
> @@ -1281,7 +1281,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>  	}
>  
>  	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
> -		u32 *filter_mask = kzalloc(32, GFP_KERNEL);
> +		u32 *filter_mask = kzalloc(sizeof(u32) * 32, GFP_KERNEL);

Just get rid of the alloc and use 32 u8's on stack.
It's small enough
No filter value is > FF and the compiler with expand
the u8 to u32 when writing to the card.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox