From 2324206337c551e0f19c223530361248167c613e Mon Sep 17 00:00:00 2001 From: Matan Azrad Date: Tue, 22 Oct 2019 07:33:35 +0000 Subject: [PATCH] net/mlx5: fix DevX event registration timing The DevX counter management triggers an asynchronous event to get back the new counters values from the HW. The counter management doesn't trigger 2 parallel events for the same pool, hence, the pool cannot be updated again in the event waiting time. When the port is stopped, the DevX event mechanism wrongly was destroyed what remained all the waiting pools in waiting state forever. As a result, the counters of the stuck pools were never updated again. Separate the DevX interrupt installation from the dev installation and remove the DevX interrupt unregistration\registration from the stop\start operations. Now, the DevX interrupt should be installed in probe and uninstalled in close. Cc: stable@dpdk.org Fixes: f15db67df09c ("net/mlx5: accelerate DV flow counter query") Signed-off-by: Matan Azrad Acked-by: Viacheslav Ovsiienko --- drivers/net/mlx5/mlx5.c | 15 +++- drivers/net/mlx5/mlx5.h | 4 + drivers/net/mlx5/mlx5_ethdev.c | 153 +++++++++++++++++++++++++-------- 3 files changed, 137 insertions(+), 35 deletions(-) diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index 7e0d7ec219..fac510507d 100644 --- a/drivers/net/mlx5/mlx5.c +++ b/drivers/net/mlx5/mlx5.c @@ -379,8 +379,10 @@ mlx5_alloc_shared_ibctx(const struct mlx5_dev_spawn_data *spawn) * there is no interrupt subhandler installed for * the given port index i. */ - for (i = 0; i < sh->max_port; i++) + for (i = 0; i < sh->max_port; i++) { sh->port[i].ih_port_id = RTE_MAX_ETHPORTS; + sh->port[i].devx_ih_port_id = RTE_MAX_ETHPORTS; + } sh->pd = mlx5_glue->alloc_pd(sh->ctx); if (sh->pd == NULL) { DRV_LOG(ERR, "PD allocation failure"); @@ -481,6 +483,15 @@ mlx5_free_shared_ibctx(struct mlx5_ibv_shared *sh) if (sh->intr_cnt) mlx5_intr_callback_unregister (&sh->intr_handle, mlx5_dev_interrupt_handler, sh); +#ifdef HAVE_MLX5_DEVX_ASYNC_SUPPORT + if (sh->devx_intr_cnt) { + if (sh->intr_handle_devx.fd) + rte_intr_callback_unregister(&sh->intr_handle_devx, + mlx5_dev_interrupt_handler_devx, sh); + if (sh->devx_comp) + mlx5dv_devx_destroy_cmd_comp(sh->devx_comp); + } +#endif pthread_mutex_destroy(&sh->intr_mutex); if (sh->pd) claim_zero(mlx5_glue->dealloc_pd(sh->pd)); @@ -845,6 +856,7 @@ mlx5_dev_close(struct rte_eth_dev *dev) ((priv->sh->ctx != NULL) ? priv->sh->ctx->device->name : "")); /* In case mlx5_dev_stop() has not been called. */ mlx5_dev_interrupt_handler_uninstall(dev); + mlx5_dev_interrupt_handler_devx_uninstall(dev); mlx5_traffic_disable(dev); mlx5_flow_flush(dev, NULL); /* Prevent crashes when queues are still in use. */ @@ -2739,6 +2751,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, rte_eth_copy_pci_info(list[i].eth_dev, pci_dev); /* Restore non-PCI flags cleared by the above call. */ list[i].eth_dev->data->dev_flags |= restore; + mlx5_dev_interrupt_handler_devx_install(list[i].eth_dev); rte_eth_dev_probing_finish(list[i].eth_dev); } if (i != ns) { diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index bac1c0a828..b6a51b2b4d 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -501,6 +501,7 @@ struct mlx5_flow_counter_mng { /* Per port data of shared IB device. */ struct mlx5_ibv_shared_port { uint32_t ih_port_id; + uint32_t devx_ih_port_id; /* * Interrupt handler port_id. Used by shared interrupt * handler to find the corresponding rte_eth device @@ -588,6 +589,7 @@ struct mlx5_ibv_shared { pthread_mutex_t intr_mutex; /* Interrupt config mutex. */ uint32_t intr_cnt; /* Interrupt handler reference counter. */ struct rte_intr_handle intr_handle; /* Interrupt handler for device. */ + uint32_t devx_intr_cnt; /* Devx interrupt handler reference counter. */ struct rte_intr_handle intr_handle_devx; /* DEVX interrupt handler. */ struct mlx5dv_devx_cmd_comp *devx_comp; /* DEVX async comp obj. */ struct mlx5_ibv_shared_port port[]; /* per device port data array. */ @@ -721,6 +723,8 @@ void mlx5_dev_interrupt_handler(void *arg); void mlx5_dev_interrupt_handler_devx(void *arg); void mlx5_dev_interrupt_handler_uninstall(struct rte_eth_dev *dev); void mlx5_dev_interrupt_handler_install(struct rte_eth_dev *dev); +void mlx5_dev_interrupt_handler_devx_uninstall(struct rte_eth_dev *dev); +void mlx5_dev_interrupt_handler_devx_install(struct rte_eth_dev *dev); int mlx5_set_link_down(struct rte_eth_dev *dev); int mlx5_set_link_up(struct rte_eth_dev *dev); int mlx5_is_removed(struct rte_eth_dev *dev); diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index 5f05b2bb50..2278b24c01 100644 --- a/drivers/net/mlx5/mlx5_ethdev.c +++ b/drivers/net/mlx5/mlx5_ethdev.c @@ -1448,6 +1448,37 @@ mlx5_dev_shared_handler_uninstall(struct rte_eth_dev *dev) mlx5_dev_interrupt_handler, sh); sh->intr_handle.fd = 0; sh->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; +exit: + pthread_mutex_unlock(&sh->intr_mutex); +} + +/** + * Uninstall devx shared asynchronous device events handler. + * This function is implemeted to support event sharing + * between multiple ports of single IB device. + * + * @param dev + * Pointer to Ethernet device. + */ +static void +mlx5_dev_shared_handler_devx_uninstall(struct rte_eth_dev *dev) +{ + struct mlx5_priv *priv = dev->data->dev_private; + struct mlx5_ibv_shared *sh = priv->sh; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) + return; + pthread_mutex_lock(&sh->intr_mutex); + assert(priv->ibv_port); + assert(priv->ibv_port <= sh->max_port); + assert(dev->data->port_id < RTE_MAX_ETHPORTS); + if (sh->port[priv->ibv_port - 1].devx_ih_port_id >= RTE_MAX_ETHPORTS) + goto exit; + assert(sh->port[priv->ibv_port - 1].devx_ih_port_id == + (uint32_t)dev->data->port_id); + sh->port[priv->ibv_port - 1].devx_ih_port_id = RTE_MAX_ETHPORTS; + if (!sh->devx_intr_cnt || --sh->devx_intr_cnt) + goto exit; if (sh->intr_handle_devx.fd) { rte_intr_callback_unregister(&sh->intr_handle_devx, mlx5_dev_interrupt_handler_devx, @@ -1490,8 +1521,9 @@ mlx5_dev_shared_handler_install(struct rte_eth_dev *dev) assert(sh->intr_cnt); goto exit; } - sh->port[priv->ibv_port - 1].ih_port_id = (uint32_t)dev->data->port_id; if (sh->intr_cnt) { + sh->port[priv->ibv_port - 1].ih_port_id = + (uint32_t)dev->data->port_id; sh->intr_cnt++; goto exit; } @@ -1500,52 +1532,81 @@ mlx5_dev_shared_handler_install(struct rte_eth_dev *dev) flags = fcntl(sh->ctx->async_fd, F_GETFL); ret = fcntl(sh->ctx->async_fd, F_SETFL, flags | O_NONBLOCK); if (ret) { - DRV_LOG(INFO, "failed to change file descriptor" - " async event queue"); - goto error; + DRV_LOG(INFO, "failed to change file descriptor async event" + " queue"); + /* Indicate there will be no interrupts. */ + dev->data->dev_conf.intr_conf.lsc = 0; + dev->data->dev_conf.intr_conf.rmv = 0; + } else { + sh->intr_handle.fd = sh->ctx->async_fd; + sh->intr_handle.type = RTE_INTR_HANDLE_EXT; + rte_intr_callback_register(&sh->intr_handle, + mlx5_dev_interrupt_handler, sh); + sh->intr_cnt++; + sh->port[priv->ibv_port - 1].ih_port_id = + (uint32_t)dev->data->port_id; + } +exit: + pthread_mutex_unlock(&sh->intr_mutex); +} + +/** + * Install devx shared asyncronous device events handler. + * This function is implemeted to support event sharing + * between multiple ports of single IB device. + * + * @param dev + * Pointer to Ethernet device. + */ +static void +mlx5_dev_shared_handler_devx_install(struct rte_eth_dev *dev) +{ + struct mlx5_priv *priv = dev->data->dev_private; + struct mlx5_ibv_shared *sh = priv->sh; + + if (rte_eal_process_type() != RTE_PROC_PRIMARY) + return; + pthread_mutex_lock(&sh->intr_mutex); + assert(priv->ibv_port); + assert(priv->ibv_port <= sh->max_port); + assert(dev->data->port_id < RTE_MAX_ETHPORTS); + if (sh->port[priv->ibv_port - 1].devx_ih_port_id < RTE_MAX_ETHPORTS) { + /* The handler is already installed for this port. */ + assert(sh->devx_intr_cnt); + goto exit; + } + if (sh->devx_intr_cnt) { + sh->devx_intr_cnt++; + sh->port[priv->ibv_port - 1].devx_ih_port_id = + (uint32_t)dev->data->port_id; + goto exit; } - sh->intr_handle.fd = sh->ctx->async_fd; - sh->intr_handle.type = RTE_INTR_HANDLE_EXT; - rte_intr_callback_register(&sh->intr_handle, - mlx5_dev_interrupt_handler, sh); if (priv->config.devx) { #ifndef HAVE_IBV_DEVX_ASYNC - goto error_unregister; + goto exit; #else sh->devx_comp = mlx5_glue->devx_create_cmd_comp(sh->ctx); if (sh->devx_comp) { - flags = fcntl(sh->devx_comp->fd, F_GETFL); - ret = fcntl(sh->devx_comp->fd, F_SETFL, + int flags = fcntl(sh->devx_comp->fd, F_GETFL); + int ret = fcntl(sh->devx_comp->fd, F_SETFL, flags | O_NONBLOCK); + if (ret) { DRV_LOG(INFO, "failed to change file descriptor" - " devx async event queue"); - goto error_unregister; + " devx async event queue"); + } else { + sh->intr_handle_devx.fd = sh->devx_comp->fd; + sh->intr_handle_devx.type = RTE_INTR_HANDLE_EXT; + rte_intr_callback_register + (&sh->intr_handle_devx, + mlx5_dev_interrupt_handler_devx, sh); + sh->devx_intr_cnt++; + sh->port[priv->ibv_port - 1].devx_ih_port_id = + (uint32_t)dev->data->port_id; } - sh->intr_handle_devx.fd = sh->devx_comp->fd; - sh->intr_handle_devx.type = RTE_INTR_HANDLE_EXT; - rte_intr_callback_register - (&sh->intr_handle_devx, - mlx5_dev_interrupt_handler_devx, sh); - } else { - DRV_LOG(INFO, "failed to create devx async command " - "completion"); - goto error_unregister; } #endif /* HAVE_IBV_DEVX_ASYNC */ } - sh->intr_cnt++; - goto exit; -error_unregister: - rte_intr_callback_unregister(&sh->intr_handle, - mlx5_dev_interrupt_handler, sh); -error: - /* Indicate there will be no interrupts. */ - dev->data->dev_conf.intr_conf.lsc = 0; - dev->data->dev_conf.intr_conf.rmv = 0; - sh->intr_handle.fd = 0; - sh->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - sh->port[priv->ibv_port - 1].ih_port_id = RTE_MAX_ETHPORTS; exit: pthread_mutex_unlock(&sh->intr_mutex); } @@ -1574,6 +1635,30 @@ mlx5_dev_interrupt_handler_install(struct rte_eth_dev *dev) mlx5_dev_shared_handler_install(dev); } +/** + * Devx uninstall interrupt handler. + * + * @param dev + * Pointer to Ethernet device. + */ +void +mlx5_dev_interrupt_handler_devx_uninstall(struct rte_eth_dev *dev) +{ + mlx5_dev_shared_handler_devx_uninstall(dev); +} + +/** + * Devx install interrupt handler. + * + * @param dev + * Pointer to Ethernet device. + */ +void +mlx5_dev_interrupt_handler_devx_install(struct rte_eth_dev *dev) +{ + mlx5_dev_shared_handler_devx_install(dev); +} + /** * DPDK callback to bring the link DOWN. * -- 2.20.1