Stefano Garzarella
2021-Feb-22 11:43 UTC
[RFC PATCH v5 03/19] af_vsock: separate receive data loop
On Thu, Feb 18, 2021 at 08:36:50AM +0300, Arseny Krasnov wrote:>This moves STREAM specific data receive logic to dedicated function: >'__vsock_stream_recvmsg()', while checks that will be same for both >types of socket are in shared function: 'vsock_connectible_recvmsg()'.I'm not a native speaker, but I would rewrite this message like this: Move STREAM specific data receive logic to '__vsock_stream_recvmsg()' dedicated function, while checks, that will be same for both STREAM and SEQPACKET sockets, stays in 'vsock_connectible_recvmsg()' shared functions. Anyway the patch LGTM: Reviewed-by: Stefano Garzarella <sgarzare at redhat.com>> >Signed-off-by: Arseny Krasnov <arseny.krasnov at kaspersky.com> >--- > net/vmw_vsock/af_vsock.c | 116 ++++++++++++++++++++++----------------- > 1 file changed, 67 insertions(+), 49 deletions(-) > >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >index 6cf7bb977aa1..d277dc1cdbdf 100644 >--- a/net/vmw_vsock/af_vsock.c >+++ b/net/vmw_vsock/af_vsock.c >@@ -1894,65 +1894,22 @@ static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait, > return data; > } > >-static int >-vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, >- int flags) >+static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg, >+ size_t len, int flags) > { >- struct sock *sk; >- struct vsock_sock *vsk; >+ struct vsock_transport_recv_notify_data recv_data; > const struct vsock_transport *transport; >- int err; >- size_t target; >+ struct vsock_sock *vsk; > ssize_t copied; >+ size_t target; > long timeout; >- struct vsock_transport_recv_notify_data recv_data; >+ int err; > > DEFINE_WAIT(wait); > >- sk = sock->sk; > vsk = vsock_sk(sk); >- err = 0; >- >- lock_sock(sk); >- > transport = vsk->transport; > >- if (!transport || sk->sk_state != TCP_ESTABLISHED) { >- /* Recvmsg is supposed to return 0 if a peer performs an >- * orderly shutdown. Differentiate between that case and when a >- * peer has not connected or a local shutdown occured >with the >- * SOCK_DONE flag. >- */ >- if (sock_flag(sk, SOCK_DONE)) >- err = 0; >- else >- err = -ENOTCONN; >- >- goto out; >- } >- >- if (flags & MSG_OOB) { >- err = -EOPNOTSUPP; >- goto out; >- } >- >- /* We don't check peer_shutdown flag here since peer may actually shut >- * down, but there can be data in the queue that a local socket can >- * receive. >- */ >- if (sk->sk_shutdown & RCV_SHUTDOWN) { >- err = 0; >- goto out; >- } >- >- /* It is valid on Linux to pass in a zero-length receive buffer. This >- * is not an error. We may as well bail out now. >- */ >- if (!len) { >- err = 0; >- goto out; >- } >- > /* We must not copy less than target bytes into the user's buffer > * before returning successfully, so we wait for the consume queue to > * have that much data to consume before dequeueing. Note that this >@@ -2011,6 +1968,67 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, > if (copied > 0) > err = copied; > >+out: >+ return err; >+} >+ >+static int >+vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, >+ int flags) >+{ >+ struct sock *sk; >+ struct vsock_sock *vsk; >+ const struct vsock_transport *transport; >+ int err; >+ >+ DEFINE_WAIT(wait); >+ >+ sk = sock->sk; >+ vsk = vsock_sk(sk); >+ err = 0; >+ >+ lock_sock(sk); >+ >+ transport = vsk->transport; >+ >+ if (!transport || sk->sk_state != TCP_ESTABLISHED) { >+ /* Recvmsg is supposed to return 0 if a peer performs an >+ * orderly shutdown. Differentiate between that case and >when a >+ * peer has not connected or a local shutdown occurred with the >+ * SOCK_DONE flag. >+ */ >+ if (sock_flag(sk, SOCK_DONE)) >+ err = 0; >+ else >+ err = -ENOTCONN; >+ >+ goto out; >+ } >+ >+ if (flags & MSG_OOB) { >+ err = -EOPNOTSUPP; >+ goto out; >+ } >+ >+ /* We don't check peer_shutdown flag here since peer may actually shut >+ * down, but there can be data in the queue that a local socket can >+ * receive. >+ */ >+ if (sk->sk_shutdown & RCV_SHUTDOWN) { >+ err = 0; >+ goto out; >+ } >+ >+ /* It is valid on Linux to pass in a zero-length receive buffer. This >+ * is not an error. We may as well bail out now. >+ */ >+ if (!len) { >+ err = 0; >+ goto out; >+ } >+ >+ err = __vsock_stream_recvmsg(sk, msg, len, flags); >+ > out: > release_sock(sk); > return err; >-- >2.25.1 >