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

[Jeff Daly <jeffd@silicom-usa.com>]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Monday, May 30, 2022 9:51 AM
> 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: 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.
> 

edit: we should have been saying 'allow_unsupported_sfp' all along.  that's the
kernel option. hopefully that didn't confuse anyone reading this thread......

so you realize that DPDK by default *always* sets allow_unsupported_sfp.  are
you now suggesting that this patch functionality falls under the same option?
meaning it will *always* be treating 1G Cu as 1G SX?