Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3 net-next 1/5] net: dsa: lan9303: Change lan9303_xxx_packet_processing() port param.
From: Florian Fainelli @ 2017-08-03 18:08 UTC (permalink / raw)
  To: Egil Hjelmeland, andrew, vivien.didelot, netdev, linux-kernel,
	kernel
In-Reply-To: <20170803094507.3439-2-privat@egil-hjelmeland.no>

On 08/03/2017 02:45 AM, Egil Hjelmeland wrote:
> lan9303_enable_packet_processing, lan9303_disable_packet_processing()
> Pass port number (0,1,2) as parameter instead of port offset.
> Because other functions in the module pass port numbers.
> And to enable simplifications in following patch.
> 
> Introduce lan9303_write_switch_port().
> 
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
> ---
>  drivers/net/dsa/lan9303-core.c | 60 ++++++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
> index 8e430d1ee297..fa19e320c5a8 100644
> --- a/drivers/net/dsa/lan9303-core.c
> +++ b/drivers/net/dsa/lan9303-core.c
> @@ -159,9 +159,7 @@
>  # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT1 (BIT(9) | BIT(8))
>  # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT0 (BIT(1) | BIT(0))
>  
> -#define LAN9303_PORT_0_OFFSET 0x400
> -#define LAN9303_PORT_1_OFFSET 0x800
> -#define LAN9303_PORT_2_OFFSET 0xc00
> +#define LAN9303_SWITCH_PORT_REG(port, reg0) (0x400 * (port) + (reg0))
>  
>  /* the built-in PHYs are of type LAN911X */
>  #define MII_LAN911X_SPECIAL_MODES 0x12
> @@ -428,6 +426,13 @@ static int lan9303_read_switch_reg(struct lan9303 *chip, u16 regnum, u32 *val)
>  	return ret;
>  }
>  
> +static int lan9303_write_switch_port(
> +	struct lan9303 *chip, int port, u16 regnum, u32 val)
> +{
> +	return lan9303_write_switch_reg(
> +		chip, LAN9303_SWITCH_PORT_REG(port, regnum), val);
> +}

This argument alignment is not looking too good, can you do this instead:

static int lan9303_write_switch_port(struct lan9303 *chip, int port
				     u16 regnum, u32 *val)
{
}

This applied to patch 5 as well (which should have included it applies
to patch 1 as well).

With that:

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

