From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9FA44A00BE; Tue, 19 Apr 2022 19:33:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 37B204068E; Tue, 19 Apr 2022 19:33:47 +0200 (CEST) Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70097.outbound.protection.outlook.com [40.107.7.97]) by mails.dpdk.org (Postfix) with ESMTP id 0691540687; Tue, 19 Apr 2022 19:33:44 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZOE5+Z5yZi9/0hHg0xsJ8YIu07EdkXTtL/AQFIjC67GcKWCQ6wox0yghuYXoBcbW3z42lrbn4ZmY17/F+x62+070l06c17XAEn5N96KpFWLhbkGIZNhqpMLAFayEJ0yOulc+2qS9UUBzmqkDmgM+FfgN7bWwHk7l6IDJ8mGAQvxT4rFnfmAGvjOFnEM40Z8i2vw03fvVPtKDX5qTxIsdmTFYu8aWIFwO9P36JYDCfpnvGWWZErDTfO1VLNflehKQWqTVjBYIFGyXfdIsC2zMPnVmAF6DyiSXvwZeQZ1t8W0YFzKknkkoqWKuhDWRcawmUoJGtHtlShL3/LUMT+sVnQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=zHu+Sv+8b6OZJ+2KFp1peYDYx/UcxApsJKw1F7Ql4u0=; b=falOmKqBbcqRBelnvKEe/44E7+rQWcEMZMxfW/0t/Ohfo9oy2iz48j5JGnuRMz3VYpjKW0ZEgZiEGRXbF0hvYnlDty7ZHKFP4+8CsK39GxQjYEecyBi/WW5j7BlAzOeN5tCVxsmV2tXJGFBnArjlICH7SVjBbH4825mL9H7YAYzbo7Ey3uhnFJnAV6SYMqD2EiEKqfha+LQfE1EKG0Rnzsn7MGrTOrVRgu2rDXIq/h0qK8PVdYfaxYjaavep3vKfzki0qcZCTvVmF+RtGytw3L37n9tGDZ3nPv++wAe5Pit+N/wa1PaxEqZEwUA/qnrt926c9svQsvfx+F/Ip0LPrg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=silicom-usa.com; dmarc=pass action=none header.from=silicom-usa.com; dkim=pass header.d=silicom-usa.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=SILICOMLTD.onmicrosoft.com; s=selector2-SILICOMLTD-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zHu+Sv+8b6OZJ+2KFp1peYDYx/UcxApsJKw1F7Ql4u0=; b=s3/3f5uIb+T8aHJRWeg9/ct3lk77paLG7qSlYDhZbEEiRhnc+BRmWKW0r0haSIdLgAcD+oYlRzyN2GUEtBjv8DbCPwC91d8Ib3u90t94lhdBJHbnPk5dtSx/F5RgazLPomvxEWwxHI/Be6NitL0pi/WamWqWX0ceYBsRF7YHP2U= Received: from VI1PR0402MB3517.eurprd04.prod.outlook.com (2603:10a6:803:b::16) by DB9PR04MB9401.eurprd04.prod.outlook.com (2603:10a6:10:36b::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5164.20; Tue, 19 Apr 2022 17:33:42 +0000 Received: from VI1PR0402MB3517.eurprd04.prod.outlook.com ([fe80::5dd5:47e1:1cef:cc4e]) by VI1PR0402MB3517.eurprd04.prod.outlook.com ([fe80::5dd5:47e1:1cef:cc4e%6]) with mapi id 15.20.5164.025; Tue, 19 Apr 2022 17:33:41 +0000 From: Jeff Daly To: "Wang, Haiyue" , "dev@dpdk.org" CC: "stable@dpdk.org" , "Yang, Qiming" Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Thread-Topic: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Thread-Index: AQHYTpSxgnie9TYA7EmxFtoINkPBYqzut1YAgAAC04CAAIB+kIAAGaEAgAboqgCAAEm7gIAAuebw Date: Tue, 19 Apr 2022 17:33:41 +0000 Message-ID: References: <20220228152937.21247-1-jeffd@silicom-usa.com> <20220412174220.31195-1-jeffd@silicom-usa.com> <20220412174220.31195-3-jeffd@silicom-usa.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=silicom-usa.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: bff0ba5b-6026-4511-9662-08da222abf47 x-ms-traffictypediagnostic: DB9PR04MB9401:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: hmAF9C7rMUnXt/jHdV1aQEKvq1I9HSZgUfCRuJW0TNSJuZAyE4fNXgFKvPW12bgVf5wbRzjaOO62uk4tcu1N4y0V2MoxWr7XeuQfENHy5GVaT3If65GtOAvd5uWqlS645nXKtaLg2GhfcJrUoC4QAVredjJcsb3iUR/Kezjf3hTZDyL567ajLM1db35WjePySLr37FTtEW0YKrtQ3AofYXdLiPz7dwvwfBROohTWwpE9jbv8odWLsnE/nzi+8pZXS/fQ4zUUXv2l+Kig6ASyyIOpF2dnKQgLuaQL21O1bSdEcCJPADuRNbN5I6gJxkxV34wCDvkxJCX0Np3EpGwyuCplBXogoHcehUAHtDSDs5gAJEQKyRntgwGoqCW/j5E/j2ySiaNhMlV4rAM+C+2OMqAm84d1EG6W6Bg5hvsW3brmf3OyYi1/u7YXPBSx2c1XINYgJ6L1PDkMgAYTf1VAziXRmj4GevRqGPXcf6t9kPciAS/+69GwPhamrezFtQoJ0FnlMKBvnn6r/i4YDFolns/6MqJXnxPV0di5tEhPFeY+uWoH9O5wni0fpCMa4fZ8IkryRDp3k89rZfZ68J8B/44eO4ZptJpuiWsqBreSdBjoJ7FgVkAA1o6sYlZ88KuLG4NJuLhU8n3WxW8fWB7qwMcnOr1qPi+frEMJOhLHsEPfpCPc5/i0BFNhw90/tU0DbBWni76k5U1/Nnr9AX/PRQ== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR0402MB3517.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(4636009)(366004)(54906003)(186003)(55016003)(8936002)(7696005)(71200400001)(316002)(76116006)(9686003)(86362001)(4326008)(8676002)(64756008)(66946007)(66476007)(66446008)(66556008)(83380400001)(5660300002)(122000001)(38100700002)(52536014)(110136005)(508600001)(6506007)(53546011)(38070700005)(2906002)(33656002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Ks6DSM64u49EI6hcan44GKVrrpZ0uKGmF+xwhJvaWLarvvjmEvYsB7VYQWo1?= =?us-ascii?Q?T3RcXTnXuBdf9TUZTFuSl1OauEjmIFlBL5YBJ/VtBnCVkeporG8FXkpIEFNI?= =?us-ascii?Q?egygIMT6VJlbqmihpqy1tX1n6Jb94Ke3TZgUOB3qWvZ1zz6PIRF+0dvX4YU3?= =?us-ascii?Q?t6g4E5D4pZIK4/18WcXvtaO3LFc1nuZyQsWCyZYpCoULd3AyoMaY1uWVkT8b?= =?us-ascii?Q?1wyFmnGc/Q+Dv3m4yeone76d7H/T5HR1LH9idztf+MkYHHUlCWEC1WeSeEs+?= =?us-ascii?Q?+rMfAg7/6vXMDZHKW5fAFslwm58ypfBTRrKN/ISRTy6LDDyIZ6TS3aRD60kT?= =?us-ascii?Q?VKqk4QGYGc86gu8NuxOkO2CQsijpOjdBUpG/FX7NzvXThbcJPEKXea1Z+zn+?= =?us-ascii?Q?wbJlSWgyj4tCY6eVRjW+UCLWWw+UJ5Ne1i9117chXlEfJX+BcuP7MtKKX+um?= =?us-ascii?Q?AvB8U/iy4v1wfJ9MbJpVR6ag9q+/g6mQVotaNem/JnXlWS8duUdAY3HRBGgm?= =?us-ascii?Q?JagTUI06/MkjMzHrUqJyVNfi9AmE5VamPIN3BWoRrnzHrHarS/lDGwOr6S5R?= =?us-ascii?Q?OKVfKYnAW4LBbY6Z1ngccjU/RjC3S/dYje14iWoEmu39+nQyqQH48o+Qox2b?= =?us-ascii?Q?CCfNJCfxhCrAL74bSXQ2YUtwAlKliRbad2yBbDdjJdAfBy+N8jlFIK1TvzIO?= =?us-ascii?Q?Io2YTJeFmmy3mvJ8jaUVe8g2PwM3qgOkZrJTAN6edxHO8oRZMW7QV+iWPncK?= =?us-ascii?Q?7XM1hAPtv64qGVfl5T0075JMYUg0d0i5mDOJ0sxMslW+w4I4q41AaMIaus7X?= =?us-ascii?Q?cgyOKdPhzreIpFvzNXW1s6ZVhnsd6CAzD+aZGCparwuSS5WwxZrFUIzLiY4K?= =?us-ascii?Q?MGYJbIojJX+YrRFsRWRLFjhZy1FG7q9VSYenbZDQoKpDqLe95uzKpFprOhKL?= =?us-ascii?Q?VjLqm2LwqkEMsREbWGl6bFzCzt9XeIRAeSeP0BUwLk9Ux0plkzfI8ve8yhwI?= =?us-ascii?Q?80Ce+UquoOs5VQdGo8tc51rQ/gW1iqJg2M7TTg9NHe5HGCpMcmAgFoTp6w+D?= =?us-ascii?Q?vaiE1av2VpRRsytOv3ofJBb2qOZAp5jcYT2pbfPhQaK3Szq78pTPNmEBNlvM?= =?us-ascii?Q?Ntl+LF/YzcUhwkqk5MCvFC/Yog1Pxc5zYrBPIMd0U4HufTEGQbXivDxjUi/z?= =?us-ascii?Q?+5jqFQMK2GaSqW5IcYBCStLV9JV9+rR4SToeFvqkQbZvW3nbdqT/ENrrK9Cn?= =?us-ascii?Q?6oa3aKmbt5r6tq9kBiYN/Tnt0O1VXWr6xLvf9eGzX7gIVegZKRGp1xd8/45N?= =?us-ascii?Q?SXerzrcZD+MIhoW1xVcG+bGdrCf77KJbi305AV3pnyc597wxehgs8KNBRVf+?= =?us-ascii?Q?Yu+7uU0k4wRRsFm13wUX5fcrvlIciwgmI+jhLESQ3j8411wsrxA/JGjLJ7T7?= =?us-ascii?Q?EN+fVYSU1TyPytBVjHjKj4RerDF/OmlhAXreLlzGAl9XBzxSa9BPXeReRys/?= =?us-ascii?Q?++HrVAdzHOfUTGvgQezJ2EmEpZFhgYU1Tg7mioEu8AxpOip134QUxpOk6Ud4?= =?us-ascii?Q?yIHc6zkrF+5sEvOZzUo9AWriKZie/6F+1xtOj1qbB2R+fHn+ayunOnQOsf7B?= =?us-ascii?Q?/nzy2cZW92DaCR+ztz/sbjljzA5y33P+wkItTUhvuCsX+UtLL96A90lM47fm?= =?us-ascii?Q?c4XLiNNIkLH2a5Rh0PRbisdU75DdMZ4OSjHxjRPhjpLf/vhDGDsnZCmkT+Xr?= =?us-ascii?Q?MsHtIwI/ss0LkQuQtsQBPIGJNU9ENvdxSE1DKABFhh73KilcMs+2?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: silicom-usa.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR0402MB3517.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: bff0ba5b-6026-4511-9662-08da222abf47 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Apr 2022 17:33:41.8675 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: c9e326d8-ce47-4930-8612-cc99d3c87ad1 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 9EvS26mciXL7wY71inRn1pQn+LHY52jsyaPHHUqewTf4gs2VVV7NR5jjBrkfhpXl+ZQUCREUfb+lfJxXElVVhA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB9401 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > -----Original Message----- > From: Wang, Haiyue > Sent: Monday, April 18, 2022 10:05 PM > To: Jeff Daly ; dev@dpdk.org > Cc: stable@dpdk.org; Yang, Qiming > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on > hotplug >=20 > Caution: This is an external email. Please take care when clicking links = or > opening attachments. >=20 >=20 > > -----Original Message----- > > From: Jeff Daly > > Sent: Tuesday, April 19, 2022 05:55 > > To: Wang, Haiyue ; dev@dpdk.org > > Cc: stable@dpdk.org > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking > > on hotplug > > > > > > > > > -----Original Message----- > > > From: Wang, Haiyue > > > Sent: Thursday, April 14, 2022 8:11 AM > > > To: Jeff Daly ; dev@dpdk.org > > > Cc: stable@dpdk.org > > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking > > > on hotplug > > > > > > Caution: This is an external email. Please take care when clicking > > > links or opening attachments. > > > > > > > > > > -----Original Message----- > > > > From: Jeff Daly > > > > Sent: Thursday, April 14, 2022 18:41 > > > > To: Wang, Haiyue ; dev@dpdk.org > > > > Cc: stable@dpdk.org > > > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and > > > > linking on hotplug > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Wang, Haiyue > > > > > Sent: Wednesday, April 13, 2022 11:00 PM > > > > > To: Jeff Daly ; dev@dpdk.org > > > > > Cc: stable@dpdk.org; Stephen Douthit > > > > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and > > > > > linking on hotplug > > > > > > > > > > Caution: This is an external email. Please take care when > > > > > clicking links or opening attachments. > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Wang, Haiyue > > > > > > Sent: Thursday, April 14, 2022 10:49 > > > > > > To: Jeff Daly ; dev@dpdk.org > > > > > > Cc: stable@dpdk.org; Stephen Douthit > > > > > > > > > > > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and > > > > > > linking on hotplug > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Jeff Daly > > > > > > > Sent: Wednesday, April 13, 2022 01:42 > > > > > > > To: dev@dpdk.org > > > > > > > Cc: stable@dpdk.org; Stephen Douthit > > > > > > > ; Wang, Haiyue > > > > > > > > > > > > > > Subject: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and > > > > > > > linking on hotplug > > > > > > > > > > > > > > Currently the ixgbe driver does not ID any SFP except for > > > > > > > the first one plugged in. This can lead to no-link, or > > > > > > > incorrect speed > > > conditions. > > > > > > > > > > > > > > For example: > > > > > > > > > > > > > > * If link is initially established with a 1G SFP, and later > > > > > > > a 1G/10G multispeed part is later installed, then the MAC > > > > > > > link setup functions are never called to change from > > > > > > > 1000BASE-X to 10GBASE-R mode, and the link stays running at t= he > slower rate. > > > > > > > > > > > > > > * If link is initially established with a 1G SFP, and later > > > > > > > a 10G only module is later installed, no link is > > > > > > > established, since we are still trasnsmitting in 1000BASE-X > > > > > > > mode to a 10GBASE-R > > > only partner. > > > > > > > > > > > > > > Refactor the SFP ID/setup, and link setup code, to more > > > > > > > closely match the flow of the mainline kernel driver which > > > > > > > does not have these issues. In that driver a service task > > > > > > > runs periodically to handle these operations based on bit > > > > > > > flags that have been set (usually via interrupt or userspace > > > > > > > request), and then get cleared once the requested subtask has > been completed. > > > > > > > > > > > > > > Fixes: af75078fece ("first public release") > > > > > > > Cc: stable@dpdk.org > > > > > > > > > > > > > > Signed-off-by: Stephen Douthit > > > > > > > Signed-off-by: Jeff Daly > > > > > > > --- > > > > > > > drivers/net/ixgbe/ixgbe_ethdev.c | 533 > > > > > > > +++++++++++++++++++++++-------- > > > > > > > +++++++++++++++++++++++drivers/net/ixgbe/ixgbe_ethdev.h | > > > > > > > 14 +- > > > > > > > 2 files changed, 410 insertions(+), 137 deletions(-) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > struct ixgbe_stat_mapping_registers { @@ -510,7 +509,7 @@ > > > > > > > struct ixgbe_adapter { > > > > > > > uint8_t pflink_fullchk; > > > > > > > uint8_t mac_ctrl_frame_fwd; > > > > > > > rte_atomic32_t link_thread_running; > > > > > > > - pthread_t link_thread_tid; > > > > > > > + pthread_t service_thread_tid; > > > > > > > > > > > > No need to rename this variable, > > > > > > > > > > Let's do link related service now, so we can keep it, I missed > > > > > to add my comment. ;-) > > > > > > > > > > > > > I don't understand this reply, are you still asking to rework the > > > > patch or > > > not? > > > > > > > > > > Different thing. > > > > > > 1. This var can be kept to trace the created thread. (change less cod= e to > keep > > > the patch clean.) > > > 2. Yes, two patches. > > > > > > > ok, I guess I'm just being thick-headed here, but I still don't > > understand why you are saying it should be split into > > 2 patches. if I understand *what* you are asking, you're saying make > > the original thread periodic to continuously >=20 > Well, ... >=20 > Your patch merges the original 'ixgbe_setup_link' task into one, this wil= l > make us hard to review the whole design. So what I said > is: firstly, let's change the thread to a service thread to handle the > 'ixgbe_setup_link' subtask firstly. Which is 'ixgbe_link_service' > in your whole patch. >=20 still not 100%, are you suggesting that the original ixgbe_dev_setup_link_t= hread_handler() which currently is not periodic and only really calls ixgbe_setup_link() be= changed to be a=20 periodic task that essentially does what the patch's ixgbe_link_service() f= unction does which would only be checking whether link config is needed and if so calls ixgbe_= setup_link() as=20 before? if I'm following the code correctly, it ends up going down to ixgbe_check_m= ac_link_generic() which looks at SDP0 (in the case of needing xtalk fix) which incorrectly wi= ll set sfp_cage_full when in fact the PRESENT# signal (or MOD_ABS#) is active low. the code is = extremely convoluted in it's effort to be smart so I may be missing something, but I *believe* t= hat what we end up with is completely unnecessary probing of i2c busses looking for SFPs that = don't exist. even when making it periodic, I don't think it's going to end up with working code. > Small patch is good for us to review, and try to do one thing. >=20 > Hope this time, I can make myself clear. ;-) >=20 > > do ixgbe_link_setup() ? I believe the problem with the setup is that > > the sfp_type is only detected once at initialization time and if > > nothing is in the cage then the code just returns IXGBE_SUCCESS, in > > which case making this task periodic is useless. the whole issue of > > hotplug is only addressed by the entire patch which > > 1) makes the > > task periodic, 2) changes the actions of the task to look for whether > > the cage has something in it and whether its been changed and needs to > > be configured again. > > > > > > > > > > we can separate this patch as least into two patches: > > > > > > > > > > > > > > > > > > > > > > 1st, change the thread handle > 'ixgbe_dev_setup_link_thread_handler' > > > > > > from > > > > > > > > > > > > run-once to as periodical, to handle the original issue. > > > > > > > > > > > > The name 'ixgbe_dev_setup_link_thread_handler' may be not > > > > > > suitable now, as it is a service thread. > > > > > > > > > > > > We can change it to "'ixgbe_link_service_thread_handler'" to > > > > > > reflect the change purpose. > > > > > > > > > > > > 2nd, add the SFP hotplug in this patch. > > > > > > > > > > > > > > > > > > > > > > > > > }; > > > > > > > > > > > > > > -- > > > > > > > 2.25.1