Netdev List
 help / color / mirror / Atom feed
* Re: [patch] net/ethernet: ks8851_mll unregister_netdev() before freeing
From: Raffaele Recalcati @ 2012-06-06 17:03 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: David S. Miller, netdev, kernel-janitors
In-Reply-To: <20120606063129.GA26829@elgon.mountain>

On 09:31 Wed 06 Jun     , Dan Carpenter wrote:
> We added another error condition here, but if we were to hit it then
> we need to unregister_netdev() before doing the free_netdev().
> Otherwise we would hit the BUG_ON() in free_netdev():
> 
> 	BUG_ON(dev->reg_state != NETREG_UNREGISTERED);
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Static checker stuff.  I don't have this hardware.
> 
> diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
> index 70bd329..875dd5e 100644
> --- a/drivers/net/ethernet/micrel/ks8851_mll.c
> +++ b/drivers/net/ethernet/micrel/ks8851_mll.c
> @@ -1606,7 +1606,7 @@ static int __devinit ks8851_probe(struct platform_device *pdev)
>  	if (!pdata) {
>  		netdev_err(netdev, "No platform data\n");
>  		err = -ENODEV;
> -		goto err_register;
> +		goto err_pdata;
>  	}
>  	memcpy(ks->mac_addr, pdata->mac_addr, 6);
>  	if (!is_valid_ether_addr(ks->mac_addr)) {
> @@ -1626,6 +1626,8 @@ static int __devinit ks8851_probe(struct platform_device *pdev)
>  		    (id >> 8) & 0xff, (id >> 4) & 0xf, (id >> 1) & 0x7);
>  	return 0;
>  
> +err_pdata:
> +	unregister_netdev(netdev);
>  err_register:
>  err_get_irq:
>  	iounmap(ks->hw_addr_cmd);

Tested-by: Raffaele Recalcati <raffaele.recalcati@bticino.it>

[    2.274993] ks8851_mll ks8851_mll: (unregistered net_device): message enable is 0
[    2.282897] ks8851_mll ks8851_mll: (unregistered net_device): the selftest passes
[    2.302246] ks8851_mll ks8851_mll: eth0: No platform data

^ permalink raw reply

* Re: Strange latency spikes/TX network stalls on Sun Fire X4150(x86) and e1000e
From: Hiroaki SHIMODA @ 2012-06-06 17:19 UTC (permalink / raw)
  To: Jesse Brandeburg
  Cc: Eric Dumazet, Tom Herbert, Denys Fedoryshchenko, netdev,
	e1000-devel, jeffrey.t.kirsher, davem
In-Reply-To: <20120606092635.00003b61@unknown>

On Wed, 6 Jun 2012 09:26:35 -0700
Jesse Brandeburg <jesse.brandeburg@intel.com> wrote:

> On Wed, 6 Jun 2012 Hiroaki SHIMODA <shimoda.hiroaki@gmail.com> wrote:
> > Sorry for long delay. I'll post.
> > (I have no idea how to fix this problem as keeping TXDCTL.WTHRESH to 5)
> 
> I don't like changing WTHRESH wholesale because making the global change
> to WTHRESH on e1000e just to fix this one bug (likely specific to a
> particular chip/hardware) will adversely effect performance on many
> models supported by e1000e not demonstrating any problem.  We could
> possibly check something in for just ESB2LAN (S5000 chipset).
> 
> Other people (tom herbert) with this same chipset have been unable to
> reproduce this issue right?

I understand your performance concern.

The affected chip would be e1000_82571, e1000_82572, e1000_82574
and e1000_80003es2lan which have FLAG2_DMA_BURST bit in
adapter->flags2.
Anyway, I have no objection intel guys NACK to my patch and
provide right fix. But in that case please consider 82574L chip too
which I observed similar behaviour.

Thanks.

^ permalink raw reply

* RE: [PATCH 1/2] Ethtool: Add EEE support
From: Ben Hutchings @ 2012-06-06 17:21 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: netdev@vger.kernel.org, Eilon Greenstein, peppe.cavallaro@st.com
In-Reply-To: <979A8436335E3744ADCD3A9F2A2B68A502687E@SJEXCHMB10.corp.ad.broadcom.com>

On Wed, 2012-06-06 at 16:27 +0000, Yuval Mintz wrote:
> Hi Ben,
> 
> I stand corrected; I'll supply a new version shortly. 
> 
> I've got 2 questions  though:
> 
> > For now, use int for booleans.  At some point I would like to see a
> > thorough cleanup of ethtool to use bool where appropriate - but that's
> > independent of this
> 
> I've noticed that the 'do_generic_set' function assumes all fields are ints.

Yes, but that can and should be fixed.

> Is this a convention we should stick to (using  __u32 in the ethtool structs)?

Yes.  It is generally preferable to use explicitly fixed-size types in
kernel-userland ABI, even though I wouldn't expect the size of int to
change for many decades to come.

> I'm asking because I'm "wasting" fields in the ethtool_eee struct as I use
> __u32 for boolean fields, simply because what seems to be the conventional
> method won't work with smaller fields (corrupts the following fields).

It's slightly wasteful but not worth worrying about.

> The seconds question - is there a dependency between your acceptance of
> this patch series and Dave's acceptance of the kernel's ethtool modification?
> I'm asking because changes in the ethtool header there should be applied in
> this patch series as well (in ethtool-copy.h).

There is a dependency, yes.  However I generally update ethtool-copy.h
as a separate commit myself (as you can see in the git commit log for
it) so I won't demand that you refresh the version in your patch.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: Strange latency spikes/TX network stalls on Sun Fire X4150(x86) and e1000e
From: David Miller @ 2012-06-06 17:26 UTC (permalink / raw)
  To: eric.dumazet
  Cc: jesse.brandeburg, shimoda.hiroaki, therbert, denys, netdev,
	e1000-devel, jeffrey.t.kirsher
