Netdev List
 help / color / mirror / Atom feed
* [PATCH 01/16] net: Add ESP offload features
From: Steffen Klassert @ 2017-04-20  8:55 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1492678515-14347-1-git-send-email-steffen.klassert@secunet.com>

This patch adds netdev features to configure IPsec offloads.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/netdev_features.h | 8 +++++++-
 include/linux/netdevice.h       | 1 +
 include/linux/skbuff.h          | 2 ++
 net/core/ethtool.c              | 3 +++
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 9a04195..1d4737c 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -54,8 +54,9 @@ enum {
 					 */
 	NETIF_F_GSO_TUNNEL_REMCSUM_BIT, /* ... TUNNEL with TSO & REMCSUM */
 	NETIF_F_GSO_SCTP_BIT,		/* ... SCTP fragmentation */
+	NETIF_F_GSO_ESP_BIT,		/* ... ESP with TSO */
 	/**/NETIF_F_GSO_LAST =		/* last bit, see GSO_MASK */
-		NETIF_F_GSO_SCTP_BIT,
+		NETIF_F_GSO_ESP_BIT,
 
 	NETIF_F_FCOE_CRC_BIT,		/* FCoE CRC32 */
 	NETIF_F_SCTP_CRC_BIT,		/* SCTP checksum offload */
@@ -73,6 +74,8 @@ enum {
 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
 
 	NETIF_F_HW_TC_BIT,		/* Offload TC infrastructure */
+	NETIF_F_HW_ESP_BIT,		/* Hardware ESP transformation offload */
+	NETIF_F_HW_ESP_TX_CSUM_BIT,	/* ESP with TX checksum offload */
 
 	/*
 	 * Add your fresh new feature above and remember to update
@@ -129,11 +132,14 @@ enum {
 #define NETIF_F_GSO_PARTIAL	 __NETIF_F(GSO_PARTIAL)
 #define NETIF_F_GSO_TUNNEL_REMCSUM __NETIF_F(GSO_TUNNEL_REMCSUM)
 #define NETIF_F_GSO_SCTP	__NETIF_F(GSO_SCTP)
+#define NETIF_F_GSO_ESP		__NETIF_F(GSO_ESP)
 #define NETIF_F_HW_VLAN_STAG_FILTER __NETIF_F(HW_VLAN_STAG_FILTER)
 #define NETIF_F_HW_VLAN_STAG_RX	__NETIF_F(HW_VLAN_STAG_RX)
 #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
 #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
 #define NETIF_F_HW_TC		__NETIF_F(HW_TC)
+#define NETIF_F_HW_ESP		__NETIF_F(HW_ESP)
+#define NETIF_F_HW_ESP_TX_CSUM	__NETIF_F(HW_ESP_TX_CSUM)
 
 #define for_each_netdev_feature(mask_addr, bit)	\
 	for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc07c3b..5bb03d1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4070,6 +4070,7 @@ static inline bool net_gso_ok(netdev_features_t features, int gso_type)
 	BUILD_BUG_ON(SKB_GSO_PARTIAL != (NETIF_F_GSO_PARTIAL >> NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_TUNNEL_REMCSUM != (NETIF_F_GSO_TUNNEL_REMCSUM >> NETIF_F_GSO_SHIFT));
 	BUILD_BUG_ON(SKB_GSO_SCTP    != (NETIF_F_GSO_SCTP >> NETIF_F_GSO_SHIFT));
+	BUILD_BUG_ON(SKB_GSO_ESP != (NETIF_F_GSO_ESP >> NETIF_F_GSO_SHIFT));
 
 	return (features & feature) == feature;
 }
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 741d75c..81ef53f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -492,6 +492,8 @@ enum {
 	SKB_GSO_TUNNEL_REMCSUM = 1 << 14,
 
 	SKB_GSO_SCTP = 1 << 15,
+
+	SKB_GSO_ESP = 1 << 16,
 };
 
 #if BITS_PER_LONG > 32
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 905a88a..03111a2 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -90,6 +90,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
 	[NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
 	[NETIF_F_GSO_PARTIAL_BIT] =	 "tx-gso-partial",
 	[NETIF_F_GSO_SCTP_BIT] =	 "tx-sctp-segmentation",
+	[NETIF_F_GSO_ESP_BIT] =		 "tx-esp-segmentation",
 
 	[NETIF_F_FCOE_CRC_BIT] =         "tx-checksum-fcoe-crc",
 	[NETIF_F_SCTP_CRC_BIT] =        "tx-checksum-sctp",
@@ -103,6 +104,8 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
 	[NETIF_F_RXALL_BIT] =            "rx-all",
 	[NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
 	[NETIF_F_HW_TC_BIT] =		 "hw-tc-offload",
+	[NETIF_F_HW_ESP_BIT] =		 "esp-hw-offload",
+	[NETIF_F_HW_ESP_TX_CSUM_BIT] =	 "esp-tx-csum-hw-offload",
 };
 
 static const char
-- 
2.7.4

^ permalink raw reply related

* pull request (net-next): ipsec-next 2017-04-20
From: Steffen Klassert @ 2017-04-20  8:54 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

This adds the basic infrastructure for IPsec hardware
offloading, it creates a configuration API and adjusts
the packet path.

1) Add the needed netdev features to configure IPsec offloads.

2) Add the IPsec hardware offloading API.

3) Prepare the ESP packet path for hardware offloading.

4) Add gso handlers for esp4 and esp6, this implements
   the software fallback for GSO packets.

5) Add xfrm replay handler functions for offloading.

6) Change ESP to use a synchronous crypto algorithm on
   offloading, we don't have the option for asynchronous
   returns when we handle IPsec at layer2.

7) Add a xfrm validate function to validate_xmit_skb. This
   implements the software fallback for non GSO packets.

8) Set the inner_network and inner_transport members of
   the SKB, as well as encapsulation, to reflect the actual
   positions of these headers, and removes them only once
   encryption is done on the payload.
   From Ilan Tayari.

9) Prepare the ESP GRO codepath for hardware offloading.

10) Fix incorrect null pointer check in esp6.
    From Colin Ian King.

11) Fix for the GSO software fallback path to detect the
    fallback correctly.
    From Ilan Tayari.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit f221dcd91d20cdcb893cf6e9c8894b7d6c97d649:

  Merge branch 'net-smc-next' (2017-04-11 23:01:15 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master

for you to fetch changes up to 8f92e03ecca390beed3d5ccc81023d050f0369fd:

  esp4/6: Fix GSO path for non-GSO SW-crypto packets (2017-04-19 07:48:57 +0200)

----------------------------------------------------------------
Colin Ian King (1):
      esp6: fix incorrect null pointer check on xo

Ilan Tayari (2):
      xfrm: Add encapsulation header offsets while SKB is not encrypted
      esp4/6: Fix GSO path for non-GSO SW-crypto packets

Steffen Klassert (13):
      net: Add ESP offload features
      xfrm: Add a xfrm type offload.
      xfrm: Move device notifications to a sepatate file
      xfrm: Add mode handlers for IPsec on layer 2
      xfrm: Add an IPsec hardware offloading API
      esp6: Remame esp_input_done2
      esp4: Reorganize esp_output
      esp6: Reorganize esp_output
      esp: Add gso handlers for esp4 and esp6
      xfrm: Add xfrm_replay_overflow functions for offloading
      esp: Use a synchronous crypto algorithm on offloading.
      net: Add a xfrm validate function to validate_xmit_skb
      xfrm: Prepare the GRO codepath for hardware offloading.

 include/linux/netdev_features.h |   8 +-
 include/linux/netdevice.h       |  15 ++
 include/linux/skbuff.h          |   2 +
 include/net/esp.h               |  19 +++
 include/net/xfrm.h              | 108 +++++++++++-
 include/uapi/linux/xfrm.h       |   8 +
 net/core/dev.c                  |   3 +
 net/core/ethtool.c              |   3 +
 net/ipv4/esp4.c                 | 370 ++++++++++++++++++++++------------------
 net/ipv4/esp4_offload.c         | 231 +++++++++++++++++++++++--
 net/ipv4/xfrm4_mode_transport.c |  34 ++++
 net/ipv4/xfrm4_mode_tunnel.c    |  28 +++
 net/ipv4/xfrm4_output.c         |   3 +-
 net/ipv6/esp6.c                 | 292 +++++++++++++++++--------------
 net/ipv6/esp6_offload.c         | 233 +++++++++++++++++++++++--
 net/ipv6/xfrm6_mode_transport.c |  34 ++++
 net/ipv6/xfrm6_mode_tunnel.c    |  27 +++
 net/ipv6/xfrm6_output.c         |   9 +-
 net/xfrm/Makefile               |   1 +
 net/xfrm/xfrm_device.c          | 208 ++++++++++++++++++++++
 net/xfrm/xfrm_input.c           |  41 ++++-
 net/xfrm/xfrm_output.c          |  46 ++++-
 net/xfrm/xfrm_policy.c          |  27 +--
 net/xfrm/xfrm_replay.c          | 162 +++++++++++++++++-
 net/xfrm/xfrm_state.c           | 147 ++++++++++++++++
 net/xfrm/xfrm_user.c            |  28 +++
 26 files changed, 1717 insertions(+), 370 deletions(-)
 create mode 100644 net/xfrm/xfrm_device.c

^ permalink raw reply

* Re: [net-next 00/15][pull request] 10GbE Intel Wired LAN Driver Updates 2017-04-18
From: Jeff Kirsher @ 2017-04-20  8:44 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: davem, netdev, nhorman, sassmann, jogreene
In-Reply-To: <20170420052515.GA77954@ast-mbp.thefacebook.com>

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Wed, 2017-04-19 at 22:25 -0700, Alexei Starovoitov wrote:
> On Tue, Apr 18, 2017 at 04:01:50PM -0700, Jeff Kirsher wrote:
> > The following are changes since commit
> > 4116c97689b9b1732ac5b68afd922406f9fc842e:
> >    Merge branch 'ftgmac100-batch5-features'
> > and are available in the git repository at:
> >    git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-
> > queue 10GbE
> 
> Nothing against this set of commits,
> but I saw ixgbe xdp support in there pending for few months.
> Now it's gone. What happened?

XDP is in the next series of patches, per Dave, we need to keep patch
series smaller to help in revue.  Next series will contain what you are
looking for, just need to get this up before I can push the next
series.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH v2] mdio_bus: Issue GPIO RESET to PHYs.
From: Roger Quadros @ 2017-04-20  8:39 UTC (permalink / raw)
  To: davem, Andrew Lunn, Florian Fainelli
  Cc: tony, nsekhar, jsarha, netdev, linux-omap, linux-kernel, rogerq
In-Reply-To: <64d6494d-41d2-0faf-a434-057f796637fe@ti.com>

Some boards [1] leave the PHYs at an invalid state
during system power-up or reset thus causing unreliability
issues with the PHY which manifests as PHY not being detected
or link not functional. To fix this, these PHYs need to be RESET
via a GPIO connected to the PHY's RESET pin.

Some boards have a single GPIO controlling the PHY RESET pin of all
PHYs on the bus whereas some others have separate GPIOs controlling
individual PHY RESETs.

In both cases, the RESET de-assertion cannot be done in the PHY driver
as the PHY will not probe till its reset is de-asserted.
So do the RESET de-assertion in the MDIO bus driver.

[1] - am572x-idk, am571x-idk, a437x-idk

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
v2:
- add device tree binding document (mdio.txt)
- specify default reset delay in of_mdio.c instead of mdio_bus.c

 Documentation/devicetree/bindings/net/mdio.txt | 20 ++++++++++++++++++++
 drivers/net/phy/mdio_bus.c                     | 22 ++++++++++++++++++++++
 drivers/of/of_mdio.c                           |  7 +++++++
 include/linux/phy.h                            |  5 +++++
 4 files changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/mdio.txt

diff --git a/Documentation/devicetree/bindings/net/mdio.txt b/Documentation/devicetree/bindings/net/mdio.txt
new file mode 100644
index 0000000..6e703d7
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio.txt
@@ -0,0 +1,20 @@
+Common MDIO bus properties.
+
+These are generic properties that can apply to any MDIO bus.
+
+Optional properties:
+- reset-gpios: List of one or more GPIOs that control the RESET lines
+  of the PHYs on that MDIO bus.
+- reset-delay-us: RESET pulse width as per PHY datasheet.
+
+Example :
+
+	davinci_mdio: ethernet@0x5c030000 {
+		compatible = "ti,davinci_mdio";
+		ti,hwmods = "davinci_mdio";
+		reg = <0x5c030000 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+		reset-delay-us = <2>;   /* PHY datasheet states 1uS min */
+	};
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index fa7d51f..b353d99 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -22,8 +22,11 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of_device.h>
 #include <linux/of_mdio.h>
+#include <linux/of_gpio.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/skbuff.h>
@@ -307,6 +310,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 {
 	struct mdio_device *mdiodev;
 	int i, err;
+	struct gpio_desc *gpiod;
 
 	if (NULL == bus || NULL == bus->name ||
 	    NULL == bus->read || NULL == bus->write)
@@ -333,6 +337,24 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 	if (bus->reset)
 		bus->reset(bus);
 
+	/* de-assert bus level PHY GPIO resets */
+	for (i = 0; i < bus->num_reset_gpios; i++) {
+		gpiod = devm_gpiod_get_index(&bus->dev, "reset", i,
+					     GPIOD_OUT_LOW);
+		if (IS_ERR(gpiod)) {
+			err = PTR_ERR(gpiod);
+			if (err != -ENOENT) {
+				pr_err("mii_bus %s couldn't get reset GPIO\n",
+				       bus->id);
+				return err;
+			}
+		} else {
+			gpiod_set_value_cansleep(gpiod, 1);
+			udelay(bus->reset_delay_us);
+			gpiod_set_value_cansleep(gpiod, 0);
+		}
+	}
+
 	for (i = 0; i < PHY_MAX_ADDR; i++) {
 		if ((bus->phy_mask & (1 << i)) == 0) {
 			struct phy_device *phydev;
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 0b29798..7e4c80f 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -22,6 +22,8 @@
 #include <linux/of_net.h>
 #include <linux/module.h>
 
+#define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
+
 MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_LICENSE("GPL");
 
@@ -221,6 +223,11 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 
 	mdio->dev.of_node = np;
 
+	/* Get bus level PHY reset GPIO details */
+	mdio->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
+	of_property_read_u32(np, "reset-delay-us", &mdio->reset_delay_us);
+	mdio->num_reset_gpios = of_gpio_named_count(np, "reset-gpios");
+
 	/* Register the MDIO bus */
 	rc = mdiobus_register(mdio);
 	if (rc)
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 43a7748..80a6574 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -217,6 +217,11 @@ struct mii_bus {
 	 * matching its address
 	 */
 	int irq[PHY_MAX_ADDR];
+
+	/* GPIO reset pulse width in uS */
+	int reset_delay_us;
+	/* Number of reset GPIOs */
+	int num_reset_gpios;
 };
 #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH net] MAINTAINERS: Add new IPsec offloading files.
From: Steffen Klassert @ 2017-04-20  8:36 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

This adds two new files to IPsec maintenance scope:

net/ipv4/esp4_offload.c
net/ipv6/ip6_offload.c

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5397f54..b01f923 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8801,12 +8801,12 @@ F:	net/core/flow.c
 F:	net/xfrm/
 F:	net/key/
 F:	net/ipv4/xfrm*
-F:	net/ipv4/esp4.c
+F:	net/ipv4/esp4*
 F:	net/ipv4/ah4.c
 F:	net/ipv4/ipcomp.c
 F:	net/ipv4/ip_vti.c
 F:	net/ipv6/xfrm*
-F:	net/ipv6/esp6.c
+F:	net/ipv6/esp6*
 F:	net/ipv6/ah6.c
 F:	net/ipv6/ipcomp6.c
 F:	net/ipv6/ip6_vti.c
-- 
2.7.4

^ permalink raw reply related

* Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone
From: Dmitry Vyukov @ 2017-04-20  8:35 UTC (permalink / raw)
  To: David Ahern
  Cc: Cong Wang, Andrey Konovalov, Eric Dumazet, Mahesh Bandewar,
	Eric Dumazet, David Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, syzkaller
In-Reply-To: <1dad955e-28c1-3f02-8876-90cce4a7d1f8@cumulusnetworks.com>

On Thu, Apr 20, 2017 at 1:51 AM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 4/19/17 5:47 PM, Cong Wang wrote:
>> On Wed, Apr 19, 2017 at 9:12 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>>>
>>> Anyway, I just finished simplifying the reproducer. Give this one a try.
>>
>> Thanks for providing such a minimal reproducer!
>>
>> The following patch could fix this crash, but I am not 100% sure if we should
>> just clear these bits or reject them with an errno.
>>
>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>> index 9db14189..cf524c2 100644
>> --- a/net/ipv6/route.c
>> +++ b/net/ipv6/route.c
>> @@ -2086,7 +2086,7 @@ static struct rt6_info
>> *ip6_route_info_create(struct fib6_config *cfg)
>>         } else
>>                 rt->rt6i_prefsrc.plen = 0;
>>
>> -       rt->rt6i_flags = cfg->fc_flags;
>> +       rt->rt6i_flags = cfg->fc_flags & ~(RTF_PCPU | RTF_CACHE);
>>
>>  install_route:
>>         rt->dst.dev = dev;
>>
>
> I sent a patch returning EINVAL if RTF_PCPU is set in fc_flags


Andrey, does it fix the other crashes?

^ permalink raw reply

* Re: [PATCH] mdio_bus: Issue GPIO RESET to PHYs.
From: Roger Quadros @ 2017-04-20  7:58 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: davem, Florian Fainelli, tony, nsekhar, jsarha, netdev,
	linux-omap, linux-kernel
In-Reply-To: <20170419133853.GA2696@lunn.ch>

On 19/04/17 16:38, Andrew Lunn wrote:
> On Wed, Apr 19, 2017 at 02:56:48PM +0300, Roger Quadros wrote:
>> Hi,
>>
>> On 19/04/17 14:39, Andrew Lunn wrote:
>>> On Wed, Apr 19, 2017 at 12:24:26PM +0300, Roger Quadros wrote:
>>>> Some boards [1] leave the PHYs at an invalid state
>>>> during system power-up or reset thus causing unreliability
>>>> issues with the PHY which manifests as PHY not being detected
>>>> or link not functional. To fix this, these PHYs need to be RESET
>>>> via a GPIO connected to the PHY's RESET pin.
>>>>
>>>> Some boards have a single GPIO controlling the PHY RESET pin of all
>>>> PHYs on the bus whereas some others have separate GPIOs controlling
>>>> individual PHY RESETs.
>>>>
>>>> In both cases, the RESET de-assertion cannot be done in the PHY driver
>>>> as the PHY will not probe till its reset is de-asserted.
>>>> So do the RESET de-assertion in the MDIO bus driver.
>>>>
>>>> [1] - am572x-idk, am571x-idk, a437x-idk
>>>>
>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>>> ---
>>>>  drivers/net/phy/mdio_bus.c | 26 ++++++++++++++++++++++++++
>>>>  drivers/of/of_mdio.c       |  4 ++++
>>>>  include/linux/phy.h        |  5 +++++
>>>>  3 files changed, 35 insertions(+)
>>>
>>> Hi Roger
>>>
>>> Thanks for making this generic.
>>>
>>> Please add device tree binding documentation. I think that actually
>>> means you have to document MDIO in general, since there currently is
>>> not a binding document.
>>
>> OK.
>>
>>>
>>>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>>>> index fa7d51f..25fda2b 100644
>>>> --- a/drivers/net/phy/mdio_bus.c
>>>> +++ b/drivers/net/phy/mdio_bus.c
>>>> @@ -22,8 +22,11 @@
>>>>  #include <linux/init.h>
>>>>  #include <linux/delay.h>
>>>>  #include <linux/device.h>
>>>> +#include <linux/gpio.h>
>>>> +#include <linux/gpio/consumer.h>
>>>>  #include <linux/of_device.h>
>>>>  #include <linux/of_mdio.h>
>>>> +#include <linux/of_gpio.h>
>>>>  #include <linux/netdevice.h>
>>>>  #include <linux/etherdevice.h>
>>>>  #include <linux/skbuff.h>
>>>> @@ -43,6 +46,8 @@
>>>>  
>>>>  #include "mdio-boardinfo.h"
>>>>  
>>>> +#define DEFAULT_GPIO_RESET_DELAY	10	/* in microseconds */
>>>> +
>>>>  int mdiobus_register_device(struct mdio_device *mdiodev)
>>>>  {
>>>>  	if (mdiodev->bus->mdio_map[mdiodev->addr])
>>>> @@ -307,6 +312,7 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>>>>  {
>>>>  	struct mdio_device *mdiodev;
>>>>  	int i, err;
>>>> +	struct gpio_desc *gpiod;
>>>>  
>>>>  	if (NULL == bus || NULL == bus->name ||
>>>>  	    NULL == bus->read || NULL == bus->write)
>>>> @@ -333,6 +339,26 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>>>>  	if (bus->reset)
>>>>  		bus->reset(bus);
>>>>  
>>>> +	/* de-assert bus level PHY GPIO resets */
>>>> +	for (i = 0; i < bus->num_reset_gpios; i++) {
>>>> +		gpiod = devm_gpiod_get_index(&bus->dev, "reset", i,
>>>> +					     GPIOD_OUT_LOW);
>>>> +		if (IS_ERR(gpiod)) {
>>>> +			err = PTR_ERR(gpiod);
>>>> +			if (err != -ENOENT) {
>>>> +				pr_err("mii_bus %s couldn't get reset GPIO\n",
>>>> +				       bus->id);
>>>> +				return err;
>>>> +			}
>>>> +		} else {
>>>> +			gpiod_set_value_cansleep(gpiod, 1);
>>>
>>>
>>>> +			if (!bus->reset_delay_us)
>>>> +				bus->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
>>>
>>> Maybe do this once, where you read the device tree property.
>>
>> I was thinking from point of view that GPIO RESET code should work even without
>> device tree. Although I'm not sure if there would be any users or not.
> 
> Hi Roger
> 
> I don't see how this would work. What would devm_gpiod_get_index()
> return? Something from ACPI? But then there would be something

Yes or just lookup tables based on platform data.

> equivalent for getting the delay.

I'm not sure about delay part.

> 
> Lets keep it simple and OF only. If there is a real need for something
> in addition to OF, it can be added later.

OK. I'll revise the patch based on your comments.

cheers,
-roger

^ permalink raw reply

* [PATCH net] Fix net/hsr/hsr_device to check for freed skb buffer.
From: Peter Heise @ 2017-04-20  7:28 UTC (permalink / raw)
  To: Arvid Brodin, David S. Miller; +Cc: netdev

Fixed an unchecked call of skb_put_padto. Return value was ignored
before, however, skb_put_padto frees skb buffer in case of error.

As reported by Dan Carpenter on kernel-janitors.

Signed-off-by: Peter Heise <mail@pheise.de>
---
 net/hsr/hsr_device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index c73160fb11e7..22d693f213be 100644
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -314,7 +314,8 @@ static void send_hsr_supervision_frame(struct hsr_port *master,
 	hsr_sp = (typeof(hsr_sp)) skb_put(skb, sizeof(struct hsr_sup_payload));
 	ether_addr_copy(hsr_sp->MacAddressA, master->dev->dev_addr);
 
-	skb_put_padto(skb, ETH_ZLEN + HSR_HLEN);
+	if(skb_put_padto(skb, ETH_ZLEN + HSR_HLEN))
+		return;
 
 	hsr_forward_skb(skb, master);
 	return;
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] nl80211: fix enumeration type
From: Johannes Berg @ 2017-04-20  7:22 UTC (permalink / raw)
  To: Stefan Agner; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20170420065513.1930-1-stefan@agner.ch>

On Wed, 2017-04-19 at 23:55 -0700, Stefan Agner wrote:
> Use type enum nl80211_rate_info for bitrate information. This fixes
> a warning when compiling with clang:
>   warning: implicit conversion from enumeration type 'enum
> nl80211_rate_info'
>   to different enumeration type 'enum nl80211_attrs'

I'm pretty sure I just applied exactly these two patches very recently.

What made *two* people suddenly see this?

johannes

^ permalink raw reply

* Re: mwifiex: MAC randomization should not be persistent
From: Kalle Valo @ 2017-04-20  7:17 UTC (permalink / raw)
  To: Brian Norris
  Cc: Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu, linux-kernel,
	Dmitry Torokhov, netdev, linux-wireless, Brian Norris
In-Reply-To: <20170405222640.4494-1-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:
> nl80211 provides the NL80211_SCAN_FLAG_RANDOM_ADDR for every scan
> request that should be randomized; the absence of such a flag means we
> should not randomize. However, mwifiex was stashing the latest
> randomization request and *always* using it for future scans, even those
> that didn't set the flag.
> 
> Let's zero out the randomization info whenever we get a scan request
> without NL80211_SCAN_FLAG_RANDOM_ADDR. I'd prefer to remove
> priv->random_mac entirely (and plumb the randomization MAC properly
> through the call sequence), but the spaghetti is a little difficult to
> unravel here for me.
> 
> Fixes: c2a8f0ff9c6c ("mwifiex: support random MAC address for scanning")
> Cc: <stable@vger.kernel.org> # 4.9+
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

7e2f18f06408 mwifiex: MAC randomization should not be persistent

-- 
https://patchwork.kernel.org/patch/9665813/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH] mac80211: fix enumeration type
From: Stefan Agner @ 2017-04-20  7:02 UTC (permalink / raw)
  To: johannes; +Cc: davem, linux-wireless, netdev, linux-kernel, Stefan Agner

Use enum nl80211_channel_type for cfg80211_chandef_create function
calls. This does not has an effect in practise since the enum values
are equal. This fixes a warning when compiling with clang:
  warning: implicit conversion from enumeration type 'enum nl80211_chan_width'
  to different enumeration type 'enum nl80211_channel_type'

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 net/mac80211/ibss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 98999d3d5262..5b748025147d 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -425,7 +425,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	case NL80211_CHAN_WIDTH_5:
 	case NL80211_CHAN_WIDTH_10:
 		cfg80211_chandef_create(&chandef, cbss->channel,
-					NL80211_CHAN_WIDTH_20_NOHT);
+					NL80211_CHAN_HT20);
 		chandef.width = sdata->u.ibss.chandef.width;
 		break;
 	case NL80211_CHAN_WIDTH_80:
@@ -437,7 +437,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	default:
 		/* fall back to 20 MHz for unsupported modes */
 		cfg80211_chandef_create(&chandef, cbss->channel,
-					NL80211_CHAN_WIDTH_20_NOHT);
+					NL80211_CHAN_HT20);
 		break;
 	}
 
-- 
2.12.2

^ permalink raw reply related

* [PATCH] nl80211: fix enumeration type
From: Stefan Agner @ 2017-04-20  6:55 UTC (permalink / raw)
  To: johannes; +Cc: davem, linux-wireless, netdev, linux-kernel, Stefan Agner

Use type enum nl80211_rate_info for bitrate information. This fixes
a warning when compiling with clang:
  warning: implicit conversion from enumeration type 'enum nl80211_rate_info'
  to different enumeration type 'enum nl80211_attrs'

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 net/wireless/nl80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2312dc2ffdb9..9af21a21ea6b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4151,7 +4151,7 @@ static bool nl80211_put_sta_rate(struct sk_buff *msg, struct rate_info *info,
 	struct nlattr *rate;
 	u32 bitrate;
 	u16 bitrate_compat;
-	enum nl80211_attrs rate_flg;
+	enum nl80211_rate_info rate_flg;
 
 	rate = nla_nest_start(msg, attr);
 	if (!rate)
-- 
2.12.2

^ permalink raw reply related

* Re: [PATCH v4 04/18] dt-bindings: syscon: Add DT bindings documentation for Allwinner syscon
From: Corentin Labbe @ 2017-04-20  6:50 UTC (permalink / raw)
  To: André Przywara
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <9382f9ae-895a-397b-3d8c-846cb0bc4486-5wv7dgnIgG8@public.gmane.org>

On Thu, Apr 20, 2017 at 12:38:50AM +0100, André Przywara wrote:
> On 12/04/17 12:13, Corentin Labbe wrote:
> > This patch adds documentation for Device-Tree bindings for the
> > syscon present in allwinner devices.
> > 
> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/misc/allwinner,syscon.txt     | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/misc/allwinner,syscon.txt b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> > new file mode 100644
> > index 0000000..c056c5b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/misc/allwinner,syscon.txt
> > @@ -0,0 +1,19 @@
> > +* Allwinner sun8i system controller
> > +
> > +This file describes the bindings for the system controller present in
> > +Allwinner SoC H3, A83T and A64.
> > +The principal function of this syscon is to control EMAC PHY choice and
> > +config.
> > +
> > +Required properties for the system controller:
> > +- reg: address and length of the register for the device.
> > +- compatible: should be "syscon" and one of the following string:
> > +		"allwinner,sun8i-h3-system-controller"
> > +		"allwinner,sun8i-a64-system-controller"
> 
> While sun8i might make some sense technically, all 64-bit sunxi
> compatible strings use the sun50i prefix to follow the Allwinner naming.
> So this should read:
> 		"allwinner,sun50i-a64-system-controller"
> 
> Also I am wondering if we should add a compatible string for the H5
> (support for that SoC is in -next already):
> 		"allwinner,sun50i-h5-system-controller"
> 
> Cheers,
> Andre.
> 

I agree with sun50i-a64-system-controller, the sun8i-a64 is a copy/paste error.
I will add sun50i-h5-system-controller since all xxx-system-controller are just hypothetic compatible.

Thanks
Regards

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* (From: Mr. James Tan (Urgent & Confidential)
From: James Tan @ 2017-04-20  6:46 UTC (permalink / raw)


-- 
From: Mr James Tan (Urgent & Confidential)

Good Day, Please Read.

My name is Mr. James Tan, I am the Bill and Exchange manager here in Bank
of Africa (BOA) Lome-Togo,West-Africa. I have a business proposal in the
tune of $9.7m, (Nine Million Seven hundred Thousand United States
Dollars)only, after the successful transfer;we shall share in ratio of 40%
for you and 60% for me.

Should you be interested, please contact me through my private email (
tanjames009@gmail.com) so we can commence all arrangements and i will give
you more information on how we would handle this project.


Please treat this business with utmost confidentiality and send me the
Following:

1. Your Full Name:............
2. Your Phone Number:.........
3. Your Age:..................
4. Your Sex:..................
5. Your Occupations:..........
6. Your Country and City:....

Kind Regards,

Mr. James Tan.
Bill & Exchange manager
(BOA) Bank of Africa.
Lome-Togo.
Email: tanjames009@gmail.com

^ permalink raw reply

* Re: XDP question: best API for returning/setting egress port?
From: Jesper Dangaard Brouer @ 2017-04-20  6:39 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andy Gospodarek, Daniel Borkmann, Alexei Starovoitov,
	Alexei Starovoitov, netdev@vger.kernel.org,
	xdp-newbies@vger.kernel.org, John Fastabend, brouer
In-Reply-To: <58F7E9F3.5090604@iogearbox.net>

On Thu, 20 Apr 2017 00:51:31 +0200
Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 04/19/2017 10:02 PM, Andy Gospodarek wrote:
> [...]
> > and then lookup this dest in a table we have the option to make that
> > dest an ifindex/socket/other.
> >
> > I did also look at JohnF's patch and I do like the simplicity of the redirect
> > action and new ndo_xdp_xmit and how it moves towards a way to transmit the
> > frame.  The downside is that it presumes an ifindex, so it might not be ideal
> > we want the lookup to return something other than an ifindex.
> >  
> [...]
> > would be handled.  If we are ultimately going to need a new netdev op to
> > handle the redirect then what may be the issue with not providing the
> > destination port the return code and the option proposed by JohnF looks
> > good to me with maybe a small tweak to not presume ifindex in some manner.  
> 
> Is there a concrete reason that all the proposed future cases like sockets
> have to be handled within the very same XDP_REDIRECT return code? F.e. why
> not XDP_TX_NIC that only assumes ifindex as proposed in the patch, and future
> ones would get a different return code f.e. XDP_TX_SK only handling sockets
> when we get there implementation-wise?

The concrete reason for an "unified" return code is to simplify changes
needed in the drivers.  The purpose is moving code out of the drivers
into the core.  IMHO we should keep drivers as simple as possible and
stop adding more return action codes. XDP_REDIRECT combined with a
port-table should be the last action-code needed in the drivers.
As the port table will know the "type" of the port.

Keeping action XDP_DROP and XDP_TX inside the drivers make sense,
because it allow for driver level optimizations. Actions that want to
transfer the packet/xdp_buff "outside" the driver should call a core
function (given it will be called with the xdp_buff struct, you can
hide all your extra extensions there without changing the driver code).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH net-next v2 2/5] virtio-net: transmit napi
From: Jason Wang @ 2017-04-20  6:27 UTC (permalink / raw)
  To: Willem de Bruijn, netdev; +Cc: Willem de Bruijn, virtualization, davem, mst
In-Reply-To: <20170418202108.61920-3-willemdebruijn.kernel@gmail.com>



On 2017年04月19日 04:21, Willem de Bruijn wrote:
> +static void virtnet_napi_tx_enable(struct virtnet_info *vi,
> +				   struct virtqueue *vq,
> +				   struct napi_struct *napi)
> +{
> +	if (!napi->weight)
> +		return;
> +
> +	if (!vi->affinity_hint_set) {
> +		napi->weight = 0;
> +		return;
> +	}
> +
> +	return virtnet_napi_enable(vq, napi);
> +}
> +
>   static void refill_work(struct work_struct *work)

Maybe I was wrong, but according to Michael's comment it looks like he 
want check affinity_hint_set just for speculative tx polling on rx napi 
instead of disabling it at all.

And I'm not convinced this is really needed, driver only provide 
affinity hint instead of affinity, so it's not guaranteed that tx and rx 
interrupt are in the same vcpus.

Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH net-next v2 5/5] virtio-net: keep tx interrupts disabled unless kick
From: Jason Wang @ 2017-04-20  6:17 UTC (permalink / raw)
  To: Willem de Bruijn, netdev; +Cc: Willem de Bruijn, virtualization, davem, mst
In-Reply-To: <20170418202108.61920-6-willemdebruijn.kernel@gmail.com>



On 2017年04月19日 04:21, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Tx napi mode increases the rate of transmit interrupts. Suppress some
> by masking interrupts while more packets are expected. The interrupts
> will be reenabled before the last packet is sent.
>
> This optimization reduces the througput drop with tx napi for
> unidirectional flows such as UDP_STREAM that do not benefit from
> cleaning tx completions in the the receive napi handler.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>   drivers/net/virtio_net.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b14c82ce0032..b107ae011632 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1196,8 +1196,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>   	bool use_napi = sq->napi.weight;
>   
>   	/* Free up any pending old buffers before queueing new ones. */
> -	if (!use_napi)
> +	if (use_napi) {
> +		if (kick)
> +			virtqueue_enable_cb_delayed(sq->vq);
> +		else
> +			virtqueue_disable_cb(sq->vq);

Since virtqueue_disable_cb() do nothing for event idx. I wonder whether 
or not just calling enable_cb_dealyed() is ok here.

Btw, it does not disable interrupt at all, I propose a patch in the past 
which can do more than this:

https://patchwork.kernel.org/patch/6472601/

Thanks



> +	} else {
>   		free_old_xmit_skbs(sq);
> +	}
>   
>   	/* timestamp packet in software */
>   	skb_tx_timestamp(skb);

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [net-next 04/14] i40e: dump VF information in debugfs
From: Or Gerlitz @ 2017-04-20  6:16 UTC (permalink / raw)
  To: Jeff Kirsher, Mitch Williams
  Cc: David Miller, Linux Netdev List, nhorman@redhat.com,
	sassmann@redhat.com, jogreene@redhat.com
In-Reply-To: <20170420015750.6828-5-jeffrey.t.kirsher@intel.com>

On Thu, Apr 20, 2017 at 4:57 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Mitch Williams <mitch.a.williams@intel.com>
>
> Dump some internal state about VFs through debugfs. This provides
> information not available with 'ip link show'.

such as?

donnwantobethedebugfspolice, but still, in the 2-3 times we tried to
push debugfs to MLNX NIC
drivers, Dave disallowed that, and lately the switch team even went
further and deleted that
portion of the mlxsw driver -- all to all,  I don't see much point for
these type of changes, thoughts?

^ permalink raw reply

* Re: [PATCH net] netdevice: Prefer NETIF_F_HW_CSUM when intersecting features
From: Michal Kubecek @ 2017-04-20  6:13 UTC (permalink / raw)
  To: Vladislav Yasevich; +Cc: netdev, tom, Vladislav Yasevich
In-Reply-To: <1492650743-28164-1-git-send-email-vyasevic@redhat.com>

On Wed, Apr 19, 2017 at 09:12:23PM -0400, Vladislav Yasevich wrote:
> While hardware device use either NETIF_F_(IP|IPV6)_CSUM or
> NETIF_F_HW_CSUM, all of the software devices use HW_CSUM.
> This results in an interesting situation when the software
> device is configured on top of hw device using (IP|IPV6)_CSUM.
> In this situation, the user can't turn off checksum offloading
> features on the software device.
> 
> This patch resolves that by prefering the NETIF_F_HW_CSUM setting
> when computing a feature intersect.

The reasoning makes sense to me but perhaps we should rename the helper
then to make it obvious that it doesn't work like intersection anymore
(not even in the logical sense).

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 97456b25..3d811c1 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4019,9 +4019,9 @@ static inline netdev_features_t netdev_intersect_features(netdev_features_t f1,
>  {
>  	if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
>  		if (f1 & NETIF_F_HW_CSUM)
> -			f1 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> +			f2 |= NETIF_F_HW_CSUM;
>  		else
> -			f2 |= (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
> +			f1 |= NETIF_F_HW_CSUM;
>  	}
>  
>  	return f1 & f2;

I hope I didn't miss something but it seems we can avoid nested ifs now:

 	if ((f1 ^ f2) & NETIF_F_HW_CSUM) {
		f1 |= NETIF_F_HW_CSUM;
		f2 |= NETIF_F_HW_CSUM;
 	}

Michal Kubecek

^ permalink raw reply

* Re: [PATCH net-next v2 2/5] virtio-net: transmit napi
From: Jason Wang @ 2017-04-20  6:12 UTC (permalink / raw)
  To: Willem de Bruijn, netdev; +Cc: mst, virtualization, davem, Willem de Bruijn
In-Reply-To: <20170418202108.61920-3-willemdebruijn.kernel@gmail.com>



On 2017年04月19日 04:21, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Convert virtio-net to a standard napi tx completion path. This enables
> better TCP pacing using TCP small queues and increases single stream
> throughput.
>
> The virtio-net driver currently cleans tx descriptors on transmission
> of new packets in ndo_start_xmit. Latency depends on new traffic, so
> is unbounded. To avoid deadlock when a socket reaches its snd limit,
> packets are orphaned on tranmission. This breaks socket backpressure,
> including TSQ.
>
> Napi increases the number of interrupts generated compared to the
> current model, which keeps interrupts disabled as long as the ring
> has enough free descriptors. Keep tx napi optional and disabled for
> now. Follow-on patches will reduce the interrupt cost.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>   drivers/net/virtio_net.c | 77 +++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 67 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b9c1df29892c..c173e85dc7b8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -33,9 +33,10 @@
>   static int napi_weight = NAPI_POLL_WEIGHT;
>   module_param(napi_weight, int, 0444);
>   
> -static bool csum = true, gso = true;
> +static bool csum = true, gso = true, napi_tx;
>   module_param(csum, bool, 0444);
>   module_param(gso, bool, 0444);
> +module_param(napi_tx, bool, 0644);
>   
>   /* FIXME: MTU in config. */
>   #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> @@ -86,6 +87,8 @@ struct send_queue {
>   
>   	/* Name of the send queue: output.$index */
>   	char name[40];
> +
> +	struct napi_struct napi;
>   };
>   
>   /* Internal representation of a receive virtqueue */
> @@ -262,12 +265,16 @@ static void virtqueue_napi_complete(struct napi_struct *napi,
>   static void skb_xmit_done(struct virtqueue *vq)
>   {
>   	struct virtnet_info *vi = vq->vdev->priv;
> +	struct napi_struct *napi = &vi->sq[vq2txq(vq)].napi;
>   
>   	/* Suppress further interrupts. */
>   	virtqueue_disable_cb(vq);
>   
> -	/* We were probably waiting for more output buffers. */
> -	netif_wake_subqueue(vi->dev, vq2txq(vq));
> +	if (napi->weight)
> +		virtqueue_napi_schedule(napi, vq);
> +	else
> +		/* We were probably waiting for more output buffers. */
> +		netif_wake_subqueue(vi->dev, vq2txq(vq));
>   }
>   
>   static unsigned int mergeable_ctx_to_buf_truesize(unsigned long mrg_ctx)
> @@ -972,6 +979,21 @@ static void virtnet_napi_enable(struct virtqueue *vq, struct napi_struct *napi)
>   	local_bh_enable();
>   }
>   
> +static void virtnet_napi_tx_enable(struct virtnet_info *vi,
> +				   struct virtqueue *vq,
> +				   struct napi_struct *napi)
> +{
> +	if (!napi->weight)
> +		return;
> +
> +	if (!vi->affinity_hint_set) {
> +		napi->weight = 0;
> +		return;
> +	}
> +
> +	return virtnet_napi_enable(vq, napi);
> +}
> +
>   static void refill_work(struct work_struct *work)
>   {
>   	struct virtnet_info *vi =
> @@ -1046,6 +1068,7 @@ static int virtnet_open(struct net_device *dev)
>   			if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
>   				schedule_delayed_work(&vi->refill, 0);
>   		virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi);
> +		virtnet_napi_tx_enable(vi, vi->sq[i].vq, &vi->sq[i].napi);
>   	}
>   
>   	return 0;
> @@ -1081,6 +1104,25 @@ static void free_old_xmit_skbs(struct send_queue *sq)
>   	u64_stats_update_end(&stats->tx_syncp);
>   }
>   
> +static int virtnet_poll_tx(struct napi_struct *napi, int budget)
> +{
> +	struct send_queue *sq = container_of(napi, struct send_queue, napi);
> +	struct virtnet_info *vi = sq->vq->vdev->priv;
> +	struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq));
> +
> +	if (__netif_tx_trylock(txq)) {
> +		free_old_xmit_skbs(sq);
> +		__netif_tx_unlock(txq);
> +	}
> +
> +	virtqueue_napi_complete(napi, sq->vq, 0);
> +
> +	if (sq->vq->num_free >= 2 + MAX_SKB_FRAGS)
> +		netif_tx_wake_queue(txq);
> +
> +	return 0;
> +}
> +
>   static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
>   {
>   	struct virtio_net_hdr_mrg_rxbuf *hdr;
> @@ -1130,9 +1172,11 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>   	int err;
>   	struct netdev_queue *txq = netdev_get_tx_queue(dev, qnum);
>   	bool kick = !skb->xmit_more;
> +	bool use_napi = sq->napi.weight;
>   
>   	/* Free up any pending old buffers before queueing new ones. */
> -	free_old_xmit_skbs(sq);
> +	if (!use_napi)
> +		free_old_xmit_skbs(sq);

I'm not sure this is best or even correct. Consider we clean xmit 
packets speculatively in virtnet_poll_tx(), we need call 
free_old_xmit_skbs() unconditionally. This can also help to reduce the 
possible of napi rescheduling in virtnet_poll_tx().

Thanks

^ permalink raw reply

* Re: xdp_redirect ifindex vs port. Was: best API for returning/setting egress port?
From: Jesper Dangaard Brouer @ 2017-04-20  6:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Daniel Borkmann, Andy Gospodarek, Daniel Borkmann,
	Alexei Starovoitov, netdev@vger.kernel.org,
	xdp-newbies@vger.kernel.org, John Fastabend, brouer
In-Reply-To: <20170420025611.GA53935@ast-mbp.thefacebook.com>

On Wed, 19 Apr 2017 19:56:13 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Thu, Apr 20, 2017 at 12:51:31AM +0200, Daniel Borkmann wrote:
> > 
> > Is there a concrete reason that all the proposed future cases like sockets
> > have to be handled within the very same XDP_REDIRECT return code? F.e. why
> > not XDP_TX_NIC that only assumes ifindex as proposed in the patch, and future
> > ones would get a different return code f.e. XDP_TX_SK only handling sockets
> > when we get there implementation-wise?  
> 
> yeah. let's keep redirect to sockets, tunnels, crypto and exotic things
> out of this discussion.
> XDP_REDIRECT should assume L2 raw packet is being redirected to another L2 netdev.
> If we make it too generic it will lose performance.
> 
> For cls_bpf the ifindex concept is symmetric. The program can access it as
> skb->ifindex on receive and can redirect to another ifindex via bpf_redirect() helper.
> Since netdev is not locked, it's actually big plus, since container management
> control plane can simply delete netns+veth and it goes away. The program can
> have dangling ifindex (if control plane is buggy and didn't update the bpf side),
> but it's harmless. Packets that redirect to non-existing ifindex are dropped.
> This approach already understood and works well, so for XDP I suggest to use
> the same approach initially before starting to reinvent the wheel.
> struct xdp_md needs ifindex field and xdp_redirect() helper that redirects
> to L2 netdev only. That's it. Simple and easy.
> I think the main use cases in John's and Jesper's minds is something like
> xdpswitch where packets are coming from VMs and from physical eths and
> being redirected to either physical eth or to VM via upcoming vhost+xdp support.
> This xdp_md->ifindex + xdp_redirect(to_ifindex) will solve it just fine.
> 
> Once we have vhost+xdp and all other bits implemented, we must come back
> to this discussion about having port mapping table. As I mentioned
> during netconf I think it's very useful, but I don't think we should
> gate vhost+xdp and xdp_redirect work on this discussion.
> As far as this port mapping table we would need 'port' field in xdp_md as well
> and xdp_redirect_port() done via helper or via extra 'out_port' field in xdp_md
> plus another XDP_REDIRECT_PORT action code.

Guess it would be easier to talk about if we name it "ingress_port" and
"egress_port".

> The actual port table (array) should be populated by user space with netdevs
> and these netdev will have their refcnt incremented. Then we'll have discussion
> what to do with netdev_unregister notifiers, whether they should be auto-removed
> from port table or bpf should have a chance to be notified and act on it.
> Such port mapping will allow us to optimize inevitable call
> dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
> away, since netdevs will be stored in the port table and direct deref
> port_map_array[xdp_md->out_port] will give us target netdev quickly.

I agree with above paragraph, and is happy that you can see that this
will actually be faster than using ifindex'es.

> It's nice optimization and there are other more powerful optimizations we
> can do with such port table (since we will know in advance which netdevs
> the program will be redirecting too), but I still think we should do
> ifindex based xdp_redirect first and only add this port table later.

No, we cannot first do an ifindex based xdp_redirect. The point of the
port table is to sandbox which ports XDP can use.

XDP is different than TC/cls_bpf, as it does "bypass", there is no
other layer that can stop or inspect these packets. The TC hooks
redirect into the network stack, which have all the usual facilities
available for filtering, inspection and debugging what is going on
(e.g. tcpdump works for TC redirect).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: __sk_buff.data_end
From: Johannes Berg @ 2017-04-20  6:07 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov; +Cc: Alexei Starovoitov, netdev
In-Reply-To: <58F802EB.9080807@iogearbox.net>

On Thu, 2017-04-20 at 02:38 +0200, Daniel Borkmann wrote:

> > Since wifi skbs have only eth in headlen, there is not much
> > pointing adding support for data/data_end to wifi.
> > Just use ld_abs/ld_ind instructions and load_bytes() helper.
> 
> Afaik, the ld_abs/ld_ind are not an option due to the data
> on the wire being in little endian, but the bpf_skb_load_bytes()
> might be the way to go initially, agree.

Oh yeah, we could really only use byte loads which would be even less
efficient, so that's a good argument for not even providing the data
load instructions... :)

johannes

^ permalink raw reply

* Re: __sk_buff.data_end
From: Johannes Berg @ 2017-04-20  6:06 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann; +Cc: Alexei Starovoitov, netdev
In-Reply-To: <20170420001218.GA38173@ast-mbp.thefacebook.com>

On Wed, 2017-04-19 at 17:12 -0700, Alexei Starovoitov wrote:
> 
> also didn't we discuss that wifi has crazy non-linear skb?

Not always, depends on the driver.

But also, you have to remember that this filter would be before the
conversion to Ethernet header. Right now, when packets enter the stack,
we always linearize the 802.11 header - or even the whole frame for the
(rare) management packets. We don't do this before monitor mode and
before this filter right now, but there's no super good reason we can't
(it was just so we still report there in case of allocation failures.)

> this data/data_end is used by cls_bpf with headlen only
this data/data_end is used by cls_bpf with headlen only
> for direct packet access where performance matters.

Well performance does matter, though perhaps not as much - the people
I'd think could be the first to use this were adding code way after
doing many function calls, so ... :)

> Since wifi skbs have only eth in headlen, there is not much
> pointing adding support for data/data_end to wifi.
> Just use ld_abs/ld_ind instructions and load_bytes() helper.

You guys just told me not to support the skb access instructions ;-)

