[PATCH RFC v2 0/2] vsock/virtio: fix issues on device hot-unplug

[PATCH RFC v2 0/2] vsock/virtio: fix issues on device hot-unplug

[Stefano Garzarella]

These patches try to handle the hot-unplug of vsock virtio transport device in
a proper way.

Maybe move the vsock_core_init()/vsock_core_exit() functions in the module_init
and module_exit of vsock_virtio_transport module can't be the best way, but the
architecture of vsock_core forces us to this approach for now.

The vsock_core proto_ops expect a valid pointer to the transport device, so we
can't call vsock_core_exit() until there are open sockets.

Another (little more complex) approach during the device removal, could be to
unregister the AF_VSOCK protocol, then reset all sockets and wait for their
destruction. At this point, we can set the transport pointer to NULL.

Any suggestions would be helpful.

v1 -> v2:
 - Fixed commit message of patch 1.

Stefano Garzarella (2):
  vsock/virtio: fix kernel panic after device hot-unplug
  vsock/virtio: reset connected sockets on device removal

 net/vmw_vsock/virtio_transport.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

-- 
2.20.1

[PATCH RFC v2 1/2] vsock/virtio: fix kernel panic after device hot-unplug

[Stefano Garzarella]

virtio_vsock_remove() invokes the vsock_core_exit() also if there
are opened sockets for the AF_VSOCK protocol family. In this way
the vsock "transport" pointer is set to NULL, triggering the
kernel panic at the first socket activity.

This patch move the vsock_core_init()/vsock_core_exit() in the
virtio_vsock respectively in module_init and module_exit functions,
that cannot be invoked until there are open sockets.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1609699
Reported-by: Yan Fu <yafu@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/vmw_vsock/virtio_transport.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5d3cce9e8744..9dae54698737 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -75,6 +75,9 @@ static u32 virtio_transport_get_local_cid(void)
 {
 	struct virtio_vsock *vsock = virtio_vsock_get();
 
+	if (!vsock)
+		return VMADDR_CID_ANY;
+
 	return vsock->guest_cid;
 }
 
@@ -584,10 +587,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 
 	virtio_vsock_update_guest_cid(vsock);
 
-	ret = vsock_core_init(&virtio_transport.transport);
-	if (ret < 0)
-		goto out_vqs;
-
 	vsock->rx_buf_nr = 0;
 	vsock->rx_buf_max_nr = 0;
 	atomic_set(&vsock->queued_replies, 0);
@@ -618,8 +617,6 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 	mutex_unlock(&the_virtio_vsock_mutex);
 	return 0;
 
-out_vqs:
-	vsock->vdev->config->del_vqs(vsock->vdev);
 out:
 	kfree(vsock);
 	mutex_unlock(&the_virtio_vsock_mutex);
@@ -669,7 +666,6 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 
 	mutex_lock(&the_virtio_vsock_mutex);
 	the_virtio_vsock = NULL;
-	vsock_core_exit();
 	mutex_unlock(&the_virtio_vsock_mutex);
 
 	vdev->config->del_vqs(vdev);
@@ -702,14 +698,28 @@ static int __init virtio_vsock_init(void)
 	virtio_vsock_workqueue = alloc_workqueue("virtio_vsock", 0, 0);
 	if (!virtio_vsock_workqueue)
 		return -ENOMEM;
+
 	ret = register_virtio_driver(&virtio_vsock_driver);
 	if (ret)
-		destroy_workqueue(virtio_vsock_workqueue);
+		goto out_wq;
+
+	ret = vsock_core_init(&virtio_transport.transport);
+	if (ret)
+		goto out_vdr;
+
+	return 0;
+
+out_vdr:
+	unregister_virtio_driver(&virtio_vsock_driver);
+out_wq:
+	destroy_workqueue(virtio_vsock_workqueue);
 	return ret;
+
 }
 
 static void __exit virtio_vsock_exit(void)
 {
+	vsock_core_exit();
 	unregister_virtio_driver(&virtio_vsock_driver);
 	destroy_workqueue(virtio_vsock_workqueue);
 }
-- 
2.20.1

[PATCH RFC v2 2/2] vsock/virtio: reset connected sockets on device removal

[Stefano Garzarella]

When the virtio transport device disappear, we should reset all
connected sockets in order to inform the users.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 net/vmw_vsock/virtio_transport.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 9dae54698737..15eb5d3d4750 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -634,6 +634,9 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	flush_work(&vsock->event_work);
 	flush_work(&vsock->send_pkt_work);
 
+	/* Reset all connected sockets when the device disappear */
+	vsock_for_each_connected_socket(virtio_vsock_reset_sock);
+
 	vdev->config->reset(vdev);
 
 	mutex_lock(&vsock->rx_lock);
-- 
2.20.1

Re: [PATCH RFC v2 0/2] vsock/virtio: fix issues on device hot-unplug

[Stefano Garzarella]

Kindly ping :)

On Fri, Jan 4, 2019 at 3:34 PM Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> These patches try to handle the hot-unplug of vsock virtio transport device in
> a proper way.
>
> Maybe move the vsock_core_init()/vsock_core_exit() functions in the module_init
> and module_exit of vsock_virtio_transport module can't be the best way, but the
> architecture of vsock_core forces us to this approach for now.
>
> The vsock_core proto_ops expect a valid pointer to the transport device, so we
> can't call vsock_core_exit() until there are open sockets.
>
> Another (little more complex) approach during the device removal, could be to
> unregister the AF_VSOCK protocol, then reset all sockets and wait for their
> destruction. At this point, we can set the transport pointer to NULL.
>
> Any suggestions would be helpful.
>
> v1 -> v2:
>  - Fixed commit message of patch 1.
>
> Stefano Garzarella (2):
>   vsock/virtio: fix kernel panic after device hot-unplug
>   vsock/virtio: reset connected sockets on device removal
>
>  net/vmw_vsock/virtio_transport.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> --
> 2.20.1
>

Re: [PATCH RFC v2 0/2] vsock/virtio: fix issues on device hot-unplug

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 286 bytes --]

On Tue, Jan 29, 2019 at 04:33:58PM +0100, Stefano Garzarella wrote:
> Kindly ping :)

Hi Stefano,
It probably didn't get picked up due to the "RFC" (Request for
Comments).

I suggest rebasing, retesting, and resending to be sure it will be
noticed and merged without conflicts.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [PATCH RFC v2 0/2] vsock/virtio: fix issues on device hot-unplug

[Stefano Garzarella]

On Fri, Feb 01, 2019 at 04:19:41PM +0800, Stefan Hajnoczi wrote:
> On Tue, Jan 29, 2019 at 04:33:58PM +0100, Stefano Garzarella wrote:
> > Kindly ping :)
>
> Hi Stefano,
> It probably didn't get picked up due to the "RFC" (Request for
> Comments).
>
> I suggest rebasing, retesting, and resending to be sure it will be
> noticed and merged without conflicts.
>

Hi Stefan,
I will send a v3 following your suggestions!

Thanks,
Stefano