Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Oliver Neukum @ 2012-12-18 13:11 UTC (permalink / raw)
  To: Lucas Stach; +Cc: netdev, linux-usb
In-Reply-To: <1355832626-3034-1-git-send-email-dev@lynxeye.de>

On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 9bbeabf..8e9516f 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -106,6 +106,7 @@ struct driver_info {
>   */
>  #define FLAG_MULTI_PACKET      0x2000
>  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */

Hi,

this looks sensible, but
why are you adding a flag unused in usbnet to usbnet.h?

	Regards
		Oliver

^ permalink raw reply

* Re: [PATCH] net: fec: forbid FEC_PTP on SoCs that do not support
From: Shawn Guo @ 2012-12-18 13:12 UTC (permalink / raw)
  To: netdev
  Cc: Sascha Hauer, Richard Cochran, David S. Miller, Frank Li,
	linux-arm-kernel
In-Reply-To: <1355836004-22067-1-git-send-email-shawn.guo@linaro.org>

Misspelled Frank's email address.  Copy him.  Sorry.

On Tue, Dec 18, 2012 at 09:06:44PM +0800, Shawn Guo wrote:
> Beside imx6q, the kernel built from imx_v6_v7_defconfig is also
> supposed to be running on other IMX SoCs that do not have the PTP
> block.  Before fec driver gets fixed to run-time detect target hardware
> rather than conditional compiling with #ifdef CONFIG_FEC_PTP, let's
> give it a quick fix in Kconfig to forbid FEC_PTP on those IMX SoCs that
> do not support PTP.
> 
> Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>  drivers/net/ethernet/freescale/Kconfig |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
> index 5ba6e1c..ec490d7 100644
> --- a/drivers/net/ethernet/freescale/Kconfig
> +++ b/drivers/net/ethernet/freescale/Kconfig
> @@ -94,9 +94,8 @@ config GIANFAR
>  
>  config FEC_PTP
>  	bool "PTP Hardware Clock (PHC)"
> -	depends on FEC && ARCH_MXC
> +	depends on FEC && ARCH_MXC && !SOC_IMX25 && !SOC_IMX27 && !SOC_IMX35 && !SOC_IMX5
>  	select PTP_1588_CLOCK
> -	default y if SOC_IMX6Q
>  	--help---
>  	  Say Y here if you want to use PTP Hardware Clock (PHC) in the
>  	  driver.  Only the basic clock operations have been implemented.
> -- 
> 1.7.9.5
> 
> 

^ permalink raw reply

* Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Lucas Stach @ 2012-12-18 13:24 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: netdev, linux-usb
In-Reply-To: <5491619.g500gaSnsh@linux-lqwf.site>

Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > index 9bbeabf..8e9516f 100644
> > --- a/include/linux/usb/usbnet.h
> > +++ b/include/linux/usb/usbnet.h
> > @@ -106,6 +106,7 @@ struct driver_info {
> >   */
> >  #define FLAG_MULTI_PACKET      0x2000
> >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> 
> Hi,
> 
> this looks sensible, but
> why are you adding a flag unused in usbnet to usbnet.h?

Right, this might not be the right place to add this. Could you point me
to a more appropriate place? The data member of usbnet might be a good
place to stuff this into, but why is this a plain long and not some kind
of pointer? It is used for a different purpose on other ASIX chips
already.

Regards,
Lucas

^ permalink raw reply

* Re: tc ipt action
From: Jamal Hadi Salim @ 2012-12-18 13:23 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Pablo Neira Ayuso, Yury Stankevich, shemonc,
	netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <alpine.LNX.2.01.1212171401180.25980@nerf07.vanv.qr>

On 12-12-17 08:28 AM, Jan Engelhardt wrote:
>
> On Monday 2012-12-17 13:58, Jamal Hadi Salim wrote:

> AFAICS, (one instance of) act_ipt today directly invokes (exactly one
> instance of) a target.

Design intent.
You can have the same target instance used by specifying the same index
on the command line.

>With act_xt2 as drafted, it instead invokes a chain, which would
>
> 1. leave the construction of the target data and calling it
>     to the subsystems they conceptually belong to - the packet filter
> 2. lets you do matches, jumps and all that.
>

I like #2. For #1 as long as it doesnt deviate from desire to have one 
or more instances of targets, we should be fine.

> Good thing you ask. Chain names are unique within a netns, and this
> act_xtables.c draft looks at the packet to get to know its netns, so
> that seems fine.

My motivation for that question:
Is it possible to ignore the hook and tablename and just use the chain
name?

> However, your question also leads to looking at whether TC Actions
> themselves are sufficiently netns-ified, and it seems this is _not_
> the case. Am I right in the observation that variables like
> "tcf_ipt_ht" are in fact global rather tha per-netns?

In general we dont need to worry about netns since actions are attached 
to the filters which are dependent on qdiscs which are dependent on 
netdevs which are per netns.
I believe actions (not filters or qdiscs) have a way where this can
be circumvented in one scenario (I can configure them bypassing the 
filter interface). Thanks for bringing this up - I will look at it.

cheers,
jamal

^ permalink raw reply

* Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Oliver Neukum @ 2012-12-18 13:33 UTC (permalink / raw)
  To: Lucas Stach
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1355837072.1490.71.camel@tellur>

On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > index 9bbeabf..8e9516f 100644
> > > --- a/include/linux/usb/usbnet.h
> > > +++ b/include/linux/usb/usbnet.h
> > > @@ -106,6 +106,7 @@ struct driver_info {
> > >   */
> > >  #define FLAG_MULTI_PACKET      0x2000
> > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > 
> > Hi,
> > 
> > this looks sensible, but
> > why are you adding a flag unused in usbnet to usbnet.h?
> 
> Right, this might not be the right place to add this. Could you point me
> to a more appropriate place? The data member of usbnet might be a good

driver_priv is intended for such stuff

> place to stuff this into, but why is this a plain long and not some kind
> of pointer? It is used for a different purpose on other ASIX chips
> already.

But there is another pointer.

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Lucas Stach @ 2012-12-18 13:38 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: netdev, linux-usb
In-Reply-To: <1672154.FXjtO9OFRj@linux-lqwf.site>

Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum:
> On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > > index 9bbeabf..8e9516f 100644
> > > > --- a/include/linux/usb/usbnet.h
> > > > +++ b/include/linux/usb/usbnet.h
> > > > @@ -106,6 +106,7 @@ struct driver_info {
> > > >   */
> > > >  #define FLAG_MULTI_PACKET      0x2000
> > > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > > 
> > > Hi,
> > > 
> > > this looks sensible, but
> > > why are you adding a flag unused in usbnet to usbnet.h?
> > 
> > Right, this might not be the right place to add this. Could you point me
> > to a more appropriate place? The data member of usbnet might be a good
> 
> driver_priv is intended for such stuff
> 
I'm not talking about the usbnet struct, but the driver_info struct. I
need a way to pass this flag from the static driver info to the bind
function. I don't even have a usbnet device at this point, where I could
hang on driver_priv data.

Regards,
Lucas

^ permalink raw reply

* Re: [PATCH 2/2] net: asix: handle packets crossing URB boundaries
From: Oliver Neukum @ 2012-12-18 13:42 UTC (permalink / raw)
  To: Lucas Stach; +Cc: netdev, linux-usb
In-Reply-To: <1355832626-3034-2-git-send-email-dev@lynxeye.de>

On Tuesday 18 December 2012 13:10:26 Lucas Stach wrote:
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -495,6 +495,10 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
>                 dev->rx_urb_size = 2048;
>         }
>  
> +       dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
> +       if (!dev->driver_priv)
> +               return -ENOMEM;
> +
>         return 0;

Where is this freed?

	Regards
		Oliver

^ permalink raw reply

* Re: [PATCH 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Oliver Neukum @ 2012-12-18 13:56 UTC (permalink / raw)
  To: Lucas Stach; +Cc: netdev, linux-usb
In-Reply-To: <1355837912.1490.73.camel@tellur>

On Tuesday 18 December 2012 14:38:32 Lucas Stach wrote:
> Am Dienstag, den 18.12.2012, 14:33 +0100 schrieb Oliver Neukum:
> > On Tuesday 18 December 2012 14:24:32 Lucas Stach wrote:
> > > Am Dienstag, den 18.12.2012, 14:11 +0100 schrieb Oliver Neukum:
> > > > On Tuesday 18 December 2012 13:10:25 Lucas Stach wrote:
> > > > > diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> > > > > index 9bbeabf..8e9516f 100644
> > > > > --- a/include/linux/usb/usbnet.h
> > > > > +++ b/include/linux/usb/usbnet.h
> > > > > @@ -106,6 +106,7 @@ struct driver_info {
> > > > >   */
> > > > >  #define FLAG_MULTI_PACKET      0x2000
> > > > >  #define FLAG_RX_ASSEMBLE       0x4000  /* rx packets may span >1 frames */
> > > > > +#define FLAG_EEPROM_MAC                0x8000  /* initialize device MAC from eeprom */
> > > > 
> > > > Hi,
> > > > 
> > > > this looks sensible, but
> > > > why are you adding a flag unused in usbnet to usbnet.h?
> > > 
> > > Right, this might not be the right place to add this. Could you point me
> > > to a more appropriate place? The data member of usbnet might be a good
> > 
> > driver_priv is intended for such stuff
> > 
> I'm not talking about the usbnet struct, but the driver_info struct. I
> need a way to pass this flag from the static driver info to the bind
> function. I don't even have a usbnet device at this point, where I could
> hang on driver_priv data.

True, sorry I misread the patch. You could split up ax88772_bind()
implicitly encoding the flag. That would be reasonably clean. If you
really don't want to do that, please add another private field. But
we cannot pollute the flags.

	Regards
		Oliver

^ permalink raw reply

* [PATCH] ksz884x: fix receive polling race condition
From: Lennert Buytenhek @ 2012-12-18 13:57 UTC (permalink / raw)
  To: Tristram Ha, Xiaotian Feng, netdev; +Cc: Chris Healy

The ksz884x driver does receive processing in a custom tasklet, and
seems to be assuming that since it takes its private interface spinlock
with spin_lock_irq(), it won't be running concurrently with its own
interrupt handler, as it cannot be preempted by it, but since its
interrupt handler doesn't do any locking whatsoever, the receive
processing tasklet and interrupt handler can end up running concurrently
on different CPUs.

As a result of this, the ksz884x receive path ends up locking up fairly
easily, when the receive processing tasklet's reenabling of receive
interrupts (due to it being done with polling the receive ring) races
with the interrupt handler's disabling of receive interrupts (due to a
new receive interrupt coming in) resulting in the receive interrupt
being masked but the receive processing tasklet not being scheduled.

Fix this by making the ksz884x interrupt handler take its private
interface spinlock.  This requires upgrading the spin_lock() in the
transmit cleanup tasklet to a spin_lock_irq(), as otherwise the IRQ
handler can preempt transmit cleanup and deadlock the system, but
with those two changes, no more receive lockups have been observed.

Reported-by: Chris Healy <cphealy@gmail.com>
Signed-off-by: Lennert Buytenhek <buytenh@wantstofly.org>

diff --git a/drivers/net/ethernet/micrel/ksz884x.c b/drivers/net/ethernet/micrel/ksz884x.c
index 69e0197..f131962 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -4761,7 +4761,7 @@ static void transmit_cleanup(struct dev_info *hw_priv, int normal)
 	struct ksz_dma_buf *dma_buf;
 	struct net_device *dev = NULL;
 
-	spin_lock(&hw_priv->hwlock);
+	spin_lock_irq(&hw_priv->hwlock);
 	last = info->last;
 
 	while (info->avail < info->alloc) {
@@ -4795,7 +4795,7 @@ static void transmit_cleanup(struct dev_info *hw_priv, int normal)
 		info->avail++;
 	}
 	info->last = last;
-	spin_unlock(&hw_priv->hwlock);
+	spin_unlock_irq(&hw_priv->hwlock);
 
 	/* Notify the network subsystem that the packet has been sent. */
 	if (dev)
@@ -5259,11 +5259,15 @@ static irqreturn_t netdev_intr(int irq, void *dev_id)
 	struct dev_info *hw_priv = priv->adapter;
 	struct ksz_hw *hw = &hw_priv->hw;
 
+	spin_lock(&hw_priv->hwlock);
+
 	hw_read_intr(hw, &int_enable);
 
 	/* Not our interrupt! */
-	if (!int_enable)
+	if (!int_enable) {
+		spin_unlock(&hw_priv->hwlock);
 		return IRQ_NONE;
+	}
 
 	do {
 		hw_ack_intr(hw, int_enable);
@@ -5310,6 +5314,8 @@ static irqreturn_t netdev_intr(int irq, void *dev_id)
 
 	hw_ena_intr(hw);
 
+	spin_unlock(&hw_priv->hwlock);
+
 	return IRQ_HANDLED;
 }
 
----

^ permalink raw reply related

* [PATCH] xen/netfront: improve truesize tracking
From: Ian Campbell @ 2012-12-18 13:51 UTC (permalink / raw)
  To: netdev
  Cc: Ian Campbell, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
	xen-devel

Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
than that. We have already accounted for this in
NETFRONT_SKB_CB(skb)->pull_to so use that instead.

Fixes WARN_ON from skb_try_coalesce.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: annie li <annie.li@oracle.com>
Cc: xen-devel@lists.xensource.com
Cc: netdev@vger.kernel.org
Cc: stable@kernel.org # 3.7.x only
---
 drivers/net/xen-netfront.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..b06ef81 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -971,17 +971,12 @@ err:
 		 * overheads. Here, we add the size of the data pulled
 		 * in xennet_fill_frags().
 		 *
-		 * We also adjust for any unused space in the main
-		 * data area by subtracting (RX_COPY_THRESHOLD -
-		 * len). This is especially important with drivers
-		 * which split incoming packets into header and data,
-		 * using only 66 bytes of the main data area (see the
-		 * e1000 driver for example.)  On such systems,
-		 * without this last adjustement, our achievable
-		 * receive throughout using the standard receive
-		 * buffer size was cut by 25%(!!!).
+		 * We also adjust for the __pskb_pull_tail done in
+		 * handle_incoming_queue which pulls data from the
+		 * frags into the head area, which is already
+		 * accounted in RX_COPY_THRESHOLD.
 		 */
-		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
+		skb->truesize += skb->data_len - NETFRONT_SKB_CB(skb)->pull_to;
 		skb->len += skb->data_len;
 
 		if (rx->flags & XEN_NETRXF_csum_blank)
-- 
1.7.2.5

^ permalink raw reply related

* Re: tc ipt action
From: Jan Engelhardt @ 2012-12-18 13:58 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: Pablo Neira Ayuso, Yury Stankevich, shemonc,
	netdev@vger.kernel.org, netfilter-devel
In-Reply-To: <50D06E66.8000600@mojatatu.com>


On Tuesday 2012-12-18 14:23, Jamal Hadi Salim wrote:
> On 12-12-17 08:28 AM, Jan Engelhardt wrote:
>>
>> With act_xt2 as drafted, it instead invokes a chain, which would
>>
>> 1. leave the construction of the target data and calling it
>>    to the subsystems they conceptually belong to - the packet filter
>> 2. lets you do matches, jumps and all that.
>
>I like #2. For #1 as long as it doesnt deviate from desire to have
>one or more instances of targets, we should be fine.

Chains can store multiple targets, so no loss.

>> Good thing you ask. Chain names are unique within a netns, and this
>> act_xtables.c draft looks at the packet to get to know its netns, so
>> that seems fine.
>
> My motivation for that question:
> Is it possible to ignore the hook and tablename and just use the chain
> name?

1. table

First, I think some targets need to relax their restrictions, such as
with xt_DSCP.

Then, only a handful of extensions remain: CT, <all NATs>,
TPROXY and REJECT. Would anyone want to call these from act_ipt?
I doubt it. :)

2. hooks

Extensions with hook limit: <NAT>, TPROXY, REJECT, CLASSIFY.
Again, I don't quite see the value of attempting to NAT from act_ipt.
CLASSIFY {c|sh?}ould be relaxed, unless I am missing something.

^ permalink raw reply

* Re: [PATCH] xen/netfront: improve truesize tracking
From: Konrad Rzeszutek Wilk @ 2012-12-18 14:10 UTC (permalink / raw)
  To: Ian Campbell; +Cc: netdev, Sander Eikelenboom, annie li, xen-devel
In-Reply-To: <1355838711-5473-1-git-send-email-ian.campbell@citrix.com>

On Tue, Dec 18, 2012 at 01:51:51PM +0000, Ian Campbell wrote:
> Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> than that. We have already accounted for this in
> NETFRONT_SKB_CB(skb)->pull_to so use that instead.
> 
> Fixes WARN_ON from skb_try_coalesce.

This should probably be also on the stable tree for 3.7 at least?

> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
  ^^ - Reported-by: 

> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
  ^^ - Acked-by:

> Cc: annie li <annie.li@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: netdev@vger.kernel.org
> Cc: stable@kernel.org # 3.7.x only
> ---
>  drivers/net/xen-netfront.c |   15 +++++----------
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index caa0110..b06ef81 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -971,17 +971,12 @@ err:
>  		 * overheads. Here, we add the size of the data pulled
>  		 * in xennet_fill_frags().
>  		 *
> -		 * We also adjust for any unused space in the main
> -		 * data area by subtracting (RX_COPY_THRESHOLD -
> -		 * len). This is especially important with drivers
> -		 * which split incoming packets into header and data,
> -		 * using only 66 bytes of the main data area (see the
> -		 * e1000 driver for example.)  On such systems,
> -		 * without this last adjustement, our achievable
> -		 * receive throughout using the standard receive
> -		 * buffer size was cut by 25%(!!!).
> +		 * We also adjust for the __pskb_pull_tail done in
> +		 * handle_incoming_queue which pulls data from the
> +		 * frags into the head area, which is already
> +		 * accounted in RX_COPY_THRESHOLD.
>  		 */
> -		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
> +		skb->truesize += skb->data_len - NETFRONT_SKB_CB(skb)->pull_to;
>  		skb->len += skb->data_len;
>  
>  		if (rx->flags & XEN_NETRXF_csum_blank)
> -- 
> 1.7.2.5
> 

^ permalink raw reply

* Re: [PATCH] xen/netfront: improve truesize tracking
From: Ian Campbell @ 2012-12-18 14:15 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: netdev@vger.kernel.org, Sander Eikelenboom, annie li,
	xen-devel@lists.xensource.com
In-Reply-To: <20121218141048.GC4518@phenom.dumpdata.com>

On Tue, 2012-12-18 at 14:10 +0000, Konrad Rzeszutek Wilk wrote:
> On Tue, Dec 18, 2012 at 01:51:51PM +0000, Ian Campbell wrote:
> > Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> > than that. We have already accounted for this in
> > NETFRONT_SKB_CB(skb)->pull_to so use that instead.
> > 
> > Fixes WARN_ON from skb_try_coalesce.
> 
> This should probably be also on the stable tree for 3.7 at least?

Yes, hence "Cc: stable@kernel.org # 3.7.x only" below ;-)

> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Sander Eikelenboom <linux@eikelenboom.it>
>   ^^ - Reported-by: 
> 
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>   ^^ - Acked-by:
> 
> > Cc: annie li <annie.li@oracle.com>
> > Cc: xen-devel@lists.xensource.com
> > Cc: netdev@vger.kernel.org
> > Cc: stable@kernel.org # 3.7.x only
> > ---
> >  drivers/net/xen-netfront.c |   15 +++++----------
> >  1 files changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index caa0110..b06ef81 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -971,17 +971,12 @@ err:
> >  		 * overheads. Here, we add the size of the data pulled
> >  		 * in xennet_fill_frags().
> >  		 *
> > -		 * We also adjust for any unused space in the main
> > -		 * data area by subtracting (RX_COPY_THRESHOLD -
> > -		 * len). This is especially important with drivers
> > -		 * which split incoming packets into header and data,
> > -		 * using only 66 bytes of the main data area (see the
> > -		 * e1000 driver for example.)  On such systems,
> > -		 * without this last adjustement, our achievable
> > -		 * receive throughout using the standard receive
> > -		 * buffer size was cut by 25%(!!!).
> > +		 * We also adjust for the __pskb_pull_tail done in
> > +		 * handle_incoming_queue which pulls data from the
> > +		 * frags into the head area, which is already
> > +		 * accounted in RX_COPY_THRESHOLD.
> >  		 */
> > -		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
> > +		skb->truesize += skb->data_len - NETFRONT_SKB_CB(skb)->pull_to;
> >  		skb->len += skb->data_len;
> >  
> >  		if (rx->flags & XEN_NETRXF_csum_blank)
> > -- 
> > 1.7.2.5
> > 

^ permalink raw reply

* Re: skb_warn_bad_offload with kernel 3.5 (maybe gso/bridge related ?)
From: Yann Dupont @ 2012-12-18 14:13 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev@vger.kernel.org, Ben Hutchings, Herbert Xu
In-Reply-To: <1343983887.9299.817.camel@edumazet-glaptop>

Le 03/08/2012 10:51, Eric Dumazet a écrit :
>
> As the problem seems more or less gso related, I've deactivated gso two
> days ago. This cure the symptom, running ok since.
>
> Anyone here  seeing this problem ?
>
> Cheers,
>
> I dont know, maybe its more a GRO issue ?
>
> When a NIC delivers skbs with ip_summed set to CHECKSUM_UNNECESSARY,
> should resulting GRO packet have ip_summed set to CHECKSUM_PARTIAL ?
>
> CC Ben and Herbert
>
>


Hello. I'm still seeing this issue with 3.7.0
example  :

[335685.629630] ------------[ cut here ]------------
[335685.629661] WARNING: at net/core/dev.c:1941 
skb_warn_bad_offload+0xb6/0xc1()
[335685.629691] Hardware name: PowerEdge M610
[335685.629720] : caps=(0x0000000000005000, 0x0000000000000000) 
len=12808 data_len=11308 gso_size=1448 gso_type=1 ip_summed=1
[335685.629769] Modules linked in: nfnetlink_log nfnetlink ip6table_raw 
iptable_raw openvswitch veth ebtable_nat ebtables dlm sctp configfs nfsd 
auth_rpcgss nfs_acl nfs lockd fscache sunrpc xt_physdev xt_multiport 
ip6table_filter ip6_tables xt_LOG xt_limit xt_tcpudp xt_state 
iptable_filter ip_tables x_tables nf_conntrack_tftp nf_conntrack_ftp 
nf_conntrack_ipv4 nf_defrag_ipv4 8021q bridge stp llc ext2 mbcache 
dm_round_robin nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack 
dm_multipath scsi_dh ipv6 coretemp kvm iTCO_wdt snd_pcm ioatdma lpc_ich 
snd_page_alloc i7core_edac mfd_core snd_timer edac_core crc32c_intel snd 
soundcore pcspkr dca dcdbas microcode joydev evdev processor hed button 
thermal_sys xfs exportfs btrfs zlib_deflate dm_mod sg sd_mod hid_generic 
usbhid hid ata_generic uhci_hcd ata_piix libata ide_pci_generic lpfc 
ide_core mptsas bnx2x scsi_transport_sas mptscsih mdio mptbase ehci_hcd 
scsi_transport_fc scsi_tgt crc32c scsi_mod libcrc32c bnx2
[335685.630305] Pid: 0, comm: swapper/4 Tainted: G        W 
3.7.0-dsiun-121008 #2
[335685.630348] Call Trace:
[335685.630368]  <IRQ>  [<ffffffff813d1400>] ? 
skb_warn_bad_offload+0x74/0xc1
[335685.630403]  [<ffffffff8103e839>] ? warn_slowpath_common+0x79/0xc0
[335685.630430]  [<ffffffff8103e935>] ? warn_slowpath_fmt+0x45/0x50
[335685.630458]  [<ffffffff813d1442>] ? skb_warn_bad_offload+0xb6/0xc1
[335685.630486]  [<ffffffff81321af6>] ? skb_gso_segment+0x206/0x280
[335685.630513]  [<ffffffff81324ada>] ? dev_hard_start_xmit+0x9a/0x4a0
[335685.630542]  [<ffffffffa0087cde>] ? ipv4_confirm+0xae/0x110 
[nf_conntrack_ipv4]
[335685.630590]  [<ffffffffa13ceeb0>] ? br_parse_ip_options+0x220/0x220 
[bridge]
[335685.630620]  [<ffffffff813403dd>] ? sch_direct_xmit+0xfd/0x1d0
[335685.630647]  [<ffffffff8132529e>] ? dev_queue_xmit+0x16e/0x410
[335685.630679]  [<ffffffffa13c8c62>] ? br_dev_queue_push_xmit+0x72/0xc0 
[bridge]
[335685.630723]  [<ffffffffa13cfb33>] ? br_nf_post_routing+0x223/0x340 
[bridge]
[335685.630754]  [<ffffffffa13c8bf0>] ? deliver_clone+0x60/0x60 [bridge]
[335685.630785]  [<ffffffff8134d50d>] ? nf_iterate+0x8d/0xc0
[335685.630813]  [<ffffffffa13cef30>] ? br_nf_dev_queue_xmit+0x80/0x80 
[bridge]
[335685.630843]  [<ffffffffa13c8bf0>] ? deliver_clone+0x60/0x60 [bridge]
[335685.630871]  [<ffffffff8134d5ae>] ? nf_hook_slow+0x6e/0x130
[335685.630898]  [<ffffffffa13c8bf0>] ? deliver_clone+0x60/0x60 [bridge]
[335685.630927]  [<ffffffffa13c8f20>] ? br_multicast_flood+0x170/0x170 
[bridge]
[335685.630958]  [<ffffffffa13c8f62>] ? br_forward_finish+0x42/0x50 [bridge]
[335685.630988]  [<ffffffffa13cefe9>] ? br_nf_forward_finish+0xb9/0x180 
[bridge]
[335685.631018]  [<ffffffffa13cf7d3>] ? br_nf_forward_ip+0x293/0x3d0 
[bridge]
[335685.631051]  [<ffffffffa13c8f20>] ? br_multicast_flood+0x170/0x170 
[bridge]
[335685.631081]  [<ffffffff8134d50d>] ? nf_iterate+0x8d/0xc0
[335685.631111]  [<ffffffffa13c8f20>] ? br_multicast_flood+0x170/0x170 
[bridge]
[335685.631140]  [<ffffffff8134d5ae>] ? nf_hook_slow+0x6e/0x130
[335685.631168]  [<ffffffffa13c8f20>] ? br_multicast_flood+0x170/0x170 
[bridge]
[335685.631198]  [<ffffffffa13c9000>] ? __br_forward+0x90/0xb0 [bridge]
[335685.631227]  [<ffffffffa13c9e44>] ? 
br_handle_frame_finish+0x214/0x2b0 [bridge]
[335685.631272]  [<ffffffffa13cf31f>] ? 
br_nf_pre_routing_finish+0x14f/0x370 [bridge]
[335685.631317]  [<ffffffffa13d01e2>] ? br_nf_pre_routing+0x3a2/0x650 
[bridge]
[335685.631348]  [<ffffffffa13c9c30>] ? br_handle_local_finish+0x50/0x50 
[bridge]
[335685.631391]  [<ffffffff8134d50d>] ? nf_iterate+0x8d/0xc0
[335685.631419]  [<ffffffffa13c9c30>] ? br_handle_local_finish+0x50/0x50 
[bridge]
[335685.631462]  [<ffffffff8134d5ae>] ? nf_hook_slow+0x6e/0x130
[335685.631514]  [<ffffffffa13c9c30>] ? br_handle_local_finish+0x50/0x50 
[bridge]
[335685.631562]  [<ffffffffa13ca0c0>] ? br_handle_frame+0x1e0/0x280 [bridge]
[335685.631591]  [<ffffffff81323135>] ? __netif_receive_skb+0x215/0x860
[335685.631619]  [<ffffffff81125417>] ? alloc_pages_current+0xb7/0x130
[335685.631648]  [<ffffffff8100a3f5>] ? read_tsc+0x5/0x20
[335685.631677]  [<ffffffff8132390a>] ? netif_receive_skb+0x1a/0x80
[335685.631704]  [<ffffffff81323a60>] ? napi_skb_finish+0x50/0x70
[335685.631735]  [<ffffffffa02456e6>] ? bnx2x_rx_int+0x6a6/0x1500 [bnx2x]
[335685.631765]  [<ffffffffa13c9c30>] ? br_handle_local_finish+0x50/0x50 
[bridge]
[335685.631810]  [<ffffffffa13ca0c0>] ? br_handle_frame+0x1e0/0x280 [bridge]
[335685.632982]  [<ffffffffa02465d3>] ? bnx2x_poll+0x93/0x2b0 [bnx2x]
[335685.633010]  [<ffffffff81323135>] ? __netif_receive_skb+0x215/0x860
[335685.633038]  [<ffffffff813242e8>] ? net_rx_action+0x138/0x240
[335685.633065]  [<ffffffff810469ae>] ? __do_softirq+0xbe/0x1f0
[335685.633092]  [<ffffffff813d5cdc>] ? call_softirq+0x1c/0x30
[335685.633118]  [<ffffffff81004cb5>] ? do_softirq+0x75/0xb0
[335685.633144]  [<ffffffff81046c45>] ? irq_exit+0xa5/0xb0
[335685.633170]  [<ffffffff8100492b>] ? do_IRQ+0x5b/0xd0
[335685.633196]  [<ffffffff813d416d>] ? common_interrupt+0x6d/0x6d
[335685.633222]  <EOI>  [<ffffffff8125204c>] ? intel_idle+0xec/0x160
[335685.633257]  [<ffffffff8125202a>] ? intel_idle+0xca/0x160
[335685.633286]  [<ffffffff812f71bd>] ? cpuidle_idle_call+0x9d/0x240
[335685.633315]  [<ffffffff8100c335>] ? cpu_idle+0x65/0xd0
[335685.633340] ---[ end trace 2142bc9cd23c0d87 ]---


Only seeing this with bridge activated, and with bnx2x
ethtool -K eth2 gso cure the problem.

Cheers,

-- 
Yann Dupont - Service IRTS, DSI Université de Nantes
Tel : 02.53.48.49.20 - Mail/Jabber : Yann.Dupont@univ-nantes.fr

^ permalink raw reply

* [PATCH 1/3] usbnet: handle PM failure gracefully
From: Oliver Neukum @ 2012-12-18 14:45 UTC (permalink / raw)
  To: netdev, davem; +Cc: Oliver Neukum, Oliver Neukum

If a device fails to do remote wakeup, this is no reason
to abort an open totally. This patch just continues without
runtime PM.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/net/usb/usbnet.c   |   15 ++++++++-------
 include/linux/usb/usbnet.h |    1 +
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index c04110b..50ed7ab 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -719,7 +719,8 @@ int usbnet_stop (struct net_device *net)
 	dev->flags = 0;
 	del_timer_sync (&dev->delay);
 	tasklet_kill (&dev->bh);
-	if (info->manage_power)
+	if (info->manage_power &&
+	    !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags))
 		info->manage_power(dev, 0);
 	else
 		usb_autopm_put_interface(dev->intf);
@@ -794,14 +795,14 @@ int usbnet_open (struct net_device *net)
 	tasklet_schedule (&dev->bh);
 	if (info->manage_power) {
 		retval = info->manage_power(dev, 1);
-		if (retval < 0)
-			goto done_manage_power_error;
-		usb_autopm_put_interface(dev->intf);
+		if (retval < 0) {
+			retval = 0;
+			set_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
+		} else {
+			usb_autopm_put_interface(dev->intf);
+		}
 	}
 	return retval;
-
-done_manage_power_error:
-	clear_bit(EVENT_DEV_OPEN, &dev->flags);
 done:
 	usb_autopm_put_interface(dev->intf);
 done_nopm:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9bbeabf..288b32a 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -69,6 +69,7 @@ struct usbnet {
 #		define EVENT_DEV_ASLEEP 6
 #		define EVENT_DEV_OPEN	7
 #		define EVENT_DEVICE_REPORT_IDLE	8
+#		define EVENT_NO_RUNTIME_PM	9
 };
 
 static inline struct usb_driver *driver_of(struct usb_interface *intf)
-- 
1.7.7

^ permalink raw reply related

* [PATCH 2/3] usbnet: generic manage_power()
From: Oliver Neukum @ 2012-12-18 14:45 UTC (permalink / raw)
  To: netdev, davem; +Cc: Oliver Neukum, Oliver Neukum

Centralise common code for manage_power() in usbnet
by making a generic simple implementation

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/net/usb/usbnet.c   |   10 ++++++++++
 include/linux/usb/usbnet.h |    2 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 50ed7ab..3d4bf01 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1616,6 +1616,16 @@ void usbnet_device_suggests_idle(struct usbnet *dev)
 }
 EXPORT_SYMBOL(usbnet_device_suggests_idle);
 
