Netdev List
 help / color / mirror / Atom feed
* Re: [GIT net] Open vSwitch
From: Jesse Gross @ 2012-11-28 19:08 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, dev
In-Reply-To: <20121128.134839.2194749267751942776.davem@davemloft.net>

On Wed, Nov 28, 2012 at 10:48 AM, David Miller <davem@davemloft.net> wrote:
>
> Date: Tue, 27 Nov 2012 10:37:01 -0800
>
> Is it really Tuesday morning where you are?

Unfortunately, no.  It's VM clock slew, which I've fixed now.

^ permalink raw reply

* Re: TCP and reordering
From: Vijay Subramanian @ 2012-11-28 18:52 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Rick Jones, Saku Ytti, netdev
In-Reply-To: <1354127585.14302.467.camel@edumazet-glaptop>

>
> Note that this sysctl controls the initial value of the per socket
> reordering value. It _does_ increase automatically (assuming SACK is
> enabled of course)
>
>
>

I believe enabling SACK is not a requirement. Even with plain Reno,
reordering  is tracked in tp->reordering and the reordering event is
counted in LINUX_MIB_TCPRENOREORDER.
tcp_add_reno_sack()-->tcp_check_reno_reordering() is the code path.

Thanks,
Vijay

^ permalink raw reply

* Re: [GIT net] Open vSwitch
From: David Miller @ 2012-11-28 18:48 UTC (permalink / raw)
  To: jesse; +Cc: netdev, dev
In-Reply-To: <1354041423-3050-1-git-send-email-jesse@nicira.com>


Date: Tue, 27 Nov 2012 10:37:01 -0800

Is it really Tuesday morning where you are?

^ permalink raw reply

* Re: TCP and reordering
From: Saku Ytti @ 2012-11-28 18:44 UTC (permalink / raw)
  To: netdev
In-Reply-To: <50B656CC.1040001@hp.com>

On (2012-11-28 10:24 -0800), Rick Jones wrote:

> By increasing net.ipv4.tcp_reordering you make the sending TCP less
> "sensitive" to duplicate ACKs and so less sensitive to reordering
> detected by the receiver.  The receiver is generating as many
> (duplicate) ACKs as before, it is just that the sender is ignoring
> them a bit more.

Thanks, I didn't know this.

-- 
  ++ytti

^ permalink raw reply

* Re: [PATCH net-next] gro: Handle inline VLAN tags
From: David Miller @ 2012-11-28 18:39 UTC (permalink / raw)
  To: gallatin; +Cc: eric.dumazet, bhutchings, netdev, linux-net-drivers, herbert
In-Reply-To: <50B64A24.60200@myri.com>

From: Andrew Gallatin <gallatin@myri.com>
Date: Wed, 28 Nov 2012 12:30:12 -0500

> Perfect, thanks.  Eric seemed OK with doing the vlan decap
> in the driver.  But.. I didn't get an "nack" or an "applied" from
> you in response to that myri10ge patchset.  Do I need to re-submit it?
> 
> Sorry, I'm not terribly familiar with the etiquette for this sort
> of thing.

Please just repost the patch set.

^ permalink raw reply

* Re: [PATCH 1/2] arping: Fix finding of a default interface when no -I is specified.
From: YOSHIFUJI Hideaki @ 2012-11-28 18:38 UTC (permalink / raw)
  To: Jan Synacek; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1354018775-4966-1-git-send-email-jsynacek@redhat.com>

Jan Synacek wrote:
> By default, no interface string should be supplied. This ensures that we can
> recognize if an interface was specified or omitted. Previously, when -I was not
> used, default interface string was compiled-in and the automatic selection
> didn't work correctly.

Okay, but to retain default device support compiled in,
different patch applied.

Thank you, anyway.

P.S. Even if default interface compiled in, you can say "-I ''" to
override that.

--yoshfuji

^ permalink raw reply