> +
>  static int lan9303_detect_phy_setup(struct lan9303 *chip)
>  {
>  	int reg;
> @@ -458,24 +463,23 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
>  	return 0;
>  }
>  
> -#define LAN9303_MAC_RX_CFG_OFFS (LAN9303_MAC_RX_CFG_0 - LAN9303_PORT_0_OFFSET)
> -#define LAN9303_MAC_TX_CFG_OFFS (LAN9303_MAC_TX_CFG_0 - LAN9303_PORT_0_OFFSET)
> -
>  static int lan9303_disable_packet_processing(struct lan9303 *chip,
>  					     unsigned int port)
>  {
>  	int ret;
>  
>  	/* disable RX, but keep register reset default values else */
> -	ret = lan9303_write_switch_reg(chip, LAN9303_MAC_RX_CFG_OFFS + port,
> -				       LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES);
> +	ret = lan9303_write_switch_port(
> +			chip, port, LAN9303_MAC_RX_CFG_0,
> +			LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES);
>  	if (ret)
>  		return ret;
>  
>  	/* disable TX, but keep register reset default values else */
> -	return lan9303_write_switch_reg(chip, LAN9303_MAC_TX_CFG_OFFS + port,
> -				LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
> -				LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE);
> +	return lan9303_write_switch_port(
> +			chip, port, LAN9303_MAC_TX_CFG_0,
> +			LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
> +			LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE);

Same here, please don't re-align the arguments, they were fine already.

>  }
>  
>  static int lan9303_enable_packet_processing(struct lan9303 *chip,
> @@ -484,17 +488,19 @@ static int lan9303_enable_packet_processing(struct lan9303 *chip,
>  	int ret;
>  
>  	/* enable RX and keep register reset default values else */
> -	ret = lan9303_write_switch_reg(chip, LAN9303_MAC_RX_CFG_OFFS + port,
> -				       LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES |
> -				       LAN9303_MAC_RX_CFG_X_RX_ENABLE);
> +	ret = lan9303_write_switch_port(
> +			chip, port, LAN9303_MAC_RX_CFG_0,
> +			LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES |
> +			LAN9303_MAC_RX_CFG_X_RX_ENABLE);
>  	if (ret)
>  		return ret;
>  
>  	/* enable TX and keep register reset default values else */
> -	return lan9303_write_switch_reg(chip, LAN9303_MAC_TX_CFG_OFFS + port,
> -				LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
> -				LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE |
> -				LAN9303_MAC_TX_CFG_X_TX_ENABLE);
> +	return lan9303_write_switch_port(
> +			chip, port, LAN9303_MAC_TX_CFG_0,
> +			LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
> +			LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE |
> +			LAN9303_MAC_TX_CFG_X_TX_ENABLE);
>  }
>  
>  /* We want a special working switch:
> @@ -558,13 +564,13 @@ static int lan9303_disable_processing(struct lan9303 *chip)
>  {
>  	int ret;
>  
> -	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
> +	ret = lan9303_disable_packet_processing(chip, 0);
>  	if (ret)
>  		return ret;
> -	ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
> +	ret = lan9303_disable_packet_processing(chip, 1);
>  	if (ret)
>  		return ret;
> -	return lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
> +	return lan9303_disable_packet_processing(chip, 2);
>  }
>  
>  static int lan9303_check_device(struct lan9303 *chip)
> @@ -634,7 +640,7 @@ static int lan9303_setup(struct dsa_switch *ds)
>  	if (ret)
>  		dev_err(chip->dev, "failed to separate ports %d\n", ret);
>  
> -	ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
> +	ret = lan9303_enable_packet_processing(chip, 0);
>  	if (ret)
>  		dev_err(chip->dev, "failed to re-enable switching %d\n", ret);
>  
> @@ -757,11 +763,9 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
>  	/* enable internal packet processing */
>  	switch (port) {
>  	case 1:
> -		return lan9303_enable_packet_processing(chip,
> -							LAN9303_PORT_1_OFFSET);
> +		return lan9303_enable_packet_processing(chip, port);
>  	case 2:
> -		return lan9303_enable_packet_processing(chip,
> -							LAN9303_PORT_2_OFFSET);
> +		return lan9303_enable_packet_processing(chip, port);
>  	default:
>  		dev_dbg(chip->dev,
>  			"Error: request to power up invalid port %d\n", port);
> @@ -778,12 +782,12 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
>  	/* disable internal packet processing */
>  	switch (port) {
>  	case 1:
> -		lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
> +		lan9303_disable_packet_processing(chip, port);
>  		lan9303_phy_write(ds, chip->phy_addr_sel_strap + 1,
>  				  MII_BMCR, BMCR_PDOWN);
>  		break;
>  	case 2:
> -		lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
> +		lan9303_disable_packet_processing(chip, port);
>  		lan9303_phy_write(ds, chip->phy_addr_sel_strap + 2,
>  				  MII_BMCR, BMCR_PDOWN);
>  		break;
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
From: Andrew Lunn @ 2017-08-03 18:11 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: David Miller, netdev, vivien.didelot, linux-kernel
In-Reply-To: <e16c44e3-41ee-09cc-a05d-46e22c1404e3@gmail.com>

On Thu, Aug 03, 2017 at 10:30:56AM -0700, Florian Fainelli wrote:
> On 08/02/2017 04:49 PM, David Miller wrote:
> > From: Florian Fainelli <f.fainelli@gmail.com>
> > Date: Tue,  1 Aug 2017 15:00:36 -0700
> > 
> >> DSA slave network devices maintain a pair of bytes and packets counters
> >> for each directions, but these are not 64-bit capable. Re-use
> >> pcpu_sw_netstats which contains exactly what we need for that purpose
> >> and update the code path to report 64-bit capable statistics.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > Applied, thanks.
> > 
> > I would run ethtool -S and ifconfig under perf to see where it is
> > spending so much time.
> > 
> 
> This appears to be way worse than I thought, will keep digging, but for
> now, I may have to send a revert. Andrew, Vivien can you see if you have
> the same problems on your boards? Thanks!
> 
> # killall iperf
> # [ ID] Interval       Transfer     Bandwidth
> [  3]  0.0-19.1 sec   500 MBytes   220 Mbits/sec
> # while true; do ethtool -S gphy; ifconfig gphy; done
> ^C^C
> 
> 
> [   64.566226] INFO: rcu_sched self-detected stall on CPU
> [   64.571487]  0-...: (25999 ticks this GP) idle=006/140000000000001/0

Hi Florian

I don't get anything so bad, but i think that is because of hardware
restrictions. I see the ethtool; ifconfig loop goes a lot slower when
there is iperf traffic, but i don't get an RCU stall. However, the
board i tested on only has a 100Mbps CPU interface, and it can handle
all that traffic without pushing the CPU to 100%. What is the CPU load
when you run your test? Even if you are going to 100% CPU load, we
still don't want RCU stalls.

      Andrew

^ permalink raw reply

* Re: [PATCH RFC 00/13] phylink and sfp support
From: Florian Fainelli @ 2017-08-03 18:11 UTC (permalink / raw)
  To: Andrew Lunn, Russell King - ARM Linux, David Miller; +Cc: netdev
In-Reply-To: <20170801143933.GI23157@lunn.ch>

On 08/01/2017 07:39 AM, Andrew Lunn wrote:
> On Tue, Jul 25, 2017 at 03:01:39PM +0100, Russell King - ARM Linux wrote:
>> Hi,
>>
>> This patch series introduces generic support for SFP sockets found on
>> various Marvell based platforms.  The idea here is to provide common
>> SFP socket support which can be re-used by network drivers as
>> appropriate, rather than each network driver having to re-implement
>> SFP socket support.
> 
> There is a lot of code here, and i'm not really going to understand it
> until i use it. I have a couple of boards with SFFs connected to
> switches, so i will spend some time over the next month or so to make
> DSA use phylink. As is usual, we can sort out any issues as we go
> along.
> 
> David, if it still applies cleanly, can you add it to net-next?

Agreed, this is in a good shape and the more people we can have testing
this, the better. It may be nice to include a consumer of the PHYLINK
API for other people to start copying from.
-- 
Florian

^ permalink raw reply

* Re: [PATCH 00/27] ip: add -json support to 'ip link show'
From: Julien Fortin @ 2017-08-03 19:05 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: netdev, Roopa Prabhu, Nikolay Aleksandrov, David Ahern
In-Reply-To: <83479680-2b0a-a668-9c19-c7d9a10291fe@hartkopp.net>

On Thu, Aug 3, 2017 at 7:08 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hi Julien,
>
> On 08/03/2017 05:54 PM, Julien Fortin wrote:
>>
>> From: Julien Fortin <julien@cumulusnetworks.com>
>
>
> what about
>
> link_veth.c
> iplink_vcan.c
> iplink_vxcan.c
>
> ??

Hello Oliver,

None of these files print any link info data, see struct link_util (.print_opt)

Regards,
Julien.

>
> Regards,
> Oliver
>
>
>>
>> This patch series adds json support to 'ip [-details] link show [dev DEV]'
>> Each patch describes the json schema it adds and provides some examples.
>>
>> Julien Fortin (27):
>>    color: add new COLOR_NONE and disable_color function
>>    ip: add new command line argument -json (mutually exclusive with
>>      -color)
>>    json_writer: add new json handlers (null, float with format, lluint,
>>      hu)
>>    ip: ip_print: add new API to print JSON or regular format output
>>    ip: ipaddress.c: add support for json output
>>    ip: iplink.c: open/close json object for ip -brief -json link show dev
>>      DEV
>>    ip: iplink_bond.c: add json output support
>>    ip: iplink_bond_slave.c: add json output support (info_slave_data)
>>    ip: iplink_hsr.c: add json output support
>>    ip: iplink_bridge.c: add json output support
>>    ip: iplink_bridge_slave.c: add json output support
>>    ip: iplink_can.c: add json output support
>>    ip: iplink_geneve.c: add json output support
>>    ip: iplink_ipoib.c: add json output support
>>    ip: iplink_ipvlan.c: add json output support
>>    ip: iplink_vrf.c: add json output support
>>    ip: iplink_vxlan.c: add json output support
>>    ip: iplink_xdp.c: add json output support
>>    ip: ipmacsec.c: add json output support
>>    ip: link_gre.c: add json output support
>>    ip: link_gre6.c: add json output support
>>    ip: link_ip6tnl.c: add json output support
>>    ip: link_iptnl.c: add json output support
>>    ip: link_vti.c: add json output support
>>    ip: link_vti6.c: add json output support
>>    ip: link_macvlan.c: add json output support
>>    ip: iplink_vlan.c: add json output support
>>
>>   include/color.h          |    2 +
>>   include/json_writer.h    |    9 +
>>   include/utils.h          |    1 +
>>   ip/Makefile              |    2 +-
>>   ip/ip.c                  |    6 +
>>   ip/ip_common.h           |   56 +++
>>   ip/ip_print.c            |  233 ++++++++++
>>   ip/ipaddress.c           | 1089
>> ++++++++++++++++++++++++++++++++--------------
>>   ip/iplink.c              |    2 +
>>   ip/iplink_bond.c         |  231 +++++++---
>>   ip/iplink_bond_slave.c   |   57 ++-
>>   ip/iplink_bridge.c       |  291 ++++++++-----
>>   ip/iplink_bridge_slave.c |  185 +++++---
>>   ip/iplink_can.c          |  276 +++++++++---
>>   ip/iplink_geneve.c       |   86 +++-
>>   ip/iplink_hsr.c          |   36 +-
>>   ip/iplink_ipoib.c        |   30 +-
>>   ip/iplink_ipvlan.c       |    8 +-
>>   ip/iplink_macvlan.c      |   37 +-
>>   ip/iplink_vlan.c         |   62 ++-
>>   ip/iplink_vrf.c          |   13 +-
>>   ip/iplink_vxlan.c        |  161 ++++---
>>   ip/iplink_xdp.c          |   31 +-
>>   ip/ipmacsec.c            |   84 +++-
>>   ip/link_gre.c            |  147 ++++---
>>   ip/link_gre6.c           |  142 ++++--
>>   ip/link_ip6tnl.c         |  172 +++++---
>>   ip/link_iptnl.c          |  155 ++++---
>>   ip/link_vti.c            |   23 +-
>>   ip/link_vti6.c           |   22 +-
>>   lib/color.c              |    9 +-
>>   lib/json_writer.c        |   44 +-
>>   32 files changed, 2668 insertions(+), 1034 deletions(-)
>>   create mode 100644 ip/ip_print.c
>>
>

^ permalink raw reply

* [PATCH v3 net-next 0/4] ulp: Generalize ULP infrastructure
From: Tom Herbert @ 2017-08-03 19:31 UTC (permalink / raw)
  To: netdev; +Cc: rohit, davejwatson, Tom Herbert

Generalize the ULP infrastructure that was recently introduced to
support kTLS. This adds a SO_ULP socket option and creates new fields in
sock structure for ULP ops and ULP data. Also, the interface allows
additional per ULP parameters to be set so that a ULP can be pushed
and operations started in one shot.

This patch sets:
  - Minor dependency fix in inet_common.h
  - Implement ULP infrastructure as a socket mechanism
  - Fixes TCP and TLS to use the new method (maintaining backwards
    API compatibility)
  - Adds a ulp.txt document

Tested: Ran simple ULP. Dave Watson verified kTLS works.

-v2: Fix compliation errors when CONFIG_ULP_SOCK not set.
-v3: FIx one more build issue, check that sk_protocol is IPPROTO_TCP
     in tsl_init

Tom Herbert (4):
  inet: include net/sock.h in inet_common.h
  sock: ULP infrastructure
  tcp: Adjust TCP ULP to defer to sockets ULP
  ulp: Documention for ULP infrastructure

 Documentation/networking/tls.txt       |   6 +-
 Documentation/networking/ulp.txt       |  82 ++++++++++++++
 arch/alpha/include/uapi/asm/socket.h   |   2 +
 arch/frv/include/uapi/asm/socket.h     |   2 +
 arch/ia64/include/uapi/asm/socket.h    |   2 +
 arch/m32r/include/uapi/asm/socket.h    |   2 +
 arch/mips/include/uapi/asm/socket.h    |   2 +
 arch/mn10300/include/uapi/asm/socket.h |   2 +
 arch/parisc/include/uapi/asm/socket.h  |   2 +
 arch/s390/include/uapi/asm/socket.h    |   2 +
 arch/sparc/include/uapi/asm/socket.h   |   2 +
 arch/xtensa/include/uapi/asm/socket.h  |   2 +
 include/linux/socket.h                 |   9 ++
 include/net/inet_common.h              |   2 +
 include/net/inet_connection_sock.h     |   4 -
 include/net/sock.h                     |   5 +
 include/net/tcp.h                      |  25 -----
 include/net/tls.h                      |   4 +-
 include/net/ulp_sock.h                 |  75 +++++++++++++
 include/uapi/asm-generic/socket.h      |   2 +
 net/Kconfig                            |   4 +
 net/core/Makefile                      |   1 +
 net/core/sock.c                        |  14 +++
 net/core/sysctl_net_core.c             |  25 +++++
 net/core/ulp_sock.c                    | 194 +++++++++++++++++++++++++++++++++
 net/ipv4/Makefile                      |   2 +-
 net/ipv4/sysctl_net_ipv4.c             |   9 +-
 net/ipv4/tcp.c                         |  40 ++++---
 net/ipv4/tcp_ipv4.c                    |   2 -
 net/ipv4/tcp_ulp.c                     | 135 -----------------------
 net/tls/Kconfig                        |   1 +
 net/tls/tls_main.c                     |  24 ++--
 32 files changed, 484 insertions(+), 201 deletions(-)
 create mode 100644 Documentation/networking/ulp.txt
 create mode 100644 include/net/ulp_sock.h
 create mode 100644 net/core/ulp_sock.c
 delete mode 100644 net/ipv4/tcp_ulp.c

-- 
2.11.0

^ permalink raw reply

* [PATCH v3 net-next 1/4] inet: include net/sock.h in inet_common.h
From: Tom Herbert @ 2017-08-03 19:31 UTC (permalink / raw)
  To: netdev; +Cc: rohit, davejwatson, Tom Herbert
In-Reply-To: <20170803193144.24108-1-tom@quantonium.net>

inet_common.h has a dependency on sock.h so it should include that.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 include/net/inet_common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index f39ae697347f..df0119a317aa 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -1,6 +1,8 @@
 #ifndef _INET_COMMON_H
 #define _INET_COMMON_H
 
+#include <net/sock.h>
+
 extern const struct proto_ops inet_stream_ops;
 extern const struct proto_ops inet_dgram_ops;
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 net-next 2/4] sock: ULP infrastructure
From: Tom Herbert @ 2017-08-03 19:31 UTC (permalink / raw)
  To: netdev; +Cc: rohit, davejwatson, Tom Herbert
In-Reply-To: <20170803193144.24108-1-tom@quantonium.net>

Generalize the TCP ULP infrastructure recently introduced to support
kTLS. This adds a SO_ULP socket option and creates new fields in
sock structure for ULP ops and ULP data. Also, the interface allows
additional per ULP parameters to be set so that a ULP can be pushed
and operations started in one shot.

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 arch/alpha/include/uapi/asm/socket.h   |   2 +
 arch/frv/include/uapi/asm/socket.h     |   2 +
 arch/ia64/include/uapi/asm/socket.h    |   2 +
 arch/m32r/include/uapi/asm/socket.h    |   2 +
 arch/mips/include/uapi/asm/socket.h    |   2 +
 arch/mn10300/include/uapi/asm/socket.h |   2 +
 arch/parisc/include/uapi/asm/socket.h  |   2 +
 arch/s390/include/uapi/asm/socket.h    |   2 +
 arch/sparc/include/uapi/asm/socket.h   |   2 +
 arch/xtensa/include/uapi/asm/socket.h  |   2 +
 include/linux/socket.h                 |   9 ++
 include/net/sock.h                     |   5 +
 include/net/ulp_sock.h                 |  75 +++++++++++++
 include/uapi/asm-generic/socket.h      |   2 +
 net/Kconfig                            |   4 +
 net/core/Makefile                      |   1 +
 net/core/sock.c                        |  14 +++
 net/core/sysctl_net_core.c             |  25 +++++
 net/core/ulp_sock.c                    | 194 +++++++++++++++++++++++++++++++++
 19 files changed, 349 insertions(+)
 create mode 100644 include/net/ulp_sock.h
 create mode 100644 net/core/ulp_sock.c

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 7b285dd4fe05..885e8fca79b0 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -109,4 +109,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index f1e3b20dce9f..8ba71f2a3bf3 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -102,5 +102,7 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 5dd5c5d0d642..2de1c53f88b5 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -111,4 +111,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index f8f7b47e247f..b2d394381787 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -102,4 +102,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 882823bec153..d0bdf8c78220 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -120,4 +120,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index c710db354ff2..686fbf497a13 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -102,4 +102,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index a0d4dc9f4eb2..d6e99deca976 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -101,4 +101,6 @@
 
 #define SO_PEERGROUPS		0x4034
 
+#define SO_ULP			0x4035
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index 52a63f4175cb..6b52f162369a 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -108,4 +108,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 186fd8199f54..e765bf781107 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -98,6 +98,8 @@
 
 #define SO_PEERGROUPS		0x003d
 
+#define SO_ULP			0x003e
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index 3eed2761c149..8eaa2e9e27b6 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -113,4 +113,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8b13db5163cc..8f6e8c46a91e 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -156,6 +156,15 @@ struct ucred {
 	__u32	gid;
 };
 
+/* ULP configuration for socket */
+
+#define ULP_NAME_MAX	16
+
+struct ulp_config {
+	char ulp_name[ULP_NAME_MAX];
+	__u8 ulp_params[0];
+};
+
 /* Supported address families. */
 #define AF_UNSPEC	0
 #define AF_UNIX		1	/* Unix domain sockets 		*/
diff --git a/include/net/sock.h b/include/net/sock.h
index 393c38e9f6aa..a1e5586aa767 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -72,6 +72,7 @@
 #include <net/tcp_states.h>
 #include <linux/net_tstamp.h>
 #include <net/smc.h>
+#include <net/ulp_sock.h>
 
 /*
  * This structure really needs to be cleaned up.
@@ -312,6 +313,8 @@ struct sock_common {
   *	@sk_destruct: called at sock freeing time, i.e. when all refcnt == 0
   *	@sk_reuseport_cb: reuseport group container
   *	@sk_rcu: used during RCU grace period
+  *	@sk_ulp_ops: pluggable ULP control hook
+  *	@sk_ulp_data: ULP private data
   */
 struct sock {
 	/*
@@ -415,6 +418,8 @@ struct sock {
 	unsigned int		sk_gso_max_size;
 	gfp_t			sk_allocation;
 	__u32			sk_txhash;
+	const struct ulp_ops	*sk_ulp_ops;
+	void			*sk_ulp_data;
 
 	/*
 	 * Because of non atomicity rules, all
diff --git a/include/net/ulp_sock.h b/include/net/ulp_sock.h
new file mode 100644
index 000000000000..37bf4d2e16b9
--- /dev/null
+++ b/include/net/ulp_sock.h
@@ -0,0 +1,75 @@
+/*
+ * Pluggable upper layer protocol support in sockets.
+ *
+ * Copyright (c) 2016-2017, Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016-2017, Dave Watson <davejwatson@fb.com>. All rights reserved.
+ * Copyright (c) 2017, Tom Herbert <tom@quantonium.net>. All rights reserved.
+ *
+ */
+
+#ifndef __NET_ULP_SOCK_H
+#define __NET_ULP_SOCK_H
+
+#include <linux/socket.h>
+
+#define ULP_MAX             128
+#define ULP_BUF_MAX         (ULP_NAME_MAX * ULP_MAX)
+
+struct ulp_ops {
+	struct list_head list;
+
+	/* initialize ulp */
+	int (*init)(struct sock *sk, char __user *optval, int len);
+
+	/* cleanup ulp */
+	void (*release)(struct sock *sk);
+
+	/* Get ULP specific parameters in getsockopt */
+	int (*get_params)(struct sock *sk, char __user *optval, int *optlen);
+
+	char name[ULP_NAME_MAX];
+	struct module *owner;
+};
+
+#ifdef CONFIG_ULP_SOCK
+
+int ulp_register(struct ulp_ops *type);
+void ulp_unregister(struct ulp_ops *type);
+int ulp_set(struct sock *sk, char __user *optval, int len);
+int ulp_get_config(struct sock *sk, char __user *optval, int *optlen);
+void ulp_get_available(char *buf, size_t len);
+void ulp_cleanup(struct sock *sk);
+
+#else
+
+static inline int ulp_register(struct ulp_ops *type)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void ulp_unregister(struct ulp_ops *type)
+{
+}
+
+static inline int ulp_set(struct sock *sk, char __user *optval, int len)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int ulp_get_config(struct sock *sk, char __user *optval,
+				 int *optlen)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline void ulp_get_available(char *buf, size_t len)
+{
+}
+
+static inline void ulp_cleanup(struct sock *sk)
+{
+}
+
+#endif
+
+#endif /* __NET_ULP_SOCK_H */
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 9861be8da65e..bf15ae98f3b2 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ULP			60
+
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/Kconfig b/net/Kconfig
index 7d57ef34b79c..2b8d2d88bc2b 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -419,6 +419,10 @@ config GRO_CELLS
 	bool
 	default n
 
+config ULP_SOCK
+	bool
+	default n
+
 config NET_DEVLINK
 	tristate "Network physical/parent device Netlink interface"
 	help
diff --git a/net/core/Makefile b/net/core/Makefile
index d501c4278015..fcfd29ef9926 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_DST_CACHE) += dst_cache.o
 obj-$(CONFIG_HWBM) += hwbm.o
 obj-$(CONFIG_NET_DEVLINK) += devlink.o
 obj-$(CONFIG_GRO_CELLS) += gro_cells.o
+obj-$(CONFIG_ULP_SOCK) += ulp_sock.o
diff --git a/net/core/sock.c b/net/core/sock.c
index 742f68c9c84a..cb03af11b536 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1055,6 +1055,11 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		if (val == 1)
 			dst_negative_advice(sk);
 		break;
+
+	case SO_ULP:
+		ret = ulp_set(sk, optval, optlen);
+		break;
+
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -1383,6 +1388,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val64 = sock_gen_cookie(sk);
 		break;
 
+	case SO_ULP:
+		return ulp_get_config(sk, optval, optlen);
+
 	default:
 		/* We implement the SO_SNDLOWAT etc to not be settable
 		 * (1003.1g 7).
@@ -2943,6 +2951,12 @@ EXPORT_SYMBOL(compat_sock_common_setsockopt);
 
 void sk_common_release(struct sock *sk)
 {
+	/* Clean up ULP before destroying protocol socket since ULP might
+	 * be dependent on transport (and not the other way around).
+	 */
+
+	ulp_cleanup(sk);
+
 	if (sk->sk_prot->destroy)
 		sk->sk_prot->destroy(sk);
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index b7cd9aafe99e..9e14f91b57eb 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -21,6 +21,7 @@
 #include <net/net_ratelimit.h>
 #include <net/busy_poll.h>
 #include <net/pkt_sched.h>
+#include <net/ulp_sock.h>
 
 static int zero = 0;
 static int one = 1;
@@ -249,6 +250,24 @@ static int proc_do_rss_key(struct ctl_table *table, int write,
 	return proc_dostring(&fake_table, write, buffer, lenp, ppos);
 }
 
+static int proc_ulp_available(struct ctl_table *ctl,
+			      int write,
+			      void __user *buffer, size_t *lenp,
+			      loff_t *ppos)
+{
+	struct ctl_table tbl = { .maxlen = ULP_BUF_MAX, };
+	int ret;
+
+	tbl.data = kmalloc(tbl.maxlen, GFP_USER);
+	if (!tbl.data)
+		return -ENOMEM;
+	ulp_get_available(tbl.data, ULP_BUF_MAX);
+	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
+	kfree(tbl.data);
+
+	return ret;
+}
+
 static struct ctl_table net_core_table[] = {
 #ifdef CONFIG_NET
 	{
@@ -460,6 +479,12 @@ static struct ctl_table net_core_table[] = {
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= &zero,
 	},
+	{
+		.procname	= "ulp_available",
+		.maxlen		= ULP_BUF_MAX,
+		.mode		= 0444,
+		.proc_handler	= proc_ulp_available,
+	},
 	{ }
 };
 
diff --git a/net/core/ulp_sock.c b/net/core/ulp_sock.c
new file mode 100644
index 000000000000..0f49489b7eb6
--- /dev/null
+++ b/net/core/ulp_sock.c
@@ -0,0 +1,194 @@
+/*
+ * Pluggable upper layer protocol support in sockets.
+ *
+ * Copyright (c) 2016-2017, Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016-2017, Dave Watson <davejwatson@fb.com>. All rights reserved.
+ * Copyright (c) 2017, Tom Herbert <tom@quantonium.net>. All rights reserved.
+ *
+ */
+
+#include <linux/gfp.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <net/sock.h>
+#include <net/ulp_sock.h>
+
+static DEFINE_SPINLOCK(ulp_list_lock);
+static LIST_HEAD(ulp_list);
+
+/* Simple linear search, don't expect many entries! */
+static struct ulp_ops *ulp_find(const char *name)
+{
+	struct ulp_ops *e;
+
+	list_for_each_entry_rcu(e, &ulp_list, list) {
+		if (strcmp(e->name, name) == 0)
+			return e;
+	}
+
+	return NULL;
+}
+
+static const struct ulp_ops *__ulp_find_autoload(const char *name)
+{
+	const struct ulp_ops *ulp = NULL;
+
+	rcu_read_lock();
+	ulp = ulp_find(name);
+
+#ifdef CONFIG_MODULES
+	if (!ulp && capable(CAP_NET_ADMIN)) {
+		rcu_read_unlock();
+		request_module("%s", name);
+		rcu_read_lock();
+		ulp = ulp_find(name);
+	}
+#endif
+	if (!ulp || !try_module_get(ulp->owner))
+		ulp = NULL;
+
+	rcu_read_unlock();
+	return ulp;
+}
+
+/* Attach new upper layer protocol to the list
+ * of available protocols.
+ */
+int ulp_register(struct ulp_ops *ulp)
+{
+	int ret = 0;
+
+	spin_lock(&ulp_list_lock);
+	if (ulp_find(ulp->name)) {
+		pr_notice("%s already registered or non-unique name\n",
+			  ulp->name);
+		ret = -EEXIST;
+	} else {
+		list_add_tail_rcu(&ulp->list, &ulp_list);
+	}
+	spin_unlock(&ulp_list_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ulp_register);
+
+void ulp_unregister(struct ulp_ops *ulp)
+{
+	spin_lock(&ulp_list_lock);
+	list_del_rcu(&ulp->list);
+	spin_unlock(&ulp_list_lock);
+
+	synchronize_rcu();
+}
+EXPORT_SYMBOL_GPL(ulp_unregister);
+
+/* Build string with list of available upper layer protocl values */
+void ulp_get_available(char *buf, size_t maxlen)
+{
+	struct ulp_ops *ulp_ops;
+	size_t offs = 0;
+
+	*buf = '\0';
+	rcu_read_lock();
+	list_for_each_entry_rcu(ulp_ops, &ulp_list, list) {
+		offs += snprintf(buf + offs, maxlen - offs,
+				 "%s%s",
+				 offs == 0 ? "" : " ", ulp_ops->name);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(ulp_get_available);
+
+void ulp_cleanup(struct sock *sk)
+{
+	if (!sk->sk_ulp_ops)
+		return;
+
+	if (sk->sk_ulp_ops->release)
+		sk->sk_ulp_ops->release(sk);
+
+	module_put(sk->sk_ulp_ops->owner);
+}
+EXPORT_SYMBOL_GPL(ulp_cleanup);
+
+/* Change upper layer protocol for socket, Called from setsockopt. */
+int ulp_set(struct sock *sk, char __user *optval, int len)
+{
+	const struct ulp_ops *ulp_ops;
+	struct ulp_config ulpc;
+	int err = 0;
+
+	if (len < sizeof(ulpc))
+		return -EINVAL;
+
+	if (copy_from_user(&ulpc, optval, sizeof(ulpc)))
+		return -EFAULT;
+
+	if (sk->sk_ulp_ops)
+		return -EEXIST;
+
+	ulp_ops = __ulp_find_autoload(ulpc.ulp_name);
+	if (!ulp_ops)
+		return -ENOENT;
+
+	optval += sizeof(ulpc);
+	len -= sizeof(ulpc);
+
+	err = ulp_ops->init(sk, optval, len);
+	if (err)
+		return err;
+
+	sk->sk_ulp_ops = ulp_ops;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ulp_set);
+
+/* Get upper layer protocol for socket. Called from getsockopt. */
+int ulp_get_config(struct sock *sk, char __user *optval, int *optlen)
+{
+	struct ulp_config ulpc;
+	int len = *optlen;
+	int used_len;
+	int ret;
+
+	if (get_user(len, optlen))
+		return -EFAULT;
+
+	if (len < sizeof(ulpc))
+		return -EINVAL;
+
+	if (!sk->sk_ulp_ops) {
+		if (put_user(0, optlen))
+			return -EFAULT;
+		return 0;
+	}
+
+	memcpy(ulpc.ulp_name, sk->sk_ulp_ops->name,
+	       sizeof(ulpc.ulp_name));
+
+	if (copy_to_user(optval, &ulpc, sizeof(ulpc)))
+		return -EFAULT;
+
+	used_len = sizeof(ulpc);
+
+	if (sk->sk_ulp_ops->get_params) {
+		len -= sizeof(ulpc);
+		optval += sizeof(ulpc);
+
+		ret = sk->sk_ulp_ops->get_params(sk, optval, &len);
+		if (ret)
+			return ret;
+
+		used_len += len;
+	}
+
+	if (put_user(used_len, optlen))
+		return -EFAULT;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ulp_get_config);
+
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 net-next 3/4] tcp: Adjust TCP ULP to defer to sockets ULP
From: Tom Herbert @ 2017-08-03 19:31 UTC (permalink / raw)
  To: netdev; +Cc: rohit, davejwatson, Tom Herbert
In-Reply-To: <20170803193144.24108-1-tom@quantonium.net>

Fix TCP and TLS to use the newer ULP infrastructure in sockets.

Tested-by: Dave Watson <davejwatson@fb.com>
Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 Documentation/networking/tls.txt   |   6 +-
 include/net/inet_connection_sock.h |   4 --
 include/net/tcp.h                  |  25 -------
 include/net/tls.h                  |   4 +-
 net/ipv4/Makefile                  |   2 +-
 net/ipv4/sysctl_net_ipv4.c         |   9 ++-
 net/ipv4/tcp.c                     |  40 ++++++-----
 net/ipv4/tcp_ipv4.c                |   2 -
 net/ipv4/tcp_ulp.c                 | 135 -------------------------------------
 net/tls/Kconfig                    |   1 +
 net/tls/tls_main.c                 |  24 ++++---
 11 files changed, 51 insertions(+), 201 deletions(-)
 delete mode 100644 net/ipv4/tcp_ulp.c

diff --git a/Documentation/networking/tls.txt b/Documentation/networking/tls.txt
index 77ed00631c12..b70309df4709 100644
--- a/Documentation/networking/tls.txt
+++ b/Documentation/networking/tls.txt
@@ -12,8 +12,12 @@ Creating a TLS connection
 
 First create a new TCP socket and set the TLS ULP.
 
+    struct ulp_config ulpc = {
+	.ulp_name = "tls",
+    };
+
   sock = socket(AF_INET, SOCK_STREAM, 0);
-  setsockopt(sock, SOL_TCP, TCP_ULP, "tls", sizeof("tls"));
+  setsockopt(sock, SOL_SOCKET, SO_ULP, &ulpc, sizeof(ulpc))
 
 Setting the TLS ULP allows us to set/get TLS socket options. Currently
 only the symmetric encryption is handled in the kernel.  After the TLS
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 13e4c89a8231..c7a577976bec 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -75,8 +75,6 @@ struct inet_connection_sock_af_ops {
  * @icsk_pmtu_cookie	   Last pmtu seen by socket
  * @icsk_ca_ops		   Pluggable congestion control hook
  * @icsk_af_ops		   Operations which are AF_INET{4,6} specific
- * @icsk_ulp_ops	   Pluggable ULP control hook
- * @icsk_ulp_data	   ULP private data
  * @icsk_ca_state:	   Congestion control state
  * @icsk_retransmits:	   Number of unrecovered [RTO] timeouts
  * @icsk_pending:	   Scheduled timer event
@@ -99,8 +97,6 @@ struct inet_connection_sock {
 	__u32			  icsk_pmtu_cookie;
 	const struct tcp_congestion_ops *icsk_ca_ops;
 	const struct inet_connection_sock_af_ops *icsk_af_ops;
-	const struct tcp_ulp_ops  *icsk_ulp_ops;
-	void			  *icsk_ulp_data;
 	unsigned int		  (*icsk_sync_mss)(struct sock *sk, u32 pmtu);
 	__u8			  icsk_ca_state:6,
 				  icsk_ca_setsockopt:1,
diff --git a/include/net/tcp.h b/include/net/tcp.h
index bb1881b4ce48..65c462da3740 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1968,31 +1968,6 @@ static inline void tcp_listendrop(const struct sock *sk)
 
 enum hrtimer_restart tcp_pace_kick(struct hrtimer *timer);
 
-/*
- * Interface for adding Upper Level Protocols over TCP
- */
-
-#define TCP_ULP_NAME_MAX	16
-#define TCP_ULP_MAX		128
-#define TCP_ULP_BUF_MAX		(TCP_ULP_NAME_MAX*TCP_ULP_MAX)
-
-struct tcp_ulp_ops {
-	struct list_head	list;
-
-	/* initialize ulp */
-	int (*init)(struct sock *sk);
-	/* cleanup ulp */
-	void (*release)(struct sock *sk);
-
-	char		name[TCP_ULP_NAME_MAX];
-	struct module	*owner;
-};
-int tcp_register_ulp(struct tcp_ulp_ops *type);
-void tcp_unregister_ulp(struct tcp_ulp_ops *type);
-int tcp_set_ulp(struct sock *sk, const char *name);
-void tcp_get_available_ulp(char *buf, size_t len);
-void tcp_cleanup_ulp(struct sock *sk);
-
 /* Call BPF_SOCK_OPS program that returns an int. If the return value
  * is < 0, then the BPF op failed (for example if the loaded BPF
  * program does not support the chosen operation or there is no BPF
diff --git a/include/net/tls.h b/include/net/tls.h
index b89d397dd62f..7d88a6e2f5a7 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -214,9 +214,7 @@ static inline void tls_fill_prepend(struct tls_context *ctx,
 
 static inline struct tls_context *tls_get_ctx(const struct sock *sk)
 {
-	struct inet_connection_sock *icsk = inet_csk(sk);
-
-	return icsk->icsk_ulp_data;
+	return sk->sk_ulp_data;
 }
 
 static inline struct tls_sw_context *tls_sw_ctx(
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index afcb435adfbe..f83de23a30e7 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -8,7 +8,7 @@ obj-y     := route.o inetpeer.o protocol.o \
 	     inet_timewait_sock.o inet_connection_sock.o \
 	     tcp.o tcp_input.o tcp_output.o tcp_timer.o tcp_ipv4.o \
 	     tcp_minisocks.o tcp_cong.o tcp_metrics.o tcp_fastopen.o \
-	     tcp_rate.o tcp_recovery.o tcp_ulp.o \
+	     tcp_rate.o tcp_recovery.o \
 	     tcp_offload.o datagram.o raw.o udp.o udplite.o \
 	     udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
 	     fib_frontend.o fib_semantics.o fib_trie.o fib_notifier.o \
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 0d3c038d7b04..9ab0c278b7ba 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -21,6 +21,7 @@
 #include <net/route.h>
 #include <net/tcp.h>
 #include <net/udp.h>
+#include <net/ulp_sock.h>
 #include <net/cipso_ipv4.h>
 #include <net/inet_frag.h>
 #include <net/ping.h>
@@ -372,13 +373,15 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
 				  void __user *buffer, size_t *lenp,
 				  loff_t *ppos)
 {
-	struct ctl_table tbl = { .maxlen = TCP_ULP_BUF_MAX, };
+	struct ctl_table tbl = { .maxlen = ULP_BUF_MAX, };
 	int ret;
 
 	tbl.data = kmalloc(tbl.maxlen, GFP_USER);
 	if (!tbl.data)
 		return -ENOMEM;
-	tcp_get_available_ulp(tbl.data, TCP_ULP_BUF_MAX);
+
+	/* Just return all ULPs for compatibility */
+	ulp_get_available(tbl.data, ULP_BUF_MAX);
 	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
 	kfree(tbl.data);
 
@@ -709,7 +712,7 @@ static struct ctl_table ipv4_table[] = {
 	},
 	{
 		.procname	= "tcp_available_ulp",
-		.maxlen		= TCP_ULP_BUF_MAX,
+		.maxlen		= ULP_BUF_MAX,
 		.mode		= 0444,
 		.proc_handler   = proc_tcp_available_ulp,
 	},
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9dd6f4dba9b1..26f0d9821e6d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2404,24 +2404,24 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
 		release_sock(sk);
 		return err;
 	}
+
 	case TCP_ULP: {
-		char name[TCP_ULP_NAME_MAX];
+		struct ulp_config ulpc;
 
 		if (optlen < 1)
 			return -EINVAL;
 
-		val = strncpy_from_user(name, optval,
-					min_t(long, TCP_ULP_NAME_MAX - 1,
+		val = strncpy_from_user(ulpc.ulp_name, optval,
+					min_t(long, ULP_NAME_MAX - 1,
 					      optlen));
 		if (val < 0)
 			return -EFAULT;
-		name[val] = 0;
+		ulpc.ulp_name[val] = 0;
 
-		lock_sock(sk);
-		err = tcp_set_ulp(sk, name);
-		release_sock(sk);
-		return err;
+		return kernel_setsockopt(sk->sk_socket, SOL_SOCKET, SO_ULP,
+					 (char *)&ulpc, sizeof(ulpc));
 	}
+
 	default:
 		/* fallthru */
 		break;
@@ -2993,20 +2993,28 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 			return -EFAULT;
 		return 0;
 
-	case TCP_ULP:
+	case TCP_ULP: {
+		struct ulp_config ulpc;
+		int ulen, ret;
+
 		if (get_user(len, optlen))
 			return -EFAULT;
-		len = min_t(unsigned int, len, TCP_ULP_NAME_MAX);
-		if (!icsk->icsk_ulp_ops) {
-			if (put_user(0, optlen))
-				return -EFAULT;
-			return 0;
-		}
+		len = min_t(unsigned int, len, ULP_NAME_MAX);
+
+		ulen = sizeof(ulpc);
+
+		/* Backwards compatbility */
+		ret = kernel_getsockopt(sk->sk_socket, SOL_SOCKET, SO_ULP,
+					(char *)&ulpc, &ulen);
+		if (ret)
+			return ret;
+
 		if (put_user(len, optlen))
 			return -EFAULT;
-		if (copy_to_user(optval, icsk->icsk_ulp_ops->name, len))
+		if (copy_to_user(optval, ulpc.ulp_name, len))
 			return -EFAULT;
 		return 0;
+	}
 
 	case TCP_THIN_LINEAR_TIMEOUTS:
 		val = tp->thin_lto;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9b51663cd5a4..89fafd2c5d36 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1858,8 +1858,6 @@ void tcp_v4_destroy_sock(struct sock *sk)
 
 	tcp_cleanup_congestion_control(sk);
 
-	tcp_cleanup_ulp(sk);
-
 	/* Cleanup up the write buffer. */
 	tcp_write_queue_purge(sk);
 
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
deleted file mode 100644
index 2417f55374c5..000000000000
--- a/net/ipv4/tcp_ulp.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * Pluggable TCP upper layer protocol support.
- *
- * Copyright (c) 2016-2017, Mellanox Technologies. All rights reserved.
- * Copyright (c) 2016-2017, Dave Watson <davejwatson@fb.com>. All rights reserved.
- *
- */
-
-#include<linux/module.h>
-#include <linux/mm.h>
-#include <linux/types.h>
-#include <linux/list.h>
-#include <linux/gfp.h>
-#include <net/tcp.h>
-
-static DEFINE_SPINLOCK(tcp_ulp_list_lock);
-static LIST_HEAD(tcp_ulp_list);
-
-/* Simple linear search, don't expect many entries! */
-static struct tcp_ulp_ops *tcp_ulp_find(const char *name)
-{
-	struct tcp_ulp_ops *e;
-
-	list_for_each_entry_rcu(e, &tcp_ulp_list, list) {
-		if (strcmp(e->name, name) == 0)
-			return e;
-	}
-
-	return NULL;
-}
-
-static const struct tcp_ulp_ops *__tcp_ulp_find_autoload(const char *name)
-{
-	const struct tcp_ulp_ops *ulp = NULL;
-
-	rcu_read_lock();
-	ulp = tcp_ulp_find(name);
-
-#ifdef CONFIG_MODULES
-	if (!ulp && capable(CAP_NET_ADMIN)) {
-		rcu_read_unlock();
-		request_module("%s", name);
-		rcu_read_lock();
-		ulp = tcp_ulp_find(name);
-	}
-#endif
-	if (!ulp || !try_module_get(ulp->owner))
-		ulp = NULL;
-
-	rcu_read_unlock();
-	return ulp;
-}
-
-/* Attach new upper layer protocol to the list
- * of available protocols.
- */
-int tcp_register_ulp(struct tcp_ulp_ops *ulp)
-{
-	int ret = 0;
-
-	spin_lock(&tcp_ulp_list_lock);
-	if (tcp_ulp_find(ulp->name)) {
-		pr_notice("%s already registered or non-unique name\n",
-			  ulp->name);
-		ret = -EEXIST;
-	} else {
-		list_add_tail_rcu(&ulp->list, &tcp_ulp_list);
-	}
-	spin_unlock(&tcp_ulp_list_lock);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(tcp_register_ulp);
-
-void tcp_unregister_ulp(struct tcp_ulp_ops *ulp)
-{
-	spin_lock(&tcp_ulp_list_lock);
-	list_del_rcu(&ulp->list);
-	spin_unlock(&tcp_ulp_list_lock);
-
-	synchronize_rcu();
-}
-EXPORT_SYMBOL_GPL(tcp_unregister_ulp);
-
-/* Build string with list of available upper layer protocl values */
-void tcp_get_available_ulp(char *buf, size_t maxlen)
-{
-	struct tcp_ulp_ops *ulp_ops;
-	size_t offs = 0;
-
-	*buf = '\0';
-	rcu_read_lock();
-	list_for_each_entry_rcu(ulp_ops, &tcp_ulp_list, list) {
-		offs += snprintf(buf + offs, maxlen - offs,
-				 "%s%s",
-				 offs == 0 ? "" : " ", ulp_ops->name);
-	}
-	rcu_read_unlock();
-}
-
-void tcp_cleanup_ulp(struct sock *sk)
-{
-	struct inet_connection_sock *icsk = inet_csk(sk);
-
-	if (!icsk->icsk_ulp_ops)
-		return;
-
-	if (icsk->icsk_ulp_ops->release)
-		icsk->icsk_ulp_ops->release(sk);
-	module_put(icsk->icsk_ulp_ops->owner);
-}
-
-/* Change upper layer protocol for socket */
-int tcp_set_ulp(struct sock *sk, const char *name)
-{
-	struct inet_connection_sock *icsk = inet_csk(sk);
-	const struct tcp_ulp_ops *ulp_ops;
-	int err = 0;
-
-	if (icsk->icsk_ulp_ops)
-		return -EEXIST;
-
-	ulp_ops = __tcp_ulp_find_autoload(name);
-	if (!ulp_ops)
-		err = -ENOENT;
-	else
-		err = ulp_ops->init(sk);
-
-	if (err)
-		goto out;
-
-	icsk->icsk_ulp_ops = ulp_ops;
- out:
-	return err;
-}
diff --git a/net/tls/Kconfig b/net/tls/Kconfig
index eb583038c67e..60ae4e9b257e 100644
--- a/net/tls/Kconfig
+++ b/net/tls/Kconfig
@@ -7,6 +7,7 @@ config TLS
 	select CRYPTO
 	select CRYPTO_AES
 	select CRYPTO_GCM
+	select ULP_SOCK
 	default n
 	---help---
 	Enable kernel support for TLS protocol. This allows symmetric
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 60aff60e30ad..1ae5b50f7b7a 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -31,15 +31,15 @@
  * SOFTWARE.
  */
 
-#include <linux/module.h>
-
-#include <net/tcp.h>
-#include <net/inet_common.h>
 #include <linux/highmem.h>
+#include <linux/module.h>
 #include <linux/netdevice.h>
 #include <linux/sched/signal.h>
-
+#include <net/inet_common.h>
+#include <net/sock.h>
+#include <net/tcp.h>
 #include <net/tls.h>
+#include <net/ulp_sock.h>
 
 MODULE_AUTHOR("Mellanox Technologies");
 MODULE_DESCRIPTION("Transport Layer Security Support");
@@ -438,19 +438,21 @@ static int tls_setsockopt(struct sock *sk, int level, int optname,
 	return do_tls_setsockopt(sk, optname, optval, optlen);
 }
 
-static int tls_init(struct sock *sk)
+static int tls_init(struct sock *sk, char __user *optval, int len)
 {
-	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tls_context *ctx;
 	int rc = 0;
 
+	if (sk->sk_protocol != IPPROTO_TCP)
+		return -EAFNOSUPPORT;
+
 	/* allocate tls context */
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx) {
 		rc = -ENOMEM;
 		goto out;
 	}
-	icsk->icsk_ulp_data = ctx;
+	sk->sk_ulp_data = ctx;
 	ctx->setsockopt = sk->sk_prot->setsockopt;
 	ctx->getsockopt = sk->sk_prot->getsockopt;
 	sk->sk_prot = &tls_base_prot;
@@ -458,7 +460,7 @@ static int tls_init(struct sock *sk)
 	return rc;
 }
 
-static struct tcp_ulp_ops tcp_tls_ulp_ops __read_mostly = {
+static struct ulp_ops ulp_tls_ops __read_mostly = {
 	.name			= "tls",
 	.owner			= THIS_MODULE,
 	.init			= tls_init,
@@ -475,14 +477,14 @@ static int __init tls_register(void)
 	tls_sw_prot.sendpage            = tls_sw_sendpage;
 	tls_sw_prot.close               = tls_sk_proto_close;
 
-	tcp_register_ulp(&tcp_tls_ulp_ops);
+	ulp_register(&ulp_tls_ops);
 
 	return 0;
 }
 
 static void __exit tls_unregister(void)
 {
-	tcp_unregister_ulp(&tcp_tls_ulp_ops);
+	ulp_unregister(&ulp_tls_ops);
 }
 
 module_init(tls_register);
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 net-next 4/4] ulp: Documention for ULP infrastructure
From: Tom Herbert @ 2017-08-03 19:31 UTC (permalink / raw)
  To: netdev; +Cc: rohit, davejwatson, Tom Herbert
In-Reply-To: <20170803193144.24108-1-tom@quantonium.net>

Add a doc in Documentation/networking

Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 Documentation/networking/ulp.txt | 82 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 Documentation/networking/ulp.txt

diff --git a/Documentation/networking/ulp.txt b/Documentation/networking/ulp.txt
new file mode 100644
index 000000000000..4d830314b0ff
--- /dev/null
+++ b/Documentation/networking/ulp.txt
@@ -0,0 +1,82 @@
+Upper Layer Protocol (ULP) Infrastructure
+=========================================
+
+The ULP kernel infrastructure provides a means to hook upper layer
+protocol support on a socket. A module may register a ULP hook
+in the kernel. ULP processing is enabled by a setsockopt on a socket
+that specifies the name of the registered ULP to invoked. An
+initialization function is defined for each ULP that can change the
+function entry points of the socket (sendmsg, rcvmsg, etc.) or change
+the socket in other fundamental ways.
+
+Note, no synchronization is enforced between the setsockopt to enable
+a ULP and ongoing asynchronous operations on the socket (such as a
+blocked read). If synchronization is required this must be handled by
+the ULP and caller.
+
+User interface
+==============
+
+The structure for the socket SOL_ULP options is defined in socket.h.
+
+Example to enable "my_ulp" ULP on a socket:
+
+struct ulp_config ulpc = {
+    .ulp_name = "my_ulp",
+};
+
+setsockopt(sock, SOL_SOCKET, SO_ULP, &ulpc, sizeof(ulpc))
+
+The ulp_config includes a "__u8 ulp_params[0]" filled that may be used
+to refer ULP specific parameters being set.
+
+Kernel interface
+================
+
+The interface for ULP infrastructure is defined in net/ulp_sock.h.
+
+ULP registration functions
+--------------------------
+
+int ulp_register(struct ulp_ops *type)
+
+     Called to register a ULP. The ulp_ops structure is described below.
+
+void ulp_unregister(struct ulp_ops *type);
+
+     Called to unregister a ULP.
+
+ulp_ops structure
+-----------------
+
+int (*init)(struct sock *sk, char __user *optval, int len)
+
+     Initialization function for the ULP. This is called from setsockopt
+     when the ULP name in the ulp_config argument matches the registered
+     ULP. optval is a userspace pointer to the ULP specific parameters.
+     len is the length of the ULP specific parameters.
+
+void (*release)(struct sock *sk)
+
+     Called when socket is being destroyed. The ULP implementation
+     should cancel any asynchronous operations (such as timers) and
+     release any acquired resources.
+
+int (*get_params)(struct sock *sk, char __user *optval, int *optlen)
+
+     Get the ULP specific parameters previous set in the init function
+     for the ULP. Note that optlen is a pointer to kernel memory.
+
+char name[ULP_NAME_MAX]
+
+     Name of the ULP. Must be NULL terminated.
+
+struct module *owner
+
+     Corresponding owner for ref count.
+
+Author
+======
+
+Tom Herbert (tom@quantonium.net)
+
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next] net: dsa: Add support for 64-bit statistics
From: Florian Fainelli @ 2017-08-03 19:43 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, netdev, vivien.didelot, linux-kernel
In-Reply-To: <20170803181156.GJ15018@lunn.ch>

On 08/03/2017 11:11 AM, Andrew Lunn wrote:
> On Thu, Aug 03, 2017 at 10:30:56AM -0700, Florian Fainelli wrote:
>> On 08/02/2017 04:49 PM, David Miller wrote:
>>> From: Florian Fainelli <f.fainelli@gmail.com>
>>> Date: Tue,  1 Aug 2017 15:00:36 -0700
>>>
>>>> DSA slave network devices maintain a pair of bytes and packets counters
>>>> for each directions, but these are not 64-bit capable. Re-use
>>>> pcpu_sw_netstats which contains exactly what we need for that purpose
>>>> and update the code path to report 64-bit capable statistics.
>>>>
>>>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>>>
>>> Applied, thanks.
>>>
>>> I would run ethtool -S and ifconfig under perf to see where it is
>>> spending so much time.
>>>
>>
>> This appears to be way worse than I thought, will keep digging, but for
>> now, I may have to send a revert. Andrew, Vivien can you see if you have
>> the same problems on your boards? Thanks!
>>
>> # killall iperf
>> # [ ID] Interval       Transfer     Bandwidth
>> [  3]  0.0-19.1 sec   500 MBytes   220 Mbits/sec
>> # while true; do ethtool -S gphy; ifconfig gphy; done
>> ^C^C
>>
>>
>> [   64.566226] INFO: rcu_sched self-detected stall on CPU
>> [   64.571487]  0-...: (25999 ticks this GP) idle=006/140000000000001/0
> 
> Hi Florian

Hi Andrew,

> 
> I don't get anything so bad, but i think that is because of hardware
> restrictions. I see the ethtool; ifconfig loop goes a lot slower when
> there is iperf traffic, but i don't get an RCU stall. However, the
> board i tested on only has a 100Mbps CPU interface, and it can handle
> all that traffic without pushing the CPU to 100%. What is the CPU load
> when you run your test? Even if you are going to 100% CPU load, we
> still don't want RCU stalls.

This is a quad core 1.5 Ghz board pushing 1Gbit/sec worth of traffic,
this is about 25% loaded. What is needed to reproduce this is basically:

iperf -c 192.168.1.1 -t 30 &
while true; do ifconfig gphy; ethtool -S gphy; done

when iperf terminates, the lock-up reliably occurs. I just converted
net/dsa/ to use per-cpu statistics and of course, I can no longer
reproduce this problem now...
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 1/3] proto_ops: Add locked held versions of sendmsg and sendpage
From: John Fastabend @ 2017-08-03 19:49 UTC (permalink / raw)
  To: Tom Herbert, netdev; +Cc: rohit
In-Reply-To: <20170728232243.3040-2-tom@quantonium.net>

On 07/28/2017 04:22 PM, Tom Herbert wrote:
> Add new proto_ops sendmsg_locked and sendpage_locked that can be
> called when the socket lock is already held. Correspondingly, add
> kernel_sendmsg_locked and kernel_sendpage_locked as front end
> functions.
> 
> These functions will be used in zero proxy so that we can take
> the socket lock in a ULP sendmsg/sendpage and then directly call the
> backend transport proto_ops functions.
> 
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---

[...]

> diff --git a/net/socket.c b/net/socket.c
> index 79d9bb964cd8..c0a12ad39610 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -652,6 +652,20 @@ int kernel_sendmsg(struct socket *sock, struct msghdr *msg,
>  }
>  EXPORT_SYMBOL(kernel_sendmsg);
>  
> +int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
> +			  struct kvec *vec, size_t num, size_t size)
> +{
> +	struct socket *sock = sk->sk_socket;
> +
> +	if (!sock->ops->sendmsg_locked)
> +		sock_no_sendmsg_locked(sk, msg, size);
> +

Should be

		return sock_no_sendmsg_locked(sk, msg, size);


> +	iov_iter_kvec(&msg->msg_iter, WRITE | ITER_KVEC, vec, num, size);
> +
> +	return sock->ops->sendmsg_locked(sk, msg, msg_data_left(msg));

Otherwise this is a null ptr deref.

> +}
> +EXPORT_SYMBOL(kernel_sendmsg_locked);
> +


Thanks,
John

^ permalink raw reply

* Re: [PATCH net-next 2/3] skbuff: Function to send an skbuf on a socket
From: John Fastabend @ 2017-08-03 19:51 UTC (permalink / raw)
  To: Tom Herbert, netdev; +Cc: rohit
In-Reply-To: <20170728232243.3040-3-tom@quantonium.net>

On 07/28/2017 04:22 PM, Tom Herbert wrote:
> Add skb_send_sock to send an skbuff on a socket within the kernel.
> Arguments include an offset so that an skbuf might be sent in mulitple
> calls (e.g. send buffer limit is hit).
> 
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---

[...]

> +/* Send skb data on a socket. Socket must be locked. */
> +int skb_send_sock_locked(struct sock *sk, struct sk_buff *skb, int offset,
> +			 int len)
> +{
> +	unsigned int orig_len = len;
> +	struct sk_buff *head = skb;
> +	unsigned short fragidx;
> +	int slen, ret;
> +
> +do_frag_list:
> +
> +	/* Deal with head data */
> +	while (offset < skb_headlen(skb) && len) {
> +		struct kvec kv;
> +		struct msghdr msg;
> +
> +		slen = min_t(int, len, skb_headlen(skb) - offset);
> +		kv.iov_base = skb->data + offset;
> +		kv.iov_len = len;
                            ^^^^^^

This should be slen right?

> +		memset(&msg, 0, sizeof(msg));
> +
> +		ret = kernel_sendmsg_locked(sk, &msg, &kv, 1, slen);
> +		if (ret <= 0)
> +			goto error;
> +
> +		offset += ret;
> +		len -= ret;
> +	}
> +

Thanks,
John

^ permalink raw reply

* [PATCH net-next] liquidio: add missing strings in oct_dev_state_str array
From: Felix Manlunas @ 2017-08-03 20:08 UTC (permalink / raw)
  To: davem
  Cc: netdev, raghu.vatsavayi, derek.chickles, satananda.burla,
	intiyaz.basha

From: Intiyaz Basha <intiyaz.basha@cavium.com>

There's supposed to be a one-to-one correspondence between the 18 macros
that #define the OCT_DEV states (in octeon_device.h) and the strings in the
oct_dev_state_str array, but there are only 14 strings in the array.

Add the missing strings (so they become 18 in total), and also revise some
incorrect/outdated text of existing strings.

Signed-off-by: Intiyaz Basha <intiyaz.basha@cavium.com>
Signed-off-by: Felix Manlunas <felix.manlunas@cavium.com>
---
 drivers/net/ethernet/cavium/liquidio/octeon_device.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index f10014f7..495cc88 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -528,9 +528,10 @@ static struct octeon_config_ptr {
 };
 
 static char oct_dev_state_str[OCT_DEV_STATES + 1][32] = {
-	"BEGIN", "PCI-MAP-DONE", "DISPATCH-INIT-DONE",
+	"BEGIN", "PCI-ENABLE-DONE", "PCI-MAP-DONE", "DISPATCH-INIT-DONE",
 	"IQ-INIT-DONE", "SCBUFF-POOL-INIT-DONE", "RESPLIST-INIT-DONE",
-	"DROQ-INIT-DONE", "IO-QUEUES-INIT-DONE", "CONSOLE-INIT-DONE",
+	"DROQ-INIT-DONE", "MBOX-SETUP-DONE", "MSIX-ALLOC-VECTOR-DONE",
+	"INTR-SET-DONE", "IO-QUEUES-INIT-DONE", "CONSOLE-INIT-DONE",
 	"HOST-READY", "CORE-READY", "RUNNING", "IN-RESET",
 	"INVALID"
 };

^ permalink raw reply related

* Re: [PATCH v3 net-next 3/5] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage
From: Egil Hjelmeland @ 2017-08-03 20:16 UTC (permalink / raw)
  To: Florian Fainelli, andrew, vivien.didelot, netdev, linux-kernel,
	kernel
In-Reply-To: <5a67a44b-6887-1466-c0f3-ce65759d51db@gmail.com>

Den 03. aug. 2017 20:06, skrev Florian Fainelli:
> On 08/03/2017 02:45 AM, Egil Hjelmeland wrote:
>> Simplify usage of lan9303_enable_packet_processing,
>> lan9303_disable_packet_processing()
>>
>> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> took a little while to figure out that we are utilizing fall through of
> the switch/case statement and that's why it's okay.
> 
>>
>>   static int lan9303_check_device(struct lan9303 *chip)
>> @@ -765,7 +766,6 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
>>   	/* enable internal packet processing */
>>   	switch (port) {
>>   	case 1:
>> -		return lan9303_enable_packet_processing(chip, port);
>>   	case 2:
>>   		return lan9303_enable_packet_processing(chip, port);
>>   	default:

I suppose if we later change to dsa_switch_alloc(...,3), then it could
be further simplified to

if (port != 0)
	return lan9303_enable_packet_processing(chip, port);

Or perhaps no test is needed at all. The driver assumes port 0 is cpu
port, which is the sensible way to use the chip. (Because port 0 has
no phy, the others have phy). Declaring a different port as cpu port in
DTS will not work, but it will not crash the kernel.

Egil

^ permalink raw reply

* Re: [PATCH net-next 1/3] proto_ops: Add locked held versions of sendmsg and sendpage
From: John Fastabend @ 2017-08-03 20:21 UTC (permalink / raw)
  To: Tom Herbert, netdev; +Cc: rohit
In-Reply-To: <20170728232243.3040-2-tom@quantonium.net>

On 07/28/2017 04:22 PM, Tom Herbert wrote:
> Add new proto_ops sendmsg_locked and sendpage_locked that can be
> called when the socket lock is already held. Correspondingly, add
> kernel_sendmsg_locked and kernel_sendpage_locked as front end
> functions.
> 
> These functions will be used in zero proxy so that we can take
> the socket lock in a ULP sendmsg/sendpage and then directly call the
> backend transport proto_ops functions.
> 

[...]

>  
> +int kernel_sendpage_locked(struct sock *sk, struct page *page, int offset,
> +			   size_t size, int flags)
> +{
> +	struct socket *sock = sk->sk_socket;
> +
> +	if (sock->ops->sendpage_locked)
> +		return sock->ops->sendpage_locked(sk, page, offset, size,
> +						  flags);
> +
> +	return sock_no_sendpage_locked(sk, page, offset, size, flags);
> +}

How about just returning EOPNOTSUPP here and force implementations to do both
sendmsg and sendpage. The only implementation of these callbacks already does
this. And if its any other socket it will just wind its way through a few
layers of calls before returning EOPNOTSUPP.

.John

^ permalink raw reply

* Re: [PATCH v3 net-next 5/5] net: dsa: lan9303: refactor lan9303_get_ethtool_stats
From: Egil Hjelmeland @ 2017-08-03 20:26 UTC (permalink / raw)
  To: Florian Fainelli, andrew, vivien.didelot, netdev, linux-kernel,
	kernel
In-Reply-To: <d91c3159-57a3-dea3-462e-24a4156127b2@gmail.com>

Den 03. aug. 2017 20:04, skrev Florian Fainelli:
> On 08/03/2017 02:45 AM, Egil Hjelmeland wrote:
>> In lan9303_get_ethtool_stats: Get rid of 0x400 constant magic
>> by using new lan9303_read_switch_reg() inside loop.
>> Reduced scope of two variables.
>>
>> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
>> ---
>>   drivers/net/dsa/lan9303-core.c | 26 ++++++++++++++++----------
>>   1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
>> index 6f409755ba1a..5aaa46146c27 100644
>> --- a/drivers/net/dsa/lan9303-core.c
>> +++ b/drivers/net/dsa/lan9303-core.c
>> @@ -435,6 +435,13 @@ static int lan9303_write_switch_port(
>>   		chip, LAN9303_SWITCH_PORT_REG(port, regnum), val);
>>   }
>>   
>> +static int lan9303_read_switch_port(
>> +	struct lan9303 *chip, int port, u16 regnum, u32 *val)
>> +{
> 
> This indentation is really funny, why not just break it up that way:
> 
> static int lan9303_read_switch_port(struct lan9303 *chip, int port
> 				    u16 regnum, u32 *val)
> {
> }
> 
> This applies to patch 5 as well, other than that:
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 

Because it is the form that passes scripts/checkpatch.pl which I find
easiest to type and maintain.

- No need to fine tune spaces.
- No need to change indentation if later renaming function to name of
   different length.

Do you have any references backing up your claim this is wrong?

Cheers
Egil

^ permalink raw reply

* Re: [RFC] iproute: Add support for extended ack to rtnl_talk
From: David Ahern @ 2017-08-03 20:26 UTC (permalink / raw)
  To: Daniel Borkmann, Stephen Hemminger; +Cc: Phil Sutter, David Miller, netdev
In-Reply-To: <1c494983-24e0-cd8f-9a2a-5d2a83e0bbc2@gmail.com>

On 5/18/17 10:24 PM, David Ahern wrote:
> On 5/18/17 3:02 AM, Daniel Borkmann wrote:
>> So effectively this means libmnl has to be used for new stuff, noone
>> has time to do the work to convert the existing tooling over (which
>> by itself might be a challenge in testing everything to make sure
>> there are no regressions) given there's not much activity around
>> lib/libnetlink.c anyway, and existing users not using libmnl today
>> won't see/notice new improvements on netlink side when they do an
>> upgrade. So we'll be stuck with that dual library mess pretty much
>> for a very long time. :(
> 
> lib/libnetlink.c with all of its duplicate functions weighs in at just
> 947 LOC -- a mere 12% of the code in lib/. From a total SLOC of iproute2
> it is a negligible part of the code base.
> 
> Given that, there is very little gain -- but a lot of risk in
> regressions -- in converting such a small, low level code base to libmnl
> just for the sake of using a library - something Phil noted in his
> cursory attempt at converting ip to libmnl. ie., The level effort
> required vs the benefit is just not worth it.
> 
> There are so many other parts of the ip code base that need work with a
> much higher return on the time investment.
> 

Stephen: It has been 3 months since the first extack patches were posted
and still nothing in iproute2, all of it hung up on your decision to
require libmnl. Do you plan to finish the libmnl support any time soon
and send out patches?

^ permalink raw reply

* [PATCH net-next v4 0/9] socket sendmsg MSG_ZEROCOPY
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Willem de Bruijn

From: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Introduce zerocopy socket send flag MSG_ZEROCOPY. This extends the
shared page support (SKBTX_SHARED_FRAG) from sendpage to sendmsg.
Implement the feature for TCP initially, as large writes benefit
most.

On a send call with MSG_ZEROCOPY, the kernel pins user pages and
links these directly into the skbuff frags[] array.

Each send call with MSG_ZEROCOPY that transmits data will eventually
queue a completion notification on the error queue: a per-socket u32
incremented on each such call. A request may have to revert to copy
to succeed, for instance when a device cannot support scatter-gather
IO. In that case a flag is passed along to notify that the operation
succeeded without zerocopy optimization.

The implementation extends the existing zerocopy infra for tuntap,
vhost and xen with features needed for TCP, notably reference
counting to handle cloning on retransmit and GSO.

For more details, see also the netdev 2.1 paper and presentation at
https://netdevconf.org/2.1/session.html?debruijn

Changelog:

  v3 -> v4:
    - dropped UDP, RAW and PF_PACKET for now
        Without loopback support, datagrams are usually smaller than
        the ~8KB size threshold needed to benefit from zerocopy.
    - style: a few reverse chrismas tree
    - minor: SO_ZEROCOPY returns ENOTSUPP on unsupported protocols
    - minor: squashed SO_EE_CODE_ZEROCOPY_COPIED patch
    - minor: rebased on top of net-next with kmap_atomic fix        

  v2 -> v3:
    - fix rebase conflict: SO_ZEROCOPY 59 -> 60

  v1 -> v2:
    - fix (kbuild-bot): do not remove uarg until patch 5
    - fix (kbuild-bot): move zerocopy_sg_from_iter doc with function
    - fix: remove unused extern in header file

  RFCv2 -> v1:
    - patch 2
        - review comment: in skb_copy_ubufs, always allocate order-0
            page, also when replacing compound source pages.
    - patch 3
        - fix: always queue completion notification on MSG_ZEROCOPY,
	    also if revert to copy.
	- fix: on syscall abort, correctly revert notification state
	- minor: skip queue notification on SOCK_DEAD
	- minor: replace BUG_ON with WARN_ON in recoverable error
    - patch 4
        - new: add socket option SOCK_ZEROCOPY.
	    only honor MSG_ZEROCOPY if set, ignore for legacy apps.
    - patch 5
        - fix: clear zerocopy state on skb_linearize
    - patch 6
        - fix: only coalesce if prev errqueue elem is zerocopy
	- minor: try coalescing with list tail instead of head
        - minor: merge bytelen limit patch
    - patch 7
        - new: signal when data had to be copied
    - patch 8 (tcp)
        - optimize: avoid setting PSH bit when exceeding max frags.
	    that limits GRO on the client. do not goto new_segment.
	- fix: fail on MSG_ZEROCOPY | MSG_FASTOPEN
	- minor: do not wait for memory: does not work for optmem
	- minor: simplify alloc
    - patch 9 (udp)
        - new: add PF_INET6
        - fix: attach zerocopy notification even if revert to copy
	- minor: simplify alloc size arithmetic
    - patch 10 (raw hdrinc)
        - new: add PF_INET6
    - patch 11 (pf_packet)
        - minor: simplify slightly
    - patch 12
        - new msg_zerocopy regression test: use veth pair to test
	    all protocols: ipv4/ipv6/packet, tcp/udp/raw, cork
	    all relevant ethtool settings: rx off, sg off
	    all relevant packet lengths: 0, <MAX_HEADER, max size

  RFC -> RFCv2:
    - review comment: do not loop skb with zerocopy frags onto rx:
          add skb_orphan_frags_rx to orphan even refcounted frags
	  call this in __netif_receive_skb_core, deliver_skb and tun:
	  same as commit 1080e512d44d ("net: orphan frags on receive")
    - fix: hold an explicit sk reference on each notification skb.
          previously relied on the reference (or wmem) held by the
	  data skb that would trigger notification, but this breaks
	  on skb_orphan.
    - fix: when aborting a send, do not inc the zerocopy counter
          this caused gaps in the notification chain
    - fix: in packet with SOCK_DGRAM, pull ll headers before calling
          zerocopy_sg_from_iter
    - fix: if sock_zerocopy_realloc does not allow coalescing,
          do not fail, just allocate a new ubuf
    - fix: in tcp, check return value of second allocation attempt
    - chg: allocate notification skbs from optmem
          to avoid affecting tcp write queue accounting (TSQ)
    - chg: limit #locked pages (ulimit) per user instead of per process
    - chg: grow notification ids from 16 to 32 bit
      - pass range [lo, hi] through 32 bit fields ee_info and ee_data
    - chg: rebased to davem-net-next on top of v4.10-rc7
    - add: limit notification coalescing
          sharing ubufs limits overhead, but delays notification until
	  the last packet is released, possibly unbounded. Add a cap. 
    - tests: add snd_zerocopy_lo pf_packet test
    - tests: two bugfixes (add do_flush_tcp, ++sent not only in debug)

Limitations / Known Issues:
    - TCP may build slightly smaller than max TSO packets due to
      exceeding MAX_SKB_FRAGS frags when zerocopy pages are unaligned.
    - All SKBTX_SHARED_FRAG may require additional __skb_linearize or
      skb_copy_ubufs calls in u32, skb_find_text, similar to
      skb_checksum_help.

Notification skbuffs are allocated from optmem. For sockets that
cannot effectively coalesce notifications, the optmem max may need
to be increased to avoid hitting -ENOBUFS:

  sysctl -w net.core.optmem_max=1048576

In application load, copy avoidance shows a roughly 5% systemwide
reduction in cycles when streaming large flows and a 4-8% reduction in
wall clock time on early tensorflow test workloads.

For the single-machine veth tests to succeed, loopback support has to
be temporarily enabled by making skb_orphan_frags_rx map to
skb_orphan_frags.

* Performance

The below table shows cycles reported by perf for a netperf process
sending a single 10 Gbps TCP_STREAM. The first three columns show
Mcycles spent in the netperf process context. The second three columns
show time spent systemwide (-a -C A,B) on the two cpus that run the
process and interrupt handler. Reported is the median of at least 3
runs. std is a standard netperf, zc uses zerocopy and % is the ratio.
Netperf is pinned to cpu 2, network interrupts to cpu3, rps and rfs
are disabled and the kernel is booted with idle=halt.

NETPERF=./netperf -t TCP_STREAM -H $host -T 2 -l 30 -- -m $size

perf stat -e cycles $NETPERF
perf stat -C 2,3 -a -e cycles $NETPERF

        --process cycles--      ----cpu cycles----
           std      zc   %      std         zc   %
4K      27,609  11,217  41      49,217  39,175  79
16K     21,370   3,823  18      43,540  29,213  67
64K     20,557   2,312  11      42,189  26,910  64
256K    21,110   2,134  10      43,006  27,104  63
1M      20,987   1,610   8      42,759  25,931  61

Perf record indicates the main source of these differences. Process
cycles only at 1M writes (perf record; perf report -n):

std:
Samples: 42K of event 'cycles', Event count (approx.): 21258597313                                                   
 79.41%         33884  netperf  [kernel.kallsyms]  [k] copy_user_generic_string
  3.27%          1396  netperf  [kernel.kallsyms]  [k] tcp_sendmsg
  1.66%           694  netperf  [kernel.kallsyms]  [k] get_page_from_freelist
  0.79%           325  netperf  [kernel.kallsyms]  [k] tcp_ack
  0.43%           188  netperf  [kernel.kallsyms]  [k] __alloc_skb

zc:
Samples: 1K of event 'cycles', Event count (approx.): 1439509124                                                     
 30.36%           584  netperf.zerocop  [kernel.kallsyms]  [k] gup_pte_range
 14.63%           284  netperf.zerocop  [kernel.kallsyms]  [k] __zerocopy_sg_from_iter
  8.03%           159  netperf.zerocop  [kernel.kallsyms]  [k] skb_zerocopy_add_frags_iter
  4.84%            96  netperf.zerocop  [kernel.kallsyms]  [k] __alloc_skb
  3.10%            60  netperf.zerocop  [kernel.kallsyms]  [k] kmem_cache_alloc_node


* Safety

The number of pages that can be pinned on behalf of a user with
MSG_ZEROCOPY is bound by the locked memory ulimit.

While the kernel holds process memory pinned, a process cannot safely
reuse those pages for other purposes. Packets looped onto the receive
stack and queued to a socket can be held indefinitely. Avoid unbounded
notification latency by restricting user pages to egress paths only.
skb_orphan_frags_rx() will create a private copy of pages even for
refcounted packets when these are looped, as did skb_orphan_frags for
the original tun zerocopy implementation.

Pages are not remapped read-only. Processes can modify packet contents
while packets are in flight in the kernel path. Bytes on which kernel
control flow depends (headers) are copied to avoid TOCTTOU attacks.
Datapath integrity does not otherwise depend on payload, with three
exceptions: checksums, optional sk_filter/tc u32/.. and device +
driver logic. The effect of wrong checksums is limited to the
misbehaving process. TC filters that access contents may have to be
excluded by adding an skb_orphan_frags_rx. 

Processes can also safely avoid OOM conditions by bounding the number
of bytes passed with MSG_ZEROCOPY and by removing shared pages after
transmission from their own memory map.


Willem de Bruijn (9):
  sock: allocate skbs from optmem
  sock: skb_copy_ubufs support for compound pages
  sock: add MSG_ZEROCOPY
  sock: add SOCK_ZEROCOPY sockopt
  sock: enable MSG_ZEROCOPY
  sock: MSG_ZEROCOPY notification coalescing
  sock: ulimit on MSG_ZEROCOPY pages
  tcp: enable MSG_ZEROCOPY
  test: add msg_zerocopy test

 arch/alpha/include/uapi/asm/socket.h        |   2 +
 arch/frv/include/uapi/asm/socket.h          |   2 +
 arch/ia64/include/uapi/asm/socket.h         |   2 +
 arch/m32r/include/uapi/asm/socket.h         |   2 +
 arch/mips/include/uapi/asm/socket.h         |   2 +
 arch/mn10300/include/uapi/asm/socket.h      |   2 +
 arch/parisc/include/uapi/asm/socket.h       |   2 +
 arch/s390/include/uapi/asm/socket.h         |   2 +
 arch/sparc/include/uapi/asm/socket.h        |   2 +
 arch/xtensa/include/uapi/asm/socket.h       |   2 +
 drivers/net/tun.c                           |   2 +-
 drivers/vhost/net.c                         |   1 +
 include/linux/sched/user.h                  |   3 +-
 include/linux/skbuff.h                      | 103 +++-
 include/linux/socket.h                      |   1 +
 include/net/sock.h                          |   4 +
 include/uapi/asm-generic/socket.h           |   2 +
 include/uapi/linux/errqueue.h               |   3 +
 net/core/datagram.c                         |  55 ++-
 net/core/dev.c                              |   4 +-
 net/core/skbuff.c                           | 362 +++++++++++++--
 net/core/sock.c                             |  47 ++
 net/ipv4/tcp.c                              |  32 +-
 tools/testing/selftests/net/.gitignore      |   1 +
 tools/testing/selftests/net/Makefile        |   2 +-
 tools/testing/selftests/net/msg_zerocopy.c  | 697 ++++++++++++++++++++++++++++
 tools/testing/selftests/net/msg_zerocopy.sh | 113 +++++
 27 files changed, 1380 insertions(+), 72 deletions(-)
 create mode 100644 tools/testing/selftests/net/msg_zerocopy.c
 create mode 100755 tools/testing/selftests/net/msg_zerocopy.sh

-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply

* [PATCH net-next v4 1/9] sock: allocate skbs from optmem
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Add sock_omalloc and sock_ofree to be able to allocate control skbs,
for instance for looping errors onto sk_error_queue.

The transmit budget (sk_wmem_alloc) is involved in transmit skb
shaping, most notably in TCP Small Queues. Using this budget for
control packets would impact transmission.

Signed-off-by: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 include/net/sock.h |  2 ++
 net/core/sock.c    | 27 +++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index 393c38e9f6aa..0f778d3c4300 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1531,6 +1531,8 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force,
 			     gfp_t priority);
 void __sock_wfree(struct sk_buff *skb);
 void sock_wfree(struct sk_buff *skb);
+struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size,
+			     gfp_t priority);
 void skb_orphan_partial(struct sk_buff *skb);
 void sock_rfree(struct sk_buff *skb);
 void sock_efree(struct sk_buff *skb);
diff --git a/net/core/sock.c b/net/core/sock.c
index 742f68c9c84a..1261880bdcc8 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1923,6 +1923,33 @@ struct sk_buff *sock_wmalloc(struct sock *sk, unsigned long size, int force,
 }
 EXPORT_SYMBOL(sock_wmalloc);
 
+static void sock_ofree(struct sk_buff *skb)
+{
+	struct sock *sk = skb->sk;
+
+	atomic_sub(skb->truesize, &sk->sk_omem_alloc);
+}
+
+struct sk_buff *sock_omalloc(struct sock *sk, unsigned long size,
+			     gfp_t priority)
+{
+	struct sk_buff *skb;
+
+	/* small safe race: SKB_TRUESIZE may differ from final skb->truesize */
+	if (atomic_read(&sk->sk_omem_alloc) + SKB_TRUESIZE(size) >
+	    sysctl_optmem_max)
+		return NULL;
+
+	skb = alloc_skb(size, priority);
+	if (!skb)
+		return NULL;
+
+	atomic_add(skb->truesize, &sk->sk_omem_alloc);
+	skb->sk = sk;
+	skb->destructor = sock_ofree;
+	return skb;
+}
+
 /*
  * Allocate a memory block from the socket's option memory buffer.
  */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 2/9] sock: skb_copy_ubufs support for compound pages
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Refine skb_copy_ubufs to support compound pages. With upcoming TCP
zerocopy sendmsg, such fragments may appear.

The existing code replaces each page one for one. Splitting each
compound page into an independent number of regular pages can result
in exceeding limit MAX_SKB_FRAGS if data is not exactly page aligned.

Instead, fill all destination pages but the last to PAGE_SIZE.
Split the existing alloc + copy loop into separate stages:
1. compute bytelength and minimum number of pages to store this.
2. allocate
3. copy, filling each page except the last to PAGE_SIZE bytes
4. update skb frag array

Signed-off-by: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 include/linux/skbuff.h |  9 +++++++--
 net/core/skbuff.c      | 53 ++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index be76082f48aa..2f64e2bbb592 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1796,13 +1796,18 @@ static inline unsigned int skb_headlen(const struct sk_buff *skb)
 	return skb->len - skb->data_len;
 }
 
-static inline unsigned int skb_pagelen(const struct sk_buff *skb)
+static inline unsigned int __skb_pagelen(const struct sk_buff *skb)
 {
 	unsigned int i, len = 0;
 
 	for (i = skb_shinfo(skb)->nr_frags - 1; (int)i >= 0; i--)
 		len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
-	return len + skb_headlen(skb);
+	return len;
+}
+
+static inline unsigned int skb_pagelen(const struct sk_buff *skb)
+{
+	return skb_headlen(skb) + __skb_pagelen(skb);
 }
 
 /**
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0f0933b338d7..a95877a8ac8b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -932,17 +932,20 @@ EXPORT_SYMBOL_GPL(skb_morph);
  */
 int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 {
-	int i;
+	struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
 	int num_frags = skb_shinfo(skb)->nr_frags;
 	struct page *page, *head = NULL;
-	struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
+	int i, new_frags;
+	u32 d_off;
 
-	for (i = 0; i < num_frags; i++) {
-		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
-		u32 p_off, p_len, copied;
-		struct page *p;
-		u8 *vaddr;
+	if (!num_frags)
+		return 0;
+
+	if (skb_shared(skb) || skb_unclone(skb, gfp_mask))
+		return -EINVAL;
 
+	new_frags = (__skb_pagelen(skb) + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	for (i = 0; i < new_frags; i++) {
 		page = alloc_page(gfp_mask);
 		if (!page) {
 			while (head) {
@@ -952,17 +955,36 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 			}
 			return -ENOMEM;
 		}
+		set_page_private(page, (unsigned long)head);
+		head = page;
+	}
+
+	page = head;
+	d_off = 0;
+	for (i = 0; i < num_frags; i++) {
+		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
+		u32 p_off, p_len, copied;
+		struct page *p;
+		u8 *vaddr;
 
 		skb_frag_foreach_page(f, f->page_offset, skb_frag_size(f),
 				      p, p_off, p_len, copied) {
+			u32 copy, done = 0;
 			vaddr = kmap_atomic(p);
-			memcpy(page_address(page) + copied, vaddr + p_off,
-			       p_len);
+
+			while (done < p_len) {
+				if (d_off == PAGE_SIZE) {
+					d_off = 0;
+					page = (struct page *)page_private(page);
+				}
+				copy = min_t(u32, PAGE_SIZE - d_off, p_len - done);
+				memcpy(page_address(page) + d_off,
+				       vaddr + p_off + done, copy);
+				done += copy;
+				d_off += copy;
+			}
 			kunmap_atomic(vaddr);
 		}
-
-		set_page_private(page, (unsigned long)head);
-		head = page;
 	}
 
 	/* skb frags release userspace buffers */
@@ -972,11 +994,12 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 	uarg->callback(uarg, false);
 
 	/* skb frags point to kernel buffers */
-	for (i = num_frags - 1; i >= 0; i--) {
-		__skb_fill_page_desc(skb, i, head, 0,
-				     skb_shinfo(skb)->frags[i].size);
+	for (i = 0; i < new_frags - 1; i++) {
+		__skb_fill_page_desc(skb, i, head, 0, PAGE_SIZE);
 		head = (struct page *)page_private(head);
 	}
+	__skb_fill_page_desc(skb, new_frags - 1, head, 0, d_off);
+	skb_shinfo(skb)->nr_frags = new_frags;
 
 	skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
 	return 0;
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 6/9] sock: MSG_ZEROCOPY notification coalescing
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

In the simple case, each sendmsg() call generates data and eventually
a zerocopy ready notification N, where N indicates the Nth successful
invocation of sendmsg() with the MSG_ZEROCOPY flag on this socket.

TCP and corked sockets can cause send() calls to append new data to an
existing sk_buff and, thus, ubuf_info. In that case the notification
must hold a range. odify ubuf_info to store a inclusive range [N..N+m]
and add skb_zerocopy_realloc() to optionally extend an existing range.

Also coalesce notifications in this common case: if a notification
[1, 1] is about to be queued while [0, 0] is the queue tail, just modify
the head of the queue to read [0, 1].

Coalescing is limited to a few TSO frames worth of data to bound
notification latency.

Signed-off-by: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 include/linux/skbuff.h | 17 +++++++--
 net/core/skbuff.c      | 99 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 106 insertions(+), 10 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e5387932c266..f5bdd93a87da 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -444,15 +444,26 @@ enum {
  */
 struct ubuf_info {
 	void (*callback)(struct ubuf_info *, bool zerocopy_success);
-	void *ctx;
-	unsigned long desc;
-	u16 zerocopy:1;
+	union {
+		struct {
+			unsigned long desc;
+			void *ctx;
+		};
+		struct {
+			u32 id;
+			u16 len;
+			u16 zerocopy:1;
+			u32 bytelen;
+		};
+	};
 	atomic_t refcnt;
 };
 
 #define skb_uarg(SKB)	((struct ubuf_info *)(skb_shinfo(SKB)->destructor_arg))
 
 struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size);
+struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
+					struct ubuf_info *uarg);
 
 static inline void sock_zerocopy_get(struct ubuf_info *uarg)
 {
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 74d3c36f8419..dcee0f64f1fa 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -915,7 +915,9 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
 	uarg = (void *)skb->cb;
 
 	uarg->callback = sock_zerocopy_callback;
-	uarg->desc = atomic_inc_return(&sk->sk_zckey) - 1;
+	uarg->id = ((u32)atomic_inc_return(&sk->sk_zckey)) - 1;
+	uarg->len = 1;
+	uarg->bytelen = size;
 	uarg->zerocopy = 1;
 	atomic_set(&uarg->refcnt, 0);
 	sock_hold(sk);
@@ -929,26 +931,101 @@ static inline struct sk_buff *skb_from_uarg(struct ubuf_info *uarg)
 	return container_of((void *)uarg, struct sk_buff, cb);
 }
 
+struct ubuf_info *sock_zerocopy_realloc(struct sock *sk, size_t size,
+					struct ubuf_info *uarg)
+{
+	if (uarg) {
+		const u32 byte_limit = 1 << 19;		/* limit to a few TSO */
+		u32 bytelen, next;
+
+		/* realloc only when socket is locked (TCP, UDP cork),
+		 * so uarg->len and sk_zckey access is serialized
+		 */
+		if (!sock_owned_by_user(sk)) {
+			WARN_ON_ONCE(1);
+			return NULL;
+		}
+
+		bytelen = uarg->bytelen + size;
+		if (uarg->len == USHRT_MAX - 1 || bytelen > byte_limit) {
+			/* TCP can create new skb to attach new uarg */
+			if (sk->sk_type == SOCK_STREAM)
+				goto new_alloc;
+			return NULL;
+		}
+
+		next = (u32)atomic_read(&sk->sk_zckey);
+		if ((u32)(uarg->id + uarg->len) == next) {
+			uarg->len++;
+			uarg->bytelen = bytelen;
+			atomic_set(&sk->sk_zckey, ++next);
+			return uarg;
+		}
+	}
+
+new_alloc:
+	return sock_zerocopy_alloc(sk, size);
+}
+EXPORT_SYMBOL_GPL(sock_zerocopy_realloc);
+
+static bool skb_zerocopy_notify_extend(struct sk_buff *skb, u32 lo, u16 len)
+{
+	struct sock_exterr_skb *serr = SKB_EXT_ERR(skb);
+	u32 old_lo, old_hi;
+	u64 sum_len;
+
+	old_lo = serr->ee.ee_info;
+	old_hi = serr->ee.ee_data;
+	sum_len = old_hi - old_lo + 1ULL + len;
+
+	if (sum_len >= (1ULL << 32))
+		return false;
+
+	if (lo != old_hi + 1)
+		return false;
+
+	serr->ee.ee_data += len;
+	return true;
+}
+
 void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
 {
-	struct sk_buff *skb = skb_from_uarg(uarg);
+	struct sk_buff *tail, *skb = skb_from_uarg(uarg);
 	struct sock_exterr_skb *serr;
 	struct sock *sk = skb->sk;
-	u16 id = uarg->desc;
+	struct sk_buff_head *q;
+	unsigned long flags;
+	u32 lo, hi;
+	u16 len;
 
-	if (sock_flag(sk, SOCK_DEAD))
+	/* if !len, there was only 1 call, and it was aborted
+	 * so do not queue a completion notification
+	 */
+	if (!uarg->len || sock_flag(sk, SOCK_DEAD))
 		goto release;
 
+	len = uarg->len;
+	lo = uarg->id;
+	hi = uarg->id + len - 1;
+
 	serr = SKB_EXT_ERR(skb);
 	memset(serr, 0, sizeof(*serr));
 	serr->ee.ee_errno = 0;
 	serr->ee.ee_origin = SO_EE_ORIGIN_ZEROCOPY;
-	serr->ee.ee_data = id;
+	serr->ee.ee_data = hi;
+	serr->ee.ee_info = lo;
 	if (!success)
 		serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;
 
-	skb_queue_tail(&sk->sk_error_queue, skb);
-	skb = NULL;
+	q = &sk->sk_error_queue;
+	spin_lock_irqsave(&q->lock, flags);
+	tail = skb_peek_tail(q);
+	if (!tail || SKB_EXT_ERR(tail)->ee.ee_origin != SO_EE_ORIGIN_ZEROCOPY ||
+	    !skb_zerocopy_notify_extend(tail, lo, len)) {
+		__skb_queue_tail(q, skb);
+		skb = NULL;
+	}
+	spin_unlock_irqrestore(&q->lock, flags);
 
 	sk->sk_error_report(sk);
 
@@ -975,6 +1052,7 @@ void sock_zerocopy_put_abort(struct ubuf_info *uarg)
 		struct sock *sk = skb_from_uarg(uarg)->sk;
 
 		atomic_dec(&sk->sk_zckey);
+		uarg->len--;
 
 		/* sock_zerocopy_put expects a ref. Most sockets take one per
 		 * skb, which is zero on abort. tcp_sendmsg holds one extra, to
@@ -995,9 +1073,16 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 			     struct msghdr *msg, int len,
 			     struct ubuf_info *uarg)
 {
+	struct ubuf_info *orig_uarg = skb_zcopy(skb);
 	struct iov_iter orig_iter = msg->msg_iter;
 	int err, orig_len = skb->len;
 
+	/* An skb can only point to one uarg. This edge case happens when
+	 * TCP appends to an skb, but zerocopy_realloc triggered a new alloc.
+	 */
+	if (orig_uarg && uarg != orig_uarg)
+		return -EEXIST;
+
 	err = __zerocopy_sg_from_iter(sk, skb, &msg->msg_iter, len);
 	if (err == -EFAULT || (err == -EMSGSIZE && skb->len == orig_len)) {
 		/* Streams do not free skb on error. Reset to prev state. */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 9/9] test: add msg_zerocopy test
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

Introduce regression test for msg_zerocopy feature. Send traffic from
one process to another with and without zerocopy.

Evaluate tcp, udp, raw and packet sockets, including variants
- udp: corking and corking with mixed copy/zerocopy calls
- raw: with and without hdrincl
- packet: at both raw and dgram level

Test on both ipv4 and ipv6, optionally with ethtool changes to
disable scatter-gather, tx checksum or tso offload. All of these
can affect zerocopy behavior.

The regression test can be run on a single machine if over a veth
pair. Then skb_orphan_frags_rx must be modified to be identical to
skb_orphan_frags to allow forwarding zerocopy locally.

The msg_zerocopy.sh script will setup the veth pair in network
namespaces and run all tests.

Signed-off-by: Willem de Bruijn <willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
 tools/testing/selftests/net/.gitignore      |   1 +
 tools/testing/selftests/net/Makefile        |   2 +-
 tools/testing/selftests/net/msg_zerocopy.c  | 697 ++++++++++++++++++++++++++++
 tools/testing/selftests/net/msg_zerocopy.sh | 113 +++++
 4 files changed, 812 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/msg_zerocopy.c
 create mode 100755 tools/testing/selftests/net/msg_zerocopy.sh

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index afe109e5508a..9801253e4802 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -1,3 +1,4 @@
+msg_zerocopy
 socket
 psock_fanout
 psock_tpacket
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index f6c9dbf478f8..6135a8448900 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -7,7 +7,7 @@ TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh
 TEST_GEN_FILES =  socket
 TEST_GEN_FILES += psock_fanout psock_tpacket
 TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
-TEST_GEN_FILES += reuseport_dualstack
+TEST_GEN_FILES += reuseport_dualstack msg_zerocopy
 
 include ../lib.mk
 
diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
new file mode 100644
index 000000000000..448c69a8af74
--- /dev/null
+++ b/tools/testing/selftests/net/msg_zerocopy.c
@@ -0,0 +1,697 @@
+/* Evaluate MSG_ZEROCOPY
+ *
+ * Send traffic between two processes over one of the supported
+ * protocols and modes:
+ *
+ * PF_INET/PF_INET6
+ * - SOCK_STREAM
+ * - SOCK_DGRAM
+ * - SOCK_DGRAM with UDP_CORK
+ * - SOCK_RAW
+ * - SOCK_RAW with IP_HDRINCL
+ *
+ * PF_PACKET
+ * - SOCK_DGRAM
+ * - SOCK_RAW
+ *
+ * Start this program on two connected hosts, one in send mode and
+ * the other with option '-r' to put it in receiver mode.
+ *
+ * If zerocopy mode ('-z') is enabled, the sender will verify that
+ * the kernel queues completions on the error queue for all zerocopy
+ * transfers.
+ */
+
+#define _GNU_SOURCE
+
+#include <arpa/inet.h>
+#include <error.h>
+#include <errno.h>
+#include <limits.h>
+#include <linux/errqueue.h>
+#include <linux/if_packet.h>
+#include <linux/ipv6.h>
+#include <linux/socket.h>
+#include <linux/sockios.h>
+#include <net/ethernet.h>
+#include <net/if.h>
+#include <netinet/ip.h>
+#include <netinet/ip6.h>
+#include <netinet/tcp.h>
+#include <netinet/udp.h>
+#include <poll.h>
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#ifndef SO_EE_ORIGIN_ZEROCOPY
+#define SO_EE_ORIGIN_ZEROCOPY		SO_EE_ORIGIN_UPAGE
+#endif
+
+#ifndef SO_ZEROCOPY
+#define SO_ZEROCOPY	59
+#endif
+
+#ifndef SO_EE_CODE_ZEROCOPY_COPIED
+#define SO_EE_CODE_ZEROCOPY_COPIED	1
+#endif
+
+#ifndef MSG_ZEROCOPY
+#define MSG_ZEROCOPY	0x4000000
+#endif
+
+static int  cfg_cork;
+static bool cfg_cork_mixed;
+static int  cfg_cpu		= -1;		/* default: pin to last cpu */
+static int  cfg_family		= PF_UNSPEC;
+static int  cfg_ifindex		= 1;
+static int  cfg_payload_len;
+static int  cfg_port		= 8000;
+static bool cfg_rx;
+static int  cfg_runtime_ms	= 4200;
+static int  cfg_verbose;
+static int  cfg_waittime_ms	= 500;
+static bool cfg_zerocopy;
+
+static socklen_t cfg_alen;
+static struct sockaddr_storage cfg_dst_addr;
+static struct sockaddr_storage cfg_src_addr;
+
+static char payload[IP_MAXPACKET];
+static long packets, bytes, completions, expected_completions;
+static int  zerocopied = -1;
+static uint32_t next_completion;
+
+static unsigned long gettimeofday_ms(void)
+{
+	struct timeval tv;
+
+	gettimeofday(&tv, NULL);
+	return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
+}
+
+static uint16_t get_ip_csum(const uint16_t *start, int num_words)
+{
+	unsigned long sum = 0;
+	int i;
+
+	for (i = 0; i < num_words; i++)
+		sum += start[i];
+
+	while (sum >> 16)
+		sum = (sum & 0xFFFF) + (sum >> 16);
+
+	return ~sum;
+}
+
+static int do_setcpu(int cpu)
+{
+	cpu_set_t mask;
+
+	CPU_ZERO(&mask);
+	CPU_SET(cpu, &mask);
+	if (sched_setaffinity(0, sizeof(mask), &mask))
+		error(1, 0, "setaffinity %d", cpu);
+
+	if (cfg_verbose)
+		fprintf(stderr, "cpu: %u\n", cpu);
+
+	return 0;
+}
+
+static void do_setsockopt(int fd, int level, int optname, int val)
+{
+	if (setsockopt(fd, level, optname, &val, sizeof(val)))
+		error(1, errno, "setsockopt %d.%d: %d", level, optname, val);
+}
+
+static int do_poll(int fd, int events)
+{
+	struct pollfd pfd;
+	int ret;
+
+	pfd.events = events;
+	pfd.revents = 0;
+	pfd.fd = fd;
+
+	ret = poll(&pfd, 1, cfg_waittime_ms);
+	if (ret == -1)
+		error(1, errno, "poll");
+
+	return ret && (pfd.revents & events);
+}
+
+static int do_accept(int fd)
+{
+	int fda = fd;
+
+	fd = accept(fda, NULL, NULL);
+	if (fd == -1)
+		error(1, errno, "accept");
+	if (close(fda))
+		error(1, errno, "close listen sock");
+
+	return fd;
+}
+
+static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy)
+{
+	int ret, len, i, flags;
+
+	len = 0;
+	for (i = 0; i < msg->msg_iovlen; i++)
+		len += msg->msg_iov[i].iov_len;
+
+	flags = MSG_DONTWAIT;
+	if (do_zerocopy)
+		flags |= MSG_ZEROCOPY;
+
+	ret = sendmsg(fd, msg, flags);
+	if (ret == -1 && errno == EAGAIN)
+		return false;
+	if (ret == -1)
+		error(1, errno, "send");
+	if (cfg_verbose && ret != len)
+		fprintf(stderr, "send: ret=%u != %u\n", ret, len);
+
+	if (len) {
+		packets++;
+		bytes += ret;
+		if (do_zerocopy && ret)
+			expected_completions++;
+	}
+
+	return true;
+}
+
+static void do_sendmsg_corked(int fd, struct msghdr *msg)
+{
+	bool do_zerocopy = cfg_zerocopy;
+	int i, payload_len, extra_len;
+
+	/* split up the packet. for non-multiple, make first buffer longer */
+	payload_len = cfg_payload_len / cfg_cork;
+	extra_len = cfg_payload_len - (cfg_cork * payload_len);
+
+	do_setsockopt(fd, IPPROTO_UDP, UDP_CORK, 1);
+
+	for (i = 0; i < cfg_cork; i++) {
+
+		/* in mixed-frags mode, alternate zerocopy and copy frags
+		 * start with non-zerocopy, to ensure attach later works
+		 */
+		if (cfg_cork_mixed)
+			do_zerocopy = (i & 1);
+
+		msg->msg_iov[0].iov_len = payload_len + extra_len;
+		extra_len = 0;
+
+		do_sendmsg(fd, msg, do_zerocopy);
+	}
+
+	do_setsockopt(fd, IPPROTO_UDP, UDP_CORK, 0);
+}
+
+static int setup_iph(struct iphdr *iph, uint16_t payload_len)
+{
+	struct sockaddr_in *daddr = (void *) &cfg_dst_addr;
+	struct sockaddr_in *saddr = (void *) &cfg_src_addr;
+
+	memset(iph, 0, sizeof(*iph));
+
+	iph->version	= 4;
+	iph->tos	= 0;
+	iph->ihl	= 5;
+	iph->ttl	= 2;
+	iph->saddr	= saddr->sin_addr.s_addr;
+	iph->daddr	= daddr->sin_addr.s_addr;
+	iph->protocol	= IPPROTO_EGP;
+	iph->tot_len	= htons(sizeof(*iph) + payload_len);
+	iph->check	= get_ip_csum((void *) iph, iph->ihl << 1);
+
+	return sizeof(*iph);
+}
+
+static int setup_ip6h(struct ipv6hdr *ip6h, uint16_t payload_len)
+{
+	struct sockaddr_in6 *daddr = (void *) &cfg_dst_addr;
+	struct sockaddr_in6 *saddr = (void *) &cfg_src_addr;
+
+	memset(ip6h, 0, sizeof(*ip6h));
+
+	ip6h->version		= 6;
+	ip6h->payload_len	= htons(payload_len);
+	ip6h->nexthdr		= IPPROTO_EGP;
+	ip6h->hop_limit		= 2;
+	ip6h->saddr		= saddr->sin6_addr;
+	ip6h->daddr		= daddr->sin6_addr;
+
+	return sizeof(*ip6h);
+}
+
+static void setup_sockaddr(int domain, const char *str_addr, void *sockaddr)
+{
+	struct sockaddr_in6 *addr6 = (void *) sockaddr;
+	struct sockaddr_in *addr4 = (void *) sockaddr;
+
+	switch (domain) {
+	case PF_INET:
+		addr4->sin_family = AF_INET;
+		addr4->sin_port = htons(cfg_port);
+		if (inet_pton(AF_INET, str_addr, &(addr4->sin_addr)) != 1)
+			error(1, 0, "ipv4 parse error: %s", str_addr);
+		break;
+	case PF_INET6:
+		addr6->sin6_family = AF_INET6;
+		addr6->sin6_port = htons(cfg_port);
+		if (inet_pton(AF_INET6, str_addr, &(addr6->sin6_addr)) != 1)
+			error(1, 0, "ipv6 parse error: %s", str_addr);
+		break;
+	default:
+		error(1, 0, "illegal domain");
+	}
+}
+
+static int do_setup_tx(int domain, int type, int protocol)
+{
+	int fd;
+
+	fd = socket(domain, type, protocol);
+	if (fd == -1)
+		error(1, errno, "socket t");
+
+	do_setsockopt(fd, SOL_SOCKET, SO_SNDBUF, 1 << 21);
+	if (cfg_zerocopy)
+		do_setsockopt(fd, SOL_SOCKET, SO_ZEROCOPY, 1);
+
+	if (domain != PF_PACKET)
+		if (connect(fd, (void *) &cfg_dst_addr, cfg_alen))
+			error(1, errno, "connect");
+
+	return fd;
+}
+
+static bool do_recv_completion(int fd)
+{
+	struct sock_extended_err *serr;
+	struct msghdr msg = {};
+	struct cmsghdr *cm;
+	uint32_t hi, lo, range;
+	int ret, zerocopy;
+	char control[100];
+
+	msg.msg_control = control;
+	msg.msg_controllen = sizeof(control);
+
+	ret = recvmsg(fd, &msg, MSG_ERRQUEUE);
+	if (ret == -1 && errno == EAGAIN)
+		return false;
+	if (ret == -1)
+		error(1, errno, "recvmsg notification");
+	if (msg.msg_flags & MSG_CTRUNC)
+		error(1, errno, "recvmsg notification: truncated");
+
+	cm = CMSG_FIRSTHDR(&msg);
+	if (!cm)
+		error(1, 0, "cmsg: no cmsg");
+	if (!((cm->cmsg_level == SOL_IP && cm->cmsg_type == IP_RECVERR) ||
+	      (cm->cmsg_level == SOL_IPV6 && cm->cmsg_type == IPV6_RECVERR) ||
+	      (cm->cmsg_level == SOL_PACKET && cm->cmsg_type == PACKET_TX_TIMESTAMP)))
+		error(1, 0, "serr: wrong type: %d.%d",
+		      cm->cmsg_level, cm->cmsg_type);
+
+	serr = (void *) CMSG_DATA(cm);
+	if (serr->ee_origin != SO_EE_ORIGIN_ZEROCOPY)
+		error(1, 0, "serr: wrong origin: %u", serr->ee_origin);
+	if (serr->ee_errno != 0)
+		error(1, 0, "serr: wrong error code: %u", serr->ee_errno);
+
+	hi = serr->ee_data;
+	lo = serr->ee_info;
+	range = hi - lo + 1;
+
+	/* Detect notification gaps. These should not happen often, if at all.
+	 * Gaps can occur due to drops, reordering and retransmissions.
+	 */
+	if (lo != next_completion)
+		fprintf(stderr, "gap: %u..%u does not append to %u\n",
+			lo, hi, next_completion);
+	next_completion = hi + 1;
+
+	zerocopy = !(serr->ee_code & SO_EE_CODE_ZEROCOPY_COPIED);
+	if (zerocopied == -1)
+		zerocopied = zerocopy;
+	else if (zerocopied != zerocopy) {
+		fprintf(stderr, "serr: inconsistent\n");
+		zerocopied = zerocopy;
+	}
+
+	if (cfg_verbose >= 2)
+		fprintf(stderr, "completed: %u (h=%u l=%u)\n",
+			range, hi, lo);
+
+	completions += range;
+	return true;
+}
+
+/* Read all outstanding messages on the errqueue */
+static void do_recv_completions(int fd)
+{
+	while (do_recv_completion(fd)) {}
+}
+
+/* Wait for all remaining completions on the errqueue */
+static void do_recv_remaining_completions(int fd)
+{
+	int64_t tstop = gettimeofday_ms() + cfg_waittime_ms;
+
+	while (completions < expected_completions &&
+	       gettimeofday_ms() < tstop) {
+		if (do_poll(fd, POLLERR))
+			do_recv_completions(fd);
+	}
+
+	if (completions < expected_completions)
+		error(1, 0, "missing notifications: %lu < %lu\n",
+		      completions, expected_completions);
+}
+
+static void do_tx(int domain, int type, int protocol)
+{
+	struct iovec iov[3] = { {0} };
+	struct sockaddr_ll laddr;
+	struct msghdr msg = {0};
+	struct ethhdr eth;
+	union {
+		struct ipv6hdr ip6h;
+		struct iphdr iph;
+	} nh;
+	uint64_t tstop;
+	int fd;
+
+	fd = do_setup_tx(domain, type, protocol);
+
+	if (domain == PF_PACKET) {
+		uint16_t proto = cfg_family == PF_INET ? ETH_P_IP : ETH_P_IPV6;
+
+		/* sock_raw passes ll header as data */
+		if (type == SOCK_RAW) {
+			memset(eth.h_dest, 0x06, ETH_ALEN);
+			memset(eth.h_source, 0x02, ETH_ALEN);
+			eth.h_proto = htons(proto);
+			iov[0].iov_base = &eth;
+			iov[0].iov_len = sizeof(eth);
+			msg.msg_iovlen++;
+		}
+
+		/* both sock_raw and sock_dgram expect name */
+		memset(&laddr, 0, sizeof(laddr));
+		laddr.sll_family	= AF_PACKET;
+		laddr.sll_ifindex	= cfg_ifindex;
+		laddr.sll_protocol	= htons(proto);
+		laddr.sll_halen		= ETH_ALEN;
+
+		memset(laddr.sll_addr, 0x06, ETH_ALEN);
+
+		msg.msg_name		= &laddr;
+		msg.msg_namelen		= sizeof(laddr);
+	}
+
+	/* packet and raw sockets with hdrincl must pass network header */
+	if (domain == PF_PACKET || protocol == IPPROTO_RAW) {
+		if (cfg_family == PF_INET)
+			iov[1].iov_len = setup_iph(&nh.iph, cfg_payload_len);
+		else
+			iov[1].iov_len = setup_ip6h(&nh.ip6h, cfg_payload_len);
+
+		iov[1].iov_base = (void *) &nh;
+		msg.msg_iovlen++;
+	}
+
+	iov[2].iov_base = payload;
+	iov[2].iov_len = cfg_payload_len;
+	msg.msg_iovlen++;
+	msg.msg_iov = &iov[3 - msg.msg_iovlen];
+
+	tstop = gettimeofday_ms() + cfg_runtime_ms;
+	do {
+		if (cfg_cork)
+			do_sendmsg_corked(fd, &msg);
+		else
+			do_sendmsg(fd, &msg, cfg_zerocopy);
+
+		while (!do_poll(fd, POLLOUT)) {
+			if (cfg_zerocopy)
+				do_recv_completions(fd);
+		}
+
+	} while (gettimeofday_ms() < tstop);
+
+	if (cfg_zerocopy)
+		do_recv_remaining_completions(fd);
+
+	if (close(fd))
+		error(1, errno, "close");
+
+	fprintf(stderr, "tx=%lu (%lu MB) txc=%lu zc=%c\n",
+		packets, bytes >> 20, completions,
+		zerocopied == 1 ? 'y' : 'n');
+}
+
+static int do_setup_rx(int domain, int type, int protocol)
+{
+	int fd;
+
+	/* If tx over PF_PACKET, rx over PF_INET(6)/SOCK_RAW,
+	 * to recv the only copy of the packet, not a clone
+	 */
+	if (domain == PF_PACKET)
+		error(1, 0, "Use PF_INET/SOCK_RAW to read");
+
+	if (type == SOCK_RAW && protocol == IPPROTO_RAW)
+		error(1, 0, "IPPROTO_RAW: not supported on Rx");
+
+	fd = socket(domain, type, protocol);
+	if (fd == -1)
+		error(1, errno, "socket r");
+
+	do_setsockopt(fd, SOL_SOCKET, SO_RCVBUF, 1 << 21);
+	do_setsockopt(fd, SOL_SOCKET, SO_RCVLOWAT, 1 << 16);
+	do_setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, 1);
+
+	if (bind(fd, (void *) &cfg_dst_addr, cfg_alen))
+		error(1, errno, "bind");
+
+	if (type == SOCK_STREAM) {
+		if (listen(fd, 1))
+			error(1, errno, "listen");
+		fd = do_accept(fd);
+	}
+
+	return fd;
+}
+
+/* Flush all outstanding bytes for the tcp receive queue */
+static void do_flush_tcp(int fd)
+{
+	int ret;
+
+	/* MSG_TRUNC flushes up to len bytes */
+	ret = recv(fd, NULL, 1 << 21, MSG_TRUNC | MSG_DONTWAIT);
+	if (ret == -1 && errno == EAGAIN)
+		return;
+	if (ret == -1)
+		error(1, errno, "flush");
+	if (!ret)
+		return;
+
+	packets++;
+	bytes += ret;
+}
+
+/* Flush all outstanding datagrams. Verify first few bytes of each. */
+static void do_flush_datagram(int fd, int type)
+{
+	int ret, off = 0;
+	char buf[64];
+
+	/* MSG_TRUNC will return full datagram length */
+	ret = recv(fd, buf, sizeof(buf), MSG_DONTWAIT | MSG_TRUNC);
+	if (ret == -1 && errno == EAGAIN)
+		return;
+
+	/* raw ipv4 return with header, raw ipv6 without */
+	if (cfg_family == PF_INET && type == SOCK_RAW) {
+		off += sizeof(struct iphdr);
+		ret -= sizeof(struct iphdr);
+	}
+
+	if (ret == -1)
+		error(1, errno, "recv");
+	if (ret != cfg_payload_len)
+		error(1, 0, "recv: ret=%u != %u", ret, cfg_payload_len);
+	if (ret > sizeof(buf) - off)
+		ret = sizeof(buf) - off;
+	if (memcmp(buf + off, payload, ret))
+		error(1, 0, "recv: data mismatch");
+
+	packets++;
+	bytes += cfg_payload_len;
+}
+
+static void do_rx(int domain, int type, int protocol)
+{
+	uint64_t tstop;
+	int fd;
+
+	fd = do_setup_rx(domain, type, protocol);
+
+	tstop = gettimeofday_ms() + cfg_runtime_ms;
+	do {
+		if (type == SOCK_STREAM)
+			do_flush_tcp(fd);
+		else
+			do_flush_datagram(fd, type);
+
+		do_poll(fd, POLLIN);
+
+	} while (gettimeofday_ms() < tstop);
+
+	if (close(fd))
+		error(1, errno, "close");
+
+	fprintf(stderr, "rx=%lu (%lu MB)\n", packets, bytes >> 20);
+}
+
+static void do_test(int domain, int type, int protocol)
+{
+	int i;
+
+	if (cfg_cork && (domain == PF_PACKET || type != SOCK_DGRAM))
+		error(1, 0, "can only cork udp sockets");
+
+	do_setcpu(cfg_cpu);
+
+	for (i = 0; i < IP_MAXPACKET; i++)
+		payload[i] = 'a' + (i % 26);
+
+	if (cfg_rx)
+		do_rx(domain, type, protocol);
+	else
+		do_tx(domain, type, protocol);
+}
+
+static void usage(const char *filepath)
+{
+	error(1, 0, "Usage: %s [options] <test>", filepath);
+}
+
+static void parse_opts(int argc, char **argv)
+{
+	const int max_payload_len = sizeof(payload) -
+				    sizeof(struct ipv6hdr) -
+				    sizeof(struct tcphdr) -
+				    40 /* max tcp options */;
+	int c;
+
+	cfg_payload_len = max_payload_len;
+
+	while ((c = getopt(argc, argv, "46c:C:D:i:mp:rs:S:t:vz")) != -1) {
+		switch (c) {
+		case '4':
+			if (cfg_family != PF_UNSPEC)
+				error(1, 0, "Pass one of -4 or -6");
+			cfg_family = PF_INET;
+			cfg_alen = sizeof(struct sockaddr_in);
+			break;
+		case '6':
+			if (cfg_family != PF_UNSPEC)
+				error(1, 0, "Pass one of -4 or -6");
+			cfg_family = PF_INET6;
+			cfg_alen = sizeof(struct sockaddr_in6);
+			break;
+		case 'c':
+			cfg_cork = strtol(optarg, NULL, 0);
+			break;
+		case 'C':
+			cfg_cpu = strtol(optarg, NULL, 0);
+			break;
+		case 'D':
+			setup_sockaddr(cfg_family, optarg, &cfg_dst_addr);
+			break;
+		case 'i':
+			cfg_ifindex = if_nametoindex(optarg);
+			if (cfg_ifindex == 0)
+				error(1, errno, "invalid iface: %s", optarg);
+			break;
+		case 'm':
+			cfg_cork_mixed = true;
+			break;
+		case 'p':
+			cfg_port = htons(strtoul(optarg, NULL, 0));
+			break;
+		case 'r':
+			cfg_rx = true;
+			break;
+		case 's':
+			cfg_payload_len = strtoul(optarg, NULL, 0);
+			break;
+		case 'S':
+			setup_sockaddr(cfg_family, optarg, &cfg_src_addr);
+			break;
+		case 't':
+			cfg_runtime_ms = 200 + strtoul(optarg, NULL, 10) * 1000;
+			break;
+		case 'v':
+			cfg_verbose++;
+			break;
+		case 'z':
+			cfg_zerocopy = true;
+			break;
+		}
+	}
+
+	if (cfg_payload_len > max_payload_len)
+		error(1, 0, "-s: payload exceeds max (%d)", max_payload_len);
+	if (cfg_cork_mixed && (!cfg_zerocopy || !cfg_cork))
+		error(1, 0, "-m: cork_mixed requires corking and zerocopy");
+
+	if (optind != argc - 1)
+		usage(argv[0]);
+}
+
+int main(int argc, char **argv)
+{
+	const char *cfg_test;
+
+	parse_opts(argc, argv);
+
+	cfg_test = argv[argc - 1];
+
+	if (!strcmp(cfg_test, "packet"))
+		do_test(PF_PACKET, SOCK_RAW, 0);
+	else if (!strcmp(cfg_test, "packet_dgram"))
+		do_test(PF_PACKET, SOCK_DGRAM, 0);
+	else if (!strcmp(cfg_test, "raw"))
+		do_test(cfg_family, SOCK_RAW, IPPROTO_EGP);
+	else if (!strcmp(cfg_test, "raw_hdrincl"))
+		do_test(cfg_family, SOCK_RAW, IPPROTO_RAW);
+	else if (!strcmp(cfg_test, "tcp"))
+		do_test(cfg_family, SOCK_STREAM, 0);
+	else if (!strcmp(cfg_test, "udp"))
+		do_test(cfg_family, SOCK_DGRAM, 0);
+	else
+		error(1, 0, "unknown cfg_test %s", cfg_test);
+
+	return 0;
+}
diff --git a/tools/testing/selftests/net/msg_zerocopy.sh b/tools/testing/selftests/net/msg_zerocopy.sh
new file mode 100755
index 000000000000..86955e9a6a7c
--- /dev/null
+++ b/tools/testing/selftests/net/msg_zerocopy.sh
@@ -0,0 +1,113 @@
+#!/bin/bash
+#
+# Send data between two processes across namespaces
+# Run twice: once without and once with zerocopy
+
+set -e
+
+readonly DEV="veth0"
+readonly DEV_MTU=65535
+readonly BIN="./msg_zerocopy"
+
+readonly RAND="$(mktemp -u XXXXXX)"
+readonly NSPREFIX="ns-${RAND}"
+readonly NS1="${NSPREFIX}1"
+readonly NS2="${NSPREFIX}2"
+
+readonly SADDR4='192.168.1.1'
+readonly DADDR4='192.168.1.2'
+readonly SADDR6='fd::1'
+readonly DADDR6='fd::2'
+
+readonly path_sysctl_mem="net.core.optmem_max"
+
+# Argument parsing
+if [[ "$#" -lt "2" ]]; then
+	echo "Usage: $0 [4|6] [tcp|udp|raw|raw_hdrincl|packet|packet_dgram] <args>"
+	exit 1
+fi
+
+readonly IP="$1"
+shift
+readonly TXMODE="$1"
+shift
+readonly EXTRA_ARGS="$@"
+
+# Argument parsing: configure addresses
+if [[ "${IP}" == "4" ]]; then
+	readonly SADDR="${SADDR4}"
+	readonly DADDR="${DADDR4}"
+elif [[ "${IP}" == "6" ]]; then
+	readonly SADDR="${SADDR6}"
+	readonly DADDR="${DADDR6}"
+else
+	echo "Invalid IP version ${IP}"
+	exit 1
+fi
+
+# Argument parsing: select receive mode
+#
+# This differs from send mode for
+# - packet:	use raw recv, because packet receives skb clones
+# - raw_hdrinc: use raw recv, because hdrincl is a tx-only option
+case "${TXMODE}" in
+'packet' | 'packet_dgram' | 'raw_hdrincl')
+	RXMODE='raw'
+	;;
+*)
+	RXMODE="${TXMODE}"
+	;;
+esac
+
+# Start of state changes: install cleanup handler
+save_sysctl_mem="$(sysctl -n ${path_sysctl_mem})"
+
+cleanup() {
+	ip netns del "${NS2}"
+	ip netns del "${NS1}"
+	sysctl -w -q "${path_sysctl_mem}=${save_sysctl_mem}"
+}
+
+trap cleanup EXIT
+
+# Configure system settings
+sysctl -w -q "${path_sysctl_mem}=1000000"
+
+# Create virtual ethernet pair between network namespaces
+ip netns add "${NS1}"
+ip netns add "${NS2}"
+
+ip link add "${DEV}" mtu "${DEV_MTU}" netns "${NS1}" type veth \
+  peer name "${DEV}" mtu "${DEV_MTU}" netns "${NS2}"
+
+# Bring the devices up
+ip -netns "${NS1}" link set "${DEV}" up
+ip -netns "${NS2}" link set "${DEV}" up
+
+# Set fixed MAC addresses on the devices
+ip -netns "${NS1}" link set dev "${DEV}" address 02:02:02:02:02:02
+ip -netns "${NS2}" link set dev "${DEV}" address 06:06:06:06:06:06
+
+# Add fixed IP addresses to the devices
+ip -netns "${NS1}" addr add 192.168.1.1/24 dev "${DEV}"
+ip -netns "${NS2}" addr add 192.168.1.2/24 dev "${DEV}"
+ip -netns "${NS1}" addr add       fd::1/64 dev "${DEV}" nodad
+ip -netns "${NS2}" addr add       fd::2/64 dev "${DEV}" nodad
+
+# Optionally disable sg or csum offload to test edge cases
+# ip netns exec "${NS1}" ethtool -K "${DEV}" sg off
+
+do_test() {
+	local readonly ARGS="$1"
+
+	echo "ipv${IP} ${TXMODE} ${ARGS}"
+	ip netns exec "${NS2}" "${BIN}" "-${IP}" -i "${DEV}" -t 2 -C 2 -S "${SADDR}" -D "${DADDR}" ${ARGS} -r "${RXMODE}" &
+	sleep 0.2
+	ip netns exec "${NS1}" "${BIN}" "-${IP}" -i "${DEV}" -t 1 -C 3 -S "${SADDR}" -D "${DADDR}" ${ARGS} "${TXMODE}"
+	wait
+}
+
+do_test "${EXTRA_ARGS}"
+do_test "-z ${EXTRA_ARGS}"
+echo ok
+
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 3/9] sock: add MSG_ZEROCOPY
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-api, Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemb@google.com>

The kernel supports zerocopy sendmsg in virtio and tap. Expand the
infrastructure to support other socket types. Introduce a completion
notification channel over the socket error queue. Notifications are
returned with ee_origin SO_EE_ORIGIN_ZEROCOPY. ee_errno is 0 to avoid
blocking the send/recv path on receiving notifications.

Add reference counting, to support the skb split, merge, resize and
clone operations possible with SOCK_STREAM and other socket types.

The patch does not yet modify any datapaths.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h        |  60 +++++++++++++++++++
 include/linux/socket.h        |   1 +
 include/net/sock.h            |   2 +
 include/uapi/linux/errqueue.h |   3 +
 net/core/datagram.c           |  55 ++++++++++-------
 net/core/skbuff.c             | 133 ++++++++++++++++++++++++++++++++++++++++++
 net/core/sock.c               |   2 +
 7 files changed, 235 insertions(+), 21 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2f64e2bbb592..59cff7aa494e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -429,6 +429,7 @@ enum {
 	SKBTX_SCHED_TSTAMP = 1 << 6,
 };
 
+#define SKBTX_ZEROCOPY_FRAG	(SKBTX_DEV_ZEROCOPY | SKBTX_SHARED_FRAG)
 #define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP    | \
 				 SKBTX_SCHED_TSTAMP)
 #define SKBTX_ANY_TSTAMP	(SKBTX_HW_TSTAMP | SKBTX_ANY_SW_TSTAMP)
@@ -445,8 +446,28 @@ struct ubuf_info {
 	void (*callback)(struct ubuf_info *, bool zerocopy_success);
 	void *ctx;
 	unsigned long desc;
+	u16 zerocopy:1;
+	atomic_t refcnt;
 };
 
+#define skb_uarg(SKB)	((struct ubuf_info *)(skb_shinfo(SKB)->destructor_arg))
+
+struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size);
+
+static inline void sock_zerocopy_get(struct ubuf_info *uarg)
+{
+	atomic_inc(&uarg->refcnt);
+}
+
+void sock_zerocopy_put(struct ubuf_info *uarg);
+void sock_zerocopy_put_abort(struct ubuf_info *uarg);
+
+void sock_zerocopy_callback(struct ubuf_info *uarg, bool success);
+
+int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
+			     struct msghdr *msg, int len,
+			     struct ubuf_info *uarg);
+
 /* This data is invariant across clones and lives at
  * the end of the header data, ie. at skb->end.
  */
