[PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[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.

In this patch set:
  - 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 compilation errors when CONFIG_ULP_SOCK not set.
-v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
     in tsl_init. Also, fix a couple of minor issues related to
     introducing locked versions of sendmsg, send page. Thanks to
     Dave Watson, John Fastabend, and Mat Martineau for testing and
     providing fixes.

Tom Herbert (5):
  proto_ops: Fixes to adding locked version of sendmsg/page
  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                     |   6 +
 include/net/tcp.h                      |  25 -----
 include/net/tls.h                      |   4 +-
 include/net/ulp_sock.h                 |  76 +++++++++++++
 include/uapi/asm-generic/socket.h      |   2 +
 net/Kconfig                            |   4 +
 net/core/Makefile                      |   1 +
 net/core/skbuff.c                      |   2 +-
 net/core/sock.c                        |  12 ++
 net/core/sysctl_net_core.c             |  25 +++++
 net/core/ulp_sock.c                    | 196 +++++++++++++++++++++++++++++++++
 net/ipv4/Makefile                      |   2 +-
 net/ipv4/inet_connection_sock.c        |   5 +
 net/ipv4/sysctl_net_ipv4.c             |   9 +-
 net/ipv4/tcp.c                         |  42 ++++---
 net/ipv4/tcp_ipv4.c                    |   2 -
 net/ipv4/tcp_ulp.c                     | 135 -----------------------
 net/socket.c                           |   2 +-
 net/tls/Kconfig                        |   1 +
 net/tls/tls_main.c                     |  24 ++--
 35 files changed, 495 insertions(+), 203 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

[PATCH v3 net-next 1/5] proto_ops: Fixes to adding locked version of sendmsg/page

[Tom Herbert]

Fixes for two issues pointed out by John Fastabend.

Fixes: 306b13eb3cf9515a ("proto_ops: Add locked held versions of sendmsg and sendpage")
Reported-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Tom Herbert <tom@quantonium.net>
---
 net/core/skbuff.c | 2 +-
 net/socket.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 42b62c716a33..e3bac9f74789 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2285,7 +2285,7 @@ int skb_send_sock_locked(struct sock *sk, struct sk_buff *skb, int offset,
 
 		slen = min_t(int, len, skb_headlen(skb) - offset);
 		kv.iov_base = skb->data + offset;
-		kv.iov_len = len;
+		kv.iov_len = slen;
 		memset(&msg, 0, sizeof(msg));
 
 		ret = kernel_sendmsg_locked(sk, &msg, &kv, 1, slen);
diff --git a/net/socket.c b/net/socket.c
index b332d1e8e4e4..c729625eb5d3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -658,7 +658,7 @@ int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
 	struct socket *sock = sk->sk_socket;
 
 	if (!sock->ops->sendmsg_locked)
-		sock_no_sendmsg_locked(sk, msg, size);
+		return sock_no_sendmsg_locked(sk, msg, size);
 
 	iov_iter_kvec(&msg->msg_iter, WRITE | ITER_KVEC, vec, num, size);
 
-- 
2.11.0

Re: [PATCH v3 net-next 1/5] proto_ops: Fixes to adding locked version of sendmsg/page

[John Fastabend]

On 08/07/2017 10:28 AM, Tom Herbert wrote:
> Fixes for two issues pointed out by John Fastabend.
> 
> Fixes: 306b13eb3cf9515a ("proto_ops: Add locked held versions of sendmsg and sendpage")
> Reported-by: John Fastabend <john.fastabend@gmail.com>
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---

This is the same patch as the one in my RFC series so its an ack from me.

Acked-by: John Fastabend <john.fastabend@gmail.com>

[PATCH v3 net-next 2/5] inet: include net/sock.h in inet_common.h

[Tom Herbert]

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

[PATCH v3 net-next 3/5] sock: ULP infrastructure

[Tom Herbert]

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                     |   6 +
 include/net/ulp_sock.h                 |  76 +++++++++++++
 include/uapi/asm-generic/socket.h      |   2 +
 net/Kconfig                            |   4 +
 net/core/Makefile                      |   1 +
 net/core/sock.c                        |  12 ++
 net/core/sysctl_net_core.c             |  25 +++++
 net/core/ulp_sock.c                    | 196 +++++++++++++++++++++++++++++++++
 net/ipv4/inet_connection_sock.c        |   5 +
 20 files changed, 356 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 c6133a045352..810e0dc8f394 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -111,4 +111,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index 9abf02d6855a..c7bb41ae784b 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -104,5 +104,7 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 002eb85a6941..c4e94563c4ce 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -113,4 +113,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index e268e51a38d1..4359388a541d 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 6c755bc07975..300eb1074611 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -122,4 +122,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index ac82a3f26dbf..c458c614afa6 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -104,4 +104,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 3b2bf7ae703b..fa25c1105faf 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -103,4 +103,6 @@
 
 #define SO_ZEROCOPY		0x4035
 
+#define SO_ULP			0x4036
+
 #endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index a56916c83565..d0bee5a5ac17 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -110,4 +110,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index b2f5c50d0947..46f5d04426e8 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -100,6 +100,8 @@
 
 #define SO_ZEROCOPY		0x003e
 
+#define SO_ULP			0x003f
+
 /* 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 220059999e74..f654e2507726 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -115,4 +115,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8ad963cdc88c..9ff7b1b2256f 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 fe1a0bc25cd3..ea67ea91db87 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.
@@ -313,6 +314,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 {
 	/*
@@ -480,6 +483,9 @@ struct sock {
 	void                    (*sk_destruct)(struct sock *sk);
 	struct sock_reuseport __rcu	*sk_reuseport_cb;
 	struct rcu_head		sk_rcu;
+
+	const struct ulp_ops	*sk_ulp_ops;
+	void			*sk_ulp_data;
 };
 
 enum sk_pacing {
diff --git a/include/net/ulp_sock.h b/include/net/ulp_sock.h
new file mode 100644
index 000000000000..b292831a5d75
--- /dev/null
+++ b/include/net/ulp_sock.h
@@ -0,0 +1,76 @@
+/*
+ * 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)
+{
+	 *buf = '\0';
+}
+
+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 e47c9e436221..41abbf7f6b66 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -106,4 +106,6 @@
 
 #define SO_ZEROCOPY		60
 
+#define SO_ULP			61
+
 #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 56d771a887b6..0d90c0ade1c8 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -29,3 +29,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 9ea988d25b0a..b2a6162cca9d 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1069,6 +1069,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 			sock_valbool_flag(sk, SOCK_ZEROCOPY, valbool);
 		break;
 
+	case SO_ULP:
+		ret = ulp_set(sk, optval, optlen);
+		break;
+
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -1401,6 +1405,9 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sock_flag(sk, SOCK_ZEROCOPY);
 		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).
@@ -2990,6 +2997,11 @@ 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..0bd9e8af18a4
--- /dev/null
+++ b/net/core/ulp_sock.c
@@ -0,0 +1,196 @@
+/*
+ * 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);
+
+	sk->sk_ulp_ops = NULL;
+}
+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);
+
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 4089c013cb03..4e9e60f738a9 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -830,6 +830,11 @@ void inet_csk_destroy_sock(struct sock *sk)
 	/* If it has not 0 inet_sk(sk)->inet_num, it must be bound */
 	WARN_ON(inet_sk(sk)->inet_num && !inet_csk(sk)->icsk_bind_hash);
 
+	/* Clean up ULP before destroying protocol socket since ULP might
+	 * be dependent on transport (and not the other way around).
+	 */
+	ulp_cleanup(sk);
+
 	sk->sk_prot->destroy(sk);
 
 	sk_stream_kill_queues(sk);
-- 
2.11.0

Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure

[John Fastabend]

On 08/07/2017 10:28 AM, Tom Herbert wrote:
> 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>
> ---

I think this generalization should not get committed until it has a user.
I see you posted the socktap stuff but that is just an RFC for now.

[...]

> +
> +static inline void ulp_get_available(char *buf, size_t len)
> +{

Do we need to check len field or is len == 0 invalid?

> +	 *buf = '\0';
> +}
> +
> +static inline void ulp_cleanup(struct sock *sk)
> +{
> +}
> +

[...]


Rest looks OK I'll take a closer look tomorrow at this and the RFC user.

Thanks,
John

Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure

[Hannes Frederic Sowa]

Tom Herbert <tom@quantonium.net> writes:

> +#ifdef CONFIG_MODULES
> +	if (!ulp && capable(CAP_NET_ADMIN)) {
> +		rcu_read_unlock();
> +		request_module("%s", name);
> +		rcu_read_lock();
> +		ulp = ulp_find(name);
> +	}
> +#endif

It looks to me that this allows users with only CAP_NET_ADMIN
privileges to load every module?

Re: [PATCH v3 net-next 3/5] sock: ULP infrastructure

[Tom Herbert]

On Tue, Aug 8, 2017 at 9:38 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Tom Herbert <tom@quantonium.net> writes:
>
>> +#ifdef CONFIG_MODULES
>> +     if (!ulp && capable(CAP_NET_ADMIN)) {
>> +             rcu_read_unlock();
>> +             request_module("%s", name);
>> +             rcu_read_lock();
>> +             ulp = ulp_find(name);
>> +     }
>> +#endif
>
> It looks to me that this allows users with only CAP_NET_ADMIN
> privileges to load every module?

It's a carryover. Probably should remove the check.

Tom

[PATCH v3 net-next 4/5] tcp: Adjust TCP ULP to defer to sockets ULP

[Tom Herbert]

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

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                     |  42 +++++++-----
 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, 53 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 5173fecde495..84adac23d324 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1969,31 +1969,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 71b25567e787..b1ca6b4c605c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2434,24 +2434,25 @@ 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;
 
-		lock_sock(sk);
-		err = tcp_set_ulp(sk, name);
-		release_sock(sk);
-		return err;
+		ulpc.ulp_name[val] = 0;
+
+		return kernel_setsockopt(sk->sk_socket, SOL_SOCKET, SO_ULP,
+					 (char *)&ulpc, sizeof(ulpc));
 	}
+
 	default:
 		/* fallthru */
 		break;
@@ -3023,20 +3024,29 @@ 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 5f708c85110e..95e47c641f17 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..f5c90efec8b4 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 -EPROTONOSUPPORT;
+
 	/* 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

Re: [PATCH v3 net-next 4/5] tcp: Adjust TCP ULP to defer to sockets ULP

[John Fastabend]

On 08/07/2017 10:28 AM, Tom Herbert wrote:
> Fix TCP and TLS to use the newer ULP infrastructure in sockets.
> 
> Signed-off-by: Tom Herbert <tom@quantonium.net>
> ---

Looks OK to me.

Acked-by: John Fastabend <john.fastabend@gmail.com>

[PATCH v3 net-next 5/5] ulp: Documention for ULP infrastructure

[Tom Herbert]

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..5f6ae2153e69
--- /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 struct includes a "__u8 ulp_params[0]" field 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

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[John Fastabend]

On 08/07/2017 10:28 AM, Tom Herbert wrote:
> 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.
> 
> In this patch set:
>   - 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 compilation errors when CONFIG_ULP_SOCK not set.
> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>      in tsl_init. Also, fix a couple of minor issues related to
>      introducing locked versions of sendmsg, send page. Thanks to
>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>      providing fixes.
> 


Hi Tom, Dave,

I'm concerned about the performance impact of walking a list and
doing string compares on every socket we create with kTLS. Dave
do you have any request/response tests for kTLS that would put pressure
on the create/destroy time of this infrastructure? We should do some
tests with dummy entries in the ULP list to understand the impact of
this list walk.

I like the underlying TCP generalized hooks, but do we really expect a
lot of these hooks to exist? If we only have two on the roadmap
(kTLS and socktap) it seems a bit overkill. Further, if we really expect
many ULP objects then the list walk and compare will become more expensive
perhaps becoming noticeable in request per second metrics.

Why not just create another socktap socketopt? That will be better from
complexity and likely performance sides.

Thanks,
.John 

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[John Fastabend]

On 08/08/2017 08:31 AM, John Fastabend wrote:
> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>> 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.
>>
>> In this patch set:
>>   - 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 compilation errors when CONFIG_ULP_SOCK not set.
>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>      in tsl_init. Also, fix a couple of minor issues related to
>>      introducing locked versions of sendmsg, send page. Thanks to
>>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>>      providing fixes.
>>
> 
> 
> Hi Tom, Dave,
> 
> I'm concerned about the performance impact of walking a list and
> doing string compares on every socket we create with kTLS. Dave
> do you have any request/response tests for kTLS that would put pressure
> on the create/destroy time of this infrastructure? We should do some
> tests with dummy entries in the ULP list to understand the impact of
> this list walk.
> 
> I like the underlying TCP generalized hooks, but do we really expect a
> lot of these hooks to exist? If we only have two on the roadmap
> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
> many ULP objects then the list walk and compare will become more expensive
> perhaps becoming noticeable in request per second metrics.
> 
> Why not just create another socktap socketopt? That will be better from
> complexity and likely performance sides.
> 
> Thanks,
> .John 
> 

@Tom, I should have added: I know you ported the list stuff from the
original code so its more of a general question about how we want to manage
ULPs vs a specific patch comment :)

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[Tom Herbert]

On Tue, Aug 8, 2017 at 8:31 AM, John Fastabend <john.fastabend@gmail.com> wrote:
> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>> 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.
>>
>> In this patch set:
>>   - 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 compilation errors when CONFIG_ULP_SOCK not set.
>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>      in tsl_init. Also, fix a couple of minor issues related to
>>      introducing locked versions of sendmsg, send page. Thanks to
>>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>>      providing fixes.
>>
>
>
> Hi Tom, Dave,
>
> I'm concerned about the performance impact of walking a list and
> doing string compares on every socket we create with kTLS. Dave
> do you have any request/response tests for kTLS that would put pressure
> on the create/destroy time of this infrastructure? We should do some
> tests with dummy entries in the ULP list to understand the impact of
> this list walk.
>
> I like the underlying TCP generalized hooks, but do we really expect a
> lot of these hooks to exist? If we only have two on the roadmap
> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
> many ULP objects then the list walk and compare will become more expensive
> perhaps becoming noticeable in request per second metrics.
>
> Why not just create another socktap socketopt? That will be better from
> complexity and likely performance sides.
>
IMO, given that there is at most two even proposed at this point I
don't there's much point addressing performance. When ULP feature
catches on and we start see a whole bunch of them then it's
straightforward to use a hash table or some more efficient mechanism.

Tom

> Thanks,
> .John
>

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[John Fastabend]

On 08/08/2017 10:04 AM, Tom Herbert wrote:
> On Tue, Aug 8, 2017 at 8:31 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>>> 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.
>>>
>>> In this patch set:
>>>   - 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 compilation errors when CONFIG_ULP_SOCK not set.
>>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>>      in tsl_init. Also, fix a couple of minor issues related to
>>>      introducing locked versions of sendmsg, send page. Thanks to
>>>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>>>      providing fixes.
>>>
>>
>>
>> Hi Tom, Dave,
>>
>> I'm concerned about the performance impact of walking a list and
>> doing string compares on every socket we create with kTLS. Dave
>> do you have any request/response tests for kTLS that would put pressure
>> on the create/destroy time of this infrastructure? We should do some
>> tests with dummy entries in the ULP list to understand the impact of
>> this list walk.
>>
>> I like the underlying TCP generalized hooks, but do we really expect a
>> lot of these hooks to exist? If we only have two on the roadmap
>> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
>> many ULP objects then the list walk and compare will become more expensive
>> perhaps becoming noticeable in request per second metrics.
>>
>> Why not just create another socktap socketopt? That will be better from
>> complexity and likely performance sides.
>>
> IMO, given that there is at most two even proposed at this point I
> don't there's much point addressing performance. When ULP feature
> catches on and we start see a whole bunch of them then it's
> straightforward to use a hash table or some more efficient mechanism.
> 

OTOH these optimizations are usually easiest to do at the beginning. And
building an enum of ULP types would allow removing string comparisons and
to do simpler unsigned comparisons. I wont complain too much here though
because this series didn't introduce the lists.

> Tom
> 
>> Thanks,
>> .John
>>

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[Tom Herbert]

On Tue, Aug 8, 2017 at 12:30 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> On 08/08/2017 10:04 AM, Tom Herbert wrote:
>> On Tue, Aug 8, 2017 at 8:31 AM, John Fastabend <john.fastabend@gmail.com> wrote:
>>> On 08/07/2017 10:28 AM, Tom Herbert wrote:
>>>> 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.
>>>>
>>>> In this patch set:
>>>>   - 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 compilation errors when CONFIG_ULP_SOCK not set.
>>>> -v3: Fix one more build issue, check that sk_protocol is IPPROTO_TCP
>>>>      in tsl_init. Also, fix a couple of minor issues related to
>>>>      introducing locked versions of sendmsg, send page. Thanks to
>>>>      Dave Watson, John Fastabend, and Mat Martineau for testing and
>>>>      providing fixes.
>>>>
>>>
>>>
>>> Hi Tom, Dave,
>>>
>>> I'm concerned about the performance impact of walking a list and
>>> doing string compares on every socket we create with kTLS. Dave
>>> do you have any request/response tests for kTLS that would put pressure
>>> on the create/destroy time of this infrastructure? We should do some
>>> tests with dummy entries in the ULP list to understand the impact of
>>> this list walk.
>>>
>>> I like the underlying TCP generalized hooks, but do we really expect a
>>> lot of these hooks to exist? If we only have two on the roadmap
>>> (kTLS and socktap) it seems a bit overkill. Further, if we really expect
>>> many ULP objects then the list walk and compare will become more expensive
>>> perhaps becoming noticeable in request per second metrics.
>>>
>>> Why not just create another socktap socketopt? That will be better from
>>> complexity and likely performance sides.
>>>
>> IMO, given that there is at most two even proposed at this point I
>> don't there's much point addressing performance. When ULP feature
>> catches on and we start see a whole bunch of them then it's
>> straightforward to use a hash table or some more efficient mechanism.
>>
>
> OTOH these optimizations are usually easiest to do at the beginning. And
> building an enum of ULP types would allow removing string comparisons and
> to do simpler unsigned comparisons. I wont complain too much here though
> because this series didn't introduce the lists.
>
Hi John,

It's a tradeoff. The nice thing about using strings is that we don't
need maintain a universal enum.

A related problem is how to combine different ULPs on the same socket.
For instance, I might want to do filtering on the application layer
messages being sent over TLS (stap+kTLS ULPs). So far I don't see an
obvious way to do that. The buffering requirement for crypto seems to
convolute this some.

Tom

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[Edward Cree]

On 08/08/17 20:50, Tom Herbert wrote:
> It's a tradeoff. The nice thing about using strings is that we don't
> need maintain a universal enum.
Hmm, that makes it sound as though you're intending for random out-of-tree
 modules to add these things; since if they're in-tree it's easy for them
 to get enum values assigned when they're added.  Do we really want to
 encourage sticking random module code into the network stack like this?

In any case, if you go with the enum approach and later it _does_ prove
 necessary to have more flexibility, you can have enum values dynamically
 assigned (like genetlink manages to do); and programs using the existing
 fixed IDs will continue to work.  It's much harder to go the other way...

-Ed

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[Tom Herbert]

On Tue, Aug 8, 2017 at 1:23 PM, Edward Cree <ecree@solarflare.com> wrote:
> On 08/08/17 20:50, Tom Herbert wrote:
>> It's a tradeoff. The nice thing about using strings is that we don't
>> need maintain a universal enum.
> Hmm, that makes it sound as though you're intending for random out-of-tree
>  modules to add these things; since if they're in-tree it's easy for them
>  to get enum values assigned when they're added.  Do we really want to
>  encourage sticking random module code into the network stack like this?
>
> In any case, if you go with the enum approach and later it _does_ prove
>  necessary to have more flexibility, you can have enum values dynamically
>  assigned (like genetlink manages to do); and programs using the existing
>  fixed IDs will continue to work.  It's much harder to go the other way...
>
There is history and precedence. The string mechanism for ulp_ops  a
direct port of the original ULP infrastructure done for kTLS. That
code based the mechanism on TCP congestion ops and that was introduced
into the kernel twelve years ago. This method doesn't seem to have
been viewed as a problem before now...

Tom

> -Ed

Re: [PATCH v3 net-next 0/5] ulp: Generalize ULP infrastructure

[David Miller]

From: Edward Cree <ecree@solarflare.com>
Date: Tue, 8 Aug 2017 21:23:03 +0100

> In any case, if you go with the enum approach and later it _does_ prove
>  necessary to have more flexibility, you can have enum values dynamically
>  assigned (like genetlink manages to do); and programs using the existing
>  fixed IDs will continue to work.

Indeed:

> It's much harder to go the other way...

^^^^ THIS!