+/*
+ * For devices that can do without special commands
+ */
+int usbnet_manage_power(struct usbnet *dev, int on)
+{
+	dev->intf->needs_remote_wakeup = on;
+	return 0;
+}
+EXPORT_SYMBOL(usbnet_manage_power);
+
 /*-------------------------------------------------------------------------*/
 static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 			     u16 value, u16 index, void *data, u16 size)
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 288b32a..bd45eb7 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -241,4 +241,6 @@ extern void usbnet_set_msglevel(struct net_device *, u32);
 extern void usbnet_get_drvinfo(struct net_device *, struct ethtool_drvinfo *);
 extern int usbnet_nway_reset(struct net_device *net);
 
+extern int usbnet_manage_power(struct usbnet *, int);
+
 #endif /* __LINUX_USB_USBNET_H */
-- 
1.7.7

^ permalink raw reply related

* [PATCH 3/3] use generic usbnet_manage_power()
From: Oliver Neukum @ 2012-12-18 14:46 UTC (permalink / raw)
  To: netdev, davem; +Cc: Oliver Neukum, Oliver Neukum

This covers the drivers that can use a primitive
implementation.

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/net/usb/cdc_ether.c |   10 ++--------
 drivers/net/usb/cdc_ncm.c   |   10 ++--------
 2 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index d012982..421b7b8 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -457,12 +457,6 @@ int usbnet_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 }
 EXPORT_SYMBOL_GPL(usbnet_cdc_bind);
 
