patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Mordechay Haimovsky <motih@mellanox.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	Mordechay Haimovsky <motih@mellanox.com>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH v2] net/mlx4: fix port start fail after device	removal
Date: Sun, 28 Jan 2018 13:54:17 +0000	[thread overview]
Message-ID: <VI1PR05MB31497BFC7A3085009BBA7584C3E60@VI1PR05MB3149.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <1516357009-15463-1-git-send-email-motih@mellanox.com>

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@dpdk.org
> 
> Signed-off-by: Moti Haimovsky <motih@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

  reply	other threads:[~2018-01-28 13:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1516356548-15057-1-git-send-email-motih@mellanox.com>
2018-01-19 10:16 ` Moti Haimovsky
2018-01-28 13:54   ` Shahaf Shuler [this message]
2018-01-29  8:34   ` [dpdk-stable] [PATCH v3] net/mlx4: fix dev rmv not detected after port stop Moti Haimovsky
2018-01-29 10:54     ` Shahaf Shuler
2018-01-30  9:21       ` Thomas Monjalon
2018-01-30  9:39       ` Adrien Mazarguil
2018-01-30 20:37         ` Shahaf Shuler
2018-01-31  9:15           ` Adrien Mazarguil
2018-01-31  9:54             ` [dpdk-stable] [dpdk-dev] " Gaëtan Rivet
2018-01-31 10:08             ` Matan Azrad
2018-01-31 10:43               ` Adrien Mazarguil
2018-01-31 13:44                 ` Matan Azrad
2018-01-31 14:31                   ` Adrien Mazarguil
2018-01-31 17:07                     ` Matan Azrad
2018-02-02 19:53                       ` Adrien Mazarguil
2018-02-03 19:42                         ` Matan Azrad

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR05MB31497BFC7A3085009BBA7584C3E60@VI1PR05MB3149.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=motih@mellanox.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).