> From: Thomas Monjalon [mailto:tho...@monjalon.net]
> Sent: Thursday, 14 April 2022 19.07
> 
> 14/04/2022 17:43, Stephen Hemminger:
> > On Thu, 14 Apr 2022 15:11:46 +0000
> > Jeff Daly <je...@silicom-usa.com> wrote:
> > > From: Thomas Monjalon <tho...@monjalon.net>
> > > > 14/04/2022 14:13, Wang, Haiyue:
> > > > > From: Thomas Monjalon <tho...@monjalon.net>
> > > > > > 14/04/2022 03:31, Wang, Haiyue:
> > > > > > > From: je...@silicom-usa.com <je...@silicom-usa.com>
> > > > > > > > From: Stephen Douthit <steph...@silicom-usa.com>
> > > > > > > >
> > > > > > > > 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 <steph...@silicom-usa.com>
> > > > > > > > Signed-off-by: Jeff Daly <je...@silicom-usa.com>
> > > > > > > > ---
> > > > > > > >  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
> 
> 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=eth/driver=foo,param=bar

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.

> 
> >    - should print message that when enabled the driver is no longer
> supported.
> 
> 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

Reply via email to