[PATCH net-next 0/3] strp: Generalize stream parser to work with other socket types

[PATCH net-next 0/3] strp: Generalize stream parser to work with other socket types

[Tom Herbert]

Add a read_sock protocol operation function that allows something like
tcp_read_sock to be called for other protocol types.

Specific changes in this patch set:
  - Add read_sock function to proto_ops. This has the same signature as
    tcp_read_sock. sk_read_actor_t is also defined in net.h.
  - Set peek_len and read_sock proto_op functions for TCPv4 and TCPv6
    stream ops.
  - Remove references to tcp in strparser.
  - Call peek_len and read_sock operations from strparser instead of
    calling TCP specific functions.

Tom Herbert (3):
  net: Add read_sock proto_op
  tcp: Set read_sock and peek_len proto_ops
  kcm: Remove TCP specific references from kcm and strparser

 include/linux/net.h       |  6 ++++++
 include/net/strparser.h   |  2 +-
 include/net/tcp.h         |  4 ++--
 net/ipv4/af_inet.c        |  2 ++
 net/ipv4/tcp.c            |  6 ++++++
 net/ipv6/af_inet6.c       |  2 ++
 net/kcm/kcmsock.c         | 30 +++++++++++++----------------
 net/strparser/strparser.c | 48 ++++++++++++++++++++++++++++-------------------
 8 files changed, 61 insertions(+), 39 deletions(-)

-- 
2.8.0.rc2

[PATCH net-next 1/3] net: Add read_sock proto_op

[Tom Herbert]

Add new function in proto_ops structure. This includes moving the
typedef got sk_read_actor into net.h and removing the definition from
tcp.h.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/linux/net.h | 6 ++++++
 include/net/tcp.h   | 2 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index b9f0ff4..cd0c8bd 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -25,6 +25,7 @@
 #include <linux/kmemcheck.h>
 #include <linux/rcupdate.h>
 #include <linux/once.h>
+#include <linux/fs.h>
 
 #include <uapi/linux/net.h>
 
@@ -128,6 +129,9 @@ struct page;
 struct sockaddr;
 struct msghdr;
 struct module;
+struct sk_buff;
+typedef int (*sk_read_actor_t)(read_descriptor_t *, struct sk_buff *,
+			       unsigned int, size_t);
 
 struct proto_ops {
 	int		family;
@@ -186,6 +190,8 @@ struct proto_ops {
 				       struct pipe_inode_info *pipe, size_t len, unsigned int flags);
 	int		(*set_peek_off)(struct sock *sk, int val);
 	int		(*peek_len)(struct socket *sock);
+	int		(*read_sock)(struct sock *sk, read_descriptor_t *desc,
+				     sk_read_actor_t recv_actor);
 };
 
 #define DECLARE_SOCKADDR(type, dst, src)	\
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 25d64f6..d56666a 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -603,8 +603,6 @@ static inline int tcp_bound_to_half_wnd(struct tcp_sock *tp, int pktsize)
 void tcp_get_info(struct sock *, struct tcp_info *);
 
 /* Read 'sendfile()'-style from a TCP socket */
-typedef int (*sk_read_actor_t)(read_descriptor_t *, struct sk_buff *,
-				unsigned int, size_t);
 int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 		  sk_read_actor_t recv_actor);
 
-- 
2.8.0.rc2

[PATCH net-next 2/3] tcp: Set read_sock and peek_len proto_ops

[Tom Herbert]

In inet_stream_ops we set read_sock to tcp_read_sock and peek_len to
tcp_peek_len (which is just a stub function that calls tcp_inq).

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/tcp.h   | 2 ++
 net/ipv4/af_inet.c  | 2 ++
 net/ipv4/tcp.c      | 6 ++++++
 net/ipv6/af_inet6.c | 2 ++
 4 files changed, 12 insertions(+)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d56666a..a5af6be 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1848,6 +1848,8 @@ static inline int tcp_inq(struct sock *sk)
 	return answ;
 }
 
