Thursday, March 21, 2019 10:11 AM, Viacheslav Ovsiienko:
> Subject: [PATCH 12/14] net/mlx5: update install/uninstall int handler routines
>
> We are implementing the support for multport Infiniband device withj
> representors attached to these multiple ports. Asynchronous device event
> notifications (link status change, removal event, etc.) should be shared
> between ports. We are going to implement shared event handler and this
> patch introduces appropriate device structure changes and updated event
> handler install and uninstall routines.
>
> Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> ---
> drivers/net/mlx5/mlx5.c | 14 ++++-
> drivers/net/mlx5/mlx5.h | 3 +-
> drivers/net/mlx5/mlx5_ethdev.c | 118
> ++++++++++++++++++++++++++++++++---------
> 3 files changed, 107 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> 312c42b..44b7a87 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -165,6 +165,7 @@ struct mlx5_dev_spawn_data { {
> struct mlx5_ibv_shared *sh;
> int err = 0;
> + uint32_t i;
>
> assert(spawn);
> /* Search for IB context by device name. */ @@ -212,6 +213,9 @@
> struct mlx5_dev_spawn_data {
> sizeof(sh->ibdev_name));
> strncpy(sh->ibdev_path, sh->ctx->device->ibdev_path,
> sizeof(sh->ibdev_path));
> + pthread_mutex_init(&sh->intr_mutex, NULL);
> + for (i = 0; i < sh->max_port; i++)
> + sh->port[i].port_id = RTE_MAX_ETHPORTS;
Why you need struct here? You port array is not just of uint32_t type?
> if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> /*
> * For secondary process we just open the IB device @@ -
> 276,6 +280,15 @@ struct mlx5_dev_spawn_data {
> assert(!sh->pd);
> }
> LIST_REMOVE(sh, next);
> + /*
> + * Ensure there is no async event handler installed.
> + * Only primary process handles async device events.
> + **/
> + assert(!sh->intr_cnt);
> + if (sh->intr_cnt)
> + rte_intr_callback_unregister
> + (&sh->intr_handle, mlx5_dev_interrupt_handler,
> sh);
> + pthread_mutex_destroy(&sh->intr_mutex);
> if (sh->pd)
> claim_zero(mlx5_glue->dealloc_pd(sh->pd));
> if (sh->ctx)
> @@ -283,7 +296,6 @@ struct mlx5_dev_spawn_data {
> rte_free(sh);
> }
>
> -
> /**
> * Prepare shared data between primary and secondary process.
> */
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> d816d24..f23298e 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -216,6 +216,8 @@ struct mlx5_ibv_shared {
> char ibdev_name[IBV_SYSFS_NAME_MAX]; /* IB device name. */
> char ibdev_path[IBV_SYSFS_PATH_MAX]; /* IB device path for
> secondary */
> struct ibv_device_attr_ex device_attr; /* Device properties. */
> + 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.
> */
> struct mlx5_ibv_shared_port port[]; /* per device port data array. */
> }; @@ -245,7 +247,6 @@ struct mlx5_priv {
> struct mlx5_txq_data *(*txqs)[]; /* TX queues. */
> struct rte_mempool *mprq_mp; /* Mempool for Multi-Packet RQ.
> */
> struct rte_eth_rss_conf rss_conf; /* RSS configuration. */
> - struct rte_intr_handle intr_handle; /* Interrupt handler. */
> unsigned int (*reta_idx)[]; /* RETA index table. */
> unsigned int reta_idx_n; /* RETA index size. */
> struct mlx5_drop drop_queue; /* Flow drop queues. */ diff --git
> a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c index
> 1b2173b..8358cd2 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1109,6 +1109,96 @@ int mlx5_fw_version_get(struct rte_eth_dev
> *dev, char *fw_ver, size_t fw_size) }
>
> /**
> + * Uninstall 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_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].port_id >= RTE_MAX_ETHPORTS)
> + goto exit;
> + assert(sh->port[priv->ibv_port - 1].port_id ==
> + (uint32_t)dev->data->port_id);
> + assert(sh->intr_cnt);
> + sh->port[priv->ibv_port - 1].port_id = RTE_MAX_ETHPORTS;
> + if (!sh->intr_cnt || --sh->intr_cnt)
> + goto exit;
> + rte_intr_callback_unregister(&sh->intr_handle,
> + 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);
> +}
> +
> +/**
> + * Install 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_install(struct rte_eth_dev *dev) {
> + struct mlx5_priv *priv = dev->data->dev_private;
> + struct mlx5_ibv_shared *sh = priv->sh;
> + int ret;
> + int flags;
> +
> + 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].port_id < RTE_MAX_ETHPORTS) {
I don't understand why need an array to understand handler is already exists.
Why not the refcnt?
> + /* The handler is already installed for this port. */
> + assert(sh->intr_cnt++);
Asserts are compiled only in debug mode. You should not put any logic (++) into
them.
> + goto exit;
> + }
> + sh->port[priv->ibv_port - 1].port_id = (uint32_t)dev->data->port_id;
> + if (sh->intr_cnt) {
> + sh->intr_cnt++;
> + goto exit;
> + }
> + /* No shared handler installed. */
> + assert(sh->ctx->async_fd > 0);
> + 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");
> + /* Indicate there will be no interrupts. */
> + dev->data->dev_conf.intr_conf.lsc = 0;
> + dev->data->dev_conf.intr_conf.rmv = 0;
> + sh->port[priv->ibv_port - 1].port_id = RTE_MAX_ETHPORTS;
> + 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);
> + sh->intr_cnt++;
> +exit:
> + pthread_mutex_unlock(&sh->intr_mutex);
> +}
> +
> +/**
> * Uninstall interrupt handler.
> *
> * @param dev
> @@ -1119,15 +1209,10 @@ int mlx5_fw_version_get(struct rte_eth_dev
> *dev, char *fw_ver, size_t fw_size) {
> struct mlx5_priv *priv = dev->data->dev_private;
>
> - if (dev->data->dev_conf.intr_conf.lsc ||
> - dev->data->dev_conf.intr_conf.rmv)
> - rte_intr_callback_unregister(&priv->intr_handle,
> - mlx5_dev_interrupt_handler,
> dev);
> + mlx5_dev_shared_handler_uninstall(dev);
> if (priv->primary_socket)
> rte_intr_callback_unregister(&priv->intr_handle_socket,
> mlx5_dev_handler_socket, dev);
> - priv->intr_handle.fd = 0;
> - priv->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
> priv->intr_handle_socket.fd = 0;
> priv->intr_handle_socket.type = RTE_INTR_HANDLE_UNKNOWN; }
> @@ -1142,28 +1227,9 @@ int mlx5_fw_version_get(struct rte_eth_dev
> *dev, char *fw_ver, size_t fw_size)
> mlx5_dev_interrupt_handler_install(struct rte_eth_dev *dev) {
> struct mlx5_priv *priv = dev->data->dev_private;
> - struct ibv_context *ctx = priv->sh->ctx;
> int ret;
> - int flags;
>
> - assert(ctx->async_fd > 0);
> - flags = fcntl(ctx->async_fd, F_GETFL);
> - ret = fcntl(ctx->async_fd, F_SETFL, flags | O_NONBLOCK);
> - if (ret) {
> - DRV_LOG(INFO,
> - "port %u failed to change file descriptor async event"
> - " queue",
> - dev->data->port_id);
> - dev->data->dev_conf.intr_conf.lsc = 0;
> - dev->data->dev_conf.intr_conf.rmv = 0;
> - }
> - if (dev->data->dev_conf.intr_conf.lsc ||
> - dev->data->dev_conf.intr_conf.rmv) {
> - priv->intr_handle.fd = ctx->async_fd;
> - priv->intr_handle.type = RTE_INTR_HANDLE_EXT;
> - rte_intr_callback_register(&priv->intr_handle,
> - mlx5_dev_interrupt_handler, dev);
> - }
> + mlx5_dev_shared_handler_install(dev);
> ret = mlx5_socket_init(dev);
> if (ret)
> DRV_LOG(ERR, "port %u cannot initialise socket: %s",
> --
> 1.8.3.1
