net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
Checks
Commit Message
From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
---
drivers/net/ixgbe/base/ixgbe_x550.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
Comments
Please, could we have a review of this patch?
+Cc new ixgbe maintainers
07/03/2022 23:34, jeffd@silicom-usa.com:
> From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> Signed-off-by: Jeff Daly <jeffd@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)
> case ixgbe_sfp_type_1g_lha_core1:
> *linear = false;
> break;
> - case ixgbe_sfp_type_unknown:
> + /* Copper SFPs are not officially supported for x550em devices, but can
> + * often be made to work at fixed 1G speeds. Pretend they're 1g_sx
> + * modules here to allow g.Fast DSL SFPs to work.
> + */
> case ixgbe_sfp_type_1g_cu_core0:
> + EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
> + *linear = false;
> + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> + break;
> case ixgbe_sfp_type_1g_cu_core1:
> + EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
> + *linear = false;
> + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> + break;
> + case ixgbe_sfp_type_unknown:
> default:
> return IXGBE_ERR_SFP_NOT_SUPPORTED;
> }
> -----Original Message-----
> From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> Sent: Tuesday, March 8, 2022 06:35
> To: dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; Daly, Jeff <jeffd@silicom-usa.com>; Wang, Haiyue
> <haiyue.wang@intel.com>
> Subject: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
>
> From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> Signed-off-by: Jeff Daly <jeffd@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.
And the DPDK keeps the same design with kernel.
> --
> 2.25.1
14/04/2022 03:31, Wang, Haiyue:
> From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > Signed-off-by: Jeff Daly <jeffd@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.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, April 14, 2022 17:42
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: Daly, Jeff <jeffd@silicom-usa.com>; dev@dpdk.org; Stephen Douthit <stephend@silicom-usa.com>
> Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
>
> 14/04/2022 03:31, Wang, Haiyue:
> > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > Signed-off-by: Jeff Daly <jeffd@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 ...
>
14/04/2022 14:13, Wang, Haiyue:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 14/04/2022 03:31, Wang, Haiyue:
> > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > Signed-off-by: Jeff Daly <jeffd@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?
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, April 14, 2022 8:19 AM
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org; Stephen Douthit
> <stephend@silicom-usa.com>; qi.z.zhang@intel.com;
> john.mcnamara@intel.com
> Subject: Re: [PATCH] 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.
>
>
> 14/04/2022 14:13, Wang, Haiyue:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 14/04/2022 03:31, Wang, Haiyue:
> > > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > Signed-off-by: Jeff Daly <jeffd@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?
On Thu, 14 Apr 2022 15:11:46 +0000
Jeff Daly <jeffd@silicom-usa.com> wrote:
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, April 14, 2022 8:19 AM
> > To: Wang, Haiyue <haiyue.wang@intel.com>
> > Cc: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org; Stephen Douthit
> > <stephend@silicom-usa.com>; qi.z.zhang@intel.com;
> > john.mcnamara@intel.com
> > Subject: Re: [PATCH] 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.
> >
> >
> > 14/04/2022 14:13, Wang, Haiyue:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 14/04/2022 03:31, Wang, Haiyue:
> > > > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > Signed-off-by: Jeff Daly <jeffd@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
- should print message that when enabled the driver is no longer supported.
use at your own risk.
14/04/2022 17:43, Stephen Hemminger:
> On Thu, 14 Apr 2022 15:11:46 +0000
> Jeff Daly <jeffd@silicom-usa.com> wrote:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 14/04/2022 14:13, Wang, Haiyue:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 14/04/2022 03:31, Wang, Haiyue:
> > > > > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > > Signed-off-by: Jeff Daly <jeffd@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
> - should print message that when enabled the driver is no longer supported.
It could be supported by Silicom.
> use at your own risk.
> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Thursday, April 14, 2022 23:12
> To: Thomas Monjalon <thomas@monjalon.net>; Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Mcnamara, John <john.mcnamara@intel.com>
> Subject: RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
>
>
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Thursday, April 14, 2022 8:19 AM
> > To: Wang, Haiyue <haiyue.wang@intel.com>
> > Cc: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org; Stephen Douthit
> > <stephend@silicom-usa.com>; qi.z.zhang@intel.com;
> > john.mcnamara@intel.com
> > Subject: Re: [PATCH] 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.
> >
> >
> > 14/04/2022 14:13, Wang, Haiyue:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 14/04/2022 03:31, Wang, Haiyue:
> > > > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > Signed-off-by: Jeff Daly <jeffd@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?
Correct, let's get more reviews in IWL.
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
>
> From: Thomas Monjalon [mailto:thomas@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 <jeffd@silicom-usa.com> wrote:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 14/04/2022 14:13, Wang, Haiyue:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 14/04/2022 03:31, Wang, Haiyue:
> > > > > > > From: jeffd@silicom-usa.com <jeffd@silicom-usa.com>
> > > > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > > > Signed-off-by: Jeff Daly <jeffd@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
> -----Original Message-----
> From: Morten Brørup <mb@smartsharesystems.com>
> Sent: Tuesday, April 19, 2022 17:12
> To: Thomas Monjalon <thomas@monjalon.net>; Daly, Jeff <jeffd@silicom-usa.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Mcnamara,
> John <john.mcnamara@intel.com>
> Subject: RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
>
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Thursday, 14 April 2022 19.07
> >
> >
> > > - 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,
The first patch author Stephen D has left Silicom:
https://patchwork.dpdk.org/project/dpdk/patch/20211206221922.644187-8-stephend@silicom-usa.com/
How can you expect people can connect to Silicom always ? ; -)
> 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
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, April 13, 2022 10:21 PM
> To: dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly <jeffd@silicom-
> usa.com>; Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
>
> Please, could we have a review of this patch?
> +Cc new ixgbe maintainers
>
>
> 07/03/2022 23:34, jeffd@silicom-usa.com:
> > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
I think we need a devargs for this feature with well documentation
So, it should not break existing behavior by default, but allow people to take risk if they know what they are doing.
Thanks
Qi
> > ---
> > 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)
> > case ixgbe_sfp_type_1g_lha_core1:
> > *linear = false;
> > break;
> > - case ixgbe_sfp_type_unknown:
> > + /* Copper SFPs are not officially supported for x550em devices, but can
> > + * often be made to work at fixed 1G speeds. Pretend they're 1g_sx
> > + * modules here to allow g.Fast DSL SFPs to work.
> > + */
> > case ixgbe_sfp_type_1g_cu_core0:
> > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> 1g_sx\n");
> > + *linear = false;
> > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > + break;
> > case ixgbe_sfp_type_1g_cu_core1:
> > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> 1g_sx\n");
> > + *linear = false;
> > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > + break;
> > + case ixgbe_sfp_type_unknown:
> > default:
> > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > }
>
>
>
>
>
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Thursday, May 19, 2022 8:15 PM
> To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly <jeffd@silicom-
> usa.com>; Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>
> Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, April 13, 2022 10:21 PM
> > To: dev@dpdk.org
> > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > <jeffd@silicom- usa.com>; Wang, Haiyue <haiyue.wang@intel.com>; Yang,
> > Qiming <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550
> > devices
> >
> > Please, could we have a review of this patch?
> > +Cc new ixgbe maintainers
> >
> >
> > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
>
> I think we need a devargs for this feature with well documentation So, it
> should not break existing behavior by default, but allow people to take risk
> if they know what they are doing.
>
there was already a patch submitted to IWL mailing list for this feature in the base
driver, which was rejected.
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
> Thanks
> Qi
>
>
> > > ---
> > > 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)
> > > case ixgbe_sfp_type_1g_lha_core1:
> > > *linear = false;
> > > break;
> > > - case ixgbe_sfp_type_unknown:
> > > + /* Copper SFPs are not officially supported for x550em devices, but
> can
> > > + * often be made to work at fixed 1G speeds. Pretend they're 1g_sx
> > > + * modules here to allow g.Fast DSL SFPs to work.
> > > + */
> > > case ixgbe_sfp_type_1g_cu_core0:
> > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > 1g_sx\n");
> > > + *linear = false;
> > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > + break;
> > > case ixgbe_sfp_type_1g_cu_core1:
> > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > 1g_sx\n");
> > > + *linear = false;
> > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > + break;
> > > + case ixgbe_sfp_type_unknown:
> > > default:
> > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > }
> >
> >
> >
> >
> >
> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Saturday, May 21, 2022 2:03 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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] 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: Thursday, May 19, 2022 8:15 PM
> > To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > <jeffd@silicom- usa.com>; Yang, Qiming <qiming.yang@intel.com>; Wu,
> > Wenjun1 <wenjun1.wu@intel.com>
> > Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Wednesday, April 13, 2022 10:21 PM
> > > To: dev@dpdk.org
> > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > <jeffd@silicom- usa.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> > > Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > > <wenjun1.wu@intel.com>
> > > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the
> > > X550 devices
> > >
> > > Please, could we have a review of this patch?
> > > +Cc new ixgbe maintainers
> > >
> > >
> > > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> >
> > I think we need a devargs for this feature with well documentation So,
> > it should not break existing behavior by default, but allow people to
> > take risk if they know what they are doing.
> >
>
> there was already a patch submitted to IWL mailing list for this feature in the
> base driver, which was rejected.
> https://patchwork.ozlabs.org/project/intel-wired-
> lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
OK, thanks for sharing this,
But base on the concern of the previous comment
" 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."
We may have a risk to accept the code as default behavior
But devargs is allowed in DPDK for device-specific features.
>
> > Thanks
> > Qi
> >
> >
> > > > ---
> > > > 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)
> > > > case ixgbe_sfp_type_1g_lha_core1:
> > > > *linear = false;
> > > > break;
> > > > - case ixgbe_sfp_type_unknown:
> > > > + /* Copper SFPs are not officially supported for x550em
> > > > + devices, but
> > can
> > > > + * often be made to work at fixed 1G speeds. Pretend they're 1g_sx
> > > > + * modules here to allow g.Fast DSL SFPs to work.
> > > > + */
> > > > case ixgbe_sfp_type_1g_cu_core0:
> > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > > 1g_sx\n");
> > > > + *linear = false;
> > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > > + break;
> > > > case ixgbe_sfp_type_1g_cu_core1:
> > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > > 1g_sx\n");
> > > > + *linear = false;
> > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > > + break;
> > > > + case ixgbe_sfp_type_unknown:
> > > > default:
> > > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > }
> > >
> > >
> > >
> > >
> > >
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Monday, May 23, 2022 1:37 AM
> To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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] 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: Saturday, May 21, 2022 2:03 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; 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] 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: Thursday, May 19, 2022 8:15 PM
> > > To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > <jeffd@silicom- usa.com>; Yang, Qiming <qiming.yang@intel.com>; Wu,
> > > Wenjun1 <wenjun1.wu@intel.com>
> > > Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Wednesday, April 13, 2022 10:21 PM
> > > > To: dev@dpdk.org
> > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > <jeffd@silicom- usa.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> > > > Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > > > <wenjun1.wu@intel.com>
> > > > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the
> > > > X550 devices
> > > >
> > > > Please, could we have a review of this patch?
> > > > +Cc new ixgbe maintainers
> > > >
> > > >
> > > > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > >
> > > I think we need a devargs for this feature with well documentation
> > > So, it should not break existing behavior by default, but allow
> > > people to take risk if they know what they are doing.
> > >
> >
> > there was already a patch submitted to IWL mailing list for this
> > feature in the base driver, which was rejected.
> > https://patchwork.ozlabs.org/project/intel-wired-
> > lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
>
> OK, thanks for sharing this,
>
> But base on the concern of the previous comment
>
> " 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."
>
> We may have a risk to accept the code as default behavior
>
> But devargs is allowed in DPDK for device-specific features.
>
ok, I will submit a revised patch that uses a devargs (or whatever) switch to allow
the behavior when selected explicitly.
But, can we *please* STOP marking patches as superseded when a follow-up patch
hasn't been submitted yet!? I've marked the patch as 'Changes Requested' for now.
When I submit a follow-up I will set this one to superseded.
>
> >
> > > Thanks
> > > Qi
> > >
> > >
> > > > > ---
> > > > > 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)
> > > > > case ixgbe_sfp_type_1g_lha_core1:
> > > > > *linear = false;
> > > > > break;
> > > > > - case ixgbe_sfp_type_unknown:
> > > > > + /* Copper SFPs are not officially supported for x550em
> > > > > + devices, but
> > > can
> > > > > + * often be made to work at fixed 1G speeds. Pretend they're
> 1g_sx
> > > > > + * modules here to allow g.Fast DSL SFPs to work.
> > > > > + */
> > > > > case ixgbe_sfp_type_1g_cu_core0:
> > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > > > 1g_sx\n");
> > > > > + *linear = false;
> > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > > > + break;
> > > > > case ixgbe_sfp_type_1g_cu_core1:
> > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP is
> > > > 1g_sx\n");
> > > > > + *linear = false;
> > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > > > + break;
> > > > > + case ixgbe_sfp_type_unknown:
> > > > > default:
> > > > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > }
> > > >
> > > >
> > > >
> > > >
> > > >
> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Monday, May 23, 2022 10:14 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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] 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: Monday, May 23, 2022 1:37 AM
> > To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> > <thomas@monjalon.net>; 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] 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: Saturday, May 21, 2022 2:03 AM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; 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] 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: Thursday, May 19, 2022 8:15 PM
> > > > To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > <jeffd@silicom- usa.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > Wu,
> > > > Wenjun1 <wenjun1.wu@intel.com>
> > > > Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Wednesday, April 13, 2022 10:21 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > > <jeffd@silicom- usa.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> > > > > Yang, Qiming <qiming.yang@intel.com>; Wu, Wenjun1
> > > > > <wenjun1.wu@intel.com>
> > > > > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the
> > > > > X550 devices
> > > > >
> > > > > Please, could we have a review of this patch?
> > > > > +Cc new ixgbe maintainers
> > > > >
> > > > >
> > > > > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > > >
> > > > I think we need a devargs for this feature with well documentation
> > > > So, it should not break existing behavior by default, but allow
> > > > people to take risk if they know what they are doing.
> > > >
> > >
> > > there was already a patch submitted to IWL mailing list for this
> > > feature in the base driver, which was rejected.
> > > https://patchwork.ozlabs.org/project/intel-wired-
> > > lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
> >
> > OK, thanks for sharing this,
> >
> > But base on the concern of the previous comment
> >
> > " 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."
> >
> > We may have a risk to accept the code as default behavior
> >
> > But devargs is allowed in DPDK for device-specific features.
> >
>
> ok, I will submit a revised patch that uses a devargs (or whatever) switch to
> allow the behavior when selected explicitly.
>
> But, can we *please* STOP marking patches as superseded when a follow-up
> patch
> hasn't been submitted yet!? I've marked the patch as 'Changes Requested' for
> now.
Sure, I should follow, thanks to correct his, but a little bit surprise, why this looks like a big deal, it just a shortcut when I expected a new version will come then I skip one status change, I think mailing list already have everything about the patch status for you.
> When I submit a follow-up I will set this one to superseded
Actually you did NOT change the below patch to superseded after you send a new version (I did this) and you didn't reply my last question yet.
https://patchwork.dpdk.org/project/dpdk/list/?series=23046
>
> >
> > >
> > > > Thanks
> > > > Qi
> > > >
> > > >
> > > > > > ---
> > > > > > 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)
> > > > > > case ixgbe_sfp_type_1g_lha_core1:
> > > > > > *linear = false;
> > > > > > break;
> > > > > > - case ixgbe_sfp_type_unknown:
> > > > > > + /* Copper SFPs are not officially supported for x550em
> > > > > > + devices, but
> > > > can
> > > > > > + * often be made to work at fixed 1G speeds. Pretend
> > > > > > + they're
> > 1g_sx
> > > > > > + * modules here to allow g.Fast DSL SFPs to work.
> > > > > > + */
> > > > > > case ixgbe_sfp_type_1g_cu_core0:
> > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP
> > > > > > + is
> > > > > 1g_sx\n");
> > > > > > + *linear = false;
> > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > > > > + break;
> > > > > > case ixgbe_sfp_type_1g_cu_core1:
> > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP
> > > > > > + is
> > > > > 1g_sx\n");
> > > > > > + *linear = false;
> > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > > > > + break;
> > > > > > + case ixgbe_sfp_type_unknown:
> > > > > > default:
> > > > > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > > }
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Monday, May 23, 2022 7:22 PM
> To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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] 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 23, 2022 10:14 PM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; 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] 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: Monday, May 23, 2022 1:37 AM
> > > To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; 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] 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: Saturday, May 21, 2022 2:03 AM
> > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; 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] 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: Thursday, May 19, 2022 8:15 PM
> > > > > To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > > <jeffd@silicom- usa.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > > Wu,
> > > > > Wenjun1 <wenjun1.wu@intel.com>
> > > > > Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > > > > > Sent: Wednesday, April 13, 2022 10:21 PM
> > > > > > To: dev@dpdk.org
> > > > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > > > <jeffd@silicom- usa.com>; Wang, Haiyue
> > > > > > <haiyue.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > > > > > Wu, Wenjun1 <wenjun1.wu@intel.com>
> > > > > > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on
> > > > > > the
> > > > > > X550 devices
> > > > > >
> > > > > > Please, could we have a review of this patch?
> > > > > > +Cc new ixgbe maintainers
> > > > > >
> > > > > >
> > > > > > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > > > >
> > > > > I think we need a devargs for this feature with well
> > > > > documentation So, it should not break existing behavior by
> > > > > default, but allow people to take risk if they know what they are
> doing.
> > > > >
> > > >
> > > > there was already a patch submitted to IWL mailing list for this
> > > > feature in the base driver, which was rejected.
> > > > https://patchwork.ozlabs.org/project/intel-wired-
> > > > lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
> > >
> > > OK, thanks for sharing this,
> > >
> > > But base on the concern of the previous comment
> > >
> > > " 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."
> > >
> > > We may have a risk to accept the code as default behavior
> > >
> > > But devargs is allowed in DPDK for device-specific features.
> > >
> >
> > ok, I will submit a revised patch that uses a devargs (or whatever)
> > switch to allow the behavior when selected explicitly.
> >
> > But, can we *please* STOP marking patches as superseded when a
> > follow-up patch
> > hasn't been submitted yet!? I've marked the patch as 'Changes
> Requested' for
> > now.
>
> Sure, I should follow, thanks to correct his, but a little bit surprise, why this
> looks like a big deal, it just a shortcut when I expected a new version will
> come then I skip one status change, I think mailing list already have
> everything about the patch status for you.
>
Maybe I'm not understanding the terms being used then in the mailing list status.
If you expect a new version (that doesn't exist yet) then wouldn't this be more
aptly "Changes Requested" vs. "Superseded". Superseded implies there's a new
version that exists and this current one no longer applies. If I just came onto the
mailing list and read a patch that was marked "Superseded" and looked for the
new one and didn't find it, I'd be very confused. If I read a patch that was marked
"Changes Requested", I'd know that this was the last patch sent by the developer
and that there would be a follow-up to this one sometime.
> > When I submit a follow-up I will set this one to superseded
>
> Actually you did NOT change the below patch to superseded after you send
> a new version (I did this) and you didn't reply my last question yet.
> https://patchwork.dpdk.org/project/dpdk/list/?series=23046
>
>
why am I confused here? The patch you linked above is not related to this patch.
The patch series linked above is an update to the hotplug patches that were
requested prior. (I screwed up the initial new series submission I admit, and marked
those as superseded).
*This* patch however I've not submitted a new update for yet.
And I don't see where you asked a question ?
> >
> > >
> > > >
> > > > > Thanks
> > > > > Qi
> > > > >
> > > > >
> > > > > > > ---
> > > > > > > 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)
> > > > > > > case ixgbe_sfp_type_1g_lha_core1:
> > > > > > > *linear = false;
> > > > > > > break;
> > > > > > > - case ixgbe_sfp_type_unknown:
> > > > > > > + /* Copper SFPs are not officially supported for x550em
> > > > > > > + devices, but
> > > > > can
> > > > > > > + * often be made to work at fixed 1G speeds. Pretend
> > > > > > > + they're
> > > 1g_sx
> > > > > > > + * modules here to allow g.Fast DSL SFPs to work.
> > > > > > > + */
> > > > > > > case ixgbe_sfp_type_1g_cu_core0:
> > > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP
> > > > > > > + is
> > > > > > 1g_sx\n");
> > > > > > > + *linear = false;
> > > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > > > > > + break;
> > > > > > > case ixgbe_sfp_type_1g_cu_core1:
> > > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu SFP
> > > > > > > + is
> > > > > > 1g_sx\n");
> > > > > > > + *linear = false;
> > > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > > > > > + break;
> > > > > > > + case ixgbe_sfp_type_unknown:
> > > > > > > default:
> > > > > > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > > > }
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Wednesday, May 25, 2022 11:23 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; 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] 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: Monday, May 23, 2022 7:22 PM
> > To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> > <thomas@monjalon.net>; 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] 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 23, 2022 10:14 PM
> > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; 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] 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: Monday, May 23, 2022 1:37 AM
> > > > To: Jeff Daly <jeffd@silicom-usa.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; 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] 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: Saturday, May 21, 2022 2:03 AM
> > > > > To: Zhang, Qi Z <qi.z.zhang@intel.com>; Thomas Monjalon
> > > > > <thomas@monjalon.net>; 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] 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: Thursday, May 19, 2022 8:15 PM
> > > > > > To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org
> > > > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > > > <jeffd@silicom- usa.com>; Yang, Qiming
> > > > > > <qiming.yang@intel.com>; Wu,
> > > > > > Wenjun1 <wenjun1.wu@intel.com>
> > > > > > Subject: RE: [PATCH] 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: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > Sent: Wednesday, April 13, 2022 10:21 PM
> > > > > > > To: dev@dpdk.org
> > > > > > > Cc: Stephen Douthit <stephend@silicom-usa.com>; Jeff Daly
> > > > > > > <jeffd@silicom- usa.com>; Wang, Haiyue
> > > > > > > <haiyue.wang@intel.com>; Yang, Qiming
> > > > > > > <qiming.yang@intel.com>; Wu, Wenjun1
> <wenjun1.wu@intel.com>
> > > > > > > Subject: Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on
> > > > > > > the
> > > > > > > X550 devices
> > > > > > >
> > > > > > > Please, could we have a review of this patch?
> > > > > > > +Cc new ixgbe maintainers
> > > > > > >
> > > > > > >
> > > > > > > 07/03/2022 23:34, jeffd@silicom-usa.com:
> > > > > > > > From: Stephen Douthit <stephend@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 <stephend@silicom-usa.com>
> > > > > > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > > > > >
> > > > > > I think we need a devargs for this feature with well
> > > > > > documentation So, it should not break existing behavior by
> > > > > > default, but allow people to take risk if they know what they
> > > > > > are
> > doing.
> > > > > >
> > > > >
> > > > > there was already a patch submitted to IWL mailing list for this
> > > > > feature in the base driver, which was rejected.
> > > > > https://patchwork.ozlabs.org/project/intel-wired-
> > > > > lan/patch/20220414201329.27714-1-jeffd@silicom-usa.com/
> > > >
> > > > OK, thanks for sharing this,
> > > >
> > > > But base on the concern of the previous comment
> > > >
> > > > " 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."
> > > >
> > > > We may have a risk to accept the code as default behavior
> > > >
> > > > But devargs is allowed in DPDK for device-specific features.
> > > >
> > >
> > > ok, I will submit a revised patch that uses a devargs (or whatever)
> > > switch to allow the behavior when selected explicitly.
> > >
> > > But, can we *please* STOP marking patches as superseded when a
> > > follow-up patch
> > > hasn't been submitted yet!? I've marked the patch as 'Changes
> > Requested' for
> > > now.
> >
> > Sure, I should follow, thanks to correct his, but a little bit
> > surprise, why this looks like a big deal, it just a shortcut when I
> > expected a new version will come then I skip one status change, I
> > think mailing list already have everything about the patch status for you.
> >
>
> Maybe I'm not understanding the terms being used then in the mailing list
> status.
> If you expect a new version (that doesn't exist yet) then wouldn't this be
> more
> aptly "Changes Requested" vs. "Superseded". Superseded implies there's a
> new
> version that exists and this current one no longer applies. If I just came onto
> the mailing list and read a patch that was marked "Superseded" and looked
> for the new one and didn't find it, I'd be very confused. If I read a patch that
> was marked "Changes Requested", I'd know that this was the last patch sent
> by the developer and that there would be a follow-up to this one sometime.
OK, I just thought we already get a agreement in the mailing list for introducing a devargs , so I move ahead
But I understand your concern now.
>
> > > When I submit a follow-up I will set this one to superseded
> >
> > Actually you did NOT change the below patch to superseded after you
> > send a new version (I did this) and you didn't reply my last question yet.
> > https://patchwork.dpdk.org/project/dpdk/list/?series=23046
> >
> >
>
> why am I confused here? The patch you linked above is not related to this
> patch.
> The patch series linked above is an update to the hotplug patches that were
> requested prior. (I screwed up the initial new series submission I admit, and
> marked those as superseded).
>
> *This* patch however I've not submitted a new update for yet.
>
> And I don't see where you asked a question ?
Sorry, I put the wrong link, it should be below patch
http://patches.dpdk.org/project/dpdk/patch/20220412174220.31195-3-jeffd@silicom-usa.com/
When you submit the new version, you didn't supersede this, this give me confusion somehow, so I guess we are even now😊
btw, I saw you have replied the answer, so we are aligned on that thread also
>
> > >
> > > >
> > > > >
> > > > > > Thanks
> > > > > > Qi
> > > > > >
> > > > > >
> > > > > > > > ---
> > > > > > > > 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)
> > > > > > > > case ixgbe_sfp_type_1g_lha_core1:
> > > > > > > > *linear = false;
> > > > > > > > break;
> > > > > > > > - case ixgbe_sfp_type_unknown:
> > > > > > > > + /* Copper SFPs are not officially supported for x550em
> > > > > > > > + devices, but
> > > > > > can
> > > > > > > > + * often be made to work at fixed 1G speeds. Pretend
> > > > > > > > + they're
> > > > 1g_sx
> > > > > > > > + * modules here to allow g.Fast DSL SFPs to work.
> > > > > > > > + */
> > > > > > > > case ixgbe_sfp_type_1g_cu_core0:
> > > > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu
> > > > > > > > + SFP is
> > > > > > > 1g_sx\n");
> > > > > > > > + *linear = false;
> > > > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
> > > > > > > > + break;
> > > > > > > > case ixgbe_sfp_type_1g_cu_core1:
> > > > > > > > + EWARN(hw, "Pretending that unsupported 1g_cu
> > > > > > > > + SFP is
> > > > > > > 1g_sx\n");
> > > > > > > > + *linear = false;
> > > > > > > > + hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
> > > > > > > > + break;
> > > > > > > > + case ixgbe_sfp_type_unknown:
> > > > > > > > default:
> > > > > > > > return IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > > > > }
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
@@ -1538,9 +1538,21 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
case ixgbe_sfp_type_1g_lha_core1:
*linear = false;
break;
- case ixgbe_sfp_type_unknown:
+ /* Copper SFPs are not officially supported for x550em devices, but can
+ * often be made to work at fixed 1G speeds. Pretend they're 1g_sx
+ * modules here to allow g.Fast DSL SFPs to work.
+ */
case ixgbe_sfp_type_1g_cu_core0:
+ EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
+ *linear = false;
+ hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
+ break;
case ixgbe_sfp_type_1g_cu_core1:
+ EWARN(hw, "Pretending that unsupported 1g_cu SFP is 1g_sx\n");
+ *linear = false;
+ hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
+ break;
+ case ixgbe_sfp_type_unknown:
default:
return IXGBE_ERR_SFP_NOT_SUPPORTED;
}