But yeah I have the load_bytes helper working. Just thought it might be
nice to get more. But we can also leave that for later.

johannes

^ permalink raw reply

* Re: __sk_buff.data_end
From: Johannes Berg @ 2017-04-20  6:01 UTC (permalink / raw)
  To: Daniel Borkmann, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <58F7FA6D.5030000@iogearbox.net>

On Thu, 2017-04-20 at 02:01 +0200, Daniel Borkmann wrote:
> 
> Yeah, should work as well for the 32 bit archs, on 64 bit we
> have this effectively already:

Right.

[...]

> Can you elaborate on why this works for mac80211? It uses cb
> only up to that point from where you invoke the prog?

No, it works because then I can move a u64 field to the same offset,
and save/restore it across the BPF call :)

But I don't have a *pointer* field to move there, and no space for the
alignment anyway (already using all 48 bytes).

Come to think of it - somebody had proposed extensions to this by
passing an on-stack pointer in addition to the data in the cb.

Perhaps we can extend BPF to have an optional second argument, and
track a second context around the verifier, if applicable? Then we can
solve all of this really easily, because it means we don't always have
to go from the SKB context but could go from the other one (which could
be that on-stack buffer).

Alternatively I can clear another pointer (u64) in the CB, store a
pointer there, and always emit code following that pointer - should be
possible right?