+int tcp_peek_len(struct socket *sock);
+
 static inline void tcp_segs_in(struct tcp_sock *tp, const struct sk_buff *skb)
 {
 	u16 segs_in;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 989a362..e94b47b 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -916,6 +916,8 @@ const struct proto_ops inet_stream_ops = {
 	.mmap		   = sock_no_mmap,
 	.sendpage	   = inet_sendpage,
 	.splice_read	   = tcp_splice_read,
+	.read_sock	   = tcp_read_sock,
+	.peek_len	   = tcp_peek_len,
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_sock_common_setsockopt,
 	.compat_getsockopt = compat_sock_common_getsockopt,
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f1a9a0a..60a4388 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1570,6 +1570,12 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 }
 EXPORT_SYMBOL(tcp_read_sock);
 
+int tcp_peek_len(struct socket *sock)
+{
+	return tcp_inq(sock->sk);
+}
+EXPORT_SYMBOL(tcp_peek_len);
+
 /*
  *	This routine copies from a sock struct into the user buffer.
  *
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index b454055..46ad699 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -545,6 +545,8 @@ const struct proto_ops inet6_stream_ops = {
 	.mmap		   = sock_no_mmap,
 	.sendpage	   = inet_sendpage,
 	.splice_read	   = tcp_splice_read,
+	.read_sock	   = tcp_read_sock,
+	.peek_len	   = tcp_peek_len,
 #ifdef CONFIG_COMPAT
 	.compat_setsockopt = compat_sock_common_setsockopt,
 	.compat_getsockopt = compat_sock_common_getsockopt,
-- 
2.8.0.rc2

[PATCH net-next 3/3] kcm: Remove TCP specific references from kcm and strparser

[Tom Herbert]

kcm and strparser need to work with any type of stream socket not just
TCP. Eliminate references to TCP and call generic proto_ops functions of
read_sock and peek_len. Also in strp_init check if the socket support
the proto_ops read_sock and peek_len.

Signed-off-by: Tom Herbert <tom@herbertland.com>
---
 include/net/strparser.h   |  2 +-
 net/kcm/kcmsock.c         | 30 +++++++++++++----------------
 net/strparser/strparser.c | 48 ++++++++++++++++++++++++++++-------------------
 3 files changed, 43 insertions(+), 37 deletions(-)

diff --git a/include/net/strparser.h b/include/net/strparser.h
index 91fa0b9..0c28ad9 100644
--- a/include/net/strparser.h
+++ b/include/net/strparser.h
@@ -137,6 +137,6 @@ void strp_stop(struct strparser *strp);
 void strp_check_rcv(struct strparser *strp);
 int strp_init(struct strparser *strp, struct sock *csk,
 	      struct strp_callbacks *cb);
-void strp_tcp_data_ready(struct strparser *strp);
+void strp_data_ready(struct strparser *strp);
 
 #endif /* __NET_STRPARSER_H_ */
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index eb731ca..2632ac7 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -26,7 +26,6 @@
 #include <net/kcm.h>
 #include <net/netns/generic.h>
 #include <net/sock.h>
-#include <net/tcp.h>
 #include <uapi/linux/kcm.h>
 
 unsigned int kcm_net_id;
@@ -340,7 +339,7 @@ static void unreserve_rx_kcm(struct kcm_psock *psock,
 }
 
 /* Lower sock lock held */
-static void psock_tcp_data_ready(struct sock *sk)
+static void psock_data_ready(struct sock *sk)
 {
 	struct kcm_psock *psock;
 
@@ -348,7 +347,7 @@ static void psock_tcp_data_ready(struct sock *sk)
 
 	psock = (struct kcm_psock *)sk->sk_user_data;
 	if (likely(psock))
-		strp_tcp_data_ready(&psock->strp);
+		strp_data_ready(&psock->strp);
 
 	read_unlock_bh(&sk->sk_callback_lock);
 }
@@ -392,7 +391,7 @@ static int kcm_read_sock_done(struct strparser *strp, int err)
 	return err;
 }
 
