Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 13/17] Tools: hv: Implement the KVP verb - KVP_OP_SET_IP_INFO
From: Ben Hutchings @ 2012-07-30 19:19 UTC (permalink / raw)
  To: KY Srinivasan
  Cc: Olaf Hering, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
	apw@canonical.com, netdev@vger.kernel.org
In-Reply-To: <426367E2313C2449837CD2DE46E7EAF9236AA7DB@SN2PRD0310MB382.namprd03.prod.outlook.com>

On Mon, Jul 30, 2012 at 06:32:15PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Olaf Hering [mailto:olaf@aepfle.de]
> > Sent: Monday, July 30, 2012 2:03 PM
> > To: KY Srinivasan
> > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; apw@canonical.com; netdev@vger.kernel.org;
> > ben@decadent.org.uk
> > Subject: Re: [PATCH 13/17] Tools: hv: Implement the KVP verb -
> > KVP_OP_SET_IP_INFO
> > 
> > On Tue, Jul 24, K. Y. Srinivasan wrote:
> > 
> > > +	/*
> > > +	 * Set the configuration for the specified interface with
> > > +	 * the information provided. Since there is no standard
> > > +	 * way to configure an interface, we will have an external
> > > +	 * script that does the job of configuring the interface and
> > > +	 * flushing the configuration.
> > > +	 *
> > > +	 * The parameters passed to this external script are:
> > > +	 * 1. A configuration file that has the specified configuration.
> > 
> > Maybe this should be written as 'A info file that has the requested
> > network configuration' or something like that.
> 
> That is the idea. This configuration file simply reflects all the
> information we have perhaps with some additional constant
> information. The script is free to ignore what it does not need. 
[...]

This does not strike me as a sensible interface.  If scripts are
'free to ignore' information then the KVP interface becomes unreliable
as a means for managing networking on Linux guests.  I would suggest
that at the least the script should be able to report that it did not
recognise some parts of the configuration.  This would be logged
and/or reported back to the hypervisor.

(This is separate from the issue of constant configuration lines;
for some distributions the script might recognise but ignore them
because they have no use on that distribution.  I don't see the
point in constant lines, but they don't seem to result in any
unreliability.)

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply

* imx6q: poor FEC Gigabit implementation
From: Wolfgang Grandegger @ 2012-07-30 19:24 UTC (permalink / raw)
  To: linux-arm-kernel, Linux Netdev List

Hello,

I did some Gigabit performance measurements with the i.MX6Q Sabrelite
board. I measure an UDP throughput of 470 Mb/s to an external host,
which is already very close to the theoretical limit listed in the Chip
Errata ERR004512 for the i.MX 6Dual/6Quad, Rev. C, 2/2012.

At a closer look to the driver I realized that there is no support for

  - NAPI
  - IP checksum offloading
  - Jumbo frames

According to the manual, these features are support by the ENET via
enhanced buffer descriptors. Even if they will not increase the
performance due to the mentioned system limitation, they should reduce
the interrupt and CPU load significantly. I did some quick tests with IP
checksum offloading and Jumbo frames but without success. Are there any
known issues? Is anybody already working on an enhanced FEC Gigabit
ethernet driver?

TIA.

Wolfgang.

^ permalink raw reply

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:35 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343669507.21269.33.camel@edumazet-glaptop>

On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> On Mon, 2012-07-30 at 18:16 +0100, Ben Hutchings wrote:
> > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > as 88 (actual MSS of 76 with timestamps).  Given that we have a
> > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > it is nevertheless possible to grow the window for such a connection
> > to the point that we will try to send just under 64K at once.  This
> > results in a single skb that expands to 861 segments.
> > 
> > In some drivers with TSO support, such an skb will require hundreds of
> > DMA descriptors; a substantial fraction of a TX ring or even more than
> > a full ring.  The TX queue selected for the skb may stall and trigger
> > the TX watchdog repeatedly (since the problem skb will be retried
> > after the TX reset).  This particularly affects sfc, for which the
> > issue is designated as CVE-2012-3412.  However it may be that some
> > hardware or firmware also fails to handle such an extreme TSO request
> > correctly.
> > 
> > Therefore, limit the number of segments per skb to 100.  This should
> > make no difference to behaviour unless the actual MSS is less than
> > about 700.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > ---
> 
> 
> Hmm, isnt GRO path also vulnerable ?

You mean, for forwarding?  If page fragments are used, the number of
segments is limited to MAX_SKB_FRAGS < 100.  But if skbs are aggregated
and build_skb() is not used (e.g. due to jumbo MTU) it appears we would
need an explicit limit.  Something like this:

---
From: Ben Hutchings <bhutchings@solarflare.com>
Subject: [PATCH net] tcp: Limit number of segments merged by GRO

In the case where GRO aggregates skbs that cannot be converted to
page-fragments, there is currently no limit to the number of
segments that may be merged and subsequently re-segmented by GSO.

Apply the same limit as was introduced for locally-generated GSO skbs
in 'tcp: Limit number of segments generated by GSO per skb'.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 net/ipv4/tcp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 51d8daf..a052d07 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3144,7 +3144,8 @@ out_check_final:
 					TCP_FLAG_RST | TCP_FLAG_SYN |
 					TCP_FLAG_FIN));
 
-	if (p && (!NAPI_GRO_CB(skb)->same_flow || flush))
+	if (p && (!NAPI_GRO_CB(skb)->same_flow || flush ||
+		  NAPI_GRO_CB(p)->count == TCP_MAX_GSO_SEGS))
 		pp = head;
 
 out:
---

> An alternative would be to drop such frames in the ndo_start_xmit(), and
> cap sk->sk_gso_max_size (since skb are no longer orphaned...)

I have implemented that workaround for the out-of-tree version of sfc.
For the in-tree driver, I thought it would be better to limit the number
of segments at source, which will avoid penalising any cases where the
window can grow so much larger than MSS.

> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> driver sets to whatever limit ?

Yes, that's another option.

Ben.

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

^ permalink raw reply related

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:41 UTC (permalink / raw)
  To: Ben Greear; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <5016C305.7080907@candelatech.com>

On Mon, 2012-07-30 at 10:23 -0700, Ben Greear wrote:
> On 07/30/2012 10:16 AM, Ben Hutchings wrote:
> > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > as 88 (actual MSS of 76 with timestamps).  Given that we have a
> > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > it is nevertheless possible to grow the window for such a connection
> > to the point that we will try to send just under 64K at once.  This
> > results in a single skb that expands to 861 segments.
> >
> > In some drivers with TSO support, such an skb will require hundreds of
> > DMA descriptors; a substantial fraction of a TX ring or even more than
> > a full ring.  The TX queue selected for the skb may stall and trigger
> > the TX watchdog repeatedly (since the problem skb will be retried
> > after the TX reset).  This particularly affects sfc, for which the
> > issue is designated as CVE-2012-3412.  However it may be that some
> > hardware or firmware also fails to handle such an extreme TSO request
> > correctly.
> >
> > Therefore, limit the number of segments per skb to 100.  This should
> > make no difference to behaviour unless the actual MSS is less than
> > about 700.
> 
> Please do not do this...or at least allow over-rides.  We love
> the trick of seting very small MSS and making the NICs generate
> huge numbers of small TCP frames with efficient user-space
> logic.   We use this for stateful TCP load testing when high
> numbers of tcp packets-per-second is desired.

Please test whether this actually makes a difference - my suspicion is
that 100 segments per skb is easily enough to prevent the host being a
bottleneck.

> Intel NICs, including 10G, work just fine with minimal MSS
> in this scenario.

I'll leave this to the Intel maintainers to answer.

Ben.

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

^ permalink raw reply

* RE: [PATCH net,1/1] hyperv: Add support for setting MAC from within guests
From: Haiyang Zhang @ 2012-07-30 19:48 UTC (permalink / raw)
  To: Olaf Hering
  Cc: davem@davemloft.net, netdev@vger.kernel.org, KY Srinivasan,
	linux-kernel@vger.kernel.org, devel@linuxdriverproject.org
In-Reply-To: <20120730123927.GA29459@aepfle.de>



> -----Original Message-----
> From: Olaf Hering [mailto:olaf@aepfle.de]
> Sent: Monday, July 30, 2012 8:39 AM
> To: Haiyang Zhang
> Cc: davem@davemloft.net; netdev@vger.kernel.org; KY Srinivasan; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org
> Subject: Re: [PATCH net,1/1] hyperv: Add support for setting MAC from
> within guests
> 
> On Tue, Jul 10, Haiyang Zhang wrote:
> 
> > diff --git a/drivers/net/hyperv/rndis_filter.c
> > b/drivers/net/hyperv/rndis_filter.c
> > index 981ebb1..fbf5394 100644
> > --- a/drivers/net/hyperv/rndis_filter.c
> > +++ b/drivers/net/hyperv/rndis_filter.c
> > @@ -47,6 +48,7 @@ struct rndis_request {
> >  	struct hv_page_buffer buf;
> >  	/* FIXME: We assumed a fixed size request here. */
> >  	struct rndis_message request_msg;
> > +	u8 ext[100];
> 
> This array is not referenced in the patch.
> Please add a comment to the code what the purpose of this array is, and why
> its size is 100 bytes.

