From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <jackmin@mellanox.com>
Received: from EUR03-AM5-obe.outbound.protection.outlook.com
 (mail-eopbgr30057.outbound.protection.outlook.com [40.107.3.57])
 by dpdk.org (Postfix) with ESMTP id EBD02F11
 for <dev@dpdk.org>; Wed, 29 Aug 2018 05:52:26 +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=fNLlx9p+PqbwqD+QyU1Bw/9CjHCKBkskeKluMv11h2U=;
 b=TtMy5jnJkxatN2YKTwnRcyxpUloeLCl/1WAOxJBxj4hqQSP2HlgfqQTnP1w+tp9t/o+iQbFFnEdW6HsA+ZqL/jcOFZtpckBkuq4uno1cYHCNxXNXVKf9FzmfmxrLzEwvUSScggMuDkZ0lI9qifIj4z8spD1US5m5I1P1QXh3jA8=
Authentication-Results: spf=none (sender IP is )
 smtp.mailfrom=jackmin@mellanox.com; 
Received: from MTBC-JACKMIN.mtl.com (58.34.204.108) by
 AM5PR0501MB2020.eurprd05.prod.outlook.com (2603:10a6:203:1a::18) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1101.14; Wed, 29 Aug
 2018 03:52:23 +0000
Date: Wed, 29 Aug 2018 11:52:07 +0800
From: Jack MIN <jackmin@mellanox.com>
To: Yongseok Koh <yskoh@mellanox.com>
Cc: Shahaf Shuler <shahafs@mellanox.com>, dev@dpdk.org, xuemingl@mellanox.com
Message-ID: <20180829035207.vlboi3og4o27rqvp@MTBC-JACKMIN.mtl.com>
References: <20180823063851.32559-1-jackmin@mellanox.com>
 <20180823210808.GA31847@yongseok-MBP.local>
 <20180824064500.uopotkliiaygvvny@MTBC-JACKMIN.mtl.com>
 <20180828204217.GA45628@yongseok-MBP.local>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180828204217.GA45628@yongseok-MBP.local>
User-Agent: NeoMutt/20180716
X-Originating-IP: [58.34.204.108]
X-ClientProxiedBy: HK0PR04CA0006.apcprd04.prod.outlook.com
 (2603:1096:203:36::18) To AM5PR0501MB2020.eurprd05.prod.outlook.com
 (2603:10a6:203:1a::18)
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: a9638b39-fdaf-45e6-7814-08d60d62d503
X-MS-Office365-Filtering-HT: Tenant
X-Microsoft-Antispam: BCL:0; PCL:0;
 RULEID:(7020095)(4652040)(8989137)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020);
 SRVR:AM5PR0501MB2020; 
X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2020;
 3:CSnCSi33dsljhDeD9na8O+eFz1ev31p458EbEcnqewy6iY8YZQa93yDr16P5L5EudPm5CsqXDaqHTvD1lSMgt5zvYRPZHu63piA4Z+S0n/5IeGKr5kcI/QGYMcL9vvRHKeoauTeOx5z6rMKJ+o6r5OH3EIafBzde2/ZoYlnJLjudb+9DA5RPUGtAk4B20ELfteS4vkYK+wAO9kO2mvy7T5jyqKf0/UFXe8PBuoGzyyG32oi+wvhXVCtMMz244n8U;
 25:5H1AuMDAs3yRvj+tIk0NBBSGl6dF5prb44mD5TWm31Jq/ON0GjhO3et5dWT1vwxL8TZKQGIbWYEFbE5WuP7pWZ11FivwNl2JwR/JaHcoFqiIiZaCEx5WLoIhVYpZ/UFXxvhC1IuRxf1G3cScfMYLtxm7n1an6cjrzT/3/dzjLnIt+un/4ZZy/D2b5VKLZECegLNQPTP6sQ5bdOQTZhuDcTeUizKNibxjKVCOiqcmrg7f9vh5koRKLS7U7Q/8T7Nd1PB/+YtOUJ2UiGK38Q7iv3eBerYMR61vk3DWlzl8SflTJjsa7Df2kNpWfWYbZfxpDTtNwJhrkDJXmP7axQ378A==;
 31:VT9U+kAviK8QOEVDaGl0YjZxj6kLhAmJTDLm2pc/Y/uRZTHD/m8VxjV2q2yTMqU5H99LPsdhTEJOIgtK55nxEVymfHYbL8f+t4kBzw+mXBenDhkej0drb0Gen/lp8yXvcOBRUoXFdSAoK+LrzuHkB3RTD13LJVGFTqusVfhtTmguuHOb7gG+rFRzxUNDznLnhR+5cdvukiWTRrdxtt88pxkdXlKplM/QlI6e1Ayuaw4=