* Re: TCP and reordering
From: Eric Dumazet @ 2012-11-28 18:33 UTC (permalink / raw)
  To: Rick Jones; +Cc: Saku Ytti, netdev
In-Reply-To: <50B656CC.1040001@hp.com>

On Wed, 2012-11-28 at 10:24 -0800, Rick Jones wrote:
> On 11/28/2012 12:54 AM, Saku Ytti wrote:
> > On (2012-11-28 00:35 -0800), Vijay Subramanian wrote:
> >
> >> Also note that reordering is tracked on the sender side using the
> >> per flow variable tp->reordering . This measures the amount of
> >> reordering on the connection so that fast retransmit and other loss
> >> recovery mechanisms are not entered prematurely. Doesn't this
> >> behavior at the  sender already provide the behavior you seek?
> >
> > Sorry I don't seem to understand what you mean. Do you mind explaining how
> > the sender can help to restore performance on reordering network?
> 
> tp->reordering is initialized via the sysctl net.ipv4.tcp_reordering 
> which controls how anxious TCP will be to fast retransmit.
> 
> By increasing net.ipv4.tcp_reordering you make the sending TCP less 
> "sensitive" to duplicate ACKs and so less sensitive to reordering 
> detected by the receiver.  The receiver is generating as many 
> (duplicate) ACKs as before, it is just that the sender is ignoring them 
> a bit more.

Note that this sysctl controls the initial value of the per socket
reordering value. It _does_ increase automatically (assuming SACK is
enabled of course)

^ permalink raw reply

* Re: [RFC PATCH] 8139cp: properly support change of MTU values
From: John Greene @ 2012-11-28 18:00 UTC (permalink / raw)
  To: Rami Rosen; +Cc: netdev
In-Reply-To: <CAKoUArk7R3_JBn7Qdn8Ry+qy7NnidroqSArTpFwdP4WkryJypA@mail.gmail.com>

