2021-06-18 13:36:41

by Stefano Garzarella

[permalink] [raw]
Subject: [PATCH net-next 0/3] vsock: small fixes for seqpacket support

This series contains few patches to clean up a bit the code
of seqpacket recently merged in the net-next tree.

No functionality changes.

Signed-off-by: Stefano Garzarella <[email protected]>

Stefano Garzarella (3):
vsock: rename vsock_has_data()
vsock: rename vsock_wait_data()
vsock/virtio: remove redundant `copy_failed` variable

net/vmw_vsock/af_vsock.c | 18 ++++++++++--------
net/vmw_vsock/virtio_transport_common.c | 7 ++-----
2 files changed, 12 insertions(+), 13 deletions(-)

--
2.31.1


2021-06-18 13:37:33

by Stefano Garzarella

[permalink] [raw]
Subject: [PATCH net-next 1/3] vsock: rename vsock_has_data()

vsock_has_data() is used only by STREAM and SEQPACKET sockets,
so let's rename it to vsock_connectible_has_data(), using the same
nomenclature (connectible) used in other functions after the
introduction of SEQPACKET.

Signed-off-by: Stefano Garzarella <[email protected]>
---
net/vmw_vsock/af_vsock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 67954afef4e1..de8249483081 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -860,7 +860,7 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk)
}
EXPORT_SYMBOL_GPL(vsock_stream_has_data);

