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

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

[Stefano Garzarella]

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 <sgarzare@redhat.com>

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

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

[Stefano Garzarella]

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 <sgarzare@redhat.com>
---
 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

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

[Arseny Krasnov]

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 <sgarzare@redhat.com>
> ---
>  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

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

[Stefano Garzarella]

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 <sgarzare@redhat.com>
---
 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

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

[Arseny Krasnov]

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 <sgarzare@redhat.com>
> ---
>  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

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

[Stefano Garzarella]

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 <sgarzare@redhat.com>
---
 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

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

[Arseny Krasnov]

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 <sgarzare@redhat.com>
> ---
>  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

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

[patchwork-bot+netdevbpf]

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 <sgarzare@redhat.com>
> 
> [...]

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