Re: [RFC PATCH v5 10/19] virtio/vsock: simplify credit update function API

From: Stefano Garzarella <sgarzare@redhat.com>
To: Arseny Krasnov <arseny.krasnov@kaspersky.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Jorgen Hansen <jhansen@vmware.com>,
	Andra Paraschiv <andraprs@amazon.com>,
	Norbert Slusarek <nslusarek@gmx.net>,
	Colin Ian King <colin.king@canonical.com>,
	kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stsp2@yandex.ru, oxffffaa@gmail.com
Subject: Re: [RFC PATCH v5 10/19] virtio/vsock: simplify credit update function API
Date: Tue, 23 Feb 2021 14:49:20 +0100	[thread overview]
Message-ID: <20210223134920.nvecqujytdfcqnbt@steredhat> (raw)
In-Reply-To: <20210218053926.1068053-1-arseny.krasnov@kaspersky.com>

On Thu, Feb 18, 2021 at 08:39:23AM +0300, Arseny Krasnov wrote:
>'virtio_transport_send_credit_update()' has some extra args:
>1) 'type' may be set in 'virtio_transport_send_pkt_info()' using type
>   of socket.
>2) This function is static and 'hdr' arg was always NULL.
>
>Signed-off-by: Arseny Krasnov <arseny.krasnov@kaspersky.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 15 ++++-----------
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 1c9d71ca5e8e..833104b71a1c 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -271,13 +271,10 @@ void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit)
> }
> EXPORT_SYMBOL_GPL(virtio_transport_put_credit);
>
>-static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
>-					       int type,
>-					       struct virtio_vsock_hdr *hdr)
>+static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
> {
> 	struct virtio_vsock_pkt_info info = {
> 		.op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
>-		.type = type,
> 		.vsk = vsk,
> 	};

I don't know if it's better to remove type with the others changes in 
the previous patch, maybe it's more consistent.

I mean only the removal of 'type' parameter, the 'hdr' parameter should 
be removed with this patch.

>
>@@ -385,11 +382,8 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
> 	 * messages, we set the limit to a high value. TODO: experiment
> 	 * with different values.
> 	 */
>-	if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) {
>-		virtio_transport_send_credit_update(vsk,
>-						    
>VIRTIO_VSOCK_TYPE_STREAM,
>-						    NULL);
>-	}
>+	if (free_space < VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
>+		virtio_transport_send_credit_update(vsk);
>
> 	return total;
>
>@@ -498,8 +492,7 @@ void virtio_transport_notify_buffer_size(struct vsock_sock *vsk, u64 *val)
>
> 	vvs->buf_alloc = *val;
>
>-	virtio_transport_send_credit_update(vsk, VIRTIO_VSOCK_TYPE_STREAM,
>-					    NULL);
>+	virtio_transport_send_credit_update(vsk);
> }
> EXPORT_SYMBOL_GPL(virtio_transport_notify_buffer_size);
>
>-- 
>2.25.1
>