On Tue, Feb 11, 2025 at 03:19:22PM +0800, Junnan Wu wrote:
Function virtio_vsock_vqs_del will be invoked in 2 cases
virtio_vsock_remove() and virtio_vsock_freeze().

And when driver freeze, the connected socket will be set to TCP_CLOSE
and it will cause that socket can not be unusable after resume.

Refactor function virtio_vsock_vqs_del to differentiate the 2 use cases.

Co-developed-by: Ying Gao <ying01....@samsung.com>
Signed-off-by: Ying Gao <ying01....@samsung.com>
Signed-off-by: Junnan Wu <junnan01...@samsung.com>
---
net/vmw_vsock/virtio_transport.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

We are still discussing this in v1:

https://lore.kernel.org/virtualization/iv6oalr6yuwsfkoxnorp4t77fdjheteyojauwf2phshucdxatf@ominy3hfcpxb/T/#u

Please stop sending new versions before reaching an agreement!

BTW I still think this is wrong, so:

Nacked-by: Stefano Garzarella <sgarz...@redhat.com>



diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index f0e48e6911fc..909048c9069b 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -716,14 +716,20 @@ static void virtio_vsock_vqs_start(struct virtio_vsock 
*vsock)
        queue_work(virtio_vsock_workqueue, &vsock->send_pkt_work);
}

-static void virtio_vsock_vqs_del(struct virtio_vsock *vsock)
+static void virtio_vsock_vqs_del(struct virtio_vsock *vsock, bool vsock_reset)
{
        struct virtio_device *vdev = vsock->vdev;
        struct sk_buff *skb;

-       /* Reset all connected sockets when the VQs disappear */
-       vsock_for_each_connected_socket(&virtio_transport.transport,
-                                       virtio_vsock_reset_sock);
+       /* When driver is removed, reset all connected
+        * sockets because the VQs disappear.

You wrote it here too, you have to reset them because the VQs are going to disappear, isn't it the same after the freeze?

+        * When driver is just freezed, don't do that because the connected
+        * socket still need to use after restore.
+        */
+       if (vsock_reset) {
+               vsock_for_each_connected_socket(&virtio_transport.transport,
+                                               virtio_vsock_reset_sock);
+       }

        /* Stop all work handlers to make sure no one is accessing the device,
         * so we can safely call virtio_reset_device().
@@ -831,7 +837,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
        rcu_assign_pointer(the_virtio_vsock, NULL);
        synchronize_rcu();

-       virtio_vsock_vqs_del(vsock);
+       virtio_vsock_vqs_del(vsock, true);

        /* Other works can be queued before 'config->del_vqs()', so we flush
         * all works before to free the vsock object to avoid use after free.
@@ -856,7 +862,7 @@ static int virtio_vsock_freeze(struct virtio_device *vdev)
        rcu_assign_pointer(the_virtio_vsock, NULL);
        synchronize_rcu();

-       virtio_vsock_vqs_del(vsock);
+       virtio_vsock_vqs_del(vsock, false);

        mutex_unlock(&the_virtio_vsock_mutex);

--
2.34.1



Reply via email to