-static s64 vsock_has_data(struct vsock_sock *vsk)
+static s64 vsock_connectible_has_data(struct vsock_sock *vsk)
{
struct sock *sk = sk_vsock(vsk);

@@ -1880,7 +1880,7 @@ static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
err = 0;
transport = vsk->transport;

- while ((data = vsock_has_data(vsk)) == 0) {
+ while ((data = vsock_connectible_has_data(vsk)) == 0) {
prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);

if (sk->sk_err != 0 ||
--
2.31.1

2021-06-18 13:38:37

by Stefano Garzarella

[permalink] [raw]
Subject: [PATCH net-next 2/3] vsock: rename vsock_wait_data()

vsock_wait_data() is used only by STREAM and SEQPACKET sockets,
so let's rename it to vsock_connectible_wait_data(), using the same
nomenclature (connectible) used in other functions after the
introduction of SEQPACKET.

Signed-off-by: Stefano Garzarella <[email protected]>
---
net/vmw_vsock/af_vsock.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index de8249483081..21ccf450e249 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1866,10 +1866,11 @@ 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)
+static int vsock_connectible_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;
@@ -1967,7 +1968,8 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg,
while (1) {
ssize_t read;

- err = vsock_wait_data(sk, &wait, timeout, &recv_data, target);
+ err = vsock_connectible_wait_data(sk, &wait, timeout,
+ &recv_data, target);
if (err <= 0)
break;

@@ -2022,7 +2024,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,

timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);

- err = vsock_wait_data(sk, &wait, timeout, NULL, 0);
+ err = vsock_connectible_wait_data(sk, &wait, timeout, NULL, 0);
if (err <= 0)
goto out;

--
2.31.1

2021-06-18 13:39:54

by Stefano Garzarella

[permalink] [raw]
Subject: [PATCH net-next 3/3] vsock/virtio: remove redundant `copy_failed` variable

When memcpy_to_msg() fails in virtio_transport_seqpacket_do_dequeue(),
we already set `dequeued_len` with the negative error value returned
by memcpy_to_msg().

So we can directly check `dequeued_len` value instead of using a
dedicated flag variable to skip the copy path for the rest of
fragments.

Signed-off-by: Stefano Garzarella <[email protected]>
---
net/vmw_vsock/virtio_transport_common.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 23704a6bc437..f014ccfdd9c2 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -413,7 +413,6 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
struct virtio_vsock_pkt *pkt;
int dequeued_len = 0;
size_t user_buf_len = msg_data_left(msg);
- bool copy_failed = false;
bool msg_ready = false;

spin_lock_bh(&vvs->rx_lock);
@@ -426,7 +425,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
while (!msg_ready) {
pkt = list_first_entry(&vvs->rx_queue, struct virtio_vsock_pkt, list);

- if (!copy_failed) {
+ if (dequeued_len >= 0) {
size_t pkt_len;
size_t bytes_to_copy;

@@ -443,11 +442,9 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,

err = memcpy_to_msg(msg, pkt->buf, bytes_to_copy);
if (err) {
- /* Copy of message failed, set flag to skip
- * copy path for rest of fragments. Rest of
+ /* Copy of message failed. Rest of
* fragments will be freed without copy.
*/
- copy_failed = true;
dequeued_len = err;
} else {
user_buf_len -= bytes_to_copy;
--
2.31.1

2021-06-18 14:52:56

by Arseny Krasnov

[permalink] [raw]
Subject: Re: [PATCH net-next 2/3] vsock: rename vsock_wait_data()


On 18.06.2021 16:35, Stefano Garzarella wrote:
> vsock_wait_data() is used only by STREAM and SEQPACKET sockets,
> so let's rename it to vsock_connectible_wait_data(), using the same
> nomenclature (connectible) used in other functions after the
> introduction of SEQPACKET.
>
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
> net/vmw_vsock/af_vsock.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index de8249483081..21ccf450e249 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -1866,10 +1866,11 @@ 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)
> +static int vsock_connectible_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;
> @@ -1967,7 +1968,8 @@ static int __vsock_stream_recvmsg(struct sock *sk, struct msghdr *msg,
> while (1) {
> ssize_t read;
>
> - err = vsock_wait_data(sk, &wait, timeout, &recv_data, target);
> + err = vsock_connectible_wait_data(sk, &wait, timeout,
> + &recv_data, target);
> if (err <= 0)
> break;
>
> @@ -2022,7 +2024,7 @@ static int __vsock_seqpacket_recvmsg(struct sock *sk, struct msghdr *msg,
>
> timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
>
> - err = vsock_wait_data(sk, &wait, timeout, NULL, 0);
> + err = vsock_connectible_wait_data(sk, &wait, timeout, NULL, 0);
> if (err <= 0)
> goto out;
>
LGTM

2021-06-18 17:52:33

by Arseny Krasnov

[permalink] [raw]
Subject: Re: [PATCH net-next 1/3] vsock: rename vsock_has_data()


On 18.06.2021 16:35, Stefano Garzarella wrote:
> vsock_has_data() is used only by STREAM and SEQPACKET sockets,
> so let's rename it to vsock_connectible_has_data(), using the same
> nomenclature (connectible) used in other functions after the
> introduction of SEQPACKET.
>
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
> net/vmw_vsock/af_vsock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 67954afef4e1..de8249483081 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -860,7 +860,7 @@ s64 vsock_stream_has_data(struct vsock_sock *vsk)
> }
> EXPORT_SYMBOL_GPL(vsock_stream_has_data);
>
> -static s64 vsock_has_data(struct vsock_sock *vsk)
> +static s64 vsock_connectible_has_data(struct vsock_sock *vsk)
> {
> struct sock *sk = sk_vsock(vsk);
>
> @@ -1880,7 +1880,7 @@ static int vsock_wait_data(struct sock *sk, struct wait_queue_entry *wait,
> err = 0;
> transport = vsk->transport;
>
> - while ((data = vsock_has_data(vsk)) == 0) {
> + while ((data = vsock_connectible_has_data(vsk)) == 0) {
> prepare_to_wait(sk_sleep(sk), wait, TASK_INTERRUPTIBLE);
>
> if (sk->sk_err != 0 ||
LGTM

2021-06-18 17:52:57

by Arseny Krasnov

[permalink] [raw]
Subject: Re: [PATCH net-next 3/3] vsock/virtio: remove redundant `copy_failed` variable


On 18.06.2021 16:35, Stefano Garzarella wrote:
> When memcpy_to_msg() fails in virtio_transport_seqpacket_do_dequeue(),
> we already set `dequeued_len` with the negative error value returned
> by memcpy_to_msg().
>
> So we can directly check `dequeued_len` value instead of using a
> dedicated flag variable to skip the copy path for the rest of
> fragments.
>
> Signed-off-by: Stefano Garzarella <[email protected]>
> ---
> net/vmw_vsock/virtio_transport_common.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 23704a6bc437..f014ccfdd9c2 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -413,7 +413,6 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> struct virtio_vsock_pkt *pkt;
> int dequeued_len = 0;
> size_t user_buf_len = msg_data_left(msg);
> - bool copy_failed = false;
> bool msg_ready = false;
>
> spin_lock_bh(&vvs->rx_lock);
> @@ -426,7 +425,7 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
> while (!msg_ready) {
> pkt = list_first_entry(&vvs->rx_queue, struct virtio_vsock_pkt, list);
>
> - if (!copy_failed) {
> + if (dequeued_len >= 0) {
> size_t pkt_len;
> size_t bytes_to_copy;
>
> @@ -443,11 +442,9 @@ static int virtio_transport_seqpacket_do_dequeue(struct vsock_sock *vsk,
>
> err = memcpy_to_msg(msg, pkt->buf, bytes_to_copy);
> if (err) {
> - /* Copy of message failed, set flag to skip
> - * copy path for rest of fragments. Rest of
> + /* Copy of message failed. Rest of
> * fragments will be freed without copy.
> */
> - copy_failed = true;
> dequeued_len = err;
> } else {
> user_buf_len -= bytes_to_copy;
LGTM

2021-06-18 20:18:06

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net-next 0/3] vsock: small fixes for seqpacket support

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 18 Jun 2021 15:35:23 +0200 you wrote:
> This series contains few patches to clean up a bit the code
> of seqpacket recently merged in the net-next tree.
>
> No functionality changes.
>
> Signed-off-by: Stefano Garzarella <[email protected]>
>
> [...]

Here is the summary with links:
- [net-next,1/3] vsock: rename vsock_has_data()
https://git.kernel.org/netdev/net-next/c/cc97141afd76
- [net-next,2/3] vsock: rename vsock_wait_data()
https://git.kernel.org/netdev/net-next/c/0de5b2e67275
- [net-next,3/3] vsock/virtio: remove redundant `copy_failed` variable
https://git.kernel.org/netdev/net-next/c/91aa49a8fa0f

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html