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 82612A00BE; Tue, 19 Apr 2022 11:11:54 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6DBDD4068E; Tue, 19 Apr 2022 11:11:54 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 5FD4540687 for ; Tue, 19 Apr 2022 11:11:53 +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] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices Date: Tue, 19 Apr 2022 11:11:49 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D86FDF@smartserver.smartshare.dk> In-Reply-To: <2536156.9Mp67QZiUf@thomas> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices Thread-Index: AdhQIgxJ6/+xQIP+RNeh+4MPx8f9gQDoJz3Q References: <20220307223442.28012-1-jeffd@silicom-usa.com> <20220414084301.509fe14a@hermes.local> <2536156.9Mp67QZiUf@thomas> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Thomas Monjalon" , "Jeff Daly" , "Stephen Hemminger" Cc: "Wang, Haiyue" , , , 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: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Thursday, 14 April 2022 19.07 >=20 > 14/04/2022 17:43, Stephen Hemminger: > > On Thu, 14 Apr 2022 15:11:46 +0000 > > Jeff Daly wrote: > > > From: Thomas Monjalon > > > > 14/04/2022 14:13, Wang, Haiyue: > > > > > From: Thomas Monjalon > > > > > > 14/04/2022 03:31, Wang, Haiyue: > > > > > > > From: jeffd@silicom-usa.com > > > > > > > > From: Stephen Douthit > > > > > > > > > > > > > > > > 1G Cu SFPs are not officially supported on the X552/X553 > family > > > > > > > > of devices but treat them as 1G SX modules since they > usually > > > > > > > > work. Print a warning though since support isn't > validated, > > > > > > > > similar to what already happens for other unofficially > supported > > > > > > > > SFPs enabled via the allow_unsupported_sfps parameter > inherited > > > > from the mainline Linux driver. > > > > > > > > > > > > > > > > Signed-off-by: Stephen Douthit = > > > > > > > > Signed-off-by: Jeff Daly > > > > > > > > --- > > > > > > > > drivers/net/ixgbe/base/ixgbe_x550.c | 14 +++++++++++++- > > > > > > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c > > > > > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c > > > > > > > > index 8810d1658e..8d1bc6c80d 100644 > > > > > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c > > > > > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c > > > > > > > > @@ -1538,9 +1538,21 @@ STATIC s32 > > > > > > > > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, > bool > > > > > > *linear) > > > > > > > > > > > > > > NACK. > > > > > > > > > > > > > > As for 1G Cu SFP treating it as 1G SX, some 1G-Base-T SFP > modules > > > > > > > require the use of RX_ILOS and some Intel Ethernet = products > don't > > > > support that. > > > > > > > > > > > > So what is the solution? > > > > > > > > > > > > > And the DPDK keeps the same design with kernel. > > > > > > > > > > > > It should not be a justification for limiting DPDK features. > > > > > > > > > > Um, this is upstream version driver to keep the same behavior. > > > > > > > > > > There are also some kind of custom release ... > > > > > > > > I don't understand. > > > > Upstream DPDK (and Linux) must support a maximum of hardware and > > > > setup. > > > > Why rejecting adding such compatibility? > > > > > > > > > > so, I will ask a question directly in case people just aren't > inclined to make a suggestion > > > (and perhaps this should be also directed to the Linux kernel > driver mailing list), but > > > if there's a driver option: module_param(allow_unsupported_sfp, > uint, 0) to allow > > > enabling non-official support of some SFPs, then I can't image = that > it wouldn't also be > > > acceptable to add: module_param(cu_sfp_as sx, uint, 0) to be able > to select whether > > > to enable this specific handling as well? > > > > > > if a patch of this nature is acceptable to Linux driver > maintainers, then it would also be > > > here as well according to your explanation of the NACK, correct? > > > > Makes sense for DPDK to have a similar option to enable (at your own > risk) SFP's. > > But: > > - there is no equivalent of module_params in DPDK; you will have > to think of something >=20 > We have devargs which is supposed to be used per port with the option = - > a. > In future, I would like to use devargs with another option > which is not necessarily tight to a port, so it can be per-driver. > The devargs syntax already allows to configure a driver, example: > class=3Deth/driver=3Dfoo,param=3Dbar I have a very strong preference for per-driver options over per-port = options! Per-port options are difficult to work with when the application detects = the hardware configuration at runtime. E.g. when using modular hardware, = where different types of NICs may be plugged into a NIC module slot. >=20 > > - should print message that when enabled the driver is no longer > supported. >=20 > It could be supported by Silicom. There's more to "supported by" than meets the eye: When an ODM designs = products using Intel chips, some sort of customer support from Intel = field application engineers is expected by the ODM. We cannot expect = Silicom to provide design support to anyone but their own customers. = E.g. if the NIC is behaving weird at the hardware bring-up phase, where = it might be any type of problem, Silicom will not be able to provide the = kind of support required. My point is: There is a difference between = community support and customer support. Let me throw up an idea for consideration... I'm trying to think out of = the box here, so please forgive me if I'm stepping on anyone's toes with = this suggestion: If Intel doesn't want to take on the responsibility and support for this = feature graciously donated by Silicom (which is obviously Intel's own = decision to make), but the DPDK community thinks the feature is = beneficial, perhaps Silicom could be accepted as the maintainer of this = part of the driver? The driver would still come with a big fat = disclaimer saying that this feature is not supported by Intel, but = maintained by Silicom, who also provides community support for it. The worst case alternative is a fork or separate add-on patch set = offered by the donor. This has certainly happened to other projects. = Don't get me wrong, we are not there at all regarding this feature! I'm = just wondering if we can make the DPDK project even more inclusive, so = we can avoid forks and add-on patch sets now and in the future. -Morten