From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <matan@mellanox.com>
Received: from EUR01-DB5-obe.outbound.protection.outlook.com
(mail-db5eur01on0061.outbound.protection.outlook.com [104.47.2.61])
by dpdk.org (Postfix) with ESMTP id 0568B1B22D
for <dev@dpdk.org>; Wed, 10 Jan 2018 13:43:34 +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=fKzL/54hSuAW5PuLBysO4GKEQIVdMvAeqPICU/lnkRQ=;
b=Uk6H7yugCi9P9R54p9rx4WG7RgYWH2CeXyIs9V4zHaDDex9pnvPBeKWb4bA6kRdiEaoJOVe+IyD0ZQvPnkOuhuFCOi/X8qHULwBO/E1MbrzJKW5slR/2+Xrm1wxsvBFT48BdyxDD4cFzOJqLnkmdBWNR6xI66maLoMM0gY+9boI=
Received: from AM6PR0502MB3797.eurprd05.prod.outlook.com (52.133.21.26) by
AM6PR0502MB3797.eurprd05.prod.outlook.com (52.133.21.26) with Microsoft SMTP
Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id
15.20.386.5; Wed, 10 Jan 2018 12:43:33 +0000
Received: from AM6PR0502MB3797.eurprd05.prod.outlook.com
([fe80::b4b4:7de8:cf70:aa3a]) by AM6PR0502MB3797.eurprd05.prod.outlook.com
([fe80::b4b4:7de8:cf70:aa3a%13]) with mapi id 15.20.0386.006; Wed, 10 Jan
2018 12:43:33 +0000
From: Matan Azrad <matan@mellanox.com>
To: Matan Azrad <matan@mellanox.com>, Gaetan Rivet <gaetan.rivet@6wind.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>
Thread-Topic: [dpdk-dev] [PATCH v4 6/6] net/failsafe: fix removed device
handling
Thread-Index: AQHTig8kD8Wr2tdbKUmod/DnFO4YcKNtC1Pg
Date: Wed, 10 Jan 2018 12:43:33 +0000
Message-ID: <AM6PR0502MB37976615C92E03DC0A143780D2110@AM6PR0502MB3797.eurprd05.prod.outlook.com>
References: <1513703415-29145-1-git-send-email-matan@mellanox.com>
<1515587465-9304-1-git-send-email-matan@mellanox.com>
<1515587465-9304-7-git-send-email-matan@mellanox.com>
In-Reply-To: <1515587465-9304-7-git-send-email-matan@mellanox.com>
Accept-Language: en-US, he-IL
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: spf=none (sender IP is )
smtp.mailfrom=matan@mellanox.com;
x-originating-ip: [193.47.165.251]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; AM6PR0502MB3797;
7:cIZMyr4ji4zw8Y3NucbQ58UYPrvbYcQosNhnMeEXNnrOE1Agp0So2EAmB5k1PIW7PQJnJkqbHxBZ5H7inC3J2Rp0UV1puXjyYamobiB0OHATvxvMr7IfQ+BN2c1DlRhbdtlZNkP1BxcCSOpoSnoEPjaY2P7/jbYcIniXQyvKnTxCfdJHO3aj4ihHeQVRSCmD4HHgQDrV3oPYSeLll0JUbDm/nly5dFflSwMV3/o7Fx7cP9ngXuMUlcsjnbD1FEHt
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: f47ef81e-3355-4c98-d1aa-08d55827c270
x-microsoft-antispam: UriScan:; BCL:0; PCL:0;
RULEID:(48565401081)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(5600026)(4604075)(3008032)(2017052603307)(7153060)(7193020);
SRVR:AM6PR0502MB3797;
x-ms-traffictypediagnostic: AM6PR0502MB3797:
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <AM6PR0502MB379707D7CE63B742F4291732D2110@AM6PR0502MB3797.eurprd05.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(278428928389397);
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0;
RULEID:(6040470)(2401047)(5005006)(8121501046)(10201501046)(3231023)(944501075)(93006095)(93001095)(3002001)(6055026)(6041268)(20161123562045)(20161123560045)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011);
SRVR:AM6PR0502MB3797; BCL:0; PCL:0; RULEID:(100000803101)(100110400095);
SRVR:AM6PR0502MB3797;
x-forefront-prvs: 0548586081
x-forefront-antispam-report: SFV:NSPM;
SFS:(10009020)(346002)(39860400002)(39380400002)(396003)(376002)(366004)(76104003)(13464003)(199004)(189003)(33656002)(25786009)(6246003)(55016002)(9686003)(5660300001)(229853002)(2950100002)(97736004)(3280700002)(99286004)(3660700001)(8936002)(86362001)(5250100002)(81166006)(68736007)(4326008)(2906002)(6436002)(53936002)(2900100001)(305945005)(7736002)(74316002)(66066001)(478600001)(8676002)(110136005)(54906003)(316002)(102836004)(7696005)(59450400001)(76176011)(81156014)(106356001)(3846002)(105586002)(6116002)(53546011)(14454004)(6506007);
DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR0502MB3797;
H:AM6PR0502MB3797.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: l5FsL7t5i0iPdsHFNeijVOe83+HCAMIXEfyXf86ZEloVXFKm6QQVOlNsIh254CVDVJUAw/U08gQ7rQDSK8LfkA==
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: f47ef81e-3355-4c98-d1aa-08d55827c270
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jan 2018 12:43:33.0998 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR0502MB3797
Subject: Re: [dpdk-dev] [PATCH v4 6/6] net/failsafe: fix removed device
handling
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: Wed, 10 Jan 2018 12:43:35 -0000
Hi Gaetan
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Matan Azrad
> Sent: Wednesday, January 10, 2018 2:31 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Gaetan Rivet
> <gaetan.rivet@6wind.com>
> Cc: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 6/6] net/failsafe: fix removed device handl=
ing
>=20
> There is time between the physical removal of the device until sub-device
> PMDs get a RMV interrupt. At this time DPDK PMDs and applications still
> don't know about the removal and may call sub-device control operation
> which should return an error.
>=20
> In previous code this error is reported to the application contrary to fa=
il-safe
> principle that the app should not be aware of device removal.
>=20
> Add an removal check in each relevant control command error flow and
> prevent an error report to application when the sub-device is removed.
>=20
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
> drivers/net/failsafe/failsafe_flow.c | 18 ++++++++++-------
> drivers/net/failsafe/failsafe_ops.c | 35 ++++++++++++++++++++++-----=
---
> ---
> drivers/net/failsafe/failsafe_private.h | 11 +++++++++++
> 3 files changed, 46 insertions(+), 18 deletions(-)
>=20
> diff --git a/drivers/net/failsafe/failsafe_flow.c
> b/drivers/net/failsafe/failsafe_flow.c
> index 153ceee..c072d1e 100644
> --- a/drivers/net/failsafe/failsafe_flow.c
> +++ b/drivers/net/failsafe/failsafe_flow.c
> @@ -87,7 +87,7 @@
> DEBUG("Calling rte_flow_validate on sub_device %d", i);
> ret =3D rte_flow_validate(PORT_ID(sdev),
> attr, patterns, actions, error);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
This assignment in "if" statement causes to checkpatch error, I sent it as =
is because you asked it like this.
If you think I need to change it, I see 2 options:
1.
ret =3D fs_err(sdev, ret);
if (ret ) {...}
2.
if (fs_err(sdev, &ret)) {..}
what do you think?
> ERROR("Operation rte_flow_validate failed for
> sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -111,7 +111,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> flow->flows[i] =3D rte_flow_create(PORT_ID(sdev),
> attr, patterns, actions, error);
> - if (flow->flows[i] =3D=3D NULL) {
> + if (flow->flows[i] =3D=3D NULL && fs_err(sdev, -rte_errno)) {
> ERROR("Failed to create flow on sub_device %d",
> i);
> goto err;
> @@ -150,7 +150,7 @@
> continue;
> local_ret =3D rte_flow_destroy(PORT_ID(sdev),
> flow->flows[i], error);
> - if (local_ret) {
> + if ((local_ret =3D fs_err(sdev, local_ret))) {
> ERROR("Failed to destroy flow on sub_device %d:
> %d",
> i, local_ret);
> if (ret =3D=3D 0)
> @@ -175,7 +175,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_flow_flush on sub_device %d", i);
> ret =3D rte_flow_flush(PORT_ID(sdev), error);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_flow_flush failed for
> sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -199,8 +199,12 @@
>=20
> sdev =3D TX_SUBDEV(dev);
> if (sdev !=3D NULL) {
> - return rte_flow_query(PORT_ID(sdev),
> - flow->flows[SUB_ID(sdev)], type, arg, error);
> + int ret =3D rte_flow_query(PORT_ID(sdev),
> + flow->flows[SUB_ID(sdev)],
> + type, arg, error);
> +
> + if ((ret =3D fs_err(sdev, ret)))
> + return ret;
> }
> WARN("No active sub_device to query about its flow");
> return -1;
> @@ -223,7 +227,7 @@
> WARN("flow isolation mode of sub_device %d in
> incoherent state.",
> i);
> ret =3D rte_flow_isolate(PORT_ID(sdev), set, error);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_flow_isolate failed for
> sub_device %d"
> " with error %d", i, ret);
> return ret;
> diff --git a/drivers/net/failsafe/failsafe_ops.c
> b/drivers/net/failsafe/failsafe_ops.c
> index e16a590..f5390db 100644
> --- a/drivers/net/failsafe/failsafe_ops.c
> +++ b/drivers/net/failsafe/failsafe_ops.c
> @@ -121,6 +121,8 @@
> dev->data->nb_tx_queues,
> &dev->data->dev_conf);
> if (ret) {
> + if (!fs_err(sdev, ret))
> + continue;
> ERROR("Could not configure sub_device %d", i);
> return ret;
> }
> @@ -163,8 +165,11 @@
> continue;
> DEBUG("Starting sub_device %d", i);
> ret =3D rte_eth_dev_start(PORT_ID(sdev));
> - if (ret)
> + if (ret) {
> + if (!fs_err(sdev, ret))
> + continue;
> return ret;
> + }
> sdev->state =3D DEV_STARTED;
> }
> if (PRIV(dev)->state < DEV_STARTED)
> @@ -196,7 +201,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_set_link_up on sub_device
> %d", i);
> ret =3D rte_eth_dev_set_link_up(PORT_ID(sdev));
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_set_link_up failed
> for sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -215,7 +220,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_set_link_down on sub_device
> %d", i);
> ret =3D rte_eth_dev_set_link_down(PORT_ID(sdev));
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_set_link_down
> failed for sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -300,7 +305,7 @@
> rx_queue_id,
> nb_rx_desc, socket_id,
> rx_conf, mb_pool);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("RX queue setup failed for sub_device %d",
> i);
> goto free_rxq;
> }
> @@ -366,7 +371,7 @@
> tx_queue_id,
> nb_tx_desc, socket_id,
> tx_conf);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("TX queue setup failed for sub_device %d", i);
> goto free_txq;
> }
> @@ -445,7 +450,8 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling link_update on sub_device %d", i);
> ret =3D (SUBOPS(sdev, link_update))(ETH(sdev),
> wait_to_complete);
> - if (ret && ret !=3D -1) {
> + if (ret && ret !=3D -1 && sdev->remove =3D=3D 0 &&
> + rte_eth_dev_is_removed(PORT_ID(sdev)) =3D=3D 0) {
> ERROR("Link update failed for sub_device %d with
> error %d",
> i, ret);
> return ret;
> @@ -469,6 +475,7 @@
> fs_stats_get(struct rte_eth_dev *dev,
> struct rte_eth_stats *stats)
> {
> + struct rte_eth_stats backup;
> struct sub_device *sdev;
> uint8_t i;
> int ret;
> @@ -478,14 +485,20 @@
> struct rte_eth_stats *snapshot =3D &sdev-
> >stats_snapshot.stats;
> uint64_t *timestamp =3D &sdev->stats_snapshot.timestamp;
>=20
> + rte_memcpy(&backup, snapshot, sizeof(backup));
> ret =3D rte_eth_stats_get(PORT_ID(sdev), snapshot);
> if (ret) {
> + if (!fs_err(sdev, ret)) {
> + rte_memcpy(snapshot, &backup,
> sizeof(backup));
> + goto inc;
> + }
> ERROR("Operation rte_eth_stats_get failed for
> sub_device %d with error %d",
> i, ret);
> *timestamp =3D 0;
> return ret;
> }
> *timestamp =3D rte_rdtsc();
> +inc:
> failsafe_stats_increment(stats, snapshot);
> }
> return 0;
> @@ -598,7 +611,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_set_mtu on sub_device %d", i);
> ret =3D rte_eth_dev_set_mtu(PORT_ID(sdev), mtu);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_set_mtu failed for
> sub_device %d with error %d",
> i, ret);
> return ret;
> @@ -617,7 +630,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_vlan_filter on sub_device %d",
> i);
> ret =3D rte_eth_dev_vlan_filter(PORT_ID(sdev), vlan_id, on);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_vlan_filter failed for
> sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -651,7 +664,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_flow_ctrl_set on sub_device
> %d", i);
> ret =3D rte_eth_dev_flow_ctrl_set(PORT_ID(sdev), fc_conf);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_flow_ctrl_set failed
> for sub_device %d"
> " with error %d", i, ret);
> return ret;
> @@ -688,7 +701,7 @@
> RTE_ASSERT(index < FAILSAFE_MAX_ETHADDR);
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> ret =3D rte_eth_dev_mac_addr_add(PORT_ID(sdev),
> mac_addr, vmdq);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_mac_addr_add
> failed for sub_device %"
> PRIu8 " with error %d", i, ret);
> return ret;
> @@ -730,7 +743,7 @@
> FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
> DEBUG("Calling rte_eth_dev_filter_ctrl on sub_device %d",
> i);
> ret =3D rte_eth_dev_filter_ctrl(PORT_ID(sdev), type, op, arg);
> - if (ret) {
> + if ((ret =3D fs_err(sdev, ret))) {
> ERROR("Operation rte_eth_dev_filter_ctrl failed for
> sub_device %d"
> " with error %d", i, ret);
> return ret;
> diff --git a/drivers/net/failsafe/failsafe_private.h
> b/drivers/net/failsafe/failsafe_private.h
> index d81cc3c..a306970 100644
> --- a/drivers/net/failsafe/failsafe_private.h
> +++ b/drivers/net/failsafe/failsafe_private.h
> @@ -375,4 +375,15 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id=
,
> rte_wmb();
> }
>=20
> +/*
> + * Adjust error value and rte_errno to the fail-safe actual error value.
> + */
> +static inline int
> +fs_err(struct sub_device *sdev, int err) {
> + /* A device removal shouldn't be reported as an error. */
> + if (sdev->remove =3D=3D 1 || err =3D=3D -EIO)
> + return rte_errno =3D 0;
> + return err;
> +}
> #endif /* _RTE_ETH_FAILSAFE_PRIVATE_H_ */
> --
> 1.8.3.1