lkml.org 
[lkml]   [2021]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v4 03/17] af_vsock: separate receive data loop
On Sun, Feb 07, 2021 at 06:15:05PM +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()'.
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>---
> net/vmw_vsock/af_vsock.c | 117 +++++++++++++++++++++++----------------
> 1 file changed, 68 insertions(+), 49 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 38927695786f..66c8a932f49b 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1898,65 +1898,22 @@ static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
> return err;
> }
>
>-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

At the end of __vsock_stream_recvmsg() you are calling release_sock(sk)
and it's wrong since we are releasing it in vsock_connectible_recvmsg().

Please fix it.

>@@ -2020,6 +1977,68 @@ vsock_connectible_recvmsg(struct socket *sock,
>struct msghdr *msg, size_t len,
> 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;
>+}
>+

The rest of the patch LGTM.

Stefano

\
 
 \ /
  Last update: 2021-02-11 12:58    [W:0.548 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site