It's a buffer for the extended info after the RNDIS message. It's referenced based
on the data offset in the RNDIS message. 100 byte size is enough for current needs, 
and should be sufficient for the near future.

I will add a comment to the code.

Thanks,
- Haiyang


^ permalink raw reply

* [PATCH iproute2] ss: Report MSS from internal TCP information
From: Ben Hutchings @ 2012-07-30 19:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 misc/ss.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index cf529ef..8ad830b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1392,6 +1392,8 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
 			       (double)info->tcpi_rttvar/1000);
 		if (info->tcpi_ato)
 			printf(" ato:%g", (double)info->tcpi_ato/1000);
+		if (info->tcpi_snd_mss)
+			printf(" mss:%d", info->tcpi_snd_mss);
 		if (info->tcpi_snd_cwnd != 2)
 			printf(" cwnd:%d", info->tcpi_snd_cwnd);
 		if (info->tcpi_snd_ssthresh < 0xFFFF)
-- 
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 net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 19:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343676952.2667.26.camel@bwh-desktop.uk.solarflarecom.com>

On Mon, 2012-07-30 at 20:35 +0100, Ben Hutchings wrote:
> On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > On Mon, 2012-07-30 at 18:16 +0100, Ben Hutchings wrote:
> > > A peer (or local user) may cause TCP to use a nominal MSS of as little
> > > as 88 (actual MSS of 76 with timestamps).  Given that we have a
> > > sufficiently prodigious local sender and the peer ACKs quickly enough,
> > > it is nevertheless possible to grow the window for such a connection
> > > to the point that we will try to send just under 64K at once.  This
> > > results in a single skb that expands to 861 segments.
> > > 
> > > In some drivers with TSO support, such an skb will require hundreds of
> > > DMA descriptors; a substantial fraction of a TX ring or even more than
> > > a full ring.  The TX queue selected for the skb may stall and trigger
> > > the TX watchdog repeatedly (since the problem skb will be retried
> > > after the TX reset).  This particularly affects sfc, for which the
> > > issue is designated as CVE-2012-3412.  However it may be that some
> > > hardware or firmware also fails to handle such an extreme TSO request
> > > correctly.
> > > 
> > > Therefore, limit the number of segments per skb to 100.  This should
> > > make no difference to behaviour unless the actual MSS is less than
> > > about 700.
[...]
> > An alternative would be to drop such frames in the ndo_start_xmit(), and
> > cap sk->sk_gso_max_size (since skb are no longer orphaned...)
> 
> I have implemented that workaround for the out-of-tree version of sfc.
> For the in-tree driver, I thought it would be better to limit the number
> of segments at source, which will avoid penalising any cases where the
> window can grow so much larger than MSS.
[...]

