netdev - Re: [PATCH 1/2] vsock/virtio: Move rx_buf_nr and rx_buf_max

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eebvrzx7zjbc326ycs3coskq5cajaoxcblp3wvcxfqaics2a2z@342wvb6zexs6>
Date: Mon, 10 Feb 2025 12:02:38 +0100
From: Luigi Leonardi <leonardi@...hat.com>
To: Junnan Wu <junnan01.wu@...sung.com>
Cc: stefanha@...hat.com, sgarzare@...hat.com, kvm@...r.kernel.org, 
	virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org, mindong.zhao@...sung.com, 
	q1.huang@...sung.com, ying01.gao@...sung.com, ying123.xu@...sung.com
Subject: Re: [PATCH 1/2] vsock/virtio: Move rx_buf_nr and rx_buf_max_nr
 initialization position

Hi Junnan, Ying

Thank you for the contribution!

A few minor comments on the process:

I think this series is missing a cover letter, not all the maintainers 
have been CCd, and you should add the tag net (because it's a fix) to 
the subject. (e.g. [PATCH net 1/2]).
Here you can find some useful information[1].

[1]https://www.kernel.org/doc/html/latest/process/submitting-patches.html

On Fri, Feb 07, 2025 at 01:20:32PM +0800, Junnan Wu wrote:
>From: Ying Gao <ying01.gao@...sung.com>
>
>In function virtio_vsock_probe, it initializes the variables
>"rx_buf_nr" and "rx_buf_max_nr",
>but in function virtio_vsock_restore it doesn't.
>
>Move the initizalition position into function virtio_vsock_vqs_start.
>
>Once executing s2r twice in a row without
I guess "s2r" is "suspend to resume" but is not that clear to me.

>initializing rx_buf_nr and rx_buf_max_nr,
>the rx_buf_max_nr increased to three times vq->num_free,
>at this time, in function virtio_transport_rx_work,
>the conditions to fill rx buffer
>(rx_buf_nr < rx_buf_max_nr / 2) can't be met.
>
>Signed-off-by: Ying Gao <ying01.gao@...sung.com>
>Signed-off-by: Junnan Wu <junnan01.wu@...sung.com>

Maybe you need a "Co-Developed-by"?

>---
> net/vmw_vsock/virtio_transport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index b58c3818f284..9eefd0fba92b 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -688,6 +688,8 @@ static void virtio_vsock_vqs_start(struct virtio_vsock *vsock)
> 	mutex_unlock(&vsock->tx_lock);
>
> 	mutex_lock(&vsock->rx_lock);
>+	vsock->rx_buf_nr = 0;
>+	vsock->rx_buf_max_nr = 0;
> 	virtio_vsock_rx_fill(vsock);
> 	vsock->rx_run = true;
> 	mutex_unlock(&vsock->rx_lock);
>@@ -779,8 +781,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
>
> 	vsock->vdev = vdev;
>
>-	vsock->rx_buf_nr = 0;
>-	vsock->rx_buf_max_nr = 0;
> 	atomic_set(&vsock->queued_replies, 0);
>
> 	mutex_init(&vsock->tx_lock);
>-- 
>2.34.1
>

Code LGTM.

Thank you,
Luigi