From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <shahafs@mellanox.com>
Received: from EUR03-VE1-obe.outbound.protection.outlook.com
 (mail-eopbgr50063.outbound.protection.outlook.com [40.107.5.63])
 by dpdk.org (Postfix) with ESMTP id E76151B2E1;
 Sun, 28 Jan 2018 14:54:19 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
 s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;
 bh=UxjSNYoFMHt5USkSsqF543dtZWaERIX0G2ramymcFvo=;
 b=eExYR1KBwY6aMgO2Wh700yi9wSe67MJ0RS+VfXsTXnYEn69ErvTLF6SbxsJy/wx83cMjPBUeMhtfi85jcGCDSC+VmbJhx7e6LvOHCGNIFOPS4TaUxxrPDDN5NwNDar/4UjIhvGMD8VvoWLyPwTMvdMBJK+wbmr9XiXD7u9RkgxE=
Received: from VI1PR05MB3149.eurprd05.prod.outlook.com (10.170.237.142) by
 VI1PR05MB1918.eurprd05.prod.outlook.com (10.166.44.145) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id
 15.20.444.14; Sun, 28 Jan 2018 13:54:17 +0000
Received: from VI1PR05MB3149.eurprd05.prod.outlook.com
 ([fe80::cd00:54ee:63a8:937c]) by VI1PR05MB3149.eurprd05.prod.outlook.com
 ([fe80::cd00:54ee:63a8:937c%13]) with mapi id 15.20.0444.016; Sun, 28 Jan
 2018 13:54:17 +0000
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>
Thread-Topic: [dpdk-stable] [PATCH v2] net/mlx4: fix port start fail after
 device	removal
Thread-Index: AQHTkQ6wq7904A+lk0eBTOmWKeiVIKOJWdfw
Date: Sun, 28 Jan 2018 13:54:17 +0000
Message-ID: <VI1PR05MB31497BFC7A3085009BBA7584C3E60@VI1PR05MB3149.eurprd05.prod.outlook.com>
References: <1516356548-15057-1-git-send-email-motih@mellanox.com>
 <1516357009-15463-1-git-send-email-motih@mellanox.com>
In-Reply-To: <1516357009-15463-1-git-send-email-motih@mellanox.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=shahafs@mellanox.com; 
x-originating-ip: [31.154.10.107]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; VI1PR05MB1918;
 7:pn+XswHVQaXi+JqrCeiPqQBt1rStxmZR6ywexTk/T65eQtV2eD7oRV8A+NN40oEStxWpnG+Gk0hC3q3IvzoRsvkGtkAqR/AJzRZki+wBMn6fl9E+yAFM5CdRLcWNmHkB7jcYjDZ8KG8J/QJp/ge83REe62xxzUiTqmjl3ADSvaoMOj92OuKj2vvRicUnfilt9ParSgZ1VYtznqtOf6Byo4Y780VSHtkgLcLZSR5JW+Y0FQa2uD1pTkP1Dw08Xz9o
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 27414481-a4b4-4cb5-23f2-08d566569fe7
x-microsoft-antispam: UriScan:; BCL:0; PCL:0;
 RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020);
 SRVR:VI1PR05MB1918; 
x-ms-traffictypediagnostic: VI1PR05MB1918:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <VI1PR05MB1918F9B7477DB5C3A419CB58C3E60@VI1PR05MB1918.eurprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:;
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
 RULEID:(6040501)(2401047)(8121501046)(5005006)(3231101)(2400081)(944501161)(10201501046)(93006095)(93001095)(3002001)(6055026)(6041288)(20161123560045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123564045)(6072148)(201708071742011);
 SRVR:VI1PR05MB1918; BCL:0; PCL:0; RULEID:; SRVR:VI1PR05MB1918; 
x-forefront-prvs: 05669A7924
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(39860400002)(346002)(396003)(376002)(366004)(39380400002)(199004)(189003)(26005)(86362001)(7736002)(33656002)(106356001)(74316002)(3660700001)(3280700002)(102836004)(7696005)(68736007)(76176011)(6246003)(5660300001)(6506007)(2950100002)(59450400001)(14454004)(55016002)(9686003)(6116002)(8936002)(66066001)(3846002)(316002)(81166006)(305945005)(6436002)(2900100001)(4326008)(81156014)(2906002)(54906003)(478600001)(229853002)(25786009)(53936002)(99286004)(97736004)(105586002)(110136005)(5250100002);
 DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB1918;
 H:VI1PR05MB3149.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords;
 A:1; MX:1; LANG:en; 
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
x-microsoft-antispam-message-info: f4xbDF6FVpYzj6d/4xNSCxLdesMWVO4CDJWYbZBsevrt5a8BI4Er0ge9UBvqoK9z/WLXl4U84N+ORKNnSHtRCA==
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 27414481-a4b4-4cb5-23f2-08d566569fe7
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jan 2018 13:54:17.8114 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB1918
Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/mlx4: fix port start
 fail after device	removal
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Sun, 28 Jan 2018 13:54:20 -0000

Hi Moti,


Friday, January 19, 2018 12:17 PM, Moti Haimovsky
> In failsafe device start can be called for ports/devices that had been pl=
ugged
> 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 cance=
led when the port is stopped, causing link removal events not to arrive to =
the failsafe PMD.=20
To fix that, you precede the installation of the interrupt handler to devic=
e configuration, and toggle only the rxq interrupt on start/stop.

Is that correct?=20
More below.=20

>=20
> Fixes: a6e8b01c3c26 ("net/mlx4: compact interrupt functions")
> Cc: stable@dpdk.org
>=20
> 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(-)
>=20
> 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?=20

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

Indentation is wrong.

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

Maybe mlx4_rxq_intr_enable?=20

>  	if (ret) {
>  		ERROR("%p: interrupt handler installation failed",
>  		     (void *)dev);
> @@ -187,7 +193,7 @@ struct mlx4_conf {
>  	dev->rx_pkt_burst =3D 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);
>  }
>=20
> 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,
>=20
>  int mlx4_intr_uninstall(struct priv *priv);  int mlx4_intr_install(struc=
t 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);
>=20
> 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 =3D &priv->intr_handle;
>=20
> +	if (intr_handle =3D=3D NULL || intr_handle->intr_vec =3D=3D NULL)
> +		return;

Is the above code related to this patch?=20

>  	rte_intr_free_epoll_fd(intr_handle);
>  	free(intr_handle->intr_vec);
>  	intr_handle->nb_efd =3D 0;
> @@ -291,7 +293,7 @@
>  	}
>  	rte_eal_alarm_cancel((void (*)(void *))mlx4_link_status_alarm,
> priv);
>  	priv->intr_alarm =3D 0;
> -	mlx4_rx_intr_vec_disable(priv);
> +	mlx4_intr_disable(priv);
>  	rte_errno =3D err;
>  	return 0;
>  }
> @@ -313,8 +315,6 @@
>  	int rc;
>=20
>  	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 =3D priv->ctx->async_fd;
>  		rc =3D 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 =3D
> +		&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 =3D rte_errno; /* Make sure rte_errno remains unchanged. */
> +
> +	mlx4_rx_intr_vec_disable(priv);
> +	rte_errno =3D err;
> +}
> --
> 1.8.3.1