On 11/28/2012 02:23 AM, Rami Rosen wrote:
> Hi,
>
> In cp_change_mtu(), there is the following check:
> ...
> if (new_mtu < CP_MIN_MTU || new_mtu > CP_MAX_MTU)
> 		return -EINVAL;
> ...
> Later on, we set dev->mtu to new_mtu.
>
> The CP_MIN_MTU is defined to be 60; shouldn't it be 68 ?
>
>
> The reason for 68 is (RFC 791,  Internet Protocol,
> http://www.ietf.org/rfc/rfc791.txt):
> "Every internet module must be able to forward a datagram of 68 octets
> without further fragmentation.  This is because an internet  header
> may be up to 60 octets, and the minimum fragment is 8 octets."
>
> See also the generic Ethernet () method in eth_change_mtu() (net/ethernet/eth.c)
>
> int eth_change_mtu(struct net_device *dev, int new_mtu)
> {
> 	if (new_mtu < 68 || new_mtu > ETH_DATA_LEN)
> 		return -EINVAL;
> 	dev->mtu = new_mtu;
> 	return 0;
> }
>
>
> regards,
> Rami Rosen
>
> http://ramirose.wix.com/ramirosen
>
> On Tue, Nov 27, 2012 at 10:08 PM, John Greene <jogreene@redhat.com> wrote:
>> The 8139cp driver has a change_mtu function that has not been
>> enabled since the dawn of the git repository. However, the
>> generic eth_change_mtu is not used in its place, so that
>> invalid MTU values can be set on the interface.
>>
>> Original patch salvages the broken code for the single case of
>> setting the MTU while the interface is down, which is safe
>> and also includes the range check.  Now enhanced to support up
>> or down interface.
>>
>> Original patch from
>> http://lkml.indiana.edu/hypermail/linux/kernel/1202.2/00770.html
>>
>> Testing: has been test on virtual 8139cp setup without issue,
>> awaiting real hardware and retest again, but wanted to post now.
>>
>> Signed-Off-By: "John Greene" <jogreene@redhat.com>
>> CC: "David S. Miller" <davem@davemloft.net>commit c3f214170cd1abb89290815c3894b945a86894fe
Author: John Greene <jogreene@redhat.com>
Date:   Mon Nov 26 16:05:06 2012 -0500

     8139cp: properly support change of MTU values

>> ---
>>   drivers/net/ethernet/realtek/8139cp.c | 22 +++-------------------
>>   1 file changed, 3 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
>> index 6cb96b4..7847c83 100644
>> --- a/drivers/net/ethernet/realtek/8139cp.c
>> +++ b/drivers/net/ethernet/realtek/8139cp.c
>> @@ -1226,12 +1226,9 @@ static void cp_tx_timeout(struct net_device *dev)
>>          spin_unlock_irqrestore(&cp->lock, flags);
>>   }
>>
>> -#ifdef BROKEN
>>   static int cp_change_mtu(struct net_device *dev, int new_mtu)
>>   {
>>          struct cp_private *cp = netdev_priv(dev);
>> -       int rc;
>> -       unsigned long flags;
>>
>>          /* check for invalid MTU, according to hardware limits */
>>          if (new_mtu < CP_MIN_MTU || new_mtu > CP_MAX_MTU)
>> @@ -1244,22 +1241,11 @@ static int cp_change_mtu(struct net_device *dev, int new_mtu)
>>                  return 0;
>>          }
>>
>> -       spin_lock_irqsave(&cp->lock, flags);
>> -
>> -       cp_stop_hw(cp);                 /* stop h/w and free rings */
>> -       cp_clean_rings(cp);
>> -
>> +       /* network IS up, close it, reset MTU, and come up again. */
>> +       cp_close(dev);
>>          dev->mtu = new_mtu;
>> -       cp_set_rxbufsize(cp);           /* set new rx buf size */
>> -
>> -       rc = cp_init_rings(cp);         /* realloc and restart h/w */
>> -       cp_start_hw(cp);
>> -
>> -       spin_unlock_irqrestore(&cp->lock, flags);
>> -
>> -       return rc;
>> +       return cp_open(dev);
>>   }
>> -#endif /* BROKEN */
>>
>>   static const char mii_2_8139_map[8] = {
>>          BasicModeCtrl,
>> @@ -1835,9 +1821,7 @@ static const struct net_device_ops cp_netdev_ops = {
>>          .ndo_start_xmit         = cp_start_xmit,
>>          .ndo_tx_timeout         = cp_tx_timeout,
>>          .ndo_set_features       = cp_set_features,
>> -#ifdef BROKEN
>>          .ndo_change_mtu         = cp_change_mtu,
>> -#endif
>>
>>   #ifdef CONFIG_NET_POLL_CONTROLLER
>>          .ndo_poll_controller    = cp_poll_controller,
>> --
>> 1.7.11.7
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Good catch Rami.  Thanks.

^ permalink raw reply

* Re: TCP and reordering
From: Rick Jones @ 2012-11-28 18:24 UTC (permalink / raw)
  To: Saku Ytti; +Cc: netdev
In-Reply-To: <20121128085404.GB26010@pob.ytti.fi>

On 11/28/2012 12:54 AM, Saku Ytti wrote:
> On (2012-11-28 00:35 -0800), Vijay Subramanian wrote:
>
>> Also note that reordering is tracked on the sender side using the
>> per flow variable tp->reordering . This measures the amount of
>> reordering on the connection so that fast retransmit and other loss
>> recovery mechanisms are not entered prematurely. Doesn't this
>> behavior at the  sender already provide the behavior you seek?
>
> Sorry I don't seem to understand what you mean. Do you mind explaining how
> the sender can help to restore performance on reordering network?

tp->reordering is initialized via the sysctl net.ipv4.tcp_reordering 
which controls how anxious TCP will be to fast retransmit.

By increasing net.ipv4.tcp_reordering you make the sending TCP less 
"sensitive" to duplicate ACKs and so less sensitive to reordering 
detected by the receiver.  The receiver is generating as many 
(duplicate) ACKs as before, it is just that the sender is ignoring them 
a bit more.

^ permalink raw reply

* Re: [PATCH v2 net-next] net: move inet_dport/inet_num in sock_common
From: Eric Dumazet @ 2012-11-28 18:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Ling Ma, Joe Perches, Ben Hutchings
In-Reply-To: <1354107378.14302.149.camel@edumazet-glaptop>

On Wed, 2012-11-28 at 04:56 -0800, Eric Dumazet wrote:

> +#define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif)	\
> +	((inet_sk(__sk)->inet_portpair == (__ports))		&&	\
> +	 (inet_sk(__sk)->inet_addrpair == (__addrpair))		&&	\
> +	 (!(__sk)->sk_bound_dev_if	||				\
> +	   ((__sk)->sk_bound_dev_if == (__dif))) 		&& 	\
> +	 net_eq(sock_net(__sk), (__net)))
> +#define INET_TW_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif)\
> +	((inet_twsk(__sk)->tw_portpair == (__ports))	&&		\
> +	 (inet_twsk(__sk)->tw_addrpair == (__addrpair))	&&		\
> +	 (!(__sk)->sk_bound_dev_if	||				\
> +	   ((__sk)->sk_bound_dev_if == (__dif)))	&&		\
> +	 net_eq(sock_net(__sk), (__net)))

Last minute change in a param name __cookie / __addrpair

I reverted this change as the 32bit part is not using the 'cookie', so
keeping __the cookie name makes sense.

I'll fix this and add performance results in the changelog.

^ permalink raw reply

* Re: [PATCH 2/2] arping: Call usage() before limiting capabilities.
From: YOSHIFUJI Hideaki @ 2012-11-28 18:18 UTC (permalink / raw)
  To: Jan Synacek; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1354018775-4966-2-git-send-email-jsynacek@redhat.com>

Jan Synacek wrote:
> Otherwise, running arping binary without the capabilities set results in printing
> warnings with the usage.
> 
> Signed-off-by: Jan Synacek <jsynacek@redhat.com>

Fixed in different way.  ping/ping6 has also been fixed.
Thank you.

--yoshfuji

^ permalink raw reply

* Re: [PATCH] smsc95xx: fix suspend buffer overflow
From: Steve Glendinning @ 2012-11-28 18:06 UTC (permalink / raw)
  To: Joe Perches; +Cc: Steve Glendinning, netdev, Dan Carpenter
In-Reply-To: <1354038178.8028.0.camel@joe-AO722>

>>       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);
>
> It's also unchecked for alloc failure.

Thanks both, I've resubmitted with an alloc failure check,

I completely agree - that filter code isn't pretty!  If you have time
to knock up a patch I'd be happy to test it.

Steve

^ permalink raw reply

* Re: TCP and reordering
From: David Woodhouse @ 2012-11-28 18:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Benjamin LaHaise, Vijay Subramanian, David Miller, saku,
	rick.jones2, netdev
In-Reply-To: <1354122996.14302.427.camel@edumazet-glaptop>

[-- Attachment #1: Type: text/plain, Size: 643 bytes --]

On Wed, 2012-11-28 at 09:16 -0800, Eric Dumazet wrote:
> Its the driver responsibility to maintain the coherent 'bytes' value for
> each transmitted/completed packet.
> 
> If a driver calls an external entity, it cannot possibly use BQL, unless
> doing an approximation (bytes becomes a fixed value)

If tracking the original destructor, I can also track the original size
before the skb got passed down the stack.

> BQL was really something to control/limit queueing on ethernet links,
> not for stacked devices, as stacked devices normally have no queue.

Stacked devices have more queue than anything else :)

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

^ permalink raw reply

* [PATCH] smsc75xx: don't call usbnet_resume if usbnet_suspend fails
From: Steve Glendinning @ 2012-11-28 17:57 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning

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 3/3] smsc95xx: don't enable remote wakeup directly
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning, Bjorn Mork
In-Reply-To: <1354124819-29531-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 2/3] smsc95xx: fix error handling in suspend failure case
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning
In-Reply-To: <1354124819-29531-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 1/3] smsc95xx: fix suspend buffer overflow
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning, Bjorn Mork, Joe Perches
In-Reply-To: <1354124819-29531-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 0/3] smsc95xx enhancements
From: Steve Glendinning @ 2012-11-28 17:46 UTC (permalink / raw)
  To: netdev; +Cc: Steve Glendinning

