Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: David Miller @ 2012-07-03  5:14 UTC (permalink / raw)
  To: linux; +Cc: fubar, netdev
In-Reply-To: <4FF27CA0.7030708@8192.net>

From: John Eaglesham <linux@8192.net>
Date: Mon, 02 Jul 2012 22:01:20 -0700

> I replaced the mixed tabs and spaces with all tabs when I updated that
> function, but in retrospect the tabs and spaces were likely
> intentional. I will revert.

They were.  Don't change existing coding style unless you are certain
it is wrong.

Besides any such changes are completely outside of the scope of
the changes you are making, so you should have left them out in
any event.

^ permalink raw reply

* Re: [PATCH v6] bonding support for IPv6 transmit hashing
From: John Eaglesham @ 2012-07-03  5:01 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <18390.1341272010@death.nxdomain>

On 7/2/2012 4:33 PM, Jay Vosburgh wrote:
>> +
>> +		(((hash >> 24) XOR (hash >> 16) XOR (hash >> 8) XOR hash)
>> +			(source MAC XOR destination MAC))
>> +				modulo slave count
>
> 	This seems to be missing an XOR, between the end of "XOR hash)"
> and the start of "(source MAC".
>

You're correct.

>> 	if (skb->protocol == htons(ETH_P_IP)) {
>> +		iph = ip_hdr(skb);
>> 		if (!ip_is_fragment(iph) &&
>> -		    (iph->protocol == IPPROTO_TCP ||
>> -		     iph->protocol == IPPROTO_UDP)) {
>> +			(iph->protocol == IPPROTO_TCP ||
>> +			iph->protocol == IPPROTO_UDP)) {
>
> 	Why did these two lines change?
>

I replaced the mixed tabs and spaces with all tabs when I updated that 
function, but in retrospect the tabs and spaces were likely intentional. 
I will revert.

>> +			layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>> +			if (iph->ihl * sizeof(u32) + sizeof(__be16) * 2 >
>> +				skb_headlen(skb) - skb_network_offset(skb))
>> +				goto short_header;
>> 			layer4_xor = ntohs((*layer4hdr ^ *(layer4hdr + 1)));
>> +		} else if (skb_network_header_len(skb) < sizeof(struct iphdr)) {
>> +			goto short_header;
>> 		}
>> -		return (layer4_xor ^
>> -			((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>> -
>> +		return (layer4_xor ^ ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>
> 	This line runs past 80 columns.  There are a few more of these
> further down.
>

I will double-check this.

>> +	} else if (skb->protocol == htons(ETH_P_IPV6)) {
>> +		ipv6h = ipv6_hdr(skb);
>> +		if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
>> +			layer4hdr = (__be16 *)((u8 *)ipv6h + sizeof(struct ipv6hdr));
>
> 	Could this be written as
>
> 			layer4hdr = (__be16 *)(ipv6h + 1);
>
> 	instead?
>
> 	-J
>

Yes, I can make that change. Thanks.

John

^ permalink raw reply

* Hello Dear
From: grace_falli2674 @ 2012-07-03  3:42 UTC (permalink / raw)


Hello Dear
 My Name is Mavis

^ permalink raw reply

* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: Krishna Kumar2 @ 2012-07-03  3:28 UTC (permalink / raw)
  To: David Miller
  Cc: Dept_NX_Linux_NIC_Driver, jitendra.kalsaria, netdev, ron.mercer
In-Reply-To: <20120702.184134.1131493483786674336.davem@davemloft.net>

> David Miller <davem@davemloft.net> 07/03/2012 07:11 AM
>
> >> As per your comments, TX ring full is not expected behavior? All I
> >> can think of increasing the TX queue to 1024 and clean-up in timer
> >> instead of interrupt?
> >
> > Your transmit function should never be invoked when the queue is
> > full, logic elsewhere in your driver should have stopped the queue
> > therefore preventing further invocations of your transmit function
> > until you wake the queue when space is liberated in the TX ring.
>
> BTW, did it even occur to you that there is a kernel log message here
> in this code path for a reason?
>
> That log message is there because this event is unexpected and a
> driver error.

Correctly coded drivers check for this condition:
"atomic_read(&tx_ring->tx_count) < 2" (without the
unlikely) once again at the end of the xmit and stop
the queue if required. Also, the message should be changed
to show that reaching here is an error.

thanks,
- KK

^ permalink raw reply

* Re: Deleting an alias causes rest to get deleted
From: Stephen Hemminger @ 2012-07-03  2:44 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Volkan Yazıcı, netdev
In-Reply-To: <1341277937.2590.25.camel@bwh-desktop.uk.solarflarecom.com>

On Tue, 3 Jul 2012 02:12:17 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Mon, 2012-07-02 at 22:54 +0300, Volkan Yazıcı wrote:
> > Hi!
> > 
> > I observe an IP aliasing anomaly that occurs when I try to delete an IP 
> > alias from an interface. That is, when I delete the first address in a 
> > set of IP aliased addresses assigned according to a particular subnet, 
> > rest of the aliases get deleted as well. Check out the below snippet.
> [...]
> > As a side note, when I first asked this question to Stephen Hemminger 
> > (he forwarded me to this mailing list) he also told me that "/In Linux 
> > the interface aliases are really a legacy from the BSD style addressing, 
> > and don't act the same. It is not common practice to use them./" Is that 
> > really the case?
> [...]
> 
> If you didn't give him the full details shown above, it's possible he
> thought you meant alias interfaces such as 'eth0:0'.

Yes, that is what I was assuming by the word 'alias'

^ permalink raw reply

* BISECTED: Re: REGRESSION: 3.4.0->3.5.0-rc2 kernel WARNING on cable plug on Acer Aspire One, no network
From: Alex Villacís Lasso @ 2012-07-03  2:33 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev
In-Reply-To: <4FF05597.3040206@palosanto.com>

El 01/07/12 08:50, Alex Villacís Lasso escribió:
> El 11/06/12 16:38, Francois Romieu escribió:
>> Alex Villacís Lasso <a_villacis@palosanto.com> :
>> [...]
>>> $ grep XID dmesg-3.5.0-rc2.txt
>>> [   15.873858] r8169 0000:02:00.0: eth0: RTL8102e at 0xf7c0e000,
>>> 00:1e:68:e5:5d:b1, XID 04a00000 IRQ 44
>> The 8102e has not been touched by that many suspect patches but I do
>> not see where the problem is :o(
>>
>> Can you peel off the r8169 patches between 3.4.0 and 3.5-rc ?
>>
> Still present in 3.5-rc5. Bisection still in progress.
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
My full bisection points to this commit:

commit 0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6
Author: Marek Szyprowski <m.szyprowski@samsung.com>
Date:   Thu Dec 29 13:09:51 2011 +0100

     X86: integrate CMA with DMA-mapping subsystem

     This patch adds support for CMA to dma-mapping subsystem for x86
     architecture that uses common pci-dma/pci-nommu implementation. This
     allows to test CMA on KVM/QEMU and a lot of common x86 boxes.

     Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
     Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
     CC: Michal Nazarewicz <mina86@mina86.com>
     Acked-by: Arnd Bergmann <arnd@arndb.de>

Is this commit somehow messing with the network card DMA?

My full bisection log follows:
git bisect start
# good: [76e10d158efb6d4516018846f60c2ab5501900bc] Linux 3.4
git bisect good 76e10d158efb6d4516018846f60c2ab5501900bc
# bad: [cfaf025112d3856637ff34a767ef785ef5cf2ca9] Linux 3.5-rc2
git bisect bad cfaf025112d3856637ff34a767ef785ef5cf2ca9
# good: [3813d4024a75562baf77d3907fb6afbf8f9c8232] Merge tag 
'ia64-3.5-merge' of git://git.kernel.org/pub/scm/linux/kernel/git/aegl/linux
git bisect good 3813d4024a75562baf77d3907fb6afbf8f9c8232
# bad: [5723aa993d83803157c22327e90cd59e3dcbe879] x86: use the new 
generic strnlen_user() function
git bisect bad 5723aa993d83803157c22327e90cd59e3dcbe879
# good: [654443e20dfc0617231f28a07c96a979ee1a0239] Merge branch 
'perf-uprobes-for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 654443e20dfc0617231f28a07c96a979ee1a0239
# good: [8c914028f5ddaa417b7d0f4b7fdc24caceaa8043] Merge branch 
'drm-nouveau-next' of 
git://anongit.freedesktop.org/git/nouveau/linux-2.6 into drm-core-next
git bisect good 8c914028f5ddaa417b7d0f4b7fdc24caceaa8043
# good: [0708500d49e8439d9fe5529795bdc1485f0f46c3] Merge tag 
'devicetree-for-linus' of git://git.secretlab.ca/git/linux-2.6
git bisect good 0708500d49e8439d9fe5529795bdc1485f0f46c3
# good: [07acfc2a9349a8ce45b236c2624dad452001966b] Merge branch 'next' 
of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 07acfc2a9349a8ce45b236c2624dad452001966b
# good: [58823de9d2f1265030d0d06cb03cc2a551994398] Merge tag 
'hda-switcheroo' of 
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good 58823de9d2f1265030d0d06cb03cc2a551994398
# bad: [0f51596bd39a5c928307ffcffc9ba07f90f42a8b] Merge branch 
'for-next-arm-dma' into for-linus
git bisect bad 0f51596bd39a5c928307ffcffc9ba07f90f42a8b
# bad: [0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6] X86: integrate CMA 
with DMA-mapping subsystem
git bisect bad 0a2b9a6ea93650b8a00f9fd5ee8fdd25671e2df6
# good: [6d4a49160de2c684fb59fa627bce80e200224331] mm: page_alloc: 
change fallbacks array handling
git bisect good 6d4a49160de2c684fb59fa627bce80e200224331
# good: [cfd3da1e49bb95c355c01c0f502d657deb3d34a4] mm: Serialize access 
to min_free_kbytes
git bisect good cfd3da1e49bb95c355c01c0f502d657deb3d34a4
# good: [49f223a9cd96c7293d7258ff88c2bdf83065f69c] mm: trigger page 
reclaim in alloc_contig_range() to stabilise watermarks
git bisect good 49f223a9cd96c7293d7258ff88c2bdf83065f69c
# good: [c64be2bb1c6eb43c838b2c6d57b074078be208dd] drivers: add 
Contiguous Memory Allocator
git bisect good c64be2bb1c6eb43c838b2c6d57b074078be208dd

^ permalink raw reply

* Re: linux-next: manual merge of the net-next tree with the wireless tree
From: John W. Linville @ 2012-07-03  2:33 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, netdev, linux-next, linux-kernel, Eliad Peller,
	Johannes Berg
In-Reply-To: <20120703114443.26e03246debd0652e8d1e92b@canb.auug.org.au>

On Tue, Jul 03, 2012 at 11:44:43AM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net-next tree got a conflict in
> net/mac80211/mlme.c between commit 0030bc586591 ("mac80211: destroy
> assoc_data correctly if assoc fails") from the wireless tree and commit
> bdcbd8e0e3ff ("mac80211: clean up debugging") from the net-next tree.
> 
> I fixed it up (see below) and can carry the fix as necessary.
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> diff --cc net/mac80211/mlme.c
> index 0db5d34,e6fe84a..0000000
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@@ -2170,17 -2148,19 +2148,17 @@@ ieee80211_rx_mgmt_assoc_resp(struct iee
>   	*bss = assoc_data->bss;
>   
>   	if (status_code != WLAN_STATUS_SUCCESS) {
> - 		printk(KERN_DEBUG "%s: %pM denied association (code=%d)\n",
> - 		       sdata->name, mgmt->sa, status_code);
> + 		sdata_info(sdata, "%pM denied association (code=%d)\n",
> + 			   mgmt->sa, status_code);
>   		ieee80211_destroy_assoc_data(sdata, false);
>   	} else {
>  -		sdata_info(sdata, "associated\n");
>  -
>   		if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
>   			/* oops -- internal error -- send timeout for now */
>  -			ieee80211_destroy_assoc_data(sdata, true);
>  -			sta_info_destroy_addr(sdata, mgmt->bssid);
>  +			ieee80211_destroy_assoc_data(sdata, false);
>   			cfg80211_put_bss(*bss);
>   			return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
>   		}
> - 		printk(KERN_DEBUG "%s: associated\n", sdata->name);
> ++		sdata_info(sdata, "associated\n");
>   
>   		/*
>   		 * destroy assoc_data afterwards, as otherwise an idle

Looks good to me.

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: [PATCH net-next 09/10] net/mlx4_en: Manage flow steering rules with ethtool
From: Ben Hutchings @ 2012-07-03  1:50 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: davem, roland, yevgenyp, oren, netdev, Hadar Hen Zion, Amir Vadai
In-Reply-To: <4FF1A2DF.5090403@mellanox.com>

On Mon, 2012-07-02 at 16:32 +0300, Or Gerlitz wrote:
[...]
> > Also, are the rules really prioritised by location, so that if rule 0
> > and 1 match a packet then only rule 0 is applied?  If they are actually
> > prioritised by the match type then you need to assign locations on the
> > driver side that reflect the actual priorities.
> 
> 
> Rules are prioritized by a scheme made of "domain" X location, see below 
> and in patch #6
> the MLX4_DOMAIN_yyy entries, higher domain value means lower priority, 
> so for instance rules
> placed by ethtool would have higher priority over rules added by the L2 
> NIC  or by future RFS
> patch. Within a domain, the location matters.
> 
> You can see that simple L2 rules (e.g contain dest-mac, possibly vlan) 
> added by the driver
> use the NIC domain, wheres those added to serve ethtool commands use the 
> ETHTOOL one.
> 
> Within the ethtool domain, we maintain the priority as the location 
> specified by the user, hope this explains
> things.
[...]

Good, I didn't read the other patches but just wanted to make sure you
had thought about this.

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 09/10] net/mlx4_en: Manage flow steering rules with ethtool
From: Ben Hutchings @ 2012-07-03  1:47 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: davem, roland, Yevgeny Petrilin, Oren Duer, netdev,
	Hadar Hen Zion, Amir Vadai
In-Reply-To: <4FF19ACD.6040602@mellanox.com>

On Mon, 2012-07-02 at 15:57 +0300, Or Gerlitz wrote:
> On 7/1/2012 7:00 PM, Ben Hutchings wrote:
[...]
> >> +	spec_l3->id = MLX4_NET_TRANS_RULE_ID_IPV4;
> >> +	spec_l3->ipv4.src_ip = cmd->fs.h_u.usr_ip4_spec.ip4src;
> >> +	if (spec_l3->ipv4.src_ip)
> >> +		spec_l3->ipv4.src_ip_msk = EN_ETHTOOL_WORD_MASK;
> >> +	spec_l3->ipv4.dst_ip = cmd->fs.h_u.usr_ip4_spec.ip4dst;
> >> +	if (spec_l3->ipv4.dst_ip)
> >> +		spec_l3->ipv4.dst_ip_msk = EN_ETHTOOL_WORD_MASK;
> >
> > The conditions should be using the mask (cmd->fs.m_u) not the value.
> 
> I'd like to clarify things here a bit - the way the code is written, is to
> 
> 1st make sure that we can deal with the mask provided by the user in the 
> ethtool
> command, e.g its all zeroes or all ones (leaving a side for a minute the 
> other
> discussion on how would be best to impl. that check...) - this check is 
> done
> in mlx4_en_validate_flow
> 
> 2nd initialize mlx4 flow spec which is all empty - see calls to kzalloc 
> spec_l2/l3/l4
> 
> 3rd import the non-zero values (not masks) from the ethtool command into 
> the mlx4
> flow specs, with FULL mask
> 
> 
> Under this logic, we can use the values and not the masks, isn't that?

No, it's perfectly valid to specify a filter that matches, for example,
a destination IP address of 0.0.0.0 with mask of 255.255.255.255.  So
you really need to check the mask.  If your filter hardware doesn't
support zero values for some fields then you'll need to reject them in
mlx4_en_validate_flow.

[...]
> >> +static int mlx4_en_ethtool_to_net_trans_rule(struct net_device *dev,
> >> +					     struct ethtool_rxnfc *cmd,
> >> +					     struct list_head *rule_list_h)
> >> +{
> >> +	int err;
> >> +	u64 mac;
> >> +	__be64 be_mac;
> >> +	struct ethhdr *eth_spec;
> >> +	struct mlx4_en_priv *priv = netdev_priv(dev);
> >> +	struct mlx4_spec_list *spec_l2;
> >> +	__be64 mac_msk = cpu_to_be64(EN_ETHTOOL_MAC_MASK << 16);
> >> +
> >> +	err = mlx4_en_validate_flow(dev, cmd);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	spec_l2 = kzalloc(sizeof *spec_l2, GFP_KERNEL);
> >> +	if (!spec_l2)
> >> +		return -ENOMEM;
> >> +
> >> +	mac = priv->mac & EN_ETHTOOL_MAC_MASK;
> >> +	be_mac = cpu_to_be64(mac << 16);
> >> +
> >> +	spec_l2->id = MLX4_NET_TRANS_RULE_ID_ETH;
> >> +	memcpy(spec_l2->eth.dst_mac_msk, &mac_msk, ETH_ALEN);
> >> +	if ((cmd->fs.flow_type & ~FLOW_EXT) != ETHER_FLOW)
> >> +		memcpy(spec_l2->eth.dst_mac, &be_mac, ETH_ALEN);
> >
> > Does the hardware require filtering by MAC address and not only by layer 3/4 addresses?
> 
> YES

That's a pity.  Maybe the API should be extended so the driver can at
least report that the filter is narrower than requested.

> >> +	if ((cmd->fs.flow_type & FLOW_EXT) && cmd->fs.m_ext.vlan_tci) {
> >> +		spec_l2->eth.vlan_id = cmd->fs.h_ext.vlan_tci;
> >> +		spec_l2->eth.vlan_id_msk = cpu_to_be16(0xfff);
> >
> > This doesn't match mlx4_en_validate_flow(); you are replacing a mask of
> > 0xffff with 0xfff.
> 
> I need to check how exactly this should be done here, vlan ID is only 12 
> bits in size, so this is
> probably the source for the 0xfff vs 0xffff
[...]

If the hardware can only match the VID then you need to validate that
the mask is either 0 or 0xfff instead of 0 or 0xffff.

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

* linux-next: manual merge of the net-next tree with the wireless tree
From: Stephen Rothwell @ 2012-07-03  1:44 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Eliad Peller, John W. Linville,
	Johannes Berg

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

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
net/mac80211/mlme.c between commit 0030bc586591 ("mac80211: destroy
assoc_data correctly if assoc fails") from the wireless tree and commit
bdcbd8e0e3ff ("mac80211: clean up debugging") from the net-next tree.

I fixed it up (see below) and can carry the fix as necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/mac80211/mlme.c
index 0db5d34,e6fe84a..0000000
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@@ -2170,17 -2148,19 +2148,17 @@@ ieee80211_rx_mgmt_assoc_resp(struct iee
  	*bss = assoc_data->bss;
  
  	if (status_code != WLAN_STATUS_SUCCESS) {
- 		printk(KERN_DEBUG "%s: %pM denied association (code=%d)\n",
- 		       sdata->name, mgmt->sa, status_code);
+ 		sdata_info(sdata, "%pM denied association (code=%d)\n",
+ 			   mgmt->sa, status_code);
  		ieee80211_destroy_assoc_data(sdata, false);
  	} else {
 -		sdata_info(sdata, "associated\n");
 -
  		if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
  			/* oops -- internal error -- send timeout for now */
 -			ieee80211_destroy_assoc_data(sdata, true);
 -			sta_info_destroy_addr(sdata, mgmt->bssid);
 +			ieee80211_destroy_assoc_data(sdata, false);
  			cfg80211_put_bss(*bss);
  			return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
  		}
- 		printk(KERN_DEBUG "%s: associated\n", sdata->name);
++		sdata_info(sdata, "associated\n");
  
  		/*
  		 * destroy assoc_data afterwards, as otherwise an idle

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: David Miller @ 2012-07-03  1:41 UTC (permalink / raw)
  To: jitendra.kalsaria; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <20120702.183826.1521103644475572622.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 02 Jul 2012 18:38:26 -0700 (PDT)

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> Date: Mon, 2 Jul 2012 18:30:47 -0700
> 
>> As per your comments, TX ring full is not expected behavior? All I
>> can think of increasing the TX queue to 1024 and clean-up in timer
>> instead of interrupt?
> 
> Your transmit function should never be invoked when the queue is
> full, logic elsewhere in your driver should have stopped the queue
> therefore preventing further invocations of your transmit function
> until you wake the queue when space is liberated in the TX ring.

BTW, did it even occur to you that there is a kernel log message here
in this code path for a reason?

That log message is there because this event is unexpected and a
driver error.

^ permalink raw reply

* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: David Miller @ 2012-07-03  1:38 UTC (permalink / raw)
  To: jitendra.kalsaria; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <5E4F49720D0BAD499EE1F01232234BA877435B266A@AVEXMB1.qlogic.org>

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon, 2 Jul 2012 18:30:47 -0700

> As per your comments, TX ring full is not expected behavior? All I
> can think of increasing the TX queue to 1024 and clean-up in timer
> instead of interrupt?

Your transmit function should never be invoked when the queue is
full, logic elsewhere in your driver should have stopped the queue
therefore preventing further invocations of your transmit function
until you wake the queue when space is liberated in the TX ring.

^ permalink raw reply

* RE: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: Jitendra Kalsaria @ 2012-07-03  1:30 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Ron Mercer, Dept-NX Linux NIC Driver
In-Reply-To: <20120702.171854.1585295090835924398.davem@davemloft.net>

As per your comments, TX ring full is not expected behavior? All I can think of increasing the TX queue to 1024 and clean-up in timer instead of interrupt?

Jiten

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net] 
Sent: Monday, July 02, 2012 5:19 PM
To: Jitendra Kalsaria
Cc: netdev; Ron Mercer; Dept-NX Linux NIC Driver
Subject: Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon,  2 Jul 2012 19:41:48 -0400

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> qlge driver was acting wrongly when considering TX ring full
> as a TX error. TX ring full is expected behavior when NIC is
> overwhelmed and is expected to happen, as far as packets are
> not lost.
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

If your driver is properly coded, this code path should never trigger,
ever.  So it is an error, and you need to fix whatever bug exists in
your driver which allows this to happen, rather than this change
here which attempts to sweep the issue under the rug.

^ permalink raw reply

* Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.
From: David Miller @ 2012-07-03  1:26 UTC (permalink / raw)
  To: bhutchings
  Cc: jitendra.kalsaria, netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341278648.2590.31.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 3 Jul 2012 02:24:08 +0100

> This is hardly an urgent fix...

This entire series is suspect, and largely inappropriate for 'net'
this late in the -rc.

^ permalink raw reply

* Re: [PATCH net 6/7] qlge: refactoring of ethtool stats.
From: Ben Hutchings @ 2012-07-03  1:24 UTC (permalink / raw)
  To: Jitendra Kalsaria; +Cc: davem, netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341272514-5156-7-git-send-email-jitendra.kalsaria@qlogic.com>

This is hardly an urgent fix...

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 4/7] qlge: Fixed double pci free upon tx_ring->q allocation failure.
From: Ben Hutchings @ 2012-07-03  1:22 UTC (permalink / raw)
  To: Jitendra Kalsaria; +Cc: davem, netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341272514-5156-5-git-send-email-jitendra.kalsaria@qlogic.com>

On Mon, 2012-07-02 at 19:41 -0400, Jitendra Kalsaria wrote:
> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> ---
>  drivers/net/ethernet/qlogic/qlge/qlge_main.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> index cdbc860..9ecd15f 100644
> --- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> +++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
> @@ -2701,20 +2701,20 @@ static int ql_alloc_tx_resources(struct ql_adapter *qdev,
>  	    pci_alloc_consistent(qdev->pdev, tx_ring->wq_size,
>  				 &tx_ring->wq_base_dma);
>  
> -	if ((tx_ring->wq_base == NULL) ||
> -	    tx_ring->wq_base_dma & WQ_ADDR_ALIGN) {
> -		netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
> -		return -ENOMEM;
> -	}
> +	if (!tx_ring->wq_base || tx_ring->wq_base_dma & WQ_ADDR_ALIGN)
> +		goto err;
> +

So in case pci_alloc_consistent() fails, you now try to free anyway.
Not sure whether that's safe; do you feel lucky?

>  	tx_ring->q =
>  	    kmalloc(tx_ring->wq_len * sizeof(struct tx_ring_desc), GFP_KERNEL);
> -	if (tx_ring->q == NULL)
> +	if (!tx_ring->q)
>  		goto err;

Unrelated change.

>  	return 0;
>  err:
>  	pci_free_consistent(qdev->pdev, tx_ring->wq_size,
> -			    tx_ring->wq_base, tx_ring->wq_base_dma);
> +			tx_ring->wq_base, tx_ring->wq_base_dma);

This was nicely indented before...

> +	tx_ring->wq_base = NULL;
> +	netif_err(qdev, ifup, qdev->ndev, "tx_ring alloc failed.\n");
>  	return -ENOMEM;
>  }
>  

-- 
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: Deleting an alias causes rest to get deleted
From: Ben Hutchings @ 2012-07-03  1:12 UTC (permalink / raw)
  To: Volkan Yazıcı; +Cc: netdev
In-Reply-To: <4FF1FC74.8080401@gmail.com>

On Mon, 2012-07-02 at 22:54 +0300, Volkan Yazıcı wrote:
> Hi!
> 
> I observe an IP aliasing anomaly that occurs when I try to delete an IP 
> alias from an interface. That is, when I delete the first address in a 
> set of IP aliased addresses assigned according to a particular subnet, 
> rest of the aliases get deleted as well. Check out the below snippet.
[...]
> As a side note, when I first asked this question to Stephen Hemminger 
> (he forwarded me to this mailing list) he also told me that "/In Linux 
> the interface aliases are really a legacy from the BSD style addressing, 
> and don't act the same. It is not common practice to use them./" Is that 
> really the case?
[...]

If you didn't give him the full details shown above, it's possible he
thought you meant alias interfaces such as 'eth0:0'.

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 06/10] {NET,IB}/mlx4: Add device managed flow steering firmware API
From: David Miller @ 2012-07-03  1:04 UTC (permalink / raw)
  To: bhutchings; +Cc: ogerlitz, roland, yevgenyp, oren, netdev, hadarh
In-Reply-To: <20120702.171507.1288066003825644221.davem@davemloft.net>


Just in case you guys _really_ and _truly_ are so unable to think
outside the box that you can't come up with something reasonable, I'll
get you started with two ideas:

1) A special "chipset" dummy netdev that a special class of ethtool
   commands can run on to set these things.

2) A "chipset" genetlink family with suitable operations and
   attributes.

In both cases appropriate mechanisms are added to make for keys that
are used for chipset matching, and device drivers simply register
a notifier handler that is called on two occaisions:

1) When settings are changed.

2) Upon initial handler registry, to acquire the initial settings.

^ permalink raw reply

* Re: [PATCH net 1/7] qlge: Fixed packet transmit errors due to potential driver errors.
From: David Miller @ 2012-07-03  0:18 UTC (permalink / raw)
  To: jitendra.kalsaria; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1341272514-5156-2-git-send-email-jitendra.kalsaria@qlogic.com>

From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
Date: Mon,  2 Jul 2012 19:41:48 -0400

> From: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> 
> qlge driver was acting wrongly when considering TX ring full
> as a TX error. TX ring full is expected behavior when NIC is
> overwhelmed and is expected to happen, as far as packets are
> not lost.
> 
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>

If your driver is properly coded, this code path should never trigger,
ever.  So it is an error, and you need to fix whatever bug exists in
your driver which allows this to happen, rather than this change
here which attempts to sweep the issue under the rug.

^ permalink raw reply

* Re: [PATCH net-next 06/10] {NET,IB}/mlx4: Add device managed flow steering firmware API
From: David Miller @ 2012-07-03  0:15 UTC (permalink / raw)
  To: bhutchings; +Cc: ogerlitz, roland, yevgenyp, oren, netdev, hadarh
In-Reply-To: <1341252445.2590.12.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 2 Jul 2012 19:07:25 +0100

> But there may not be enough commonality to define a non- vendor-specific
> API.  And ethtool really isn't a good way to expose parameters that are
> per-controller rather than per-net-device, particularly if changing them
> may disrupt all running net devices on that controller and not just the
> one used to invoke SIOCETHTOOL.

I fundamentally disagree with you.

Are you really saying that it's OK for every damn vendor to define
their own magic knob to control stuff like this?  Surely you're not.

^ permalink raw reply

* Re: [PATCH v6] sctp: be more restrictive in transport selection on bundled sacks
From: David Miller @ 2012-07-03  0:10 UTC (permalink / raw)
  To: nhorman; +Cc: netdev, vyasevich, linux-sctp
In-Reply-To: <20120702122531.GA29681@hmsreliant.think-freely.org>

From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 2 Jul 2012 08:25:31 -0400

> On Sun, Jul 01, 2012 at 07:44:25PM -0400, Neil Horman wrote:
>> On Sun, Jul 01, 2012 at 02:43:19PM -0700, David Miller wrote:
>> > From: Neil Horman <nhorman@tuxdriver.com>
>> > Date: Sun, 1 Jul 2012 08:47:50 -0400
>> > 
>> > > Perhaps we could modify the SubmittingPatches document to indicate that an
>> > > Acked-by from a subsystem maintainer implicitly confers authority on the
>> > > upstream receiver to request reasonable stylistic changes that don't affect the
>> > > functionality of the patch in the interests of maintaining coding conventions.
>> > 
>> > Yes, that would make sense.
>> > 
>> 
>> 
>> I'll propose it in a few days.
>> Neil
>> 
> How does this language sound to you?

Looks fine to me.

^ permalink raw reply

* Re: [PATCH 00/12] Swap-over-NFS without deadlocking V8
From: Eric B Munson @ 2012-07-03  0:10 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Andrew Morton, Linux-MM, Linux-Netdev, Linux-NFS, LKML,
	David Miller, Trond Myklebust, Neil Brown, Christoph Hellwig,
	Peter Zijlstra, Mike Christie, Sebastian Andrzej Siewior
In-Reply-To: <20120702143556.GT14154@suse.de>

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

On Mon, 02 Jul 2012, Mel Gorman wrote:

> On Sun, Jul 01, 2012 at 01:22:54PM -0400, Eric B Munson wrote:
> > On Fri, 29 Jun 2012, Mel Gorman wrote:
> > 
> > > Changelog since V7
> > >   o Rebase to linux-next 20120629
> > >   o bi->page_dma instead of bi->page in intel driver
> > >   o Build fix for !CONFIG_NET					(sebastian)
> > >   o Restore PF_MEMALLOC flags correctly in all cases		(jlayton)
> > > 
> > > Changelog since V6
> > >   o Rebase to linux-next 20120622
> > > 
> > > Changelog since V5
> > >   o Rebase to v3.5-rc3
> > > 
> > > Changelog since V4
> > >   o Catch if SOCK_MEMALLOC flag is cleared with rmem tokens	(davem)
> > > 
> > > Changelog since V3
> > >   o Rebase to 3.4-rc5
> > >   o kmap pages for writing to swap				(akpm)
> > >   o Move forward declaration to reduce chance of duplication	(akpm)
> > > 
> > > Changelog since V2
> > >   o Nothing significant, just rebases. A radix tree lookup is replaced with
> > >     a linear search would be the biggest rebase artifact
> > > 
> > > This patch series is based on top of "Swap-over-NBD without deadlocking v14"
> > > as it depends on the same reservation of PF_MEMALLOC reserves logic.
> > > 
> > > When a user or administrator requires swap for their application, they
> > > create a swap partition and file, format it with mkswap and activate it with
> > > swapon. In diskless systems this is not an option so if swap if required
> > > then swapping over the network is considered.  The two likely scenarios
> > > are when blade servers are used as part of a cluster where the form factor
> > > or maintenance costs do not allow the use of disks and thin clients.
> > > 
> > > The Linux Terminal Server Project recommends the use of the Network
> > > Block Device (NBD) for swap but this is not always an option.  There is
> > > no guarantee that the network attached storage (NAS) device is running
> > > Linux or supports NBD. However, it is likely that it supports NFS so there
> > > are users that want support for swapping over NFS despite any performance
> > > concern. Some distributions currently carry patches that support swapping
> > > over NFS but it would be preferable to support it in the mainline kernel.
> > > 
> > > Patch 1 avoids a stream-specific deadlock that potentially affects TCP.
> > > 
> > > Patch 2 is a small modification to SELinux to avoid using PFMEMALLOC
> > > 	reserves.
> > > 
> > > Patch 3 adds three helpers for filesystems to handle swap cache pages.
> > > 	For example, page_file_mapping() returns page->mapping for
> > > 	file-backed pages and the address_space of the underlying
> > > 	swap file for swap cache pages.
> > > 
> > > Patch 4 adds two address_space_operations to allow a filesystem
> > > 	to pin all metadata relevant to a swapfile in memory. Upon
> > > 	successful activation, the swapfile is marked SWP_FILE and
> > > 	the address space operation ->direct_IO is used for writing
> > > 	and ->readpage for reading in swap pages.
> > > 
> > > Patch 5 notes that patch 3 is bolting
> > > 	filesystem-specific-swapfile-support onto the side and that
> > > 	the default handlers have different information to what
> > > 	is available to the filesystem. This patch refactors the
> > > 	code so that there are generic handlers for each of the new
> > > 	address_space operations.
> > > 
> > > Patch 6 adds an API to allow a vector of kernel addresses to be
> > > 	translated to struct pages and pinned for IO.
> > > 
> > > Patch 7 adds support for using highmem pages for swap by kmapping
> > > 	the pages before calling the direct_IO handler.
> > > 
> > > Patch 8 updates NFS to use the helpers from patch 3 where necessary.
> > > 
> > > Patch 9 avoids setting PF_private on PG_swapcache pages within NFS.
> > > 
> > > Patch 10 implements the new swapfile-related address_space operations
> > > 	for NFS and teaches the direct IO handler how to manage
> > > 	kernel addresses.
> > > 
> > > Patch 11 prevents page allocator recursions in NFS by using GFP_NOIO
> > > 	where appropriate.
> > > 
> > > Patch 12 fixes a NULL pointer dereference that occurs when using
> > > 	swap-over-NFS.
> > > 
> > > With the patches applied, it is possible to mount a swapfile that is on an
> > > NFS filesystem. Swap performance is not great with a swap stress test taking
> > > roughly twice as long to complete than if the swap device was backed by NBD.
> > 
> > To test this set I am using memory cgroups to force swap usage.  I am seeing
> > the cgroup controller killing my processes instead of using the nfs swapfile.
> > 
> 
> How sure are you that this is not a cgroup bug? For dirty file data on some
> kernels, cgroups can prematurely kill processes if pages are not being
> cleaned fast enough. I would not expect the same problem for anonymous
> pages but it's worth considering. Please also test with a normal swapfile.
> 
> If OOM is disabled and the process hangs, try capturing a sysrq+t and
> see where the process is stuck.
> 

It looks like the problem is with cgroups, when I run without cgroups and limit
memory on the boot command line everything works fine.  To test I limited the
machine to 1G of ram then ran several memory benchmarks with work set sizes of
1.5G, all completed successfully with my swap file located on an NFS share.

Tested-by: Eric B Munson <emunson@mgebm.net>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [RFC] [TCP 0/3] Receive from socket into bio without copying
From: Willy Tarreau @ 2012-07-03  0:02 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: chetan loke, Andreas Gruenbacher, netdev, linux-kernel,
	Herbert Xu, David S. Miller
In-Reply-To: <1341265024.22621.464.camel@edumazet-glaptop>

Hi Eric,

On Mon, Jul 02, 2012 at 11:37:04PM +0200, Eric Dumazet wrote:
> On Mon, 2012-07-02 at 15:41 -0400, chetan loke wrote:
> > On Mon, Jul 2, 2012 at 12:06 PM, Andreas Gruenbacher <agruen@linbit.com> wrote:
> > > On Mon, 2012-07-02 at 15:54 +0200, Eric Dumazet wrote:
> > >> So I will just say no to your patches, unless you demonstrate the
> > >> splice() problems, and how you can fix the alignment problem in a new
> > >> layer instead of in the existing zero copy standard one.
> > >
> > > Again, splice or not is not the issue here. It does not, by itself, allow zero
> > > copy from the network directly to disk but it could likely be made to support
> > > that if we can get the alignment right first.  The proposed MSG_NEW_PACKET flag
> > > helps with that, but maybe someone has a better idea.
> > >
> > 
> > Eric - by using splice do you mean something like:
> > 
> > int filedes[2];
> > PIPE_SIZE (64*1024)
> > pipe(filedes);
> > ret = splice (sock_fd_from, &from_offset, filedes [1], NULL, PIPE_SIZE,
> >                      SPLICE_F_MORE | SPLICE_F_MOVE);
> > 
> > 
> > ret = splice (filedes [0], NULL, file_fd_to,
> >                          &to_offset, ret,
> >                          SPLICE_F_MORE | SPLICE_F_MOVE);
> > 
> 
> Yes, thats more or less the plan. You also can play with bigger
> PIPE_SIZE if needed.

I confirm, this is recommended at high bit rates if you're working with
large windows.

> > i.e. splice-in from socket to pipe, and splice-out from pipe to destination?
> > 
> > Andreas - if the above assumption is true then can you apply the
> > 'MSG_NEW_PACKET' on the sender and see if the above pseudo-splice code
> > achieves something similar to what you expect on the receive side(you
> > can also play w/ F_SETPIPE_SZ -  although I found very little
> > reduction in CPU usage)? Note: My personal experience - using splice
> > from an input-file-A to output-file-B bought very minimal cpu
> > reduction(yes, both the files used O_DIRECT). Instead, a simple
> > read/write w/ O_DIRECT from file-A to file-B was much much faster.
> 
> splice() performance from socket to pipe have improved a lot in
> linux-3.5
> 
> It was not true zero copy, until very recent patches.

In fact it has been true zero copy in 2.6.25 until we faced a large
amount of data corruption and the zero copy was disabled in 2.6.25.X.
Since then it remained that way until you brought your patches to
re-instantiate it.

> (It was zero copy only on certain class of NIC, not on the ones found
> on appliances or cheap platforms)
> 
> Willy Tarreau mentioned a nice boost of performance with haproxy.

Yes definitely. The savings are more noticeable on small systems where
memory bandwidth is limited. On a small ARM system bound by RAM bandwidth,
the performance was basically doubled. But I also observed nice savings
on a core2duo equipped with 2 myricom 10Gig NICs forwarding at line rate.

> Willy wanted to work on a direct splice from socket to socket, but
> I am not sure it'll bring major speed improvement.

I'm not sure at all either, I'm betting a few percent saved from the
reduction of syscalls, not much more. This is why I'll probably check
this when I have enough time to kill.

Regards,
Willy

^ permalink raw reply

* I wanna share some happiness with you. Would you like some?)
From: Shizue Rothman @ 2012-07-03  0:01 UTC (permalink / raw)
  To: biuro@horn.com.pl

How's it going gorgeous! ))
I am Shizue.
One girl gave me ur contact and she also told me that you r cute ;)
So, I can't wait to make a new friend, I guess it is your turn now!
Let's start right now, what you think? :)

^ permalink raw reply

* [PATCH net 5/7] qlge: Categorize receive frame errors from firmware.
From: Jitendra Kalsaria @ 2012-07-02 23:41 UTC (permalink / raw)
  To: davem; +Cc: netdev, ron.mercer, Dept_NX_Linux_NIC_Driver, Jitendra Kalsaria
In-Reply-To: <1341272514-5156-1-git-send-email-jitendra.kalsaria@qlogic.com>

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

Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge.h         |    8 +++
 drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c |   14 +++++
 drivers/net/ethernet/qlogic/qlge/qlge_main.c    |   63 +++++++++++++----------
 3 files changed, 58 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index 5a639df..e81bbb7 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -1535,6 +1535,14 @@ struct nic_stats {
 	u64 rx_1024_to_1518_pkts;
 	u64 rx_1519_to_max_pkts;
 	u64 rx_len_err_pkts;
+	/* Receive Mac Err stats */
+	u64 rx_code_err;
+	u64 rx_oversize_err;
+	u64 rx_undersize_err;
+	u64 rx_preamble_err;
+	u64 rx_frame_len_err;
+	u64 rx_crc_err;
+	u64 rx_err_count;
 	/*
 	 * These stats come from offset 500h to 5C8h
 	 * in the XGMAC register.
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
index 31ee6dc..7163f5d 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_ethtool.c
@@ -226,6 +226,13 @@ static char ql_stats_str_arr[][ETH_GSTRING_LEN] = {
 	{"rx_1024_to_1518_pkts"},
 	{"rx_1519_to_max_pkts"},
 	{"rx_len_err_pkts"},
+	{"rx_code_err"},
+	{"rx_oversize_err"},
+	{"rx_undersize_err"},
+	{"rx_preamble_err"},
+	{"rx_frame_len_err"},
+	{"rx_crc_err"},
+	{"rx_err_count"},
 	{"tx_cbfc_pause_frames0"},
 	{"tx_cbfc_pause_frames1"},
 	{"tx_cbfc_pause_frames2"},
@@ -320,6 +327,13 @@ ql_get_ethtool_stats(struct net_device *ndev,
 	*data++ = s->rx_1024_to_1518_pkts;
 	*data++ = s->rx_1519_to_max_pkts;
 	*data++ = s->rx_len_err_pkts;
+	*data++ = s->rx_code_err;
+	*data++ = s->rx_oversize_err;
+	*data++ = s->rx_undersize_err;
+	*data++ = s->rx_preamble_err;
+	*data++ = s->rx_frame_len_err;
+	*data++ = s->rx_crc_err;
+	*data++ = s->rx_err_count;
 	*data++ = s->tx_cbfc_pause_frames0;
 	*data++ = s->tx_cbfc_pause_frames1;
 	*data++ = s->tx_cbfc_pause_frames2;
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index 9ecd15f..06dfafe 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -1433,6 +1433,36 @@ map_error:
 	return NETDEV_TX_BUSY;
 }
 
+/* Categorizing receive firmware frame errors */
+static void ql_categorize_rx_err(struct ql_adapter *qdev, u8 rx_err)
+{
+	struct nic_stats *stats = &qdev->nic_stats;
+
+	stats->rx_err_count++;
+
+	switch (rx_err & IB_MAC_IOCB_RSP_ERR_MASK) {
+	case IB_MAC_IOCB_RSP_ERR_CODE_ERR:
+		stats->rx_code_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_OVERSIZE:
+		stats->rx_oversize_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_UNDERSIZE:
+		stats->rx_undersize_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_PREAMBLE:
+		stats->rx_preamble_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_FRAME_LEN:
+		stats->rx_frame_len_err++;
+		break;
+	case IB_MAC_IOCB_RSP_ERR_CRC:
+		stats->rx_crc_err++;
+	default:
+		break;
+	}
+}
+
 /* Process an inbound completion from an rx ring. */
 static void ql_process_mac_rx_gro_page(struct ql_adapter *qdev,
 					struct rx_ring *rx_ring,
@@ -1499,15 +1529,6 @@ static void ql_process_mac_rx_page(struct ql_adapter *qdev,
 	addr = lbq_desc->p.pg_chunk.va;
 	prefetch(addr);
 
-
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		rx_ring->rx_errors++;
-		goto err_out;
-	}
-
 	/* The max framesize filter on this chip is set higher than
 	 * MTU since FCoE uses 2k frames.
 	 */
@@ -1593,15 +1614,6 @@ static void ql_process_mac_rx_skb(struct ql_adapter *qdev,
 	memcpy(skb_put(new_skb, length), skb->data, length);
 	skb = new_skb;
 
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		dev_kfree_skb_any(skb);
-		rx_ring->rx_errors++;
-		return;
-	}
-
 	/* loopback self test for ethtool */
 	if (test_bit(QL_SELFTEST, &qdev->flags)) {
 		ql_check_lb_frame(qdev, skb);
@@ -1908,15 +1920,6 @@ static void ql_process_mac_split_rx_intr(struct ql_adapter *qdev,
 		return;
 	}
 
-	/* Frame error, so drop the packet. */
-	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
-		netif_info(qdev, drv, qdev->ndev,
-			  "Receive error, flags2 = 0x%x\n", ib_mac_rsp->flags2);
-		dev_kfree_skb_any(skb);
-		rx_ring->rx_errors++;
-		return;
-	}
-
 	/* The max framesize filter on this chip is set higher than
 	 * MTU since FCoE uses 2k frames.
 	 */
@@ -1999,6 +2002,12 @@ static unsigned long ql_process_mac_rx_intr(struct ql_adapter *qdev,
 
 	QL_DUMP_IB_MAC_RSP(ib_mac_rsp);
 
+	/* Frame error, so drop the packet. */
+	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) {
+		ql_categorize_rx_err(qdev, ib_mac_rsp->flags2);
+		return (unsigned long)length;
+	}
+
 	if (ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV) {
 		/* The data and headers are split into
 		 * separate buffers.
-- 
1.7.1

^ permalink raw reply related


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