In-Reply-To: <1339002304.26966.16.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 06 Jun 2012 19:05:04 +0200

> You cant hold a TX completion indefinitely, this breaks BQL but also
> other stuff.

True.

^ permalink raw reply

* sky2: BUG_ON(sky2->hw->flags & SKY2_HW_NEW_LE) in sky2_rx_checksum triggers
From: Kirill Smelkov @ 2012-06-06 17:20 UTC (permalink / raw)
  To: Mirko Lindner, Stephen Hemminger; +Cc: netdev

Hello up there,

I was playing with `ethtool -K rx on|off` on today's net-next (cbfc6071)
and got BUG_ON in sky2_rx_checksum. Steps to reproduce:

# (cable connected to net)
$ ethtool -K eth0 rx off
$ ethtool -K eth0 rx on

then it bugs:


------------[ cut here ]------------
kernel BUG at drivers/net/ethernet/marvell/sky2.c:2684!
invalid opcode: 0000 [#1] PREEMPT SMP
Modules linked in: sky2

Pid: 0, comm: swapper/1 Not tainted 3.5.0-rc1-netmini+ #75 Hewlett-Packard HP Mini 5103/1608
EIP: 0060:[<efe7a18f>] EFLAGS: 00010202 CPU: 1
EIP is at sky2_poll+0x725/0x8cc [sky2]
EAX: ed52e540 EBX: edfd6800 ECX: 00040000 EDX: 00041004
ESI: 631e631e EDI: 000000e6 EBP: 00000400 ESP: edc7ff1c
 DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
CR0: 8005003b CR2: a76110c4 CR3: 01545000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process swapper/1 (pid: 0, ti=edc7e000 task=edc614a0 task.ti=edc74000)
Stack:
 edc02400 ed51c080 edc7ff7c b1340f90 8020001f ed51c700 8020001e edc7ff6c
 ed52e540 ed51de00 001e006c ed6cbd40 00000000 00000046 edfd6bc0 00000000
 ed52e548 000007ad 00000040 00000040 00000282 ed51c700 edc02400 b14b8200
Call Trace:
 [<b1340f90>] ? __slab_free.isra.49+0xac/0x157
 [<b10256b7>] ? rcu_read_unlock_sched_notrace+0x19/0x19
 [<b12e3add>] ? net_rx_action+0x8f/0x182
 [<b10256b7>] ? rcu_read_unlock_sched_notrace+0x19/0x19
 [<b1025747>] ? __do_softirq+0x90/0x165
 [<b10256b7>] ? rcu_read_unlock_sched_notrace+0x19/0x19
 <IRQ>
 [<b10259c3>] ? irq_exit+0x32/0x8a
 [<b10034f8>] ? do_IRQ+0x74/0x89
 [<b104d1e7>] ? tick_program_event+0x1f/0x22
 [<b1349ce9>] ? common_interrupt+0x29/0x30
 [<b102007b>] ? do_oops_enter_exit.part.3+0x8d/0xac
 [<b1184327>] ? intel_idle+0xc6/0xe9
 [<b12a8e7a>] ? cpuidle_enter+0xb/0xe
 [<b12a92d0>] ? cpuidle_idle_call+0xb5/0x167
 [<b10075fc>] ? cpu_idle+0x43/0x7b
Code: ff e9 19 01 00 00 0f b7 d5 8b 44 24 38 e8 f4 b7 ff ff f6 83 8f 00 00 00
20 0f 84 00 01 00 00 8b 83 c0 03 00 00 f6 40 44 20 74 02 <0f> 0b 89 f2 c1 ea 10
66 39 f2 75 29 0f b7 83 5c 04 00 00 c1 e0
EIP: [<efe7a18f>] sky2_poll+0x725/0x8cc [sky2] SS:ESP 0068:edc7ff1c
Kernel panic - not syncing: Fatal exception in interrupt
panic occured, switching back to text console

---- 8< ----
(not sure I've ocr'ed it correctly. If in doubt, there is this bug
 screenshot here:
 https://github.com/navytux/navytux.github.com/raw/master/~kirr/misc/sky2_rx_checksum_BUGON_hwflags.jpg)


My hardware is

    02:00.0 0200: 11ab:4381 (rev 11)

i.e.

    02:00.0 Ethernet controller: Marvell Technology Group Ltd. Yukon
        Optima 88E8059 [PCIe Gigabit Ethernet Controller with AVB] (rev 11)


and dmesg when sky2 driver loads:

[    8.090421] sky2: driver version 1.30
[    8.094079] sky2 0000:02:00.0: Yukon-2 Optima chip revision 1
[    8.098148] sky2 0000:02:00.0: irq 43 for MSI/MSI-X
[    8.099524] sky2 0000:02:00.0: eth0: addr 1c:c1:de:ae:73:8a
[   18.134971] sky2 0000:02:00.0: eth0: enabling interface
[   75.544140] sky2 0000:02:00.0: eth0: Link is up at 1000 Mbps, full duplex, flow control rx


Thanks beforehand,
Kirill

^ permalink raw reply

* [PATCH] sparc bpf_jit: support BPF_S_ANC_ALU_XOR_X instruction
From: David Miller @ 2012-06-06 17:31 UTC (permalink / raw)
  To: netdev; +Cc: sparclinux


Signed-off-by: David S. Miller <davem@davemloft.net>
---

Committed to net-next

 arch/sparc/net/bpf_jit_comp.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/sparc/net/bpf_jit_comp.c b/arch/sparc/net/bpf_jit_comp.c
index 1a69244..e9073e9 100644
--- a/arch/sparc/net/bpf_jit_comp.c
+++ b/arch/sparc/net/bpf_jit_comp.c
@@ -96,6 +96,7 @@ static void bpf_flush_icache(void *start_, void *end_)
 #define AND		F3(2, 0x01)
 #define ANDCC		F3(2, 0x11)
 #define OR		F3(2, 0x02)
+#define XOR		F3(2, 0x03)
 #define SUB		F3(2, 0x04)
 #define SUBCC		F3(2, 0x14)
 #define MUL		F3(2, 0x0a)	/* umul */
@@ -462,6 +463,9 @@ void bpf_jit_compile(struct sk_filter *fp)
 			case BPF_S_ALU_OR_K:	/* A |= K */
 				emit_alu_K(OR, K);
 				break;
+			case BPF_S_ANC_ALU_XOR_X: /* A ^= X; */
+				emit_alu_X(XOR);
+				break;
 			case BPF_S_ALU_LSH_X:	/* A <<= X */
 				emit_alu_X(SLL);
 				break;
-- 
1.7.6.5


^ permalink raw reply related

* Re: [PATCH v9] tilegx network driver: initial support
From: Eric Dumazet @ 2012-06-06 17:31 UTC (permalink / raw)
  To: Chris Metcalf; +Cc: bhutchings, arnd, David Miller, linux-kernel, netdev
In-Reply-To: <201206042023.q54KNEZp003834@farm-0002.internal.tilera.com>

On Mon, 2012-06-04 at 16:12 -0400, Chris Metcalf wrote:
> This change adds support for the tilegx network driver based on the
> GXIO IORPC support in the tilegx software stack, using the on-chip
> mPIPE packet processing engine.

> +static void tile_net_setup(struct net_device *dev)
> +{
> +	ether_setup(dev);
> +	dev->netdev_ops = &tile_net_ops;
> +	dev->watchdog_timeo = TILE_NET_TIMEOUT;
> +	dev->features |= NETIF_F_LLTX;
> +	dev->features |= NETIF_F_HW_CSUM;
> +	dev->features |= NETIF_F_SG;
> +	dev->features |= NETIF_F_TSO;
> +	dev->tx_queue_len = 0;
> +	dev->mtu = 1500;
> +}

Why is tx_queue_len set to 0 ?

^ permalink raw reply

* Re: [PATCH net-next] fec: Add support for Coldfire M5441x enet-mac.
From: Jan Ceuleers @ 2012-06-06 17:34 UTC (permalink / raw)
  To: Steven King; +Cc: netdev, uClinux development list, Greg Ungerer
In-Reply-To: <201206061006.21288.sfking@fdwdc.com>

On 06/06/2012 07:06 PM, Steven King wrote:
> Add support for the Freescale Coldfire M5441x; as these parts have an 
> enet-mac, add a quirk to distinguish them from the other Coldfire parts so we 
> can use the existing enet-mac support.

Stephen,

You are activating certain functionality based on whether M5441x is
defined. But where is this being defined? Should this not be added in a
Kconfig somewhere as a platform option?

Thanks, Jan

^ permalink raw reply

* Re: [PATCH] Net: mv643xx_eth: Fix compile error for architectures without clk.
From: David Miller @ 2012-06-06 17:38 UTC (permalink / raw)
  To: andrew; +Cc: jwboyer, buytenh, olof, netdev, ben
In-Reply-To: <1339000843-26078-1-git-send-email-andrew@lunn.ch>

From: Andrew Lunn <andrew@lunn.ch>
Date: Wed,  6 Jun 2012 18:40:43 +0200

> Commit 452503ebc (ARM: Orion: Eth: Add clk/clkdev support.) broke
> the building of the driver on architectures which don't have clk
> support. In particular PPC32 Pegasos which uses this driver.
> 
> Add #ifdef around the clk API usage.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net: sierra_net: device IDs for Aircard 320U++
From: David Miller @ 2012-06-06 17:40 UTC (permalink / raw)
  To: gregkh
  Cc: bjorn, netdev, linux-usb, dcbw, linux, autif.mlist, tomas.cassidy,
	stable
In-Reply-To: <20120606121633.GB8535@kroah.com>

From: Greg KH <gregkh@linuxfoundation.org>
Date: Wed, 6 Jun 2012 21:16:33 +0900

> Ok, thanks for clearing that up, I'll take the serial patch, and I'm
> sure that David will take this one.
> 
> 	Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I'll apply this, thanks.

Greg, could you not put that TAB at the beginning of your
signoffs?  It causes patchwork not to pick them up so I have
to add it manually.

Thanks.

^ permalink raw reply

* Re: [PATCH v9] tilegx network driver: initial support
From: Eric Dumazet @ 2012-06-06 17:40 UTC (permalink / raw)
  To: Chris Metcalf; +Cc: bhutchings, arnd, David Miller, linux-kernel, netdev
In-Reply-To: <201206042023.q54KNEZp003834@farm-0002.internal.tilera.com>

On Mon, 2012-06-04 at 16:12 -0400, Chris Metcalf wrote:

> +/* Allocate and push a buffer. */
> +static bool tile_net_provide_buffer(bool small)
> +{
> +	int stack = small ? small_buffer_stack : large_buffer_stack;
> +	const unsigned long buffer_alignment = 128;
> +	struct sk_buff *skb;
> +	int len;
> +
> +	len = sizeof(struct sk_buff **) + buffer_alignment;
> +	len += (small ? 128 : 1664);

1664 is a magic number, it should be a nice define

#define ..... ( ETH_DATA_LEN + .... )

> +	skb = dev_alloc_skb(len);
> +	if (skb == NULL)
> +		return false;
> +
> +	/* Make room for a back-pointer to 'skb' and guarantee alignment. */
> +	skb_reserve(skb, sizeof(struct sk_buff **));
> +	skb_reserve(skb, -(long)skb->data & (buffer_alignment - 1));
> +
> +	/* Save a back-pointer to 'skb'. */
> +	*(struct sk_buff **)(skb->data - sizeof(struct sk_buff **)) = skb;
> +
> +	/* Make sure "skb" and the back-pointer have been flushed. */
> +	wmb();

Interesting, have you considered using build_skb() instead of this
convoluted thing ?

This could save some cache misses...

^ permalink raw reply

* RE: [net-next PATCH v2 1/3] Added kernel support in EEE Ethtool commands
From: Ben Hutchings @ 2012-06-06 17:41 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Eilon Greenstein,
	peppe.cavallaro@st.com
In-Reply-To: <979A8436335E3744ADCD3A9F2A2B68A50268AA@SJEXCHMB10.corp.ad.broadcom.com>

On Wed, 2012-06-06 at 16:40 +0000, Yuval Mintz wrote:
> > > + * @supported: Link speeds for which there is eee support.
> > > + * @advertised: Link speeds the interface advertises (AN) as eee capable.
> > > + * @lp_advertised: Link speeds the link partner advertised as eee capable.
> > 
> > And these are bitmasks of SUPPORTED_* & ADVERTISED_* flags, right?
> 
> Right.
> 
> > Maybe 'link modes' not 'link speeds'?
> 
> Not that it matters greatly, but there are SUPPORTED & ADVERTISED flags for
> things other than link speeds, such as connection type and flow control,
> so using exactly the same semantic in description might confuse someone.

What I'm getting at is that we don't have flags for speeds; we have
flags for modes (speed/duplex combination).  I think EEE only works with
full-duplex modes but clients and drivers will still have to specify
that explicitly in flag names.

I can see that 'link modes' might be slightly ambiguous, so how about:

        @supported: Mask of %SUPPORTED_* flags for the speed/duplex
        combinations for which there is EEE support.

and similarly for the other fields.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next v2 1/2] inetpeer: add namespace support for inetpeer
From: David Miller @ 2012-06-06 17:43 UTC (permalink / raw)
  To: gaofeng
  Cc: serge.hallyn, ebiederm, herbert, steffen.klassert, eric.dumazet,
	netdev, containers
In-Reply-To: <1338882737-11914-1-git-send-email-gaofeng@cn.fujitsu.com>

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Tue, 5 Jun 2012 15:52:17 +0800

> now inetpeer doesn't support namespace,the information will
> be leaking across namespace.
> 
> this patch move the global vars v4_peers and v6_peers to
> netns_ipv4 and netns_ipv6 as a field peers.
> 
> add struct pernet_operations inetpeer_ops to initial pernet
> inetpeer data.
> 
> and change family_to_base and inet_getpeer to support namespace.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

As stated yesterday we have to move the inetpeer tree roots into
the FIB rule entries to fix other bugs, and that as a result will
transparently fix this problem too.

So I'm dropping these two patches and will work on the mentioned
approach to this fix.

Thanks.

^ permalink raw reply

* Re: [PATCH v2] inetpeer: fix a race in inetpeer_gc_worker()
From: David Miller @ 2012-06-06 17:45 UTC (permalink / raw)
  To: steffen.klassert; +Cc: eric.dumazet, netdev
In-Reply-To: <20120605130953.GG27795@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 5 Jun 2012 15:09:54 +0200

> On Tue, Jun 05, 2012 at 03:00:18PM +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> commit 5faa5df1fa2024 (inetpeer: Invalidate the inetpeer tree along with
>> the routing cache) added a race :
>> 
>> Before freeing an inetpeer, we must respect a RCU grace period, and make
>> sure no user will attempt to increase refcnt.
>> 
>> inetpeer_invalidate_tree() waits for a RCU grace period before inserting
>> inetpeer tree into gc_list and waking the worker. At that time, no
>> concurrent lookup can find a inetpeer in this tree.
>> 
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

I'll apply this and queue it up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH] macvtap: use set_curren_state to ensure mb
From: David Miller @ 2012-06-06 17:48 UTC (permalink / raw)
  To: honkiko; +Cc: eric.dumazet, netdev, arnd, zhiguo.hong, vikifang
In-Reply-To: <CAA7+ByUH8LsSrbetHngEZmaHkvK5aUO5v7-F=+wbveo6q68f-g@mail.gmail.com>

From: Hong zhi guo <honkiko@gmail.com>
Date: Tue, 5 Jun 2012 20:05:34 +0800

> On Tue, Jun 5, 2012 at 7:05 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> Why not using the more standard :
>>
>> prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
>>
>> and
>>
>> finish_wait(sk_sleep(sk), &wait);
>>
> 
> Thanks for your comments.  The difference is that prepare_to_wait
> inside loop introduces extra "list_empty" judgement than original
> patch. But personally I prefer the newer version too:
> 
> Signed-off-by: Hong Zhiguo <honkiko@gmail.com>

Please don't submit patches like this:

1) When submitting new versions of a patch, make a fresh
   new mailing list posting with just the patch and it's
   commit message.

   Do not post new versions by replying to other conversations.

2) Your email client has severely corrupted the patch, breaking
   up long lines etc.  Please read Documentation/email-clients.txt
   to learn how to fix this.

