RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Monday, May 30, 2022 9:33 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550
> devices
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Sunday, May 29, 2022 6:49 PM
> > To: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org
> > Cc: Stephen Douthit <stephend@silicom-usa.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Subject: RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the
> > X550 devices
> >
> > Caution: This is an external email. Please take care when clicking
> > links or opening attachments.
> >
> >
> > > -----Original Message-----
> > > From: Jeff Daly <jeffd@silicom-usa.com>
> > > Sent: Friday, May 27, 2022 4:44 AM
> > > To: dev@dpdk.org
> > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Yang, Qiming
> > > <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > Subject: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550
> > > devices
> > >
> > > 1G Cu SFPs are not officially supported on the X552/X553 family of
> > > devices but create an option cu_sfp_as_sx to 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: Jeff Daly <jeffd@silicom-usa.com>
> > > Suggested-by: Stephen Douthit <stephend@silicom-usa.com>
> > > ---
> > > v2:
> > > * Introduced cu_sfp_as_sx option, default off.
> > > ---
> > >  doc/guides/nics/ixgbe.rst           | 16 ++++++++++++++
> > >  drivers/net/ixgbe/base/ixgbe_type.h |  1 +
> > > drivers/net/ixgbe/base/ixgbe_x550.c | 12 ++++++++++-
> > >  drivers/net/ixgbe/ixgbe_ethdev.c    | 33
> > +++++++++++++++++++++++++++++
> > >  drivers/net/ixgbe/ixgbe_ethdev.h    |  3 +++
> > >  5 files changed, 64 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> > > index
> > > 82fa453fa28e..5db63083eef8 100644
> > > --- a/doc/guides/nics/ixgbe.rst
> > > +++ b/doc/guides/nics/ixgbe.rst
> > > @@ -101,6 +101,22 @@ To guarantee the constraint, capabilities in
> > > dev_conf.rxmode.offloads will be ch
> > >
> > >  fdir_conf->mode will also be checked.
> > >
> > > +Runtime Options
> > > +^^^^^^^^^^^^^^^^^^
> > > +
> > > +The following ``devargs`` options can be enabled at runtime. They
> > > +must be passed as part of EAL arguments. For example,
> > > +
> > > +.. code-block:: console
> > > +
> > > +   dpdk-testpmd -a af:10.0,cu_sfp_as_sx=1 -- -i
> > > +
> > > +- ``cu_sfp_as_sx`` (default **0**)
> >
> > Can we make this devargs more generic e.g.: "allow_unsupported_phy"
> > So we don't need to add a devarg for similar requirement case by case
> > in future, of cause we still need to well explain all the unsupported
> > cases in the document.
> >
> >
> 
> this patch is specifically to change the driver's recognition of Cu transceivers
> and treat them as optical transceivers.  so should we consider this an
> unsupported phy and use that same switch 'allow_unsupported_phy' or are
> you looking for a more generic name than 'cu_sfp_as_sx'?  if you are looking
> for a more generic name vs just reusing allow_unsupported_phy, then please
> pick something and I'll submit a new patch, but I don't want to guess what
> would be ok by submitting patches.
> 

I'm not sure if there will be a situation we need to enable a unsupported phys  in a different way,
But as kernel driver take allow_unsupported_spf as module_param, so I will prefer we keep the same in DPDK.