net/virtio: fix interrupt unregistering for listening socket

virtio_user_dev_server_reconnect() is typically called from the
interrupt context while checking the link state:

  vhost_user_update_link_state()
  --> virtio_user_dev_server_reconnect()

Under this conditions callback unregistering always fails.  This means
that listenfd is never unregistered and continue to trigger interrupts.
For example, if second client will try to connect to the same socket,
the server will receive interrupts infinitely because it will not
accept them while listen fd is readable and generates epoll events.

Fix that by moving reconfiguration of interrupts out of the
interrupt context to alarm handler.

'virtio_user_dev_delayed_handler' renamed to
'virtio_user_dev_delayed_disconnect_handler' to better reflect its
purpose.

Additionally improved error logging around interrupt management.

Fixes: bd8f50a45d0f ("net/virtio-user: support server mode")
Cc: stable@dpdk.org
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index bc56d60d6c3053c080f143e29629378778ebb7bb..aec666405f51504635bc90f6e93ab635dbafdaf6 100644 (file)
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -957,7 +957,9 @@ vhost_user_update_link_state(struct virtio_user_dev *dev)
                         * of interrupt handling, callback cannot be
                         * unregistered here, set an alarm to do it.
                         */
-                       rte_eal_alarm_set(1, virtio_user_dev_delayed_handler, (void *)dev);
+                       rte_eal_alarm_set(1,
+                               virtio_user_dev_delayed_disconnect_handler,
+                               (void *)dev);
                } else {
                        dev->net_status |= VIRTIO_NET_S_LINK_UP;
                }

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index d0776739a750f5a938cd82d6004a641f44a606bc..364f43e21c4598b55956f5d78a399955e43bda2e 100644 (file)
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -13,6 +13,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 
+#include <rte_alarm.h>
 #include <rte_string_fns.h>
 #include <rte_eal_memconfig.h>
 
@@ -885,7 +886,7 @@ virtio_user_dev_reset_queues_packed(struct rte_eth_dev *eth_dev)
 }
 
 void
-virtio_user_dev_delayed_handler(void *param)
+virtio_user_dev_delayed_disconnect_handler(void *param)
 {
        struct virtio_user_dev *dev = param;
        struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id];
@@ -894,14 +895,27 @@ virtio_user_dev_delayed_handler(void *param)
                PMD_DRV_LOG(ERR, "interrupt disable failed");
                return;
        }
-       rte_intr_callback_unregister(eth_dev->intr_handle,
-                                    virtio_interrupt_handler, eth_dev);
+       PMD_DRV_LOG(DEBUG, "Unregistering intr fd: %d",
+                   eth_dev->intr_handle->fd);
+       if (rte_intr_callback_unregister(eth_dev->intr_handle,
+                                        virtio_interrupt_handler,
+                                        eth_dev) != 1)
+               PMD_DRV_LOG(ERR, "interrupt unregister failed");
+
        if (dev->is_server) {
                if (dev->ops->server_disconnect)
                        dev->ops->server_disconnect(dev);
+
                eth_dev->intr_handle->fd = dev->ops->get_intr_fd(dev);
-               rte_intr_callback_register(eth_dev->intr_handle,
-                                          virtio_interrupt_handler, eth_dev);
+
+               PMD_DRV_LOG(DEBUG, "Registering intr fd: %d",
+                           eth_dev->intr_handle->fd);
+
+               if (rte_intr_callback_register(eth_dev->intr_handle,
+                                              virtio_interrupt_handler,
+                                              eth_dev))
+                       PMD_DRV_LOG(ERR, "interrupt register failed");
+
                if (rte_intr_enable(eth_dev->intr_handle) < 0) {
                        PMD_DRV_LOG(ERR, "interrupt enable failed");
                        return;
@@ -909,6 +923,32 @@ virtio_user_dev_delayed_handler(void *param)
        }
 }
 
+static void
+virtio_user_dev_delayed_intr_reconfig_handler(void *param)
+{
+       struct virtio_user_dev *dev = param;
+       struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->hw.port_id];
+
+       PMD_DRV_LOG(DEBUG, "Unregistering intr fd: %d",
+                   eth_dev->intr_handle->fd);
+
+       if (rte_intr_callback_unregister(eth_dev->intr_handle,
+                                        virtio_interrupt_handler,
+                                        eth_dev) != 1)
+               PMD_DRV_LOG(ERR, "interrupt unregister failed");
+
+       eth_dev->intr_handle->fd = dev->ops->get_intr_fd(dev);
+
+       PMD_DRV_LOG(DEBUG, "Registering intr fd: %d", eth_dev->intr_handle->fd);
+
+       if (rte_intr_callback_register(eth_dev->intr_handle,
+                                      virtio_interrupt_handler, eth_dev))
+               PMD_DRV_LOG(ERR, "interrupt register failed");
+
+       if (rte_intr_enable(eth_dev->intr_handle) < 0)
+               PMD_DRV_LOG(ERR, "interrupt enable failed");
+}
+
 int
 virtio_user_dev_server_reconnect(struct virtio_user_dev *dev)
 {
@@ -974,18 +1014,14 @@ virtio_user_dev_server_reconnect(struct virtio_user_dev *dev)
                        PMD_DRV_LOG(ERR, "interrupt disable failed");
                        return -1;
                }
-               rte_intr_callback_unregister(eth_dev->intr_handle,
-                                            virtio_interrupt_handler,
-                                            eth_dev);
-
-               eth_dev->intr_handle->fd = dev->ops->get_intr_fd(dev);
-               rte_intr_callback_register(eth_dev->intr_handle,
-                                          virtio_interrupt_handler, eth_dev);
-
-               if (rte_intr_enable(eth_dev->intr_handle) < 0) {
-                       PMD_DRV_LOG(ERR, "interrupt enable failed");
-                       return -1;
-               }
+               /*
+                * This function can be called from the interrupt handler, so
+                * we can't unregister interrupt handler here.  Setting
+                * alarm to do that later.
+                */
+               rte_eal_alarm_set(1,
+                       virtio_user_dev_delayed_intr_reconfig_handler,
+                       (void *)dev);
        }
        PMD_INIT_LOG(NOTICE, "server mode virtio-user reconnection succeeds!");
        return 0;

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 7fd4622f01fcf131165f9889ebc46a80ad1cfadd..58ad5198b639b6d1f4eb73233a34e6591f459c04 100644 (file)
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -77,7 +77,7 @@ uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs);
 int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status);
 int virtio_user_dev_update_status(struct virtio_user_dev *dev);
 int virtio_user_dev_update_link_state(struct virtio_user_dev *dev);
-void virtio_user_dev_delayed_handler(void *param);
+void virtio_user_dev_delayed_disconnect_handler(void *param);
 int virtio_user_dev_server_reconnect(struct virtio_user_dev *dev);
 extern const char * const virtio_user_backend_strings[];
 #endif