-static int cdc_manage_power(struct usbnet *dev, int on)
-{
-	dev->intf->needs_remote_wakeup = on;
-	return 0;
-}
-
 static const struct driver_info	cdc_info = {
 	.description =	"CDC Ethernet Device",
 	.flags =	FLAG_ETHER | FLAG_POINTTOPOINT,
@@ -470,7 +464,7 @@ static const struct driver_info	cdc_info = {
 	.bind =		usbnet_cdc_bind,
 	.unbind =	usbnet_cdc_unbind,
 	.status =	usbnet_cdc_status,
-	.manage_power =	cdc_manage_power,
+	.manage_power =	usbnet_manage_power,
 };
 
 static const struct driver_info wwan_info = {
@@ -479,7 +473,7 @@ static const struct driver_info wwan_info = {
 	.bind =		usbnet_cdc_bind,
 	.unbind =	usbnet_cdc_unbind,
 	.status =	usbnet_cdc_status,
-	.manage_power =	cdc_manage_power,
+	.manage_power =	usbnet_manage_power,
 };
 
 /*-------------------------------------------------------------------------*/
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d38bc20..71b6e92 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1129,19 +1129,13 @@ static void cdc_ncm_disconnect(struct usb_interface *intf)
 	usbnet_disconnect(intf);
 }
 
-static int cdc_ncm_manage_power(struct usbnet *dev, int status)
-{
-	dev->intf->needs_remote_wakeup = status;
-	return 0;
-}
-
 static const struct driver_info cdc_ncm_info = {
 	.description = "CDC NCM",
 	.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
 	.bind = cdc_ncm_bind,
 	.unbind = cdc_ncm_unbind,
 	.check_connect = cdc_ncm_check_connect,
-	.manage_power = cdc_ncm_manage_power,
+	.manage_power = usbnet_manage_power,
 	.status = cdc_ncm_status,
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
@@ -1155,7 +1149,7 @@ static const struct driver_info wwan_info = {
 	.bind = cdc_ncm_bind,
 	.unbind = cdc_ncm_unbind,
 	.check_connect = cdc_ncm_check_connect,
-	.manage_power = cdc_ncm_manage_power,
+	.manage_power = usbnet_manage_power,
 	.status = cdc_ncm_status,
 	.rx_fixup = cdc_ncm_rx_fixup,
 	.tx_fixup = cdc_ncm_tx_fixup,
-- 
1.7.7

^ permalink raw reply related

* TCP delayed ACK heuristic
From: Cong Wang @ 2012-12-18 15:11 UTC (permalink / raw)
  To: netdev
  Cc: Ben Greear, David Miller, Eric Dumazet, Stephen Hemminger,
	Rick Jones, Thomas Graf
In-Reply-To: <270756364.27707018.1355842632348.JavaMail.root@redhat.com>

Hello, TCP experts!

Some time ago, Ben sent a patch [1] to add some knobs for
tuning TCP delayed ACK, but rejected by David.

David's point is that we can do some heuristics for TCP
delayed ACK, so the question is that what kind of heuristics
can we use?

RFC1122 explicitly mentions:

            A TCP SHOULD implement a delayed ACK, but an ACK should not
            be excessively delayed; in particular, the delay MUST be
            less than 0.5 seconds, and in a stream of full-sized
            segments there SHOULD be an ACK for at least every second
            segment.

so this prevents us from using any heuristic for the number
of coalesced delayed ACK.

For the timeout of a delayed ACK, my idea is guessing how many
packet we suppose to receive is the TCP stream is fully utilized,
something like below:

+static inline u32 tcp_expect_packets(struct sock *sk)
+{
+       struct tcp_sock *tp = tcp_sk(sk);
+       int rtt = tp->srtt >> 3;
+       u32 idle = tcp_time_stamp - inet_csk(sk)->icsk_ack.lrcvtime;
+
+       return idle * 2 / rtt;
+}
...
+       ato -= tcp_expect_packets(sk) * delta;


The more we expect, the less we should delay. However this is
not accurate due to congestion control.

Meanwhile, we can also check how many packets are pending in TCP
sending queue, the more we pend, the more we can piggyback with
a single ACK, but not beyond how much we are able to send at
that time.

Comments? Ideas?

Thanks.

1. http://thread.gmane.org/gmane.linux.network/233859

^ permalink raw reply

* Re: [PATCH] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2012-12-18 15:12 UTC (permalink / raw)
  To: Ian Campbell
  Cc: netdev, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
	xen-devel
In-Reply-To: <1355838711-5473-1-git-send-email-ian.campbell@citrix.com>

On Tue, 2012-12-18 at 13:51 +0000, Ian Campbell wrote:
> Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> than that. We have already accounted for this in
> NETFRONT_SKB_CB(skb)->pull_to so use that instead.
> 
> Fixes WARN_ON from skb_try_coalesce.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: annie li <annie.li@oracle.com>
> Cc: xen-devel@lists.xensource.com
> Cc: netdev@vger.kernel.org
> Cc: stable@kernel.org # 3.7.x only
> ---
>  drivers/net/xen-netfront.c |   15 +++++----------
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index caa0110..b06ef81 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -971,17 +971,12 @@ err:
>  		 * overheads. Here, we add the size of the data pulled
>  		 * in xennet_fill_frags().
>  		 *
> -		 * We also adjust for any unused space in the main
> -		 * data area by subtracting (RX_COPY_THRESHOLD -
> -		 * len). This is especially important with drivers
> -		 * which split incoming packets into header and data,
> -		 * using only 66 bytes of the main data area (see the
> -		 * e1000 driver for example.)  On such systems,
> -		 * without this last adjustement, our achievable
> -		 * receive throughout using the standard receive
> -		 * buffer size was cut by 25%(!!!).
> +		 * We also adjust for the __pskb_pull_tail done in
> +		 * handle_incoming_queue which pulls data from the
> +		 * frags into the head area, which is already
> +		 * accounted in RX_COPY_THRESHOLD.
>  		 */
> -		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
> +		skb->truesize += skb->data_len - NETFRONT_SKB_CB(skb)->pull_to;
>  		skb->len += skb->data_len;
>  
>  		if (rx->flags & XEN_NETRXF_csum_blank)


But skb truesize is not what you think.

You must account the exact memory used by this skb, not only the used
part of it.

At the very minimum, it should be

skb->truesize += skb->data_len;

But it really should be the allocated size of the fragment.

If its a page, then its a page, even if you use one single byte in it.

^ permalink raw reply

* network namespace and DNS lookups
From: Ravi Aysola @ 2012-12-18 15:09 UTC (permalink / raw)
  To: netdev

Has there been any work in any of the recent kernels to limit the DNS lookup
to a particular network namespace?  Do we have any facility to specify the
DNS resolvers on network namespace basis (such as /etc/ns/resolv.conf)?

thank you
ravi/

^ permalink raw reply

* [PATCH v2 1/2] net: asix: init ASIX AX88772B MAC from EEPROM
From: Lucas Stach @ 2012-12-18 15:21 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Oliver Neukum, linux-usb-u79uwXL29TY76Z2rM5mHXA

The device comes up with a MAC address of all zeros. We need to read the
initial device MAC from EEPROM so it can be set properly later.

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
A similar fix was added into U-Boot:
http://patchwork.ozlabs.org/patch/179409/

v2: pass flag in the data field instead of clobbering the global flags
    space
---
 drivers/net/usb/asix.h         |  3 +++
 drivers/net/usb/asix_devices.c | 30 +++++++++++++++++++++++++++---
 2 Dateien geändert, 30 Zeilen hinzugefügt(+), 3 Zeilen entfernt(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index e889631..7afe8ac 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -167,6 +167,9 @@ struct asix_data {
 	u8 res;
 };
 
+/* ASIX specific flags */
+#define FLAG_EEPROM_MAC		(1UL << 0)  /* init device MAC from eeprom */
+
 int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 		  u16 size, void *data);
 
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 7a6e758..0ecc3bc 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -422,14 +422,25 @@ static const struct net_device_ops ax88772_netdev_ops = {
 
 static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 {
-	int ret, embd_phy;
+	int ret, embd_phy, i;
 	u8 buf[ETH_ALEN];
 	u32 phyid;
 
 	usbnet_get_endpoints(dev,intf);
 
 	/* Get the MAC address */
-	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
+	if (dev->driver_info->data & FLAG_EEPROM_MAC) {
+		for (i = 0; i < (ETH_ALEN >> 1); i++) {
+			ret = asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x04 + i,
+					0, 2, buf + i * 2);
+			if (ret < 0)
+				break;
+		}
+	} else {
+		ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID,
+				0, 0, ETH_ALEN, buf);
+	}
+
 	if (ret < 0) {
 		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
@@ -872,6 +883,19 @@ static const struct driver_info ax88772_info = {
 	.tx_fixup = asix_tx_fixup,
 };
 
+static const struct driver_info ax88772b_info = {
+	.description = "ASIX AX88772B USB 2.0 Ethernet",
+	.bind = ax88772_bind,
+	.status = asix_status,
+	.link_reset = ax88772_link_reset,
+	.reset = ax88772_reset,
+	.flags = FLAG_ETHER | FLAG_FRAMING_AX | FLAG_LINK_INTR |
+	         FLAG_MULTI_PACKET,
+	.rx_fixup = asix_rx_fixup,
+	.tx_fixup = asix_tx_fixup,
+	.data = FLAG_EEPROM_MAC,
+};
+
 static const struct driver_info ax88178_info = {
 	.description = "ASIX AX88178 USB 2.0 Ethernet",
 	.bind = ax88178_bind,
@@ -953,7 +977,7 @@ static const struct usb_device_id	products [] = {
 }, {
 	// ASIX AX88772B 10/100
 	USB_DEVICE (0x0b95, 0x772b),
-	.driver_info = (unsigned long) &ax88772_info,
+	.driver_info = (unsigned long) &ax88772b_info,
 }, {
 	// ASIX AX88772 10/100
 	USB_DEVICE (0x0b95, 0x7720),
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 2/2] net: asix: handle packets crossing URB boundaries
From: Lucas Stach @ 2012-12-18 15:21 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Oliver Neukum, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1355844083-14285-1-git-send-email-dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>

ASIX AX88772B started to pack data even more tightly. Packets and the ASIX packet
header may now cross URB boundaries. To handle this we have to introduce
some state between individual calls to asix_rx_fixup().

Signed-off-by: Lucas Stach <dev-8ppwABl0HbeELgA04lAiVw@public.gmane.org>
---
I've running this patch for some weeks already now and it gets rid of all
the commonly seen rx failures with AX88772B.

v2: don't forget to free driver_private
---
 drivers/net/usb/asix.h         | 11 ++++++
 drivers/net/usb/asix_common.c  | 81 +++++++++++++++++++++++++++++-------------
 drivers/net/usb/asix_devices.c | 17 +++++++++
 3 Dateien geändert, 85 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)

diff --git a/drivers/net/usb/asix.h b/drivers/net/usb/asix.h
index 7afe8ac..4dfdbf6 100644
--- a/drivers/net/usb/asix.h
+++ b/drivers/net/usb/asix.h
@@ -167,6 +167,17 @@ struct asix_data {
 	u8 res;
 };
 
+struct asix_rx_fixup_info {
+	struct sk_buff *ax_skb;
+	u32 header;
+	u16 size;
+	bool split_head;
+};
+
+struct asix_private {
+	struct asix_rx_fixup_info rx_fixup_info;
+};
+
 /* ASIX specific flags */
 #define FLAG_EEPROM_MAC		(1UL << 0)  /* init device MAC from eeprom */
 
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 50d1673..17f9801 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -53,44 +53,77 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
 
 int asix_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 {
+	struct asix_private *dp = dev->driver_priv;
+	struct asix_rx_fixup_info *rx = &dp->rx_fixup_info;
 	int offset = 0;
 
-	while (offset + sizeof(u32) < skb->len) {
-		struct sk_buff *ax_skb;
-		u16 size;
-		u32 header = get_unaligned_le32(skb->data + offset);
-
-		offset += sizeof(u32);
-
-		/* get the packet length */
-		size = (u16) (header & 0x7ff);
-		if (size != ((~header >> 16) & 0x07ff)) {
-			netdev_err(dev->net, "asix_rx_fixup() Bad Header Length\n");
-			return 0;
+	while (offset + sizeof(u16) <= skb->len) {
+		u16 remaining = 0;
+		unsigned char *data;
+
+		if (!rx->size) {
+			if ((skb->len - offset == sizeof(u16)) ||
+			    rx->split_head) {
+				if(!rx->split_head) {
+					rx->header = get_unaligned_le16(
+							skb->data + offset);
+					rx->split_head = true;
+					offset += sizeof(u16);
+					break;
+				} else {
+					rx->header |= (get_unaligned_le16(
+							skb->data + offset)
+							<< 16);
+					rx->split_head = false;
+					offset += sizeof(u16);
+				}
+			} else {
+				rx->header = get_unaligned_le32(skb->data +
+								offset);
+				offset += sizeof(u32);
+			}
+
+			/* get the packet length */
+			rx->size = (u16) (rx->header & 0x7ff);
+			if (rx->size != ((~rx->header >> 16) & 0x7ff)) {
+				netdev_err(dev->net, "asix_rx_fixup() Bad Header Length 0x%x, offset %d\n",
+					   rx->header, offset);
+				rx->size = 0;
+				return 0;
+			}
+			rx->ax_skb = netdev_alloc_skb_ip_align(dev->net,
+							       rx->size);
+			if (!rx->ax_skb)
+				return 0;
 		}
 
-		if ((size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) ||
-		    (size + offset > skb->len)) {
+		if (rx->size > dev->net->mtu + ETH_HLEN + VLAN_HLEN) {
 			netdev_err(dev->net, "asix_rx_fixup() Bad RX Length %d\n",
-				   size);
+				   rx->size);
+			kfree_skb(rx->ax_skb);
 			return 0;
 		}
-		ax_skb = netdev_alloc_skb_ip_align(dev->net, size);
-		if (!ax_skb)
-			return 0;
 
-		skb_put(ax_skb, size);
-		memcpy(ax_skb->data, skb->data + offset, size);
-		usbnet_skb_return(dev, ax_skb);
+		if (rx->size > skb->len - offset) {
+			remaining = rx->size - (skb->len - offset);
+			rx->size = skb->len - offset;
+		}
+
+		data = skb_put(rx->ax_skb, rx->size);
+		memcpy(data, skb->data + offset, rx->size);
+		if (!remaining)
+			usbnet_skb_return(dev, rx->ax_skb);
 
-		offset += (size + 1) & 0xfffe;
+		offset += (rx->size + 1) & 0xfffe;
+		rx->size = remaining;
 	}
 
 	if (skb->len != offset) {
-		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d\n",
-			   skb->len);
+		netdev_err(dev->net, "asix_rx_fixup() Bad SKB Length %d, %d\n",
+			   skb->len, offset);
 		return 0;
 	}
+
 	return 1;
 }
 
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index 0ecc3bc..c651243 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -495,9 +495,19 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+		return -ENOMEM;
+
 	return 0;
 }
 
+void ax88772_unbind(struct usbnet *dev, struct usb_interface *intf)
+{
+	if (dev->driver_priv)
+		kfree(dev->driver_priv);
+}
+
 static const struct ethtool_ops ax88178_ethtool_ops = {
 	.get_drvinfo		= asix_get_drvinfo,
 	.get_link		= asix_get_link,
@@ -829,6 +839,10 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->rx_urb_size = 2048;
 	}
 
+	dev->driver_priv = kzalloc(sizeof(struct asix_private), GFP_KERNEL);
+	if (!dev->driver_priv)
+			return -ENOMEM;
+
 	return 0;
 }
 
@@ -875,6 +889,7 @@ static const struct driver_info hawking_uf200_info = {
 static const struct driver_info ax88772_info = {
 	.description = "ASIX AX88772 USB 2.0 Ethernet",
 	.bind = ax88772_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
@@ -886,6 +901,7 @@ static const struct driver_info ax88772_info = {
 static const struct driver_info ax88772b_info = {
 	.description = "ASIX AX88772B USB 2.0 Ethernet",
 	.bind = ax88772_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88772_link_reset,
 	.reset = ax88772_reset,
@@ -899,6 +915,7 @@ static const struct driver_info ax88772b_info = {
 static const struct driver_info ax88178_info = {
 	.description = "ASIX AX88178 USB 2.0 Ethernet",
 	.bind = ax88178_bind,
+	.unbind = ax88772_unbind,
 	.status = asix_status,
 	.link_reset = ax88178_link_reset,
 	.reset = ax88178_reset,
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] xen/netfront: improve truesize tracking
From: Ian Campbell @ 2012-12-18 15:26 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, Sander Eikelenboom, Konrad Rzeszutek Wilk,
	annie li, xen-devel@lists.xensource.com
In-Reply-To: <1355843525.9380.18.camel@edumazet-glaptop>

On Tue, 2012-12-18 at 15:12 +0000, Eric Dumazet wrote:
> On Tue, 2012-12-18 at 13:51 +0000, Ian Campbell wrote:
> > Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> > than that. We have already accounted for this in
> > NETFRONT_SKB_CB(skb)->pull_to so use that instead.
> > 
> > Fixes WARN_ON from skb_try_coalesce.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Sander Eikelenboom <linux@eikelenboom.it>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: annie li <annie.li@oracle.com>
> > Cc: xen-devel@lists.xensource.com
> > Cc: netdev@vger.kernel.org
> > Cc: stable@kernel.org # 3.7.x only
> > ---
> >  drivers/net/xen-netfront.c |   15 +++++----------
> >  1 files changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index caa0110..b06ef81 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -971,17 +971,12 @@ err:
> >  		 * overheads. Here, we add the size of the data pulled
> >  		 * in xennet_fill_frags().
> >  		 *
> > -		 * We also adjust for any unused space in the main
> > -		 * data area by subtracting (RX_COPY_THRESHOLD -
> > -		 * len). This is especially important with drivers
> > -		 * which split incoming packets into header and data,
> > -		 * using only 66 bytes of the main data area (see the
> > -		 * e1000 driver for example.)  On such systems,
> > -		 * without this last adjustement, our achievable
> > -		 * receive throughout using the standard receive
> > -		 * buffer size was cut by 25%(!!!).
> > +		 * We also adjust for the __pskb_pull_tail done in
> > +		 * handle_incoming_queue which pulls data from the
> > +		 * frags into the head area, which is already
> > +		 * accounted in RX_COPY_THRESHOLD.
> >  		 */
> > -		skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
> > +		skb->truesize += skb->data_len - NETFRONT_SKB_CB(skb)->pull_to;
> >  		skb->len += skb->data_len;
> >  
> >  		if (rx->flags & XEN_NETRXF_csum_blank)
> 
> 
> But skb truesize is not what you think.

Indeed, it seems I was completely backwards about what it means!

> You must account the exact memory used by this skb, not only the used
> part of it.
> 
> At the very minimum, it should be
> 
> skb->truesize += skb->data_len;
> 
> But it really should be the allocated size of the fragment.
> 
> If its a page, then its a page, even if you use one single byte in it.

So actually we want += PAGE_SIZE * skb_shinfo(skb)->nr_frags ?

Sander, can you try that change?

Ian.

^ permalink raw reply

* inaccurate packet scheduling
From: Jiri Pirko @ 2012-12-18 15:04 UTC (permalink / raw)
  To: netdev; +Cc: jhs, davem, edumazet, tgraf, shemminger

Hi all.

Run one of the following 2 scripts on machine A:

#!/bin/bash
tc qdisc del dev eth0 root
sleep 1
tc -batch << EOF
qdisc add dev eth0 root handle 1: prio bands 2 priomap 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
qdisc add dev eth0 parent 1:1 handle 10: pfifo limit 50
qdisc add dev eth0 parent 1:2 handle 20 tbf latency 100ms rate 4mbit burst 2m
filter add dev eth0 parent 1: protocol ip u32 match ip dst $machineB_ip flowid 1:2
EOF

#!/bin/bash
tc qdisc del dev eth0 root
sleep 1
tc -batch << EOF
qdisc add dev eth0 root handle 1: prio bands 2 priomap 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
qdisc add dev eth0 parent 1:1 handle 10: pfifo limit 20
qdisc add dev eth0 parent 1:2 handle 20: pfifo limit 20
filter add dev eth0 parent 1: protocol ip pref 10 \
u32 match ip dst $machineB_ip \
flowid 1:2 \
police rate 4Mbit burst 2m conform-exceed drop
EOF

And run:
[machineB ~]# iperf -s
[machineA ~]# iperf -c machineB_ip -t 60

Expected results are: ~3.8-4.2 Mbits/s
But actual results are: ~130-170 Kbits/s with tbf, ~70-300 Kbits/s with policy rate

[machineA ~]# tc -s qdisc list dev eth0
qdisc prio 1: root refcnt 9 bands 2 priomap  0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 Sent 1512384 bytes 1032 pkt (dropped 729, overlimits 0 requeues 0) 
 backlog 0b 0p requeues 0 
qdisc pfifo 10: parent 1:1 limit 50p
 Sent 4560 bytes 32 pkt (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 0p requeues 0 
qdisc tbf 20: parent 1:2 rate 4000Kbit burst 2Mb lat 100.0ms 
 Sent 1507824 bytes 1000 pkt (dropped 729, overlimits 0 requeues 0) 
 backlog 0b 0p requeues 0 


Tested with kernel pulled from linus's git today. This happens with older
kernels as well (I tried 2.6.32-based rhel6 kernels).

This happens to me on following machines:
HP DL360G8 (x86_64) http://people.redhat.com/jpirko/aThoo2Ei/dl380g8/
HP DL360G3 (i686) 
IBM JS22 (ppc64) http://people.redhat.com/jpirko/aThoo2Ei/ibmjs22/

On following machines, I do not observe this issue:
qemu kvm (x86_64)
IBM Zseries (s390x) http://people.redhat.com/jpirko/aThoo2Ei/ibmz/

Please ask in case you need me to provide any other details.

Thanks.

Jiri

^ permalink raw reply

* Re: network namespace and DNS lookups
From: Ravi Aysola @ 2012-12-18 15:49 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CAFaHj6GVgDf2QXUWyaKYnvOqLJRjG+kA32eEk5psPkmc-RPY1Q@mail.gmail.com>

I think I sent my earlier email a bit prematurely.  I do have
/etc/netns/<namespace-name>/resolv.conf
files under each of my namespaces. Now the question is, how does a
user space process
(say bind)  look at a namespace specific resolv.conf instead of
default one?  Have any of
these standard applications been modified to work with namespace
specific config files?

thanks again
ravi/

On Tue, Dec 18, 2012 at 10:09 AM, Ravi Aysola <ravi.mlists@gmail.com> wrote:
> Has there been any work in any of the recent kernels to limit the DNS lookup
> to a particular network namespace?  Do we have any facility to specify the
> DNS resolvers on network namespace basis (such as /etc/ns/resolv.conf)?
>
> thank you
> ravi/

^ 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