From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01on0060.outbound.protection.outlook.com [104.47.0.60]) by dpdk.org (Postfix) with ESMTP id 46F534CB9 for ; Fri, 31 Aug 2018 23:49:00 +0200 (CEST) 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:X-MS-Exchange-SenderADCheck; bh=ehfq5KkTlPK7IVrKpzYYrrYm8IT9w4VE4nverKSjsVY=; b=X2wBvCnUpj7PbuhHz2ALYUk+t4x7EDX8GCwUkhGFLn3SI6INWg06JNB6fTybvmGzDosIhDlX6Fi+yKdk3/j7+m1lsRPfjLjvY+T2XsGi02hOyqmFYMta6OiryROXESFTQWLPGdOvmGa/6wQIeMA5fwn0+Ld3+u7tcNiPmYbKU8U= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB3995.eurprd05.prod.outlook.com (52.134.72.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1101.16; Fri, 31 Aug 2018 21:48:58 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::d45:8e84:6d63:c57c]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::d45:8e84:6d63:c57c%2]) with mapi id 15.20.1080.015; Fri, 31 Aug 2018 21:48:58 +0000 From: Yongseok Koh To: Jack Min CC: Shahaf Shuler , "dev@dpdk.org" , "Xueming(Steven) Li" Thread-Topic: [dpdk-dev] [PATCH] net/mlx5: handle expected errno properly Thread-Index: AQHUOqwSA/scc8YuM0yHRpWqYq6CXKTNYBcAgAEWhQCABr3qgIAA7XGAgARRgwA= Date: Fri, 31 Aug 2018 21:48:58 +0000 Message-ID: <66AD7D88-8518-4AD3-AABB-B7B8FF900354@mellanox.com> References: <20180823063851.32559-1-jackmin@mellanox.com> <20180823210808.GA31847@yongseok-MBP.local> <20180824064500.uopotkliiaygvvny@MTBC-JACKMIN.mtl.com> <20180828204217.GA45628@yongseok-MBP.local> <20180829035207.vlboi3og4o27rqvp@MTBC-JACKMIN.mtl.com> In-Reply-To: <20180829035207.vlboi3og4o27rqvp@MTBC-JACKMIN.mtl.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=yskoh@mellanox.com; x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB3995; 6:O+6e5NDZydLva5h+CKv//1iKEAcqrfdvMbndf3YoXr636Pto3dMsmnG0BVV/mttBy2gP14oX3bY736g/SDOQM1G0LWfd4q7hHXs6JXuK2WoPnFhALLx2MP59X/3cdmpX19axYYLVhigEOzjjjcQzm+LJB1rwIiN29VhJjhohn3z0B0YXr5EwW2k3qYlPyOVvXO43nyIgQjTUJVvZ9gx5ZXudi71+LVG0YD0iDb7bBJMJ+Ns63fRTuGrpL70RWtr6MOuPIKWvG9BCq/xnmu646iLUyO4GUV8XC7BEpS+vFH9kv3be1qzlovPO81IcPy20S3Dyyl89nI1A0qzTMWYjPlWJwTSlVVXrYyzViYPmvkShmsZlXB6XVTRnPWWJWolaU3eWXEgae/oWDoKZR7HqC5EJByLvarz4oEsJ06NvZjpwN1WYtTYqPqV4IPWOgWIRHS5ZkJC4lamrE85UHNMsXQ==; 5:9iXFAhXTVvzkIGa4zxJVOoLyNjRtqUN7664uXXWrJGjdIE6uUR87iLyNXOQvvumdSe1qIxKALzZOfxOb2rn/6ElAT6CaBoyckvEhJVjdOVxK/5OTSeDFQ9ObgyfXJUgmhD7lEtzKKR6UvUSyORsGzXD7YI1BQKcfIZtZsntnT1o=; 7:iZ4NFer9d30iBClPXEgH6QUkzaRCqXSgM1peGkiByuraqiJvZ1Fy+AAEBCHCo3pzucRBqbmWo0OthO8QJSTGl/yZQrzCekJArv9zvbvmBaAQ7gTA9nRi6aETZYhUTljeRtbSyUptgpev/s9lvn/sddlgangLdTnaAGBwQXxfxOvXXRcd+ffjrHXDCySIxQptegDR46QFkp8ylNXL3qZrvPFrbT40+Bkqd7UThxVWFlE0mFr6OjiFOelkscPFyhmR x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 1cf85f92-4f81-44c5-181d-08d60f8b8e5c x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989137)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:DB3PR0502MB3995; x-ms-traffictypediagnostic: DB3PR0502MB3995: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(823301075)(10201501046)(93006095)(93001095)(3231311)(944501410)(52105095)(3002001)(6055026)(149027)(150027)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123560045)(20161123564045)(20161123562045)(201708071742011)(7699016); SRVR:DB3PR0502MB3995; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB3995; x-forefront-prvs: 07817FCC2D x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(396003)(136003)(376002)(39860400002)(366004)(346002)(51444003)(199004)(189003)(2616005)(26005)(486006)(86362001)(93886005)(76176011)(54906003)(107886003)(6246003)(4326008)(37006003)(6862004)(2906002)(8676002)(6116002)(5660300001)(102836004)(66066001)(53546011)(3846002)(446003)(11346002)(2900100001)(476003)(6506007)(14444005)(97736004)(256004)(25786009)(83716003)(82746002)(36756003)(33656002)(6512007)(81156014)(81166006)(53936002)(7736002)(305945005)(5250100002)(316002)(229853002)(6436002)(105586002)(106356001)(8936002)(478600001)(99286004)(14454004)(68736007)(6486002)(6636002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3995; H:DB3PR0502MB3980.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: Ih32cPi//Aar3BSj9pL7lavEQuFWnaoMukPnNB98uszqjKeABuI2fErX7WdNiKXoeppo/cqOrqqD2PRLOc/f8N84IvUs+doAnariui06JpXfTgYQU8BO2L7fQJ4YXRYFSGyStQIGaUwcW12yVI8jOH+eTZSxR5oecDmYAf/EQrhAvTjY1nMlBXG23vE6ZATOkp1Nm6wD7pm0MrTZSGeunRW6f/Kk3oUsCJhPkVYVTi0kasFz6QG8VSn6j5pMEFpjIyitfSMiIBq+gxLmkGonb6he44ee0/NW6FccHjvI01sRLJquVZJioGCqvI6ZIoQ4PD6hNT4qRzQiYbsNnP96JIgXAHJp8lNPUDl3beeNIbE= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1cf85f92-4f81-44c5-181d-08d60f8b8e5c X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Aug 2018 21:48:58.1924 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB3995 Subject: Re: [dpdk-dev] [PATCH] net/mlx5: handle expected errno properly 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: Fri, 31 Aug 2018 21:49:00 -0000 > On Aug 28, 2018, at 8:52 PM, Jack Min wrote: >=20 > On Tue, Aug 28, 2018 at 01:42:18PM -0700, Yongseok Koh wrote: >> On Fri, Aug 24, 2018 at 02:45:00PM +0800, Jack MIN wrote: >>> On Thu, Aug 23, 2018 at 02:08:09PM -0700, Yongseok Koh wrote: >>>> On Thu, Aug 23, 2018 at 02:38:51PM +0800, Xiaoyu Min wrote: >>>>> rte_errno is a per thread variable and is widely used as an >>>>> error indicator, which means a function could affect >>>>> other functions' results by setting rte_errno carelessly >>>>>=20 >>>>> During rxq setup, an EINVAL rte_errno is expected since >>>>> the queues are not created yet >>>>> So rte_errno is cleared when it is EINVAL as expected >>>>>=20 >>>>> Signed-off-by: Xiaoyu Min >>>>> --- >>>>> drivers/net/mlx5/mlx5_rxq.c | 20 +++++++++++++++----- >>>>> 1 file changed, 15 insertions(+), 5 deletions(-) >>>>>=20 >>>>> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.= c >>>>> index 1f7bfd4..e7056e8 100644 >>>>> --- a/drivers/net/mlx5/mlx5_rxq.c >>>>> +++ b/drivers/net/mlx5/mlx5_rxq.c >>>>> @@ -443,6 +443,7 @@ >>>>> struct mlx5_rxq_data *rxq =3D (*priv->rxqs)[idx]; >>>>> struct mlx5_rxq_ctrl *rxq_ctrl =3D >>>>> container_of(rxq, struct mlx5_rxq_ctrl, rxq); >>>>> + int ret =3D 0; >>>>>=20 >>>>> if (!rte_is_power_of_2(desc)) { >>>>> desc =3D 1 << log2above(desc); >>>>> @@ -459,13 +460,21 @@ >>>>> rte_errno =3D EOVERFLOW; >>>>> return -rte_errno; >>>>> } >>>>> - if (!mlx5_rxq_releasable(dev, idx)) { >>>>> + ret =3D mlx5_rxq_releasable(dev, idx); >>>>> + if (!ret) { >>>>> DRV_LOG(ERR, "port %u unable to release queue index %u", >>>>> dev->data->port_id, idx); >>>>> rte_errno =3D EBUSY; >>>>> return -rte_errno; >>>>> + } else if (ret =3D=3D -EINVAL) { >>>>> + /** >>>>> + * on the first time, rx queue doesn't exist, >>>>> + * so just ignore this error and reset rte_errno. >>>>> + */ >>>>> + rte_errno =3D 0; >>>>=20 >>>> Unless this function returns failure, the rte_errno will be ignored by= caller >>>> and caller shouldn't assume rte_errno has 0. Caller will assume it is = garbage >>>> data in case of success. So we can silently ignore this case. Does it = cause any >>>> issue in application side? >>>>=20 >>> Not application side but mlx5 PMD this time: >>> **mlx5_fdir_filter_delete**=20 >>> which just _return -rte_errno;_ >>=20 >> Looks like an error. mlx5_fdir_filter_delete() can't be like that. We se= em to >> have lost the code while refactoring it. Let take it offline. >>=20 > Sure ~ >=20 >>> For sure, _mlx5_fdir_filter_delete_ should be more defensive, should no= t assume >>> rte_errno is zero if no error happened. >>> However if the caller know that an error will happen and rte_errno will= become >>> meaningless (garbage) for the succeeding functions, Catching the expect= ed error >>> and resetting rte_errno will be better. What do you think? >>=20 >> Still don't understand clearly. There would be many other similar cases = where we >> don't clear rte_errno when returning success. I don't understand why thi= s case >> should be taken as a special one?? >>=20 > No, the _mlx5_rxq_releasable()_ returns *error* (-EINVAL)(the rxq doesn't= exist) > as my understanding > We only check if rxq is not releasable(=3D=3D0) in previouse version but = we didn't > check if function returns error or success I think that was intended. The case where rxq doesn't exist is silently ign= ored. I know it is a little weird but still I don't see any need to clear rte_err= no. Thanks, Yongseok=