@@ -1214,6 +1235,45 @@ static inline struct skb_shared_hwtstamps *skb_hwtstamps(struct sk_buff *skb)
 	return &skb_shinfo(skb)->hwtstamps;
 }
 
+static inline struct ubuf_info *skb_zcopy(struct sk_buff *skb)
+{
+	bool is_zcopy = skb && skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY;
+
+	return is_zcopy ? skb_uarg(skb) : NULL;
+}
+
+static inline void skb_zcopy_set(struct sk_buff *skb, struct ubuf_info *uarg)
+{
+	if (skb && uarg && !skb_zcopy(skb)) {
+		sock_zerocopy_get(uarg);
+		skb_shinfo(skb)->destructor_arg = uarg;
+		skb_shinfo(skb)->tx_flags |= SKBTX_ZEROCOPY_FRAG;
+	}
+}
+
+/* Release a reference on a zerocopy structure */
+static inline void skb_zcopy_clear(struct sk_buff *skb, bool zerocopy)
+{
+	struct ubuf_info *uarg = skb_zcopy(skb);
+
+	if (uarg) {
+		uarg->zerocopy = uarg->zerocopy && zerocopy;
+		sock_zerocopy_put(uarg);
+		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
+	}
+}
+
+/* Abort a zerocopy operation and revert zckey on error in send syscall */
+static inline void skb_zcopy_abort(struct sk_buff *skb)
+{
+	struct ubuf_info *uarg = skb_zcopy(skb);
+
+	if (uarg) {
+		sock_zerocopy_put_abort(uarg);
+		skb_shinfo(skb)->tx_flags &= ~SKBTX_ZEROCOPY_FRAG;
+	}
+}
+
 /**
  *	skb_queue_empty - check if a queue is empty
  *	@list: queue head
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8b13db5163cc..8ad963cdc88c 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -287,6 +287,7 @@ struct ucred {
 #define MSG_BATCH	0x40000 /* sendmmsg(): more messages coming */
 #define MSG_EOF         MSG_FIN
 