^ permalink raw reply

* [PATCH net-next 0/3] qlcnic: Bug fixes
From: Anirban Chakraborty @ 2012-06-06 17:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver

Please apply.

Thanks,
Anirban

^ permalink raw reply

* [PATCH net-next 2/3] qlcnic: fix unsupported CDRP command error message.
From: Anirban Chakraborty @ 2012-06-06 17:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Jitendra Kalsaria
In-Reply-To: <1339004108-7356-1-git-send-email-anirban.chakraborty@qlogic.com>

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

Add debug messages for FW CDRP command failure.

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h     |    4 +++
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c |   34 +++++++++++++++++++---
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 520ff03..df4552f 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -612,7 +612,11 @@ struct qlcnic_recv_context {
 #define QLCNIC_CDRP_CMD_GET_MAC_STATS		0x00000037
 
 #define QLCNIC_RCODE_SUCCESS		0
+#define QLCNIC_RCODE_INVALID_ARGS	6
 #define QLCNIC_RCODE_NOT_SUPPORTED	9
+#define QLCNIC_RCODE_NOT_PERMITTED	10
+#define QLCNIC_RCODE_NOT_IMPL		15
+#define QLCNIC_RCODE_INVALID		16
 #define QLCNIC_RCODE_TIMEOUT		17
 #define QLCNIC_DESTROY_CTX_RESET	0
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index cfa174d..b8ead69 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -53,12 +53,39 @@ qlcnic_issue_cmd(struct qlcnic_adapter *adapter, struct qlcnic_cmd_args *cmd)
 	rsp = qlcnic_poll_rsp(adapter);
 
 	if (rsp == QLCNIC_CDRP_RSP_TIMEOUT) {
-		dev_err(&pdev->dev, "card response timeout.\n");
+		dev_err(&pdev->dev, "CDRP response timeout.\n");
 		cmd->rsp.cmd = QLCNIC_RCODE_TIMEOUT;
 	} else if (rsp == QLCNIC_CDRP_RSP_FAIL) {
 		cmd->rsp.cmd = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
-		dev_err(&pdev->dev, "failed card response code:0x%x\n",
+		switch (cmd->rsp.cmd) {
+		case QLCNIC_RCODE_INVALID_ARGS:
+			dev_err(&pdev->dev, "CDRP invalid args: 0x%x.\n",
 				cmd->rsp.cmd);
+			break;
+		case QLCNIC_RCODE_NOT_SUPPORTED:
+		case QLCNIC_RCODE_NOT_IMPL:
+			dev_err(&pdev->dev,
+				"CDRP command not supported: 0x%x.\n",
+				cmd->rsp.cmd);
+			break;
+		case QLCNIC_RCODE_NOT_PERMITTED:
+			dev_err(&pdev->dev,
+				"CDRP requested action not permitted: 0x%x.\n",
+				cmd->rsp.cmd);
+			break;
+		case QLCNIC_RCODE_INVALID:
+			dev_err(&pdev->dev,
+				"CDRP invalid or unknown cmd received: 0x%x.\n",
+				cmd->rsp.cmd);
+			break;
+		case QLCNIC_RCODE_TIMEOUT:
+			dev_err(&pdev->dev, "CDRP command timeout: 0x%x.\n",
+				cmd->rsp.cmd);
+			break;
+		default:
+			dev_err(&pdev->dev, "CDRP command failed: 0x%x.\n",
+				cmd->rsp.cmd);
+		}
 	} else if (rsp == QLCNIC_CDRP_RSP_OK) {
 		cmd->rsp.cmd = QLCNIC_RCODE_SUCCESS;
 		if (cmd->rsp.arg2)
@@ -957,9 +984,6 @@ int qlcnic_get_mac_stats(struct qlcnic_adapter *adapter,
 		mac_stats->mac_rx_jabber = le64_to_cpu(stats->mac_rx_jabber);
 		mac_stats->mac_rx_dropped = le64_to_cpu(stats->mac_rx_dropped);
 		mac_stats->mac_rx_crc_error = le64_to_cpu(stats->mac_rx_crc_error);
-	} else {
-		dev_info(&adapter->pdev->dev,
-			"%s: Get mac stats failed =%d.\n", __func__, err);
 	}
 
 	dma_free_coherent(&adapter->pdev->dev, stats_size, stats_addr,
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH net-next 3/3] qlcnic: Fix protcol type in case of inband vlan.
From: Anirban Chakraborty @ 2012-06-06 17:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Rajesh Borundia
In-Reply-To: <1339004108-7356-1-git-send-email-anirban.chakraborty@qlogic.com>

From: Rajesh Borundia <rajesh.borundia@qlogic.com>

o Use correct l3 (ETH_IP or ETH_IPV6)protcol in case
of inband vlan. Because of incorrect protcol type driver
was setting incorrect opcode. This resulted in adapter calculating
checksum incorrectly.
o Updated driver version to 5.0.29

Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h      |    4 ++--
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index df4552f..eaa1db9 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -36,8 +36,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 28
-#define QLCNIC_LINUX_VERSIONID  "5.0.28"
+#define _QLCNIC_LINUX_SUBVERSION 29
+#define QLCNIC_LINUX_VERSIONID  "5.0.29"
 #define QLCNIC_DRV_IDC_VER  0x01
 #define QLCNIC_DRIVER_VERSION  ((_QLCNIC_LINUX_MAJOR << 16) |\
 		 (_QLCNIC_LINUX_MINOR << 8) | (_QLCNIC_LINUX_SUBVERSION))
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 707b5ca..33c3e46 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -2033,6 +2033,7 @@ qlcnic_tx_pkt(struct qlcnic_adapter *adapter,
 		vh = (struct vlan_ethhdr *)skb->data;
 		flags = FLAGS_VLAN_TAGGED;
 		vlan_tci = vh->h_vlan_TCI;
+		protocol = ntohs(vh->h_vlan_encapsulated_proto);
 	} else if (vlan_tx_tag_present(skb)) {
 		flags = FLAGS_VLAN_OOB;
 		vlan_tci = vlan_tx_tag_get(skb);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH net-next 1/3] qlcnic: Fix estimation of recv MSS in case of LRO
From: Anirban Chakraborty @ 2012-06-06 17:35 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Rajesh Borundia
In-Reply-To: <1339004108-7356-1-git-send-email-anirban.chakraborty@qlogic.com>

From: Rajesh Borundia <rajesh.borundia@qlogic.com>

o Linux stack estimates MSS from skb->len or skb_shinfo(skb)->gso_size.
In case of LRO skb->len is aggregate of len of number of packets hence MSS
obtained using skb->len would be incorrect. Incorrect estimation of recv MSS
would lead to delayed acks in some traffic patterns (which sends two or three
packets and wait for ack and only then send remaining packets). This leads to
drop in performance. Hence we need to set gso_size to MSS obtained from firmware.

o This is fixed recently in firmware hence the MSS is obtained based on
capability. If fw is capable of sending the MSS then only driver sets the gso_size.

Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic.h      |    7 +++++++
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c  |    3 +++
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h  |    1 +
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c |    3 +++
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    9 +++++++++
 5 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 8680a5d..520ff03 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -258,6 +258,8 @@ struct rcv_desc {
 	(((sts_data) >> 52) & 0x1)
 #define qlcnic_get_lro_sts_seq_number(sts_data)		\
 	((sts_data) & 0x0FFFFFFFF)
+#define qlcnic_get_lro_sts_mss(sts_data1)		\
+	((sts_data1 >> 32) & 0x0FFFF)
 
 
 struct status_desc {
@@ -623,6 +625,7 @@ struct qlcnic_recv_context {
 #define QLCNIC_CAP0_JUMBO_CONTIGUOUS	(1 << 7)
 #define QLCNIC_CAP0_LRO_CONTIGUOUS	(1 << 8)
 #define QLCNIC_CAP0_VALIDOFF		(1 << 11)
+#define QLCNIC_CAP0_LRO_MSS		(1 << 21)
 
 /*
  * Context state
@@ -829,6 +832,9 @@ struct qlcnic_mac_list_s {
 #define QLCNIC_FW_CAPABILITY_FVLANTX		BIT_9
 #define QLCNIC_FW_CAPABILITY_HW_LRO		BIT_10
 #define QLCNIC_FW_CAPABILITY_MULTI_LOOPBACK	BIT_27
+#define QLCNIC_FW_CAPABILITY_MORE_CAPS		BIT_31
+
+#define QLCNIC_FW_CAPABILITY_2_LRO_MAX_TCP_SEG	BIT_2
 
 /* module types */
 #define LINKEVENT_MODULE_NOT_PRESENT			1
@@ -918,6 +924,7 @@ struct qlcnic_ipaddr {
 #define QLCNIC_NEED_FLR			0x1000
 #define QLCNIC_FW_RESET_OWNER		0x2000
 #define QLCNIC_FW_HANG			0x4000
+#define QLCNIC_FW_LRO_MSS_CAP		0x8000
 #define QLCNIC_IS_MSI_FAMILY(adapter) \
 	((adapter)->flags & (QLCNIC_MSI_ENABLED | QLCNIC_MSIX_ENABLED))
 
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
index 8db8524..cfa174d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
@@ -237,6 +237,9 @@ qlcnic_fw_cmd_create_rx_ctx(struct qlcnic_adapter *adapter)
 						| QLCNIC_CAP0_VALIDOFF);
 	cap |= (QLCNIC_CAP0_JUMBO_CONTIGUOUS | QLCNIC_CAP0_LRO_CONTIGUOUS);
 
+	if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
+		cap |= QLCNIC_CAP0_LRO_MSS;
+
 	prq->valid_field_offset = offsetof(struct qlcnic_hostrq_rx_ctx,
 							 msix_handler);
 	prq->txrx_sds_binding = nsds_rings - 1;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
index 6ced319..28a6b28 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hdr.h
@@ -588,6 +588,7 @@ enum {
 #define CRB_DRIVER_VERSION		(QLCNIC_REG(0x2a0))
 
 #define CRB_FW_CAPABILITIES_1		(QLCNIC_CAM_RAM(0x128))
+#define CRB_FW_CAPABILITIES_2		(QLCNIC_CAM_RAM(0x12c))
 #define CRB_MAC_BLOCK_START		(QLCNIC_CAM_RAM(0x1c0))
 
 /*
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
index 799fd40..8620b69 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_init.c
@@ -1653,6 +1653,9 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
 
 	length = skb->len;
 
+	if (adapter->flags & QLCNIC_FW_LRO_MSS_CAP)
+		skb_shinfo(skb)->gso_size = qlcnic_get_lro_sts_mss(sts_data1);
+
 	if (vid != 0xffff)
 		__vlan_hwaccel_put_tag(skb, vid);
 	netif_receive_skb(skb);
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 46e77a2..707b5ca 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -1136,6 +1136,8 @@ static int
 __qlcnic_up(struct qlcnic_adapter *adapter, struct net_device *netdev)
 {
 	int ring;
+	u32 capab2;
+
 	struct qlcnic_host_rds_ring *rds_ring;
 
 	if (adapter->is_up != QLCNIC_ADAPTER_UP_MAGIC)
@@ -1146,6 +1148,12 @@ __qlcnic_up(struct qlcnic_adapter *adapter, struct net_device *netdev)
 	if (qlcnic_set_eswitch_port_config(adapter))
 		return -EIO;
 
+	if (adapter->capabilities & QLCNIC_FW_CAPABILITY_MORE_CAPS) {
+		capab2 = QLCRD32(adapter, CRB_FW_CAPABILITIES_2);
+		if (capab2 & QLCNIC_FW_CAPABILITY_2_LRO_MAX_TCP_SEG)
+			adapter->flags |= QLCNIC_FW_LRO_MSS_CAP;
+	}
+
 	if (qlcnic_fw_create_ctx(adapter))
 		return -EIO;
 
@@ -1215,6 +1223,7 @@ __qlcnic_down(struct qlcnic_adapter *adapter, struct net_device *netdev)
 	qlcnic_napi_disable(adapter);
 
 	qlcnic_fw_destroy_ctx(adapter);
+	adapter->flags &= ~QLCNIC_FW_LRO_MSS_CAP;
 
 	qlcnic_reset_rx_buffers_list(adapter);
 	qlcnic_release_tx_buffers(adapter);
-- 
1.7.4.1

^ permalink raw reply related

* Re: pull-request: can-next 2012-06-04
From: Oliver Hartkopp @ 2012-06-06 18:02 UTC (permalink / raw)
  To: davem; +Cc: Marc Kleine-Budde, Linux Netdev List, linux-can@vger.kernel.org
In-Reply-To: <4FCCD9A8.7030207@pengutronix.de>

Hello Dave,

i think there was a confusion about

	linux-can and linux-can-next

pull requests that overlapped in the mail traffic that day.

The pull request of linux-can-next is still pending.

git://gitorious.org/linux-can/linux-can-next.git master

Regards,
Oliver



On 04.06.2012 17:52, Marc Kleine-Budde wrote:

> On 06/04/2012 12:52 AM, Marc Kleine-Budde wrote:
>> Hello David,
>>
>> here are the first patches for net-next, they add power management
>> support for the flexcan driver and clarify the documentation with
>> respect to error messages.
>>
>> regards, Marc
>>
>> ---
>>
>> The following changes since commit 31a67102f4762df5544bc2dfb34a931233d2a5b2:
>>
>>   Fix blocking allocations called very early during bootup (2012-05-21 12:52:42 -0700)
>>
>> are available in the git repository at:
>>   git@gitorious.org:linux-can/linux-can-next.git master
> 
> That should be:
> 
> git://gitorious.org/linux-can/linux-can-next.git master
> 
> Sorry for the noise,
> Marc
> 



^ permalink raw reply

* Re: [PATCH v9] tilegx network driver: initial support
From: Eric Dumazet @ 2012-06-06 18:10 UTC (permalink / raw)
  To: Chris Metcalf; +Cc: bhutchings, arnd, David Miller, linux-kernel, netdev
In-Reply-To: <201206042023.q54KNEZp003834@farm-0002.internal.tilera.com>

On Mon, 2012-06-04 at 16:12 -0400, Chris Metcalf wrote:
> This change adds support for the tilegx network driver based on the
> GXIO IORPC support in the tilegx software stack, using the on-chip
> mPIPE packet processing engine.
> 

> +
> +/* Do "TSO" handling for egress.
> + *
> + * Normally drivers set NETIF_F_TSO only to support hardware TSO;
> + * otherwise the stack uses scatter-gather to implement GSO in software.
> + * On our testing, enabling GSO support (via NETIF_F_SG) drops network
> + * performance down to around 7.5 Gbps on the 10G interfaces, although
> + * also dropping cpu utilization way down, to under 8%.  But
> + * implementing "TSO" in the driver brings performance back up to line
> + * rate, while dropping cpu usage even further, to less than 4%.  In
> + * practice, profiling of GSO shows that skb_segment() is what causes
> + * the performance overheads; we benefit in the driver from using
> + * preallocated memory to duplicate the TCP/IP headers.
> + */

All this stuff cost about 300 lines of code in this driver, without IPv6
support.

I am pretty sure this performance problem should be solved in net/{core|
ipv4|ipv6} instead

What TCP performance do you get with TSO/GSO and SG off ?

^ permalink raw reply

* Re: [PATCH net-next 2/3] qlcnic: fix unsupported CDRP command error message.
From: Joe Perches @ 2012-06-06 18:14 UTC (permalink / raw)
  To: Anirban Chakraborty
  Cc: davem, netdev, Dept_NX_Linux_NIC_Driver, Jitendra Kalsaria
In-Reply-To: <1339004108-7356-3-git-send-email-anirban.chakraborty@qlogic.com>

On Wed, 2012-06-06 at 13:35 -0400, Anirban Chakraborty wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> Add debug messages for FW CDRP command failure.

trivia:

Please be consistent with the use of (preferably _no_) periods
at the end of logging messages.

$ git grep -E "[^\.]\\\\n\"" drivers/net/ethernet/qlogic/qlcnic/ | wc -l
187
$ git grep -E "\.\\\\n\"" drivers/net/ethernet/qlogic/qlcnic/ | wc -l
22

[]
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ctx.c
[]
> @@ -53,12 +53,39 @@ qlcnic_issue_cmd(struct qlcnic_adapter *adapter, struct qlcnic_cmd_args *cmd)
>  	rsp = qlcnic_poll_rsp(adapter);
>  
>  	if (rsp == QLCNIC_CDRP_RSP_TIMEOUT) {
> -		dev_err(&pdev->dev, "card response timeout.\n");
> +		dev_err(&pdev->dev, "CDRP response timeout.\n");

ie: no period necessary.

CDRP is kind of an odd acronym.
Is it for CarD ResPonse?

If it is, then I think a lot of the below messages are
not particularly sensible and the CDRP should be dropped.

>  		cmd->rsp.cmd = QLCNIC_RCODE_TIMEOUT;
>  	} else if (rsp == QLCNIC_CDRP_RSP_FAIL) {
>  		cmd->rsp.cmd = QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
> -		dev_err(&pdev->dev, "failed card response code:0x%x\n",
> +		switch (cmd->rsp.cmd) {
> +		case QLCNIC_RCODE_INVALID_ARGS:
> +			dev_err(&pdev->dev, "CDRP invalid args: 0x%x.\n",
>  				cmd->rsp.cmd);
> +			break;
> +		case QLCNIC_RCODE_NOT_SUPPORTED:
> +		case QLCNIC_RCODE_NOT_IMPL:
> +			dev_err(&pdev->dev,
> +				"CDRP command not supported: 0x%x.\n",
> +				cmd->rsp.cmd);
> +			break;
> +		case QLCNIC_RCODE_NOT_PERMITTED:
> +			dev_err(&pdev->dev,
> +				"CDRP requested action not permitted: 0x%x.\n",
> +				cmd->rsp.cmd);
> +			break;
> +		case QLCNIC_RCODE_INVALID:
> +			dev_err(&pdev->dev,
> +				"CDRP invalid or unknown cmd received: 0x%x.\n",
> +				cmd->rsp.cmd);
> +			break;
> +		case QLCNIC_RCODE_TIMEOUT:
> +			dev_err(&pdev->dev, "CDRP command timeout: 0x%x.\n",
> +				cmd->rsp.cmd);
> +			break;
> +		default:
> +			dev_err(&pdev->dev, "CDRP command failed: 0x%x.\n",
> +				cmd->rsp.cmd);
> +		}
>  	} else if (rsp == QLCNIC_CDRP_RSP_OK) {
>  		cmd->rsp.cmd = QLCNIC_RCODE_SUCCESS;
>  		if (cmd->rsp.arg2)
> @@ -957,9 +984,6 @@ int qlcnic_get_mac_stats(struct qlcnic_adapter *adapter,
>  		mac_stats->mac_rx_jabber = le64_to_cpu(stats->mac_rx_jabber);
>  		mac_stats->mac_rx_dropped = le64_to_cpu(stats->mac_rx_dropped);
>  		mac_stats->mac_rx_crc_error = le64_to_cpu(stats->mac_rx_crc_error);
> -	} else {
> -		dev_info(&adapter->pdev->dev,
> -			"%s: Get mac stats failed =%d.\n", __func__, err);
>  	}
>  
>  	dma_free_coherent(&adapter->pdev->dev, stats_size, stats_addr,

^ permalink raw reply

* Re: pull-request: can-next 2012-06-04
From: David Miller @ 2012-06-06 18:15 UTC (permalink / raw)
  To: socketcan; +Cc: mkl, netdev, linux-can
In-Reply-To: <4FCF9B3B.1070105@hartkopp.net>

From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Wed, 06 Jun 2012 20:02:35 +0200

> i think there was a confusion about
> 
> 	linux-can and linux-can-next
> 
> pull requests that overlapped in the mail traffic that day.
> 
> The pull request of linux-can-next is still pending.
> 
> git://gitorious.org/linux-can/linux-can-next.git master

Strange, I thought I had pulled it, I've done so now, thanks.

^ permalink raw reply

* Re: [PATCH v9] tilegx network driver: initial support
From: David Miller @ 2012-06-06 18:17 UTC (permalink / raw)
  To: eric.dumazet; +Cc: cmetcalf, bhutchings, arnd, linux-kernel, netdev
In-Reply-To: <1339006223.26966.36.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 06 Jun 2012 20:10:23 +0200

> I am pretty sure this performance problem should be solved in net/{core|
> ipv4|ipv6} instead
> 
> What TCP performance do you get with TSO/GSO and SG off ?

We have other drivers already doing this.

I tried a few years ago to make this generic, because NIU could
benefit from it as well, but I couldn't figure out a clean enough
way to abstract this.

Therefore it is absolutely reasonable to continue to let drivers
do this locally until we actually have a reasonable solution.

The gains are definitely significant for chips that lack real TSO
hardware, I absolutely do not require "proof" of this, it is clearly
evident to anyone who considers the issue.

^ permalink raw reply

* Re: [PATCH v9] tilegx network driver: initial support
From: Ben Hutchings @ 2012-06-06 18:19 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Chris Metcalf, arnd, David Miller, linux-kernel, netdev
In-Reply-To: <1339006223.26966.36.camel@edumazet-glaptop>

On Wed, 2012-06-06 at 20:10 +0200, Eric Dumazet wrote:
> On Mon, 2012-06-04 at 16:12 -0400, Chris Metcalf wrote:
> > This change adds support for the tilegx network driver based on the
> > GXIO IORPC support in the tilegx software stack, using the on-chip
> > mPIPE packet processing engine.
> > 
> 
> > +
> > +/* Do "TSO" handling for egress.
> > + *
> > + * Normally drivers set NETIF_F_TSO only to support hardware TSO;
> > + * otherwise the stack uses scatter-gather to implement GSO in software.
> > + * On our testing, enabling GSO support (via NETIF_F_SG) drops network
> > + * performance down to around 7.5 Gbps on the 10G interfaces, although
> > + * also dropping cpu utilization way down, to under 8%.  But
> > + * implementing "TSO" in the driver brings performance back up to line
> > + * rate, while dropping cpu usage even further, to less than 4%.  In
> > + * practice, profiling of GSO shows that skb_segment() is what causes
> > + * the performance overheads; we benefit in the driver from using
> > + * preallocated memory to duplicate the TCP/IP headers.
> > + */
> 
> All this stuff cost about 300 lines of code in this driver, without IPv6
> support.
> 
> I am pretty sure this performance problem should be solved in net/{core|
> ipv4|ipv6} instead
> 
> What TCP performance do you get with TSO/GSO and SG off ?

It's a real problem and we have soft-TSO in the sfc driver for the same
reason.  GSO means more allocation, more DMA mapping, more calls into
the driver and more register writes.

If drivers could use GSO explicitly from their ndo_start_xmit function,
more like they do with GRO, much of this would presumably be avoidable.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ 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