I mean any *legitimate* cases where this can happen.

Ben.

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

^ permalink raw reply

* Re: [RFC v2 2/2] net: Add support for NTB virtual ethernet device
From: Jiri Pirko @ 2012-07-30 20:09 UTC (permalink / raw)
  To: Jon Mason; +Cc: linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120730181910.GB987@jonmason-lab>

Mon, Jul 30, 2012 at 08:19:11PM CEST, jon.mason@intel.com wrote:
>On Mon, Jul 30, 2012 at 04:02:16PM +0200, Jiri Pirko wrote:
>> Mon, Jul 30, 2012 at 02:26:34AM CEST, jon.mason@intel.com wrote:
>> >+static int __devinit ntb_netdev_probe(struct pci_dev *pdev)
>> >+{
>> >+	struct net_device *ndev;
>> >+	struct ntb_netdev *dev;
>> >+	int rc;
>> >+
>> >+	ndev = alloc_etherdev(sizeof(struct ntb_netdev));
>> >+	if (!ndev)
>> >+		return -ENOMEM;
>> >+
>> >+	dev = netdev_priv(ndev);
>> >+	dev->ndev = ndev;
>> >+	dev->pdev = pdev;
>> >+	BUG_ON(!dev->pdev);
>> >+	ndev->features = NETIF_F_HIGHDMA;
>> >+
>> >+	//ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>> 	^^ I guess you forgot to un-comment this.
>
>Oops.  An easy fix.  Anything else?

The rest looks good to me.

>
>Thanks,
>Jon

^ permalink raw reply

* [PATCH 1/2] drivers: net: ethernet: cpsw: Add SOC dependency support for cpsw dependent modules
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, Mugunthan V N
In-Reply-To: <1343679434-2369-1-git-send-email-mugunthanvnm@ti.com>

cpsw is dependent on davinci_cpdma and davinci_mdio, so adding SOC support for
dependent modules

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 drivers/net/ethernet/ti/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 1b173a6..b26cbda 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
 
 config TI_DAVINCI_MDIO
 	tristate "TI DaVinci MDIO Support"
-	depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 )
+	depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
 	select PHYLIB
 	---help---
 	  This driver supports TI's DaVinci MDIO module.
@@ -42,7 +42,7 @@ config TI_DAVINCI_MDIO
 
 config TI_DAVINCI_CPDMA
 	tristate "TI DaVinci CPDMA Support"
-	depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 )
+	depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
 	---help---
 	  This driver supports TI's DaVinci CPDMA dma engine.
 
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 2/2] drivers: net: ethernet: cpsw: Add device tree support to CPSW
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, Mugunthan V N
In-Reply-To: <1343679434-2369-1-git-send-email-mugunthanvnm@ti.com>