-static void psock_tcp_state_change(struct sock *sk)
+static void psock_state_change(struct sock *sk)
 {
 	/* TCP only does a POLLIN for a half close. Do a POLLHUP here
 	 * since application will normally not poll with POLLIN
@@ -402,7 +401,7 @@ static void psock_tcp_state_change(struct sock *sk)
 	report_csk_error(sk, EPIPE);
 }
 
-static void psock_tcp_write_space(struct sock *sk)
+static void psock_write_space(struct sock *sk)
 {
 	struct kcm_psock *psock;
 	struct kcm_mux *mux;
@@ -1383,19 +1382,12 @@ static int kcm_attach(struct socket *sock, struct socket *csock,
 	struct list_head *head;
 	int index = 0;
 	struct strp_callbacks cb;
-
-	if (csock->ops->family != PF_INET &&
-	    csock->ops->family != PF_INET6)
-		return -EINVAL;
+	int err;
 
 	csk = csock->sk;
 	if (!csk)
 		return -EINVAL;
 
-	/* Only support TCP for now */
-	if (csk->sk_protocol != IPPROTO_TCP)
-		return -EINVAL;
-
 	psock = kmem_cache_zalloc(kcm_psockp, GFP_KERNEL);
 	if (!psock)
 		return -ENOMEM;
@@ -1409,7 +1401,11 @@ static int kcm_attach(struct socket *sock, struct socket *csock,
 	cb.parse_msg = kcm_parse_func_strparser;
 	cb.read_sock_done = kcm_read_sock_done;
 
-	strp_init(&psock->strp, csk, &cb);
+	err = strp_init(&psock->strp, csk, &cb);
+	if (err) {
+		kmem_cache_free(kcm_psockp, psock);
+		return err;
+	}
 
 	sock_hold(csk);
 
@@ -1418,9 +1414,9 @@ static int kcm_attach(struct socket *sock, struct socket *csock,
 	psock->save_write_space = csk->sk_write_space;
 	psock->save_state_change = csk->sk_state_change;
 	csk->sk_user_data = psock;
-	csk->sk_data_ready = psock_tcp_data_ready;
-	csk->sk_write_space = psock_tcp_write_space;
-	csk->sk_state_change = psock_tcp_state_change;
+	csk->sk_data_ready = psock_data_ready;
+	csk->sk_write_space = psock_write_space;
+	csk->sk_state_change = psock_state_change;
 	write_unlock_bh(&csk->sk_callback_lock);
 
 	/* Finished initialization, now add the psock to the MUX. */
diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c
index 4ecfc10..5c7549b 100644
--- a/net/strparser/strparser.c
+++ b/net/strparser/strparser.c
@@ -26,7 +26,6 @@
 #include <net/strparser.h>
 #include <net/netns/generic.h>
 #include <net/sock.h>
-#include <net/tcp.h>
 
 static struct workqueue_struct *strp_wq;
 
@@ -80,9 +79,16 @@ static void strp_parser_err(struct strparser *strp, int err,
 	strp->cb.abort_parser(strp, err);
 }
 
+static inline int strp_peek_len(struct strparser *strp)
+{
+	struct socket *sock = strp->sk->sk_socket;
+
+	return sock->ops->peek_len(sock);
+}
+
 /* Lower socket lock held */
-static int strp_tcp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
-			 unsigned int orig_offset, size_t orig_len)
+static int strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
+		     unsigned int orig_offset, size_t orig_len)
 {
 	struct strparser *strp = (struct strparser *)desc->arg.data;
 	struct _strp_rx_msg *rxm;
@@ -266,12 +272,12 @@ static int strp_tcp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb,
 		if (extra < 0) {
 			/* Message not complete yet. */
 			if (rxm->strp.full_len - rxm->accum_len >
-			    tcp_inq(strp->sk)) {
+			    strp_peek_len(strp)) {
 				/* Don't have the whole messages in the socket
 				 * buffer. Set strp->rx_need_bytes to wait for
 				 * the rest of the message. Also, set "early
 				 * eaten" since we've already buffered the skb
-				 * but don't consume yet per tcp_read_sock.
+				 * but don't consume yet per strp_read_sock.
 				 */
 
 				if (!rxm->accum_len) {
@@ -329,16 +335,17 @@ static int default_read_sock_done(struct strparser *strp, int err)
 }
 
 /* Called with lock held on lower socket */
-static int strp_tcp_read_sock(struct strparser *strp)
+static int strp_read_sock(struct strparser *strp)
 {
+	struct socket *sock = strp->sk->sk_socket;
 	read_descriptor_t desc;
 
 	desc.arg.data = strp;
 	desc.error = 0;
 	desc.count = 1; /* give more than one skb per call */
 
-	/* sk should be locked here, so okay to do tcp_read_sock */
-	tcp_read_sock(strp->sk, &desc, strp_tcp_recv);
+	/* sk should be locked here, so okay to do read_sock */
+	sock->ops->read_sock(strp->sk, &desc, strp_recv);
 
 	desc.error = strp->cb.read_sock_done(strp, desc.error);
 
@@ -346,10 +353,8 @@ static int strp_tcp_read_sock(struct strparser *strp)
 }
 
 /* Lower sock lock held */
-void strp_tcp_data_ready(struct strparser *strp)
+void strp_data_ready(struct strparser *strp)
 {
-	struct sock *csk = strp->sk;
-
 	if (unlikely(strp->rx_stopped))
 		return;
 
@@ -360,7 +365,7 @@ void strp_tcp_data_ready(struct strparser *strp)
 	 * allows a thread in BH context to safely check if the process
 	 * lock is held. In this case, if the lock is held, queue work.
 	 */
-	if (sock_owned_by_user(csk)) {
+	if (sock_owned_by_user(strp->sk)) {
 		queue_work(strp_wq, &strp->rx_work);
 		return;
 	}
@@ -369,24 +374,24 @@ void strp_tcp_data_ready(struct strparser *strp)
 		return;
 
 	if (strp->rx_need_bytes) {
-		if (tcp_inq(csk) >= strp->rx_need_bytes)
+		if (strp_peek_len(strp) >= strp->rx_need_bytes)
 			strp->rx_need_bytes = 0;
 		else
 			return;
 	}
 
-	if (strp_tcp_read_sock(strp) == -ENOMEM)
+	if (strp_read_sock(strp) == -ENOMEM)
 		queue_work(strp_wq, &strp->rx_work);
 }
-EXPORT_SYMBOL_GPL(strp_tcp_data_ready);
+EXPORT_SYMBOL_GPL(strp_data_ready);
 
 static void do_strp_rx_work(struct strparser *strp)
 {
 	read_descriptor_t rd_desc;
 	struct sock *csk = strp->sk;
 
-	/* We need the read lock to synchronize with strp_tcp_data_ready. We
-	 * need the socket lock for calling tcp_read_sock.
+	/* We need the read lock to synchronize with strp_data_ready. We
+	 * need the socket lock for calling strp_read_sock.
 	 */
 	lock_sock(csk);
 
@@ -398,7 +403,7 @@ static void do_strp_rx_work(struct strparser *strp)
 
 	rd_desc.arg.data = strp;
 
-	if (strp_tcp_read_sock(strp) == -ENOMEM)
+	if (strp_read_sock(strp) == -ENOMEM)
 		queue_work(strp_wq, &strp->rx_work);
 
 out:
@@ -424,9 +429,14 @@ static void strp_rx_msg_timeout(unsigned long arg)
 int strp_init(struct strparser *strp, struct sock *csk,
 	      struct strp_callbacks *cb)
 {
+	struct socket *sock = csk->sk_socket;
+
 	if (!cb || !cb->rcv_msg || !cb->parse_msg)
 		return -EINVAL;
 
+	if (!sock->ops->read_sock || !sock->ops->peek_len)
+		return -EAFNOSUPPORT;
+
 	memset(strp, 0, sizeof(*strp));
 
 	strp->sk = csk;
@@ -456,7 +466,7 @@ void strp_unpause(struct strparser *strp)
 }
 EXPORT_SYMBOL_GPL(strp_unpause);
 
-/* strp must already be stopped so that strp_tcp_recv will no longer be called.
+/* strp must already be stopped so that strp_recv will no longer be called.
  * Note that strp_done is not called with the lower socket held.
  */
 void strp_done(struct strparser *strp)
-- 
2.8.0.rc2

Re: [PATCH net-next 0/3] strp: Generalize stream parser to work with other socket types

[David Miller]

From: Tom Herbert <tom@herbertland.com>
Date: Sun, 28 Aug 2016 14:43:16 -0700

> Add a read_sock protocol operation function that allows something like
> tcp_read_sock to be called for other protocol types.
> 
> Specific changes in this patch set:
>   - Add read_sock function to proto_ops. This has the same signature as
>     tcp_read_sock. sk_read_actor_t is also defined in net.h.
>   - Set peek_len and read_sock proto_op functions for TCPv4 and TCPv6
>     stream ops.
>   - Remove references to tcp in strparser.
>   - Call peek_len and read_sock operations from strparser instead of
>     calling TCP specific functions.

I'll apply this, but I want you to shore up these new ops.

A check has to happen somewhere to make sure the proto_ops in
question have a non-NULL read_sock and peek_len method before
starting to use it.

Re: [PATCH net-next 0/3] strp: Generalize stream parser to work with other socket types

[Tom Herbert]

On Sun, Aug 28, 2016 at 8:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Sun, 28 Aug 2016 14:43:16 -0700
>
>> Add a read_sock protocol operation function that allows something like
>> tcp_read_sock to be called for other protocol types.
>>
>> Specific changes in this patch set:
>>   - Add read_sock function to proto_ops. This has the same signature as
>>     tcp_read_sock. sk_read_actor_t is also defined in net.h.
>>   - Set peek_len and read_sock proto_op functions for TCPv4 and TCPv6
>>     stream ops.
>>   - Remove references to tcp in strparser.
>>   - Call peek_len and read_sock operations from strparser instead of
>>     calling TCP specific functions.
>
> I'll apply this, but I want you to shore up these new ops.
>
> A check has to happen somewhere to make sure the proto_ops in
> question have a non-NULL read_sock and peek_len method before
> starting to use it.

Is the check in strp_init not enough?...

@@ -424,9 +429,14 @@ static void strp_rx_msg_timeout(unsigned long arg)
 int strp_init(struct strparser *strp, struct sock *csk,
              struct strp_callbacks *cb)
 {
+       struct socket *sock = csk->sk_socket;
+
        if (!cb || !cb->rcv_msg || !cb->parse_msg)
                return -EINVAL;

+       if (!sock->ops->read_sock || !sock->ops->peek_len)
+               return -EAFNOSUPPORT;
+