This patchset is a resubmission of 1 updated patch and two new
enhancements.

Please consider all for net-next

Steve Glendinning (3):
  smsc95xx: fix suspend buffer overflow
  smsc95xx: fix error handling in suspend failure case
  smsc95xx: don't enable remote wakeup directly

 drivers/net/usb/smsc95xx.c |   85 ++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 46 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: [PATCH] bonding: make arp_ip_target parameter checks consistent with sysfs
From: Bjørn Mork @ 2012-11-28 17:34 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: David Miller, netdev, andy, fubar
In-Reply-To: <50B63CF4.8070404@redhat.com>

Nikolay Aleksandrov <nikolay@redhat.com> writes:

> Actually I did see it from kernel/module.c and didn't come up with it
> myself.

Just FYI, running  scripts/checkpatch.pl -f kernel/module.c results in:

total: 27 errors, 65 warnings, 3639 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

so I'd not use that as a style guide if I were you....



Bjørn

^ permalink raw reply

* Re: [PATCH net-next] gro: Handle inline VLAN tags
From: Andrew Gallatin @ 2012-11-28 17:30 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, bhutchings, netdev, linux-net-drivers, herbert
In-Reply-To: <20121128.114602.1208211617672057364.davem@davemloft.net>

On 11/28/12 11:46, David Miller wrote:
> From: Andrew Gallatin <gallatin@myri.com>
> Date: Mon, 26 Nov 2012 10:04:44 -0500
>
>> How do you feel about the patchset I posted on 11/14/2012
>> ([PATCH net-next 0/3] myri10ge: LRO to GRO conversion,
>> http://marc.info/?l=linux-netdev&m=135289838223920&w=2)
>> which moves myri10ge from LRO to GRO?
>>
>> Specifically, if doing vlan decap in GRO is not OK, then how
>> about doing it in the driver?
>
> I'm going to trust Eric's judgment on this one, as he has been
> thinking more about the long term GRO issues than I have.
>
> Thanks.
>

Perfect, thanks.  Eric seemed OK with doing the vlan decap
in the driver.  But.. I didn't get an "nack" or an "applied" from
you in response to that myri10ge patchset.  Do I need to re-submit it?

Sorry, I'm not terribly familiar with the etiquette for this sort
of thing.

Thanks,

Drew

^ permalink raw reply

* Re: [PATCH v2 net-next] net: move inet_dport/inet_num in sock_common
From: Eric Dumazet @ 2012-11-28 17:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, ling.ma.program, joe, bhutchings
In-Reply-To: <20121128.120243.1053933388686939425.davem@davemloft.net>

On Wed, 2012-11-28 at 12:02 -0500, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 28 Nov 2012 11:48:35 -0500 (EST)
> 
> > I like this a lot, applied, thanks Eric!
> 
> And I'd like it even more if it was build tested :-/
> 
> net/ipv4/inet_hashtables.c: In function ‘__inet_lookup_established’:
> net/ipv4/inet_hashtables.c:242:7: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:246:8: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:267:7: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:274:8: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:226:2: warning: unused variable ‘acookie’ [-Wunused-variable]
> net/ipv4/inet_hashtables.c: In function ‘__inet_check_established’:
> net/ipv4/inet_hashtables.c:326:7: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:341:7: error: expected expression before ‘)’ token
> net/ipv4/inet_hashtables.c:307:2: warning: unused variable ‘acookie’ [-Wunused-variable]
> net/ipv4/inet_hashtables.c:306:6: warning: unused variable ‘dif’ [-Wunused-variable]
> 
> This is with gcc-4.7.2-2 on Fedora 17 on an allmodconfig build.

