From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 To: Matan Azrad , Gaetan Rivet CC: "dev@dpdk.org" , Thomas Monjalon 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: 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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 ; Gaetan Rivet > > 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 > --- > 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