| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210222112924.hu2sfoiwni5kt5wm@steredhat>
Date: Mon, 22 Feb 2021 12:29:24 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Arseny Krasnov <arseny.krasnov@...persky.com>
Cc: Stefan Hajnoczi <stefanha@...hat.com>,
"Michael S. Tsirkin" <mst@...hat.com>,
Jason Wang <jasowang@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Jorgen Hansen <jhansen@...are.com>,
Andra Paraschiv <andraprs@...zon.com>,
Colin Ian King <colin.king@...onical.com>,
Norbert Slusarek <nslusarek@....net>, kvm@...r.kernel.org,
virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, stsp2@...dex.ru, oxffffaa@...il.com
Subject: Re: [RFC PATCH v5 02/19] af_vsock: separate wait data loop
On Thu, Feb 18, 2021 at 08:36:33AM +0300, Arseny Krasnov wrote:
>This moves wait loop for data to dedicated function, because later
>it will be used by SEQPACKET data receive loop.
The patch LGTM, maybe just add a line in the commit message with
something like this:
While moving the code around, let's update an old comment.
Whit that fixed:
Reviewed-by: Stefano Garzarella <sgarzare@...hat.com>
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@...persky.com>
>---
> net/vmw_vsock/af_vsock.c | 155 +++++++++++++++++++++------------------
> 1 file changed, 83 insertions(+), 72 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 656370e11707..6cf7bb977aa1 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1832,6 +1832,68 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
> return err;
> }
>
>+static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
>+ long timeout,
>+ struct vsock_transport_recv_notify_data *recv_data,
>+ size_t target)
>+{
>+ const struct vsock_transport *transport;
>+ struct vsock_sock *vsk;
>+ s64 data;
>+ int err;
>+
>+ vsk = vsock_sk(sk);
>+ err = 0;
>+ transport = vsk->transport;
>+ prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);
>+
>+ while ((data = vsock_stream_has_data(vsk)) == 0) {
>+ if (sk->sk_err != 0 ||
>+ (sk->sk_shutdown & RCV_SHUTDOWN) ||
>+ (vsk->peer_shutdown & SEND_SHUTDOWN)) {
>+ break;
>+ }
>+
>+ /* Don't wait for non-blocking sockets. */
>+ if (timeout == 0) {
>+ err = -EAGAIN;
>+ break;
>+ }
>+
>+ if (recv_data) {
>+ err = transport->notify_recv_pre_block(vsk, target, recv_data);
>+ if (err < 0)
>+ break;
>+ }
>+
>+ release_sock(sk);
>+ timeout = schedule_timeout(timeout);
>+ lock_sock(sk);
>+
>+ if (signal_pending(current)) {
>+ err = sock_intr_errno(timeout);
>+ break;
>+ } else if (timeout == 0) {
>+ err = -EAGAIN;
>+ break;
>+ }
>+ }
>+
>+ finish_wait(sk_sleep(sk), wait);
>+
>+ if (err)
>+ return err;
>+
>+ /* Internal transport error when checking for available
>+ * data. XXX This should be changed to a connection
>+ * reset in a later change.
>+ */
>+ if (data < 0)
>+ return -ENOMEM;
>+
>+ return data;
>+}
>+
> static int
> vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> int flags)
>@@ -1911,85 +1973,34 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
>
>
> while (1) {
>- s64 ready;
>-
>- prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
>- ready = vsock_stream_has_data(vsk);
>-
>- if (ready == 0) {
>- if (sk->sk_err != 0 ||
>- (sk->sk_shutdown & RCV_SHUTDOWN) ||
>- (vsk->peer_shutdown & SEND_SHUTDOWN)) {
>- finish_wait(sk_sleep(sk), &wait);
>- break;
>- }
>- /* Don't wait for non-blocking sockets. */
>- if (timeout == 0) {
>- err = -EAGAIN;
>- finish_wait(sk_sleep(sk), &wait);
>- break;
>- }
>+ ssize_t read;
>
>- err = transport->notify_recv_pre_block(
>- vsk, target, &recv_data);
>- if (err < 0) {
>- finish_wait(sk_sleep(sk), &wait);
>- break;
>- }
>- release_sock(sk);
>- timeout = schedule_timeout(timeout);
>- lock_sock(sk);
>+ err = vsock_wait_data(sk, &wait, timeout, &recv_data, target);
>+ if (err <= 0)
>+ break;
>
>- if (signal_pending(current)) {
>- err = sock_intr_errno(timeout);
>- finish_wait(sk_sleep(sk), &wait);
>- break;
>- } else if (timeout == 0) {
>- err = -EAGAIN;
>- finish_wait(sk_sleep(sk), &wait);
>- break;
>- }
>- } else {
>- ssize_t read;
>-
>- finish_wait(sk_sleep(sk), &wait);
>-
>- if (ready < 0) {
>- /* Invalid queue pair content. XXX This should
>- * be changed to a connection reset in a later
>- * change.
>- */
>-
>- err = -ENOMEM;
>- goto out;
>- }
>-
>- err = transport->notify_recv_pre_dequeue(
>- vsk, target, &recv_data);
>- if (err < 0)
>- break;
>+ err = transport->notify_recv_pre_dequeue(vsk, target,
>+ &recv_data);
>+ if (err < 0)
>+ break;
>
>- read = transport->stream_dequeue(
>- vsk, msg,
>- len - copied, flags);
>- if (read < 0) {
>- err = -ENOMEM;
>- break;
>- }
>+ read = transport->stream_dequeue(vsk, msg, len - copied, flags);
>+ if (read < 0) {
>+ err = -ENOMEM;
>+ break;
>+ }
>
>- copied += read;
>+ copied += read;
>
>- err = transport->notify_recv_post_dequeue(
>- vsk, target, read,
>- !(flags & MSG_PEEK), &recv_data);
>- if (err < 0)
>- goto out;
>+ err = transport->notify_recv_post_dequeue(vsk, target, read,
>+ !(flags & MSG_PEEK), &recv_data);
>+ if (err < 0)
>+ goto out;
>
>- if (read >= target || flags & MSG_PEEK)
>- break;
>+ if (read >= target || flags & MSG_PEEK)
>+ break;
>
>- target -= read;
>- }
>+ target -= read;
> }
>
> if (sk->sk_err)
>--
>2.25.1
>
Powered by blists - more mailing lists