Hmm, sorry David, I'll double check and send a v3.

Thanks

^ permalink raw reply

* Re: TCP and reordering
From: Eric Dumazet @ 2012-11-28 17:16 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Benjamin LaHaise, Vijay Subramanian, David Miller, saku,
	rick.jones2, netdev
In-Reply-To: <1354120887.21562.87.camel@shinybook.infradead.org>

On Wed, 2012-11-28 at 16:41 +0000, David Woodhouse wrote:

> Another fun issue with tunnelling protocols and BQL... packets tend to
> *grow* as they get encapsulated. So you might end up calling
> netdev_sent_queue() with a given size, then netdev_completed_queue()
> with a bigger packet later...

Its the driver responsibility to maintain the coherent 'bytes' value for
each transmitted/completed packet.

If a driver calls an external entity, it cannot possibly use BQL, unless
doing an approximation (bytes becomes a fixed value)

BQL was really something to control/limit queueing on ethernet links,
not for stacked devices, as stacked devices normally have no queue.

^ permalink raw reply

* Re: [PATCH 1/2] smsc75xx: refactor entering suspend modes
From: Steve Glendinning @ 2012-11-28 17:16 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: Alan Stern, netdev, linux-usb, David Miller
In-Reply-To: <87wqx528ci.fsf@nemi.mork.no>

