Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 0/9] bnx2x: Start utilizing 7.10.51
From: David Miller @ 2014-08-22 19:31 UTC (permalink / raw)
  To: Yuval.Mintz; +Cc: netdev, Ariel.Elior
In-Reply-To: <1408283271-16176-1-git-send-email-Yuval.Mintz@qlogic.com>

From: Yuval Mintz <Yuval.Mintz@qlogic.com>
Date: Sun, 17 Aug 2014 16:47:42 +0300

> This series will enable bnx2x to start utlizing its 7.10.51 FW.
> In addition, it will also add timestamping support, as well as a couple
> of routine semantic cleanups.
> 
> Please consider applying this series to `net-next'.

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH] stmmac: simple cleanups
From: David Miller @ 2014-08-22 19:34 UTC (permalink / raw)
  To: pavel; +Cc: peppe.cavallaro, netdev, linux-kernel
In-Reply-To: <20140819105646.GA27984@amd>

From: Pavel Machek <pavel@ucw.cz>
Date: Tue, 19 Aug 2014 12:56:46 +0200

> This adds simple cleanups for stmmac, removing test we know is always
> true, fixing whitespace, and moving code out of if().
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>

This does not apply cleanly to the current net-next tree.

^ permalink raw reply

* [PATCH v2 net] vxlan: fix incorrect initializer in union vxlan_addr
From: Gerhard Stenzel @ 2014-08-22 19:34 UTC (permalink / raw)
  To: netdev, davem

The first initializer in the following

        union vxlan_addr ipa = {
            .sin.sin_addr.s_addr = tip,
            .sa.sa_family = AF_INET,
        };

is optimised away by the compiler, due to the second initializer,
therefore initialising .sin.sin_addr.s_addr always to 0.
This results in netlink messages indicating a L3 miss never contain the
missed IP address. This was observed with GCC 4.8 and 4.9. I do not know about previous versions.
The problem affects user space programs relying on an IP address being
sent as part of a netlink message indicating a L3 miss.

Changing
            .sa.sa_family = AF_INET,
to
            .sin.sin_family = AF_INET,
fixes the problem.

Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com>

--- 

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1fb7b37..beb377b 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1327,7 +1327,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb)
 	} else if (vxlan->flags & VXLAN_F_L3MISS) {
 		union vxlan_addr ipa = {
 			.sin.sin_addr.s_addr = tip,
-			.sa.sa_family = AF_INET,
+			.sin.sin_family = AF_INET,
 		};
 
 		vxlan_ip_miss(dev, &ipa);
@@ -1488,7 +1488,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb)
 	} else if (vxlan->flags & VXLAN_F_L3MISS) {
 		union vxlan_addr ipa = {
 			.sin6.sin6_addr = msg->target,
-			.sa.sa_family = AF_INET6,
+			.sin6.sin6_family = AF_INET6,
 		};
 
 		vxlan_ip_miss(dev, &ipa);
@@ -1521,7 +1521,7 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
 		if (!n && (vxlan->flags & VXLAN_F_L3MISS)) {
 			union vxlan_addr ipa = {
 				.sin.sin_addr.s_addr = pip->daddr,
-				.sa.sa_family = AF_INET,
+				.sin.sin_family = AF_INET,
 			};
 
 			vxlan_ip_miss(dev, &ipa);
@@ -1542,7 +1542,7 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
 		if (!n && (vxlan->flags & VXLAN_F_L3MISS)) {
 			union vxlan_addr ipa = {
 				.sin6.sin6_addr = pip6->daddr,
-				.sa.sa_family = AF_INET6,
+				.sin6.sin6_family = AF_INET6,
 			};
 
 			vxlan_ip_miss(dev, &ipa);

^ permalink raw reply related

* Re: [patch net-next] bonding: create netlink event when bonding option is changed
From: David Miller @ 2014-08-22 19:34 UTC (permalink / raw)
  To: jiri; +Cc: netdev, j.vosburgh, vfalico, andy, nikolay, dingtianhong, sfeldma
In-Reply-To: <1408456932-10318-1-git-send-email-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 19 Aug 2014 16:02:12 +0200

> Userspace needs to be notified if one changes some option.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Applied.

^ permalink raw reply

* Re: [PATCH net-next v2] r8169:add support for RTL8168H and RTL8107E
From: David Miller @ 2014-08-22 19:34 UTC (permalink / raw)
  To: hau; +Cc: netdev, nic_swsd, linux-kernel
In-Reply-To: <1408470844-30038-1-git-send-email-hau@realtek.com>

From: Chun-Hao Lin <hau@realtek.com>
Date: Wed, 20 Aug 2014 01:54:04 +0800

> RTL8168H is Realtek PCIe Gigabit Ethernet controller.
> RTL8107E is Realtek PCIe Fast Ethernet controller.
> 
> This patch add support for these two chips.
> 
> Signed-off-by: Chun-Hao Lin <hau@realtek.com>

Applied.

^ permalink raw reply

* Re: [PATCH] net: stmmac: add fix_mac_speed support for socfpga
From: David Miller @ 2014-08-22 19:35 UTC (permalink / raw)
  To: lftan; +Cc: netdev, linux-kernel, lftan.linux, peppe.cavallaro, vbridger
In-Reply-To: <1408516413-2928-1-git-send-email-lftan@altera.com>

From: Ley Foon Tan <lftan@altera.com>
Date: Wed, 20 Aug 2014 14:33:33 +0800

> This patch adds fix_mac_speed() support for
> Altera socfpga Ethernet controller. Emac splitter is a
> soft IP core in FPGA system that converts GMII interface from
> Synopsys mac to RGMII/SGMII interface. This splitter core is
> an optional IP if user would like to use RGMII/SGMII
> interface in their system. Software needs to update a register
> in splitter core when there is speed change.
> 
> Signed-off-by: Ley Foon Tan <lftan@altera.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] dp83640: Fix length check for event timestamp status messages
From: David Miller @ 2014-08-22 19:36 UTC (permalink / raw)
  To: christian.riesch; +Cc: netdev, richardcochran
In-Reply-To: <1408627024-12932-1-git-send-email-christian.riesch@omicron.at>

From: Christian Riesch <christian.riesch@omicron.at>
Date: Thu, 21 Aug 2014 15:17:04 +0200

> Event timestamp status messages have a variable length, ranging from
> 1 to 5 words (16 bit words). The current code however requires
> a minimum message length of sizeof(*phy_txts). In most cases this
> condition is fulfilled due to padding bytes. However, if several events
> are signaled in a single message, padding bytes may not be present.
> For short event timestamp status messages, the length check will fail,
> and the event timestamp will be dropped.
> 
> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>

Applied.

^ permalink raw reply

* Re: [patch net-next RFC 10/12] openvswitch: add support for datapath hardware offload
From: John Fastabend @ 2014-08-22 19:39 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w,
	jasowang-H+wXaHxf7aLQT0dZR+AlfA,
	john.r.fastabend-ral2JQCrhuEAvxtiuMwx3w,
	Neil.Jerram-QnUH15yq9NYqDJ6do+/SaQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, andy-QlMahl40kYEqcZcGjlUOXw,
	dev-yBygre7rU0TnMu66kgdUjQ, nbd-p3rKhJxN3npAfugRpC6u6w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, ronye-VPRAkNaXOzVWk0Htik3J/w,
	jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
	ogerlitz-VPRAkNaXOzVWk0Htik3J/w, ben-/+tVBieCtBitmTQ+vhA3Yw,
	buytenh-OLH4Qvv75CYX/NnBR394Jw,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	jhs-jkUAjuhPggJWk0Htik3J/w, aviadr-VPRAkNaXOzVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w,
	vyasevic-H+wXaHxf7aLQT0dZR+AlfA, nhorman-2XuSBdqkA4R54TAoqtyWWQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	dborkman-H+wXaHxf7aLQT0dZR+AlfA, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1408637945-10390-11-git-send-email-jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>

On 08/21/2014 09:19 AM, Jiri Pirko wrote:
> Benefit from the possibility to work with flows in switch devices and
> use the swdev api to offload flow datapath.

we should add a description here on the strategy being used.

If I read this correctly this will try to add any flow to the
hardware along with the actions and duplicate it in software.

There are a couple things I don't like,

  - this requires OVS to be loaded to work. If all I want is
    direct access to the hardware flow tables requiring openvswitch.ko
    shouldn't be needed IMO. For example I may want to use the
    hardware flow tables with something not openvswitch and we
    shouldn't preclude that.

  - Also there is no programmatic way to learn which flows are
    in hardware and which in software. There is a pr_warn but
    that doesn't help when interacting with the hardware remotely.
    I need some mechanism to dump the set of hardware tables and
    the set of software tables.

  - Simply duplicating the software flow/action into
    hardware may not optimally use the hardware tables. If I have
    a TCAM in hardware for instance. (This is how I read the patch
    let me know if I missed something)

  - I need a way to specify put this flow/action in hardware,
    put this flow/action in software, or put this in both software
    and hardware.

    We did this with a bitmask in the fdb L2 stuff and it seems to
    work reasonable well so maybe something like that would help.

    For example if I don't have this what happens if I have an
    entry to decrement TTL in both hardware and software. If the
    flow hits both the hardware path and software path the TTL
    gets decremented. Here userspace needs to indicate where to
    do the decrement to avoid the duplication.

I think if we can pull this out OVS and add the hw/sw bitmask (or
maybe a better implementation of that idea) then this should work
for the stuff I'm looking at. I want to try and get it working on
the i40e driver as a fdir replacement but it might take me a bit
to get to it.


Thanks,
John


-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* Re: [PATCH v2 0/3] net: Add Keystone NetCP ethernet driver support
From: Santosh Shilimkar @ 2014-08-22 19:45 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	grant.likely-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, sandeep_n-l0cyMroinI0
In-Reply-To: <20140821.163612.282672926741753926.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

Hi David,

On Thursday 21 August 2014 07:36 PM, David Miller wrote:
> From: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
> Date: Fri, 15 Aug 2014 11:12:39 -0400
> 
>> Update version after incorporating David Miller's comment from earlier
>> posting [1]. I would like to get these merged for upcoming 3.18 merge
>> window if there are no concerns on this version.
>>
>> The network coprocessor (NetCP) is a hardware accelerator that processes
>> Ethernet packets. NetCP has a gigabit Ethernet (GbE) subsystem with a ethernet
>> switch sub-module to send and receive packets. NetCP also includes a packet
>> accelerator (PA) module to perform packet classification operations such as
>> header matching, and packet modification operations such as checksum
>> generation. NetCP can also optionally include a Security Accelerator(SA)
>> capable of performing IPSec operations on ingress/egress packets.
>>     
>> Keystone SoC's also have a 10 Gigabit Ethernet Subsystem (XGbE) which
>> includes a 3-port Ethernet switch sub-module capable of 10Gb/s and
>> 1Gb/s rates per Ethernet port.
>>     
>> NetCP driver has a plug-in module architecture where each of the NetCP
>> sub-modules exist as a loadable kernel module which plug in to the netcp
>> core. These sub-modules are represented as "netcp-devices" in the dts
>> bindings. It is mandatory to have the ethernet switch sub-module for
>> the ethernet interface to be operational. Any other sub-module like the
>> PA is optional.
>>
>> Both GBE and XGBE network processors supported using common driver. It
>> is also designed to handle future variants of NetCP.
> 
> I don't want to see an offload driver that doesn't plug into the existing
> generic frameworks for configuration et al.
> 
> If no existing facility exists to support what you need, you must work
> with the upstream maintainers to design and create one.
> 
> It is absolutely no reasonable for every "switch on a chip" driver to
> export it's own configuration knob, we need a standard interface all
> such drivers will plug into and provide.
> 
The NetCP plugin module infrastructure use all the standard kernel
infrastructure and its very tiny. To best represent the Network processor
and its sub module hardware which have inter dependency and ordering
needs, we needed such infrastructure. This lets us handle all the
hardware needs without any code duplication per module.

To elaborate more, there are 4 variants of network switch modules and
then few accelerator modules like Packet accelerator, QOS and Security
accelerator. There can be multiple instances of switches on same SOC.
Example 1 Gbe and 10 Gbe switches. Then additional accelerator modules
are inter connected with switch, streaming fabric and packet DMA.
Packet routing changes based on the various offload modules presence and hence
needs hooks for tx/rx to be called in particular order with special
handling. This scheme is very hardware specific and doesn't have ways
to isolate the modules from each other.

On the other hand, we definitely wanted to have minimal code
instead of duplicating ndo operations and core packet processing logic
in multiple drivers or layers. The module approach helps
to isolate the code based on the customer choice who can choose
say not to build 10 Gbe hardware or say don't need QOS or Security
accelerators. That way we keep the packet processing hot path as
what we need without any overhead.

As you can see, the tiny module handling was added more to represent
the hardware, keep the modularity and avoid code duplication. The
infrastructure is very minimal and NETCP specific. With this small
infrastructure we are able to re-use code for NetCP1.0, NetCP1.5,
10 GBe and upcoming NetCP variants from just *one* driver.

Hope this gives you a better idea and rationale behind the design.

Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* net-next is OPEN
From: David Miller @ 2014-08-22 19:47 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netfilter-devel-u79uwXL29TY76Z2rM5mHXA


The net-next GIT tree is now open again, please feel free to submit
new features and cleanups.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/3] net: Add Keystone NetCP ethernet driver
From: Santosh Shilimkar @ 2014-08-22 19:50 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem, netdev, linux-arm-kernel, linux-kernel, robh+dt,
	grant.likely, devicetree, sandeep_n
In-Reply-To: <20140821194838.32f2c3f1@uryu.home.lan>

On Thursday 21 August 2014 10:48 PM, Stephen Hemminger wrote:
> On Fri, 15 Aug 2014 11:12:41 -0400
> Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> 
>> NetCP driver has a plug-in module architecture where each of the NetCP
>> sub-modules exist as a loadable kernel module which plug in to the netcp
>> core. These sub-modules are represented as "netcp-devices" in the dts
>> bindings. It is mandatory to have the ethernet switch sub-module for
>> the ethernet interface to be operational. Any other sub-module like the
>> PA is optional.
> 
> What are you doing to prevent/discourage proprietary binary only
> sub-modules?
> 
Not sure if you mistook this infrastructure as a replacement for the
kernel module. The sub modules are traditional kernel modules that
just registers into the NetCP framework. Kernel module infrastructure
already discourages the proprietary binary only sub-modules.

Regards,
Santosh

^ permalink raw reply

* [PATCH] phylib: use MDIO_DEVS[12]
From: Sergei Shtylyov @ 2014-08-22 19:56 UTC (permalink / raw)
  To: netdev, f.fainelli

The bare register numbers are used despite <uapi/linux/mdio.h> has MDIO_DEVS[12]
#define'd for those.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against DaveM's 'net-next.git' repo.

 drivers/net/phy/phy_device.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: net-next/drivers/net/phy/phy_device.c
===================================================================
--- net-next.orig/drivers/net/phy/phy_device.c
+++ net-next/drivers/net/phy/phy_device.c
@@ -230,13 +230,13 @@ static int get_phy_c45_ids(struct mii_bu
 	for (i = 1;
 	     i < num_ids && c45_ids->devices_in_package == 0;
 	     i++) {
-		reg_addr = MII_ADDR_C45 | i << 16 | 6;
+		reg_addr = MII_ADDR_C45 | i << 16 | MDIO_DEVS2;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;
 		c45_ids->devices_in_package = (phy_reg & 0xffff) << 16;
 
-		reg_addr = MII_ADDR_C45 | i << 16 | 5;
+		reg_addr = MII_ADDR_C45 | i << 16 | MDIO_DEVS1;
 		phy_reg = mdiobus_read(bus, addr, reg_addr);
 		if (phy_reg < 0)
 			return -EIO;

^ permalink raw reply

* Re: [PATCH net-next v2] ipv4: Restore accept_local behaviour in fib_validate_source()
From: Julian Anastasov @ 2014-08-22 20:07 UTC (permalink / raw)
  To: Sébastien Barré
  Cc: David Miller, netdev, Gregory Detal, Christoph Paasch,
	Hannes Frederic Sowa, Sergei Shtylyov
In-Reply-To: <1408259994-31458-1-git-send-email-sebastien.barre@uclouvain.be>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1384 bytes --]


	Hello,

On Sun, 17 Aug 2014, Sébastien Barré wrote:

> Commit 7a9bc9b81a5b ("ipv4: Elide fib_validate_source() completely when possible.")
> introduced a short-circuit to avoid calling fib_validate_source when not
> needed. That change took rp_filter into account, but not accept_local.
> This resulted in a change of behaviour: with rp_filter and accept_local
> off, incoming packets with a local address in the source field should be
> dropped.
> 
> Here is how to reproduce the change pre/post 7a9bc9b81a5b commit:
> -configure the same IPv4 address on hosts A and B.
> -try to send an ARP request from B to A.
> -The ARP request will be dropped before that commit, but accepted and answered
> after that commit.
> 
> This adds a check for ACCEPT_LOCAL, to maintain full
> fib validation in case it is 0. We also leave __fib_validate_source() earlier
> when possible, based on the same check as fib_validate_source(), once the
> accept_local stuff is verified.

	I remember there was related change for docs,
may be commit c801e3cc1925e0 should be reverted ?

> Cc: Gregory Detal <gregory.detal@uclouvain.be>
> Cc: Christoph Paasch <christoph.paasch@uclouvain.be>
> Cc: Hannes Frederic Sowa <hannes@redhat.com>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Sébastien Barré <sebastien.barre@uclouvain.be>

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* [PATCH v2 net-next 0/6] net: Checksum offload changes - Part V
From: Tom Herbert @ 2014-08-22 20:33 UTC (permalink / raw)
  To: davem, netdev

I am working on overhauling RX checksum offload. Goals of this effort
are:

- Specify what exactly it means when driver returns CHECKSUM_UNNECESSARY
- Preserve CHECKSUM_COMPLETE through encapsulation layers
- Don't do skb_checksum more than once per packet
- Unify GRO and non-GRO csum verification as much as possible
- Unify the checksum functions (checksum_init)
- Simplify code

What is in this fifth patch set:

- Added GRO checksum validation functions
- Call the GRO validations functions from TCP and GRE gro_receive
- Perform checksum verification in the UDP gro_receive path using
  GRO functions and add support for gro_receive in UDP6

Changes in V2:

- Change ip_summed to CHECKSUM_UNNECESSARY instead of moving it
  to CHECKSUM_COMPLETE from GRO checksum validation. This avoids
  performance penalty in checksumming bytes which are before the header
  GRO is at.

Please review carefully and test if possible, mucking with basic
checksum functions is always a little precarious :-)

----

Test results with this patch set are below. I did not notice any
performace regression.

Tests run:
   TCP_STREAM: super_netperf with 200 streams
   TCP_RR: super_netperf with 200 streams and -r 1,1

Device bnx2x (10Gbps):
   No GRE RSS hash (RX interrupts occur on one core)
   UDP RSS port hashing enabled.

* GRE with checksum with IPv4 encapsulated packets
  With fix:
    TCP_STREAM
        9.91% CPU utilization
        5163.78 Mbps
    TCP_RR
        50.64% CPU utilization
        219/347/502 90/95/99% latencies
        834103 tps
  Without fix:
    TCP_STREAM
        10.05% CPU utilization
        5186.22 tps
    TCP_RR
        49.70% CPU utilization
        227/338/486 90/95/99% latencies
        813450 tps

* GRE without checksum with IPv4 encapsulated packets
  With fix:
    TCP_STREAM
        10.18% CPU utilization
        5159 Mbps
    TCP_RR
        51.86% CPU utilization
        214/325/471 90/95/99% latencies
        865943 tps
  Without fix:
    TCP_STREAM
        10.26% CPU utilization
        5307.87 Mbps
    TCP_RR
        50.59% CPU utilization
        224/325/476 90/95/99% latencies
        846429 tps

*** Simulate device returns CHECKSUM_COMPLETE

* VXLAN with checksum
  With fix:
    TCP_STREAM
        13.03% CPU utilization
        9093.9 Mbps
    TCP_RR
        95.96% CPU utilization
        161/259/474 90/95/99% latencies
        1.14806e+06 tps
  Without fix:
    TCP_STREAM
        13.59% CPU utilization
        9093.97 Mbps
    TCP_RR
        93.95% CPU utilization
        160/259/484 90/95/99% latencies
        1.10262e+06 tps

* VXLAN without checksum
  With fix:
    TCP_STREAM
        13.28% CPU utilization
        9093.87 Mbps
    TCP_RR
        95.04% CPU utilization
        155/246/439 90/95/99% latencies
        1.15e+06 tps
  Without fix:
    TCP_STREAM
        13.37% CPU utilization
        9178.45 Mbps
    TCP_RR
        93.74% CPU utilization
        161/257/469 90/95/99% latencies
        1.1068e+06 Mbps

^ permalink raw reply

* [PATCH v2 net-next 1/6] net: skb_gro_checksum_* functions
From: Tom Herbert @ 2014-08-22 20:33 UTC (permalink / raw)
  To: davem, netdev

Add skb_gro_checksum_validate, skb_gro_checksum_validate_zero_check,
and skb_gro_checksum_simple_validate, and __skb_gro_checksum_complete.
These are the cognates of the normal checksum functions but are used
in the gro_receive path and operate on GRO related fields in sk_buffs.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 include/linux/netdevice.h | 76 +++++++++++++++++++++++++++++++++++++++++++++--
 net/core/dev.c            | 34 ++++++++++++++++++++-
 2 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3837739..0fac884 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1883,7 +1883,13 @@ struct napi_gro_cb {
 	u16	proto;
 
 	/* Used in udp_gro_receive */
-	u16	udp_mark;
+	u8	udp_mark:1;
+
+	/* GRO checksum is valid */
+	u8	csum_valid:1;
+
+	/* Number encapsulation layers crossed */
+	u8	encapsulation;
 
 	/* used to support CHECKSUM_COMPLETE for tunneling protocols */
 	__wsum	csum;
@@ -2153,11 +2159,77 @@ static inline void *skb_gro_network_header(struct sk_buff *skb)
 static inline void skb_gro_postpull_rcsum(struct sk_buff *skb,
 					const void *start, unsigned int len)
 {
-	if (skb->ip_summed == CHECKSUM_COMPLETE)
+	if (NAPI_GRO_CB(skb)->csum_valid)
 		NAPI_GRO_CB(skb)->csum = csum_sub(NAPI_GRO_CB(skb)->csum,
 						  csum_partial(start, len, 0));
 }
 
+/* GRO checksum functions. These are logical equivalents of the normal
+ * checksum functions (in skbuff.h) except that they operate on the GRO
+ * offsets and fields in sk_buff.
+ */
+
+__sum16 __skb_gro_checksum_complete(struct sk_buff *skb);
+
+static inline bool __skb_gro_checksum_validate_needed(struct sk_buff *skb,
+						      bool zero_okay,
+						      __sum16 check)
+{
+	return (skb->ip_summed != CHECKSUM_PARTIAL &&
+		(skb->ip_summed != CHECKSUM_UNNECESSARY ||
+		 (NAPI_GRO_CB(skb)->encapsulation > skb->encapsulation)) &&
+		(!zero_okay || check));
+}
+
+static inline __sum16 __skb_gro_checksum_validate_complete(struct sk_buff *skb,
+							   __wsum psum)
+{
+	if (NAPI_GRO_CB(skb)->csum_valid &&
+	    !csum_fold(csum_add(psum, NAPI_GRO_CB(skb)->csum)))
+		return 0;
+
+	NAPI_GRO_CB(skb)->csum = psum;
+
+	return __skb_gro_checksum_complete(skb);
+}
+
+/* Update skb for CHECKSUM_UNNECESSARY when we verified a top level
+ * checksum or an encapsulated one during GRO. This saves work
+ * if we fallback to normal path with the packet.
+ */
+static inline void skb_gro_incr_csum_unnecessary(struct sk_buff *skb)
+{
+	if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
+		if (NAPI_GRO_CB(skb)->encapsulation)
+			skb->encapsulation = 1;
+	} else if (skb->ip_summed != CHECKSUM_PARTIAL) {
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+		skb->encapsulation = 0;
+	}
+}
+
+#define __skb_gro_checksum_validate(skb, proto, zero_okay, check,	\
+				    compute_pseudo)			\
+({									\
+	__sum16 __ret = 0;						\
+	if (__skb_gro_checksum_validate_needed(skb, zero_okay, check))	\
+		__ret = __skb_gro_checksum_validate_complete(skb,	\
+				compute_pseudo(skb, proto));		\
+	if (!__ret)							\
+		skb_gro_incr_csum_unnecessary(skb);			\
+	__ret;								\
+})
+
+#define skb_gro_checksum_validate(skb, proto, compute_pseudo)		\
+	__skb_gro_checksum_validate(skb, proto, false, 0, compute_pseudo)
+
+#define skb_gro_checksum_validate_zero_check(skb, proto, check,		\
+					     compute_pseudo)		\
+	__skb_gro_checksum_validate(skb, proto, true, check, compute_pseudo)
+
+#define skb_gro_checksum_simple_validate(skb)				\
+	__skb_gro_checksum_validate(skb, 0, false, 0, null_compute_pseudo)
+
 static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
 				  unsigned short type,
 				  const void *daddr, const void *saddr,
diff --git a/net/core/dev.c b/net/core/dev.c
index b65a505..2a0d416 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3963,7 +3963,13 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 		goto normal;
 
 	gro_list_prepare(napi, skb);
-	NAPI_GRO_CB(skb)->csum = skb->csum; /* Needed for CHECKSUM_COMPLETE */
+
+	if (skb->ip_summed == CHECKSUM_COMPLETE) {
+		NAPI_GRO_CB(skb)->csum = skb->csum;
+		NAPI_GRO_CB(skb)->csum_valid = 1;
+	} else {
+		NAPI_GRO_CB(skb)->csum_valid = 0;
+	}
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(ptype, head, list) {
@@ -3976,6 +3982,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 		NAPI_GRO_CB(skb)->flush = 0;
 		NAPI_GRO_CB(skb)->free = 0;
 		NAPI_GRO_CB(skb)->udp_mark = 0;
+		NAPI_GRO_CB(skb)->encapsulation = 0;
 
 		pp = ptype->callbacks.gro_receive(&napi->gro_list, skb);
 		break;
@@ -4206,6 +4213,31 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
 }
 EXPORT_SYMBOL(napi_gro_frags);
 
+/* Compute the checksum from gro_offset and return the folded value
+ * after adding in any pseudo checksum.
+ */
+__sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
+{
+	__wsum wsum;
+	__sum16 sum;
+
+	wsum = skb_checksum(skb, skb_gro_offset(skb), skb_gro_len(skb), 0);
+
+	/* NAPI_GRO_CB(skb)->csum holds pseudo checksum */
+	sum = csum_fold(csum_add(NAPI_GRO_CB(skb)->csum, wsum));
+	if (likely(!sum)) {
+		if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
+		    !skb->csum_complete_sw)
+			netdev_rx_csum_fault(skb->dev);
+	}
+
+	NAPI_GRO_CB(skb)->csum = wsum;
+	NAPI_GRO_CB(skb)->csum_valid = 1;
+
+	return sum;
+}
+EXPORT_SYMBOL(__skb_gro_checksum_complete);
+
 /*
  * net_rps_action_and_irq_enable sends any pending IPI's for rps.
  * Note: called with local irq disabled, but exits with local irq enabled.
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v2 net-next 2/6] net: add gro_compute_pseudo functions
From: Tom Herbert @ 2014-08-22 20:34 UTC (permalink / raw)
  To: davem, netdev

Add inet_gro_compute_pseudo and ip6_gro_compute_pseudo. These are
the logical equivalents of inet_compute_pseudo and ip6_compute_pseudo
for GRO path. The IP header is taken from skb_gro_network_header.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 include/net/ip.h           | 8 ++++++++
 include/net/ip6_checksum.h | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/net/ip.h b/include/net/ip.h
index db4a771..c8fd611 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -364,6 +364,14 @@ static inline void inet_set_txhash(struct sock *sk)
 	sk->sk_txhash = flow_hash_from_keys(&keys);
 }
 
+static inline __wsum inet_gro_compute_pseudo(struct sk_buff *skb, int proto)
+{
+	const struct iphdr *iph = skb_gro_network_header(skb);
+
+	return csum_tcpudp_nofold(iph->saddr, iph->daddr,
+				  skb_gro_len(skb), proto, 0);
+}
+
 /*
  *	Map a multicast IP onto multicast MAC for type ethernet.
  */
diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
index 55236cb..1a49b73 100644
--- a/include/net/ip6_checksum.h
+++ b/include/net/ip6_checksum.h
@@ -48,6 +48,14 @@ static inline __wsum ip6_compute_pseudo(struct sk_buff *skb, int proto)
 					    skb->len, proto, 0));
 }
 
+static inline __wsum ip6_gro_compute_pseudo(struct sk_buff *skb, int proto)
+{
+	const struct ipv6hdr *iph = skb_gro_network_header(skb);
+
+	return ~csum_unfold(csum_ipv6_magic(&iph->saddr, &iph->daddr,
+					    skb_gro_len(skb), proto, 0));
+}
+
 static __inline__ __sum16 tcp_v6_check(int len,
 				   const struct in6_addr *saddr,
 				   const struct in6_addr *daddr,
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v2 net-next 3/6] gre: call skb_gro_checksum_simple_validate
From: Tom Herbert @ 2014-08-22 20:34 UTC (permalink / raw)
  To: davem, netdev

Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/gre_offload.c | 43 +++++++------------------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 6556263..d1bd169 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -119,28 +119,6 @@ out:
 	return segs;
 }
 
-/* Compute the whole skb csum in s/w and store it, then verify GRO csum
- * starting from gro_offset.
- */
-static __sum16 gro_skb_checksum(struct sk_buff *skb)
-{
-	__sum16 sum;
-
-	skb->csum = skb_checksum(skb, 0, skb->len, 0);
-	NAPI_GRO_CB(skb)->csum = csum_sub(skb->csum,
-		csum_partial(skb->data, skb_gro_offset(skb), 0));
-	sum = csum_fold(NAPI_GRO_CB(skb)->csum);
-	if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE)) {
-		if (unlikely(!sum) && !skb->csum_complete_sw)
-			netdev_rx_csum_fault(skb->dev);
-	} else {
-		skb->ip_summed = CHECKSUM_COMPLETE;
-		skb->csum_complete_sw = 1;
-	}
-
-	return sum;
-}
-
 static struct sk_buff **gre_gro_receive(struct sk_buff **head,
 					struct sk_buff *skb)
 {
@@ -192,22 +170,15 @@ static struct sk_buff **gre_gro_receive(struct sk_buff **head,
 		if (unlikely(!greh))
 			goto out_unlock;
 	}
-	if (greh->flags & GRE_CSUM) { /* Need to verify GRE csum first */
-		__sum16 csum = 0;
-
-		if (skb->ip_summed == CHECKSUM_COMPLETE)
-			csum = csum_fold(NAPI_GRO_CB(skb)->csum);
-		/* Don't trust csum error calculated/reported by h/w */
-		if (skb->ip_summed == CHECKSUM_NONE || csum != 0)
-			csum = gro_skb_checksum(skb);
-
-		/* GRE CSUM is the 1's complement of the 1's complement sum
-		 * of the GRE hdr plus payload so it should add up to 0xffff
-		 * (and 0 after csum_fold()) just like the IPv4 hdr csum.
-		 */
-		if (csum)
+
+	/* Don't bother verifying checksum if we're going to flush anyway. */
+	if (greh->flags & GRE_CSUM) {
+		if (!NAPI_GRO_CB(skb)->flush &&
+		    skb_gro_checksum_simple_validate(skb))
 			goto out_unlock;
+		NAPI_GRO_CB(skb)->encapsulation++;
 	}
+
 	flush = 0;
 
 	for (p = *head; p; p = p->next) {
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v2 net-next 4/6] tcp: Call skb_gro_checksum_validate
From: Tom Herbert @ 2014-08-22 20:34 UTC (permalink / raw)
  To: davem, netdev

In tcp[64]_gro_receive call skb_gro_checksum_validate to validate TCP
checksum in the gro context.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/tcp_offload.c   | 27 +++------------------------
 net/ipv6/tcpv6_offload.c | 26 +++-----------------------
 2 files changed, 6 insertions(+), 47 deletions(-)

diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index bc1b83c..7291253 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -288,35 +288,14 @@ static int tcp_v4_gso_send_check(struct sk_buff *skb)
 
 static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *skb)
 {
-	/* Use the IP hdr immediately proceeding for this transport */
-	const struct iphdr *iph = skb_gro_network_header(skb);
-	__wsum wsum;
-
 	/* Don't bother verifying checksum if we're going to flush anyway. */
-	if (NAPI_GRO_CB(skb)->flush)
-		goto skip_csum;
-
-	wsum = NAPI_GRO_CB(skb)->csum;
-
-	switch (skb->ip_summed) {
-	case CHECKSUM_NONE:
-		wsum = skb_checksum(skb, skb_gro_offset(skb), skb_gro_len(skb),
-				    0);
-
-		/* fall through */
-
-	case CHECKSUM_COMPLETE:
-		if (!tcp_v4_check(skb_gro_len(skb), iph->saddr, iph->daddr,
-				  wsum)) {
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
-			break;
-		}
-
+	if (!NAPI_GRO_CB(skb)->flush &&
+	    skb_gro_checksum_validate(skb, IPPROTO_TCP,
+				      inet_gro_compute_pseudo)) {
 		NAPI_GRO_CB(skb)->flush = 1;
 		return NULL;
 	}
 
-skip_csum:
 	return tcp_gro_receive(head, skb);
 }
 
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index 01b0ff9..dbb3d92 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -35,34 +35,14 @@ static int tcp_v6_gso_send_check(struct sk_buff *skb)
 static struct sk_buff **tcp6_gro_receive(struct sk_buff **head,
 					 struct sk_buff *skb)
 {
-	const struct ipv6hdr *iph = skb_gro_network_header(skb);
-	__wsum wsum;
-
 	/* Don't bother verifying checksum if we're going to flush anyway. */
-	if (NAPI_GRO_CB(skb)->flush)
-		goto skip_csum;
-
-	wsum = NAPI_GRO_CB(skb)->csum;
-
-	switch (skb->ip_summed) {
-	case CHECKSUM_NONE:
-		wsum = skb_checksum(skb, skb_gro_offset(skb), skb_gro_len(skb),
-				    wsum);
-
-		/* fall through */
-
-	case CHECKSUM_COMPLETE:
-		if (!tcp_v6_check(skb_gro_len(skb), &iph->saddr, &iph->daddr,
-				  wsum)) {
-			skb->ip_summed = CHECKSUM_UNNECESSARY;
-			break;
-		}
-
+	if (!NAPI_GRO_CB(skb)->flush &&
+	    skb_gro_checksum_validate(skb, IPPROTO_TCP,
+				      ip6_gro_compute_pseudo)) {
 		NAPI_GRO_CB(skb)->flush = 1;
 		return NULL;
 	}
 
-skip_csum:
 	return tcp_gro_receive(head, skb);
 }
 
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v2 net-next 5/6 v2] udp: additional GRO support
From: Tom Herbert @ 2014-08-22 20:34 UTC (permalink / raw)
  To: davem, netdev

Implement GRO for UDPv6. Add UDP checksum verification in gro_receive
for both UDP4 and UDP6 calling skb_gro_checksum_validate_zero_check.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 include/net/udp.h      | 18 +++++++++++++++
 net/ipv4/udp.c         |  1 +
 net/ipv4/udp_offload.c | 61 ++++++++++++++++++++++++++++++++++++--------------
 net/ipv6/udp_offload.c | 33 +++++++++++++++++++++++++++
 4 files changed, 96 insertions(+), 17 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 70f9413..16f4e80 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -158,6 +158,24 @@ static inline __sum16 udp_v4_check(int len, __be32 saddr,
 void udp_set_csum(bool nocheck, struct sk_buff *skb,
 		  __be32 saddr, __be32 daddr, int len);
 
+struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
+				 struct udphdr *uh);
+int udp_gro_complete(struct sk_buff *skb, int nhoff);
+
+static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
+{
+	struct udphdr *uh;
+	unsigned int hlen, off;
+
+	off  = skb_gro_offset(skb);
+	hlen = off + sizeof(*uh);
+	uh   = skb_gro_header_fast(skb, off);
+	if (skb_gro_header_hard(skb, hlen))
+		uh = skb_gro_header_slow(skb, hlen, off);
+
+	return uh;
+}
+
 /* hash routines shared between UDPv4/6 and UDP-Litev4/6 */
 static inline void udp_lib_hash(struct sock *sk)
 {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f57c0e4..ed62b44 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -99,6 +99,7 @@
 #include <linux/slab.h>
 #include <net/tcp_states.h>
 #include <linux/skbuff.h>
+#include <linux/netdevice.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <net/net_namespace.h>
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 59035bc..8ed460e 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -228,29 +228,22 @@ unlock:
 }
 EXPORT_SYMBOL(udp_del_offload);
 
-static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
+struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
+				 struct udphdr *uh)
 {
 	struct udp_offload_priv *uo_priv;
 	struct sk_buff *p, **pp = NULL;
-	struct udphdr *uh, *uh2;
-	unsigned int hlen, off;
+	struct udphdr *uh2;
+	unsigned int off = skb_gro_offset(skb);
 	int flush = 1;
 
 	if (NAPI_GRO_CB(skb)->udp_mark ||
-	    (!skb->encapsulation && skb->ip_summed != CHECKSUM_COMPLETE))
+	    (!skb->encapsulation && !NAPI_GRO_CB(skb)->csum_valid))
 		goto out;
 
 	/* mark that this skb passed once through the udp gro layer */
 	NAPI_GRO_CB(skb)->udp_mark = 1;
-
-	off  = skb_gro_offset(skb);
-	hlen = off + sizeof(*uh);
-	uh   = skb_gro_header_fast(skb, off);
-	if (skb_gro_header_hard(skb, hlen)) {
-		uh = skb_gro_header_slow(skb, hlen, off);
-		if (unlikely(!uh))
-			goto out;
-	}
+	NAPI_GRO_CB(skb)->encapsulation++;
 
 	rcu_read_lock();
 	uo_priv = rcu_dereference(udp_offload_base);
@@ -269,7 +262,12 @@ unflush:
 			continue;
 
 		uh2 = (struct udphdr   *)(p->data + off);
-		if ((*(u32 *)&uh->source != *(u32 *)&uh2->source)) {
+
+		/* Match ports and either checksums are either both zero
+		 * or nonzero.
+		 */
+		if ((*(u32 *)&uh->source != *(u32 *)&uh2->source) ||
+		    (!uh->check ^ !uh2->check)) {
 			NAPI_GRO_CB(p)->same_flow = 0;
 			continue;
 		}
@@ -286,7 +284,24 @@ out:
 	return pp;
 }
 
-static int udp_gro_complete(struct sk_buff *skb, int nhoff)
+static struct sk_buff **udp4_gro_receive(struct sk_buff **head,
+					 struct sk_buff *skb)
+{
+	struct udphdr *uh = udp_gro_udphdr(skb);
+
+	/* Don't bother verifying checksum if we're going to flush anyway. */
+	if (unlikely(!uh) ||
+	    (!NAPI_GRO_CB(skb)->flush &&
+	     skb_gro_checksum_validate_zero_check(skb, IPPROTO_UDP, uh->check,
+						  inet_gro_compute_pseudo))) {
+		NAPI_GRO_CB(skb)->flush = 1;
+		return NULL;
+	}
+
+	return udp_gro_receive(head, skb, uh);
+}
+
+int udp_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	struct udp_offload_priv *uo_priv;
 	__be16 newlen = htons(skb->len - nhoff);
@@ -311,12 +326,24 @@ static int udp_gro_complete(struct sk_buff *skb, int nhoff)
 	return err;
 }
 
+int udp4_gro_complete(struct sk_buff *skb, int nhoff)
+{
+	const struct iphdr *iph = ip_hdr(skb);
+	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
+
+	if (uh->check)
+		uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
+					  iph->daddr, 0);
+
+	return udp_gro_complete(skb, nhoff);
+}
+
 static const struct net_offload udpv4_offload = {
 	.callbacks = {
 		.gso_send_check = udp4_ufo_send_check,
 		.gso_segment = udp4_ufo_fragment,
-		.gro_receive  =	udp_gro_receive,
-		.gro_complete =	udp_gro_complete,
+		.gro_receive  =	udp4_gro_receive,
+		.gro_complete =	udp4_gro_complete,
 	},
 };
 
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 0ae3d98..b13e377 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -10,6 +10,7 @@
  *      UDPv6 GSO support
  */
 #include <linux/skbuff.h>
+#include <linux/netdevice.h>
 #include <net/protocol.h>
 #include <net/ipv6.h>
 #include <net/udp.h>
@@ -127,10 +128,42 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 out:
 	return segs;
 }
+
+static struct sk_buff **udp6_gro_receive(struct sk_buff **head,
+					 struct sk_buff *skb)
+{
+	struct udphdr *uh = udp_gro_udphdr(skb);
+
+	/* Don't bother verifying checksum if we're going to flush anyway. */
+	if (unlikely(!uh) ||
+	    (!NAPI_GRO_CB(skb)->flush &&
+	     skb_gro_checksum_validate_zero_check(skb, IPPROTO_UDP, uh->check,
+						  ip6_gro_compute_pseudo))) {
+		NAPI_GRO_CB(skb)->flush = 1;
+		return NULL;
+	}
+
+	return udp_gro_receive(head, skb, uh);
+}
+
+int udp6_gro_complete(struct sk_buff *skb, int nhoff)
+{
+	const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
+
+	if (uh->check)
+		uh->check = ~udp_v6_check(skb->len - nhoff, &ipv6h->saddr,
+					  &ipv6h->daddr, 0);
+
+	return udp_gro_complete(skb, nhoff);
+}
+
 static const struct net_offload udpv6_offload = {
 	.callbacks = {
 		.gso_send_check =	udp6_ufo_send_check,
 		.gso_segment	=	udp6_ufo_fragment,
+		.gro_receive	=	udp6_gro_receive,
+		.gro_complete	=	udp6_gro_complete,
 	},
 };
 
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v2 net-next 6/6] gre: When GRE csum is present count as encap layer wrt csum
From: Tom Herbert @ 2014-08-22 20:34 UTC (permalink / raw)
  To: davem, netdev


In GRE demux if the GRE checksum pop rcv encapsulation so that any
encapsulated checksums are treated as tunnel checksums.

Signed-off-by: Tom Herbert <therbert@google.com>
---
 net/ipv4/gre_demux.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/gre_demux.c b/net/ipv4/gre_demux.c
index 0485bf7..7c1a8ff 100644
--- a/net/ipv4/gre_demux.c
+++ b/net/ipv4/gre_demux.c
@@ -125,6 +125,7 @@ static int parse_gre_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
 			*csum_err = true;
 			return -EINVAL;
 		}
+		skb_pop_rcv_encapsulation(skb);
 		options++;
 	}
 
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* Re: [PATCH v2] carl9170: Remove redundant protection check
From: Christian Lamparter @ 2014-08-22 20:38 UTC (permalink / raw)
  To: Andreea-Cristina Bernat
  Cc: linville, linux-wireless, netdev, linux-kernel, paulmck
In-Reply-To: <20140822191431.GA5827@ada>

On Friday, August 22, 2014 10:14:31 PM Andreea-Cristina Bernat wrote:
> The carl9170_op_ampdu_action() function is used only by the mac80211
> framework. Since the mac80211 already takes care of checks and 
> properly serializing calls to the driver's function there is no
> need for the driver to do the same thing.
> 
> Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
> ---
> Changes in v2:
>  - Change subject line from
>    "carl9170: Replace rcu_dereference() with rcu_access_pointer()"
>    to
>    "carl9170: Remove redundant protection check"
>  - Update the commit message according to the modifications
>  - Delete the lines of interest at the suggestion and explanations of
>    Christian Lamparter <chunkeey@googlemail.com>
> 
>  drivers/net/wireless/ath/carl9170/main.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> index 12018ff..6758b9a 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -1430,12 +1430,6 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
>  		if (!sta_info->ht_sta)
>  			return -EOPNOTSUPP;
>  
> -		rcu_read_lock();
> -		if (rcu_access_pointer(sta_info->agg[tid])) {
> -			rcu_read_unlock();
> -			return -EBUSY;
> -		}
> -
>  		tid_info = kzalloc(sizeof(struct carl9170_sta_tid),
>  				   GFP_ATOMIC);
>  		if (!tid_info) {
> 

sparse [0] hit a bug when testing the patch:

drivers/net/wireless/ath/carl9170/main.c:1440:17: 
  warning: context imbalance in 'carl9170_op_ampdu_action' - unexpected unlock

This warning is caused by the remaining, stray rcu_read protection
in the code below (Sorry! Guess RCU needed a bit more explanation
in the previous post. If you are looking for *pointers*, there are
excellent resources available in Documentation/RCU/ [1]).

I've attached a full patch (see below) with all changes so far and
tested if the device/driver still behaves ;-). This patch applies 
cleanly on top of wireless-testing.

@John,
can you please take it?

---
From: Andreea-Cristina Bernat <bernat.ada@gmail.com>

The carl9170_op_ampdu_action() function is used only by the mac80211
framework. Since the mac80211 already takes care of checks and 
properly serializing calls to the driver's function there is no
need for the driver to do the same thing.
 
Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
[chunkeey@googlemail.com: remove two stray rcu_read_unlock()]
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index f8ded84..ef5b6dc 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1430,18 +1430,10 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
 		if (!sta_info->ht_sta)
 			return -EOPNOTSUPP;
 
-		rcu_read_lock();
-		if (rcu_dereference(sta_info->agg[tid])) {
-			rcu_read_unlock();
-			return -EBUSY;
-		}
-
 		tid_info = kzalloc(sizeof(struct carl9170_sta_tid),
 				   GFP_ATOMIC);
-		if (!tid_info) {
-			rcu_read_unlock();
+		if (!tid_info)
 			return -ENOMEM;
-		}
 
 		tid_info->hsn = tid_info->bsn = tid_info->snx = (*ssn);
 		tid_info->state = CARL9170_TID_STATE_PROGRESS;
@@ -1460,7 +1452,6 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
 		list_add_tail_rcu(&tid_info->list, &ar->tx_ampdu_list);
 		rcu_assign_pointer(sta_info->agg[tid], tid_info);
 		spin_unlock_bh(&ar->tx_ampdu_list_lock);
-		rcu_read_unlock();
 
 		ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid);
 		break;
---

Regards
Christian

[0] <http://kernelnewbies.org/Sparse>
[1] <https://www.kernel.org/doc/Documentation/RCU/whatisRCU.txt>

^ permalink raw reply related

* Re: [PATCH] phylib: use MDIO_DEVS[12]
From: Florian Fainelli @ 2014-08-22 20:41 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: netdev
In-Reply-To: <1920521.UOn8o4CGAS@wasted.cogentembedded.com>

2014-08-22 12:56 GMT-07:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> The bare register numbers are used despite <uapi/linux/mdio.h> has MDIO_DEVS[12]
> #define'd for those.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>

>
> ---
> This patch is against DaveM's 'net-next.git' repo.
>
>  drivers/net/phy/phy_device.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: net-next/drivers/net/phy/phy_device.c
> ===================================================================
> --- net-next.orig/drivers/net/phy/phy_device.c
> +++ net-next/drivers/net/phy/phy_device.c
> @@ -230,13 +230,13 @@ static int get_phy_c45_ids(struct mii_bu
>         for (i = 1;
>              i < num_ids && c45_ids->devices_in_package == 0;
>              i++) {
> -               reg_addr = MII_ADDR_C45 | i << 16 | 6;
> +               reg_addr = MII_ADDR_C45 | i << 16 | MDIO_DEVS2;
>                 phy_reg = mdiobus_read(bus, addr, reg_addr);
>                 if (phy_reg < 0)
>                         return -EIO;
>                 c45_ids->devices_in_package = (phy_reg & 0xffff) << 16;
>
> -               reg_addr = MII_ADDR_C45 | i << 16 | 5;
> +               reg_addr = MII_ADDR_C45 | i << 16 | MDIO_DEVS1;
>                 phy_reg = mdiobus_read(bus, addr, reg_addr);
>                 if (phy_reg < 0)
>                         return -EIO;
>



-- 
Florian

^ permalink raw reply

* RE: [PATCH net-next] hyperv: Add handling of IP header with option field in netvsc_set_hash()
From: Haiyang Zhang @ 2014-08-22 21:06 UTC (permalink / raw)
  To: David Miller
  Cc: olaf@aepfle.de, netdev@vger.kernel.org, jasowang@redhat.com,
	driverdev-devel@linuxdriverproject.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20140821.213057.1438247518790500739.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 22, 2014 12:31 AM
> To: Haiyang Zhang
> Cc: netdev@vger.kernel.org; KY Srinivasan; olaf@aepfle.de;
> jasowang@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net-next] hyperv: Add handling of IP header with
> option field in netvsc_set_hash()
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Tue, 19 Aug 2014 20:53:55 +0000
> 
> > @@ -200,12 +202,18 @@ static bool netvsc_set_hash(u32 *hash, struct
> sk_buff *skb)
> >  	iphdr = ip_hdr(skb);
> >
> >  	if (iphdr->version == 4) {
> > -		if (iphdr->protocol == IPPROTO_TCP)
> > +		data = (u8 *)&iphdr->saddr;
> > +		if (iphdr->protocol == IPPROTO_TCP) {
> >  			data_len = 12;
> > -		else
> > +			if (iphdr->ihl > 5) {
> > +				memcpy(dbuf, &iphdr->saddr, 8);
> > +				memcpy(&dbuf[8], &tcp_hdr(skb)->source, 4);
> 
> This is rediculous.
> 
> Make hash_comp() take a void pointer for the buffer.
> 
> Then your code is simply:
> 
> 	be32 dbuf[2];
> 
> 	dbuf[1] = iph->saddr;
> 	dbuf[2] = iph->daddr;
> 	dbuf[3] = *(be32 *)tcph->source;
> 
> 	*hash = comp_hash(netvsc_hash_key, HASH_KEYLEN, dbuf, 12);
> 
> No special cases for IP options or any garbage like that.

I have submitted a revised patch with the suggested changes.

Thanks,
- Haiyang

^ permalink raw reply

* Re: [PATCH v2] carl9170: Remove redundant protection check
From: Eric Dumazet @ 2014-08-22 21:08 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Andreea-Cristina Bernat, linville, linux-wireless, netdev,
	linux-kernel, paulmck
In-Reply-To: <1679858.MgakDZE2Vr@debian64>

On Fri, 2014-08-22 at 22:38 +0200, Christian Lamparter wrote:
> On Friday, August 22, 2014 10:14:31 PM Andreea-Cristina Bernat wrote:
> > The carl9170_op_ampdu_action() function is used only by the mac80211
> > framework. Since the mac80211 already takes care of checks and 
> > properly serializing calls to the driver's function there is no
> > need for the driver to do the same thing.
> > 
> > Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
> > ---
> > Changes in v2:
> >  - Change subject line from
> >    "carl9170: Replace rcu_dereference() with rcu_access_pointer()"
> >    to
> >    "carl9170: Remove redundant protection check"
> >  - Update the commit message according to the modifications
> >  - Delete the lines of interest at the suggestion and explanations of
> >    Christian Lamparter <chunkeey@googlemail.com>
> > 
> >  drivers/net/wireless/ath/carl9170/main.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> > index 12018ff..6758b9a 100644
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1430,12 +1430,6 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
> >  		if (!sta_info->ht_sta)
> >  			return -EOPNOTSUPP;
> >  
> > -		rcu_read_lock();
> > -		if (rcu_access_pointer(sta_info->agg[tid])) {
> > -			rcu_read_unlock();
> > -			return -EBUSY;
> > -		}
> > -
> >  		tid_info = kzalloc(sizeof(struct carl9170_sta_tid),
> >  				   GFP_ATOMIC);
> >  		if (!tid_info) {
> > 
> 
> sparse [0] hit a bug when testing the patch:
> 
> drivers/net/wireless/ath/carl9170/main.c:1440:17: 
>   warning: context imbalance in 'carl9170_op_ampdu_action' - unexpected unlock
> 
> This warning is caused by the remaining, stray rcu_read protection
> in the code below (Sorry! Guess RCU needed a bit more explanation
> in the previous post. If you are looking for *pointers*, there are
> excellent resources available in Documentation/RCU/ [1]).
> 
> I've attached a full patch (see below) with all changes so far and
> tested if the device/driver still behaves ;-). This patch applies 
> cleanly on top of wireless-testing.
> 
> @John,
> can you please take it?
> 
> ---
> From: Andreea-Cristina Bernat <bernat.ada@gmail.com>
> 
> The carl9170_op_ampdu_action() function is used only by the mac80211
> framework. Since the mac80211 already takes care of checks and 
> properly serializing calls to the driver's function there is no
> need for the driver to do the same thing.
>  
> Signed-off-by: Andreea-Cristina Bernat <bernat.ada@gmail.com>
> [chunkeey@googlemail.com: remove two stray rcu_read_unlock()]
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
> diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
> index f8ded84..ef5b6dc 100644
> --- a/drivers/net/wireless/ath/carl9170/main.c
> +++ b/drivers/net/wireless/ath/carl9170/main.c
> @@ -1430,18 +1430,10 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
>  		if (!sta_info->ht_sta)
>  			return -EOPNOTSUPP;
>  
> -		rcu_read_lock();
> -		if (rcu_dereference(sta_info->agg[tid])) {
> -			rcu_read_unlock();
> -			return -EBUSY;
> -		}
> -
>  		tid_info = kzalloc(sizeof(struct carl9170_sta_tid),
>  				   GFP_ATOMIC);
> -		if (!tid_info) {
> -			rcu_read_unlock();
> +		if (!tid_info)
>  			return -ENOMEM;
> -		}
>  
>  		tid_info->hsn = tid_info->bsn = tid_info->snx = (*ssn);
>  		tid_info->state = CARL9170_TID_STATE_PROGRESS;
> @@ -1460,7 +1452,6 @@ static int carl9170_op_ampdu_action(struct ieee80211_hw *hw,
>  		list_add_tail_rcu(&tid_info->list, &ar->tx_ampdu_list);
>  		rcu_assign_pointer(sta_info->agg[tid], tid_info);
>  		spin_unlock_bh(&ar->tx_ampdu_list_lock);
> -		rcu_read_unlock();
>  
>  		ieee80211_start_tx_ba_cb_irqsafe(vif, sta->addr, tid);
>  		break;
> ---
> 
> Regards
> Christian
> 
> [0] <http://kernelnewbies.org/Sparse>
> [1] <https://www.kernel.org/doc/Documentation/RCU/whatisRCU.txt>
> 


Sorry but this patch is not complete.

You need to somehow return -EBUSY if sta_info->agg[tid] is set.

The test has to be done inside the
spin_lock_bh(&ar->tx_ampdu_list_lock) /
spin_unlock_bh(&ar->tx_ampdu_list_lock); region.

You are correct the RCU bits were wrong in this context, but we still
have to avoid leaks.

^ permalink raw reply

* [PATCH net-next] tcp: improve undo on timeout
From: Yuchung Cheng @ 2014-08-22 21:15 UTC (permalink / raw)
  To: davem; +Cc: ncardwell, netdev, Yuchung Cheng

Upon timeout, undo (via both timestamps/Eifel and DSACKs) was
disabled if any retransmits were still in flight.  The concern was
perhaps that spurious retransmission sent in a previous recovery
episode may trigger DSACKs to falsely undo the current recovery.

However, this inadvertently misses undo opportunities (using either
TCP timestamps or DSACKs) when timeout occurs during a loss episode,
i.e.  recurring timeouts or timeout during fast recovery. In these
cases some retransmissions will be in flight but we should allow
undo. Furthermore, we should only reset undo_marker and undo_retrans
upon timeout if we are starting a new recovery episode. Finally,
when we do reset our undo state, we now do so in a manner similar
to tcp_enter_recovery(), so that we require a DSACK for each of
the outstsanding retransmissions. This will achieve the original
goal by requiring that we receive the same number of DSACKs as
retransmissions.

This patch increases the undo events by 50% on Google servers.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 include/linux/tcp.h  |  2 +-
 net/ipv4/tcp_input.c | 26 +++++++++++---------------
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fa5258f..e567f0d 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -276,7 +276,7 @@ struct tcp_sock {
 	u32	retrans_stamp;	/* Timestamp of the last retransmit,
 				 * also used in SYN-SENT to remember stamp of
 				 * the first SYN. */
-	u32	undo_marker;	/* tracking retrans started here. */
+	u32	undo_marker;	/* snd_una upon a new recovery episode. */
 	int	undo_retrans;	/* number of undoable retransmissions. */
 	u32	total_retrans;	/* Total retransmits for entire connection */
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a906e02..aba4926 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1888,21 +1888,21 @@ static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
 	tp->sacked_out = 0;
 }
 
-static void tcp_clear_retrans_partial(struct tcp_sock *tp)
+void tcp_clear_retrans(struct tcp_sock *tp)
 {
 	tp->retrans_out = 0;
 	tp->lost_out = 0;
-
 	tp->undo_marker = 0;
 	tp->undo_retrans = -1;
+	tp->fackets_out = 0;
+	tp->sacked_out = 0;
 }
 
-void tcp_clear_retrans(struct tcp_sock *tp)
+static inline void tcp_init_undo(struct tcp_sock *tp)
 {
-	tcp_clear_retrans_partial(tp);
-
-	tp->fackets_out = 0;
-	tp->sacked_out = 0;
+	tp->undo_marker = tp->snd_una;
+	/* Retransmission still in flight may cause DSACKs later. */
+	tp->undo_retrans = tp->retrans_out ? : -1;
 }
 
 /* Enter Loss state. If we detect SACK reneging, forget all SACK information
@@ -1925,18 +1925,18 @@ void tcp_enter_loss(struct sock *sk)
 		tp->prior_ssthresh = tcp_current_ssthresh(sk);
 		tp->snd_ssthresh = icsk->icsk_ca_ops->ssthresh(sk);
 		tcp_ca_event(sk, CA_EVENT_LOSS);
+		tcp_init_undo(tp);
 	}
 	tp->snd_cwnd	   = 1;
 	tp->snd_cwnd_cnt   = 0;
 	tp->snd_cwnd_stamp = tcp_time_stamp;
 
-	tcp_clear_retrans_partial(tp);
+	tp->retrans_out = 0;
+	tp->lost_out = 0;
 
 	if (tcp_is_reno(tp))
 		tcp_reset_reno_sack(tp);
 
-	tp->undo_marker = tp->snd_una;
-
 	skb = tcp_write_queue_head(sk);
 	is_reneg = skb && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED);
 	if (is_reneg) {
@@ -1950,9 +1950,6 @@ void tcp_enter_loss(struct sock *sk)
 		if (skb == tcp_send_head(sk))
 			break;
 
-		if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS)
-			tp->undo_marker = 0;
-
 		TCP_SKB_CB(skb)->sacked &= (~TCPCB_TAGBITS)|TCPCB_SACKED_ACKED;
 		if (!(TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_ACKED) || is_reneg) {
 			TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_ACKED;
@@ -2671,8 +2668,7 @@ static void tcp_enter_recovery(struct sock *sk, bool ece_ack)
 	NET_INC_STATS_BH(sock_net(sk), mib_idx);
 
 	tp->prior_ssthresh = 0;
-	tp->undo_marker = tp->snd_una;
-	tp->undo_retrans = tp->retrans_out ? : -1;
+	tcp_init_undo(tp);
 
 	if (inet_csk(sk)->icsk_ca_state < TCP_CA_CWR) {
 		if (!ece_ack)
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related


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