Re: [Patch net 2/2] vsock/virtio: Don't reset the created SOCKET during suspend to ram
From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2025-02-11 08:49:41
Also in:
kvm, lkml, virtualization
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 <redacted> Signed-off-by: Ying Gao <redacted> Signed-off-by: Junnan Wu <redacted> --- 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 (local) Please stop sending new versions before reaching an agreement! BTW I still think this is wrong, so: Nacked-by: Stefano Garzarella [off-list ref]
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
+ * 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