X-MS-TrafficTypeDiagnostic: AM5PR0501MB2020:
X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2020;
 20:4W+tv/qxXvQRRulnZK90dmOV6SfdY5VZ/+WK3GFMCeTxSdV9VfESyZ5QLdGkiTTqd1Te6iQMYCE223OGnV6YF6AQsEfsLxjLfnyebCrhcS10K+2Fs9xx/C3+zXY10kFeqaZ+EQMai4wB2Vny1yT8nnqgBWS5t6TSXN5wixzJHDnC79SkPS4/Cj9Gl04A8SbmSJdJEsPrxDl0HtEIPnatNJs0hVFxMa1mZvm5kDx3AOpPQT2vNOUTEPNlK+kq9e0D0DyC47Dy5+F64g8KUlH1QTvgr5jjYeoS3w4G7CKTB0DCqrsw794VAgh5r0qA/2QdvJGzCKuT3nSzR1ziqzMp+WKq15OdcUMaOhJRP//og6mhymAqTWMcXSDko0WKi+j32tHsmwFG9x/1RB3QRFXOI2OGLQ+gya4kFFoPGAR+uhvlc728+w+COT8yNB00FlPqgTYD0En3NaY+CyjdnuiSdxJPXO0q9nv1mhJafxDjKCsDLrWX7nAWUb+939TKYhd4;
 4:LfpoqY1IR3Q6eJDQT6mV7Y6+e5wXaiB8CPMiaaNhN6xmhZB7FLOHYt6rseAjt1V3zjVaC7MN//u6aLm1LAxm0xWSonG6naNd2vEKNxGSgJJVFgvdu8/0kTwfBWR9YV2Nftx6np/v/Jqj9gND3kOII5zCq+FVACsCGoDeCNs0QV0V7BamEPxR1TrlidIGx42OIMABfIWwCanmNj3V4DAB4qYDYn7mw2UB1egL7ny0nwL1uVWHjF2wGbq2gK3MF+q4DoaxyLc2khEQi1V/SYhTyg==
X-Microsoft-Antispam-PRVS: <AM5PR0501MB202047BBA55FF9433061D1AECC090@AM5PR0501MB2020.eurprd05.prod.outlook.com>
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)(3002001)(10201501046)(3231311)(944501410)(52105095)(93006095)(93001095)(6055026)(149027)(150027)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123562045)(20161123564045)(20161123560045)(201708071742011)(7699016);
 SRVR:AM5PR0501MB2020; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0501MB2020; 
X-Forefront-PRVS: 077929D941
X-Forefront-Antispam-Report: SFV:NSPM;
 SFS:(10009020)(136003)(366004)(396003)(39860400002)(346002)(376002)(189003)(199004)(25786009)(16526019)(478600001)(14444005)(68736007)(8936002)(97736004)(4326008)(5660300001)(11346002)(6862004)(446003)(86362001)(26005)(6506007)(386003)(6246003)(93886005)(956004)(53416004)(476003)(107886003)(16586007)(316002)(58126008)(47776003)(52116002)(7696005)(7736002)(8676002)(229853002)(76176011)(81156014)(486006)(50466002)(66066001)(2906002)(305945005)(105586002)(53936002)(81166006)(6666003)(6636002)(106356001)(23726003)(9686003)(1076002)(3846002)(6116002)(55016002);
 DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0501MB2020; H:MTBC-JACKMIN.mtl.com; FPR:;
 SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; 