This patch adds device tree support for cpsw driver

Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  104 ++++++++++++++
 drivers/net/ethernet/ti/cpsw.c                 |  174 +++++++++++++++++++++++-
 2 files changed, 272 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw.txt

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
new file mode 100644
index 0000000..acca48c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -0,0 +1,104 @@
+TI SoC Ethernet Switch Controller Device Tree Bindings
+------------------------------------------------------
+
+Required properties:
+- compatible		: Should be "ti,cpsw"
+- reg			: physical base address and size of the cpsw
+			  registers map
+- interrupts		: property with a value describing the interrupt
+			  number
+- interrupt-parent	: The parent interrupt controller
+- cpdma_channels 	: Specifies number of channels in CPDMA
+- host_port_no		: Specifies host port shift
+- cpdma_reg_ofs		: Specifies CPDMA submodule register offset
+- ale_reg_ofs		: Specifies ALE submodule register offset
+- ale_entries		: Specifies No of entries ALE can hold
+- host_port_reg_ofs	: Specifies host port register offset
+- hw_stats_reg_ofs	: Specifies hardware statistics register offset
+- bd_ram_ofs		: Specifies internal desciptor RAM offset
+- bd_ram_size		: Specifies internal descriptor RAM size
+- rx_descs		: Specifies number of Rx descriptors
+- mac_control		: Specifies Default MAC control register content
+			  for the specific platform
+- slaves		: Specifies number for slaves
+- slave_reg_ofs		: Specifies slave register offset
+- sliver_reg_ofs	: Specifies slave sliver register offset
+- phy_id		: Specifies slave phy id
+- mac-address		: Specifies slave MAC address
+
+Optional properties:
+- ti,hwmods		: Must be "cpgmac0"
+- no_bd_ram		: Must be 0 or 1
+
+Note: "ti,hwmods" field is used to fetch the base address and irq
+resources from TI, omap hwmod data base during device registration.
+Future plan is to migrate hwmod data base contents into device tree
+blob so that, all the required data will be used from device tree dts
+file.
+
+Examples:
+
+	mac: ethernet@4A100000 {
+		compatible = "ti,cpsw";
+		reg = <0x4A100000 0x1000>;
+		interrupts = <55 0x4>;
+		interrupt-parent = <&intc>;
+		cpdma_channels = 8;
+		host_port_no = 0;
+		cpdma_reg_ofs = 0x800;
+		ale_reg_ofs = 0xd00;
+		ale_entries = 1024;
+		host_port_reg_ofs = 0x108;
+		hw_stats_reg_ofs = 0x900;
+		bd_ram_ofs = 0x2000;
+		bd_ram_size = 0x2000;
+		no_bd_ram = 0;
+		rx_descs = 64;
+		mac_control = 0x20;
+		slaves = 2;
+		slave@0 {
+			slave_reg_ofs = 0x208;
+			sliver_reg_ofs = 0xd80;
+			phy_id = "davinci_mdio-0:00"
+			mac-address = [00 04 9F 01 1B B8];
+		};
+		slave@1 {
+			slave_reg_ofs = 0x208;
+			sliver_reg_ofs = 0xd80;
+			phy_id = "davinci_mdio-0:01"
+			mac-address = [00 04 9F 01 1B B9];
+		};
+	};
+
+(or)
+
+	mac: ethernet@4A100000 {
+		compatible = "ti,cpsw";
+		ti,hwmods = "cpgmac0";
+		cpdma_channels = 8;
+		host_port_no = 0;
+		cpdma_reg_ofs = 0x800;
+		ale_reg_ofs = 0xd00;
+		ale_entries = 1024;
+		host_port_reg_ofs = 0x108;
+		hw_stats_reg_ofs = 0x900;
+		bd_ram_ofs = 0x2000;
+		bd_ram_size = 0x2000;
+		no_bd_ram = 0;
+		rx_descs = 64;
+		mac_control = 0x20;
+		slaves = 2;
+		slave@0 {
+			slave_reg_ofs = 0x208;
+			sliver_reg_ofs = 0xd80;
+			phy_id = "davinci_mdio-0:00"
+			mac-address = [00 04 9F 01 1B B8];
+		};
+		slave@1 {
+			slave_reg_ofs = 0x208;
+			sliver_reg_ofs = 0xd80;
+			phy_id = "davinci_mdio-0:01"
+			mac-address = [00 04 9F 01 1B B9];
+		};
+
+	};
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 1e5d85b..0cbc0e5 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -28,6 +28,9 @@
 #include <linux/workqueue.h>
 #include <linux/delay.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_net.h>
+#include <linux/of_device.h>
 
 #include <linux/platform_data/cpsw.h>
 
@@ -709,6 +712,158 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	slave->sliver	= regs + data->sliver_reg_ofs;
 }
 
