* Re: [PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
From: Naresh Kumar Inna @ 2012-08-24 19:04 UTC (permalink / raw)
To: David Miller
Cc: JBottomley@parallels.com, linux-scsi@vger.kernel.org,
Dimitrios Michailidis, netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <20120824.130406.1059160453285824849.davem@davemloft.net>
On 8/24/2012 10:34 PM, David Miller wrote:
> From: Naresh Kumar Inna <naresh@chelsio.com>
> Date: Fri, 24 Aug 2012 03:57:45 +0530
>
>> This is the initial submission of the Chelsio FCoE offload driver (csiostor)
>> to the upstream kernel. This driver currently supports FCoE offload
>> functionality over Chelsio T4-based 10Gb Converged Network Adapters.
>>
>> The following patches contain the driver sources for csiostor driver and
>> updates to firmware/hardware header files shared between csiostor and
>> cxgb4 (Chelsio T4-based NIC driver). The csiostor driver is dependent on these
>> header updates. These patches have been generated against scsi 'misc' branch.
>>
>> csiostor is a low level SCSI driver that interfaces with PCI, SCSI midlayer and
>> FC transport subsystems. This driver claims the FCoE PCIe function on the
>> Chelsio Converged Network Adapter. It relies on firmware events for slow path
>> operations like discovery, thereby offloading session management. The driver
>> programs firmware via Work Request interfaces for fast path I/O offload
>> features.
>
> You are going to have to get rid of these module parameters.
>
> That have to do with things that are in no way specific to your device,
> and therefore should be configured using generic kernel facilities.
>
> Using driver specific module parameters results in a poor user
> experience, because in order to make a configuration change the user
> has to know exactly what kind of device and driver is underneath,
> and then learn what the unique method is to make that configuration
> change.
>
> If you use a generic facility, the user only needs to learn one way to
> make a configuration change, regardless of device type and driver.
>
Hi Dave,
Thanks for reviewing. Is your comment with respect to any particular
module parameter[s] in this driver or all of them?
Regards,
Naresh.
^ permalink raw reply
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: David Miller @ 2012-08-24 19:07 UTC (permalink / raw)
To: swarren; +Cc: timur, afleming, ddaney.cavm, netdev, devicetree-discuss,
scottwood
In-Reply-To: <5037CE45.4010309@wwwdotorg.org>
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Fri, 24 Aug 2012 12:56:05 -0600
> In the I2C case, the address spaces are disjoint, so there's never any
> mapping between them, so there's no need for ranges.
>
> Any time the child address space is intended to be part of the parent's
> address space, I believe ranges is supposed to be specified, perhaps
> even mandatory, even if the translation is 1:1.
Regardless, you really can't just generically translate ranges
in some universal way and expect it to work in all cases.
You need bus specific drivers to deal with various special
cases.
See the *_map() methods implemented in:
arch/sparc/kernel/of_device_64.c
for example.
^ permalink raw reply
* Re: [PATCH 5/8] csiostor: Chelsio FCoE offload driver submission (sources part 5).
From: Naresh Kumar Inna @ 2012-08-24 19:07 UTC (permalink / raw)
To: Joe Perches
Cc: Nicholas A. Bellinger, JBottomley@parallels.com,
linux-scsi@vger.kernel.org, Dimitrios Michailidis,
netdev@vger.kernel.org, Chethan Seshadri
In-Reply-To: <1345832854.32359.11.camel@joe2Laptop>
On 8/24/2012 11:57 PM, Joe Perches wrote:
> On Fri, 2012-08-24 at 23:10 +0530, Naresh Kumar Inna wrote:
>> Is there a TRUE/FALSE define in a standard header file?
>
> include/linux/stddef.h:
> enum {
> false = 0,
> true = 1
> };
>
>> I see a lot of
>> kernel code defining their own TRUE/FALSE.
>
> Those should be changed eventually.
>
Thanks! I will switch over to these macros.
^ permalink raw reply
* Re: [PATCH 0/8] csiostor: Chelsio FCoE offload driver submission
From: David Miller @ 2012-08-24 19:10 UTC (permalink / raw)
To: naresh; +Cc: JBottomley, linux-scsi, dm, netdev, chethan
In-Reply-To: <5037D043.7000302@chelsio.com>
From: Naresh Kumar Inna <naresh@chelsio.com>
Date: Sat, 25 Aug 2012 00:34:35 +0530
> Thanks for reviewing. Is your comment with respect to any particular
> module parameter[s] in this driver or all of them?
All of them.
^ permalink raw reply
* [PATCH] [v3] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Timur Tabi @ 2012-08-24 19:10 UTC (permalink / raw)
To: Andy Fleming, David Miller, swarren-3lzwWm7+Weoh9ZMKESR00Q,
ddaney.cavm-Re5JQEeQqe8AvxtiuMwx3w,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
netdev-u79uwXL29TY76Z2rM5mHXA
Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
device, like an FPGA. The device must be memory-mapped and contain only
8-bit registers (which keeps things simple).
Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
to the localbus.
Signed-off-by: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
.../devicetree/bindings/net/mdio-mux-mmioreg.txt | 77 +++++++++
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-mux-mmioreg.c | 170 ++++++++++++++++++++
4 files changed, 261 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
create mode 100644 drivers/net/phy/mdio-mux-mmioreg.c
diff --git a/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
new file mode 100644
index 0000000..aff8fd9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
@@ -0,0 +1,77 @@
+Properties for an MDIO bus multiplexer controlled by a memory-mapped device
+
+This is a special case of a MDIO bus multiplexer. A memory-mapped device,
+like an FPGA, is used to control which child bus is connected. The mdio-mux
+node must be a child of the memory-mapped device. The driver currently only
+supports devices with eight-bit registers.
+
+Required properties in addition to the generic multiplexer properties:
+
+- compatible : string, must contain "mdio-mux-mmioreg"
+
+- reg : integer, contains the offset of the register that controls the bus
+ multiplexer. The size field in the 'reg' property is the size of
+ register, and must therefore be 1.
+
+- mux-mask : integer, contains an eight-bit mask that specifies which
+ bits in the register control the actual bus multiplexer. The
+ 'reg' property of each child mdio-mux node must be constrained by
+ this mask.
+
+Example:
+
+The FPGA node defines a memory-mapped FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 9 (BRDCFG1) controls the mux on that bus.
+A bitmask of 0x6 means that bits 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG1 that control the actual mux.
+
+ /* The FPGA node */
+ fpga: board-control@3,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+ reg = <3 0 0x30>;
+ ranges = <0 3 0 0x30>;
+
+ mdio-mux-emi2 {
+ compatible = "mdio-mux-mmioreg", "mdio-mux";
+ mdio-parent-bus = <&xmdio0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <9 1>; // BRDCFG1
+ mux-mask = <0x6>; // EMI2
+
+ emi2_slot1: mdio@0 { // Slot 1 XAUI (FM2)
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot1: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <4>;
+ };
+ };
+
+ emi2_slot2: mdio@2 { // Slot 2 XAUI (FM1)
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot2: ethernet-phy@4 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <0>;
+ };
+ };
+ };
+ };
+
+ /* The parent MDIO bus. */
+ xmdio0: mdio@f1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,fman-xmdio";
+ reg = <0xf1000 0x1000>;
+ interrupts = <100 1 0 0>;
+ };
+
+
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..0268edd 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,6 +159,19 @@ config MDIO_BUS_MUX_GPIO
several child MDIO busses to a parent bus. Child bus
selection is under the control of GPIO lines.
+config MDIO_BUS_MUX_MMIOREG
+ tristate "Support for MMIO device-controlled MDIO bus multiplexers"
+ depends on OF_MDIO
+ select MDIO_BUS_MUX
+ help
+ This module provides a driver for MDIO bus multiplexers that
+ are controlled via a simple memory-mapped device, like an FPGA.
+ The multiplexer connects one of several child MDIO busses to a
+ parent bus. Child bus selection is under the control of one of
+ the FPGA's registers.
+
+ Currently, only 8-bit registers are supported.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..426674d 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
+obj-$(CONFIG_MDIO_BUS_MUX_MMIOREG) += mdio-mux-mmioreg.o
diff --git a/drivers/net/phy/mdio-mux-mmioreg.c b/drivers/net/phy/mdio-mux-mmioreg.c
new file mode 100644
index 0000000..098239a
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-mmioreg.c
@@ -0,0 +1,170 @@
+/*
+ * Simple memory-mapped device MDIO MUX driver
+ *
+ * Author: Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/phy.h>
+#include <linux/mdio-mux.h>
+
+struct mdio_mux_mmioreg_state {
+ void *mux_handle;
+ phys_addr_t phys;
+ uint8_t mask;
+};
+
+/*
+ * MDIO multiplexing switch function
+ *
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ *
+ * 'current_child' is the current value of the mux register (masked via
+ * s->mask).
+ *
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ *
+ * The first time this function is called, current_child == -1.
+ *
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ */
+static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
+ void *data)
+{
+ struct mdio_mux_mmioreg_state *s = data;
+
+ if (current_child ^ desired_child) {
+ void *p = ioremap(s->phys, 1);
+ uint8_t x, y;
+
+ if (!p)
+ return -ENOMEM;
+
+ x = ioread8(p);
+ y = (x & ~s->mask) | desired_child;
+ if (x != y) {
+ iowrite8((x & ~s->mask) | desired_child, p);
+ pr_debug("%s: %02x -> %02x\n", __func__, x, y);
+ }
+
+ iounmap(p);
+ }
+
+ return 0;
+}
+
+static int __devinit mdio_mux_mmioreg_probe(struct platform_device *pdev)
+{
+ struct device_node *np2, *np = pdev->dev.of_node;
+ struct mdio_mux_mmioreg_state *s;
+ struct resource res;
+ const __be32 *iprop;
+ int len, ret;
+
+ dev_dbg(&pdev->dev, "probing node %s\n", np->full_name);
+
+ s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ ret = of_address_to_resource(np, 0, &res);
+ if (ret) {
+ dev_err(&pdev->dev, "could not obtain memory map for node %s\n",
+ np->full_name);
+ return ret;
+ }
+ s->phys = res.start;
+
+ if (resource_size(&res) != sizeof(uint8_t)) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+
+ iprop = of_get_property(np, "mux-mask", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing or invalid mux-mask property\n");
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) > 255) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+ s->mask = be32_to_cpup(iprop);
+
+ /*
+ * Verify that the 'reg' property of each child MDIO bus does not
+ * set any bits outside of the 'mask'.
+ */
+ for_each_available_child_of_node(np, np2) {
+ iprop = of_get_property(np2, "reg", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "mdio-mux child node %s is "
+ "missing a 'reg' property\n", np2->full_name);
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) & ~s->mask) {
+ dev_err(&pdev->dev, "mdio-mux child node %s has "
+ "a 'reg' value with unmasked bits\n",
+ np2->full_name);
+ return -ENODEV;
+ }
+ }
+
+ ret = mdio_mux_init(&pdev->dev, mdio_mux_mmioreg_switch_fn,
+ &s->mux_handle, s);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register mdio-mux bus %s\n",
+ np->full_name);
+ return ret;
+ }
+
+ pdev->dev.platform_data = s;
+
+ return 0;
+}
+
+static int __devexit mdio_mux_mmioreg_remove(struct platform_device *pdev)
+{
+ struct mdio_mux_mmioreg_state *s = dev_get_platdata(&pdev->dev);
+
+ mdio_mux_uninit(s->mux_handle);
+
+ return 0;
+}
+
+static struct of_device_id mdio_mux_mmioreg_match[] = {
+ {
+ .compatible = "mdio-mux-mmioreg",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_mmioreg_match);
+
+static struct platform_driver mdio_mux_mmioreg_driver = {
+ .driver = {
+ .name = "mdio-mux-mmioreg",
+ .owner = THIS_MODULE,
+ .of_match_table = mdio_mux_mmioreg_match,
+ },
+ .probe = mdio_mux_mmioreg_probe,
+ .remove = __devexit_p(mdio_mux_mmioreg_remove),
+};
+
+module_platform_driver(mdio_mux_mmioreg_driver);
+
+MODULE_AUTHOR("Timur Tabi <timur-KZfg59tc24xl57MIdRCFDg@public.gmane.org>");
+MODULE_DESCRIPTION("Memory-mapped device MDIO MUX driver");
+MODULE_LICENSE("GPL v2");
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] [v2] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Scott Wood @ 2012-08-24 19:18 UTC (permalink / raw)
To: David Miller
Cc: swarren, timur, afleming, ddaney.cavm, netdev, devicetree-discuss
In-Reply-To: <20120824.150740.9047511945034908.davem@davemloft.net>
On 08/24/2012 02:07 PM, David Miller wrote:
> From: Stephen Warren <swarren@wwwdotorg.org>
> Date: Fri, 24 Aug 2012 12:56:05 -0600
>
>> In the I2C case, the address spaces are disjoint, so there's never any
>> mapping between them, so there's no need for ranges.
>>
>> Any time the child address space is intended to be part of the parent's
>> address space, I believe ranges is supposed to be specified, perhaps
>> even mandatory, even if the translation is 1:1.
Yes, it's mandatory (even if the kernel lets you get away without it for
the sake of some broken Apple firmware, IIRC). If the translation is an
identity map you can use an empty "ranges;".
> Regardless, you really can't just generically translate ranges
> in some universal way and expect it to work in all cases.
>
> You need bus specific drivers to deal with various special
> cases.
>
> See the *_map() methods implemented in:
>
> arch/sparc/kernel/of_device_64.c
>
> for example.
We don't expect it to work in all cases. We expect it to work if the
bus node is on the whitelist for which we create devices on
platform_bus, there's a platform driver that binds against it, and that
driver calls of_iomap() or equivalent because the binding says that reg
refers to something that is memory mapped.
-Scott
^ permalink raw reply
* Re: pull request: wireless 2012-08-24
From: David Miller @ 2012-08-24 19:18 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120824161330.GC27147@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 24 Aug 2012 12:13:31 -0400
> This batch of fixes is intended for 3.6...
>
> Johannes Berg gives us a pair of iwlwifi fixes. One corrects some
> improperly defined ifdefs that lead to crashes and BUG_ONs. The other
> prevents attempts to read SRAM for devices that aren't actually started.
>
> Julia Lawall provides an ipw2100 fix to properly set the return code
> from a function call before testing it! :-)
>
> Thomas Huehn corrects the improper use of a constant related to a power
> setting in ath5k.
>
> Thomas Pedersen offers a mac80211 fix to properly handle destination
> addresses of unicast frames passing though a mesh gate.
>
> Vladimir Zapolskiy provides a brcmsmac fix to properly mark the
> interface state when the device goes down.
Pulled, thanks a lot John.
^ permalink raw reply
* Re: pull request: wireless-next 2012-08-24
From: David Miller @ 2012-08-24 19:18 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev
In-Reply-To: <20120824173138.GD27147@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 24 Aug 2012 13:31:38 -0400
> This is a batch of updates intended for 3.7. The bulk of it is
> mac80211 changes, including some mesh work from Thomas Pederson and
> some multi-channel work from Johannes. A variety of driver updates
> and other bits are scattered in there as well.
Pulled, thanks!
^ permalink raw reply
* Re: pull-request: can 2012-08-24
From: David Miller @ 2012-08-24 19:21 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can
In-Reply-To: <1345800643-29456-1-git-send-email-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri, 24 Aug 2012 11:30:41 +0200
> here are two fixes for the v3.6 release cycle. Alexey Khoroshilov submitted a
> fix for a memory leak in the softing driver (in softing_load_fw()) in case a
> krealloc() fails. Sven Schmitt fixed the misuse of the IRQF_SHARED flag in the
> irq resouce of the sja1000 platform driver, now the correct flag is used. There
> are no mainline users of this feature which need to be converted.
Pulled, thanks Marc.
^ permalink raw reply
* RE: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Keller, Jacob E @ 2012-08-24 19:38 UTC (permalink / raw)
To: Ben Hutchings, Richard Cochran
Cc: Vick, Matthew, Kirsher, Jeffrey T, davem@davemloft.net,
netdev@vger.kernel.org, gospo@redhat.com, sassmann@redhat.com
In-Reply-To: <1345827088.2694.11.camel@bwh-desktop.uk.solarflarecom.com>
> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, August 24, 2012 9:51 AM
> To: Richard Cochran
> Cc: Vick, Matthew; Keller, Jacob E; Kirsher, Jeffrey T;
> davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com;
> sassmann@redhat.com
> Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
>
> On Fri, 2012-08-24 at 12:10 +0200, Richard Cochran wrote:
> > On Thu, Aug 23, 2012 at 06:40:25PM +0000, Vick, Matthew wrote:
> > >
> > > I tend to agree with Jake here--I like having the information. I'm
> fine removing them, but I'd like to do it for all CONFIG_IGB_PTP wrapping
> if we're going to do it. What do you think, Richard?
> >
> > Come to think of it, I never liked the CONFIG_IGB_PTP very much in the
> > first place. These were added after the fact by Jeff Kirsher. He had
> > said off list that there was some issue with CONFIG_PTP_1588_CLOCK and
> > igb as a module, or something like that. At that time I said, just go
> > ahead and fix it up.
>
> If there is some feature of a driver that depends on an optional
> modularisable subsystem, and you want the driver to be built without that
> feature when the subsystem is missing, then the feature has to be disabled
> when the driver is built-in and the subsystem is a module. So in general
> you need:
>
> config DRIVER_FEATURE
> bool "This is a really neat feature for the driver"
> depends on DRIVER && SUBSYSTEM && !(DRIVER=y && SUBYSTEM=m)
>
> > I think it would be better if the "time stamp all Rx packets" of the
> > 82580 were always available, and that the PHC feature always be
> > compiled when CONFIG_PTP_1588_CLOCK is selected.
> >
> > Maybe you could ask Jeff what the issue was, and then see if there is
> > a way to remove CONFIG_IGB_PTP altogether.
>
> Even if they are to be enabled automatically, CONFIG_IGB_PTP and similar
> config symbols are important as shorthand. So I think what you want is:
>
> config IGB_PTP
> def_bool y
> depends on IGB && PTP_1588_CLOCK && !(IGB=y && PTP_1588_CLOCK=m)
>
> (But currently it's actually *selecting* PTP_1588_CLOCK. We should
> probably have drivers consistently select or depend on it, not a
> mixture.)
>
> Ben.
>
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
The IGP_PTP is necessary otherwise you have to do something like #if defined(CONFIG_PTP_1588_CLOCK), or #ifdef CONFIG_PTP_1588_CLOCK \ #ifdef CONFIG_PTP_1588_CLOCK_MODULE
The main reason that I wouldn't want to un-wrap the timestamping is because the hwtstamp ioctl is somewhat problematic because it is almost all ptp only. Also, some of the parts for igb driver don't support timestamp all, they only support ptp only packets, and this would be a lot more confusing since I would say still only allow that if ptp is on.. (since those values are useless except with the PHC clock)
- Jake
^ permalink raw reply
* Re: [PATCH 14/15] netpoll: re-enable irq in poll_napi()
From: Andrew Morton @ 2012-08-24 19:43 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David Miller
In-Reply-To: <1344597891-32242-15-git-send-email-amwang@redhat.com>
On Fri, 10 Aug 2012 19:24:50 +0800
Cong Wang <amwang@redhat.com> wrote:
> napi->poll() needs IRQ enabled, so we have to re-enable IRQ before
> calling it.
>
> Cc: David Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
> net/core/netpoll.c | 10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index e4ba3e7..346b1eb 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -168,16 +168,24 @@ static void poll_napi(struct net_device *dev)
> struct napi_struct *napi;
> int budget = 16;
>
> + WARN_ON_ONCE(!irqs_disabled());
> +
> list_for_each_entry(napi, &dev->napi_list, dev_list) {
> + local_irq_enable();
> if (napi->poll_owner != smp_processor_id() &&
> spin_trylock(&napi->poll_lock)) {
> + rcu_read_lock_bh();
> budget = poll_one_napi(rcu_dereference_bh(dev->npinfo),
> napi, budget);
> + rcu_read_unlock_bh();
> spin_unlock(&napi->poll_lock);
>
> - if (!budget)
> + if (!budget) {
> + local_irq_disable();
> break;
> + }
> }
> + local_irq_disable();
> }
> }
This commit (6bdb7fe3104 in mainline) makes my netconsole-using x86_64
box lock up during boot. Dunno why, but I do have a cellphone:
http://ozlabs.org/~akpm/stuff/IMG_20120824_122054.jpg
^ permalink raw reply
* pull request: sfc-next 2012-08-24
From: Ben Hutchings @ 2012-08-24 19:46 UTC (permalink / raw)
To: David Miller; +Cc: linux-net-drivers, netdev
The following changes since commit 8f4cccbbd92f2ad0ddbbc498ef7cee2a1c3defe9:
net: Set device operstate at registration time (2012-08-24 12:46:13 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
(commit 8f8b3d518999fd1c342310910aa1e49112c86d05)
1. Change the TX path to stop queues earlier and avoid returning
NETDEV_TX_BUSY.
2. Remove some inefficiencies in soft-TSO.
3. Fix various bugs involving device state transitions and/or reset
scheduling by error handlers.
4. Take advantage of my previous change to operstate initialisation.
5. Miscellaneous cleanup.
Ben.
Ben Hutchings (16):
sfc: Refactor struct efx_tx_buffer to use a flags field
sfc: Stop TX queues before they fill up
sfc: Simplify TSO header buffer allocation
sfc: Replace tso_state::full_packet_space with ip_base_len
sfc: Stash header offsets for TSO in struct tso_state
sfc: Change state names to be clearer, and comment them
sfc: Hold the RTNL lock for more of the suspend/resume cycle
sfc: Keep disabled NICs quiescent during suspend/resume
sfc: Hold RTNL lock (only) when calling efx_stop_interrupts()
sfc: Never try to stop and start a NIC that is disabled
sfc: Improve log messages in case we abort probe due to a pending reset
sfc: Fix reset vs probe/remove/PM races involving efx_nic::state
sfc: Remove overly paranoid locking assertions from netdev operations
sfc: Remove bogus comment about MTU change and RX buffer overrun
sfc: Assign efx and efx->type as early as possible in efx_pci_probe()
sfc: Fix the initial device operstate
drivers/net/ethernet/sfc/efx.c | 235 +++++++-----
drivers/net/ethernet/sfc/ethtool.c | 4 +-
drivers/net/ethernet/sfc/falcon_boards.c | 2 +-
drivers/net/ethernet/sfc/net_driver.h | 49 ++--
drivers/net/ethernet/sfc/nic.c | 6 +-
drivers/net/ethernet/sfc/tx.c | 621 ++++++++++++------------------
6 files changed, 410 insertions(+), 507 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-08-24 19:47 UTC (permalink / raw)
To: Tejun Heo
Cc: torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
rostedt, mingo, ebiederm, aarcange, ericvh, netdev, josh,
eric.dumazet, mathieu.desnoyers, axboe, agk, dm-devel, neilb,
ccaulfie, teigland, Trond.Myklebust, bfields, fweisbec, jesse,
venkat.x.venkatsubra, ejt, snitzer, edumazet, linux-nfs, dev,
rds-devel, lw
In-Reply-To: <20120823200456.GD14962@google.com>
On 08/23/2012 10:04 PM, Tejun Heo wrote:
> Hello, Sasha.
>
> On Thu, Aug 23, 2012 at 02:24:32AM +0200, Sasha Levin wrote:
>>> I think the almost trivial nature of hlist hashtables makes this a bit
>>> tricky and I'm not very sure but having this combinatory explosion is
>>> a bit dazzling when the same functionality can be achieved by simply
>>> combining operations which are already defined and named considering
>>> hashtable. I'm not feeling too strong about this tho. What do others
>>> think?
>>
>> I'm thinking that this hashtable API will have 2 purposes: First, it would
>> prevent the excessive duplication of hashtable implementations all around the code.
>>
>> Second, it will allow more easily interchangeable hashtable implementations to
>> find their way into the kernel. There are several maintainers who would be happy
>> to see dynamically sized RCU hashtable, and I'm guessing that several more
>> variants could be added based on needs in specific modules.
>>
>> The second reason is why several things you've mentioned look the way they are:
>>
>> - No DEFINE_HASHTABLE(): I wanted to force the use of hash_init() since
>> initialization for other hashtables may be more complicated than the static
>> initialization for this implementation, which means that any place that used
>> DEFINE_HASHTABLE() and didn't do hash_init() will be buggy.
>
> I think this is problematic. It looks exactly like other existing
> DEFINE macros yet what its semantics is different. I don't think
> that's a good idea.
I can switch that to be DECLARE_HASHTABLE() if the issue is semantics.
>> I'm actually tempted in hiding hlist completely from hashtable users, probably
>> by simply defining a hash_head/hash_node on top of the hlist_ counterparts.
>
> I think that it would be best to keep this one simple & obvious, which
> already has enough in-kernel users to justify its existence. There
> are significant benefits in being trivially understandable and
> expectable. If we want more advanced ones - say resizing, hybrid or
> what not, let's make that a separate one. No need to complicate the
> common straight-forward case for that.
>
> So, I think it would be best to keep this one as straight-forward and
> trivial as possible. Helper macros to help its users are fine but
> let's please not go for full encapsulation.
What if we cut off the dynamic allocated (but not resizable) hashtable out for
the moment, and focus on the most common statically allocated hashtable case?
The benefits would be:
- Getting rid of all the _size() macros, which will make the amount of helpers
here reasonable.
- Dynamically allocated hashtable can be easily added as a separate
implementation using the same API. We already have some of those in the kernel...
- When that's ready, I feel it's a shame to lose full encapsulation just due to
hash_hashed().
Thanks,
Sasha
>
> Thanks.
>
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH net-next 01/16] sfc: Refactor struct efx_tx_buffer to use a flags field
From: Ben Hutchings @ 2012-08-24 19:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
Add a flags field to struct efx_tx_buffer, replacing the
continuation and map_single booleans.
Since a single descriptor cannot be both a TSO header and the last
descriptor for an skb, unionise efx_tx_buffer::{skb,tsoh} and add
flags for validity of these fields.
Clear all flags in free buffers (whereas previously the continuation
flag would be set).
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/net_driver.h | 27 ++++++-----
drivers/net/ethernet/sfc/nic.c | 4 +-
drivers/net/ethernet/sfc/tx.c | 78 +++++++++++++++------------------
3 files changed, 53 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index cd9c0a9..0ac01fa 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -91,29 +91,30 @@ struct efx_special_buffer {
};
/**
- * struct efx_tx_buffer - An Efx TX buffer
- * @skb: The associated socket buffer.
- * Set only on the final fragment of a packet; %NULL for all other
- * fragments. When this fragment completes, then we can free this
- * skb.
- * @tsoh: The associated TSO header structure, or %NULL if this
- * buffer is not a TSO header.
+ * struct efx_tx_buffer - buffer state for a TX descriptor
+ * @skb: When @flags & %EFX_TX_BUF_SKB, the associated socket buffer to be
+ * freed when descriptor completes
+ * @tsoh: When @flags & %EFX_TX_BUF_TSOH, the associated TSO header structure.
* @dma_addr: DMA address of the fragment.
+ * @flags: Flags for allocation and DMA mapping type
* @len: Length of this fragment.
* This field is zero when the queue slot is empty.
- * @continuation: True if this fragment is not the end of a packet.
- * @unmap_single: True if dma_unmap_single should be used.
* @unmap_len: Length of this fragment to unmap
*/
struct efx_tx_buffer {
- const struct sk_buff *skb;
- struct efx_tso_header *tsoh;
+ union {
+ const struct sk_buff *skb;
+ struct efx_tso_header *tsoh;
+ };
dma_addr_t dma_addr;
+ unsigned short flags;
unsigned short len;
- bool continuation;
- bool unmap_single;
unsigned short unmap_len;
};
+#define EFX_TX_BUF_CONT 1 /* not last descriptor of packet */
+#define EFX_TX_BUF_SKB 2 /* buffer is last part of skb */
+#define EFX_TX_BUF_TSOH 4 /* buffer is TSO header */
+#define EFX_TX_BUF_MAP_SINGLE 8 /* buffer was mapped with dma_map_single() */
/**
* struct efx_tx_queue - An Efx TX queue
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index 326d799..aa11370 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -401,8 +401,10 @@ void efx_nic_push_buffers(struct efx_tx_queue *tx_queue)
++tx_queue->write_count;
/* Create TX descriptor ring entry */
+ BUILD_BUG_ON(EFX_TX_BUF_CONT != 1);
EFX_POPULATE_QWORD_4(*txd,
- FSF_AZ_TX_KER_CONT, buffer->continuation,
+ FSF_AZ_TX_KER_CONT,
+ buffer->flags & EFX_TX_BUF_CONT,
FSF_AZ_TX_KER_BYTE_COUNT, buffer->len,
FSF_AZ_TX_KER_BUF_REGION, 0,
FSF_AZ_TX_KER_BUF_ADDR, buffer->dma_addr);
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 1871343..24c82f3 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -39,25 +39,25 @@ static void efx_dequeue_buffer(struct efx_tx_queue *tx_queue,
struct device *dma_dev = &tx_queue->efx->pci_dev->dev;
dma_addr_t unmap_addr = (buffer->dma_addr + buffer->len -
buffer->unmap_len);
- if (buffer->unmap_single)
+ if (buffer->flags & EFX_TX_BUF_MAP_SINGLE)
dma_unmap_single(dma_dev, unmap_addr, buffer->unmap_len,
DMA_TO_DEVICE);
else
dma_unmap_page(dma_dev, unmap_addr, buffer->unmap_len,
DMA_TO_DEVICE);
buffer->unmap_len = 0;
- buffer->unmap_single = false;
}
- if (buffer->skb) {
+ if (buffer->flags & EFX_TX_BUF_SKB) {
(*pkts_compl)++;
(*bytes_compl) += buffer->skb->len;
dev_kfree_skb_any((struct sk_buff *) buffer->skb);
- buffer->skb = NULL;
netif_vdbg(tx_queue->efx, tx_done, tx_queue->efx->net_dev,
"TX queue %d transmission id %x complete\n",
tx_queue->queue, tx_queue->read_count);
}
+
+ buffer->flags &= EFX_TX_BUF_TSOH;
}
/**
@@ -89,14 +89,14 @@ static void efx_tsoh_heap_free(struct efx_tx_queue *tx_queue,
static void efx_tsoh_free(struct efx_tx_queue *tx_queue,
struct efx_tx_buffer *buffer)
{
- if (buffer->tsoh) {
+ if (buffer->flags & EFX_TX_BUF_TSOH) {
if (likely(!buffer->tsoh->unmap_len)) {
buffer->tsoh->next = tx_queue->tso_headers_free;
tx_queue->tso_headers_free = buffer->tsoh;
} else {
efx_tsoh_heap_free(tx_queue, buffer->tsoh);
}
- buffer->tsoh = NULL;
+ buffer->flags &= ~EFX_TX_BUF_TSOH;
}
}
@@ -163,7 +163,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
unsigned int len, unmap_len = 0, fill_level, insert_ptr;
dma_addr_t dma_addr, unmap_addr = 0;
unsigned int dma_len;
- bool unmap_single;
+ unsigned short dma_flags;
int q_space, i = 0;
netdev_tx_t rc = NETDEV_TX_OK;
@@ -190,7 +190,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
* since this is more efficient on machines with sparse
* memory.
*/
- unmap_single = true;
+ dma_flags = EFX_TX_BUF_MAP_SINGLE;
dma_addr = dma_map_single(dma_dev, skb->data, len, PCI_DMA_TODEVICE);
/* Process all fragments */
@@ -234,10 +234,8 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
insert_ptr = tx_queue->insert_count & tx_queue->ptr_mask;
buffer = &tx_queue->buffer[insert_ptr];
efx_tsoh_free(tx_queue, buffer);
- EFX_BUG_ON_PARANOID(buffer->tsoh);
- EFX_BUG_ON_PARANOID(buffer->skb);
+ EFX_BUG_ON_PARANOID(buffer->flags);
EFX_BUG_ON_PARANOID(buffer->len);
- EFX_BUG_ON_PARANOID(!buffer->continuation);
EFX_BUG_ON_PARANOID(buffer->unmap_len);
dma_len = efx_max_tx_len(efx, dma_addr);
@@ -247,13 +245,14 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
/* Fill out per descriptor fields */
buffer->len = dma_len;
buffer->dma_addr = dma_addr;
+ buffer->flags = EFX_TX_BUF_CONT;
len -= dma_len;
dma_addr += dma_len;
++tx_queue->insert_count;
} while (len);
/* Transfer ownership of the unmapping to the final buffer */
- buffer->unmap_single = unmap_single;
+ buffer->flags = EFX_TX_BUF_CONT | dma_flags;
buffer->unmap_len = unmap_len;
unmap_len = 0;
@@ -264,14 +263,14 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
len = skb_frag_size(fragment);
i++;
/* Map for DMA */
- unmap_single = false;
+ dma_flags = 0;
dma_addr = skb_frag_dma_map(dma_dev, fragment, 0, len,
DMA_TO_DEVICE);
}
/* Transfer ownership of the skb to the final buffer */
buffer->skb = skb;
- buffer->continuation = false;
+ buffer->flags = EFX_TX_BUF_SKB | dma_flags;
netdev_tx_sent_queue(tx_queue->core_txq, skb->len);
@@ -302,7 +301,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
/* Free the fragment we were mid-way through pushing */
if (unmap_len) {
- if (unmap_single)
+ if (dma_flags & EFX_TX_BUF_MAP_SINGLE)
dma_unmap_single(dma_dev, unmap_addr, unmap_len,
DMA_TO_DEVICE);
else
@@ -340,7 +339,6 @@ static void efx_dequeue_buffers(struct efx_tx_queue *tx_queue,
}
efx_dequeue_buffer(tx_queue, buffer, pkts_compl, bytes_compl);
- buffer->continuation = true;
buffer->len = 0;
++tx_queue->read_count;
@@ -484,7 +482,7 @@ int efx_probe_tx_queue(struct efx_tx_queue *tx_queue)
{
struct efx_nic *efx = tx_queue->efx;
unsigned int entries;
- int i, rc;
+ int rc;
/* Create the smallest power-of-two aligned ring */
entries = max(roundup_pow_of_two(efx->txq_entries), EFX_MIN_DMAQ_SIZE);
@@ -500,8 +498,6 @@ int efx_probe_tx_queue(struct efx_tx_queue *tx_queue)
GFP_KERNEL);
if (!tx_queue->buffer)
return -ENOMEM;
- for (i = 0; i <= tx_queue->ptr_mask; ++i)
- tx_queue->buffer[i].continuation = true;
/* Allocate hardware ring */
rc = efx_nic_probe_tx(tx_queue);
@@ -546,7 +542,6 @@ void efx_release_tx_buffers(struct efx_tx_queue *tx_queue)
unsigned int pkts_compl = 0, bytes_compl = 0;
buffer = &tx_queue->buffer[tx_queue->read_count & tx_queue->ptr_mask];
efx_dequeue_buffer(tx_queue, buffer, &pkts_compl, &bytes_compl);
- buffer->continuation = true;
buffer->len = 0;
++tx_queue->read_count;
@@ -631,7 +626,7 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
* @in_len: Remaining length in current SKB fragment
* @unmap_len: Length of SKB fragment
* @unmap_addr: DMA address of SKB fragment
- * @unmap_single: DMA single vs page mapping flag
+ * @dma_flags: TX buffer flags for DMA mapping - %EFX_TX_BUF_MAP_SINGLE or 0
* @protocol: Network protocol (after any VLAN header)
* @header_len: Number of bytes of header
* @full_packet_size: Number of bytes to put in each outgoing segment
@@ -651,7 +646,7 @@ struct tso_state {
unsigned in_len;
unsigned unmap_len;
dma_addr_t unmap_addr;
- bool unmap_single;
+ unsigned short dma_flags;
__be16 protocol;
unsigned header_len;
@@ -833,9 +828,7 @@ static int efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
efx_tsoh_free(tx_queue, buffer);
EFX_BUG_ON_PARANOID(buffer->len);
EFX_BUG_ON_PARANOID(buffer->unmap_len);
- EFX_BUG_ON_PARANOID(buffer->skb);
- EFX_BUG_ON_PARANOID(!buffer->continuation);
- EFX_BUG_ON_PARANOID(buffer->tsoh);
+ EFX_BUG_ON_PARANOID(buffer->flags);
buffer->dma_addr = dma_addr;
@@ -845,7 +838,8 @@ static int efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
if (dma_len >= len)
break;
- buffer->len = dma_len; /* Don't set the other members */
+ buffer->len = dma_len;
+ buffer->flags = EFX_TX_BUF_CONT;
dma_addr += dma_len;
len -= dma_len;
}
@@ -873,12 +867,11 @@ static void efx_tso_put_header(struct efx_tx_queue *tx_queue,
efx_tsoh_free(tx_queue, buffer);
EFX_BUG_ON_PARANOID(buffer->len);
EFX_BUG_ON_PARANOID(buffer->unmap_len);
- EFX_BUG_ON_PARANOID(buffer->skb);
- EFX_BUG_ON_PARANOID(!buffer->continuation);
- EFX_BUG_ON_PARANOID(buffer->tsoh);
+ EFX_BUG_ON_PARANOID(buffer->flags);
buffer->len = len;
buffer->dma_addr = tsoh->dma_addr;
buffer->tsoh = tsoh;
+ buffer->flags = EFX_TX_BUF_TSOH | EFX_TX_BUF_CONT;
++tx_queue->insert_count;
}
@@ -896,11 +889,11 @@ static void efx_enqueue_unwind(struct efx_tx_queue *tx_queue)
buffer = &tx_queue->buffer[tx_queue->insert_count &
tx_queue->ptr_mask];
efx_tsoh_free(tx_queue, buffer);
- EFX_BUG_ON_PARANOID(buffer->skb);
+ EFX_BUG_ON_PARANOID(buffer->flags & EFX_TX_BUF_SKB);
if (buffer->unmap_len) {
unmap_addr = (buffer->dma_addr + buffer->len -
buffer->unmap_len);
- if (buffer->unmap_single)
+ if (buffer->flags & EFX_TX_BUF_MAP_SINGLE)
dma_unmap_single(&tx_queue->efx->pci_dev->dev,
unmap_addr, buffer->unmap_len,
DMA_TO_DEVICE);
@@ -911,7 +904,7 @@ static void efx_enqueue_unwind(struct efx_tx_queue *tx_queue)
buffer->unmap_len = 0;
}
buffer->len = 0;
- buffer->continuation = true;
+ buffer->flags = 0;
}
}
@@ -938,7 +931,7 @@ static void tso_start(struct tso_state *st, const struct sk_buff *skb)
st->out_len = skb->len - st->header_len;
st->unmap_len = 0;
- st->unmap_single = false;
+ st->dma_flags = 0;
}
static int tso_get_fragment(struct tso_state *st, struct efx_nic *efx,
@@ -947,7 +940,7 @@ static int tso_get_fragment(struct tso_state *st, struct efx_nic *efx,
st->unmap_addr = skb_frag_dma_map(&efx->pci_dev->dev, frag, 0,
skb_frag_size(frag), DMA_TO_DEVICE);
if (likely(!dma_mapping_error(&efx->pci_dev->dev, st->unmap_addr))) {
- st->unmap_single = false;
+ st->dma_flags = 0;
st->unmap_len = skb_frag_size(frag);
st->in_len = skb_frag_size(frag);
st->dma_addr = st->unmap_addr;
@@ -965,7 +958,7 @@ static int tso_get_head_fragment(struct tso_state *st, struct efx_nic *efx,
st->unmap_addr = dma_map_single(&efx->pci_dev->dev, skb->data + hl,
len, DMA_TO_DEVICE);
if (likely(!dma_mapping_error(&efx->pci_dev->dev, st->unmap_addr))) {
- st->unmap_single = true;
+ st->dma_flags = EFX_TX_BUF_MAP_SINGLE;
st->unmap_len = len;
st->in_len = len;
st->dma_addr = st->unmap_addr;
@@ -990,7 +983,7 @@ static int tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
struct tso_state *st)
{
struct efx_tx_buffer *buffer;
- int n, end_of_packet, rc;
+ int n, rc;
if (st->in_len == 0)
return 0;
@@ -1008,17 +1001,18 @@ static int tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
rc = efx_tx_queue_insert(tx_queue, st->dma_addr, n, &buffer);
if (likely(rc == 0)) {
- if (st->out_len == 0)
+ if (st->out_len == 0) {
/* Transfer ownership of the skb */
buffer->skb = skb;
-
- end_of_packet = st->out_len == 0 || st->packet_space == 0;
- buffer->continuation = !end_of_packet;
+ buffer->flags = EFX_TX_BUF_SKB;
+ } else if (st->packet_space != 0) {
+ buffer->flags = EFX_TX_BUF_CONT;
+ }
if (st->in_len == 0) {
/* Transfer ownership of the DMA mapping */
buffer->unmap_len = st->unmap_len;
- buffer->unmap_single = st->unmap_single;
+ buffer->flags |= st->dma_flags;
st->unmap_len = 0;
}
}
@@ -1195,7 +1189,7 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
unwind:
/* Free the DMA mapping we were in the process of writing out */
if (state.unmap_len) {
- if (state.unmap_single)
+ if (state.dma_flags & EFX_TX_BUF_MAP_SINGLE)
dma_unmap_single(&efx->pci_dev->dev, state.unmap_addr,
state.unmap_len, DMA_TO_DEVICE);
else
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 02/16] sfc: Stop TX queues before they fill up
From: Ben Hutchings @ 2012-08-24 19:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
We now have a definite upper bound on the number of descriptors per
skb; use that to stop the queue when the next packet might not fit.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 10 ++
drivers/net/ethernet/sfc/net_driver.h | 5 +
drivers/net/ethernet/sfc/tx.c | 212 +++++++++++++++------------------
3 files changed, 112 insertions(+), 115 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 65a8d49..3b3f084 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -630,6 +630,16 @@ static void efx_start_datapath(struct efx_nic *efx)
efx->rx_buffer_order = get_order(efx->rx_buffer_len +
sizeof(struct efx_rx_page_state));
+ /* We must keep at least one descriptor in a TX ring empty.
+ * We could avoid this when the queue size does not exactly
+ * match the hardware ring size, but it's not that important.
+ * Therefore we stop the queue when one more skb might fill
+ * the ring completely. We wake it when half way back to
+ * empty.
+ */
+ efx->txq_stop_thresh = efx->txq_entries - efx_tx_max_skb_descs(efx);
+ efx->txq_wake_thresh = efx->txq_stop_thresh / 2;
+
/* Initialise the channels */
efx_for_each_channel(channel, efx) {
efx_for_each_channel_tx_queue(tx_queue, channel)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 0ac01fa..28a6d62 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -665,6 +665,8 @@ struct vfdi_status;
* should be allocated for this NIC
* @rxq_entries: Size of receive queues requested by user.
* @txq_entries: Size of transmit queues requested by user.
+ * @txq_stop_thresh: TX queue fill level at or above which we stop it.
+ * @txq_wake_thresh: TX queue fill level at or below which we wake it.
* @tx_dc_base: Base qword address in SRAM of TX queue descriptor caches
* @rx_dc_base: Base qword address in SRAM of RX queue descriptor caches
* @sram_lim_qw: Qword address limit of SRAM
@@ -775,6 +777,9 @@ struct efx_nic {
unsigned rxq_entries;
unsigned txq_entries;
+ unsigned int txq_stop_thresh;
+ unsigned int txq_wake_thresh;
+
unsigned tx_dc_base;
unsigned rx_dc_base;
unsigned sram_lim_qw;
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 24c82f3..330d911 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -22,14 +22,6 @@
#include "nic.h"
#include "workarounds.h"
-/*
- * TX descriptor ring full threshold
- *
- * The tx_queue descriptor ring fill-level must fall below this value
- * before we restart the netif queue
- */
-#define EFX_TXQ_THRESHOLD(_efx) ((_efx)->txq_entries / 2u)
-
static void efx_dequeue_buffer(struct efx_tx_queue *tx_queue,
struct efx_tx_buffer *buffer,
unsigned int *pkts_compl,
@@ -138,6 +130,56 @@ unsigned int efx_tx_max_skb_descs(struct efx_nic *efx)
return max_descs;
}
+/* Get partner of a TX queue, seen as part of the same net core queue */
+static struct efx_tx_queue *efx_tx_queue_partner(struct efx_tx_queue *tx_queue)
+{
+ if (tx_queue->queue & EFX_TXQ_TYPE_OFFLOAD)
+ return tx_queue - EFX_TXQ_TYPE_OFFLOAD;
+ else
+ return tx_queue + EFX_TXQ_TYPE_OFFLOAD;
+}
+
+static void efx_tx_maybe_stop_queue(struct efx_tx_queue *txq1)
+{
+ /* We need to consider both queues that the net core sees as one */
+ struct efx_tx_queue *txq2 = efx_tx_queue_partner(txq1);
+ struct efx_nic *efx = txq1->efx;
+ unsigned int fill_level;
+
+ fill_level = max(txq1->insert_count - txq1->old_read_count,
+ txq2->insert_count - txq2->old_read_count);
+ if (likely(fill_level < efx->txq_stop_thresh))
+ return;
+
+ /* We used the stale old_read_count above, which gives us a
+ * pessimistic estimate of the fill level (which may even
+ * validly be >= efx->txq_entries). Now try again using
+ * read_count (more likely to be a cache miss).
+ *
+ * If we read read_count and then conditionally stop the
+ * queue, it is possible for the completion path to race with
+ * us and complete all outstanding descriptors in the middle,
+ * after which there will be no more completions to wake it.
+ * Therefore we stop the queue first, then read read_count
+ * (with a memory barrier to ensure the ordering), then
+ * restart the queue if the fill level turns out to be low
+ * enough.
+ */
+ netif_tx_stop_queue(txq1->core_txq);
+ smp_mb();
+ txq1->old_read_count = ACCESS_ONCE(txq1->read_count);
+ txq2->old_read_count = ACCESS_ONCE(txq2->read_count);
+
+ fill_level = max(txq1->insert_count - txq1->old_read_count,
+ txq2->insert_count - txq2->old_read_count);
+ EFX_BUG_ON_PARANOID(fill_level >= efx->txq_entries);
+ if (likely(fill_level < efx->txq_stop_thresh)) {
+ smp_mb();
+ if (likely(!efx->loopback_selftest))
+ netif_tx_start_queue(txq1->core_txq);
+ }
+}
+
/*
* Add a socket buffer to a TX queue
*
@@ -151,7 +193,7 @@ unsigned int efx_tx_max_skb_descs(struct efx_nic *efx)
* This function is split out from efx_hard_start_xmit to allow the
* loopback test to direct packets via specific TX queues.
*
- * Returns NETDEV_TX_OK or NETDEV_TX_BUSY
+ * Returns NETDEV_TX_OK.
* You must hold netif_tx_lock() to call this function.
*/
netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
@@ -160,12 +202,11 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
struct device *dma_dev = &efx->pci_dev->dev;
struct efx_tx_buffer *buffer;
skb_frag_t *fragment;
- unsigned int len, unmap_len = 0, fill_level, insert_ptr;
+ unsigned int len, unmap_len = 0, insert_ptr;
dma_addr_t dma_addr, unmap_addr = 0;
unsigned int dma_len;
unsigned short dma_flags;
- int q_space, i = 0;
- netdev_tx_t rc = NETDEV_TX_OK;
+ int i = 0;
EFX_BUG_ON_PARANOID(tx_queue->write_count != tx_queue->insert_count);
@@ -183,9 +224,6 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
return NETDEV_TX_OK;
}
- fill_level = tx_queue->insert_count - tx_queue->old_read_count;
- q_space = efx->txq_entries - 1 - fill_level;
-
/* Map for DMA. Use dma_map_single rather than dma_map_page
* since this is more efficient on machines with sparse
* memory.
@@ -205,32 +243,6 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
/* Add to TX queue, splitting across DMA boundaries */
do {
- if (unlikely(q_space-- <= 0)) {
- /* It might be that completions have
- * happened since the xmit path last
- * checked. Update the xmit path's
- * copy of read_count.
- */
- netif_tx_stop_queue(tx_queue->core_txq);
- /* This memory barrier protects the
- * change of queue state from the access
- * of read_count. */
- smp_mb();
- tx_queue->old_read_count =
- ACCESS_ONCE(tx_queue->read_count);
- fill_level = (tx_queue->insert_count
- - tx_queue->old_read_count);
- q_space = efx->txq_entries - 1 - fill_level;
- if (unlikely(q_space-- <= 0)) {
- rc = NETDEV_TX_BUSY;
- goto unwind;
- }
- smp_mb();
- if (likely(!efx->loopback_selftest))
- netif_tx_start_queue(
- tx_queue->core_txq);
- }
-
insert_ptr = tx_queue->insert_count & tx_queue->ptr_mask;
buffer = &tx_queue->buffer[insert_ptr];
efx_tsoh_free(tx_queue, buffer);
@@ -277,6 +289,8 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
/* Pass off to hardware */
efx_nic_push_buffers(tx_queue);
+ efx_tx_maybe_stop_queue(tx_queue);
+
return NETDEV_TX_OK;
dma_err:
@@ -288,7 +302,6 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
/* Mark the packet as transmitted, and free the SKB ourselves */
dev_kfree_skb_any(skb);
- unwind:
/* Work backwards until we hit the original insert pointer value */
while (tx_queue->insert_count != tx_queue->write_count) {
unsigned int pkts_compl = 0, bytes_compl = 0;
@@ -309,7 +322,7 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
DMA_TO_DEVICE);
}
- return rc;
+ return NETDEV_TX_OK;
}
/* Remove packets from the TX queue
@@ -448,6 +461,7 @@ void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index)
{
unsigned fill_level;
struct efx_nic *efx = tx_queue->efx;
+ struct efx_tx_queue *txq2;
unsigned int pkts_compl = 0, bytes_compl = 0;
EFX_BUG_ON_PARANOID(index > tx_queue->ptr_mask);
@@ -455,15 +469,18 @@ void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index)
efx_dequeue_buffers(tx_queue, index, &pkts_compl, &bytes_compl);
netdev_tx_completed_queue(tx_queue->core_txq, pkts_compl, bytes_compl);
- /* See if we need to restart the netif queue. This barrier
- * separates the update of read_count from the test of the
- * queue state. */
+ /* See if we need to restart the netif queue. This memory
+ * barrier ensures that we write read_count (inside
+ * efx_dequeue_buffers()) before reading the queue status.
+ */
smp_mb();
if (unlikely(netif_tx_queue_stopped(tx_queue->core_txq)) &&
likely(efx->port_enabled) &&
likely(netif_device_present(efx->net_dev))) {
- fill_level = tx_queue->insert_count - tx_queue->read_count;
- if (fill_level < EFX_TXQ_THRESHOLD(efx))
+ txq2 = efx_tx_queue_partner(tx_queue);
+ fill_level = max(tx_queue->insert_count - tx_queue->read_count,
+ txq2->insert_count - txq2->read_count);
+ if (fill_level <= efx->txq_wake_thresh)
netif_tx_wake_queue(tx_queue->core_txq);
}
@@ -776,47 +793,19 @@ efx_tsoh_heap_free(struct efx_tx_queue *tx_queue, struct efx_tso_header *tsoh)
* @len: Length of fragment
* @final_buffer: The final buffer inserted into the queue
*
- * Push descriptors onto the TX queue. Return 0 on success or 1 if
- * @tx_queue full.
+ * Push descriptors onto the TX queue.
*/
-static int efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
- dma_addr_t dma_addr, unsigned len,
- struct efx_tx_buffer **final_buffer)
+static void efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
+ dma_addr_t dma_addr, unsigned len,
+ struct efx_tx_buffer **final_buffer)
{
struct efx_tx_buffer *buffer;
struct efx_nic *efx = tx_queue->efx;
- unsigned dma_len, fill_level, insert_ptr;
- int q_space;
+ unsigned dma_len, insert_ptr;
EFX_BUG_ON_PARANOID(len <= 0);
- fill_level = tx_queue->insert_count - tx_queue->old_read_count;
- /* -1 as there is no way to represent all descriptors used */
- q_space = efx->txq_entries - 1 - fill_level;
-
while (1) {
- if (unlikely(q_space-- <= 0)) {
- /* It might be that completions have happened
- * since the xmit path last checked. Update
- * the xmit path's copy of read_count.
- */
- netif_tx_stop_queue(tx_queue->core_txq);
- /* This memory barrier protects the change of
- * queue state from the access of read_count. */
- smp_mb();
- tx_queue->old_read_count =
- ACCESS_ONCE(tx_queue->read_count);
- fill_level = (tx_queue->insert_count
- - tx_queue->old_read_count);
- q_space = efx->txq_entries - 1 - fill_level;
- if (unlikely(q_space-- <= 0)) {
- *final_buffer = NULL;
- return 1;
- }
- smp_mb();
- netif_tx_start_queue(tx_queue->core_txq);
- }
-
insert_ptr = tx_queue->insert_count & tx_queue->ptr_mask;
buffer = &tx_queue->buffer[insert_ptr];
++tx_queue->insert_count;
@@ -847,7 +836,6 @@ static int efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
EFX_BUG_ON_PARANOID(!len);
buffer->len = len;
*final_buffer = buffer;
- return 0;
}
@@ -975,20 +963,19 @@ static int tso_get_head_fragment(struct tso_state *st, struct efx_nic *efx,
* @st: TSO state
*
* Form descriptors for the current fragment, until we reach the end
- * of fragment or end-of-packet. Return 0 on success, 1 if not enough
- * space in @tx_queue.
+ * of fragment or end-of-packet.
*/
-static int tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
- const struct sk_buff *skb,
- struct tso_state *st)
+static void tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
+ const struct sk_buff *skb,
+ struct tso_state *st)
{
struct efx_tx_buffer *buffer;
- int n, rc;
+ int n;
if (st->in_len == 0)
- return 0;
+ return;
if (st->packet_space == 0)
- return 0;
+ return;
EFX_BUG_ON_PARANOID(st->in_len <= 0);
EFX_BUG_ON_PARANOID(st->packet_space <= 0);
@@ -999,26 +986,24 @@ static int tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
st->out_len -= n;
st->in_len -= n;
- rc = efx_tx_queue_insert(tx_queue, st->dma_addr, n, &buffer);
- if (likely(rc == 0)) {
- if (st->out_len == 0) {
- /* Transfer ownership of the skb */
- buffer->skb = skb;
- buffer->flags = EFX_TX_BUF_SKB;
- } else if (st->packet_space != 0) {
- buffer->flags = EFX_TX_BUF_CONT;
- }
+ efx_tx_queue_insert(tx_queue, st->dma_addr, n, &buffer);
- if (st->in_len == 0) {
- /* Transfer ownership of the DMA mapping */
- buffer->unmap_len = st->unmap_len;
- buffer->flags |= st->dma_flags;
- st->unmap_len = 0;
- }
+ if (st->out_len == 0) {
+ /* Transfer ownership of the skb */
+ buffer->skb = skb;
+ buffer->flags = EFX_TX_BUF_SKB;
+ } else if (st->packet_space != 0) {
+ buffer->flags = EFX_TX_BUF_CONT;
+ }
+
+ if (st->in_len == 0) {
+ /* Transfer ownership of the DMA mapping */
+ buffer->unmap_len = st->unmap_len;
+ buffer->flags |= st->dma_flags;
+ st->unmap_len = 0;
}
st->dma_addr += n;
- return rc;
}
@@ -1112,13 +1097,13 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
*
* Add socket buffer @skb to @tx_queue, doing TSO or return != 0 if
* @skb was not enqueued. In all cases @skb is consumed. Return
- * %NETDEV_TX_OK or %NETDEV_TX_BUSY.
+ * %NETDEV_TX_OK.
*/
static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
struct sk_buff *skb)
{
struct efx_nic *efx = tx_queue->efx;
- int frag_i, rc, rc2 = NETDEV_TX_OK;
+ int frag_i, rc;
struct tso_state state;
/* Find the packet protocol and sanity-check it */
@@ -1150,11 +1135,7 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
goto mem_err;
while (1) {
- rc = tso_fill_packet_with_fragment(tx_queue, skb, &state);
- if (unlikely(rc)) {
- rc2 = NETDEV_TX_BUSY;
- goto unwind;
- }
+ tso_fill_packet_with_fragment(tx_queue, skb, &state);
/* Move onto the next fragment? */
if (state.in_len == 0) {
@@ -1178,6 +1159,8 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
/* Pass off to hardware */
efx_nic_push_buffers(tx_queue);
+ efx_tx_maybe_stop_queue(tx_queue);
+
tx_queue->tso_bursts++;
return NETDEV_TX_OK;
@@ -1186,7 +1169,6 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
"Out of memory for TSO headers, or DMA mapping error\n");
dev_kfree_skb_any(skb);
- unwind:
/* Free the DMA mapping we were in the process of writing out */
if (state.unmap_len) {
if (state.dma_flags & EFX_TX_BUF_MAP_SINGLE)
@@ -1198,7 +1180,7 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
}
efx_enqueue_unwind(tx_queue);
- return rc2;
+ return NETDEV_TX_OK;
}
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 03/16] sfc: Simplify TSO header buffer allocation
From: Ben Hutchings @ 2012-08-24 19:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
TSO header buffers contain a control structure immediately followed by
the packet headers, and are kept on a free list when not in use. This
complicates buffer management and tends to result in cache read misses
when we recycle such buffers (particularly if DMA-coherent memory
requires caches to be disabled).
Replace the free list with a simple mapping by descriptor index. We
know that there is always a payload descriptor between any two
descriptors with TSO header buffers, so we can allocate only one
such buffer for each two descriptors.
While we're at it, use a standard error code for allocation failure,
not -1.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/net_driver.h | 13 +-
drivers/net/ethernet/sfc/nic.c | 2 +-
drivers/net/ethernet/sfc/tx.c | 315 +++++++++++----------------------
3 files changed, 112 insertions(+), 218 deletions(-)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 28a6d62..a4fe9a7 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -94,7 +94,8 @@ struct efx_special_buffer {
* struct efx_tx_buffer - buffer state for a TX descriptor
* @skb: When @flags & %EFX_TX_BUF_SKB, the associated socket buffer to be
* freed when descriptor completes
- * @tsoh: When @flags & %EFX_TX_BUF_TSOH, the associated TSO header structure.
+ * @heap_buf: When @flags & %EFX_TX_BUF_HEAP, the associated heap buffer to be
+ * freed when descriptor completes.
* @dma_addr: DMA address of the fragment.
* @flags: Flags for allocation and DMA mapping type
* @len: Length of this fragment.
@@ -104,7 +105,7 @@ struct efx_special_buffer {
struct efx_tx_buffer {
union {
const struct sk_buff *skb;
- struct efx_tso_header *tsoh;
+ void *heap_buf;
};
dma_addr_t dma_addr;
unsigned short flags;
@@ -113,7 +114,7 @@ struct efx_tx_buffer {
};
#define EFX_TX_BUF_CONT 1 /* not last descriptor of packet */
#define EFX_TX_BUF_SKB 2 /* buffer is last part of skb */
-#define EFX_TX_BUF_TSOH 4 /* buffer is TSO header */
+#define EFX_TX_BUF_HEAP 4 /* buffer was allocated with kmalloc() */
#define EFX_TX_BUF_MAP_SINGLE 8 /* buffer was mapped with dma_map_single() */
/**
@@ -134,6 +135,7 @@ struct efx_tx_buffer {
* @channel: The associated channel
* @core_txq: The networking core TX queue structure
* @buffer: The software buffer ring
+ * @tsoh_page: Array of pages of TSO header buffers
* @txd: The hardware descriptor ring
* @ptr_mask: The size of the ring minus 1.
* @initialised: Has hardware queue been initialised?
@@ -157,9 +159,6 @@ struct efx_tx_buffer {
* variable indicates that the queue is full. This is to
* avoid cache-line ping-pong between the xmit path and the
* completion path.
- * @tso_headers_free: A list of TSO headers allocated for this TX queue
- * that are not in use, and so available for new TSO sends. The list
- * is protected by the TX queue lock.
* @tso_bursts: Number of times TSO xmit invoked by kernel
* @tso_long_headers: Number of packets with headers too long for standard
* blocks
@@ -176,6 +175,7 @@ struct efx_tx_queue {
struct efx_channel *channel;
struct netdev_queue *core_txq;
struct efx_tx_buffer *buffer;
+ struct efx_buffer *tsoh_page;
struct efx_special_buffer txd;
unsigned int ptr_mask;
bool initialised;
@@ -188,7 +188,6 @@ struct efx_tx_queue {
unsigned int insert_count ____cacheline_aligned_in_smp;
unsigned int write_count;
unsigned int old_read_count;
- struct efx_tso_header *tso_headers_free;
unsigned int tso_bursts;
unsigned int tso_long_headers;
unsigned int tso_packets;
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index aa11370..cdff40b 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -298,7 +298,7 @@ efx_free_special_buffer(struct efx_nic *efx, struct efx_special_buffer *buffer)
/**************************************************************************
*
* Generic buffer handling
- * These buffers are used for interrupt status and MAC stats
+ * These buffers are used for interrupt status, MAC stats, etc.
*
**************************************************************************/
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 330d911..61bc0ed 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -47,51 +47,16 @@ static void efx_dequeue_buffer(struct efx_tx_queue *tx_queue,
netif_vdbg(tx_queue->efx, tx_done, tx_queue->efx->net_dev,
"TX queue %d transmission id %x complete\n",
tx_queue->queue, tx_queue->read_count);
+ } else if (buffer->flags & EFX_TX_BUF_HEAP) {
+ kfree(buffer->heap_buf);
}
- buffer->flags &= EFX_TX_BUF_TSOH;
+ buffer->len = 0;
+ buffer->flags = 0;
}
-/**
- * struct efx_tso_header - a DMA mapped buffer for packet headers
- * @next: Linked list of free ones.
- * The list is protected by the TX queue lock.
- * @dma_unmap_len: Length to unmap for an oversize buffer, or 0.
- * @dma_addr: The DMA address of the header below.
- *
- * This controls the memory used for a TSO header. Use TSOH_DATA()
- * to find the packet header data. Use TSOH_SIZE() to calculate the
- * total size required for a given packet header length. TSO headers
- * in the free list are exactly %TSOH_STD_SIZE bytes in size.
- */
-struct efx_tso_header {
- union {
- struct efx_tso_header *next;
- size_t unmap_len;
- };
- dma_addr_t dma_addr;
-};
-
static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
struct sk_buff *skb);
-static void efx_fini_tso(struct efx_tx_queue *tx_queue);
-static void efx_tsoh_heap_free(struct efx_tx_queue *tx_queue,
- struct efx_tso_header *tsoh);
-
-static void efx_tsoh_free(struct efx_tx_queue *tx_queue,
- struct efx_tx_buffer *buffer)
-{
- if (buffer->flags & EFX_TX_BUF_TSOH) {
- if (likely(!buffer->tsoh->unmap_len)) {
- buffer->tsoh->next = tx_queue->tso_headers_free;
- tx_queue->tso_headers_free = buffer->tsoh;
- } else {
- efx_tsoh_heap_free(tx_queue, buffer->tsoh);
- }
- buffer->flags &= ~EFX_TX_BUF_TSOH;
- }
-}
-
static inline unsigned
efx_max_tx_len(struct efx_nic *efx, dma_addr_t dma_addr)
@@ -245,7 +210,6 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
do {
insert_ptr = tx_queue->insert_count & tx_queue->ptr_mask;
buffer = &tx_queue->buffer[insert_ptr];
- efx_tsoh_free(tx_queue, buffer);
EFX_BUG_ON_PARANOID(buffer->flags);
EFX_BUG_ON_PARANOID(buffer->len);
EFX_BUG_ON_PARANOID(buffer->unmap_len);
@@ -309,7 +273,6 @@ netdev_tx_t efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb)
insert_ptr = tx_queue->insert_count & tx_queue->ptr_mask;
buffer = &tx_queue->buffer[insert_ptr];
efx_dequeue_buffer(tx_queue, buffer, &pkts_compl, &bytes_compl);
- buffer->len = 0;
}
/* Free the fragment we were mid-way through pushing */
@@ -352,7 +315,6 @@ static void efx_dequeue_buffers(struct efx_tx_queue *tx_queue,
}
efx_dequeue_buffer(tx_queue, buffer, pkts_compl, bytes_compl);
- buffer->len = 0;
++tx_queue->read_count;
read_ptr = tx_queue->read_count & tx_queue->ptr_mask;
@@ -495,6 +457,21 @@ void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index)
}
}
+/* Size of page-based TSO header buffers. Larger blocks must be
+ * allocated from the heap.
+ */
+#define TSOH_STD_SIZE 128
+#define TSOH_PER_PAGE (PAGE_SIZE / TSOH_STD_SIZE)
+
+/* At most half the descriptors in the queue at any time will refer to
+ * a TSO header buffer, since they must always be followed by a
+ * payload descriptor referring to an skb.
+ */
+static unsigned int efx_tsoh_page_count(struct efx_tx_queue *tx_queue)
+{
+ return DIV_ROUND_UP(tx_queue->ptr_mask + 1, 2 * TSOH_PER_PAGE);
+}
+
int efx_probe_tx_queue(struct efx_tx_queue *tx_queue)
{
struct efx_nic *efx = tx_queue->efx;
@@ -516,14 +493,27 @@ int efx_probe_tx_queue(struct efx_tx_queue *tx_queue)
if (!tx_queue->buffer)
return -ENOMEM;
+ if (tx_queue->queue & EFX_TXQ_TYPE_OFFLOAD) {
+ tx_queue->tsoh_page =
+ kcalloc(efx_tsoh_page_count(tx_queue),
+ sizeof(tx_queue->tsoh_page[0]), GFP_KERNEL);
+ if (!tx_queue->tsoh_page) {
+ rc = -ENOMEM;
+ goto fail1;
+ }
+ }
+
/* Allocate hardware ring */
rc = efx_nic_probe_tx(tx_queue);
if (rc)
- goto fail;
+ goto fail2;
return 0;
- fail:
+fail2:
+ kfree(tx_queue->tsoh_page);
+ tx_queue->tsoh_page = NULL;
+fail1:
kfree(tx_queue->buffer);
tx_queue->buffer = NULL;
return rc;
@@ -559,7 +549,6 @@ void efx_release_tx_buffers(struct efx_tx_queue *tx_queue)
unsigned int pkts_compl = 0, bytes_compl = 0;
buffer = &tx_queue->buffer[tx_queue->read_count & tx_queue->ptr_mask];
efx_dequeue_buffer(tx_queue, buffer, &pkts_compl, &bytes_compl);
- buffer->len = 0;
++tx_queue->read_count;
}
@@ -580,13 +569,12 @@ void efx_fini_tx_queue(struct efx_tx_queue *tx_queue)
efx_nic_fini_tx(tx_queue);
efx_release_tx_buffers(tx_queue);
-
- /* Free up TSO header cache */
- efx_fini_tso(tx_queue);
}
void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
{
+ int i;
+
if (!tx_queue->buffer)
return;
@@ -594,6 +582,14 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
"destroying TX queue %d\n", tx_queue->queue);
efx_nic_remove_tx(tx_queue);
+ if (tx_queue->tsoh_page) {
+ for (i = 0; i < efx_tsoh_page_count(tx_queue); i++)
+ efx_nic_free_buffer(tx_queue->efx,
+ &tx_queue->tsoh_page[i]);
+ kfree(tx_queue->tsoh_page);
+ tx_queue->tsoh_page = NULL;
+ }
+
kfree(tx_queue->buffer);
tx_queue->buffer = NULL;
}
@@ -616,17 +612,6 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
#define TSOH_OFFSET NET_IP_ALIGN
#endif
-#define TSOH_BUFFER(tsoh) ((u8 *)(tsoh + 1) + TSOH_OFFSET)
-
-/* Total size of struct efx_tso_header, buffer and padding */
-#define TSOH_SIZE(hdr_len) \
- (sizeof(struct efx_tso_header) + TSOH_OFFSET + hdr_len)
-
-/* Size of blocks on free list. Larger blocks must be allocated from
- * the heap.
- */
-#define TSOH_STD_SIZE 128
-
#define PTR_DIFF(p1, p2) ((u8 *)(p1) - (u8 *)(p2))
#define ETH_HDR_LEN(skb) (skb_network_header(skb) - (skb)->data)
#define SKB_TCP_OFF(skb) PTR_DIFF(tcp_hdr(skb), (skb)->data)
@@ -699,91 +684,43 @@ static __be16 efx_tso_check_protocol(struct sk_buff *skb)
return protocol;
}
-
-/*
- * Allocate a page worth of efx_tso_header structures, and string them
- * into the tx_queue->tso_headers_free linked list. Return 0 or -ENOMEM.
- */
-static int efx_tsoh_block_alloc(struct efx_tx_queue *tx_queue)
+static u8 *efx_tsoh_get_buffer(struct efx_tx_queue *tx_queue,
+ struct efx_tx_buffer *buffer, unsigned int len)
{
- struct device *dma_dev = &tx_queue->efx->pci_dev->dev;
- struct efx_tso_header *tsoh;
- dma_addr_t dma_addr;
- u8 *base_kva, *kva;
+ u8 *result;
- base_kva = dma_alloc_coherent(dma_dev, PAGE_SIZE, &dma_addr, GFP_ATOMIC);
- if (base_kva == NULL) {
- netif_err(tx_queue->efx, tx_err, tx_queue->efx->net_dev,
- "Unable to allocate page for TSO headers\n");
- return -ENOMEM;
- }
-
- /* dma_alloc_coherent() allocates pages. */
- EFX_BUG_ON_PARANOID(dma_addr & (PAGE_SIZE - 1u));
-
- for (kva = base_kva; kva < base_kva + PAGE_SIZE; kva += TSOH_STD_SIZE) {
- tsoh = (struct efx_tso_header *)kva;
- tsoh->dma_addr = dma_addr + (TSOH_BUFFER(tsoh) - base_kva);
- tsoh->next = tx_queue->tso_headers_free;
- tx_queue->tso_headers_free = tsoh;
- }
-
- return 0;
-}
-
-
-/* Free up a TSO header, and all others in the same page. */
-static void efx_tsoh_block_free(struct efx_tx_queue *tx_queue,
- struct efx_tso_header *tsoh,
- struct device *dma_dev)
-{
- struct efx_tso_header **p;
- unsigned long base_kva;
- dma_addr_t base_dma;
+ EFX_BUG_ON_PARANOID(buffer->len);
+ EFX_BUG_ON_PARANOID(buffer->flags);
+ EFX_BUG_ON_PARANOID(buffer->unmap_len);
- base_kva = (unsigned long)tsoh & PAGE_MASK;
- base_dma = tsoh->dma_addr & PAGE_MASK;
+ if (likely(len <= TSOH_STD_SIZE - TSOH_OFFSET)) {
+ unsigned index =
+ (tx_queue->insert_count & tx_queue->ptr_mask) / 2;
+ struct efx_buffer *page_buf =
+ &tx_queue->tsoh_page[index / TSOH_PER_PAGE];
+ unsigned offset =
+ TSOH_STD_SIZE * (index % TSOH_PER_PAGE) + TSOH_OFFSET;
- p = &tx_queue->tso_headers_free;
- while (*p != NULL) {
- if (((unsigned long)*p & PAGE_MASK) == base_kva)
- *p = (*p)->next;
- else
- p = &(*p)->next;
- }
+ if (unlikely(!page_buf->addr) &&
+ efx_nic_alloc_buffer(tx_queue->efx, page_buf, PAGE_SIZE))
+ return NULL;
- dma_free_coherent(dma_dev, PAGE_SIZE, (void *)base_kva, base_dma);
-}
+ result = (u8 *)page_buf->addr + offset;
+ buffer->dma_addr = page_buf->dma_addr + offset;
+ buffer->flags = EFX_TX_BUF_CONT;
+ } else {
+ tx_queue->tso_long_headers++;
-static struct efx_tso_header *
-efx_tsoh_heap_alloc(struct efx_tx_queue *tx_queue, size_t header_len)
-{
- struct efx_tso_header *tsoh;
-
- tsoh = kmalloc(TSOH_SIZE(header_len), GFP_ATOMIC | GFP_DMA);
- if (unlikely(!tsoh))
- return NULL;
-
- tsoh->dma_addr = dma_map_single(&tx_queue->efx->pci_dev->dev,
- TSOH_BUFFER(tsoh), header_len,
- DMA_TO_DEVICE);
- if (unlikely(dma_mapping_error(&tx_queue->efx->pci_dev->dev,
- tsoh->dma_addr))) {
- kfree(tsoh);
- return NULL;
+ buffer->heap_buf = kmalloc(TSOH_OFFSET + len, GFP_ATOMIC);
+ if (unlikely(!buffer->heap_buf))
+ return NULL;
+ result = (u8 *)buffer->heap_buf + TSOH_OFFSET;
+ buffer->flags = EFX_TX_BUF_CONT | EFX_TX_BUF_HEAP;
}
- tsoh->unmap_len = header_len;
- return tsoh;
-}
+ buffer->len = len;
-static void
-efx_tsoh_heap_free(struct efx_tx_queue *tx_queue, struct efx_tso_header *tsoh)
-{
- dma_unmap_single(&tx_queue->efx->pci_dev->dev,
- tsoh->dma_addr, tsoh->unmap_len,
- DMA_TO_DEVICE);
- kfree(tsoh);
+ return result;
}
/**
@@ -814,7 +751,6 @@ static void efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
tx_queue->read_count >=
efx->txq_entries);
- efx_tsoh_free(tx_queue, buffer);
EFX_BUG_ON_PARANOID(buffer->len);
EFX_BUG_ON_PARANOID(buffer->unmap_len);
EFX_BUG_ON_PARANOID(buffer->flags);
@@ -846,53 +782,42 @@ static void efx_tx_queue_insert(struct efx_tx_queue *tx_queue,
* a single fragment, and we know it doesn't cross a page boundary. It
* also allows us to not worry about end-of-packet etc.
*/
-static void efx_tso_put_header(struct efx_tx_queue *tx_queue,
- struct efx_tso_header *tsoh, unsigned len)
+static int efx_tso_put_header(struct efx_tx_queue *tx_queue,
+ struct efx_tx_buffer *buffer, u8 *header)
{
- struct efx_tx_buffer *buffer;
-
- buffer = &tx_queue->buffer[tx_queue->insert_count & tx_queue->ptr_mask];
- efx_tsoh_free(tx_queue, buffer);
- EFX_BUG_ON_PARANOID(buffer->len);
- EFX_BUG_ON_PARANOID(buffer->unmap_len);
- EFX_BUG_ON_PARANOID(buffer->flags);
- buffer->len = len;
- buffer->dma_addr = tsoh->dma_addr;
- buffer->tsoh = tsoh;
- buffer->flags = EFX_TX_BUF_TSOH | EFX_TX_BUF_CONT;
+ if (unlikely(buffer->flags & EFX_TX_BUF_HEAP)) {
+ buffer->dma_addr = dma_map_single(&tx_queue->efx->pci_dev->dev,
+ header, buffer->len,
+ DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(&tx_queue->efx->pci_dev->dev,
+ buffer->dma_addr))) {
+ kfree(buffer->heap_buf);
+ buffer->len = 0;
+ buffer->flags = 0;
+ return -ENOMEM;
+ }
+ buffer->unmap_len = buffer->len;
+ buffer->flags |= EFX_TX_BUF_MAP_SINGLE;
+ }
++tx_queue->insert_count;
+ return 0;
}
-/* Remove descriptors put into a tx_queue. */
+/* Remove buffers put into a tx_queue. None of the buffers must have
+ * an skb attached.
+ */
static void efx_enqueue_unwind(struct efx_tx_queue *tx_queue)
{
struct efx_tx_buffer *buffer;
- dma_addr_t unmap_addr;
/* Work backwards until we hit the original insert pointer value */
while (tx_queue->insert_count != tx_queue->write_count) {
--tx_queue->insert_count;
buffer = &tx_queue->buffer[tx_queue->insert_count &
tx_queue->ptr_mask];
- efx_tsoh_free(tx_queue, buffer);
- EFX_BUG_ON_PARANOID(buffer->flags & EFX_TX_BUF_SKB);
- if (buffer->unmap_len) {
- unmap_addr = (buffer->dma_addr + buffer->len -
- buffer->unmap_len);
- if (buffer->flags & EFX_TX_BUF_MAP_SINGLE)
- dma_unmap_single(&tx_queue->efx->pci_dev->dev,
- unmap_addr, buffer->unmap_len,
- DMA_TO_DEVICE);
- else
- dma_unmap_page(&tx_queue->efx->pci_dev->dev,
- unmap_addr, buffer->unmap_len,
- DMA_TO_DEVICE);
- buffer->unmap_len = 0;
- }
- buffer->len = 0;
- buffer->flags = 0;
+ efx_dequeue_buffer(tx_queue, buffer, NULL, NULL);
}
}
@@ -1014,35 +939,24 @@ static void tso_fill_packet_with_fragment(struct efx_tx_queue *tx_queue,
* @st: TSO state
*
* Generate a new header and prepare for the new packet. Return 0 on
- * success, or -1 if failed to alloc header.
+ * success, or -%ENOMEM if failed to alloc header.
*/
static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
const struct sk_buff *skb,
struct tso_state *st)
{
- struct efx_tso_header *tsoh;
+ struct efx_tx_buffer *buffer =
+ &tx_queue->buffer[tx_queue->insert_count & tx_queue->ptr_mask];
struct tcphdr *tsoh_th;
unsigned ip_length;
u8 *header;
+ int rc;
- /* Allocate a DMA-mapped header buffer. */
- if (likely(TSOH_SIZE(st->header_len) <= TSOH_STD_SIZE)) {
- if (tx_queue->tso_headers_free == NULL) {
- if (efx_tsoh_block_alloc(tx_queue))
- return -1;
- }
- EFX_BUG_ON_PARANOID(!tx_queue->tso_headers_free);
- tsoh = tx_queue->tso_headers_free;
- tx_queue->tso_headers_free = tsoh->next;
- tsoh->unmap_len = 0;
- } else {
- tx_queue->tso_long_headers++;
- tsoh = efx_tsoh_heap_alloc(tx_queue, st->header_len);
- if (unlikely(!tsoh))
- return -1;
- }
+ /* Allocate and insert a DMA-mapped header buffer. */
+ header = efx_tsoh_get_buffer(tx_queue, buffer, st->header_len);
+ if (!header)
+ return -ENOMEM;
- header = TSOH_BUFFER(tsoh);
tsoh_th = (struct tcphdr *)(header + SKB_TCP_OFF(skb));
/* Copy and update the headers. */
@@ -1078,12 +992,13 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
tsoh_iph->payload_len = htons(ip_length - sizeof(*tsoh_iph));
}
+ rc = efx_tso_put_header(tx_queue, buffer, header);
+ if (unlikely(rc))
+ return rc;
+
st->packet_space = skb_shinfo(skb)->gso_size;
++tx_queue->tso_packets;
- /* Form a descriptor for this header. */
- efx_tso_put_header(tx_queue, tsoh, st->header_len);
-
return 0;
}
@@ -1182,23 +1097,3 @@ static int efx_enqueue_skb_tso(struct efx_tx_queue *tx_queue,
efx_enqueue_unwind(tx_queue);
return NETDEV_TX_OK;
}
-
-
-/*
- * Free up all TSO datastructures associated with tx_queue. This
- * routine should be called only once the tx_queue is both empty and
- * will no longer be used.
- */
-static void efx_fini_tso(struct efx_tx_queue *tx_queue)
-{
- unsigned i;
-
- if (tx_queue->buffer) {
- for (i = 0; i <= tx_queue->ptr_mask; ++i)
- efx_tsoh_free(tx_queue, &tx_queue->buffer[i]);
- }
-
- while (tx_queue->tso_headers_free != NULL)
- efx_tsoh_block_free(tx_queue, tx_queue->tso_headers_free,
- &tx_queue->efx->pci_dev->dev);
-}
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 04/16] sfc: Replace tso_state::full_packet_space with ip_base_len
From: Ben Hutchings @ 2012-08-24 19:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
We only use tso_state::full_packet_space to calculate the IPv4 tot_len
or IPv6 payload_len, not to set tso_state::packet_space. Replace it
with an ip_base_len field holding the value of tot_len or payload_len
before including the TCP payload, which is much more useful when
constructing the new headers.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/tx.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 61bc0ed..d9dbebc 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -631,7 +631,7 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
* @dma_flags: TX buffer flags for DMA mapping - %EFX_TX_BUF_MAP_SINGLE or 0
* @protocol: Network protocol (after any VLAN header)
* @header_len: Number of bytes of header
- * @full_packet_size: Number of bytes to put in each outgoing segment
+ * @ip_base_len: IPv4 tot_len or IPv6 payload_len, before TCP payload
*
* The state used during segmentation. It is put into this data structure
* just to make it easy to pass into inline functions.
@@ -652,7 +652,7 @@ struct tso_state {
__be16 protocol;
unsigned header_len;
- int full_packet_size;
+ unsigned int ip_base_len;
};
@@ -830,12 +830,14 @@ static void tso_start(struct tso_state *st, const struct sk_buff *skb)
*/
st->header_len = ((tcp_hdr(skb)->doff << 2u)
+ PTR_DIFF(tcp_hdr(skb), skb->data));
- st->full_packet_size = st->header_len + skb_shinfo(skb)->gso_size;
- if (st->protocol == htons(ETH_P_IP))
+ if (st->protocol == htons(ETH_P_IP)) {
+ st->ip_base_len = st->header_len - ETH_HDR_LEN(skb);
st->ipv4_id = ntohs(ip_hdr(skb)->id);
- else
+ } else {
+ st->ip_base_len = tcp_hdr(skb)->doff << 2u;
st->ipv4_id = 0;
+ }
st->seqnum = ntohl(tcp_hdr(skb)->seq);
EFX_BUG_ON_PARANOID(tcp_hdr(skb)->urg);
@@ -966,15 +968,16 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
st->seqnum += skb_shinfo(skb)->gso_size;
if (st->out_len > skb_shinfo(skb)->gso_size) {
/* This packet will not finish the TSO burst. */
- ip_length = st->full_packet_size - ETH_HDR_LEN(skb);
+ st->packet_space = skb_shinfo(skb)->gso_size;
tsoh_th->fin = 0;
tsoh_th->psh = 0;
} else {
/* This packet will be the last in the TSO burst. */
- ip_length = st->header_len - ETH_HDR_LEN(skb) + st->out_len;
+ st->packet_space = st->out_len;
tsoh_th->fin = tcp_hdr(skb)->fin;
tsoh_th->psh = tcp_hdr(skb)->psh;
}
+ ip_length = st->ip_base_len + st->packet_space;
if (st->protocol == htons(ETH_P_IP)) {
struct iphdr *tsoh_iph =
@@ -989,14 +992,13 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
struct ipv6hdr *tsoh_iph =
(struct ipv6hdr *)(header + SKB_IPV6_OFF(skb));
- tsoh_iph->payload_len = htons(ip_length - sizeof(*tsoh_iph));
+ tsoh_iph->payload_len = htons(ip_length);
}
rc = efx_tso_put_header(tx_queue, buffer, header);
if (unlikely(rc))
return rc;
- st->packet_space = skb_shinfo(skb)->gso_size;
++tx_queue->tso_packets;
return 0;
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 05/16] sfc: Stash header offsets for TSO in struct tso_state
From: Ben Hutchings @ 2012-08-24 19:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/tx.c | 28 ++++++++++++----------------
1 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index d9dbebc..ebca75e 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -613,10 +613,6 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
#endif
#define PTR_DIFF(p1, p2) ((u8 *)(p1) - (u8 *)(p2))
-#define ETH_HDR_LEN(skb) (skb_network_header(skb) - (skb)->data)
-#define SKB_TCP_OFF(skb) PTR_DIFF(tcp_hdr(skb), (skb)->data)
-#define SKB_IPV4_OFF(skb) PTR_DIFF(ip_hdr(skb), (skb)->data)
-#define SKB_IPV6_OFF(skb) PTR_DIFF(ipv6_hdr(skb), (skb)->data)
/**
* struct tso_state - TSO state for an SKB
@@ -630,6 +626,8 @@ void efx_remove_tx_queue(struct efx_tx_queue *tx_queue)
* @unmap_addr: DMA address of SKB fragment
* @dma_flags: TX buffer flags for DMA mapping - %EFX_TX_BUF_MAP_SINGLE or 0
* @protocol: Network protocol (after any VLAN header)
+ * @ip_off: Offset of IP header
+ * @tcp_off: Offset of TCP header
* @header_len: Number of bytes of header
* @ip_base_len: IPv4 tot_len or IPv6 payload_len, before TCP payload
*
@@ -651,6 +649,8 @@ struct tso_state {
unsigned short dma_flags;
__be16 protocol;
+ unsigned int ip_off;
+ unsigned int tcp_off;
unsigned header_len;
unsigned int ip_base_len;
};
@@ -825,17 +825,14 @@ static void efx_enqueue_unwind(struct efx_tx_queue *tx_queue)
/* Parse the SKB header and initialise state. */
static void tso_start(struct tso_state *st, const struct sk_buff *skb)
{
- /* All ethernet/IP/TCP headers combined size is TCP header size
- * plus offset of TCP header relative to start of packet.
- */
- st->header_len = ((tcp_hdr(skb)->doff << 2u)
- + PTR_DIFF(tcp_hdr(skb), skb->data));
-
+ st->ip_off = skb_network_header(skb) - skb->data;
+ st->tcp_off = skb_transport_header(skb) - skb->data;
+ st->header_len = st->tcp_off + (tcp_hdr(skb)->doff << 2u);
if (st->protocol == htons(ETH_P_IP)) {
- st->ip_base_len = st->header_len - ETH_HDR_LEN(skb);
+ st->ip_base_len = st->header_len - st->ip_off;
st->ipv4_id = ntohs(ip_hdr(skb)->id);
} else {
- st->ip_base_len = tcp_hdr(skb)->doff << 2u;
+ st->ip_base_len = st->header_len - st->tcp_off;
st->ipv4_id = 0;
}
st->seqnum = ntohl(tcp_hdr(skb)->seq);
@@ -959,7 +956,7 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
if (!header)
return -ENOMEM;
- tsoh_th = (struct tcphdr *)(header + SKB_TCP_OFF(skb));
+ tsoh_th = (struct tcphdr *)(header + st->tcp_off);
/* Copy and update the headers. */
memcpy(header, skb->data, st->header_len);
@@ -980,8 +977,7 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
ip_length = st->ip_base_len + st->packet_space;
if (st->protocol == htons(ETH_P_IP)) {
- struct iphdr *tsoh_iph =
- (struct iphdr *)(header + SKB_IPV4_OFF(skb));
+ struct iphdr *tsoh_iph = (struct iphdr *)(header + st->ip_off);
tsoh_iph->tot_len = htons(ip_length);
@@ -990,7 +986,7 @@ static int tso_start_new_packet(struct efx_tx_queue *tx_queue,
st->ipv4_id++;
} else {
struct ipv6hdr *tsoh_iph =
- (struct ipv6hdr *)(header + SKB_IPV6_OFF(skb));
+ (struct ipv6hdr *)(header + st->ip_off);
tsoh_iph->payload_len = htons(ip_length);
}
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 06/16] sfc: Change state names to be clearer, and comment them
From: Ben Hutchings @ 2012-08-24 19:50 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
STATE_INIT and STATE_FINI are equivalent and represent incompletely
initialised states; combine them as STATE_UNINIT.
Rename STATE_RUNNING to STATE_READY, to avoid confusion with
netif_running() and IFF_RUNNING.
The comments do not quite match current usage, but this will be
corrected in subsequent fixes.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 24 +++++++++++-------------
drivers/net/ethernet/sfc/ethtool.c | 2 +-
drivers/net/ethernet/sfc/falcon_boards.c | 2 +-
drivers/net/ethernet/sfc/net_driver.h | 10 ++++------
4 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 3b3f084..9f88ad8 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -202,7 +202,7 @@ static void efx_stop_all(struct efx_nic *efx);
#define EFX_ASSERT_RESET_SERIALISED(efx) \
do { \
- if ((efx->state == STATE_RUNNING) || \
+ if ((efx->state == STATE_READY) || \
(efx->state == STATE_DISABLED)) \
ASSERT_RTNL(); \
} while (0)
@@ -1556,7 +1556,7 @@ static void efx_start_all(struct efx_nic *efx)
* of these flags are safe to read under just the rtnl lock */
if (efx->port_enabled)
return;
- if ((efx->state != STATE_RUNNING) && (efx->state != STATE_INIT))
+ if ((efx->state != STATE_READY) && (efx->state != STATE_UNINIT))
return;
if (!netif_running(efx->net_dev))
return;
@@ -2286,11 +2286,11 @@ static void efx_reset_work(struct work_struct *data)
if (!pending)
return;
- /* If we're not RUNNING then don't reset. Leave the reset_pending
+ /* If we're not READY then don't reset. Leave the reset_pending
* flags set so that efx_pci_probe_main will be retried */
- if (efx->state != STATE_RUNNING) {
+ if (efx->state != STATE_READY) {
netif_info(efx, drv, efx->net_dev,
- "scheduled reset quenched. NIC not RUNNING\n");
+ "scheduled reset quenched; NIC not ready\n");
return;
}
@@ -2402,7 +2402,7 @@ static int efx_init_struct(struct efx_nic *efx, const struct efx_nic_type *type,
INIT_DELAYED_WORK(&efx->selftest_work, efx_selftest_async_work);
efx->pci_dev = pci_dev;
efx->msg_enable = debug;
- efx->state = STATE_INIT;
+ efx->state = STATE_UNINIT;
strlcpy(efx->name, pci_name(pci_dev), sizeof(efx->name));
efx->net_dev = net_dev;
@@ -2490,7 +2490,7 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
/* Mark the NIC as fini, then stop the interface */
rtnl_lock();
- efx->state = STATE_FINI;
+ efx->state = STATE_UNINIT;
dev_close(efx->net_dev);
/* Allow any queued efx_resets() to complete */
@@ -2684,9 +2684,9 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
goto fail4;
}
- /* Switch to the running state before we expose the device to the OS,
+ /* Switch to the READY state before we expose the device to the OS,
* so that dev_open()|efx_start_all() will actually start the device */
- efx->state = STATE_RUNNING;
+ efx->state = STATE_READY;
rc = efx_register_netdev(efx);
if (rc)
@@ -2727,7 +2727,7 @@ static int efx_pm_freeze(struct device *dev)
{
struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
- efx->state = STATE_FINI;
+ efx->state = STATE_UNINIT;
netif_device_detach(efx->net_dev);
@@ -2741,8 +2741,6 @@ static int efx_pm_thaw(struct device *dev)
{
struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
- efx->state = STATE_INIT;
-
efx_start_interrupts(efx, false);
mutex_lock(&efx->mac_lock);
@@ -2753,7 +2751,7 @@ static int efx_pm_thaw(struct device *dev)
netif_device_attach(efx->net_dev);
- efx->state = STATE_RUNNING;
+ efx->state = STATE_READY;
efx->type->resume_wol(efx);
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 8cba2df..5d0e2a3 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -531,7 +531,7 @@ static void efx_ethtool_self_test(struct net_device *net_dev,
ASSERT_RTNL();
- if (efx->state != STATE_RUNNING) {
+ if (efx->state != STATE_READY) {
rc = -EIO;
goto fail1;
}
diff --git a/drivers/net/ethernet/sfc/falcon_boards.c b/drivers/net/ethernet/sfc/falcon_boards.c
index 8687a6c..ec1e99d 100644
--- a/drivers/net/ethernet/sfc/falcon_boards.c
+++ b/drivers/net/ethernet/sfc/falcon_boards.c
@@ -380,7 +380,7 @@ static ssize_t set_phy_flash_cfg(struct device *dev,
new_mode = PHY_MODE_SPECIAL;
if (!((old_mode ^ new_mode) & PHY_MODE_SPECIAL)) {
err = 0;
- } else if (efx->state != STATE_RUNNING || netif_running(efx->net_dev)) {
+ } else if (efx->state != STATE_READY || netif_running(efx->net_dev)) {
err = -EBUSY;
} else {
/* Reset the PHY, reconfigure the MAC and enable/disable
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index a4fe9a7..7ab1232 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -430,11 +430,9 @@ enum efx_int_mode {
#define EFX_INT_MODE_USE_MSI(x) (((x)->interrupt_mode) <= EFX_INT_MODE_MSI)
enum nic_state {
- STATE_INIT = 0,
- STATE_RUNNING = 1,
- STATE_FINI = 2,
- STATE_DISABLED = 3,
- STATE_MAX,
+ STATE_UNINIT = 0, /* device being probed/removed or is frozen */
+ STATE_READY = 1, /* hardware ready and netdev registered */
+ STATE_DISABLED = 2, /* device disabled due to hardware errors */
};
/*
@@ -654,7 +652,7 @@ struct vfdi_status;
* @irq_rx_adaptive: Adaptive IRQ moderation enabled for RX event queues
* @irq_rx_moderation: IRQ moderation time for RX event queues
* @msg_enable: Log message enable flags
- * @state: Device state flag. Serialised by the rtnl_lock.
+ * @state: Device state number (%STATE_*). Serialised by the rtnl_lock.
* @reset_pending: Bitmask for pending resets
* @tx_queue: TX DMA queues
* @rx_queue: RX DMA queues
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 07/16] sfc: Hold the RTNL lock for more of the suspend/resume cycle
From: Ben Hutchings @ 2012-08-24 19:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
I don't think these PM functions can race with userland net device
operations, but it's much easier to reason about locking if state is
consistently guarded by the same lock.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 9f88ad8..e5f3a17 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2727,6 +2727,8 @@ static int efx_pm_freeze(struct device *dev)
{
struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
+ rtnl_lock();
+
efx->state = STATE_UNINIT;
netif_device_detach(efx->net_dev);
@@ -2734,6 +2736,8 @@ static int efx_pm_freeze(struct device *dev)
efx_stop_all(efx);
efx_stop_interrupts(efx, false);
+ rtnl_unlock();
+
return 0;
}
@@ -2741,6 +2745,8 @@ static int efx_pm_thaw(struct device *dev)
{
struct efx_nic *efx = pci_get_drvdata(to_pci_dev(dev));
+ rtnl_lock();
+
efx_start_interrupts(efx, false);
mutex_lock(&efx->mac_lock);
@@ -2755,6 +2761,8 @@ static int efx_pm_thaw(struct device *dev)
efx->type->resume_wol(efx);
+ rtnl_unlock();
+
/* Reschedule any quenched resets scheduled during efx_pm_freeze() */
queue_work(reset_workqueue, &efx->reset_work);
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 08/16] sfc: Keep disabled NICs quiescent during suspend/resume
From: Ben Hutchings @ 2012-08-24 19:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
Currently we ignore and clear the disabled state.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 28 ++++++++++++++++------------
1 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index e5f3a17..f02591b 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2729,12 +2729,14 @@ static int efx_pm_freeze(struct device *dev)
rtnl_lock();
- efx->state = STATE_UNINIT;
+ if (efx->state != STATE_DISABLED) {
+ efx->state = STATE_UNINIT;
- netif_device_detach(efx->net_dev);
+ netif_device_detach(efx->net_dev);
- efx_stop_all(efx);
- efx_stop_interrupts(efx, false);
+ efx_stop_all(efx);
+ efx_stop_interrupts(efx, false);
+ }
rtnl_unlock();
@@ -2747,19 +2749,21 @@ static int efx_pm_thaw(struct device *dev)
rtnl_lock();
- efx_start_interrupts(efx, false);
+ if (efx->state != STATE_DISABLED) {
+ efx_start_interrupts(efx, false);
- mutex_lock(&efx->mac_lock);
- efx->phy_op->reconfigure(efx);
- mutex_unlock(&efx->mac_lock);
+ mutex_lock(&efx->mac_lock);
+ efx->phy_op->reconfigure(efx);
+ mutex_unlock(&efx->mac_lock);
- efx_start_all(efx);
+ efx_start_all(efx);
- netif_device_attach(efx->net_dev);
+ netif_device_attach(efx->net_dev);
- efx->state = STATE_READY;
+ efx->state = STATE_READY;
- efx->type->resume_wol(efx);
+ efx->type->resume_wol(efx);
+ }
rtnl_unlock();
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH] [v3] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped device
From: Stephen Warren @ 2012-08-24 19:51 UTC (permalink / raw)
To: Timur Tabi
Cc: Andy Fleming, David Miller, ddaney.cavm, devicetree-discuss,
netdev
In-Reply-To: <1345835453-8611-1-git-send-email-timur@freescale.com>
On 08/24/2012 01:10 PM, Timur Tabi wrote:
> Add support for an MDIO bus multiplexer controlled by a simple memory-mapped
> device, like an FPGA. The device must be memory-mapped and contain only
> 8-bit registers (which keeps things simple).
>
> Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
> to the localbus.
> +++ b/Documentation/devicetree/bindings/net/mdio-mux-mmioreg.txt
> +Properties for an MDIO bus multiplexer controlled by a memory-mapped device
> +
> +This is a special case of a MDIO bus multiplexer. A memory-mapped device,
> +like an FPGA, is used to control which child bus is connected. The mdio-mux
> +node must be a child of the memory-mapped device. The driver currently only
> +supports devices with eight-bit registers.
That last sentence seems like a property of the driver, not the binding;
I could easily anticipate allowing the size to be 1 or 2 or 4, and a
driver adapter to that in the future.
Otherwise, this binding looks great now.
> +++ b/drivers/net/phy/mdio-mux-mmioreg.c
> +static int mdio_mux_mmioreg_switch_fn(int current_child, int desired_child,
> + void *data)
> +{
> + struct mdio_mux_mmioreg_state *s = data;
> +
> + if (current_child ^ desired_child) {
> + void *p = ioremap(s->phys, 1);
> + uint8_t x, y;
> +
> + if (!p)
> + return -ENOMEM;
Why not map it during probe?
> + x = ioread8(p);
> + y = (x & ~s->mask) | desired_child;
> + if (x != y) {
Isn't that always true, given if (current_child ^ desired_child) above?
> + iowrite8((x & ~s->mask) | desired_child, p);
> + pr_debug("%s: %02x -> %02x\n", __func__, x, y);
> + }
> +
> + iounmap(p);
> + }
> +
> + return 0;
> +}
^ permalink raw reply
* [PATCH net-next 09/16] sfc: Hold RTNL lock (only) when calling efx_stop_interrupts()
From: Ben Hutchings @ 2012-08-24 19:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
Interrupt state should be consistently guarded by the RTNL lock once
the net device is registered.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index f02591b..629029e 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2164,9 +2164,9 @@ void efx_reset_down(struct efx_nic *efx, enum reset_type method)
EFX_ASSERT_RESET_SERIALISED(efx);
efx_stop_all(efx);
- mutex_lock(&efx->mac_lock);
-
efx_stop_interrupts(efx, false);
+
+ mutex_lock(&efx->mac_lock);
if (efx->port_initialized && method != RESET_TYPE_INVISIBLE)
efx->phy_op->fini(efx);
efx->type->fini(efx);
@@ -2492,11 +2492,11 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
rtnl_lock();
efx->state = STATE_UNINIT;
dev_close(efx->net_dev);
+ efx_stop_interrupts(efx, false);
/* Allow any queued efx_resets() to complete */
rtnl_unlock();
- efx_stop_interrupts(efx, false);
efx_sriov_fini(efx);
efx_unregister_netdev(efx);
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 10/16] sfc: Never try to stop and start a NIC that is disabled
From: Ben Hutchings @ 2012-08-24 19:51 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
efx_change_mtu() and efx_realloc_channels() each stop and start much
of the NIC, even if it has been disabled. Since efx_start_all() is a
no-op when the NIC is disabled, this is probably harmless in the case
of efx_change_mtu(), but efx_realloc_channels() also reenables
interrupts which could be a bad thing to do.
Change efx_start_all() and efx_start_interrupts() to assert that the
NIC is not disabled, but make efx_stop_interrupts() do nothing if the
NIC is disabled (since it is already stopped), consistent with
efx_stop_all().
Update comments for efx_start_all() and efx_stop_all() to describe
their purpose and preconditions more accurately.
Add a common function to check and log if the NIC is disabled, and use
it in efx_net_open(), efx_change_mtu() and efx_realloc_channels().
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 65 +++++++++++++++++++++++++++-------------
1 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 629029e..977fc3a 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -207,6 +207,16 @@ static void efx_stop_all(struct efx_nic *efx);
ASSERT_RTNL(); \
} while (0)
+static int efx_check_disabled(struct efx_nic *efx)
+{
+ if (efx->state == STATE_DISABLED) {
+ netif_err(efx, drv, efx->net_dev,
+ "device is disabled due to earlier errors\n");
+ return -EIO;
+ }
+ return 0;
+}
+
/**************************************************************************
*
* Event queue processing
@@ -740,7 +750,11 @@ efx_realloc_channels(struct efx_nic *efx, u32 rxq_entries, u32 txq_entries)
struct efx_channel *other_channel[EFX_MAX_CHANNELS], *channel;
u32 old_rxq_entries, old_txq_entries;
unsigned i, next_buffer_table = 0;
- int rc = 0;
+ int rc;
+
+ rc = efx_check_disabled(efx);
+ if (rc)
+ return rc;
/* Not all channels should be reallocated. We must avoid
* reallocating their buffer table entries.
@@ -1375,6 +1389,8 @@ static void efx_start_interrupts(struct efx_nic *efx, bool may_keep_eventq)
{
struct efx_channel *channel;
+ BUG_ON(efx->state == STATE_DISABLED);
+
if (efx->legacy_irq)
efx->legacy_irq_enabled = true;
efx_nic_enable_interrupts(efx);
@@ -1392,6 +1408,9 @@ static void efx_stop_interrupts(struct efx_nic *efx, bool may_keep_eventq)
{
struct efx_channel *channel;
+ if (efx->state == STATE_DISABLED)
+ return;
+
efx_mcdi_mode_poll(efx);
efx_nic_disable_interrupts(efx);
@@ -1543,22 +1562,21 @@ static int efx_probe_all(struct efx_nic *efx)
return rc;
}
-/* Called after previous invocation(s) of efx_stop_all, restarts the port,
- * kernel transmit queues and NAPI processing, and ensures that the port is
- * scheduled to be reconfigured. This function is safe to call multiple
- * times when the NIC is in any state.
+/* If the interface is supposed to be running but is not, start
+ * the hardware and software data path, regular activity for the port
+ * (MAC statistics, link polling, etc.) and schedule the port to be
+ * reconfigured. Interrupts must already be enabled. This function
+ * is safe to call multiple times, so long as the NIC is not disabled.
+ * Requires the RTNL lock.
*/
static void efx_start_all(struct efx_nic *efx)
{
EFX_ASSERT_RESET_SERIALISED(efx);
+ BUG_ON(efx->state == STATE_DISABLED);
/* Check that it is appropriate to restart the interface. All
* of these flags are safe to read under just the rtnl lock */
- if (efx->port_enabled)
- return;
- if ((efx->state != STATE_READY) && (efx->state != STATE_UNINIT))
- return;
- if (!netif_running(efx->net_dev))
+ if (efx->port_enabled || !netif_running(efx->net_dev))
return;
efx_start_port(efx);
@@ -1592,11 +1610,11 @@ static void efx_flush_all(struct efx_nic *efx)
cancel_work_sync(&efx->mac_work);
}
-/* Quiesce hardware and software without bringing the link down.
- * Safe to call multiple times, when the nic and interface is in any
- * state. The caller is guaranteed to subsequently be in a position
- * to modify any hardware and software state they see fit without
- * taking locks. */
+/* Quiesce the hardware and software data path, and regular activity
+ * for the port without bringing the link down. Safe to call multiple
+ * times with the NIC in almost any state, but interrupts should be
+ * enabled. Requires the RTNL lock.
+ */
static void efx_stop_all(struct efx_nic *efx)
{
EFX_ASSERT_RESET_SERIALISED(efx);
@@ -1830,13 +1848,16 @@ static void efx_netpoll(struct net_device *net_dev)
static int efx_net_open(struct net_device *net_dev)
{
struct efx_nic *efx = netdev_priv(net_dev);
+ int rc;
+
EFX_ASSERT_RESET_SERIALISED(efx);
netif_dbg(efx, ifup, efx->net_dev, "opening device on CPU %d\n",
raw_smp_processor_id());
- if (efx->state == STATE_DISABLED)
- return -EIO;
+ rc = efx_check_disabled(efx);
+ if (rc)
+ return rc;
if (efx->phy_mode & PHY_MODE_SPECIAL)
return -EBUSY;
if (efx_mcdi_poll_reboot(efx) && efx_reset(efx, RESET_TYPE_ALL))
@@ -1862,10 +1883,8 @@ static int efx_net_stop(struct net_device *net_dev)
netif_dbg(efx, ifdown, efx->net_dev, "closing on CPU %d\n",
raw_smp_processor_id());
- if (efx->state != STATE_DISABLED) {
- /* Stop the device and flush all the channels */
- efx_stop_all(efx);
- }
+ /* Stop the device and flush all the channels */
+ efx_stop_all(efx);
return 0;
}
@@ -1925,9 +1944,13 @@ static void efx_watchdog(struct net_device *net_dev)
static int efx_change_mtu(struct net_device *net_dev, int new_mtu)
{
struct efx_nic *efx = netdev_priv(net_dev);
+ int rc;
EFX_ASSERT_RESET_SERIALISED(efx);
+ rc = efx_check_disabled(efx);
+ if (rc)
+ return rc;
if (new_mtu > EFX_MAX_MTU)
return -EINVAL;
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 11/16] sfc: Improve log messages in case we abort probe due to a pending reset
From: Ben Hutchings @ 2012-08-24 19:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1345837574.2694.22.camel@bwh-desktop.uk.solarflarecom.com>
The current informational message doesn't properly explain what
happens, and could also appear if we defer a reset during
suspend/resume.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 977fc3a..5555e9f 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2311,11 +2311,8 @@ static void efx_reset_work(struct work_struct *data)
/* If we're not READY then don't reset. Leave the reset_pending
* flags set so that efx_pci_probe_main will be retried */
- if (efx->state != STATE_READY) {
- netif_info(efx, drv, efx->net_dev,
- "scheduled reset quenched; NIC not ready\n");
+ if (efx->state != STATE_READY)
return;
- }
rtnl_lock();
(void)efx_reset(efx, fls(pending) - 1);
@@ -2703,6 +2700,8 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
* probably hosed anyway.
*/
if (efx->reset_pending) {
+ netif_err(efx, probe, efx->net_dev,
+ "aborting probe due to scheduled reset\n");
rc = -EIO;
goto fail4;
}
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply 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