johannes

^ permalink raw reply

* Re: [PATCH net-next 2/2] tcp: remove poll() flakes with FastOpen
From: Yuchung Cheng @ 2017-04-20  5:55 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet
In-Reply-To: <20170418164552.29261-3-edumazet@google.com>

On Tue, Apr 18, 2017 at 9:45 AM, Eric Dumazet <edumazet@google.com> wrote:
>
> When using TCP FastOpen for an active session, we send one wakeup event
> from tcp_finish_connect(), right before the data eventually contained in
> the received SYNACK is queued to sk->sk_receive_queue.
>
> This means that depending on machine load or luck, poll() users
> might receive POLLOUT events instead of POLLIN|POLLOUT
>
> To fix this, we need to move the call to sk->sk_state_change()
> after the (optional) call to tcp_rcv_fastopen_synack()
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>

Thanks for the fix!

> ---
>  net/ipv4/tcp_input.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 37e2aa925f62395cfb48145cd3a76b6afebb64b1..341f021f02a2931cd75b2e1e71af9729fc4c7895 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -5580,10 +5580,6 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
>         else
>                 tp->pred_flags = 0;
>
> -       if (!sock_flag(sk, SOCK_DEAD)) {
> -               sk->sk_state_change(sk);
> -               sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
> -       }
>  }
>
>  static bool tcp_rcv_fastopen_synack(struct sock *sk, struct sk_buff *synack,
> @@ -5652,6 +5648,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>         struct tcp_sock *tp = tcp_sk(sk);
>         struct tcp_fastopen_cookie foc = { .len = -1 };
>         int saved_clamp = tp->rx_opt.mss_clamp;
> +       bool fastopen_fail;
>
>         tcp_parse_options(skb, &tp->rx_opt, 0, &foc);
>         if (tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr)
> @@ -5755,10 +5752,15 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>
>                 tcp_finish_connect(sk, skb);
>
> -               if ((tp->syn_fastopen || tp->syn_data) &&
> -                   tcp_rcv_fastopen_synack(sk, skb, &foc))
> -                       return -1;
> +               fastopen_fail = (tp->syn_fastopen || tp->syn_data) &&
> +                               tcp_rcv_fastopen_synack(sk, skb, &foc);
>
> +               if (!sock_flag(sk, SOCK_DEAD)) {
> +                       sk->sk_state_change(sk);
> +                       sk_wake_async(sk, SOCK_WAKE_IO, POLL_OUT);
> +               }
> +               if (fastopen_fail)
> +                       return -1;
>                 if (sk->sk_write_pending ||
>                     icsk->icsk_accept_queue.rskq_defer_accept ||
>                     icsk->icsk_ack.pingpong) {
> --
> 2.12.2.762.g0e3151a226-goog
>

^ 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