+static int cpsw_probe_dt(struct cpsw_platform_data *data,
+			 struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct device_node *slave_node;
+	int i = 0, ret;
+	u32 prop;
+
+	if (!node)
+		return -EINVAL;
+
+	if (of_property_read_u32(node, "slaves", &prop)) {
+		pr_err("Missing slaves property in the DT.\n");
+		return -EINVAL;
+	}
+	data->slaves = prop;
+
+	data->slave_data = kzalloc(sizeof(struct cpsw_slave_data) *
+				   data->slaves, GFP_KERNEL);
+	if (!data->slave_data) {
+		pr_err("Could not allocate slave memory.\n");
+		return -EINVAL;
+	}
+
+	data->no_bd_ram = of_property_read_bool(node, "no_bd_ram");
+
+	if (of_property_read_u32(node, "cpdma_channels", &prop)) {
+		pr_err("Missing cpdma_channels property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->channels = prop;
+
+	if (of_property_read_u32(node, "host_port_no", &prop)) {
+		pr_err("Missing host_port_no property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->host_port_num = prop;
+
+	if (of_property_read_u32(node, "cpdma_reg_ofs", &prop)) {
+		pr_err("Missing cpdma_reg_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->cpdma_reg_ofs = prop;
+
+	if (of_property_read_u32(node, "cpdma_sram_ofs", &prop)) {
+		pr_err("Missing cpdma_sram_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->cpdma_sram_ofs = prop;
+
+	if (of_property_read_u32(node, "ale_reg_ofs", &prop)) {
+		pr_err("Missing ale_reg_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->ale_reg_ofs = prop;
+
+	if (of_property_read_u32(node, "ale_entries", &prop)) {
+		pr_err("Missing ale_entries property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->ale_entries = prop;
+
+	if (of_property_read_u32(node, "host_port_reg_ofs", &prop)) {
+		pr_err("Missing host_port_reg_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->host_port_reg_ofs = prop;
+
+	if (of_property_read_u32(node, "hw_stats_reg_ofs", &prop)) {
+		pr_err("Missing hw_stats_reg_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->hw_stats_reg_ofs = prop;
+
+	if (of_property_read_u32(node, "bd_ram_ofs", &prop)) {
+		pr_err("Missing bd_ram_ofs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->bd_ram_ofs = prop;
+
+	if (of_property_read_u32(node, "bd_ram_size", &prop)) {
+		pr_err("Missing bd_ram_size property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->bd_ram_size = prop;
+
+	if (of_property_read_u32(node, "rx_descs", &prop)) {
+		pr_err("Missing rx_descs property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->rx_descs = prop;
+
+	if (of_property_read_u32(node, "mac_control", &prop)) {
+		pr_err("Missing mac_control property in the DT.\n");
+		ret = -EINVAL;
+		goto error_ret;
+	}
+	data->mac_control = prop;
+
+	for_each_child_of_node(node, slave_node) {
+		struct cpsw_slave_data *slave_data = data->slave_data + i;
+		const char *phy_id = NULL;
+		const void *mac_addr = NULL;
+
+		if (of_property_read_string(slave_node, "phy_id", &phy_id)) {
+			pr_err("Missing slave[%d] phy_id property\n", i);
+			ret = -EINVAL;
+			goto error_ret;
+		}
+		slave_data->phy_id = phy_id;
+
+		if (of_property_read_u32(slave_node, "slave_reg_ofs", &prop)) {
+			pr_err("Missing slave[%d] slave_reg_ofs property\n", i);
+			ret = -EINVAL;
+			goto error_ret;
+		}
+		slave_data->slave_reg_ofs = prop;
+
+		if (of_property_read_u32(slave_node, "sliver_reg_ofs",
+					 &prop)) {
+			pr_err("Missing slave[%d] sliver_reg_ofs property\n",
+				i);
+			ret = -EINVAL;
+			goto error_ret;
+		}
+		slave_data->sliver_reg_ofs = prop;
+
+		mac_addr = of_get_mac_address(slave_node);
+		if (mac_addr)
+			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
+
+		i++;
+	}
+
+	return 0;
+
+error_ret:
+	kfree(data->slave_data);
+	return ret;
+}
+
 static int __devinit cpsw_probe(struct platform_device *pdev)
 {
 	struct cpsw_platform_data	*data = pdev->dev.platform_data;
@@ -720,11 +875,6 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 	struct resource			*res;
 	int ret = 0, i, k = 0;
 
-	if (!data) {
-		pr_err("platform data missing\n");
-		return -ENODEV;
-	}
-
 	ndev = alloc_etherdev(sizeof(struct cpsw_priv));
 	if (!ndev) {
 		pr_err("error allocating net_device\n");
@@ -734,13 +884,19 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, ndev);
 	priv = netdev_priv(ndev);
 	spin_lock_init(&priv->lock);
-	priv->data = *data;
 	priv->pdev = pdev;
 	priv->ndev = ndev;
 	priv->dev  = &ndev->dev;
 	priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
 	priv->rx_packet_max = max(rx_packet_max, 128);
 
+	if (cpsw_probe_dt(&priv->data, pdev)) {
+		pr_err("cpsw: platform data missing\n");
+		ret = -ENODEV;
+		goto clean_ndev_ret;
+	}
+	data = &priv->data;
+
 	if (is_valid_ether_addr(data->slave_data[0].mac_addr)) {
 		memcpy(priv->mac_addr, data->slave_data[0].mac_addr, ETH_ALEN);
 		pr_info("Detected MACID = %pM", priv->mac_addr);
@@ -996,11 +1152,17 @@ static const struct dev_pm_ops cpsw_pm_ops = {
 	.resume		= cpsw_resume,
 };
 
+static const struct of_device_id cpsw_of_mtable[] = {
+	{ .compatible = "ti,cpsw", },
+	{ /* sentinel */ },
+};
+
 static struct platform_driver cpsw_driver = {
 	.driver = {
 		.name	 = "cpsw",
 		.owner	 = THIS_MODULE,
 		.pm	 = &cpsw_pm_ops,
+		.of_match_table = of_match_ptr(cpsw_of_mtable),
 	},
 	.probe = cpsw_probe,
 	.remove = __devexit_p(cpsw_remove),
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 0/2] Add device tree support and resolving SOC dependency to cpsw driver
From: Mugunthan V N @ 2012-07-30 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, Mugunthan V N

This patch set adds SOC dependency for CPSW dependent modules and adds support
for device tree for CPSW driver

Mugunthan V N (2):
  drivers: net: ethernet: cpsw: Add SOC dependency support for cpsw
    dependent modules
  drivers: net: ethernet: cpsw: Add device tree support to CPSW

 Documentation/devicetree/bindings/net/cpsw.txt |  104 ++++++++++++++
 drivers/net/ethernet/ti/Kconfig                |    4 +-
 drivers/net/ethernet/ti/cpsw.c                 |  174 +++++++++++++++++++++++-
 3 files changed, 274 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/cpsw.txt

^ permalink raw reply

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Greear @ 2012-07-30 21:00 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, netdev, linux-net-drivers
In-Reply-To: <1343677270.2667.31.camel@bwh-desktop.uk.solarflarecom.com>

On 07/30/2012 12:41 PM, Ben Hutchings wrote:
> On Mon, 2012-07-30 at 10:23 -0700, Ben Greear wrote:
>> On 07/30/2012 10:16 AM, Ben Hutchings wrote:
>>> A peer (or local user) may cause TCP to use a nominal MSS of as little
>>> as 88 (actual MSS of 76 with timestamps).  Given that we have a
>>> sufficiently prodigious local sender and the peer ACKs quickly enough,
>>> it is nevertheless possible to grow the window for such a connection
>>> to the point that we will try to send just under 64K at once.  This
>>> results in a single skb that expands to 861 segments.
>>>
>>> In some drivers with TSO support, such an skb will require hundreds of
>>> DMA descriptors; a substantial fraction of a TX ring or even more than
>>> a full ring.  The TX queue selected for the skb may stall and trigger
>>> the TX watchdog repeatedly (since the problem skb will be retried
>>> after the TX reset).  This particularly affects sfc, for which the
>>> issue is designated as CVE-2012-3412.  However it may be that some
>>> hardware or firmware also fails to handle such an extreme TSO request
>>> correctly.
>>>
>>> Therefore, limit the number of segments per skb to 100.  This should
>>> make no difference to behaviour unless the actual MSS is less than
>>> about 700.
>>
>> Please do not do this...or at least allow over-rides.  We love
>> the trick of seting very small MSS and making the NICs generate
>> huge numbers of small TCP frames with efficient user-space
>> logic.   We use this for stateful TCP load testing when high
>> numbers of tcp packets-per-second is desired.
>
> Please test whether this actually makes a difference - my suspicion is
> that 100 segments per skb is easily enough to prevent the host being a
> bottleneck.

Any CPU I can save I can use for other tasks.  If we can use the
NIC's offload features to segment pkts, then we get near linear
increase in pkts-per-second by adding NICs..at least up to whatever
the total bandwidth of the system is...

If you want to have the OS default to a safe value, that is
fine by me..but please give us a tunable so that we can get
the old behaviour.

It's always possible I'm not the only one using this,
and I think it would be considered bad form to break
existing features and provide no work-around.

Thanks,
Ben

>
>> Intel NICs, including 10G, work just fine with minimal MSS
>> in this scenario.
>
> I'll leave this to the Intel maintainers to answer.
>
> Ben.
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* [PATCH net] net: Remove unused variables in rt_cache_stat
From: Vijay Subramanian @ 2012-07-30 21:45 UTC (permalink / raw)
  To: netdev; +Cc: davem, Vijay Subramanian

With the removal of the routing cache, some variables in rt_cache_stat are no
longer used. Remove them from rt_cache_stat and do not print them out in
/proc/net/stat/rt_cache.

Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
checkpatch complains that the seq_printf line is over 80 chars which was already
the case. I left it as is to aid in grepping the sources. 

 include/net/route.h |    8 --------
 net/ipv4/route.c    |   16 +++-------------
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index 8c52bc6..69e54f9 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -84,22 +84,14 @@ struct ip_rt_acct {
 };
 
 struct rt_cache_stat {
-        unsigned int in_hit;
         unsigned int in_slow_tot;
         unsigned int in_slow_mc;
         unsigned int in_no_route;
         unsigned int in_brd;
         unsigned int in_martian_dst;
         unsigned int in_martian_src;
-        unsigned int out_hit;
         unsigned int out_slow_tot;
         unsigned int out_slow_mc;
-        unsigned int gc_total;
-        unsigned int gc_ignored;
-        unsigned int gc_goal_miss;
-        unsigned int gc_dst_overflow;
-        unsigned int in_hlist_search;
-        unsigned int out_hlist_search;
 };
 
 extern struct ip_rt_acct __percpu *ip_rt_acct;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index fc1a81c..114a6c9 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -298,14 +298,12 @@ static int rt_cpu_seq_show(struct seq_file *seq, void *v)
 	struct rt_cache_stat *st = v;
 
 	if (v == SEQ_START_TOKEN) {
-		seq_printf(seq, "entries  in_hit in_slow_tot in_slow_mc in_no_route in_brd in_martian_dst in_martian_src  out_hit out_slow_tot out_slow_mc  gc_total gc_ignored gc_goal_miss gc_dst_overflow in_hlist_search out_hlist_search\n");
+		seq_printf(seq, "entries  in_slow_tot in_slow_mc in_no_route in_brd in_martian_dst in_martian_src out_slow_tot out_slow_mc\n");
 		return 0;
 	}
 
-	seq_printf(seq,"%08x  %08x %08x %08x %08x %08x %08x %08x "
-		   " %08x %08x %08x %08x %08x %08x %08x %08x %08x \n",
+	seq_printf(seq, "%08x  %08x %08x %08x %08x %08x %08x %08x %08x\n",
 		   dst_entries_get_slow(&ipv4_dst_ops),
-		   st->in_hit,
 		   st->in_slow_tot,
 		   st->in_slow_mc,
 		   st->in_no_route,
@@ -313,16 +311,8 @@ static int rt_cpu_seq_show(struct seq_file *seq, void *v)
 		   st->in_martian_dst,
 		   st->in_martian_src,
 
-		   st->out_hit,
 		   st->out_slow_tot,
-		   st->out_slow_mc,
-
-		   st->gc_total,
-		   st->gc_ignored,
-		   st->gc_goal_miss,
-		   st->gc_dst_overflow,
-		   st->in_hlist_search,
-		   st->out_hlist_search
+		   st->out_slow_mc
 		);
 	return 0;
 }
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: David Miller @ 2012-07-30 21:46 UTC (permalink / raw)
  To: bhutchings; +Cc: eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <1343676952.2667.26.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 30 Jul 2012 20:35:52 +0100

> On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
>> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
>> driver sets to whatever limit ?
> 
> Yes, that's another option.

This is how I want this handled.

^ permalink raw reply

* Re: [PATCH net] net: Remove unused variables in rt_cache_stat
From: David Miller @ 2012-07-30 21:52 UTC (permalink / raw)
  To: subramanian.vijay; +Cc: netdev
In-Reply-To: <1343684750-2987-1-git-send-email-subramanian.vijay@gmail.com>

From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Mon, 30 Jul 2012 14:45:50 -0700

> With the removal of the routing cache, some variables in rt_cache_stat are no
> longer used. Remove them from rt_cache_stat and do not print them out in
> /proc/net/stat/rt_cache.
> 
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> checkpatch complains that the seq_printf line is over 80 chars which was already
> the case. I left it as is to aid in grepping the sources. 

You cannot make this change, these fields are exported via procfs and
therefore you will break any application that is parsing the existing
layout.

^ permalink raw reply

* Re: [PATCH] codel: refine one condition to avoid a nul rec_inv_sqrt
From: David Miller @ 2012-07-30 21:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, lp2s1h
In-Reply-To: <1343631141.2626.13293.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 08:52:21 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> One condition before codel_Newton_step() was not good if
> we never left the dropping state for a flow. As a result
> rec_inv_sqrt was 0, instead of the ~0 initial value.
> 
> codel control law was then set to a very aggressive mode, dropping
> many packets before reaching 'target' and recovering from this problem.
> 
> To keep codel_vars_init() as efficient as possible, refine
> the condition to make sure rec_inv_sqrt initial value is correct
> 
> Many thanks to Anton Mich for discovering the issue and suggesting
> a fix.
> 
> Reported-by: Anton Mich <lp2s1h@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable.

^ permalink raw reply

* Re: [PATCH] net: TCP early demux cleanup
From: David Miller @ 2012-07-30 21:54 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343631973.2626.13317.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 09:06:13 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> early_demux() handlers should be called in RCU context, and as we
> use skb_dst_set_noref(skb, dst), caller must not exit from RCU context
> before dst use (skb_dst(skb)) or release (skb_drop(dst))
> 
> Therefore, rcu_read_lock()/rcu_read_unlock() pairs around
> ->early_demux() are confusing and not needed :
> 
> Protocol handlers are already in an RCU read lock section.
> (__netif_receive_skb() does the rcu_read_lock() )
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I wondered about this very issue when I wrote this code, thanks
for clearing things up.

Applied.

^ permalink raw reply

* Re: [PATCH] net: ipv4: fix RCU races on dst refcounts
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343640037.21269.15.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 11:20:37 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> commit c6cffba4ffa2 (ipv4: Fix input route performance regression.)
> added various fatal races with dst refcounts.
> 
> crashes happen on tcp workloads if routes are added/deleted at the same
> time.
> 
> The dst_free() calls from free_fib_info_rcu() are clearly racy.
> 
> We need instead regular dst refcounting (dst_release()) and make
> sure dst_release() is aware of RCU grace periods :
> 
> Add DST_RCU_FREE flag so that dst_release() respects an RCU grace period
> before dst destruction for cached dst
> 
> Introduce a new inet_sk_rx_dst_set() helper, using atomic_inc_not_zero()
> to make sure we dont increase a zero refcount (On a dst currently
> waiting an rcu grace period before destruction)
> 
> rt_cache_route() must take a reference on the new cached route, and
> release it if was not able to install it.
> 
> With this patch, my machines survive various benchmarks.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I'm applying this patch, however:

> +static inline void inet_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb)
> +{
> +	struct dst_entry *dst = skb_dst(skb);
> +
> +	if (atomic_inc_not_zero(&dst->__refcnt)) {
> +		if (!(dst->flags & DST_RCU_FREE))
> +			dst->flags |= DST_RCU_FREE;
> +
> +		sk->sk_rx_dst = dst;
> +		inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
> +	}
> +}

This is not safe.

We cannot allow clients outside of the DST providers make non-atomic
changes to the dst attributes, as you are here with this dst->flags
modification.

Make this "needs RCU liberation" indication at the spot where we get
rid of sk->sk_rx_dst, make a dst_release_rcu() or somthing like that.

^ permalink raw reply

* Re: [PATCH] ipv4: remove rt_cache_rebuild_count
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343668469.21269.26.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 19:14:29 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> After IP route cache removal, rt_cache_rebuild_count is no longer
> used.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

^ permalink raw reply

* Re: [PATCH] bridge: make port attributes const
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20120730115549.0d53d8cb@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 30 Jul 2012 11:55:49 -0700

> Simple table that can be marked const.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

^ permalink raw reply

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 22:20 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <20120730.144632.478408817308488569.davem@davemloft.net>

On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 30 Jul 2012 20:35:52 +0100
> 
> > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> >> driver sets to whatever limit ?
> > 
> > Yes, that's another option.
> 
> This is how I want this handled.

How should that be applied in the GRO-forwarding case?

Ben.

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

^ permalink raw reply

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Stephen Hemminger @ 2012-07-30 22:50 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <1343686857.2667.60.camel@bwh-desktop.uk.solarflarecom.com>

On Mon, 30 Jul 2012 23:20:57 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Mon, 30 Jul 2012 20:35:52 +0100
> > 
> > > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> > >> driver sets to whatever limit ?
> > > 
> > > Yes, that's another option.
> > 
> > This is how I want this handled.
> 
> How should that be applied in the GRO-forwarding case?
> 
> Ben.
> 
Why not make max_frags a property of the device?

Something like the following untested idea:
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ebaea1..bfb005b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2159,14 +2159,16 @@ EXPORT_SYMBOL(netif_skb_features);
  *	   at least one of fragments is in highmem and device does not
  *	   support DMA from it.
  */
-static inline int skb_needs_linearize(struct sk_buff *skb,
-				      int features)
+static inline bool skb_needs_linearize(struct sk_buff *skb,
+				       int features, unsigned int maxfrags)
 {
-	return skb_is_nonlinear(skb) &&
-			((skb_has_frag_list(skb) &&
-				!(features & NETIF_F_FRAGLIST)) ||
-			(skb_shinfo(skb)->nr_frags &&
-				!(features & NETIF_F_SG)));
+	if (!skb_is_nonlinear(skb))
+		return false;
+
+	if (skb_has_frag_list(skb))
+		return !(features & NETIF_F_FRAGLIST);
+	else
+		return skb_shinfo(skb)->nr_frags > maxfrags;
 }
 
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
@@ -2206,7 +2208,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			if (skb->next)
 				goto gso;
 		} else {
-			if (skb_needs_linearize(skb, features) &&
+			if (skb_needs_linearize(skb, features, dev->max_frags) &&
 			    __skb_linearize(skb))
 				goto out_kfree_skb;
 
@@ -5544,6 +5546,20 @@ int register_netdevice(struct net_device *dev)
 	dev->features |= NETIF_F_SOFT_FEATURES;
 	dev->wanted_features = dev->features & dev->hw_features;
 
+	if (dev->max_frags > 0) {
+		if (!(features & NETIF_F_SG)) {
+			netdev_dbg(dev,
+				   "Resetting max fragments since no NETIF_F_SG\n");
+			dev->max_frags = 0;
+		}
+	} else {
+		/* If device has not set maximum number of fragments
+		 * then assume it can take any number of them
+		 */
+		if (features & NETIF_F_SG)
+			dev->max_frags = MAX_SKB_FRAGS;
+	}
+
 	/* Turn on no cache copy if HW is doing checksum */
 	if (!(dev->flags & IFF_LOOPBACK)) {
 		dev->hw_features |= NETIF_F_NOCACHE_COPY;

^ permalink raw reply related

* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 23:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <20120730155026.7460a9a6@nehalam.linuxnetplumber.net>

On Mon, 2012-07-30 at 15:50 -0700, Stephen Hemminger wrote:
> On Mon, 30 Jul 2012 23:20:57 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> > > From: Ben Hutchings <bhutchings@solarflare.com>
> > > Date: Mon, 30 Jul 2012 20:35:52 +0100
> > > 
> > > > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > > >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> > > >> driver sets to whatever limit ?
> > > > 
> > > > Yes, that's another option.
> > > 
> > > This is how I want this handled.
> > 
> > How should that be applied in the GRO-forwarding case?
> > 
> > Ben.
> > 
> Why not make max_frags a property of the device?
[...]

This has nothing to do with the number of input fragments.  But I think
you're on the right track - this can be checked in netif_skb_features()
or something like that.

Ben.

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

^ permalink raw reply

* Re: [PATCH] net: ipv4: fix RCU races on dst refcounts
From: David Miller @ 2012-07-31  0:40 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <20120730.145637.906924670032055493.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 30 Jul 2012 14:56:37 -0700 (PDT)

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 30 Jul 2012 11:20:37 +0200
> 
>> From: Eric Dumazet <edumazet@google.com>
>> 
>> We need instead regular dst refcounting (dst_release()) and make
>> sure dst_release() is aware of RCU grace periods :

Eric, we really cannot do this.

We absolutely must call dst_free() directly when cached entries
are flushed.

Your delayed scheme using dst_release() doesn't work, it exposes us to
the "stale netdevice references" issue even for cached entries.  This
exact issue is why I moved away from a dst_release() based scheme to
a !DST_NOCACHE + dst_free() one.

dst_free() is special.  It is special in that if there are existing
references, it adds the dst onto the generic GC list of busy dsts in
net/core/dst.c

This is important, because it means that even if references are still
held on the dst, we will still be able to purge spurious netdevice
references when those netdevices try to go down or unregister
themselves.

You can't defer the dst_free() to the final refcount drop like your
code does now.  A socket, or other dst caching entity, can hold onto
the dst forever, do no socket operations at all, and therefore hold
onto a netdevice for an infinite amount of time.

Look at how net/core/dst.c:dst_dev_event() walks dst_busy_list.

That's why we must propagate cached dsts into dst_busy_list no later
than when we trim them from ->nh_rth_{input,output}

I noticed this because I'm trying to work on a fix the lingering dst
netdevice reference problem for non-cached dsts.

^ permalink raw reply

* [PATCH 0/2] ipv4: Fix dangling netdev refs
From: David Miller @ 2012-07-31  1:23 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev


Eric, this should give you an idea of what I was working on.

It really assumes the universe in place before your change last
night.  It is very likely we'll have to revert it and look for
another solution to the problem you were trying to solve.  And
actually I don't understand the actual bug very well.

I can only assume that the core issue was that, unlike back when
we had the routing cache, the fib_info nexthops are not persistent
memory like the routing cache hash table was?

Otherwise I can see absolutely no change in reference counting and
dst destruction logic between the routing cache, and how I modified
fib_info nexthop cached routes to behave.

Anyways, the first patch caches routes in the nexthop exception
entries.

And the second patch has a global list for uncached routes so we
can purge netdevice references properly when such devices are
unregistered.

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

^ permalink raw reply


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