* skb_gso_segment oops
From: kendo @ 2012-06-06 2:36 UTC (permalink / raw)
To: netdev
Hello!
I use Intel 82574L network, open the GSO/SG/RX Checksum/txchecksum support, but the system may occasionally occur caused by skb_gso_segment Oops, this was where do I configure error, kernel Bug is this? (In fact, using the igb also met once)
------------------------------------------
Jun 5 14:03:32 AnShion <12> klogd: [14579.562843] ------------[ cut here ]------------
Jun 5 14:03:32 AnShion <12> klogd: [14579.562859] WARNING: at net/core/dev.c:1861 skb_gso_segment+0x225/0x300()
Jun 5 14:03:32 AnShion <12> klogd: [14579.562869] Hardware name: To Be Filled By O.E.M.
Jun 5 14:03:32 AnShion <12> klogd: [14579.562874] : caps=(0x2000, 0x0) len=40 data_len=0 ip_summed=1
Jun 5 14:03:32 AnShion <12> klogd: [14579.562878] Modules linked in: ipmi_watchdog igb e1000e
Jun 5 14:03:32 AnShion <12> klogd: [14579.562913] Pid: 0, comm: kworker/0:0 Not tainted 2.6.38.8 #360
Jun 5 14:03:32 AnShion <12> klogd: [14579.562917] Call Trace:
Jun 5 14:03:32 AnShion <12> klogd: [14579.562925] [<c06a7c25>] ? skb_gso_segment+0x225/0x300
Jun 5 14:03:32 AnShion <12> klogd: [14579.562934] [<c0145efc>] ? warn_slowpath_common+0x6c/0xa0
Jun 5 14:03:32 AnShion <12> klogd: [14579.562941] [<c06a7c25>] ? skb_gso_segment+0x225/0x300
Jun 5 14:03:32 AnShion <12> klogd: [14579.562948] [<c0145fae>] ? warn_slowpath_fmt+0x2e/0x30
Jun 5 14:03:32 AnShion <12> klogd: [14579.562954] [<c06a7c25>] ? skb_gso_segment+0x225/0x300
Jun 5 14:03:32 AnShion <12> klogd: [14579.562964] [<c06a7f1f>] ? dev_hard_start_xmit+0x21f/0x5d0
Jun 5 14:03:32 AnShion <12> klogd: [14579.562972] [<c0742723>] ? ipv4_confirm+0xb3/0x170
Jun 5 14:03:32 AnShion <12> klogd: [14579.562979] [<c06bdd8d>] ? sch_direct_xmit+0x13d/0x1a0
Jun 5 14:03:32 AnShion <12> klogd: [14579.562986] [<c06aa6cd>] ? dev_queue_xmit+0x26d/0x470
Jun 5 14:03:32 AnShion <12> klogd: [14579.562993] [<c06db887>] ? nf_iterate+0x67/0x90
Jun 5 14:03:32 AnShion <12> klogd: [14579.563001] [<c0705f00>] ? ip_finish_output+0x240/0x2f0
Jun 5 14:03:32 AnShion <12> klogd: [14579.563007] [<c0705cc0>] ? ip_finish_output+0x0/0x2f0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563014] [<c0706067>] ? ip_output+0xb7/0xc0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563020] [<c0705cc0>] ? ip_finish_output+0x0/0x2f0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563028] [<c0701e96>] ? ip_forward_finish+0x46/0x60
Jun 5 14:03:33 AnShion <12> klogd: [14579.563035] [<c070209f>] ? ip_forward+0x1ef/0x430
Jun 5 14:03:33 AnShion <12> klogd: [14579.563041] [<c0701e50>] ? ip_forward_finish+0x0/0x60
Jun 5 14:03:33 AnShion <12> klogd: [14579.563049] [<c0700751>] ? ip_rcv_finish+0x241/0x3c0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563056] [<c0700b76>] ? ip_rcv+0x2a6/0x320
Jun 5 14:03:33 AnShion <12> klogd: [14579.563063] [<c0700510>] ? ip_rcv_finish+0x0/0x3c0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563070] [<c06a94b8>] ? __netif_receive_skb+0x258/0x520
Jun 5 14:03:33 AnShion <12> klogd: [14579.563076] [<c0706067>] ? ip_output+0xb7/0xc0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563083] [<c06a98b3>] ? netif_receive_skb+0x23/0x50
Jun 5 14:03:33 AnShion <12> klogd: [14579.563090] [<c06a99d7>] ? napi_skb_finish+0x37/0x50
Jun 5 14:03:33 AnShion <12> klogd: [14579.563097] [<c06aa01b>] ? napi_gro_receive+0xdb/0xf0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563113] [<f8202c17>] ? e1000_receive_skb+0x47/0x60 [e1000e]
Jun 5 14:03:33 AnShion <12> klogd: [14579.563125] [<f82040a7>] ? e1000_clean_rx_irq+0x2a7/0x360 [e1000e]
Jun 5 14:03:33 AnShion <12> klogd: [14579.563139] [<f8208053>] ? e1000e_poll+0x73/0x320 [e1000e]
Jun 5 14:03:33 AnShion <12> klogd: [14579.563146] [<c06a9e4a>] ? net_rx_action+0xaa/0x1a0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563154] [<c014bff1>] ? __do_softirq+0xb1/0x190
Jun 5 14:03:33 AnShion <12> klogd: [14579.563161] [<c014bf40>] ? __do_softirq+0x0/0x190
Jun 5 14:03:33 AnShion <12> klogd: [14579.563165] <IRQ> [<c014bebd>] ? irq_exit+0x5d/0x80
Jun 5 14:03:33 AnShion <12> klogd: [14579.563177] [<c0104bc5>] ? do_IRQ+0x45/0xb0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563183] [<c014bed5>] ? irq_exit+0x75/0x80
Jun 5 14:03:33 AnShion <12> klogd: [14579.563191] [<c011c9f6>] ? smp_apic_timer_interrupt+0x56/0x90
Jun 5 14:03:33 AnShion <12> klogd: [14579.563198] [<c01036f0>] ? common_interrupt+0x30/0x38
Jun 5 14:03:33 AnShion <12> klogd: [14579.563205] [<c010b0be>] ? mwait_idle+0xfe/0x190
Jun 5 14:03:33 AnShion <12> klogd: [14579.563212] [<c010214a>] ? cpu_idle+0x8a/0xc0
Jun 5 14:03:33 AnShion <12> klogd: [14579.563219] [<c0813789>] ? start_secondary+0x1a1/0x1e8
Jun 5 14:03:33 AnShion <12> klogd: [14579.563225] ---[ end trace c07d72448ce8e376 ]---
kendo
kendo999@163.com
2012-06-06
^ permalink raw reply
* Re: [PATCH] netdev: mv643xx_eth: Prevent build on PPC32
From: Josh Boyer @ 2012-06-06 2:38 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Lennert Buytenhek, Andrew Lunn, Olof Johansson, netdev
In-Reply-To: <1338939005.21665.54.camel@deadeye.wl.decadent.org.uk>
On Wed, Jun 06, 2012 at 12:30:05AM +0100, Ben Hutchings wrote:
> On Tue, 2012-06-05 at 15:28 -0400, Josh Boyer wrote:
> > Commit 452503ebc (ARM: Orion: Eth: Add clk/clkdev support.) added use of
> > the clk driver API which results in compile errors on architectures that
> > don't implement the clk API.
> >
> > ERROR: "clk_enable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_disable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_put" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_get_rate" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_get" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> >
> > Selecting CLKDEV_LOOKUP doesn't fix this either, as the build then fails with:
> >
> > In file included from drivers/clk/clkdev.c:21:0:
> > include/linux/clkdev.h:15:24: fatal error: asm/clkdev.h: No such file or directory
> >
> > So we just prevent this from building at all on PPC32.
> [...]
>
> This dependency was introduced by:
>
> commit 16b817579fb61050f1abcc0e81089974328a9c27
> Author: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Sat Apr 16 15:24:30 2005 -0700
>
> [PATCH] ppc32: MV643XX ethernet is an option for Pegasos
>
> commit 06ede91017d015a03cf8c1c87b3ff668f9a846e0
> Author: Dale Farnsworth <dale@farnsworth.org>
> Date: Wed Sep 20 12:24:34 2006 -0700
>
> [PATCH] mv643xx_eth: restrict to 32-bit PPC_MULTIPLATFORM
>
> If Pegasos is still supposed to be supported then this needs to be fixed
> properly.
The proper fix, from my minimal looking, was one of:
1) revert the change for ARM that introduced th clk stuff
2) do a similar change as the original commit but with a bunch of
#ifdef-ery
3) implement the clkdev API stuff for 32-bit ppc
Honestly, I'd go for either 1 or 2. The commit that introduced it was
broken to begin with, but that isn't my call.
josh
^ permalink raw reply
* Re: [PATCH] netdev: mv643xx_eth: Prevent build on PPC32
From: Josh Boyer @ 2012-06-06 2:40 UTC (permalink / raw)
To: Lennert Buytenhek; +Cc: Andrew Lunn, Olof Johansson, netdev
In-Reply-To: <20120606004907.GG725@wantstofly.org>
On Wed, Jun 06, 2012 at 02:49:07AM +0200, Lennert Buytenhek wrote:
> On Tue, Jun 05, 2012 at 03:28:21PM -0400, Josh Boyer wrote:
>
> > Commit 452503ebc (ARM: Orion: Eth: Add clk/clkdev support.) added use of
> > the clk driver API which results in compile errors on architectures that
> > don't implement the clk API.
> >
> > ERROR: "clk_enable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_disable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_put" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_get_rate" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > ERROR: "clk_get" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> >
> > Selecting CLKDEV_LOOKUP doesn't fix this either, as the build then fails with:
> >
> > In file included from drivers/clk/clkdev.c:21:0:
> > include/linux/clkdev.h:15:24: fatal error: asm/clkdev.h: No such file or directory
> >
> > So we just prevent this from building at all on PPC32.
>
> If the PPC32 dependency is no longer relevant (e.g. if Pegasos platform
> support was removed from the kernel), then the commit message should
> mention that -- the above reasoning is a poor sole justification for
> this change.
You are correct. As it stands, it's no better than just breaking it
outright with commit 452503ebc. I've described the 3 possible solutions
in my other reply.
josh
^ permalink raw reply
* Re: [PATCH] netdev: mv643xx_eth: Prevent build on PPC32
From: Lennert Buytenhek @ 2012-06-06 3:02 UTC (permalink / raw)
To: Josh Boyer
Cc: Andrew Lunn, Olof Johansson, Jamie Lentin, Mike Turquette, netdev
In-Reply-To: <20120606024013.GE7683@zod.bos.redhat.com>
On Tue, Jun 05, 2012 at 10:40:14PM -0400, Josh Boyer wrote:
> > > Commit 452503ebc (ARM: Orion: Eth: Add clk/clkdev support.) added use of
> > > the clk driver API which results in compile errors on architectures that
> > > don't implement the clk API.
> > >
> > > ERROR: "clk_enable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > > ERROR: "clk_disable" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > > ERROR: "clk_put" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > > ERROR: "clk_get_rate" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > > ERROR: "clk_get" [drivers/net/ethernet/marvell/mv643xx_eth.ko] undefined!
> > >
> > > Selecting CLKDEV_LOOKUP doesn't fix this either, as the build then fails with:
> > >
> > > In file included from drivers/clk/clkdev.c:21:0:
> > > include/linux/clkdev.h:15:24: fatal error: asm/clkdev.h: No such file or directory
> > >
> > > So we just prevent this from building at all on PPC32.
> >
> > If the PPC32 dependency is no longer relevant (e.g. if Pegasos platform
> > support was removed from the kernel), then the commit message should
> > mention that -- the above reasoning is a poor sole justification for
> > this change.
>
> You are correct. As it stands, it's no better than just breaking it
> outright with commit 452503ebc.
ACK. If I'd have seen that commit ("ARM: Orion: Eth: Add clk/clkdev
support.") come by I would have said something about it, but noone
bothered to CC me on it -- and it doesn't seem that it was CCed to
netdev@ either...?
^ permalink raw reply
* RE: [PATCH RFC] c_can_pci: generic module for c_can on PCI
From: Bhupesh SHARMA @ 2012-06-06 3:50 UTC (permalink / raw)
To: rubini@gnudd.com, anilkumar@ti.com
Cc: mkl@pengutronix.de, federico.vaga@gmail.com,
alan@lxorguk.ukuu.org.uk, wg@grandegger.com, Giancarlo ASNAGHI,
alan@linux.intel.com, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20120605165008.GA21871@mail.gnudd.com>
Hi,
> -----Original Message-----
> From: rubini@gnudd.com [mailto:rubini@gnudd.com]
> Sent: Tuesday, June 05, 2012 10:20 PM
> To: anilkumar@ti.com
> Cc: mkl@pengutronix.de; Bhupesh SHARMA; federico.vaga@gmail.com;
> alan@lxorguk.ukuu.org.uk; wg@grandegger.com; Giancarlo ASNAGHI;
> alan@linux.intel.com; linux-can@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH RFC] c_can_pci: generic module for c_can on PCI
>
> > I am late to the discussion, is there any specific reason to maintain
> a
> > separate platform file (c_can_pci.c).
>
> Because it depends on pci and ifdef is bad.
>
> > I think 90% of the code is copied from c_can_paltform.c, code
> > changes will be less if you merge to existing c_can platform driver.
>
> Yes, but then we need to ifdef around, which merges two bad files
> into a single but worse file.
>
> But since the only current user of c_can is the platform device, why
> not merging the platform with the core and having pci just register a
> platform device? The only problem I see is that we need cooperation,
> because neither me nor federico have a c_can equipped board besides
> the pci one.
>
I can see examples of where different platform files are present for SJA CAN controller
as well depending on the underlying bus being used: OpenFirmware, ISA, PCI, etc..,
whilst there is a single core file there as well 'sja1000.c'
[1] Kvaser PCI platform driver, using services exposed by sja1000 core:
http://lxr.linux.no/linux+v3.4.1/drivers/net/can/sja1000/kvaser_pci.c
[2] EMS PCI platform driver, using services exposed by sja1000 core:
http://lxr.linux.no/linux+v3.4.1/drivers/net/can/sja1000/ems_pci.c
[3] SJA1000 core:
http://lxr.linux.no/linux+v3.4.1/drivers/net/can/sja1000/sja1000.c
Here each platform driver has its own version of register read/write routine implementation.
The C_CAN approach is similar to that used by SJA1000. Instead of merging the "platform with the core",
I would instead suggest to have two separate platform drivers (for each bus type) and invoke common
routines kept in say another file 'c_can_platform_common.c', thus insuring that there is no code
duplicity and we have a clean hierarchical structure as well. So we can have:
- Core file, c_can.c
- Common platform file, c_can_platform_common.c
- Platform file, c_can_platform.c, c_can_pci.c, etc..
This ensures that nothing breaks at the end of the existing C_CAN users and we have a clean
file structure as well.
Ofcourse, Wolfgang has a better idea of this structure, as he defined the same for SJA1000 and I
consulted with him on this, while he was reviewing my initial C_CAN patch set. I will let him and Marc
comment further on my proposal. Your comments are also most welcome :)
Regards,
Bhupesh
^ permalink raw reply
* [net 0/3][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-06-06 4:08 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains 2 fixes for ixgbe and a fix for e1000e.
The following are changes since commit dc5cd894cace7bda4a743487a9f87d59a3f0a095:
net/hyperv: Use wait_event on outstanding sends during device removal
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Bruce Allan (1):
e1000e: test for valid check_reset_block function pointer
John Fastabend (2):
ixgbe: fix_features rxvlan is independent of DCB and needs to be set
ixgbe: IXGBE_RXD_STAT_VP set even with Rx stripping enabled
drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++--
drivers/net/ethernet/intel/e1000e/mac.c | 2 +-
drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
drivers/net/ethernet/intel/e1000e/phy.c | 8 +++++---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 22 ++++++++++------------
5 files changed, 22 insertions(+), 20 deletions(-)
--
1.7.10.2
^ permalink raw reply
* [net 1/3] e1000e: test for valid check_reset_block function pointer
From: Jeff Kirsher @ 2012-06-06 4:08 UTC (permalink / raw)
To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1338955735-8967-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Bruce Allan <bruce.w.allan@intel.com>
commit 44abd5c12767a8c567dc4e45fd9aec3b13ca85e0 introduced NULL pointer
dereferences when attempting to access the check_reset_block function
pointer on 8257x and 80003es2lan non-copper devices.
This fix should be applied back through 3.4.
Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/e1000e/ethtool.c | 6 ++++--
drivers/net/ethernet/intel/e1000e/mac.c | 2 +-
drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
drivers/net/ethernet/intel/e1000e/phy.c | 8 +++++---
4 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c
index d863075..905e214 100644
--- a/drivers/net/ethernet/intel/e1000e/ethtool.c
+++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
@@ -258,7 +258,8 @@ static int e1000_set_settings(struct net_device *netdev,
* When SoL/IDER sessions are active, autoneg/speed/duplex
* cannot be changed
*/
- if (hw->phy.ops.check_reset_block(hw)) {
+ if (hw->phy.ops.check_reset_block &&
+ hw->phy.ops.check_reset_block(hw)) {
e_err("Cannot change link characteristics when SoL/IDER is active.\n");
return -EINVAL;
}
@@ -1615,7 +1616,8 @@ static int e1000_loopback_test(struct e1000_adapter *adapter, u64 *data)
* PHY loopback cannot be performed if SoL/IDER
* sessions are active
*/
- if (hw->phy.ops.check_reset_block(hw)) {
+ if (hw->phy.ops.check_reset_block &&
+ hw->phy.ops.check_reset_block(hw)) {
e_err("Cannot do PHY loopback test when SoL/IDER is active.\n");
*data = 0;
goto out;
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c
index 026e8b3..a134399 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.c
+++ b/drivers/net/ethernet/intel/e1000e/mac.c
@@ -709,7 +709,7 @@ s32 e1000e_setup_link_generic(struct e1000_hw *hw)
* In the case of the phy reset being blocked, we already have a link.
* We do not need to set it up again.
*/
- if (hw->phy.ops.check_reset_block(hw))
+ if (hw->phy.ops.check_reset_block && hw->phy.ops.check_reset_block(hw))
return 0;
/*
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index a4b0435..31d37a2 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6237,7 +6237,7 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
adapter->hw.phy.ms_type = e1000_ms_hw_default;
}
- if (hw->phy.ops.check_reset_block(hw))
+ if (hw->phy.ops.check_reset_block && hw->phy.ops.check_reset_block(hw))
e_info("PHY reset is blocked due to SOL/IDER session.\n");
/* Set initial default active device features */
@@ -6404,7 +6404,7 @@ err_register:
if (!(adapter->flags & FLAG_HAS_AMT))
e1000e_release_hw_control(adapter);
err_eeprom:
- if (!hw->phy.ops.check_reset_block(hw))
+ if (hw->phy.ops.check_reset_block && !hw->phy.ops.check_reset_block(hw))
e1000_phy_hw_reset(&adapter->hw);
err_hw_init:
kfree(adapter->tx_ring);
diff --git a/drivers/net/ethernet/intel/e1000e/phy.c b/drivers/net/ethernet/intel/e1000e/phy.c
index 0334d01..b860d4f 100644
--- a/drivers/net/ethernet/intel/e1000e/phy.c
+++ b/drivers/net/ethernet/intel/e1000e/phy.c
@@ -2155,9 +2155,11 @@ s32 e1000e_phy_hw_reset_generic(struct e1000_hw *hw)
s32 ret_val;
u32 ctrl;
- ret_val = phy->ops.check_reset_block(hw);
- if (ret_val)
- return 0;
+ if (phy->ops.check_reset_block) {
+ ret_val = phy->ops.check_reset_block(hw);
+ if (ret_val)
+ return 0;
+ }
ret_val = phy->ops.acquire(hw);
if (ret_val)
--
1.7.10.2
^ permalink raw reply related
* [net 2/3] ixgbe: fix_features rxvlan is independent of DCB and needs to be set
From: Jeff Kirsher @ 2012-06-06 4:08 UTC (permalink / raw)
To: davem
Cc: John Fastabend, netdev, gospo, sassmann, Alexander Duyck,
Jeff Kirsher
In-Reply-To: <1338955735-8967-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
DCB can be used independent of if RX VLAN stripping is enabled
or disabled so remove erroneous check.
Also enable or disable VLAN stripping when features are applied so
hardware and feature flags are in sync.
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-by: Marcus Dennis <marcusx.e.dennis@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index bf20457..b8b2087 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3607,10 +3607,6 @@ static void ixgbe_configure_dcb(struct ixgbe_adapter *adapter)
if (hw->mac.type == ixgbe_mac_82598EB)
netif_set_gso_max_size(adapter->netdev, 32768);
-
- /* Enable VLAN tag insert/strip */
- adapter->netdev->features |= NETIF_F_HW_VLAN_RX;
-
hw->mac.ops.set_vfta(&adapter->hw, 0, 0, true);
#ifdef IXGBE_FCOE
@@ -6701,11 +6697,6 @@ static netdev_features_t ixgbe_fix_features(struct net_device *netdev,
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
-#ifdef CONFIG_DCB
- if (adapter->flags & IXGBE_FLAG_DCB_ENABLED)
- features &= ~NETIF_F_HW_VLAN_RX;
-#endif
-
/* return error if RXHASH is being enabled when RSS is not supported */
if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED))
features &= ~NETIF_F_RXHASH;
@@ -6718,7 +6709,6 @@ static netdev_features_t ixgbe_fix_features(struct net_device *netdev,
if (!(adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE))
features &= ~NETIF_F_LRO;
-
return features;
}
@@ -6766,6 +6756,11 @@ static int ixgbe_set_features(struct net_device *netdev,
need_reset = true;
}
+ if (features & NETIF_F_HW_VLAN_RX)
+ ixgbe_vlan_strip_enable(adapter);
+ else
+ ixgbe_vlan_strip_disable(adapter);
+
if (changed & NETIF_F_RXALL)
need_reset = true;
--
1.7.10.2
^ permalink raw reply related
* [net 3/3] ixgbe: IXGBE_RXD_STAT_VP set even with Rx stripping enabled
From: Jeff Kirsher @ 2012-06-06 4:08 UTC (permalink / raw)
To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1338955735-8967-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
The hardware bit IXGBE_RXD_STAT_VP appears to be set even when Rx
stripping is disabled. This results in passing frames up the stack
which do not have the 802.1Q tag stripped but have the tci bits
set as if it was.
Working around this with a check for the feature flag bit. I
would welcome any better ideas or a pointer to exactly which
bits in the hardware register need to be cleared to get the
IXGBE_RXD_STAT_VP bit to be set per data sheet.
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Marcus Dennis <marcusx.e.dennis@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b8b2087..17ad6a3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1390,6 +1390,8 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
union ixgbe_adv_rx_desc *rx_desc,
struct sk_buff *skb)
{
+ struct net_device *dev = rx_ring->netdev;
+
ixgbe_update_rsc_stats(rx_ring, skb);
ixgbe_rx_hash(rx_ring, rx_desc, skb);
@@ -1401,14 +1403,15 @@ static void ixgbe_process_skb_fields(struct ixgbe_ring *rx_ring,
ixgbe_ptp_rx_hwtstamp(rx_ring->q_vector, skb);
#endif
- if (ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_VP)) {
+ if ((dev->features & NETIF_F_HW_VLAN_RX) &&
+ ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_VP)) {
u16 vid = le16_to_cpu(rx_desc->wb.upper.vlan);
__vlan_hwaccel_put_tag(skb, vid);
}
skb_record_rx_queue(skb, rx_ring->queue_index);
- skb->protocol = eth_type_trans(skb, rx_ring->netdev);
+ skb->protocol = eth_type_trans(skb, dev);
}
static void ixgbe_rx_skb(struct ixgbe_q_vector *q_vector,
--
1.7.10.2
^ permalink raw reply related
* Re: tg3: transmit timed out, resetting
From: Eric Dumazet @ 2012-06-06 4:12 UTC (permalink / raw)
To: ethan zhao; +Cc: Matt Carlson, Christian Kujau, LKML, netdev
In-Reply-To: <CABawtvNVKkxAgrQqJS7FJGoswQ9ZVyJcHcNwbSJQk4yscTn8gw@mail.gmail.com>
On Wed, 2012-06-06 at 10:29 +0800, ethan zhao wrote:
> So no way to fix it via firmware update or Linux driver ? :<
Yes, but you need to cooperate, or else it might take more time than
necessary.
Asking questions like that on lkml is not going to help very much.
So, once again, we kindly ask you try a recent kernel and post
register dump and some additional information when transmit timeouts
happen.
The 'latest kernel' is either linux-3.5.rc1, or one of David Miller
tree :
http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=summary
or
http://git.kernel.org/?p=linux/kernel/git/davem/net.git;a=summary
Thanks
^ permalink raw reply
* Re: skb_gso_segment oops
From: Eric Dumazet @ 2012-06-06 4:41 UTC (permalink / raw)
To: kendo; +Cc: netdev
In-Reply-To: <4FCEC21F.257F91.32344@m12-16.163.com>
On Wed, 2012-06-06 at 10:36 +0800, kendo wrote:
> Hello!
>
> I use Intel 82574L network, open the GSO/SG/RX Checksum/txchecksum
> support, but the system may occasionally occur caused by
> skb_gso_segment Oops, this was where do I configure error, kernel Bug
> is this? (In fact, using the igb also met once)
New kernels have debugging facility to better diagnose what its going
on. (gso_size, gso_type)
Please upgrade your kernel.
^ permalink raw reply
* Re: tg3: transmit timed out, resetting
From: ethan zhao @ 2012-06-06 4:52 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Matt Carlson, Christian Kujau, LKML, netdev
In-Reply-To: <1338955977.2760.3603.camel@edumazet-glaptop>
Eric,
That is ask for confirmation from Matt Carlson of Broadcom.
Ethan
On Wed, Jun 6, 2012 at 12:12 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2012-06-06 at 10:29 +0800, ethan zhao wrote:
>> So no way to fix it via firmware update or Linux driver ? :<
>
> Yes, but you need to cooperate, or else it might take more time than
> necessary.
>
> Asking questions like that on lkml is not going to help very much.
>
> So, once again, we kindly ask you try a recent kernel and post
> register dump and some additional information when transmit timeouts
> happen.
>
> The 'latest kernel' is either linux-3.5.rc1, or one of David Miller
> tree :
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=summary
>
> or
>
> http://git.kernel.org/?p=linux/kernel/git/davem/net.git;a=summary
>
> Thanks
>
>
^ permalink raw reply
* Re: [net-next RFC PATCH] virtio_net: collect satistics and export through ethtool
From: Jason Wang @ 2012-06-06 5:02 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20120605101037.GA19834@redhat.com>
On 06/05/2012 06:10 PM, Michael S. Tsirkin wrote:
> On Tue, Jun 05, 2012 at 04:38:41PM +0800, Jason Wang wrote:
>> Satistics counters is useful for debugging and performance optimization, so this
>> patch lets virtio_net driver collect following and export them to userspace
>> through "ethtool -S":
>>
>> - number of packets sent/received
>> - number of bytes sent/received
>> - number of callbacks for tx/rx
>> - number of kick for tx/rx
>> - number of bytes/packets queued for tx
>>
>> As virtnet_stats were per-cpu, so both per-cpu and gloabl satistics were exposed
>> like:
>>
>> NIC statistics:
>> tx_bytes[0]: 2551
>> tx_packets[0]: 12
>> tx_kick[0]: 12
>> tx_callbacks[0]: 1
>> tx_queued_packets[0]: 12
>> tx_queued_bytes[0]: 3055
>> rx_bytes[0]: 0
>> rx_packets[0]: 0
>> rx_kick[0]: 0
>> rx_callbacks[0]: 0
>> tx_bytes[1]: 5575
>> tx_packets[1]: 37
>> tx_kick[1]: 38
>> tx_callbacks[1]: 0
>> tx_queued_packets[1]: 38
>> tx_queued_bytes[1]: 5217
>> rx_bytes[1]: 4175
>> rx_packets[1]: 25
>> rx_kick[1]: 1
>> rx_callbacks[1]: 16
>> tx_bytes: 8126
>> tx_packets: 49
>> tx_kick: 50
>> tx_callbacks: 1
>> tx_queued_packets: 50
>> tx_queued_bytes: 8272
>> rx_bytes: 4175
>> rx_packets: 25
>> rx_kick: 1
>> rx_callbacks: 16
>>
>> TODO:
>>
>> - more satistics
>> - unitfy the ndo_get_stats64 and get_ethtool_stats
>> - calculate the pending bytes/pkts
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>> drivers/net/virtio_net.c | 130 +++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 127 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 5214b1e..7ab0cc1 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -41,6 +41,10 @@ module_param(gso, bool, 0444);
>> #define VIRTNET_SEND_COMMAND_SG_MAX 2
>> #define VIRTNET_DRIVER_VERSION "1.0.0"
>>
>> +#define VIRTNET_STAT_OFF(m) offsetof(struct virtnet_stats, m)
>> +#define VIRTNET_STAT(stat, i) (*((u64 *)((char *)stat + \
> What's going on? Why cast to char *?
It's used to let the pointer advance at the unit of bytes instead of the
whole stat strcuture.
>> + virtnet_stats_str_attr[i].stat_offset)))
> These are confusing unless you see what virtnet_stats_str_attr
> is so please move them near that definition.
ok.
>> +
>> struct virtnet_stats {
>> struct u64_stats_sync syncp;
>> u64 tx_bytes;
>> @@ -48,8 +52,33 @@ struct virtnet_stats {
>>
>> u64 rx_bytes;
>> u64 rx_packets;
>> +
>> + u64 tx_kick;
>> + u64 rx_kick;
>> + u64 tx_callbacks;
>> + u64 rx_callbacks;
>> + u64 tx_queued_packets;
>> + u64 tx_queued_bytes;
>> +};
> I have an idea (not a must): why don't we simply create an enum
> enum virtnet_stats {
> VIRTNET_TX_KICK,
> VIRTNET_RX_KICK,
> ....
> VIRTNET_MAX_STAT,
> }
>
>
> now stats can just do
> stats->data[VIRTNET_RX_KICK] instead of stats->rx_kick
> which is not a big problem, but copying them in bulk
> becomes straight-forward, no need for macros at all.
>
> If we decide to do this, needs to be a separate patch,
> then this one on top.
Make sense, would do this.
>> +
>> +static struct {
> static const.
>
>> + char string[ETH_GSTRING_LEN];
>> + int stat_offset;
>> +} virtnet_stats_str_attr[] = {
>> + { "tx_bytes", VIRTNET_STAT_OFF(tx_bytes)},
>> + { "tx_packets", VIRTNET_STAT_OFF(tx_packets)},
>> + { "tx_kick", VIRTNET_STAT_OFF(tx_kick)},
>> + { "tx_callbacks", VIRTNET_STAT_OFF(tx_callbacks)},
>> + { "tx_queued_packets", VIRTNET_STAT_OFF(tx_queued_packets)},
>> + { "tx_queued_bytes", VIRTNET_STAT_OFF(tx_queued_bytes)},
>> + { "rx_bytes" , VIRTNET_STAT_OFF(rx_bytes)},
>> + { "rx_packets", VIRTNET_STAT_OFF(rx_packets)},
>> + { "rx_kick", VIRTNET_STAT_OFF(rx_kick)},
>> + { "rx_callbacks", VIRTNET_STAT_OFF(rx_callbacks)},
> VIRTNET_STAT_OFF does not save much here, but if you are after
> saving characters then make the macro instanciate the string
> as well.
>
>> };
>>
>> +#define VIRTNET_NUM_STATS ARRAY_SIZE(virtnet_stats_str_attr)
>> +
> if you pass virtnet_stats_str_attr to VIRTNET_STAT macro,
> then it's explicit and VIRTNET_NUM_STATS won't be needed either.
It's used to report the number of satistics through .get_sset_count.
>
>> struct virtnet_info {
>> struct virtio_device *vdev;
>> struct virtqueue *rvq, *svq, *cvq;
>> @@ -142,6 +171,11 @@ static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
>> static void skb_xmit_done(struct virtqueue *svq)
>> {
>> struct virtnet_info *vi = svq->vdev->priv;
>> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> +
>> + u64_stats_update_begin(&stats->syncp);
>> + stats->tx_callbacks++;
>> + u64_stats_update_end(&stats->syncp);
>>
>> /* Suppress further interrupts. */
>> virtqueue_disable_cb(svq);
>> @@ -461,6 +495,7 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
>> {
>> int err;
>> bool oom;
>> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>>
>> do {
>> if (vi->mergeable_rx_bufs)
>> @@ -477,13 +512,24 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
>> } while (err> 0);
>> if (unlikely(vi->num> vi->max))
>> vi->max = vi->num;
>> - virtqueue_kick(vi->rvq);
>> + if (virtqueue_kick_prepare(vi->rvq)) {
>> + virtqueue_notify(vi->rvq);
>> + u64_stats_update_begin(&stats->syncp);
>> + stats->rx_kick++;
>> + u64_stats_update_end(&stats->syncp);
>> + }
>> return !oom;
>> }
>>
>> static void skb_recv_done(struct virtqueue *rvq)
>> {
>> struct virtnet_info *vi = rvq->vdev->priv;
>> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> +
>> + u64_stats_update_begin(&stats->syncp);
>> + stats->rx_callbacks++;
>> + u64_stats_update_end(&stats->syncp);
>> +
>> /* Schedule NAPI, Suppress further interrupts if successful. */
>> if (napi_schedule_prep(&vi->napi)) {
>> virtqueue_disable_cb(rvq);
>> @@ -626,7 +672,9 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
>> static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>> {
>> struct virtnet_info *vi = netdev_priv(dev);
>> + struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>> int capacity;
>> + bool kick;
>>
>> /* Free up any pending old buffers before queueing new ones. */
>> free_old_xmit_skbs(vi);
>> @@ -651,7 +699,17 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>> kfree_skb(skb);
>> return NETDEV_TX_OK;
>> }
>> - virtqueue_kick(vi->svq);
>> +
>> + kick = virtqueue_kick_prepare(vi->svq);
>> + if (kick)
> probably
> if (unlikely(kick))
>
>> + virtqueue_notify(vi->svq);
>> +
>> + u64_stats_update_begin(&stats->syncp);
>> + if (kick)
> this too
>
>> + stats->tx_kick++;
>> + stats->tx_queued_bytes += skb->len;
>> + stats->tx_queued_packets++;
>> + u64_stats_update_end(&stats->syncp);
>>
>> /* Don't wait up for transmitted skbs to be freed. */
>> skb_orphan(skb);
>> @@ -926,7 +984,6 @@ static void virtnet_get_ringparam(struct net_device *dev,
>>
>> }
>>
>> -
>> static void virtnet_get_drvinfo(struct net_device *dev,
>> struct ethtool_drvinfo *info)
>> {
>> @@ -939,10 +996,77 @@ static void virtnet_get_drvinfo(struct net_device *dev,
>>
>> }
>>
>> +static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
>> +{
>> + int i, cpu;
>> + switch (stringset) {
>> + case ETH_SS_STATS:
>> + for_each_possible_cpu(cpu)
>> + for (i = 0; i< VIRTNET_NUM_STATS; i++) {
>> + sprintf(buf, "%s[%u]",
>> + virtnet_stats_str_attr[i].string, cpu);
>> + buf += ETH_GSTRING_LEN;
>> + }
>> + for (i = 0; i< VIRTNET_NUM_STATS; i++) {
>> + memcpy(buf, virtnet_stats_str_attr[i].string,
>> + ETH_GSTRING_LEN);
>> + buf += ETH_GSTRING_LEN;
>> + }
>> + break;
>> + }
>> +}
>> +
>> +static int virtnet_get_sset_count(struct net_device *dev, int sset)
>> +{
>> + switch (sset) {
>> + case ETH_SS_STATS:
>> + return VIRTNET_NUM_STATS * (num_online_cpus() + 1);
> This will allocate buffers for online cpus only, but the above
> will fill them in for all possible cpus.
> Will this overrun some buffer?
>
Yes, a typo here, should be num_possible_cpus().
Thanks
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> +}
>> +
>> +static void virtnet_get_ethtool_stats(struct net_device *dev,
>> + struct ethtool_stats *stats, u64 *buf)
> The coding style says
> Descendants are always substantially shorter than the parent and
> are placed substantially to the right.
>
> you can't call it substantially to the right if it's to the left of
> the opening '(' :), so please indent it aligning on the opening.
Looks like something wrong in my emacs c-style confiugration, would
check it.
>> +{
>> + struct virtnet_info *vi = netdev_priv(dev);
>> + int cpu, i;
>> + unsigned int start;
>> + struct virtnet_stats sample, total;
>> +
>> + memset(&total, 0, sizeof(total));
>> + memset(&sample, 0, sizeof(sample));
>> +
>> + for_each_possible_cpu(cpu) {
>> + struct virtnet_stats *stats = per_cpu_ptr(vi->stats, cpu);
>> + do {
>> + start = u64_stats_fetch_begin(&stats->syncp);
>> + for (i = 0; i< VIRTNET_NUM_STATS; i++) {
>> + VIRTNET_STAT(&sample, i) =
>> + VIRTNET_STAT(stats, i);
> when you feel the need to break lines like this - don't :)
> use an inline function instead.
>
>> +
> kill empty line here
>> + }
> don't put {} around single statements pls.
Sure
>> + } while (u64_stats_fetch_retry(&stats->syncp, start));
>> + for (i = 0; i< VIRTNET_NUM_STATS; i++) {
>> + *buf = VIRTNET_STAT(&sample, i);
>> + VIRTNET_STAT(&total, i) += VIRTNET_STAT(stats, i);
>> + buf++;
>> + }
>> + }
>> +
>> + for (i = 0; i< VIRTNET_NUM_STATS; i++) {
>> + *buf = VIRTNET_STAT(&total, i);
>> + buf++;
>> + }
>> +}
>> +
>> static const struct ethtool_ops virtnet_ethtool_ops = {
>> .get_drvinfo = virtnet_get_drvinfo,
>> .get_link = ethtool_op_get_link,
>> .get_ringparam = virtnet_get_ringparam,
>> + .get_ethtool_stats = virtnet_get_ethtool_stats,
>> + .get_strings = virtnet_get_strings,
>> + .get_sset_count = virtnet_get_sset_count,
>> };
>>
>> #define MIN_MTU 68
> --
> 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
^ permalink raw reply
* Re: Strange latency spikes/TX network stalls on Sun Fire X4150(x86) and e1000e
From: Eric Dumazet @ 2012-06-06 5:10 UTC (permalink / raw)
To: Hiroaki SHIMODA
Cc: Tom Herbert, Denys Fedoryshchenko, netdev, e1000-devel,
jeffrey.t.kirsher, jesse.brandeburg, davem
In-Reply-To: <20120529232518.e5b41759.shimoda.hiroaki@gmail.com>
On Tue, 2012-05-29 at 23:25 +0900, Hiroaki SHIMODA wrote:
> If I understand the code and spec correctly, TX interrupts are
> generated when TXDCTL.WTHRESH descriptors have been accumulated
> and write backed.
>
> I tentatively changed the TXDCTL.WTHRESH to 1, then it seems
> that latency spikes are disappear.
>
> drivers/net/ethernet/intel/e1000e/e1000.h
> @@ -181,7 +181,7 @@ struct e1000_info;
> #define E1000_TXDCTL_DMA_BURST_ENABLE \
> (E1000_TXDCTL_GRAN | /* set descriptor granularity */ \
> E1000_TXDCTL_COUNT_DESC | \
> - (5 << 16) | /* wthresh must be +1 more than desired */\
> + (1 << 16) | /* wthresh must be +1 more than desired */\
> (1 << 8) | /* hthresh */ \
> 0x1f) /* pthresh */
>
Was this patch officially submitted ?
Thanks !
^ permalink raw reply
* Re: [PATCH] netdev: mv643xx_eth: Prevent build on PPC32
From: Andrew Lunn @ 2012-06-06 5:29 UTC (permalink / raw)
To: Josh Boyer
Cc: Ben Hutchings, Lennert Buytenhek, Andrew Lunn, Olof Johansson,
netdev
In-Reply-To: <20120606023842.GD7683@zod.bos.redhat.com>
> The proper fix, from my minimal looking, was one of:
>
> 1) revert the change for ARM that introduced th clk stuff
> 2) do a similar change as the original commit but with a bunch of
> #ifdef-ery
> 3) implement the clkdev API stuff for 32-bit ppc
>
> Honestly, I'd go for either 1 or 2. The commit that introduced it was
> broken to begin with, but that isn't my call.
I broke it. Sorry.
At the time, there was a push to remove all the #ifdefs. The following
patchset was doing this:
https://lkml.org/lkml/2012/4/21/94
it would provide dummy implementations for those systems without clk
support. However, it seems that patch set never made it in, and i did
not declare my dependency on it.
I'm happy to add #ifdef. However, i would first like to understand
what was 'broken to begin with'.
Thanks
Andrew
^ permalink raw reply
* [PATCH net-next 3/7] be2net: do not modify PCI MaxReadReq size
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
Setting the PCI MRRS to a value of 4096 (overriding the system decided
value) had provided perf improvement in TX.
But, IBM has provided feedback that on POWER platforms, this value is set
by the system firmware, and drivers modifying this value can cause
unpredictable results (like EEH errors.) So, backing off this change.
On POWER7 platforms most slots, it seems, do get a MRRS of 4096.
This patch reverts the following commit:
"be2net: Modified PCI MaxReadReq size to 4096 bytes"
commit 5a56eb10babbcd7b3796dc3c28c271260aa3608d.
Suggested-by: Brian King <bjking1@us.ibm.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index cfbec8b..695c3cf 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2766,8 +2766,6 @@ static int be_setup(struct be_adapter *adapter)
be_cmd_set_flow_control(adapter, adapter->tx_fc,
adapter->rx_fc);
- pcie_set_readrq(adapter->pdev, 4096);
-
if (be_physfn(adapter) && num_vfs) {
if (adapter->dev_num_vfs)
be_vf_setup(adapter);
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 2/7] be2net: cleanup be_vid_config()
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
- get rid of 2 unused arguments to the routine and some unused code
- don't use the term "vlan_tag" in place of "vid" as they are different
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 25 +++++++++----------------
1 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index f1b0926..cfbec8b 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -785,19 +785,12 @@ static int be_change_mtu(struct net_device *netdev, int new_mtu)
* A max of 64 (BE_NUM_VLANS_SUPPORTED) vlans can be configured in BE.
* If the user configures more, place BE in vlan promiscuous mode.
*/
-static int be_vid_config(struct be_adapter *adapter, bool vf, u32 vf_num)
+static int be_vid_config(struct be_adapter *adapter)
{
- struct be_vf_cfg *vf_cfg = &adapter->vf_cfg[vf_num];
- u16 vtag[BE_NUM_VLANS_SUPPORTED];
- u16 ntags = 0, i;
+ u16 vids[BE_NUM_VLANS_SUPPORTED];
+ u16 num = 0, i;
int status = 0;
- if (vf) {
- vtag[0] = cpu_to_le16(vf_cfg->vlan_tag);
- status = be_cmd_vlan_config(adapter, vf_cfg->if_handle, vtag,
- 1, 1, 0);
- }
-
/* No need to further configure vids if in promiscuous mode */
if (adapter->promiscuous)
return 0;
@@ -808,10 +801,10 @@ static int be_vid_config(struct be_adapter *adapter, bool vf, u32 vf_num)
/* Construct VLAN Table to give to HW */
for (i = 0; i < VLAN_N_VID; i++)
if (adapter->vlan_tag[i])
- vtag[ntags++] = cpu_to_le16(i);
+ vids[num++] = cpu_to_le16(i);
status = be_cmd_vlan_config(adapter, adapter->if_handle,
- vtag, ntags, 1, 0);
+ vids, num, 1, 0);
/* Set to VLAN promisc mode as setting VLAN filter failed */
if (status) {
@@ -840,7 +833,7 @@ static int be_vlan_add_vid(struct net_device *netdev, u16 vid)
adapter->vlan_tag[vid] = 1;
if (adapter->vlans_added <= (adapter->max_vlans + 1))
- status = be_vid_config(adapter, false, 0);
+ status = be_vid_config(adapter);
if (!status)
adapter->vlans_added++;
@@ -862,7 +855,7 @@ static int be_vlan_rem_vid(struct net_device *netdev, u16 vid)
adapter->vlan_tag[vid] = 0;
if (adapter->vlans_added <= adapter->max_vlans)
- status = be_vid_config(adapter, false, 0);
+ status = be_vid_config(adapter);
if (!status)
adapter->vlans_added--;
@@ -889,7 +882,7 @@ static void be_set_rx_mode(struct net_device *netdev)
be_cmd_rx_filter(adapter, IFF_PROMISC, OFF);
if (adapter->vlans_added)
- be_vid_config(adapter, false, 0);
+ be_vid_config(adapter);
}
/* Enable multicast promisc if num configured exceeds what we support */
@@ -2763,7 +2756,7 @@ static int be_setup(struct be_adapter *adapter)
be_cmd_get_fw_ver(adapter, adapter->fw_ver, NULL);
if (adapter->vlans_added)
- be_vid_config(adapter, false, 0);
+ be_vid_config(adapter);
be_set_rx_mode(adapter->netdev);
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 1/7] be2net: don't call vid_config() when there's no vlan config
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
be_vid_config() is called from be_setup() to replay config cmds after
a card reset. Skip calling it when no vlans are configured.
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 08efd30..f1b0926 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2762,7 +2762,8 @@ static int be_setup(struct be_adapter *adapter)
be_cmd_get_fw_ver(adapter, adapter->fw_ver, NULL);
- be_vid_config(adapter, false, 0);
+ if (adapter->vlans_added)
+ be_vid_config(adapter, false, 0);
be_set_rx_mode(adapter->netdev);
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 4/7] be2net: fix reporting number of actual rx queues
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 695c3cf..3aa478f 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1890,6 +1890,12 @@ static int be_rx_cqs_create(struct be_adapter *adapter)
*/
adapter->num_rx_qs = (num_irqs(adapter) > 1) ?
num_irqs(adapter) + 1 : 1;
+ if (adapter->num_rx_qs != MAX_RX_QS) {
+ rtnl_lock();
+ netif_set_real_num_rx_queues(adapter->netdev,
+ adapter->num_rx_qs);
+ rtnl_unlock();
+ }
adapter->big_page_size = (1 << get_order(rx_frag_size)) * PAGE_SIZE;
for_all_rx_queues(adapter, rxo, i) {
@@ -3740,7 +3746,7 @@ static int __devinit be_probe(struct pci_dev *pdev,
goto disable_dev;
pci_set_master(pdev);
- netdev = alloc_etherdev_mq(sizeof(struct be_adapter), MAX_TX_QS);
+ netdev = alloc_etherdev_mqs(sizeof(*adapter), MAX_TX_QS, MAX_RX_QS);
if (netdev == NULL) {
status = -ENOMEM;
goto rel_reg;
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 5/7] be2net: remove unnecessary usage of unlikely()
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3aa478f..42ee75b 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -719,8 +719,8 @@ static netdev_tx_t be_xmit(struct sk_buff *skb,
* 60 bytes long.
* As a workaround disable TX vlan offloading in such cases.
*/
- if (unlikely(vlan_tx_tag_present(skb) &&
- (skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60))) {
+ if (vlan_tx_tag_present(skb) &&
+ (skb->ip_summed != CHECKSUM_PARTIAL || skb->len <= 60)) {
skb = skb_share_check(skb, GFP_ATOMIC);
if (unlikely(!skb))
goto tx_drop;
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 7/7] be2net: update driver version
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index c5c4c0e..7b5cc2b 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -34,7 +34,7 @@
#include "be_hw.h"
#include "be_roce.h"
-#define DRV_VER "4.2.220u"
+#define DRV_VER "4.2.248.0u"
#define DRV_NAME "be2net"
#define BE_NAME "ServerEngines BladeEngine2 10Gbps NIC"
#define BE3_NAME "ServerEngines BladeEngine3 10Gbps NIC"
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 6/7] be2net: do not use SCRATCHPAD register
From: Sathya Perla @ 2012-06-06 5:37 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
In-Reply-To: <1338961043-26820-1-git-send-email-sathya.perla@emulex.com>
The CUST_SCRATCHPAD_CSR register is used for marking if FW cleanup is
needed. This is used in a crash kernel scenario. Do no use this register as
it is not available for some functions. Instead, always issue an FLR when
a function is probed *except* when VFs are preset (enabled in the previous
PF load).
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_hw.h | 2 --
drivers/net/ethernet/emulex/benet/be_main.c | 10 +++-------
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_hw.h b/drivers/net/ethernet/emulex/benet/be_hw.h
index d9fb0c5..7c8a710 100644
--- a/drivers/net/ethernet/emulex/benet/be_hw.h
+++ b/drivers/net/ethernet/emulex/benet/be_hw.h
@@ -58,8 +58,6 @@
#define SLI_PORT_CONTROL_IP_MASK 0x08000000
-#define PCICFG_CUST_SCRATCHPAD_CSR 0x1EC
-
/********* Memory BAR register ************/
#define PCICFG_MEMBAR_CTRL_INT_CTRL_OFFSET 0xfc
/* Host Interrupt Enable, if set interrupts are enabled although "PCI Interrupt
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 42ee75b..f29827f 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1049,6 +1049,8 @@ static int be_find_vfs(struct be_adapter *adapter, int vf_state)
u16 offset, stride;
pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_SRIOV);
+ if (!pos)
+ return 0;
pci_read_config_word(pdev, pos + PCI_SRIOV_VF_OFFSET, &offset);
pci_read_config_word(pdev, pos + PCI_SRIOV_VF_STRIDE, &stride);
@@ -2542,7 +2544,6 @@ static int be_clear(struct be_adapter *adapter)
be_cmd_fw_clean(adapter);
be_msix_disable(adapter);
- pci_write_config_dword(adapter->pdev, PCICFG_CUST_SCRATCHPAD_CSR, 0);
return 0;
}
@@ -2785,8 +2786,6 @@ static int be_setup(struct be_adapter *adapter)
schedule_delayed_work(&adapter->work, msecs_to_jiffies(1000));
adapter->flags |= BE_FLAGS_WORKER_SCHEDULED;
-
- pci_write_config_dword(adapter->pdev, PCICFG_CUST_SCRATCHPAD_CSR, 1);
return 0;
err:
be_clear(adapter);
@@ -3724,10 +3723,7 @@ reschedule:
static bool be_reset_required(struct be_adapter *adapter)
{
- u32 reg;
-
- pci_read_config_dword(adapter->pdev, PCICFG_CUST_SCRATCHPAD_CSR, ®);
- return reg;
+ return be_find_vfs(adapter, ENABLED) > 0 ? false : true;
}
static int __devinit be_probe(struct pci_dev *pdev,
--
1.7.4
^ permalink raw reply related
* [PATCH net-next 0/7] be2net: patch series
From: Sathya Perla @ 2012-06-06 5:39 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
Pls apply.
Sathya Perla (7):
be2net: do not call be_vid_config() when there's no vlan config
be2net: cleanup be_vid_config()
be2net: do not modify PCI MaxReadReq size
be2net: fix reporting number of actual rx queues
be2net: remove unnecessary usage of unlikely()
be2net: do not use SCRATCHPAD register
be2net: update driver version
drivers/net/ethernet/emulex/benet/be.h | 2 +-
drivers/net/ethernet/emulex/benet/be_hw.h | 2 -
drivers/net/ethernet/emulex/benet/be_main.c | 50 ++++++++++++---------------
3 files changed, 23 insertions(+), 31 deletions(-)
--
1.7.4
^ permalink raw reply
* [patch] net/ethernet: ks8851_mll unregister_netdev() before freeing
From: Dan Carpenter @ 2012-06-06 6:31 UTC (permalink / raw)
To: Raffaele Recalcati; +Cc: David S. Miller, netdev, kernel-janitors
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);
^ permalink raw reply related
* Re: [net-next RFC PATCH] virtio_net: collect satistics and export through ethtool
From: Michael S. Tsirkin @ 2012-06-06 6:49 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <4FCEE45B.8080800@redhat.com>
On Wed, Jun 06, 2012 at 01:02:19PM +0800, Jason Wang wrote:
> On 06/05/2012 06:10 PM, Michael S. Tsirkin wrote:
> >On Tue, Jun 05, 2012 at 04:38:41PM +0800, Jason Wang wrote:
> >>Satistics counters is useful for debugging and performance optimization, so this
> >>patch lets virtio_net driver collect following and export them to userspace
> >>through "ethtool -S":
> >>
> >>- number of packets sent/received
> >>- number of bytes sent/received
> >>- number of callbacks for tx/rx
> >>- number of kick for tx/rx
> >>- number of bytes/packets queued for tx
> >>
> >>As virtnet_stats were per-cpu, so both per-cpu and gloabl satistics were exposed
> >>like:
> >>
> >>NIC statistics:
> >> tx_bytes[0]: 2551
> >> tx_packets[0]: 12
> >> tx_kick[0]: 12
> >> tx_callbacks[0]: 1
> >> tx_queued_packets[0]: 12
> >> tx_queued_bytes[0]: 3055
> >> rx_bytes[0]: 0
> >> rx_packets[0]: 0
> >> rx_kick[0]: 0
> >> rx_callbacks[0]: 0
> >> tx_bytes[1]: 5575
> >> tx_packets[1]: 37
> >> tx_kick[1]: 38
> >> tx_callbacks[1]: 0
> >> tx_queued_packets[1]: 38
> >> tx_queued_bytes[1]: 5217
> >> rx_bytes[1]: 4175
> >> rx_packets[1]: 25
> >> rx_kick[1]: 1
> >> rx_callbacks[1]: 16
> >> tx_bytes: 8126
> >> tx_packets: 49
> >> tx_kick: 50
> >> tx_callbacks: 1
> >> tx_queued_packets: 50
> >> tx_queued_bytes: 8272
> >> rx_bytes: 4175
> >> rx_packets: 25
> >> rx_kick: 1
> >> rx_callbacks: 16
> >>
> >>TODO:
> >>
> >>- more satistics
> >>- unitfy the ndo_get_stats64 and get_ethtool_stats
> >>- calculate the pending bytes/pkts
> >>
> >>Signed-off-by: Jason Wang<jasowang@redhat.com>
> >>---
> >> drivers/net/virtio_net.c | 130 +++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 files changed, 127 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>index 5214b1e..7ab0cc1 100644
> >>--- a/drivers/net/virtio_net.c
> >>+++ b/drivers/net/virtio_net.c
> >>@@ -41,6 +41,10 @@ module_param(gso, bool, 0444);
> >> #define VIRTNET_SEND_COMMAND_SG_MAX 2
> >> #define VIRTNET_DRIVER_VERSION "1.0.0"
> >>
> >>+#define VIRTNET_STAT_OFF(m) offsetof(struct virtnet_stats, m)
> >>+#define VIRTNET_STAT(stat, i) (*((u64 *)((char *)stat + \
> >What's going on? Why cast to char *?
>
> It's used to let the pointer advance at the unit of bytes instead of
> the whole stat strcuture.
Make offset be in units of sizeof u64 and you won't need
this hack.
Also, macro parameters must be surrounded with ().
> >>+ virtnet_stats_str_attr[i].stat_offset)))
> >These are confusing unless you see what virtnet_stats_str_attr
> >is so please move them near that definition.
>
> ok.
> >>+
> >> struct virtnet_stats {
> >> struct u64_stats_sync syncp;
> >> u64 tx_bytes;
> >>@@ -48,8 +52,33 @@ struct virtnet_stats {
> >>
> >> u64 rx_bytes;
> >> u64 rx_packets;
> >>+
> >>+ u64 tx_kick;
> >>+ u64 rx_kick;
> >>+ u64 tx_callbacks;
> >>+ u64 rx_callbacks;
> >>+ u64 tx_queued_packets;
> >>+ u64 tx_queued_bytes;
> >>+};
> >I have an idea (not a must): why don't we simply create an enum
> >enum virtnet_stats {
> > VIRTNET_TX_KICK,
> > VIRTNET_RX_KICK,
> > ....
> > VIRTNET_MAX_STAT,
> >}
> >
> >
> >now stats can just do
> > stats->data[VIRTNET_RX_KICK] instead of stats->rx_kick
> >which is not a big problem, but copying them in bulk
> >becomes straight-forward, no need for macros at all.
> >
> >If we decide to do this, needs to be a separate patch,
> >then this one on top.
>
> Make sense, would do this.
> >>+
> >>+static struct {
> >static const.
> >
> >>+ char string[ETH_GSTRING_LEN];
> >>+ int stat_offset;
> >>+} virtnet_stats_str_attr[] = {
> >>+ { "tx_bytes", VIRTNET_STAT_OFF(tx_bytes)},
> >>+ { "tx_packets", VIRTNET_STAT_OFF(tx_packets)},
> >>+ { "tx_kick", VIRTNET_STAT_OFF(tx_kick)},
> >>+ { "tx_callbacks", VIRTNET_STAT_OFF(tx_callbacks)},
> >>+ { "tx_queued_packets", VIRTNET_STAT_OFF(tx_queued_packets)},
> >>+ { "tx_queued_bytes", VIRTNET_STAT_OFF(tx_queued_bytes)},
> >>+ { "rx_bytes" , VIRTNET_STAT_OFF(rx_bytes)},
> >>+ { "rx_packets", VIRTNET_STAT_OFF(rx_packets)},
> >>+ { "rx_kick", VIRTNET_STAT_OFF(rx_kick)},
> >>+ { "rx_callbacks", VIRTNET_STAT_OFF(rx_callbacks)},
> >VIRTNET_STAT_OFF does not save much here, but if you are after
> >saving characters then make the macro instanciate the string
> >as well.
> >
> >> };
> >>
> >>+#define VIRTNET_NUM_STATS ARRAY_SIZE(virtnet_stats_str_attr)
> >>+
> >if you pass virtnet_stats_str_attr to VIRTNET_STAT macro,
> >then it's explicit and VIRTNET_NUM_STATS won't be needed either.
>
> It's used to report the number of satistics through .get_sset_count.
Yes but you can then open-code.
> >
> >> struct virtnet_info {
> >> struct virtio_device *vdev;
> >> struct virtqueue *rvq, *svq, *cvq;
> >>@@ -142,6 +171,11 @@ static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
> >> static void skb_xmit_done(struct virtqueue *svq)
> >> {
> >> struct virtnet_info *vi = svq->vdev->priv;
> >>+ struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> >>+
> >>+ u64_stats_update_begin(&stats->syncp);
> >>+ stats->tx_callbacks++;
> >>+ u64_stats_update_end(&stats->syncp);
> >>
> >> /* Suppress further interrupts. */
> >> virtqueue_disable_cb(svq);
> >>@@ -461,6 +495,7 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
> >> {
> >> int err;
> >> bool oom;
> >>+ struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> >>
> >> do {
> >> if (vi->mergeable_rx_bufs)
> >>@@ -477,13 +512,24 @@ static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
> >> } while (err> 0);
> >> if (unlikely(vi->num> vi->max))
> >> vi->max = vi->num;
> >>- virtqueue_kick(vi->rvq);
> >>+ if (virtqueue_kick_prepare(vi->rvq)) {
> >>+ virtqueue_notify(vi->rvq);
> >>+ u64_stats_update_begin(&stats->syncp);
> >>+ stats->rx_kick++;
> >>+ u64_stats_update_end(&stats->syncp);
> >>+ }
> >> return !oom;
> >> }
> >>
> >> static void skb_recv_done(struct virtqueue *rvq)
> >> {
> >> struct virtnet_info *vi = rvq->vdev->priv;
> >>+ struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> >>+
> >>+ u64_stats_update_begin(&stats->syncp);
> >>+ stats->rx_callbacks++;
> >>+ u64_stats_update_end(&stats->syncp);
> >>+
> >> /* Schedule NAPI, Suppress further interrupts if successful. */
> >> if (napi_schedule_prep(&vi->napi)) {
> >> virtqueue_disable_cb(rvq);
> >>@@ -626,7 +672,9 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> >> static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> >> {
> >> struct virtnet_info *vi = netdev_priv(dev);
> >>+ struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> >> int capacity;
> >>+ bool kick;
> >>
> >> /* Free up any pending old buffers before queueing new ones. */
> >> free_old_xmit_skbs(vi);
> >>@@ -651,7 +699,17 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> >> kfree_skb(skb);
> >> return NETDEV_TX_OK;
> >> }
> >>- virtqueue_kick(vi->svq);
> >>+
> >>+ kick = virtqueue_kick_prepare(vi->svq);
> >>+ if (kick)
> >probably
> > if (unlikely(kick))
> >
> >>+ virtqueue_notify(vi->svq);
> >>+
> >>+ u64_stats_update_begin(&stats->syncp);
> >>+ if (kick)
> >this too
> >
> >>+ stats->tx_kick++;
> >>+ stats->tx_queued_bytes += skb->len;
> >>+ stats->tx_queued_packets++;
> >>+ u64_stats_update_end(&stats->syncp);
> >>
> >> /* Don't wait up for transmitted skbs to be freed. */
> >> skb_orphan(skb);
> >>@@ -926,7 +984,6 @@ static void virtnet_get_ringparam(struct net_device *dev,
> >>
> >> }
> >>
> >>-
> >> static void virtnet_get_drvinfo(struct net_device *dev,
> >> struct ethtool_drvinfo *info)
> >> {
> >>@@ -939,10 +996,77 @@ static void virtnet_get_drvinfo(struct net_device *dev,
> >>
> >> }
> >>
> >>+static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> >>+{
> >>+ int i, cpu;
> >>+ switch (stringset) {
> >>+ case ETH_SS_STATS:
> >>+ for_each_possible_cpu(cpu)
> >>+ for (i = 0; i< VIRTNET_NUM_STATS; i++) {
> >>+ sprintf(buf, "%s[%u]",
> >>+ virtnet_stats_str_attr[i].string, cpu);
> >>+ buf += ETH_GSTRING_LEN;
> >>+ }
> >>+ for (i = 0; i< VIRTNET_NUM_STATS; i++) {
> >>+ memcpy(buf, virtnet_stats_str_attr[i].string,
> >>+ ETH_GSTRING_LEN);
> >>+ buf += ETH_GSTRING_LEN;
> >>+ }
> >>+ break;
> >>+ }
> >>+}
> >>+
> >>+static int virtnet_get_sset_count(struct net_device *dev, int sset)
> >>+{
> >>+ switch (sset) {
> >>+ case ETH_SS_STATS:
> >>+ return VIRTNET_NUM_STATS * (num_online_cpus() + 1);
> >This will allocate buffers for online cpus only, but the above
> >will fill them in for all possible cpus.
> >Will this overrun some buffer?
> >
>
> Yes, a typo here, should be num_possible_cpus().
> Thanks
> >>+ default:
> >>+ return -EOPNOTSUPP;
> >>+ }
> >>+}
> >>+
> >>+static void virtnet_get_ethtool_stats(struct net_device *dev,
> >>+ struct ethtool_stats *stats, u64 *buf)
> >The coding style says
> > Descendants are always substantially shorter than the parent and
> > are placed substantially to the right.
> >
> >you can't call it substantially to the right if it's to the left of
> >the opening '(' :), so please indent it aligning on the opening.
>
> Looks like something wrong in my emacs c-style confiugration, would
> check it.
> >>+{
> >>+ struct virtnet_info *vi = netdev_priv(dev);
> >>+ int cpu, i;
> >>+ unsigned int start;
> >>+ struct virtnet_stats sample, total;
> >>+
> >>+ memset(&total, 0, sizeof(total));
> >>+ memset(&sample, 0, sizeof(sample));
> >>+
> >>+ for_each_possible_cpu(cpu) {
> >>+ struct virtnet_stats *stats = per_cpu_ptr(vi->stats, cpu);
> >>+ do {
> >>+ start = u64_stats_fetch_begin(&stats->syncp);
> >>+ for (i = 0; i< VIRTNET_NUM_STATS; i++) {
> >>+ VIRTNET_STAT(&sample, i) =
> >>+ VIRTNET_STAT(stats, i);
> >when you feel the need to break lines like this - don't :)
> >use an inline function instead.
> >
> >>+
> >kill empty line here
> >>+ }
> >don't put {} around single statements pls.
>
> Sure
> >>+ } while (u64_stats_fetch_retry(&stats->syncp, start));
> >>+ for (i = 0; i< VIRTNET_NUM_STATS; i++) {
> >>+ *buf = VIRTNET_STAT(&sample, i);
> >>+ VIRTNET_STAT(&total, i) += VIRTNET_STAT(stats, i);
> >>+ buf++;
> >>+ }
> >>+ }
> >>+
> >>+ for (i = 0; i< VIRTNET_NUM_STATS; i++) {
> >>+ *buf = VIRTNET_STAT(&total, i);
> >>+ buf++;
> >>+ }
> >>+}
> >>+
> >> static const struct ethtool_ops virtnet_ethtool_ops = {
> >> .get_drvinfo = virtnet_get_drvinfo,
> >> .get_link = ethtool_op_get_link,
> >> .get_ringparam = virtnet_get_ringparam,
> >>+ .get_ethtool_stats = virtnet_get_ethtool_stats,
> >>+ .get_strings = virtnet_get_strings,
> >>+ .get_sset_count = virtnet_get_sset_count,
> >> };
> >>
> >> #define MIN_MTU 68
> >--
> >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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox