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 176C7A050B; Wed, 13 Apr 2022 14:54:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9E9F24068E; Wed, 13 Apr 2022 14:54:48 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id F0C5B4068B; Wed, 13 Apr 2022 14:54:46 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Date: Wed, 13 Apr 2022 14:54:45 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86FD9@smartserver.smartshare.dk> In-Reply-To: X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Thread-Index: AQHYTpWG7ngQWSPahkCox1ZHMx0Ya6ztIqNAgABE6pCAAAO7MIAAAnFAgABMqICAAA2vYA== References: <20220228152937.21247-1-jeffd@silicom-usa.com> <20220412174220.31195-1-jeffd@silicom-usa.com> <20220412174220.31195-3-jeffd@silicom-usa.com> <98CBD80474FA8B44BF855DF32C47DC35D86FD6@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D86FD8@smartserver.smartshare.dk> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Wang, Haiyue" , "Daly, Jeff" , Cc: , "Stephen Douthit" , "Yang, Qiming" 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 > From: Wang, Haiyue [mailto:haiyue.wang@intel.com] > Sent: Wednesday, 13 April 2022 13.49 >=20 > > From: Morten Br=F8rup > > Sent: Wednesday, April 13, 2022 15:20 > > > > > From: Wang, Haiyue [mailto:haiyue.wang@intel.com] > > > Sent: Wednesday, 13 April 2022 09.02 > > > > > > > From: Morten Br=F8rup > > > > Sent: Wednesday, April 13, 2022 14:58 > > > > > > > > > From: Wang, Haiyue [mailto:haiyue.wang@intel.com] > > > > > Sent: Wednesday, 13 April 2022 04.47 > > > > > To: Daly, Jeff; dev@dpdk.org > > > > > Cc: stable@dpdk.org; Stephen Douthit; Yang, Qiming > > > > > > > > > > > From: Jeff Daly > > > > > > Sent: Wednesday, April 13, 2022 01:42 > > > > > > To: dev@dpdk.org > > > > > > Cc: stable@dpdk.org; Stephen Douthit usa.com>; > > > > > Wang, Haiyue > > > > > > > > > > > > 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 the 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 > > > > > > > > > > > > > > > > So BIG change for new platform, DON'T CC to stable! > > > > > > > > What do you mean by "new platform"? The ixgbe hardware and = driver > is > > > not new. > > > > > > > > > > It's soc NIC, ixgbe not support before. > > > > If the patch only fixes the driver for a new NIC that not supported > by older DPDK versions, and that > > NIC is not going to be supported by older DPDK versions, then I = agree > that there is no point in > > backporting it or CC'ing stable. > > > > However, if the patch could also apply to any other ixgbe NIC that = is > potentially supported by older > > DPDK versions, then it should be backported. >=20 > It's hard to say, these years, I see many ixgbe link related fixes. The physical layers have always been tricky... Apparently, they still = are. :-) > At least now, no big link fix for normal ixgbe NICs. I would not discriminate between normal and less common NICs. If they = are not EOL according to Intel, the drivers should support them. In the real world, though, driver development resources will be = allocated to the important customers and/or the high volume products. So = I do understand your concern! Not being directly involved in this work = myself, it is easy to voice my opinion as a "backseat driver". :-) >=20 > This patch still have some kind of TODOs. And this is not acceptable > for > us to maintain this kind of code for released stable DPDK version. I > don't > want to see many follow fixes ... >=20 > And we have two DPDK development cycle (22.07 22.11) to make it for > next > stable release. >=20 I 100 % agree that all the TODOs should be solved first, so the driver = is reliable and complete before any backporting starts. Adding follow-on = fixes in multiple DPDK versions is a waste of maintainer time, and I = agree with your pushback when this is the reason. With that in mind, CC'ing stable could be postponed until the patch = reaches backporting worthy quality. > > > > > > > > > This patch fixes a bug (with a serious impact when occurring), = so > it > > > should be backported. The size of > > > > the patch does not disqualify it for backporting. > > > > > > > > -Morten > > > >=20