[dpdk-dev] [dpdk-stable] [PATCH v2] net/mlx4: fix port start fail after device removal

Shahaf Shuler shahafs at mellanox.com
Sun Jan 28 14:54:17 CET 2018


Hi Moti,


Friday, January 19, 2018 12:17 PM, Moti Haimovsky
> In failsafe device start can be called for ports/devices that had been plugged
> out.
> This patch fixes mlx4 port start failure when called by the failsafe PMD for
> removed devices.

I think the commit log not fully describes the issue you try to fix.
If I understand correctly, the issue is that the interrupt handler is canceled when the port is stopped, causing link removal events not to arrive to the failsafe PMD. 
To fix that, you precede the installation of the interrupt handler to device configuration, and toggle only the rxq interrupt on start/stop.

Is that correct? 
More below. 

> 
> Fixes: a6e8b01c3c26 ("net/mlx4: compact interrupt functions")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Moti Haimovsky <motih at mellanox.com>
> ---
> V2:
> Fixed commit message.
> ---
>  drivers/net/mlx4/mlx4.c      | 10 ++++++++--
>  drivers/net/mlx4/mlx4.h      |  2 ++
>  drivers/net/mlx4/mlx4_intr.c | 43
> ++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 50 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index
> 61c5bf4..c67b221 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -108,7 +108,13 @@ struct mlx4_conf {
>  		      " flow error type %d, cause %p, message: %s",
>  		      -ret, strerror(-ret), error.type, error.cause,
>  		      error.message ? error.message : "(unspecified)");
> +		goto exit;

This fix is related to the patch? 

>  	}
> +	ret = mlx4_intr_install(priv);
> +	if (ret)
> +		ERROR("%p: interrupt handler installation failed",
> +		     (void *)dev);

Indentation is wrong.

> +exit:
>  	return ret;
>  }
> 
> @@ -141,7 +147,7 @@ struct mlx4_conf {
>  		      (void *)dev, strerror(-ret));
>  		goto err;
>  	}
> -	ret = mlx4_intr_install(priv);
> +	ret = mlx4_intr_enable(priv);

Maybe mlx4_rxq_intr_enable? 

>  	if (ret) {
>  		ERROR("%p: interrupt handler installation failed",
>  		     (void *)dev);
> @@ -187,7 +193,7 @@ struct mlx4_conf {
>  	dev->rx_pkt_burst = mlx4_rx_burst_removed;
>  	rte_wmb();
>  	mlx4_flow_sync(priv, NULL);
> -	mlx4_intr_uninstall(priv);
> +	mlx4_intr_disable(priv);

Maybe mlx4_rxq_intr_disable?

>  	mlx4_rss_deinit(priv);
>  }
> 
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index
> 99dc335..ab4f396 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -176,6 +176,8 @@ int mlx4_flow_ctrl_set(struct rte_eth_dev *dev,
> 
>  int mlx4_intr_uninstall(struct priv *priv);  int mlx4_intr_install(struct priv
> *priv);
> +int mlx4_intr_enable(struct priv *priv); void mlx4_intr_disable(struct
> +priv *priv);
>  int mlx4_rx_intr_disable(struct rte_eth_dev *dev, uint16_t idx);  int
> mlx4_rx_intr_enable(struct rte_eth_dev *dev, uint16_t idx);
> 
> diff --git a/drivers/net/mlx4/mlx4_intr.c b/drivers/net/mlx4/mlx4_intr.c
> index 9becee4..eeab625 100644
> --- a/drivers/net/mlx4/mlx4_intr.c
> +++ b/drivers/net/mlx4/mlx4_intr.c
> @@ -73,6 +73,8 @@
>  {
>  	struct rte_intr_handle *intr_handle = &priv->intr_handle;
> 
> +	if (intr_handle == NULL || intr_handle->intr_vec == NULL)
> +		return;

Is the above code related to this patch? 

>  	rte_intr_free_epoll_fd(intr_handle);
>  	free(intr_handle->intr_vec);
>  	intr_handle->nb_efd = 0;
> @@ -291,7 +293,7 @@
>  	}
>  	rte_eal_alarm_cancel((void (*)(void *))mlx4_link_status_alarm,
> priv);
>  	priv->intr_alarm = 0;
> -	mlx4_rx_intr_vec_disable(priv);
> +	mlx4_intr_disable(priv);
>  	rte_errno = err;
>  	return 0;
>  }
> @@ -313,8 +315,6 @@
>  	int rc;
> 
>  	mlx4_intr_uninstall(priv);
> -	if (intr_conf->rxq && mlx4_rx_intr_vec_enable(priv) < 0)
> -		goto error;
>  	if (intr_conf->lsc | intr_conf->rmv) {
>  		priv->intr_handle.fd = priv->ctx->async_fd;
>  		rc = rte_intr_callback_register(&priv->intr_handle,
> @@ -395,3 +395,40 @@
>  	}
>  	return -ret;
>  }
> +
> +/**
> + * Enable datapath interrupts.
> + *
> + * @param priv
> + *   Pointer to private structure.
> + *
> + * @return
> + *   0 on success, negative errno value otherwise and rte_errno is set.
> + */
> +int
> +mlx4_intr_enable(struct priv *priv)
> +{
> +	const struct rte_intr_conf *const intr_conf =
> +		&priv->dev->data->dev_conf.intr_conf;
> +
> +	if (intr_conf->rxq && mlx4_rx_intr_vec_enable(priv) < 0)
> +		goto error;
> +	return 0;
> +error:
> +	return -rte_errno;
> +}
> +
> +/**
> + * Disable datapath interrupts, keeping other interrupts intact.
> + *
> + * @param priv
> + *   Pointer to private structure.
> + */
> +void
> +mlx4_intr_disable(struct priv *priv)
> +{
> +	int err = rte_errno; /* Make sure rte_errno remains unchanged. */
> +
> +	mlx4_rx_intr_vec_disable(priv);
> +	rte_errno = err;
> +}
> --
> 1.8.3.1



More information about the dev mailing list