From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10062.outbound.protection.outlook.com [40.107.1.62]) by dpdk.org (Postfix) with ESMTP id D9F5A1C88C for ; Thu, 5 Apr 2018 07:36:01 +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; bh=AzQCn7AOVzcoLo5xEUcjPFxEO6r1bagPAYr/74MI8zg=; b=Uq8SYeLkuZ4lMTCTOnCw3zqd07lduQAf6dWFbpD1Ysyej4MdarnRWvc39KqOkECS37mfTOIBeYtht5tPzrCachZAMVU1IYi3tJ+2EfL/V07/v0hzifdI2ZEXGjfSOAEw1pe63lHZBVOT3agOq/fOhRQ9XuOWosrKTI8h+Ci/+uk= Received: from DB7PR05MB4426.eurprd05.prod.outlook.com (52.134.109.15) by DB7PR05MB4361.eurprd05.prod.outlook.com (52.134.108.150) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.631.10; Thu, 5 Apr 2018 05:35:58 +0000 Received: from DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::808d:386e:26f3:859f]) by DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::808d:386e:26f3:859f%13]) with mapi id 15.20.0631.013; Thu, 5 Apr 2018 05:35:58 +0000 From: Shahaf Shuler To: =?iso-8859-1?Q?N=E9lio_Laranjeiro?= CC: Adrien Mazarguil , Yongseok Koh , "dev@dpdk.org" Thread-Topic: [PATCH] net/mlx5: fix link status initialization Thread-Index: AQHTy+bChNPXB2fkBUSu8VuSnLilOaPwRWOwgAA+wYCAASMw0A== Date: Thu, 5 Apr 2018 05:35:57 +0000 Message-ID: References: <20180403044817.27457-1-shahafs@mellanox.com> <20180404073009.zgqu3yrj26trwdfr@laranjeiro-vm.dev.6wind.com> <20180404121051.ersiyf75gykwfon5@laranjeiro-vm.dev.6wind.com> In-Reply-To: <20180404121051.ersiyf75gykwfon5@laranjeiro-vm.dev.6wind.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; DB7PR05MB4361; 7:psqmJ+kSM5zUBoSb4meA0xGc4+/s9LPI7jr2dBM/D0DFT7RJAMo4+0aphAbZFV8FhwF4i0vcNgC5Te9rYptYMaU/TXEJI5LW6OIIg6l+K1iyOPvK24tw41Y7S9qbZRr1mLVvxH2mxGCBcGsqK4DxR/PZ+mP4MHAUcrd72FBQKhJixOONyaRl4lQtXNvEBtw1LLgtpDrnk1t5MJgIDazLCQFIwx0eNRJRSY8MvP+NceKZEZKoUZMF5PadyBTFZtXa x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: c181d72d-6aba-4c4e-fed4-08d59ab71bff x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(3008032)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:DB7PR05MB4361; x-ms-traffictypediagnostic: DB7PR05MB4361: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231221)(944501327)(52105095)(93006095)(93001095)(10201501046)(3002001)(6055026)(6041310)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123558120)(20161123560045)(6072148)(201708071742011); SRVR:DB7PR05MB4361; BCL:0; PCL:0; RULEID:; SRVR:DB7PR05MB4361; x-forefront-prvs: 06339BAE63 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(396003)(39380400002)(376002)(346002)(39860400002)(189003)(199004)(305945005)(74316002)(6506007)(14454004)(7736002)(76176011)(59450400001)(81156014)(5660300001)(6246003)(8936002)(81166006)(55016002)(9686003)(486006)(7696005)(186003)(476003)(105586002)(86362001)(99286004)(97736004)(25786009)(102836004)(106356001)(11346002)(3846002)(6436002)(66066001)(53936002)(6916009)(478600001)(2900100001)(54906003)(6116002)(316002)(4326008)(229853002)(26005)(8676002)(68736007)(5250100002)(3280700002)(446003)(2906002)(33656002)(93886005)(3660700001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB7PR05MB4361; H:DB7PR05MB4426.eurprd05.prod.outlook.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-antispam-message-info: z89ld4HxbZxwCN2DDzUei/1pwW3myWI3fbIp3RFgcBZG71Wx4Bj1rTRqeWHFqT0jOjef3HGoWoWG5cKCNTWR5O9s70OWCFLeJlx/ZK88RwkWc/sYiUahYsjWKQNHwqfE8QfbOOQzayNLyk8ZuvnW7eUe3CRjVlHwcDSFbgwcXNRH+CKFvrjreNhpW53l09KjCS77H0tVcJ93UY7vFuBao1el7/LlQ3rmixL+4zgH5QTj+LkzbLw+vUiUjoYI30REHC8qVodxH3re4tSuMZl92JlYRHXQQUG7BJNPLiX/+41ga4t75yscAm+A51yp/VhFFtzLKLH29YWhcz9wxlFZOEx+p85Q73CrRHe5sQdVEvcvRO8+s38CcaxwKEoA2IuN4pX/wP0F73yikIXdTFqaYxfQdkG1ZUC96+6BVqR80FU= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: c181d72d-6aba-4c4e-fed4-08d59ab71bff X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Apr 2018 05:35:58.0372 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR05MB4361 Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization 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: Thu, 05 Apr 2018 05:36:02 -0000 Wednesday, April 4, 2018 3:11 PM, N=E9lio Laranjeiro: > Subject: Re: [PATCH] net/mlx5: fix link status initialization >=20 > On Wed, Apr 04, 2018 at 09:58:33AM +0000, Shahaf Shuler wrote: > > Wednesday, April 4, 2018 10:30 AM, N=E9lio Laranjeiro: > > > Subject: Re: [PATCH] net/mlx5: fix link status initialization > > > > > > On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote: > > > > Following commit 7ba5320baa32 ("net/mlx5: fix link status > > > > behavior") > > > > > > > > The initial link status is no longer set as part of the port start. > > > > This may cause application to query the link as down while in fact > > > > it was already up before the DPDK application start. > > > > > > There is something wrong in this explanation, the application should > > > query the link using this same callback, why the PMD should call it? > > > > It is how ethdev is implemented. The application is doing nothing > > wrong, it queries the link status using rte_eth_link_get_nowait > > > > When the application works with LSC interrupts the ethdev layer skips > > the PMD callback and just update according to the link status exists > > on device data. > > It is because it assumes the link status on the device data is the > > correct one since any link change is processed by the application. > > > > The issue is with the initial state of the link. If the link is > > already up when the PMD starts there will be no callback for the > > application. > > > > I think this logic is OK, and it is also a good practice to initialize > > the link status to the actual state of the link as part of the port > > probing. >=20 > The commit log should be re-worded to include this explanation. Will add. >=20 > > > > Fixes: 7ba5320baa32 ("net/mlx5: fix link status behavior") > > > > Cc: nelio.laranjeiro@6wind.com > > > > > > > > Signed-off-by: Shahaf Shuler > > > > Acked-by: Yongseok Koh > > > > --- > > > > drivers/net/mlx5/mlx5.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c > > > > index > > > > 7d58d66bb9..f954ea2862 100644 > > > > --- a/drivers/net/mlx5/mlx5.c > > > > +++ b/drivers/net/mlx5/mlx5.c > > > > @@ -961,6 +961,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv > > > __rte_unused, > > > > DRV_LOG(DEBUG, "port %u forcing Ethernet interface up", > > > > eth_dev->data->port_id); > > > > mlx5_set_link_up(eth_dev); > > > > + mlx5_link_update(eth_dev, 1); > > > > /* Store device configuration on private structure. */ > > > > priv->config =3D config; > > > > continue; > > > > -- > > > > 2.12.0 >=20 > According to your analysis, this is only necessary when the LCS is config= ured > in the device. Why not adding this call to > mlx5_dev_interrupt_handler_install() which is responsible to install the = LCS > callback. I think it is good practice whether or not LSC is set.=20 The link status should be initialized to the correct value after the probe. >=20 > Another point, the wait to complete flag is useless, if the link is up, t= he status > and speed will be accurate, if not, it will receive an LSC event later. Agree. So how about keeping the code on the current place, just removing the wait_= to_complete?=20 >=20 > Regards, >=20 > -- > N=E9lio Laranjeiro > 6WIND