From: Stefano Garzarella <sgarzare@redhat.com>
To: Arseny Krasnov <arseny.krasnov@kaspersky.com>
Cc: Andra Paraschiv <andraprs@amazon.com>,
kvm@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
Jeff Vander Stoep <jeffv@google.com>,
stsp2@yandex.ru, linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org, oxffffaa@gmail.com,
netdev@vger.kernel.org, Norbert Slusarek <nslusarek@gmx.net>,
Stefan Hajnoczi <stefanha@redhat.com>,
Colin Ian King <colin.king@canonical.com>,
Jakub Kicinski <kuba@kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Jorgen Hansen <jhansen@vmware.com>,
Alexander Popov <alex.popov@linux.com>
Subject: Re: [RFC PATCH v7 04/22] af_vsock: implement SEQPACKET receive loop
Date: Thu, 25 Mar 2021 10:34:15 +0100 [thread overview]
Message-ID: <20210325093415.mpysybt5vfnsl7fg@steredhat> (raw)
In-Reply-To: <20210323131006.2460058-1-arseny.krasnov@kaspersky.com>
On Tue, Mar 23, 2021 at 04:10:03PM +0300, Arseny Krasnov wrote:
>This adds receive loop for SEQPACKET. It looks like receive loop for
>STREAM, but there is a little bit difference:
>1) It doesn't call notify callbacks.
>2) It doesn't care about 'SO_SNDLOWAT' and 'SO_RCVLOWAT' values, because
> there is no sense for these values in SEQPACKET case.
>3) It waits until whole record is received or error is found during
> receiving.
>4) It processes and sets 'MSG_TRUNC' flag.
>
>So to avoid extra conditions for two types of socket inside one loop, two
>independent functions were created.
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>---
> v6 -> v7:
> 'seqpacket_get_len' callback now removed, length of message is returned
> by 'seqpacket_dequeue' callback.
>
> include/net/af_vsock.h | 4 ++
> net/vmw_vsock/af_vsock.c | 88 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 91 insertions(+), 1 deletion(-)
>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index b1c717286993..74ac8a4c4168 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -135,6 +135,10 @@ struct vsock_transport {
> bool (*stream_is_active)(struct vsock_sock *);
> bool (*stream_allow)(u32 cid, u32 port);
>
>+ /* SEQ_PACKET. */
>+ int (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg,
>+ int flags, bool *msg_ready, size_t *record_len);
>+
Why not using ssize_t as return value and return the length or a
negative value in case of error?
In this way we can remove the 'record_len' parameter.
> /* Notification. */
> int (*notify_poll_in)(struct vsock_sock *, size_t, bool *);
> int (*notify_poll_out)(struct vsock_sock *, size_t, bool *);
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 0bc661e54262..fa0c37f97330 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1973,6 +1973,89 @@ static int __vsock_stream_recvmsg(struct sock
>*sk, struct msghdr *msg,
> return err;
> }
>
>+static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr
>*msg,
>+ size_t len, int flags)
>+{
>+ const struct vsock_transport *transport;
>+ const struct iovec *orig_iov;
>+ unsigned long orig_nr_segs;
>+ bool msg_ready;
>+ struct vsock_sock *vsk;
>+ size_t record_len;
>+ long timeout;
>+ int err = 0;
>+ DEFINE_WAIT(wait);
>+
>+ vsk = vsock_sk(sk);
>+ transport = vsk->transport;
>+
>+ timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
>+ orig_nr_segs = msg->msg_iter.nr_segs;
>+ orig_iov = msg->msg_iter.iov;
>+ msg_ready = false;
>+ record_len = 0;
>+
>+ while (1) {
>+ err = vsock_wait_data(sk, &wait, timeout, NULL, 0);
>+
>+ if (err <= 0) {
>+ /* In case of any loop break(timeout, signal
>+ * interrupt or shutdown), we report user that
>+ * nothing was copied.
>+ */
>+ err = 0;
>+ break;
>+ }
>+
>+ err = transport->seqpacket_dequeue(vsk, msg, flags, &msg_ready, &record_len);
>+
>+ if (err < 0) {
>+ if (err == -EAGAIN) {
>+ iov_iter_init(&msg->msg_iter, READ,
>+ orig_iov, orig_nr_segs,
>+ len);
>+ /* Clear 'MSG_EOR' here, because dequeue
>+ * callback above set it again if it was
>+ * set by sender. This 'MSG_EOR' is from
>+ * dropped record.
>+ */
>+ msg->msg_flags &= ~MSG_EOR;
>+ record_len = 0;
>+ continue;
>+ }
>+
>+ err = -ENOMEM;
>+ break;
>+ }
>+
>+ if (msg_ready)
>+ break;
>+ }
>+
>+ if (sk->sk_err)
>+ err = -sk->sk_err;
>+ else if (sk->sk_shutdown & RCV_SHUTDOWN)
>+ err = 0;
>+
>+ if (msg_ready) {
If 'err' is not 0, should we skip this branch?
>+ /* User sets MSG_TRUNC, so return real length of
>+ * packet.
>+ */
>+ if (flags & MSG_TRUNC)
>+ err = record_len;
>+ else
>+ err = len - msg->msg_iter.count;
>+
>+ /* Always set MSG_TRUNC if real length of packet is
>+ * bigger than user's buffer.
>+ */
>+ if (record_len > len)
>+ msg->msg_flags |= MSG_TRUNC;
>+ }
>+
>+ return err;
>+}
>+
> static int
> vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> int flags)
>@@ -2028,7 +2111,10 @@ vsock_connectible_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> goto out;
> }
>
>- err = __vsock_stream_recvmsg(sk, msg, len, flags);
>+ if (sk->sk_type == SOCK_STREAM)
>+ err = __vsock_stream_recvmsg(sk, msg, len, flags);
>+ else
>+ err = __vsock_seqpacket_recvmsg(sk, msg, len, flags);
>
> out:
> release_sock(sk);
>--
>2.25.1
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2021-03-25 9:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20210323130716.2459195-1-arseny.krasnov@kaspersky.com>
[not found] ` <20210323130939.2459901-1-arseny.krasnov@kaspersky.com>
2021-03-25 9:06 ` [RFC PATCH v7 03/22] af_vsock: separate receive data loop Stefano Garzarella
[not found] ` <20210323131006.2460058-1-arseny.krasnov@kaspersky.com>
2021-03-25 9:34 ` Stefano Garzarella [this message]
[not found] ` <20210323131026.2460194-1-arseny.krasnov@kaspersky.com>
2021-03-25 9:37 ` [RFC PATCH v7 05/22] af_vsock: separate wait space loop Stefano Garzarella
[not found] ` <20210323131045.2460319-1-arseny.krasnov@kaspersky.com>
2021-03-25 9:42 ` [RFC PATCH v7 06/22] af_vsock: implement send logic for SEQPACKET Stefano Garzarella
[not found] ` <20210323131244.2461050-1-arseny.krasnov@kaspersky.com>
2021-03-25 9:56 ` [RFC PATCH v7 11/22] virtio/vsock: dequeue callback for SOCK_SEQPACKET Stefano Garzarella
[not found] ` <20210323131258.2461163-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:08 ` [RFC PATCH v7 12/22] virtio/vsock: fetch length for SEQPACKET record Stefano Garzarella
[not found] ` <20210323131316.2461284-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:09 ` [RFC PATCH v7 13/22] virtio/vsock: add SEQPACKET receive logic Stefano Garzarella
[not found] ` <20210323131332.2461409-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:18 ` [RFC PATCH v7 14/22] virtio/vsock: rest of SOCK_SEQPACKET support Stefano Garzarella
[not found] ` <20210323131352.2461534-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:26 ` [RFC PATCH v7 15/22] virtio/vsock: SEQPACKET support feature bit Stefano Garzarella
[not found] ` <20210323131406.2461651-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:39 ` [RFC PATCH v7 16/22] virtio/vsock: setup SEQPACKET ops for transport Stefano Garzarella
[not found] ` <20210323131421.2461760-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:42 ` [RFC PATCH v7 17/22] vhost/vsock: " Stefano Garzarella
[not found] ` <20210323131436.2461881-1-arseny.krasnov@kaspersky.com>
2021-03-25 10:48 ` [RFC PATCH v7 18/22] vsock/loopback: " Stefano Garzarella
2021-03-25 10:52 ` [RFC PATCH v7 00/22] virtio/vsock: introduce SOCK_SEQPACKET support Stefano Garzarella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210325093415.mpysybt5vfnsl7fg@steredhat \
--to=sgarzare@redhat.com \
--cc=alex.popov@linux.com \
--cc=andraprs@amazon.com \
--cc=arseny.krasnov@kaspersky.com \
--cc=colin.king@canonical.com \
--cc=davem@davemloft.net \
--cc=jeffv@google.com \
--cc=jhansen@vmware.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=nslusarek@gmx.net \
--cc=oxffffaa@gmail.com \
--cc=stefanha@redhat.com \
--cc=stsp2@yandex.ru \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).