Received-SPF: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM5PR0501MB2020;
 23:Sy1acWsvof2gsxiam4Up027nswOlPOANyeu5Rnw?=
 =?us-ascii?Q?EFAWqXOEjE3XfHXuCTYusaGX9ctyxJ/eyEUNuTejLmB+pDSBejb21Q1eM524?=
 =?us-ascii?Q?mrBMJ1HVYqFxfjoZWicGWHeoEW4LQ1ZHswsn5gHiTJMgMALTgFG/7rueZkZa?=
 =?us-ascii?Q?jsZZQg4BVG8dcNIwBKPhvXyHZcJLHVkEZRtJrt1XAKaiDfKNJ5stQq5YYuYs?=
 =?us-ascii?Q?9mpXWeZRERvw12QXHpmAHSifkzPGRfimWTBnoR6blk9F8V1wiX1HdaPDOQRR?=
 =?us-ascii?Q?TWYNU+s+KXs1f5eLWYvaqqPa4IOS5zemVHmiWoYc+xj2aTf21p7W3oS8dk9G?=
 =?us-ascii?Q?Bne3A2wxKnh70UWsq6LjmtDkqJmZ4B55+A/NG+fgJYcZrb7tAiFfTwKkJarA?=
 =?us-ascii?Q?YuLHQxh2345Vo+XNgvrB41JT/gP9Vvif+lt4oHI5vVTC4zW59ApXPgzoe8HB?=
 =?us-ascii?Q?dz6OzxFrPXeM/r3CZYaDL2lgIRuHprFNPRTjB4mvt2hvkP2yfYFkjDaWsiiL?=
 =?us-ascii?Q?8ziIu6hjn/jXB9pRBjK62H1Et1kk2Qu9DG2UYZEUso6LrEr92nECYjF7L5z8?=
 =?us-ascii?Q?UJJCsVrR7MzKG8tphDh1r2GDbNuf7/1wrIuM5S3kAy96uqY7NbBNupQaI2Ea?=
 =?us-ascii?Q?pHfi7J9oPs+M5xnmYZRxh/Zv9IJjzVNZ6tUYBWM4XUe2XWdCajiaLhLhcDh/?=
 =?us-ascii?Q?mOs9nMQ4xQhJhRJO9CW0yR9tX4YYglxLnp0Pxtw4mVa5NZODbQrXCIn8kD8m?=
 =?us-ascii?Q?4/aQy0Y2QphiUiP9a+L0G+BmFlU7PR5OiE1rWuVbmNvjINXhWavgrUVDZQZB?=
 =?us-ascii?Q?ru1owmhGzokDieWV1O7/vV5Vto0s9ynbbvEABW7lZkoqV//u7GVfjYNWMXAX?=
 =?us-ascii?Q?Rm/1g9C5cyyhFJipEX1WI70KSF1X+/MOK54GGCickv1imzOcNbvd7T+e0X2N?=
 =?us-ascii?Q?NRxkvjiVojRWOrcnwnRgKVfIf5uayv82uQs6CYjGNInRTs+Tdzi+rdrnNqcc?=
 =?us-ascii?Q?fvXtoOgVfBMLS9naPSIVyQ+xwKAEtREjDt/Mm3BbuD2jEtbwF8XevJkWv5Cb?=
 =?us-ascii?Q?QO5h0xYbLDMRaQv+VdaZsUg0mDr9wUiXkM/3Rp1/UIgKxRsY3SGgg56Jlxe1?=
 =?us-ascii?Q?Y0xpvmV6JgxeClq8Bk+ew3v6PWS4ciTteWrPrl/Pc+EM0LDwEFxLB2Y3J3kt?=
 =?us-ascii?Q?lMRCeg9GTJCrsU9CubyjKfXtnRdtSaC5VJczBIFlctjGsMFzGs96RdgZvGyb?=
 =?us-ascii?Q?FqhJHinBtHnot4WvSf5IuhuOPdtrfQ2wNATFywJDq?=