>> +
>> + ret = device_set_wakeup_enable(&net->dev, pdata->wolopts);
>
> You are touching the network device here.  That should have been the USB
> device.  Try something like
>
>  ret = device_set_wakeup_enable(&dev->udev->dev, pdata->wolopts);

Perfect, this works exactly as expected now.  Patch included in
resubmitted patchset, thanks Bjorn!

^ permalink raw reply

* Re: [PATCH] br2684: don't send frames on not-ready vcc
From: David Miller @ 2012-11-28 17:11 UTC (permalink / raw)
  To: dwmw2; +Cc: chas, krzysiek, netdev, linux-kernel, nathan
In-Reply-To: <1354122555.21562.92.camel@shinybook.infradead.org>

From: David Woodhouse <dwmw2@infradead.org>
Date: Wed, 28 Nov 2012 17:09:15 +0000

> And then I'll either send an explicit pull request, or submit it as
> patches ― whichever you prefer.

The canonical thing is to do both, send the pull request in the
"[PATCH 0/N]" email, and then the patches so everyone can see the
final form of the changes.

> Hopefully in the next day or so; the merge window is approaching...

Yep, thanks.

^ permalink raw reply

* Re: [PATCH] br2684: don't send frames on not-ready vcc
From: David Woodhouse @ 2012-11-28 17:09 UTC (permalink / raw)
  To: David Miller; +Cc: chas, krzysiek, netdev, linux-kernel, nathan
In-Reply-To: <20121128.120415.778262714601372592.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On Wed, 2012-11-28 at 12:04 -0500, David Miller wrote:
> Do you want me to pull that tree into net-next or is there a plan to
> repost the entire series of work for a final submission?

I think it needs a little more testing/consensus first. I'd like an ack
from Chas on the atm ->release_cb() thing, at least. And I wouldn't mind
confirmation from Nathan's customer that they're no longer seeing the
panics.

And then I'll either send an explicit pull request, or submit it as
patches — whichever you prefer.

Hopefully in the next day or so; the merge window is approaching...

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]

^ 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