+#define MSG_ZEROCOPY	0x4000000	/* Use user data in kernel path */
 #define MSG_FASTOPEN	0x20000000	/* Send data in TCP SYN */
 #define MSG_CMSG_CLOEXEC 0x40000000	/* Set close_on_exec for file
 					   descriptor received through
diff --git a/include/net/sock.h b/include/net/sock.h
index 0f778d3c4300..fe1a0bc25cd3 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -294,6 +294,7 @@ struct sock_common {
   *	@sk_stamp: time stamp of last packet received
   *	@sk_tsflags: SO_TIMESTAMPING socket options
   *	@sk_tskey: counter to disambiguate concurrent tstamp requests
+  *	@sk_zckey: counter to order MSG_ZEROCOPY notifications
   *	@sk_socket: Identd and reporting IO signals
   *	@sk_user_data: RPC layer private data
   *	@sk_frag: cached page frag
@@ -462,6 +463,7 @@ struct sock {
 	u16			sk_tsflags;
 	u8			sk_shutdown;
 	u32			sk_tskey;
+	atomic_t		sk_zckey;
 	struct socket		*sk_socket;
 	void			*sk_user_data;
 #ifdef CONFIG_SECURITY
diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index 07bdce1f444a..78fdf52d6b2f 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -18,10 +18,13 @@ struct sock_extended_err {
 #define SO_EE_ORIGIN_ICMP	2
 #define SO_EE_ORIGIN_ICMP6	3
 #define SO_EE_ORIGIN_TXSTATUS	4
+#define SO_EE_ORIGIN_ZEROCOPY	5
 #define SO_EE_ORIGIN_TIMESTAMPING SO_EE_ORIGIN_TXSTATUS
 
 #define SO_EE_OFFENDER(ee)	((struct sockaddr*)((ee)+1))
 
+#define SO_EE_CODE_ZEROCOPY_COPIED	1
+
 /**
  *	struct scm_timestamping - timestamps exposed through cmsg
  *
diff --git a/net/core/datagram.c b/net/core/datagram.c
index ee5647bd91b3..2f3277945d35 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -573,27 +573,12 @@ int skb_copy_datagram_from_iter(struct sk_buff *skb, int offset,
 }
 EXPORT_SYMBOL(skb_copy_datagram_from_iter);
 
-/**
- *	zerocopy_sg_from_iter - Build a zerocopy datagram from an iov_iter
- *	@skb: buffer to copy
- *	@from: the source to copy from
- *
- *	The function will first copy up to headlen, and then pin the userspace
- *	pages and build frags through them.
- *
- *	Returns 0, -EFAULT or -EMSGSIZE.
- */
-int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
+int __zerocopy_sg_from_iter(struct sock *sk, struct sk_buff *skb,
+			    struct iov_iter *from, size_t length)
 {
-	int len = iov_iter_count(from);
-	int copy = min_t(int, skb_headlen(skb), len);
-	int frag = 0;
+	int frag = skb_shinfo(skb)->nr_frags;
 
-	/* copy up to skb headlen */
-	if (skb_copy_datagram_from_iter(skb, 0, from, copy))
-		return -EFAULT;
-
-	while (iov_iter_count(from)) {
+	while (length && iov_iter_count(from)) {
 		struct page *pages[MAX_SKB_FRAGS];
 		size_t start;
 		ssize_t copied;
@@ -603,18 +588,24 @@ int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
 		if (frag == MAX_SKB_FRAGS)
 			return -EMSGSIZE;
 
-		copied = iov_iter_get_pages(from, pages, ~0U,
+		copied = iov_iter_get_pages(from, pages, length,
 					    MAX_SKB_FRAGS - frag, &start);
 		if (copied < 0)
 			return -EFAULT;
 
 		iov_iter_advance(from, copied);
+		length -= copied;
 
 		truesize = PAGE_ALIGN(copied + start);
 		skb->data_len += copied;
 		skb->len += copied;
 		skb->truesize += truesize;
-		refcount_add(truesize, &skb->sk->sk_wmem_alloc);
+		if (sk && sk->sk_type == SOCK_STREAM) {
+			sk->sk_wmem_queued += truesize;
+			sk_mem_charge(sk, truesize);
+		} else {
+			refcount_add(truesize, &skb->sk->sk_wmem_alloc);
+		}
 		while (copied) {
 			int size = min_t(int, copied, PAGE_SIZE - start);
 			skb_fill_page_desc(skb, frag++, pages[n], start, size);
@@ -625,6 +616,28 @@ int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(__zerocopy_sg_from_iter);
+
+/**
+ *	zerocopy_sg_from_iter - Build a zerocopy datagram from an iov_iter
+ *	@skb: buffer to copy
+ *	@from: the source to copy from
+ *
+ *	The function will first copy up to headlen, and then pin the userspace
+ *	pages and build frags through them.
+ *
+ *	Returns 0, -EFAULT or -EMSGSIZE.
+ */
+int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
+{
+	int copy = min_t(int, skb_headlen(skb), iov_iter_count(from));
+
+	/* copy up to skb headlen */
+	if (skb_copy_datagram_from_iter(skb, 0, from, copy))
+		return -EFAULT;
+
+	return __zerocopy_sg_from_iter(NULL, skb, from, ~0U);
+}
 EXPORT_SYMBOL(zerocopy_sg_from_iter);
 
 static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a95877a8ac8b..0603e44950da 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -915,6 +915,139 @@ struct sk_buff *skb_morph(struct sk_buff *dst, struct sk_buff *src)
 }
 EXPORT_SYMBOL_GPL(skb_morph);
 
+struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
+{
+	struct ubuf_info *uarg;
+	struct sk_buff *skb;
+
+	WARN_ON_ONCE(!in_task());
+
+	skb = sock_omalloc(sk, 0, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	BUILD_BUG_ON(sizeof(*uarg) > sizeof(skb->cb));
+	uarg = (void *)skb->cb;
+
+	uarg->callback = sock_zerocopy_callback;
+	uarg->desc = atomic_inc_return(&sk->sk_zckey) - 1;
+	uarg->zerocopy = 1;
+	atomic_set(&uarg->refcnt, 0);
+	sock_hold(sk);
+
+	return uarg;
+}
+EXPORT_SYMBOL_GPL(sock_zerocopy_alloc);
+
+static inline struct sk_buff *skb_from_uarg(struct ubuf_info *uarg)
+{
+	return container_of((void *)uarg, struct sk_buff, cb);
+}
+
+void sock_zerocopy_callback(struct ubuf_info *uarg, bool success)
+{
+	struct sk_buff *skb = skb_from_uarg(uarg);
+	struct sock_exterr_skb *serr;
+	struct sock *sk = skb->sk;
+	u16 id = uarg->desc;
+
+	if (sock_flag(sk, SOCK_DEAD))
+		goto release;
+
+	serr = SKB_EXT_ERR(skb);
+	memset(serr, 0, sizeof(*serr));
+	serr->ee.ee_errno = 0;
+	serr->ee.ee_origin = SO_EE_ORIGIN_ZEROCOPY;
+	serr->ee.ee_data = id;
+	if (!success)
+		serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;
+
+	skb_queue_tail(&sk->sk_error_queue, skb);
+	skb = NULL;
+
+	sk->sk_error_report(sk);
+
+release:
+	consume_skb(skb);
+	sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(sock_zerocopy_callback);
+
+void sock_zerocopy_put(struct ubuf_info *uarg)
+{
+	if (uarg && atomic_dec_and_test(&uarg->refcnt)) {
+		if (uarg->callback)
+			uarg->callback(uarg, uarg->zerocopy);
+		else
+			consume_skb(skb_from_uarg(uarg));
+	}
+}
+EXPORT_SYMBOL_GPL(sock_zerocopy_put);
+
+void sock_zerocopy_put_abort(struct ubuf_info *uarg)
+{
+	if (uarg) {
+		struct sock *sk = skb_from_uarg(uarg)->sk;
+
+		atomic_dec(&sk->sk_zckey);
+
+		/* sock_zerocopy_put expects a ref. Most sockets take one per
+		 * skb, which is zero on abort. tcp_sendmsg holds one extra, to
+		 * avoid an skb send inside the main loop triggering uarg free.
+		 */
+		if (sk->sk_type != SOCK_STREAM)
+			atomic_inc(&uarg->refcnt);
+
+		sock_zerocopy_put(uarg);
+	}
+}
+EXPORT_SYMBOL_GPL(sock_zerocopy_put_abort);
+
+extern int __zerocopy_sg_from_iter(struct sock *sk, struct sk_buff *skb,
+				   struct iov_iter *from, size_t length);
+
+int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
+			     struct msghdr *msg, int len,
+			     struct ubuf_info *uarg)
+{
+	struct iov_iter orig_iter = msg->msg_iter;
+	int err, orig_len = skb->len;
+
+	err = __zerocopy_sg_from_iter(sk, skb, &msg->msg_iter, len);
+	if (err == -EFAULT || (err == -EMSGSIZE && skb->len == orig_len)) {
+		/* Streams do not free skb on error. Reset to prev state. */
+		msg->msg_iter = orig_iter;
+		___pskb_trim(skb, orig_len);
+		return err;
+	}
+
+	skb_zcopy_set(skb, uarg);
+	return skb->len - orig_len;
+}
+EXPORT_SYMBOL_GPL(skb_zerocopy_iter_stream);
+
+/* unused only until next patch in the series; will remove attribute */
+static int __attribute__((unused))
+	   skb_zerocopy_clone(struct sk_buff *nskb, struct sk_buff *orig,
+			      gfp_t gfp_mask)
+{
+	if (skb_zcopy(orig)) {
+		if (skb_zcopy(nskb)) {
+			/* !gfp_mask callers are verified to !skb_zcopy(nskb) */
+			if (!gfp_mask) {
+				WARN_ON_ONCE(1);
+				return -ENOMEM;
+			}
+			if (skb_uarg(nskb) == skb_uarg(orig))
+				return 0;
+			if (skb_copy_ubufs(nskb, GFP_ATOMIC))
+				return -EIO;
+		}
+		skb_zcopy_set(nskb, skb_uarg(orig));
+	}
+	return 0;
+}
+
 /**
  *	skb_copy_ubufs	-	copy userspace skb frags buffers to kernel
  *	@skb: the skb to modify
diff --git a/net/core/sock.c b/net/core/sock.c
index 1261880bdcc8..e8b696858cad 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1670,6 +1670,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
 		atomic_set(&newsk->sk_drops, 0);
 		newsk->sk_send_head	= NULL;
 		newsk->sk_userlocks	= sk->sk_userlocks & ~SOCK_BINDPORT_LOCK;
+		atomic_set(&newsk->sk_zckey, 0);
 
 		sock_reset_flag(newsk, SOCK_DONE);
 
@@ -2722,6 +2723,7 @@ void sock_init_data(struct socket *sock, struct sock *sk)
 	sk->sk_sndtimeo		=	MAX_SCHEDULE_TIMEOUT;
 
 	sk->sk_stamp = SK_DEFAULT_STAMP;
+	atomic_set(&sk->sk_zckey, 0);
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	sk->sk_napi_id		=	0;
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 4/9] sock: add SOCK_ZEROCOPY sockopt
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-api, Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemb@google.com>

The send call ignores unknown flags. Legacy applications may already
unwittingly pass MSG_ZEROCOPY. Continue to ignore this flag unless a
socket opts in to zerocopy.

Introduce socket option SO_ZEROCOPY to enable MSG_ZEROCOPY processing.
Processes can also query this socket option to detect kernel support
for the feature. Older kernels will return ENOPROTOOPT.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 arch/alpha/include/uapi/asm/socket.h   |  2 ++
 arch/frv/include/uapi/asm/socket.h     |  2 ++
 arch/ia64/include/uapi/asm/socket.h    |  2 ++
 arch/m32r/include/uapi/asm/socket.h    |  2 ++
 arch/mips/include/uapi/asm/socket.h    |  2 ++
 arch/mn10300/include/uapi/asm/socket.h |  2 ++
 arch/parisc/include/uapi/asm/socket.h  |  2 ++
 arch/s390/include/uapi/asm/socket.h    |  2 ++
 arch/sparc/include/uapi/asm/socket.h   |  2 ++
 arch/xtensa/include/uapi/asm/socket.h  |  2 ++
 include/uapi/asm-generic/socket.h      |  2 ++
 net/core/skbuff.c                      |  3 +++
 net/core/sock.c                        | 18 ++++++++++++++++++
 13 files changed, 43 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 7b285dd4fe05..c6133a045352 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -109,4 +109,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index f1e3b20dce9f..9abf02d6855a 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -102,5 +102,7 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 5dd5c5d0d642..002eb85a6941 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -111,4 +111,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index f8f7b47e247f..e268e51a38d1 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -102,4 +102,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 882823bec153..6c755bc07975 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -120,4 +120,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index c710db354ff2..ac82a3f26dbf 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -102,4 +102,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index a0d4dc9f4eb2..3b2bf7ae703b 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -101,4 +101,6 @@
 
 #define SO_PEERGROUPS		0x4034
 
+#define SO_ZEROCOPY		0x4035
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index 52a63f4175cb..a56916c83565 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -108,4 +108,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 186fd8199f54..b2f5c50d0947 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -98,6 +98,8 @@
 
 #define SO_PEERGROUPS		0x003d
 
+#define SO_ZEROCOPY		0x003e
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index 3eed2761c149..220059999e74 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -113,4 +113,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 9861be8da65e..e47c9e436221 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_PEERGROUPS		59
 
+#define SO_ZEROCOPY		60
+
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 0603e44950da..29e34bc6a17c 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -922,6 +922,9 @@ struct ubuf_info *sock_zerocopy_alloc(struct sock *sk, size_t size)
 
 	WARN_ON_ONCE(!in_task());
 
+	if (!sock_flag(sk, SOCK_ZEROCOPY))
+		return NULL;
+
 	skb = sock_omalloc(sk, 0, GFP_KERNEL);
 	if (!skb)
 		return NULL;
diff --git a/net/core/sock.c b/net/core/sock.c
index e8b696858cad..9ea988d25b0a 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1055,6 +1055,20 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		if (val == 1)
 			dst_negative_advice(sk);
 		break;
+
+	case SO_ZEROCOPY:
+		if (sk->sk_family != PF_INET && sk->sk_family != PF_INET6)
+			ret = -ENOTSUPP;
+		else if (sk->sk_protocol != IPPROTO_TCP)
+			ret = -ENOTSUPP;
+		else if (sk->sk_state != TCP_CLOSE)
+			ret = -EBUSY;
+		else if (val < 0 || val > 1)
+			ret = -EINVAL;
+		else
+			sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
+		break;
+
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -1383,6 +1397,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val64 = sock_gen_cookie(sk);
 		break;
 
+	case SO_ZEROCOPY:
+		v.val = sock_flag(sk, SOCK_ZEROCOPY);
+		break;
+
 	default:
 		/* We implement the SO_SNDLOWAT etc to not be settable
 		 * (1003.1g 7).
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ permalink raw reply related

* [PATCH net-next v4 5/9] sock: enable MSG_ZEROCOPY
From: Willem de Bruijn @ 2017-08-03 20:29 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-api, Willem de Bruijn
In-Reply-To: <20170803202945.70750-1-willemdebruijn.kernel@gmail.com>

From: Willem de Bruijn <willemb@google.com>

Prepare the datapath for refcounted ubuf_info. Clone ubuf_info with
skb_zerocopy_clone() wherever needed due to skb split, merge, resize
or clone.

Split skb_orphan_frags into two variants. The split, merge, .. paths
support reference counted zerocopy buffers, so do not do a deep copy.
Add skb_orphan_frags_rx for paths that may loop packets to receive
sockets. That is not allowed, as it may cause unbounded latency.
Deep copy all zerocopy copy buffers, ref-counted or not, in this path.

The exact locations to modify were chosen by exhaustively searching
through all code that might modify skb_frag references and/or the
the SKBTX_DEV_ZEROCOPY tx_flags bit.

The changes err on the safe side, in two ways.

(1) legacy ubuf_info paths virtio and tap are not modified. They keep
    a 1:1 ubuf_info to sk_buff relationship. Calls to skb_orphan_frags
    still call skb_copy_ubufs and thus copy frags in this case.

(2) not all copies deep in the stack are addressed yet. skb_shift,
    skb_split and skb_try_coalesce can be refined to avoid copying.
    These are not in the hot path and this patch is hairy enough as
    is, so that is left for future refinement.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/tun.c      |  2 +-
 drivers/vhost/net.c    |  1 +
 include/linux/skbuff.h | 14 +++++++++++++-
 net/core/dev.c         |  4 ++--
 net/core/skbuff.c      | 48 +++++++++++++++++++-----------------------------
 5 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 68add55f8460..d21510d47aa2 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -892,7 +892,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	    sk_filter(tfile->socket.sk, skb))
 		goto drop;
 
-	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
+	if (unlikely(skb_orphan_frags_rx(skb, GFP_ATOMIC)))
 		goto drop;
 
 	skb_tx_timestamp(skb);
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 06d044862e58..ba08b78ed630 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -533,6 +533,7 @@ static void handle_tx(struct vhost_net *net)
 			ubuf->callback = vhost_zerocopy_callback;
 			ubuf->ctx = nvq->ubufs;
 			ubuf->desc = nvq->upend_idx;
+			atomic_set(&ubuf->refcnt, 1);
 			msg.msg_control = ubuf;
 			msg.msg_controllen = sizeof(ubuf);
 			ubufs = nvq->ubufs;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 59cff7aa494e..e5387932c266 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2512,7 +2512,17 @@ static inline void skb_orphan(struct sk_buff *skb)
  */
 static inline int skb_orphan_frags(struct sk_buff *skb, gfp_t gfp_mask)
 {
-	if (likely(!(skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY)))
+	if (likely(!skb_zcopy(skb)))
+		return 0;
+	if (skb_uarg(skb)->callback == sock_zerocopy_callback)
+		return 0;
+	return skb_copy_ubufs(skb, gfp_mask);
+}
+
+/* Frags must be orphaned, even if refcounted, if skb might loop to rx path */
+static inline int skb_orphan_frags_rx(struct sk_buff *skb, gfp_t gfp_mask)
+{
+	if (likely(!skb_zcopy(skb)))
 		return 0;
 	return skb_copy_ubufs(skb, gfp_mask);
 }
@@ -2944,6 +2954,8 @@ static inline int skb_add_data(struct sk_buff *skb,
 static inline bool skb_can_coalesce(struct sk_buff *skb, int i,
 				    const struct page *page, int off)
 {
+	if (skb_zcopy(skb))
+		return false;
 	if (i) {
 		const struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[i - 1];
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 8ea6b4b42611..1d75499add72 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1853,7 +1853,7 @@ static inline int deliver_skb(struct sk_buff *skb,
 			      struct packet_type *pt_prev,
 			      struct net_device *orig_dev)
 {
-	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
+	if (unlikely(skb_orphan_frags_rx(skb, GFP_ATOMIC)))
 		return -ENOMEM;
 	refcount_inc(&skb->users);
 	return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
@@ -4412,7 +4412,7 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
 	}
 
 	if (pt_prev) {
-		if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
+		if (unlikely(skb_orphan_frags_rx(skb, GFP_ATOMIC)))
 			goto drop;
 		else
 			ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 29e34bc6a17c..74d3c36f8419 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -567,21 +567,10 @@ static void skb_release_data(struct sk_buff *skb)
 	for (i = 0; i < shinfo->nr_frags; i++)
 		__skb_frag_unref(&shinfo->frags[i]);
 
-	/*
-	 * If skb buf is from userspace, we need to notify the caller
-	 * the lower device DMA has done;
-	 */
-	if (shinfo->tx_flags & SKBTX_DEV_ZEROCOPY) {
-		struct ubuf_info *uarg;
-
-		uarg = shinfo->destructor_arg;
-		if (uarg->callback)
-			uarg->callback(uarg, true);
-	}
-
 	if (shinfo->frag_list)
 		kfree_skb_list(shinfo->frag_list);
 
+	skb_zcopy_clear(skb, true);
 	skb_free_head(skb);
 }
 
@@ -695,14 +684,7 @@ EXPORT_SYMBOL(kfree_skb_list);
  */
 void skb_tx_error(struct sk_buff *skb)
 {
-	if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
-		struct ubuf_info *uarg;
-
-		uarg = skb_shinfo(skb)->destructor_arg;
-		if (uarg->callback)
-			uarg->callback(uarg, false);
-		skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
-	}
+	skb_zcopy_clear(skb, true);
 }
 EXPORT_SYMBOL(skb_tx_error);
 
@@ -1029,9 +1011,7 @@ int skb_zerocopy_iter_stream(struct sock *sk, struct sk_buff *skb,
 }
 EXPORT_SYMBOL_GPL(skb_zerocopy_iter_stream);
 
-/* unused only until next patch in the series; will remove attribute */
-static int __attribute__((unused))
-	   skb_zerocopy_clone(struct sk_buff *nskb, struct sk_buff *orig,
+static int skb_zerocopy_clone(struct sk_buff *nskb, struct sk_buff *orig,
 			      gfp_t gfp_mask)
 {
 	if (skb_zcopy(orig)) {
@@ -1068,7 +1048,6 @@ static int __attribute__((unused))
  */
 int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 {
-	struct ubuf_info *uarg = skb_shinfo(skb)->destructor_arg;
 	int num_frags = skb_shinfo(skb)->nr_frags;
 	struct page *page, *head = NULL;
 	int i, new_frags;
@@ -1127,8 +1106,6 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 	for (i = 0; i < num_frags; i++)
 		skb_frag_unref(skb, i);
 
-	uarg->callback(uarg, false);
-
 	/* skb frags point to kernel buffers */
 	for (i = 0; i < new_frags - 1; i++) {
 		__skb_fill_page_desc(skb, i, head, 0, PAGE_SIZE);
@@ -1137,7 +1114,7 @@ int skb_copy_ubufs(struct sk_buff *skb, gfp_t gfp_mask)
 	__skb_fill_page_desc(skb, new_frags - 1, head, 0, d_off);
 	skb_shinfo(skb)->nr_frags = new_frags;
 
-	skb_shinfo(skb)->tx_flags &= ~SKBTX_DEV_ZEROCOPY;
+	skb_zcopy_clear(skb, false);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(skb_copy_ubufs);
@@ -1298,7 +1275,8 @@ struct sk_buff *__pskb_copy_fclone(struct sk_buff *skb, int headroom,
 	if (skb_shinfo(skb)->nr_frags) {
 		int i;
 
-		if (skb_orphan_frags(skb, gfp_mask)) {
+		if (skb_orphan_frags(skb, gfp_mask) ||
+		    skb_zerocopy_clone(n, skb, gfp_mask)) {
 			kfree_skb(n);
 			n = NULL;
 			goto out;
@@ -1375,9 +1353,10 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 	 * be since all we did is relocate the values
 	 */
 	if (skb_cloned(skb)) {
-		/* copy this zero copy skb frags */
 		if (skb_orphan_frags(skb, gfp_mask))
 			goto nofrags;
+		if (skb_zcopy(skb))
+			atomic_inc(&skb_uarg(skb)->refcnt);
 		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
 			skb_frag_ref(skb, i);
 
@@ -1872,6 +1851,9 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta)
 	skb->tail     += delta;
 	skb->data_len -= delta;
 
+	if (!skb->data_len)
+		skb_zcopy_clear(skb, false);
+
 	return skb_tail_pointer(skb);
 }
 EXPORT_SYMBOL(__pskb_pull_tail);
@@ -2627,6 +2609,7 @@ skb_zerocopy(struct sk_buff *to, struct sk_buff *from, int len, int hlen)
 		skb_tx_error(from);
 		return -ENOMEM;
 	}
+	skb_zerocopy_clone(to, from, GFP_ATOMIC);
 
 	for (i = 0; i < skb_shinfo(from)->nr_frags; i++) {
 		if (!len)
@@ -2924,6 +2907,7 @@ void skb_split(struct sk_buff *skb, struct sk_buff *skb1, const u32 len)
 
 	skb_shinfo(skb1)->tx_flags |= skb_shinfo(skb)->tx_flags &
 				      SKBTX_SHARED_FRAG;
+	skb_zerocopy_clone(skb1, skb, 0);
 	if (len < pos)	/* Split line is inside header. */
 		skb_split_inside_header(skb, skb1, len, pos);
 	else		/* Second chunk has no header, nothing to copy. */
@@ -2967,6 +2951,8 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
 
 	if (skb_headlen(skb))
 		return 0;
+	if (skb_zcopy(tgt) || skb_zcopy(skb))
+		return 0;
 
 	todo = shiftlen;
 	from = 0;
@@ -3540,6 +3526,8 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
 		skb_shinfo(nskb)->tx_flags |= skb_shinfo(head_skb)->tx_flags &
 					      SKBTX_SHARED_FRAG;
+		if (skb_zerocopy_clone(nskb, head_skb, GFP_ATOMIC))
+			goto err;
 
 		while (pos < offset + len) {
 			if (i >= nfrags) {
@@ -4663,6 +4651,8 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
 
 	if (skb_has_frag_list(to) || skb_has_frag_list(from))
 		return false;
+	if (skb_zcopy(to) || skb_zcopy(from))
+		return false;
 
 	if (skb_headlen(from) != 0) {
 		struct page *page;
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

^ 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