* Re: [PATCH][NET] several cleanups and bugfixes for fec.c: fix the .remove code
From: Shawn Guo @ 2011-12-07 13:28 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, Shawn Guo, linux-kernel, linux-arm-kernel
In-Reply-To: <04c11563fee748562d7b53d5e5b3c5cf37fef200.1323163127.git.LW@KARO-electronics.de>
On Tue, Dec 06, 2011 at 11:27:15AM +0100, Lothar Waßmann wrote:
> The .remove code is broken in several ways.
> - mdiobus_unregister() is called twice for the same object in case of dual FEC
> - phy_disconnect() is being called when the PHY is already disconnected
> - the requested IRQ(s) are not freed
> - fec_stop() is being called with the inteface already stopped
>
> All of those lead to kernel crashes if the remove function is actually used.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
> drivers/net/ethernet/freescale/fec.c | 30 +++++++++++++++++++++---------
> 1 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index ab0afb5..70ec7ec 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -235,6 +235,7 @@ struct fec_enet_private {
>
> /* Phylib and MDIO interface */
> struct mii_bus *mii_bus;
> + int mii_cnt;
> struct phy_device *phy_dev;
> int mii_timeout;
> uint phy_speed;
> @@ -1040,8 +1041,12 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> */
> if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id > 0) {
> /* fec1 uses fec0 mii_bus */
> - fep->mii_bus = fec0_mii_bus;
> - return 0;
> + if (fep->mii_cnt && fec0_mii_bus) {
This seems broken. The second fec has its own fep and fep->mii_cnt is
always 0 here.
Regards,
Shawn
> + fep->mii_bus = fec0_mii_bus;
> + fep->mii_cnt++;
> + return 0;
> + }
> + return -ENOENT;
> }
>
> fep->mii_timeout = 0;
> @@ -1086,6 +1091,8 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> if (mdiobus_register(fep->mii_bus))
> goto err_out_free_mdio_irq;
>
> + fep->mii_cnt++;
> +
> /* save fec0 mii_bus */
> if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
> fec0_mii_bus = fep->mii_bus;
> @@ -1102,11 +1109,11 @@ err_out:
>
> static void fec_enet_mii_remove(struct fec_enet_private *fep)
> {
> - if (fep->phy_dev)
> - phy_disconnect(fep->phy_dev);
> - mdiobus_unregister(fep->mii_bus);
> - kfree(fep->mii_bus->irq);
> - mdiobus_free(fep->mii_bus);
> + if (--fep->mii_cnt == 0) {
> + mdiobus_unregister(fep->mii_bus);
> + kfree(fep->mii_bus->irq);
> + mdiobus_free(fep->mii_bus);
> + }
> }
>
> static int fec_enet_get_settings(struct net_device *ndev,
> @@ -1646,13 +1653,18 @@ fec_drv_remove(struct platform_device *pdev)
> struct net_device *ndev = platform_get_drvdata(pdev);
> struct fec_enet_private *fep = netdev_priv(ndev);
> struct resource *r;
> + int i;
>
> - fec_stop(ndev);
> + unregister_netdev(ndev);
> fec_enet_mii_remove(fep);
> + for (i = 0; i < FEC_IRQ_NUM; i++) {
> + int irq = platform_get_irq(pdev, i);
> + if (irq > 0)
> + free_irq(irq, ndev);
> + }
> clk_disable(fep->clk);
> clk_put(fep->clk);
> iounmap(fep->hwp);
> - unregister_netdev(ndev);
> free_netdev(ndev);
>
> r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> --
> 1.5.6.5
^ permalink raw reply
* Re: [PATCH 0/4] skb paged fragment destructors
From: Ian Campbell @ 2011-12-07 13:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Jesse Brandeburg, netdev@vger.kernel.org
In-Reply-To: <1323177878.2448.18.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Tue, 2011-12-06 at 13:24 +0000, Eric Dumazet wrote:
> Le mardi 06 décembre 2011 à 11:57 +0000, Ian Campbell a écrit :
> > On Wed, 2011-11-09 at 15:01 +0000, Ian Campbell wrote:
> > > * split linear data allocation and shinfo allocation into two. I
> > > suspect this will have its own performance implications? On the
> > > positive side skb_shared_info could come from its own fixed size
> > > pool/cache which might have some benefits
> >
> > I played with this to see how it would look. Illustrative patch below.
> >
> > I figure that lots of small frames is the interesting workload for a
> > change such as this but I don't know if iperf is necessarily the best
> > benchmark for measuring that.
> > Before changing things I got:
> > iperf -c qarun -m -t 60 -u -b 10000M -l 64
> > ------------------------------------------------------------
> > Client connecting to qarun, UDP port 5001
> > Sending 64 byte datagrams
> > UDP buffer size: 224 KByte (default)
> > ------------------------------------------------------------
> > [ 3] local 10.80.225.63 port 45857 connected with 10.80.224.22 port 5001
> > [ ID] Interval Transfer Bandwidth
> > [ 3] 0.0-60.0 sec 844 MBytes 118 Mbits/sec
> > [ 3] Sent 13820376 datagrams
> > [ 3] Server Report:
> > [ 3] 0.0-60.0 sec 844 MBytes 118 Mbits/sec 0.005 ms 0/13820375 (0%)
> > [ 3] 0.0-60.0 sec 1 datagrams received out-of-order
> > whereas with the patch:
> > # iperf -c qarun -m -t 60 -u -b 10000M -l 64
> > ------------------------------------------------------------
> > Client connecting to qarun, UDP port 5001
> > Sending 64 byte datagrams
> > UDP buffer size: 224 KByte (default)
> > ------------------------------------------------------------
> > [ 3] local 10.80.225.63 port 42504 connected with 10.80.224.22 port 5001
> > [ ID] Interval Transfer Bandwidth
> > [ 3] 0.0-60.0 sec 833 MBytes 116 Mbits/sec
> > [ 3] Sent 13645857 datagrams
> > [ 3] Server Report:
> > [ 3] 0.0-60.0 sec 833 MBytes 116 Mbits/sec 0.005 ms 0/13645856 (0%)
> > [ 3] 0.0-60.0 sec 1 datagrams received out-of-order
> >
> > With 1200 byte datagrams I get basically identical throughput.
> >
> > (nb: none of the skb destructor stuff was present in either case)
>
> Sorry, but the real problem is that if skb producer and consumer are not
> on same CPU, each skb will now hit SLUB slowpath three times instead of
> two.
>
> Some workloads are : One cpu fully handling IRQ from device, dispatching
> skbs to consumers on other cpus.
So something like a multithreaded apache benchmark would be interesting?
> Plus skb->truesize is wrong after your patch.
> Not sure if cloning is correct either...
Me neither, the patch was just one which happened to run well enough to
run some numbers on. I'll be sure to double-check/correct that stuff if
I persist with the approach.
> Anyway, do we _really_ need 16 frags per skb, I dont know....
MAX_SKB_FRAGS is:
/* To allow 64K frame to be packed as single skb without frag_list. Since
* GRO uses frags we allocate at least 16 regardless of page size.
*/
#if (65536/PAGE_SIZE + 2) < 16
#define MAX_SKB_FRAGS 16UL
#else
#define MAX_SKB_FRAGS (65536/PAGE_SIZE + 2)
#endif
So I think actually it turns out to be 18 on systems with 4k pages. If
we reduced that to be 16 that would save 48 bytes on amd64 which pulls
the shinfo size down to 440 and then
NET_SKB_PAD (64) + 1500 + 14 + 440 = 2018
which does fit in half a page.
Using 16 still means that a 64k frame fits precisely in frags on a 4096
page size system. I don't know what the extra 2 was for (it predates
git), perhaps just to allow for some slop due to misalignment in the
first page of data?
> This gives problems when/if skb must be linearized and we hit
> PAGE_ALLOC_COSTLY_ORDER
>
> Alternatively, we could use order-1 or order-2 pages on x86 to get
> 8192/16384 bytes frags. (fallback to order-0 pages in case of allocation
> failures)
Or fallback to separate allocation of shinfo?
Ian.
^ permalink raw reply
* [PATCH v2 0/8][NET] fec.c: several cleanups and bugfixes
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
The following set of patches provides some cleanup and bugfixes for
drivers/net/ethernet/freescale/fec.c and makes the driver buildable as
a module.
Changes wrt v1:
- more appropriate subject lines
- fix a messup in patch 7: mii_cnt must be a driver global variable
Lothar Waßmann (8):
misc cleanups
set con_id in clk_get() call to NULL
prevent dobule restart of interface on FDX/HDX change
don't request invalid IRQ
don't munge MAC address from platform data
preserve MII/RMII setting in fec_stop()
fix the .remove code
make FEC driver buildable as module
drivers/net/ethernet/freescale/Kconfig | 2 +-
drivers/net/ethernet/freescale/fec.c | 63 ++++++++++++++++++++++----------
2 files changed, 44 insertions(+), 21 deletions(-)
^ permalink raw reply
* [PATCH v2 1/8][NET] fec.c: misc cleanups
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
- remove some bogus whitespace
- remove line wraps from printk messages
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 1124ce0..f224e58 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -99,7 +99,7 @@ static struct platform_device_id fec_devtype[] = {
MODULE_DEVICE_TABLE(platform, fec_devtype);
enum imx_fec_type {
- IMX25_FEC = 1, /* runs on i.mx25/50/53 */
+ IMX25_FEC = 1, /* runs on i.mx25/50/53 */
IMX27_FEC, /* runs on i.mx27/35/51 */
IMX28_FEC,
IMX6Q_FEC,
@@ -132,7 +132,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
#elif defined (CONFIG_M5272C3)
#define FEC_FLASHMAC (0xffe04000 + 4)
#elif defined(CONFIG_MOD5272)
-#define FEC_FLASHMAC 0xffc0406b
+#define FEC_FLASHMAC 0xffc0406b
#else
#define FEC_FLASHMAC 0
#endif
@@ -972,8 +972,9 @@ static int fec_enet_mii_probe(struct net_device *ndev)
}
if (phy_id >= PHY_MAX_ADDR) {
- printk(KERN_INFO "%s: no PHY, assuming direct connection "
- "to switch\n", ndev->name);
+ printk(KERN_INFO
+ "%s: no PHY, assuming direct connection to switch\n",
+ ndev->name);
strncpy(mdio_bus_id, "0", MII_BUS_ID_SIZE);
phy_id = 0;
}
@@ -998,8 +999,9 @@ static int fec_enet_mii_probe(struct net_device *ndev)
fep->link = 0;
fep->full_duplex = 0;
- printk(KERN_INFO "%s: Freescale FEC PHY driver [%s] "
- "(mii_bus:phy_addr=%s, irq=%d)\n", ndev->name,
+ printk(KERN_INFO
+ "%s: Freescale FEC PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)\n",
+ ndev->name,
fep->phy_dev->drv->name, dev_name(&fep->phy_dev->dev),
fep->phy_dev->irq);
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 2/8][NET] fec.c: set con_id in clk_get() call to NULL
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
The con_id is actually not needed for clk_get().
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index f224e58..65ee506 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1585,7 +1585,7 @@ fec_probe(struct platform_device *pdev)
}
}
- fep->clk = clk_get(&pdev->dev, "fec_clk");
+ fep->clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(fep->clk)) {
ret = PTR_ERR(fep->clk);
goto failed_clk;
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 3/8][NET] fec.c: prevent dobule restart of interface on FDX/HDX change
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
Upon detection of a FDX/HDX change the interface is restarted twice.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 65ee506..7ef408f 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -865,6 +865,8 @@ static void fec_enet_adjust_link(struct net_device *ndev)
if (phy_dev->link) {
if (fep->full_duplex != phy_dev->duplex) {
fec_restart(ndev, phy_dev->duplex);
+ /* prevent unnecessary second fec_restart() below */
+ fep->link = phy_dev->link;
status_change = 1;
}
}
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 4/8][NET] fec.c: don't request invalid IRQ
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
prevent calling request_irq() with a known invalid IRQ number and
preserve the return value of the platform_get_irq() function
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 7ef408f..e2b5ce6 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1575,8 +1575,12 @@ fec_probe(struct platform_device *pdev)
for (i = 0; i < FEC_IRQ_NUM; i++) {
irq = platform_get_irq(pdev, i);
- if (i && irq < 0)
- break;
+ if (irq < 0) {
+ if (i)
+ break;
+ ret = irq;
+ goto failed_irq;
+ }
ret = request_irq(irq, fec_enet_interrupt, IRQF_DISABLED, pdev->name, ndev);
if (ret) {
while (--i >= 0) {
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 5/8][NET] fec.c: don't munge MAC address from platform data
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
When the MAC address is supplied via platform_data it should be OK as
it is and should not be modified in case of a dual FEC setup.
Also copying the MAC from platform_data to the single 'macaddr'
variable will overwrite the MAC for the first interface in case of a
dual FEC setup.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index e2b5ce6..11534b9 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -818,7 +818,7 @@ static void __inline__ fec_get_mac(struct net_device *ndev)
iap = (unsigned char *)FEC_FLASHMAC;
#else
if (pdata)
- memcpy(iap, pdata->mac, ETH_ALEN);
+ iap = (unsigned char *)&pdata->mac;
#endif
}
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 6/8][NET] fec.c: preserve MII/RMII setting in fec_stop()
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
Additionally to setting the ETHER_EN bit in FEC_ECNTRL the MII/RMII
setting in FEC_R_CNTRL needs to be preserved to keep the MII interface
functional.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 11534b9..ab0afb5 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -515,6 +515,7 @@ fec_stop(struct net_device *ndev)
struct fec_enet_private *fep = netdev_priv(ndev);
const struct platform_device_id *id_entry =
platform_get_device_id(fep->pdev);
+ u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & (1 << 8);
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
@@ -531,8 +532,10 @@ fec_stop(struct net_device *ndev)
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
/* We have to keep ENET enabled to have MII interrupt stay working */
- if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
+ if (id_entry->driver_data & FEC_QUIRK_ENET_MAC) {
writel(2, fep->hwp + FEC_ECNTRL);
+ writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
+ }
}
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 7/8][NET] fec.c: fix the .remove code
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
The .remove code is broken in several ways.
- mdiobus_unregister() is called twice for the same object in case of dual FEC
- phy_disconnect() is being called when the PHY is already disconnected
- the requested IRQ(s) are not freed
- fec_stop() is being called with the inteface already stopped
All of those lead to kernel crashes if the remove function is actually used.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/fec.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index ab0afb5..01ee9cc 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -259,6 +259,8 @@ struct fec_enet_private {
/* Transmitter timeout */
#define TX_TIMEOUT (2 * HZ)
+static int mii_cnt;
+
static void *swap_buffer(void *bufaddr, int len)
{
int i;
@@ -1040,8 +1042,12 @@ static int fec_enet_mii_init(struct platform_device *pdev)
*/
if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id > 0) {
/* fec1 uses fec0 mii_bus */
- fep->mii_bus = fec0_mii_bus;
- return 0;
+ if (mii_cnt && fec0_mii_bus) {
+ fep->mii_bus = fec0_mii_bus;
+ mii_cnt++;
+ return 0;
+ }
+ return -ENOENT;
}
fep->mii_timeout = 0;
@@ -1086,6 +1092,8 @@ static int fec_enet_mii_init(struct platform_device *pdev)
if (mdiobus_register(fep->mii_bus))
goto err_out_free_mdio_irq;
+ mii_cnt++;
+
/* save fec0 mii_bus */
if (id_entry->driver_data & FEC_QUIRK_ENET_MAC)
fec0_mii_bus = fep->mii_bus;
@@ -1102,11 +1110,11 @@ err_out:
static void fec_enet_mii_remove(struct fec_enet_private *fep)
{
- if (fep->phy_dev)
- phy_disconnect(fep->phy_dev);
- mdiobus_unregister(fep->mii_bus);
- kfree(fep->mii_bus->irq);
- mdiobus_free(fep->mii_bus);
+ if (--mii_cnt == 0) {
+ mdiobus_unregister(fep->mii_bus);
+ kfree(fep->mii_bus->irq);
+ mdiobus_free(fep->mii_bus);
+ }
}
static int fec_enet_get_settings(struct net_device *ndev,
@@ -1646,13 +1654,18 @@ fec_drv_remove(struct platform_device *pdev)
struct net_device *ndev = platform_get_drvdata(pdev);
struct fec_enet_private *fep = netdev_priv(ndev);
struct resource *r;
+ int i;
- fec_stop(ndev);
+ unregister_netdev(ndev);
fec_enet_mii_remove(fep);
+ for (i = 0; i < FEC_IRQ_NUM; i++) {
+ int irq = platform_get_irq(pdev, i);
+ if (irq > 0)
+ free_irq(irq, ndev);
+ }
clk_disable(fep->clk);
clk_put(fep->clk);
iounmap(fep->hwp);
- unregister_netdev(ndev);
free_netdev(ndev);
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
--
1.5.6.5
^ permalink raw reply related
* [PATCH v2 8/8][NET] fec.c: make FEC driver buildable as module
From: Lothar Waßmann @ 2011-12-07 13:37 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, Shawn Guo, Lothar Waßmann
In-Reply-To: <cover.1323256319.git.LW@KARO-electronics.de>
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
drivers/net/ethernet/freescale/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index c520cfd..b02e315 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -21,7 +21,7 @@ config NET_VENDOR_FREESCALE
if NET_VENDOR_FREESCALE
config FEC
- bool "FEC ethernet controller (of ColdFire and some i.MX CPUs)"
+ tristate "FEC ethernet controller (of ColdFire and some i.MX CPUs)"
depends on (M523x || M527x || M5272 || M528x || M520x || M532x || \
ARCH_MXC || ARCH_MXS)
select PHYLIB
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH][NET] several cleanups and bugfixes for fec.c: preserve MII/RMII setting in fec_stop()
From: Shawn Guo @ 2011-12-07 13:37 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, Shawn Guo, linux-kernel, linux-arm-kernel
In-Reply-To: <20191.17172.493079.722098@ipc1.ka-ro>
On Wed, Dec 07, 2011 at 11:42:28AM +0100, Lothar Waßmann wrote:
> I'm using a TX28 which has the Ethernet PHY connected in RMII
> mode. You should see the problem, if you unplug the ethernet cable on
> a configured link. Without this patch the kernel will not detect when
> the cable is plugged back in.
>
Tested-by: Shawn Guo <shawn.guo@linaro.org>
--
Regards,
Shawn
^ permalink raw reply
* Re: [PATCH net-next v2 2/4] can: cc770: add legacy ISA bus driver for the CC770 and AN82527
From: Wolfgang Grandegger @ 2011-12-07 13:42 UTC (permalink / raw)
To: info-PyqsHJVlJN8AvxtiuMwx3w
Cc: Oliver Hartkopp, linux-can-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
socketcan-users-0fE9KPoRgkgATYTw5x5z8w
In-Reply-To: <4EDE8435.5080100-PyqsHJVlJN8AvxtiuMwx3w@public.gmane.org>
Hi Wolfgang,
On 12/06/2011 10:08 PM, Wolfgang Zarre wrote:
> Hello Wolfgang,
>
>> Hi Wolfgang,
>>
>> On 12/04/2011 07:47 PM, Wolfgang Zarre wrote:
...
>>>
>>> Should be not a problem at all.
>>
>> Great, thanks.
>
> Ok, here we go:
>
> uname -r: 3.2.0-rc4
>
> modprobe cc770_isa irq=0xa port=0x384 indirect=1 cir=0x61 bcr=0x4A
> ip link set can0 up type can bitrate 500000;
>
> kern.log
> Dec 6 20:42:19 svserv01 kernel: [ 2111.900735] CAN device driver interface
> Dec 6 20:42:19 svserv01 kernel: [ 2111.903072] cc770: CAN netdevice driver
> Dec 6 20:42:19 svserv01 kernel: [ 2111.904692] cc770_isa: platform
> device 0: port=0x384, mem=0x0, irq=10
> Dec 6 20:42:19 svserv01 kernel: [ 2111.904726] cc770_isa cc770_isa.0:
> probing idx=0: port=0x384, mem=0x0, irq=10
> Dec 6 20:42:19 svserv01 kernel: [ 2111.904779] cc770_isa cc770_isa.0:
> (unregistered net_device): i82527 mode with additional functions
> Dec 6 20:42:19 svserv01 kernel: [ 2111.906407] cc770_isa cc770_isa.0:
> device registered (reg_base=0x00000384, irq=10)
> Dec 6 20:42:19 svserv01 kernel: [ 2111.906457] cc770_isa: driver for
> max. 8 devices registered
> 6 20:44:17 svserv01 kernel: [ 2229.886845] cc770_isa cc770_isa.0:
> can0: setting BTR0=0x00 BTR1=0x1c
> Dec 6 20:44:17 svserv01 kernel: [ 2229.886920] cc770_isa cc770_isa.0:
> can0: Message object 15 for RX data, RTR, SFF and EFF
> Dec 6 20:44:17 svserv01 kernel: [ 2229.886937] cc770_isa cc770_isa.0:
> can0: Message object 11 for TX data, RTR, SFF and EFF
> Dec 6 20:52:40 svserv01 kernel: [ 2733.172845] can: controller area
> network core (rev 20090105 abi 8)
> Dec 6 20:52:40 svserv01 kernel: [ 2733.172967] NET: Registered protocol
> family 29
> Dec 6 20:52:40 svserv01 kernel: [ 2733.178187] can: raw protocol (rev
> 20090105)
>
>
> ip -d -s link show
> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> qlen 10
> link/can
> can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 500000 sample-point 0.875
> tq 125 prop-seg 6 phase-seg1 7 phase-seg2 2 sjw 1
> cc770: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0
> RX: bytes packets errors dropped overrun mcast
> 0 0 0 0 0 0
> TX: bytes packets errors dropped carrier collsns
> 0 0 0 0 0 0
>
>
> After rebooting the PLC with proprietary buggy kernel:
> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> qlen 10
> link/can
> can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 500000 sample-point 0.875
> tq 125 prop-seg 6 phase-seg1 7 phase-seg2 2 sjw 1
> cc770: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0
> RX: bytes packets errors dropped overrun mcast
> 414 267 0 267 0 0
> TX: bytes packets errors dropped carrier collsns
> 0 0 0 0 0 0
>
>
>
> After sending 100,000 PDO's with 13ms displacement:
> 4: can0: <NOARP,UP,LOWER_UP,ECHO> mtu 16 qdisc pfifo_fast state UNKNOWN
> qlen 10
> link/can
> can state ERROR-ACTIVE (berr-counter tx 0 rx 0) restart-ms 0
> bitrate 500000 sample-point 0.875
> tq 125 prop-seg 6 phase-seg1 7 phase-seg2 2 sjw 1
> cc770: tseg1 1..16 tseg2 1..8 sjw 1..4 brp 1..64 brp-inc 1
> clock 8000000
> re-started bus-errors arbit-lost error-warn error-pass bus-off
> 0 0 0 0 0 0
> RX: bytes packets errors dropped overrun mcast
> 4544 4284 0 331 0 0
> TX: bytes packets errors dropped carrier collsns
> 802660 202653 0 0 0 0
>
>
> The dropped ones of RX may be the not processed input packages and
> therefore ok.
>
> Let me know if You need more or some other tests.
You could provoke some state changes or bus-off conditions to see if the
berr-counter shows reasonable results. I'm currently consolidating and
unifying error state and bus-off handling. Would be nice if you could do
some further tests when I have the patches ready...
Thanks for testing.
Wolfgang.
^ permalink raw reply
* Re: [PATCH][NET] several cleanups and bugfixes for fec.c: fix the .remove code
From: Shawn Guo @ 2011-12-07 13:43 UTC (permalink / raw)
To: Lothar Waßmann; +Cc: netdev, Shawn Guo, linux-kernel, linux-arm-kernel
In-Reply-To: <20191.27018.478290.890636@ipc1.ka-ro>
On Wed, Dec 07, 2011 at 02:26:34PM +0100, Lothar Waßmann wrote:
> Hi,
>
> Shawn Guo writes:
> > On Tue, Dec 06, 2011 at 11:27:15AM +0100, Lothar Waßmann wrote:
> > > The .remove code is broken in several ways.
> > > - mdiobus_unregister() is called twice for the same object in case of dual FEC
> > > - phy_disconnect() is being called when the PHY is already disconnected
> > > - the requested IRQ(s) are not freed
> > > - fec_stop() is being called with the inteface already stopped
> > >
> > > All of those lead to kernel crashes if the remove function is actually used.
> > >
> > > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> > > ---
> > > drivers/net/ethernet/freescale/fec.c | 30 +++++++++++++++++++++---------
> > > 1 files changed, 21 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> > > index ab0afb5..70ec7ec 100644
> > > --- a/drivers/net/ethernet/freescale/fec.c
> > > +++ b/drivers/net/ethernet/freescale/fec.c
> > > @@ -235,6 +235,7 @@ struct fec_enet_private {
> > >
> > > /* Phylib and MDIO interface */
> > > struct mii_bus *mii_bus;
> > > + int mii_cnt;
> > > struct phy_device *phy_dev;
> > > int mii_timeout;
> > > uint phy_speed;
> > > @@ -1040,8 +1041,12 @@ static int fec_enet_mii_init(struct platform_device *pdev)
> > > */
> > > if ((id_entry->driver_data & FEC_QUIRK_ENET_MAC) && pdev->id > 0) {
> > > /* fec1 uses fec0 mii_bus */
> > > - fep->mii_bus = fec0_mii_bus;
> > > - return 0;
> > > + if (fep->mii_cnt && fec0_mii_bus) {
> >
> > This seems broken. The second fec has its own fep and fep->mii_cnt is
> > always 0 here.
> >
> I already noticed that myself.
>
With a quick fix here, the last two patches:
Tested-by: Shawn Guo <shawn.guo@linaro.org>
--
Regards,
Shawn
^ permalink raw reply
* [PATCH net-next 1/2] bnx2x: dont handle storage drv_info req if no cnic
From: Barak Witkowski @ 2011-12-07 13:45 UTC (permalink / raw)
To: davem, netdev; +Cc: Barak Witkowski, Eilon Greenstein
In-Reply-To: <Barak Witkowski <barak@broadcom.com>
This patch returns ACK with zeroed buffer to FW upon fcoe/iscsi drv_info
request if cnic is not defined. This is better handling in comparison to
send NACK to FW upon such request, as it's a valid request.
Reported-by: Joe Perches <joe@perches.com>
Signed-off-by: Barak Witkowski <barak@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 12 ++++++++----
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 418e7d3..d104695 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -2941,9 +2941,9 @@ static void bnx2x_drv_info_ether_stat(struct bnx2x *bp)
ether_stat->rxq_size = bp->rx_ring_size;
}
+#ifdef BCM_CNIC
static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp)
{
-#ifdef BCM_CNIC
struct bnx2x_dcbx_app_params *app = &bp->dcbx_port_params.app;
struct fcoe_stats_info *fcoe_stat =
&bp->slowpath->drv_info_to_mcp.fcoe_stat;
@@ -3029,12 +3029,12 @@ static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp)
/* ask L5 driver to add data to the struct */
bnx2x_cnic_notify(bp, CNIC_CTL_FCOE_STATS_GET_CMD);
-#endif
}
+#endif
+#ifdef BCM_CNIC
static void bnx2x_drv_info_iscsi_stat(struct bnx2x *bp)
{
-#ifdef BCM_CNIC
struct bnx2x_dcbx_app_params *app = &bp->dcbx_port_params.app;
struct iscsi_stats_info *iscsi_stat =
&bp->slowpath->drv_info_to_mcp.iscsi_stat;
@@ -3046,8 +3046,8 @@ static void bnx2x_drv_info_iscsi_stat(struct bnx2x *bp)
/* ask L5 driver to add data to the struct */
bnx2x_cnic_notify(bp, CNIC_CTL_ISCSI_STATS_GET_CMD);
-#endif
}
+#endif
/* called due to MCP event (on pmf):
* reread new bandwidth configuration
@@ -3091,10 +3091,14 @@ static void bnx2x_handle_drv_info_req(struct bnx2x *bp)
bnx2x_drv_info_ether_stat(bp);
break;
case FCOE_STATS_OPCODE:
+#ifdef BCM_CNIC
bnx2x_drv_info_fcoe_stat(bp);
+#endif
break;
case ISCSI_STATS_OPCODE:
+#ifdef BCM_CNIC
bnx2x_drv_info_iscsi_stat(bp);
+#endif
break;
default:
/* if op code isn't supported - send NACK */
--
1.7.7.4
^ permalink raw reply related
* [PATCH net-next 2/2] bnx2x: fix typo in fcoe stats collection
From: Barak Witkowski @ 2011-12-07 13:45 UTC (permalink / raw)
To: davem, netdev; +Cc: Barak Witkowski, Eilon Greenstein
In-Reply-To: <1323265536-9760-1-git-send-email-barak@broadcom.com>
Signed-off-by: Barak Witkowski <barak@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index d104695..12b8001 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -2994,7 +2994,7 @@ static void bnx2x_drv_info_fcoe_stat(struct bnx2x *bp)
fcoe_q_tstorm_stats->rcv_bcast_pkts);
ADD_64(fcoe_stat->rx_frames_hi, 0, fcoe_stat->rx_frames_lo,
- fcoe_q_tstorm_stats->rcv_ucast_pkts);
+ fcoe_q_tstorm_stats->rcv_mcast_pkts);
ADD_64(fcoe_stat->tx_bytes_hi, 0, fcoe_stat->tx_bytes_lo,
fw_fcoe_stat->tx_stat.fcoe_tx_byte_cnt);
--
1.7.7.4
^ permalink raw reply related
* Re: [PATCH v3 1/2] net/macb: add DT support for Cadence macb/gem driver
From: Nicolas Ferre @ 2011-12-07 13:49 UTC (permalink / raw)
To: netdev, David Miller
Cc: robherring2, devicetree-discuss, plagnioj, linux-arm-kernel,
grant.likely, linux-kernel, jamie
In-Reply-To: <714ca7492d8d45b50eab34448c4b70933ce5701c.1323086095.git.nicolas.ferre@atmel.com>
On 12/05/2011 12:59 PM, Nicolas Ferre :
> From: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
>
> Allow the device tree to provide the mac address and the phy mode.
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD<plagnioj@jcrosoft.com>
> [nicolas.ferre@atmel.com: change "compatible" node property, doc and DT hwaddr]
> Signed-off-by: Nicolas Ferre<nicolas.ferre@atmel.com>
> [jamie@jamieiles.com: add "gem" compatibility strings and doc]
> Acked-by: Jamie Iles<jamie@jamieiles.com>
David, can I have your acknowledgment on this patch so that I can send
it through arm-soc git tree? This way it will reside on top of previous
work by Jamie that is already in arm-soc git (for-next branch).
Thanks, best regards,
> ---
> v3: add "gem" compatibility strings
> v2: modify macb_get_hwaddr_dt() parameter
>
> Documentation/devicetree/bindings/net/macb.txt | 24 ++++++++
> drivers/net/ethernet/cadence/macb.c | 73 +++++++++++++++++++++---
> drivers/net/ethernet/cadence/macb.h | 2 +
> 3 files changed, 91 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/macb.txt
>
> diff --git a/Documentation/devicetree/bindings/net/macb.txt b/Documentation/devicetree/bindings/net/macb.txt
> new file mode 100644
> index 0000000..2e24f05
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/macb.txt
> @@ -0,0 +1,24 @@
> +* Cadence MACB/GEM Ethernet controller
> +
> +Required properties:
> +- compatible: Should be "cdns,[<chip>-]{macb|gem}"
> + Use "cdns,at91sam9260-macb" Atmel at91sam9260 and at91sam9263 SoCs.
> + Use "cdns,at32ap7000-macb" for other 10/100 usage or use the generic form: "cdns,macb".
> + Use "cnds,pc302-gem" for Picochip picoXcell pc302 and later devices based on
> + the Cadence GEM, or the generic form: "cdns,gem".
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain macb interrupt
> +- phy-mode: String, operation mode of the PHY interface.
> + Supported values are: "mii", "rmii", "gmii", "rgmii".
> +
> +Optional properties:
> +- local-mac-address: 6 bytes, mac address
> +
> +Examples:
> +
> + macb0: ethernet@fffc4000 {
> + compatible = "cdns,at32ap7000-macb";
> + reg =<0xfffc4000 0x4000>;
> + interrupts =<21>;
> + phy-mode = "rmii";
> + };
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 64d6146..baf1a0d 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -23,6 +23,8 @@
> #include<linux/platform_data/macb.h>
> #include<linux/platform_device.h>
> #include<linux/phy.h>
> +#include<linux/of_device.h>
> +#include<linux/of_net.h>
>
> #include "macb.h"
>
> @@ -191,7 +193,6 @@ static int macb_mii_probe(struct net_device *dev)
> {
> struct macb *bp = netdev_priv(dev);
> struct phy_device *phydev;
> - struct macb_platform_data *pdata;
> int ret;
>
> phydev = phy_find_first(bp->mii_bus);
> @@ -200,14 +201,11 @@ static int macb_mii_probe(struct net_device *dev)
> return -1;
> }
>
> - pdata = bp->pdev->dev.platform_data;
> /* TODO : add pin_irq */
>
> /* attach the mac to the phy */
> ret = phy_connect_direct(dev, phydev,&macb_handle_link_change, 0,
> - pdata&& pdata->is_rmii ?
> - PHY_INTERFACE_MODE_RMII :
> - PHY_INTERFACE_MODE_MII);
> + bp->phy_interface);
> if (ret) {
> netdev_err(dev, "Could not attach to PHY\n");
> return ret;
> @@ -1244,6 +1242,52 @@ static const struct net_device_ops macb_netdev_ops = {
> #endif
> };
>
> +#if defined(CONFIG_OF)
> +static const struct of_device_id macb_dt_ids[] = {
> + { .compatible = "cdns,at32ap7000-macb" },
> + { .compatible = "cdns,at91sam9260-macb" },
> + { .compatible = "cdns,macb" },
> + { .compatible = "cdns,pc302-gem" },
> + { .compatible = "cdns,gem" },
> + { /* sentinel */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, macb_dt_ids);
> +
> +static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> +
> + if (np)
> + return of_get_phy_mode(np);
> +
> + return -ENODEV;
> +}
> +
> +static int __devinit macb_get_hwaddr_dt(struct macb *bp)
> +{
> + struct device_node *np = bp->pdev->dev.of_node;
> + if (np) {
> + const char *mac = of_get_mac_address(np);
> + if (mac) {
> + memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
> + return 0;
> + }
> + }
> +
> + return -ENODEV;
> +}
> +#else
> +static int __devinit macb_get_phy_mode_dt(struct platform_device *pdev)
> +{
> + return -ENODEV;
> +}
> +static int __devinit macb_get_hwaddr_dt(struct macb *bp)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> static int __init macb_probe(struct platform_device *pdev)
> {
> struct macb_platform_data *pdata;
> @@ -1318,10 +1362,22 @@ static int __init macb_probe(struct platform_device *pdev)
> config |= macb_dbw(bp);
> macb_writel(bp, NCFGR, config);
>
> - macb_get_hwaddr(bp);
> - pdata = pdev->dev.platform_data;
> + err = macb_get_hwaddr_dt(bp);
> + if (err< 0)
> + macb_get_hwaddr(bp);
> +
> + err = macb_get_phy_mode_dt(pdev);
> + if (err< 0) {
> + pdata = pdev->dev.platform_data;
> + if (pdata&& pdata->is_rmii)
> + bp->phy_interface = PHY_INTERFACE_MODE_RMII;
> + else
> + bp->phy_interface = PHY_INTERFACE_MODE_MII;
> + } else {
> + bp->phy_interface = err;
> + }
>
> - if (pdata&& pdata->is_rmii)
> + if (bp->phy_interface == PHY_INTERFACE_MODE_RMII)
> #if defined(CONFIG_ARCH_AT91)
> macb_or_gem_writel(bp, USRIO, (MACB_BIT(RMII) |
> MACB_BIT(CLKEN)));
> @@ -1444,6 +1500,7 @@ static struct platform_driver macb_driver = {
> .driver = {
> .name = "macb",
> .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(macb_dt_ids),
> },
> };
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 1931078..335e288 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -532,6 +532,8 @@ struct macb {
> unsigned int link;
> unsigned int speed;
> unsigned int duplex;
> +
> + phy_interface_t phy_interface;
> };
>
> static inline bool macb_is_gem(struct macb *bp)
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: Fix compile errors if CONFIG_CNIC is not set
From: Eilon Greenstein @ 2011-12-07 13:53 UTC (permalink / raw)
To: Michael Chan, Joe Perches
Cc: Dmitry Kravkov, Eric Dumazet, davem@davemloft.net,
netdev@vger.kernel.org, Barak Witkowski
In-Reply-To: <1323209811.11998.3.camel@lb-tlvb-eilong.il.broadcom.com>
On Wed, 2011-12-07 at 00:16 +0200, Eilon Greenstein wrote:
> On Tue, 2011-12-06 at 13:42 -0800, Michael Chan wrote:
> > On Tue, 2011-12-06 at 13:25 -0800, Joe Perches wrote:
> > > On Tue, 2011-12-06 at 22:03 +0100, Eric Dumazet wrote:
> > > > Le mardi 06 décembre 2011 à 12:58 -0800, Michael Chan a écrit :
> > > > > Don't provide FCoE and iSCSI statistics to management firmware if
> > > > > CONFIG_CNIC is not set. Some needed structure fields are not defined
> > > > > without CONFIG_CNIC.
> > > > Thanks for the fast answer, and yes, no more build error :)
> > >
> > > That works, but is that the best solution?
> > >
> > > Another option is for bnx2x_handle_drv_info_req
> > > to return DRV_MSG_CODE_DRV_INFO_NACK
> > >
> >
> > Eilon (bnx2x lead maintainer) will need to decide which is the most
> > appropriate solution as he is most familiar with the firmware. I don't
> > know if sending NACK to the firmware will have other negative effects or
> > not. Eilon should be online in about 8 hours and he can send a
> > follow-up patch if necessary.
> >
> > > Maybe like:
> > >
> > > switch (op_code) {
> > > case ETH_STATS_OPCODE:
> > > bnx2x_drv_info_ether_stat(bp);
> > > break;
> > > #ifdef BCM_CNIC
> > > case FCOE_STATS_OPCODE:
> > > bnx2x_drv_info_fcoe_stat(bp);
> > > break;
> > > case ISCSI_STATS_OPCODE:
> > > bnx2x_drv_info_iscsi_stat(bp);
> > > break;
> > > #endif
> > > default:
> > > /* if op code isn't supported - send NACK */
> > > bnx2x_fw_command(bp, DRV_MSG_CODE_DRV_INFO_NACK, 0);
> > > return;
> > > }
> > >
> > >
> > >
> >
>
> Joe is right. This latest patch breaks the FW assumption, but at least
> everything compiles now, so it is not as urgent. We will send a patch in
> that spirit in the morning.
As you probably saw in the patch Barak just sent, after thinking about
it some more and testing the FW flows, it is best to return ACK without
updating the counters.
Eilon
^ permalink raw reply
* RE: Latency difference between fifo and pfifo_fast
From: David Laight @ 2011-12-07 14:08 UTC (permalink / raw)
To: Dave Taht, Eric Dumazet; +Cc: John A. Sullivan III, netdev, Rick Jones
In-Reply-To: <CAA93jw58s4DG7Ow+j-cCgtD4UsWRUPZ-pbmeF1J1cZcaxCDRsw@mail.gmail.com>
...
> If he's hovering at close to line rate (wow), and yet experiencing
> serious delays on short traffic, perhaps what I describe
> below may apply.
...
> 1) TCP grabs all the bandwidth it can. If you have no packet loss,
> it will eat more bandwidth, as rapidly as it can ramp up.
> Until it eventually has packet loss.
The 'ramp up' may be part of the problem!
At a guess iSCSI is using the TCP connection to carry
many, separate, commands and responses. As such Nagle
will cause serious grief and is likely to be disabled.
TCP 'slow start' will apply whenever there is no unacked
data - which might be after any slight lull in the traffic.
IIRC (from looking at traces) Linux TCP will only send 4 data
packets following 'slow start' until it has received an ack.
Linux (at least some versions) will also delay sending an
ack until the next clock tick - rather than the traditional
scheme of always acking every other packet.
So if there are no responses, the requests can be delayed.
This will increase latency.
My suspicions (as I've said before) is that slow start
is broken for very low latency local networks.
Might be worth disabling it - but that is a massive system-wide
switch.
Very high packet rates can cause packet loss, but buying better
network infrastructure should mitigate that. In any case, 'slow
start' doesn't limit packet rate.
David
^ permalink raw reply
* [Patch ] net: doc: cleanup Documentation/networking/scaling.txt
From: Shan Wei @ 2011-12-07 14:22 UTC (permalink / raw)
To: Randy Dunlap (maintainer:DOCUMENTATION), David Miller, willemb,
benjamin.poirier, jkosina, linux-doc,
Network Developer Mailing List, therbert
1) Fix some typos.
2) Change mode of the punctuation from full to half, eg.’,“ .
So that the punctuation can be read at console.
Signed-off-by: Shan Wei<shanwei88@gmail.com>
---
I feel uncertain when reading following contents that no variable
in rps_dev_flow_table or softnet_data records the length of
the current backlog. Just last_qtail variable pointers the tail of the backlog.
"The counter in rps_dev_flow_table values records the length of the current
CPU's backlog when a packet in this flow was last enqueued. "
If missing something, please correct me.
---
Documentation/networking/scaling.txt | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/Documentation/networking/scaling.txt b/Documentation/networking/scaling.txt
index a177de2..1215fcc 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -26,7 +26,7 @@ queues to distribute processing among CPUs. The NIC distributes packets by
applying a filter to each packet that assigns it to one of a small number
of logical flows. Packets for each flow are steered to a separate receive
queue, which in turn can be processed by separate CPUs. This mechanism is
-generally known as “Receive-side Scaling” (RSS). The goal of RSS and
+generally known as "Receive-side Scaling" (RSS). The goal of RSS and
the other scaling techniques is to increase performance uniformly.
Multi-queue distribution can also be used for traffic prioritization, but
that is not the focus of these techniques.
@@ -42,7 +42,7 @@ indirection table and reading the corresponding value.
Some advanced NICs allow steering packets to queues based on
programmable filters. For example, webserver bound TCP port 80 packets
-can be directed to their own receive queue. Such “n-tuple” filters can
+can be directed to their own receive queue. Such "n-tuple" filters can
be configured from ethtool (--config-ntuple).
==== RSS Configuration
@@ -104,7 +104,7 @@ RSS. Being in software, it is necessarily called later in the datapath.
Whereas RSS selects the queue and hence CPU that will run the hardware
interrupt handler, RPS selects the CPU to perform protocol processing
above the interrupt handler. This is accomplished by placing the packet
-on the desired CPU’s backlog queue and waking up the CPU for processing.
+on the desired CPU's backlog queue and waking up the CPU for processing.
RPS has some advantages over RSS: 1) it can be used with any NIC,
2) software filters can easily be added to hash over new protocols,
3) it does not increase hardware device interrupt rate (although it does
@@ -116,20 +116,20 @@ netif_receive_skb(). These call the get_rps_cpu() function, which
selects the queue that should process a packet.
The first step in determining the target CPU for RPS is to calculate a
-flow hash over the packet’s addresses or ports (2-tuple or 4-tuple hash
+flow hash over the packet's addresses or ports (2-tuple or 4-tuple hash
depending on the protocol). This serves as a consistent hash of the
associated flow of the packet. The hash is either provided by hardware
or will be computed in the stack. Capable hardware can pass the hash in
the receive descriptor for the packet; this would usually be the same
hash used for RSS (e.g. computed Toeplitz hash). The hash is saved in
skb->rx_hash and can be used elsewhere in the stack as a hash of the
-packet’s flow.
+packet's flow.
Each receive hardware queue has an associated list of CPUs to which
RPS may enqueue packets for processing. For each received packet,
an index into the list is computed from the flow hash modulo the size
of the list. The indexed CPU is the target for processing the packet,
-and the packet is queued to the tail of that CPU’s backlog queue. At
+and the packet is queued to the tail of that CPU's backlog queue. At
the end of the bottom half routine, IPIs are sent to any CPUs for which
packets have been queued to their backlog queue. The IPI wakes backlog
processing on the remote CPU, and any queued packets are then processed
@@ -208,7 +208,7 @@ The counter in rps_dev_flow_table values records the length of the current
CPU's backlog when a packet in this flow was last enqueued. Each backlog
queue has a head counter that is incremented on dequeue. A tail counter
is computed as head counter + queue length. In other words, the counter
-in rps_dev_flow_table[i] records the last element in flow i that has
+in rps_dev_flow[i] records the last element in flow i that has
been enqueued onto the currently designated CPU for flow i (of course,
entry i is actually selected by hash and multiple flows may hash to the
same entry i).
@@ -218,13 +218,13 @@ CPU for packet processing (from get_rps_cpu()) the rps_sock_flow table
and the rps_dev_flow table of the queue that the packet was received on
are compared. If the desired CPU for the flow (found in the
rps_sock_flow table) matches the current CPU (found in the rps_dev_flow
-table), the packet is enqueued onto that CPU’s backlog. If they differ,
+table), the packet is enqueued onto that CPU's backlog. If they differ,
the current CPU is updated to match the desired CPU if one of the
following is true:
- The current CPU's queue head counter >= the recorded tail counter
value in rps_dev_flow[i]
-- The current CPU is unset (equal to NR_CPUS)
+- The current CPU is unset (equal to RPS_NO_CPU)
- The current CPU is offline
After this check, the packet is sent to the (possibly updated) current
@@ -235,7 +235,7 @@ CPU.
==== RFS Configuration
-RFS is only available if the kconfig symbol CONFIG_RFS is enabled (on
+RFS is only available if the kconfig symbol CONFIG_RPS is enabled (on
by default for SMP). The functionality remains disabled until explicitly
configured. The number of entries in the global flow table is set through:
@@ -258,7 +258,7 @@ For a single queue device, the rps_flow_cnt value for the single queue
would normally be configured to the same value as rps_sock_flow_entries.
For a multi-queue device, the rps_flow_cnt for each queue might be
configured as rps_sock_flow_entries / N, where N is the number of
-queues. So for instance, if rps_flow_entries is set to 32768 and there
+queues. So for instance, if rps_sock_flow_entries is set to 32768 and there
are 16 configured receive queues, rps_flow_cnt for each queue might be
configured as 2048.
@@ -272,7 +272,7 @@ the application thread consuming the packets of each flow is running.
Accelerated RFS should perform better than RFS since packets are sent
directly to a CPU local to the thread consuming the data. The target CPU
will either be the same CPU where the application runs, or at least a CPU
-which is local to the application thread’s CPU in the cache hierarchy.
+which is local to the application thread's CPU in the cache hierarchy.
To enable accelerated RFS, the networking stack calls the
ndo_rx_flow_steer driver function to communicate the desired hardware
@@ -285,7 +285,7 @@ The hardware queue for a flow is derived from the CPU recorded in
rps_dev_flow_table. The stack consults a CPU to hardware queue map which
is maintained by the NIC driver. This is an auto-generated reverse map of
the IRQ affinity table shown by /proc/interrupts. Drivers can use
-functions in the cpu_rmap (“CPU affinity reverse map”) kernel library
+functions in the cpu_rmap ("CPU affinity reverse map") kernel library
to populate the map. For each CPU, the corresponding queue in the map is
set to be one whose processing CPU is closest in cache locality.
-- 1.7.1
^ permalink raw reply related
* [Patch V2 ] net: doc: cleanup Documentation/networking/scaling.txt
From: Shan Wei @ 2011-12-07 14:26 UTC (permalink / raw)
To: Randy Dunlap (maintainer:DOCUMENTATION), David Miller, willemb,
benjamin.poirier, 单卫, therbert
Cc: linux-doc, Network Developer Mailing List
1) Fix some typos.
2) Change mode of the punctuation from full to half, eg.’,“ .
So that the punctuation can be read at console.
Signed-off-by: Shan Wei<shanwei88@gmail.com>
---
v2: fix the broken in patchwork.
I feel uncertain when reading following contents that no variable
in rps_dev_flow_table or softnet_data records the length of the current
backlog. Just last_qtail variable pointers the tail of the backlog.
"The counter in rps_dev_flow_table values records the length of the current
CPU's backlog when a packet in this flow was last enqueued. "
If missing something, please correct me.
---
Documentation/networking/scaling.txt | 26 +++++++++++++-------------
1 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/Documentation/networking/scaling.txt
b/Documentation/networking/scaling.txt
index a177de2..1215fcc 100644
--- a/Documentation/networking/scaling.txt
+++ b/Documentation/networking/scaling.txt
@@ -26,7 +26,7 @@ queues to distribute processing among CPUs. The NIC
distributes packets by
applying a filter to each packet that assigns it to one of a small number
of logical flows. Packets for each flow are steered to a separate receive
queue, which in turn can be processed by separate CPUs. This mechanism is
-generally known as “Receive-side Scaling” (RSS). The goal of RSS and
+generally known as "Receive-side Scaling" (RSS). The goal of RSS and
the other scaling techniques is to increase performance uniformly.
Multi-queue distribution can also be used for traffic prioritization, but
that is not the focus of these techniques.
@@ -42,7 +42,7 @@ indirection table and reading the corresponding value.
Some advanced NICs allow steering packets to queues based on
programmable filters. For example, webserver bound TCP port 80 packets
-can be directed to their own receive queue. Such “n-tuple” filters can
+can be directed to their own receive queue. Such "n-tuple" filters can
be configured from ethtool (--config-ntuple).
==== RSS Configuration
@@ -104,7 +104,7 @@ RSS. Being in software, it is necessarily called
later in the datapath.
Whereas RSS selects the queue and hence CPU that will run the hardware
interrupt handler, RPS selects the CPU to perform protocol processing
above the interrupt handler. This is accomplished by placing the packet
-on the desired CPU’s backlog queue and waking up the CPU for processing.
+on the desired CPU's backlog queue and waking up the CPU for processing.
RPS has some advantages over RSS: 1) it can be used with any NIC,
2) software filters can easily be added to hash over new protocols,
3) it does not increase hardware device interrupt rate (although it does
@@ -116,20 +116,20 @@ netif_receive_skb(). These call the get_rps_cpu()
function, which
selects the queue that should process a packet.
The first step in determining the target CPU for RPS is to calculate a
-flow hash over the packet’s addresses or ports (2-tuple or 4-tuple hash
+flow hash over the packet's addresses or ports (2-tuple or 4-tuple hash
depending on the protocol). This serves as a consistent hash of the
associated flow of the packet. The hash is either provided by hardware
or will be computed in the stack. Capable hardware can pass the hash in
the receive descriptor for the packet; this would usually be the same
hash used for RSS (e.g. computed Toeplitz hash). The hash is saved in
skb->rx_hash and can be used elsewhere in the stack as a hash of the
-packet’s flow.
+packet's flow.
Each receive hardware queue has an associated list of CPUs to which
RPS may enqueue packets for processing. For each received packet,
an index into the list is computed from the flow hash modulo the size
of the list. The indexed CPU is the target for processing the packet,
-and the packet is queued to the tail of that CPU’s backlog queue. At
+and the packet is queued to the tail of that CPU's backlog queue. At
the end of the bottom half routine, IPIs are sent to any CPUs for which
packets have been queued to their backlog queue. The IPI wakes backlog
processing on the remote CPU, and any queued packets are then processed
@@ -208,7 +208,7 @@ The counter in rps_dev_flow_table values records the
length of the current
CPU's backlog when a packet in this flow was last enqueued. Each backlog
queue has a head counter that is incremented on dequeue. A tail counter
is computed as head counter + queue length. In other words, the counter
-in rps_dev_flow_table[i] records the last element in flow i that has
+in rps_dev_flow[i] records the last element in flow i that has
been enqueued onto the currently designated CPU for flow i (of course,
entry i is actually selected by hash and multiple flows may hash to the
same entry i).
@@ -218,13 +218,13 @@ CPU for packet processing (from get_rps_cpu()) the
rps_sock_flow table
and the rps_dev_flow table of the queue that the packet was received on
are compared. If the desired CPU for the flow (found in the
rps_sock_flow table) matches the current CPU (found in the rps_dev_flow
-table), the packet is enqueued onto that CPU’s backlog. If they differ,
+table), the packet is enqueued onto that CPU's backlog. If they differ,
the current CPU is updated to match the desired CPU if one of the
following is true:
- The current CPU's queue head counter >= the recorded tail counter
value in rps_dev_flow[i]
-- The current CPU is unset (equal to NR_CPUS)
+- The current CPU is unset (equal to RPS_NO_CPU)
- The current CPU is offline
After this check, the packet is sent to the (possibly updated) current
@@ -235,7 +235,7 @@ CPU.
==== RFS Configuration
-RFS is only available if the kconfig symbol CONFIG_RFS is enabled (on
+RFS is only available if the kconfig symbol CONFIG_RPS is enabled (on
by default for SMP). The functionality remains disabled until explicitly
configured. The number of entries in the global flow table is set through:
@@ -258,7 +258,7 @@ For a single queue device, the rps_flow_cnt value
for the single queue
would normally be configured to the same value as rps_sock_flow_entries.
For a multi-queue device, the rps_flow_cnt for each queue might be
configured as rps_sock_flow_entries / N, where N is the number of
-queues. So for instance, if rps_flow_entries is set to 32768 and there
+queues. So for instance, if rps_sock_flow_entries is set to 32768 and there
are 16 configured receive queues, rps_flow_cnt for each queue might be
configured as 2048.
@@ -272,7 +272,7 @@ the application thread consuming the packets of
each flow is running.
Accelerated RFS should perform better than RFS since packets are sent
directly to a CPU local to the thread consuming the data. The target CPU
will either be the same CPU where the application runs, or at least a CPU
-which is local to the application thread’s CPU in the cache hierarchy.
+which is local to the application thread's CPU in the cache hierarchy.
To enable accelerated RFS, the networking stack calls the
ndo_rx_flow_steer driver function to communicate the desired hardware
@@ -285,7 +285,7 @@ The hardware queue for a flow is derived from the
CPU recorded in
rps_dev_flow_table. The stack consults a CPU to hardware queue map which
is maintained by the NIC driver. This is an auto-generated reverse map of
the IRQ affinity table shown by /proc/interrupts. Drivers can use
-functions in the cpu_rmap (“CPU affinity reverse map”) kernel library
+functions in the cpu_rmap ("CPU affinity reverse map") kernel library
to populate the map. For each CPU, the corresponding queue in the map is
set to be one whose processing CPU is closest in cache locality.
^ permalink raw reply related
* Re: WARNING: at mm/slub.c:3357, kernel BUG at mm/slub.c:3413
From: Robert Richter @ 2011-12-07 14:32 UTC (permalink / raw)
To: Markus Trippelsdorf
Cc: Jerome Glisse, Christoph Lameter, Alex, Shi, Dave Airlie,
Eric Dumazet, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel, Pekka Enberg,
linux-mm@kvack.org, Matt Mackall, tj, Alex Deucher
In-Reply-To: <20111202204820.GB1603@x4.trippels.de>
On 02.12.11 21:48:20, Markus Trippelsdorf wrote:
> BTW I always see (mostly only on screen, sometimes in the logs):
>
> [Firmware Bug]: cpu 2, try to use APIC500 (LVT offset 0) for vector 0x10400, but the register is already in use for vector 0xf9 on another cpu
> [Firmware Bug]: cpu 2, IBS interrupt offset 0 not available (MSRC001103A=0x0000000000000100)
> [Firmware Bug]: using offset 1 for IBS interrupts
> [Firmware Bug]: workaround enabled for IBS LVT offset
> perf: AMD IBS detected (0x0000001f)
>
> But I hope that it is only a harmless warning.
> (perf Instruction-Based Sampling)
Yes, the message always apears on AMD family 10h. Nothing to worry
about.
A patch is on the way to soften the message to not scare the people:
http://git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=16e5294e5f8303756a179cf218e37dfb9ed34417
-Robert
--
Advanced Micro Devices, Inc.
Operating System Research Center
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: WARNING: at mm/slub.c:3357, kernel BUG at mm/slub.c:3413
From: Markus Trippelsdorf @ 2011-12-07 14:39 UTC (permalink / raw)
To: Robert Richter
Cc: Jerome Glisse, Christoph Lameter, Alex, Shi, Dave Airlie,
Eric Dumazet, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dri-devel, Pekka Enberg,
linux-mm@kvack.org, Matt Mackall, tj, Alex Deucher
In-Reply-To: <20111207143223.GX15738@erda.amd.com>
On 2011.12.07 at 15:32 +0100, Robert Richter wrote:
> On 02.12.11 21:48:20, Markus Trippelsdorf wrote:
> > BTW I always see (mostly only on screen, sometimes in the logs):
> >
> > [Firmware Bug]: cpu 2, try to use APIC500 (LVT offset 0) for vector 0x10400, but the register is already in use for vector 0xf9 on another cpu
> > [Firmware Bug]: cpu 2, IBS interrupt offset 0 not available (MSRC001103A=0x0000000000000100)
> > [Firmware Bug]: using offset 1 for IBS interrupts
> > [Firmware Bug]: workaround enabled for IBS LVT offset
> > perf: AMD IBS detected (0x0000001f)
> >
> > But I hope that it is only a harmless warning.
> > (perf Instruction-Based Sampling)
>
> Yes, the message always apears on AMD family 10h. Nothing to worry
> about.
>
> A patch is on the way to soften the message to not scare the people:
>
> http://git.kernel.org/?p=linux/kernel/git/tip/tip.git;a=commit;h=16e5294e5f8303756a179cf218e37dfb9ed34417
Thanks.
It's already in mainline and the message is gone now.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commit;h=16e5294e5f8303756a179cf218e37dfb9ed34417
--
Markus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [net-next RFC PATCH 5/5] virtio-net: flow director support
From: Stefan Hajnoczi @ 2011-12-07 15:04 UTC (permalink / raw)
To: Jason Wang
Cc: krkumar2, kvm, mst, netdev, rusty, virtualization, levinsasha928,
bhutchings
In-Reply-To: <4EDF57C7.90406@redhat.com>
On Wed, Dec 7, 2011 at 12:10 PM, Jason Wang <jasowang@redhat.com> wrote:
> On 12/07/2011 05:08 PM, Stefan Hajnoczi wrote:
> [...]
>
>>> > Consider the complexity of the host nic each with their own steering
>>> > features, this series make the first step with minimal effort to try
>>> > to let
>>> > guest driver and host tap/macvtap co-operate like what physical nic
>>> > does.
>>> > There may be other method, but performance numbers is also needed to
>>> > give
>>> > the answer.
>>
>> I agree that performance results for this need to be shown.
>>
>> My original point is really that it's not a good idea to take
>> individual steps without a good big picture because this will change
>> the virtio-net device specification. If this turns out to be a dead
>> end then hosts will need to continue to support the interface forever
>> (legacy guests could still try to use it). So please first explain
>> what the full stack picture is going to look like and how you think it
>> will lead to better performance. You don't need to have all the code
>> or evidence, but just enough explanation so we see where this is all
>> going.
>
> I think I mention too little in the cover message.
>
> There's no much changes with Krishna's series except the method that
> choosing a rx virtqueue. Since original series use different hash methods in
> host (rxhash) and guest (txhash), a different virtqueue were chose for a
> flow which could lead packets of a flow to be handled by different vhost
> thread and vcpu. This may damage the performance.
>
> This series tries to let one vhost thread to process the packets of a flow
> and also let the packets to be sent directly to a vcpu local to the thread
> process the data. This is done by letting guest tell the desired queue form
> which it want to receive the pakcet of a dedicated flow.
>
> So passing the hash from host to guest is needed to get the same hash in the
> two sides. Then a guest programmable hash to queue table were introduced and
> guest co-operate with the host through accelerate RFS in guest.
Thanks for the context, that helps me understand it a little better.
Stefan
^ permalink raw reply
* [PATCH RFC] virtio_net: fix refill related races
From: Michael S. Tsirkin @ 2011-12-07 15:21 UTC (permalink / raw)
To: Amit Shah; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
Fix theoretical races related to refill work:
1. After napi is disabled by ndo_stop, refill work
can run and re-enable it.
2. Refill can reschedule itself, if this happens
it can run after cancel_delayed_work_sync,
and will access device after it is destroyed.
As a solution, add flags to track napi state and
to disable refill, and toggle them on start, stop
and remove; check these flags on refill.
refill work structure and new fields aren't used
on data path, so put them together near the end of
struct virtnet_info.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
RFC only since it's untested at this point.
A bugfix so 3.2 material?
Comments?
drivers/net/virtio_net.c | 57 +++++++++++++++++++++++++++++++++++++---------
1 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f5b3f19..39eb24c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -21,6 +21,7 @@
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/virtio.h>
#include <linux/virtio_net.h>
#include <linux/scatterlist.h>
@@ -68,15 +69,24 @@ struct virtnet_info {
/* Active statistics */
struct virtnet_stats __percpu *stats;
- /* Work struct for refilling if we run low on memory. */
- struct delayed_work refill;
-
/* Chain pages by the private ptr. */
struct page *pages;
/* fragments + linear part + virtio header */
struct scatterlist rx_sg[MAX_SKB_FRAGS + 2];
struct scatterlist tx_sg[MAX_SKB_FRAGS + 2];
+
+ /* Work struct for refilling if we run low on memory. */
+ struct delayed_work refill;
+
+ /* Set flag to allow delayed refill work, protected by a refill_lock. */
+ bool refill_enable;
+
+ /* Whether napi is enabled, protected by a refill_lock. */
+ bool napi_enable;
+
+ /* Lock to protect refill and napi enable/disable operations. */
+ struct mutex refill_lock;
};
struct skb_vnet_hdr {
@@ -477,20 +487,35 @@ static void virtnet_napi_enable(struct virtnet_info *vi)
}
}
+static void virtnet_refill_enable(struct virtnet_info *vi, bool enable)
+{
+ mutex_lock(&vi->refill_lock);
+ vi->refill_enable = enable;
+ mutex_unlock(&vi->refill_lock);
+}
+
static void refill_work(struct work_struct *work)
{
struct virtnet_info *vi;
bool still_empty;
vi = container_of(work, struct virtnet_info, refill.work);
- napi_disable(&vi->napi);
- still_empty = !try_fill_recv(vi, GFP_KERNEL);
- virtnet_napi_enable(vi);
- /* In theory, this can happen: if we don't get any buffers in
- * we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(&vi->refill, HZ/2);
+ mutex_lock(&vi->refill_lock);
+ if (vi->refill_enable) {
+ if (vi->napi_enable)
+ napi_disable(&vi->napi);
+ still_empty = !try_fill_recv(vi, GFP_KERNEL);
+ if (vi->napi_enable)
+ virtnet_napi_enable(vi);
+
+ /* In theory, this can happen: if we don't get any buffers in
+ * we will *never* try to fill again. */
+ if (still_empty)
+ schedule_delayed_work(&vi->refill, HZ/2);
+
+ }
+ mutex_unlock(&vi->refill_lock);
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -708,7 +733,10 @@ static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
+ mutex_lock(&vi->refill_lock);
+ vi->napi_enable = true;
virtnet_napi_enable(vi);
+ mutex_unlock(&vi->refill_lock);
return 0;
}
@@ -761,7 +789,10 @@ static int virtnet_close(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
+ mutex_lock(&vi->refill_lock);
+ vi->napi_enable = false;
napi_disable(&vi->napi);
+ mutex_unlock(&vi->refill_lock);
return 0;
}
@@ -1010,6 +1041,7 @@ static int virtnet_probe(struct virtio_device *vdev)
if (vi->stats == NULL)
goto free;
+ mutex_init(&vi->refill_lock);
INIT_DELAYED_WORK(&vi->refill, refill_work);
sg_init_table(vi->rx_sg, ARRAY_SIZE(vi->rx_sg));
sg_init_table(vi->tx_sg, ARRAY_SIZE(vi->tx_sg));
@@ -1047,6 +1079,8 @@ static int virtnet_probe(struct virtio_device *vdev)
goto free_vqs;
}
+ virtnet_refill_enable(vi, true);
+
/* Last of all, set up some receive buffers. */
try_fill_recv(vi, GFP_KERNEL);
@@ -1107,10 +1141,11 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev->priv;
+ virtnet_refill_enable(vi, false);
+
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
-
unregister_netdev(vi->dev);
cancel_delayed_work_sync(&vi->refill);
--
1.7.5.53.gc233e
^ permalink raw reply related
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