X-Microsoft-Antispam-Message-Info: 6PStmokgSdicd5U8kQ2YUmTE4DULZIUoxLLoYj+QY84E95LrAqZNMMkHayXf7Rnb6dGMrgwGQsfZdhGJF1lY8+rRCBj/O4ckgOrsFCVyCfzGdkXb8CQqEhAoBpNQ8b2X2FHlbtdgbIDIkZCx91zoi6zpRUQmqXmFOOojbnTfCmBcLLnG4UImzZtF2uNylFew436mebwsSrNH+ELb07QpjXHNgX0qsF2aacsrCgdTWn6HQZiKqScsmkLJfs6qu5087IGNidQmc3tll1bFhOYvHvpjYnjS1gyMys1JHX+Q1OgqmBvPanWb4rmnRYem6wp34yt2edoFVQR6AZ7gTRIOuIbU5UPe6BD+nRhogjql3NY=
X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2020;
 6:M/qRBvQQ0v2HtnUkXWbxI/ELQFEkKEzxtVwCkOMdvVo9F9Ihff+ebyM3w1xU44j6cJ1HT4RuwsnnSe5eBUX0XcH+Fmr9TYJIBe+DqC+KV4Eg1M2oucgJ7+SzpQwCN1vTqHjr2KwyxCwdgmOKDdqhSFsb8y9TXA4de8P5eZWrMyYfRGm/SgSFb8ldEWSDYMsnmlYojnhZvrblSIxNQgVbgQ8E4lz6sOqKhcgsONrFg67rBwP7+85vTqTAJ1X9qn/K6nAPHhCON3vJLV2ouJveYsdp8/rP+NTjz5f68WTOgDLl3LwWl33XmbKR/jFPUhAvv6EH3nXPaW6dQVlsQzMHT/y+k1MDDNwR9ZRDtNiZqTZ2U6Ajc9I8ca3bJ8Vz7iFvYJD0jtRiFbq5vJDPQ9xUHHS1oJj6vDCL4omEiUlzd0Q63TPyho1twBSbaghfloT8GZtwGeJcifgoVXoY4cPpLw==;
 5:zc3k2FAnnJWks9XVARZYS8B6QHkDwSMD2fzA3DfKhXil0Lh6PRayHYujQTqjunYlgnqxn7efKcYAvZm006dS564KWPJ+LfJewir7H/qDmLhRLmG0F77+L2tUDnZIL9H/shqfTWHOHHNzzq31bvfuOzD4n7rCw1/qy9J1CL6qQ4A=;
 7:ruQ57tg5mp+B3lQ5xoCEE5Xvu9xpwTTMOJKkRnARmnVzAD0n3WqNJjIkjhsCjYu6onGgw7ndOPTUxbez1/RA+15xfnoYcHHczhc/zzCbWc1e0RmSsRGx5dX0xWkexyOU798CcMK1KoMfLV5FAssejeeedwOzjJZseeUBMDy4Wj7IK5lb7bfzG6HHCEO074KbbxOizltD6AeTLAekW9Rw3OFnsvQVW75fwuDQJevqKhzQaSOVMxBJmXhvZTO6fju1
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Aug 2018 03:52:23.3329 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: a9638b39-fdaf-45e6-7814-08d60d62d503
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0501MB2020
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 29 Aug 2018 03:52:27 -0000

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
> > > > 
> > > > 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
> > > > 
> > > > Signed-off-by: Xiaoyu Min <jackmin@mellanox.com>
> > > > ---
> > > >  drivers/net/mlx5/mlx5_rxq.c | 20 +++++++++++++++-----
> > > >  1 file changed, 15 insertions(+), 5 deletions(-)
> > > > 
> > > > 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 = (*priv->rxqs)[idx];
> > > >  	struct mlx5_rxq_ctrl *rxq_ctrl =
> > > >  		container_of(rxq, struct mlx5_rxq_ctrl, rxq);
> > > > +	int ret = 0;
> > > >  
> > > >  	if (!rte_is_power_of_2(desc)) {
> > > >  		desc = 1 << log2above(desc);
> > > > @@ -459,13 +460,21 @@
> > > >  		rte_errno = EOVERFLOW;
> > > >  		return -rte_errno;
> > > >  	}
> > > > -	if (!mlx5_rxq_releasable(dev, idx)) {
> > > > +	ret = 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 = EBUSY;
> > > >  		return -rte_errno;
> > > > +	} else if (ret == -EINVAL) {
> > > > +		/**
> > > > +		 * on the first time, rx queue doesn't exist,
> > > > +		 * so just ignore this error and reset rte_errno.
> > > > +		 */
> > > > +		rte_errno = 0;
> > > 
> > > 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?
> > > 
> > Not application side but mlx5 PMD this time:
> > **mlx5_fdir_filter_delete** 
> > which just _return -rte_errno;_
> 
> Looks like an error. mlx5_fdir_filter_delete() can't be like that. We seem to
> have lost the code while refactoring it. Let take it offline.
> 
Sure ~

> > For sure, _mlx5_fdir_filter_delete_ should be more defensive, should not 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 expected error
> > and resetting rte_errno will be better. What do you think?
> 
> 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 this case
> should be taken as a special one??
> 
No, the _mlx5_rxq_releasable()_ returns *error* (-EINVAL)(the rxq doesn't exist)
as my understanding
We only check if rxq is not releasable(==0) in previouse version but we didn't
check if function returns error or success
> Thanks
> Yongseok