DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
@ 2022-03-07 22:34 jeffd
  2022-04-13 14:21 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: jeffd @ 2022-03-07 22:34 UTC (permalink / raw)
  To: dev; +Cc: Stephen Douthit, Jeff Daly, Haiyue Wang

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;
 	}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-03-07 22:34 [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices jeffd
@ 2022-04-13 14:21 ` Thomas Monjalon
  2022-05-20  0:14   ` Zhang, Qi Z
  2022-04-14  1:31 ` Wang, Haiyue
  2022-05-26 20:43 ` [PATCH v2] " Jeff Daly
  2 siblings, 1 reply; 25+ messages in thread
From: Thomas Monjalon @ 2022-04-13 14:21 UTC (permalink / raw)
  To: dev; +Cc: Stephen Douthit, Jeff Daly, Haiyue Wang, Qiming Yang, Wenjun Wu

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;
>  	}







^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-03-07 22:34 [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices jeffd
  2022-04-13 14:21 ` Thomas Monjalon
@ 2022-04-14  1:31 ` Wang, Haiyue
  2022-04-14  9:42   ` Thomas Monjalon
  2022-05-26 20:43 ` [PATCH v2] " Jeff Daly
  2 siblings, 1 reply; 25+ messages in thread
From: Wang, Haiyue @ 2022-04-14  1:31 UTC (permalink / raw)
  To: Daly, Jeff, dev; +Cc: Stephen Douthit, Daly, Jeff

> -----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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14  1:31 ` Wang, Haiyue
@ 2022-04-14  9:42   ` Thomas Monjalon
  2022-04-14 12:13     ` Wang, Haiyue
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Monjalon @ 2022-04-14  9:42 UTC (permalink / raw)
  To: Wang, Haiyue; +Cc: Daly, Jeff, dev, Stephen Douthit

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.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14  9:42   ` Thomas Monjalon
@ 2022-04-14 12:13     ` Wang, Haiyue
  2022-04-14 12:18       ` Thomas Monjalon
  0 siblings, 1 reply; 25+ messages in thread
From: Wang, Haiyue @ 2022-04-14 12:13 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Daly, Jeff, dev, Stephen Douthit

> -----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 ...

> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 12:13     ` Wang, Haiyue
@ 2022-04-14 12:18       ` Thomas Monjalon
  2022-04-14 15:11         ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Monjalon @ 2022-04-14 12:18 UTC (permalink / raw)
  To: Wang, Haiyue; +Cc: Daly, Jeff, dev, Stephen Douthit, qi.z.zhang, john.mcnamara

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?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 12:18       ` Thomas Monjalon
@ 2022-04-14 15:11         ` Jeff Daly
  2022-04-14 15:43           ` Stephen Hemminger
  2022-04-15  1:10           ` Wang, Haiyue
  0 siblings, 2 replies; 25+ messages in thread
From: Jeff Daly @ 2022-04-14 15:11 UTC (permalink / raw)
  To: Thomas Monjalon, Wang, Haiyue; +Cc: dev, qi.z.zhang, john.mcnamara



> -----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?



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 15:11         ` Jeff Daly
@ 2022-04-14 15:43           ` Stephen Hemminger
  2022-04-14 17:06             ` Thomas Monjalon
  2022-04-15  1:10           ` Wang, Haiyue
  1 sibling, 1 reply; 25+ messages in thread
From: Stephen Hemminger @ 2022-04-14 15:43 UTC (permalink / raw)
  To: Jeff Daly; +Cc: Thomas Monjalon, Wang, Haiyue, dev, qi.z.zhang, john.mcnamara

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.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 15:43           ` Stephen Hemminger
@ 2022-04-14 17:06             ` Thomas Monjalon
  2022-04-19  9:11               ` Morten Brørup
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Monjalon @ 2022-04-14 17:06 UTC (permalink / raw)
  To: Jeff Daly, Stephen Hemminger; +Cc: Wang, Haiyue, dev, qi.z.zhang, john.mcnamara

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.




^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 15:11         ` Jeff Daly
  2022-04-14 15:43           ` Stephen Hemminger
@ 2022-04-15  1:10           ` Wang, Haiyue
  1 sibling, 0 replies; 25+ messages in thread
From: Wang, Haiyue @ 2022-04-15  1:10 UTC (permalink / raw)
  To: Daly, Jeff, Thomas Monjalon; +Cc: dev, Zhang, Qi Z, Mcnamara, John

> -----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/

> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-14 17:06             ` Thomas Monjalon
@ 2022-04-19  9:11               ` Morten Brørup
  2022-04-19 12:32                 ` Wang, Haiyue
  0 siblings, 1 reply; 25+ messages in thread
From: Morten Brørup @ 2022-04-19  9:11 UTC (permalink / raw)
  To: Thomas Monjalon, Jeff Daly, Stephen Hemminger
  Cc: Wang, Haiyue, dev, qi.z.zhang, john.mcnamara

> 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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-19  9:11               ` Morten Brørup
@ 2022-04-19 12:32                 ` Wang, Haiyue
  0 siblings, 0 replies; 25+ messages in thread
From: Wang, Haiyue @ 2022-04-19 12:32 UTC (permalink / raw)
  To: Morten Brørup, Thomas Monjalon, Daly, Jeff, Stephen Hemminger
  Cc: dev, Zhang, Qi Z, Mcnamara, John, Yang, Qiming

> -----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


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-04-13 14:21 ` Thomas Monjalon
@ 2022-05-20  0:14   ` Zhang, Qi Z
  2022-05-20 18:02     ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-20  0:14 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Stephen Douthit, Daly, Jeff, Yang, Qiming, Wu, Wenjun1



> -----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;
> >  	}
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-20  0:14   ` Zhang, Qi Z
@ 2022-05-20 18:02     ` Jeff Daly
  2022-05-23  5:36       ` Zhang, Qi Z
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-20 18:02 UTC (permalink / raw)
  To: Zhang, Qi Z, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > >     }
> >
> >
> >
> >
> >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-20 18:02     ` Jeff Daly
@ 2022-05-23  5:36       ` Zhang, Qi Z
  2022-05-23 14:13         ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-23  5:36 UTC (permalink / raw)
  To: Daly, Jeff, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > > >     }
> > >
> > >
> > >
> > >
> > >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-23  5:36       ` Zhang, Qi Z
@ 2022-05-23 14:13         ` Jeff Daly
  2022-05-23 23:22           ` Zhang, Qi Z
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-23 14:13 UTC (permalink / raw)
  To: Zhang, Qi Z, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > > > >     }
> > > >
> > > >
> > > >
> > > >
> > > >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-23 14:13         ` Jeff Daly
@ 2022-05-23 23:22           ` Zhang, Qi Z
  2022-05-25 15:23             ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-23 23:22 UTC (permalink / raw)
  To: Daly, Jeff, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > > > > >     }
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-23 23:22           ` Zhang, Qi Z
@ 2022-05-25 15:23             ` Jeff Daly
  2022-05-26  0:28               ` Zhang, Qi Z
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-25 15:23 UTC (permalink / raw)
  To: Zhang, Qi Z, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > > > > > >     }
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-25 15:23             ` Jeff Daly
@ 2022-05-26  0:28               ` Zhang, Qi Z
  0 siblings, 0 replies; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-26  0:28 UTC (permalink / raw)
  To: Daly, Jeff, Thomas Monjalon, dev
  Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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;
> > > > > > > >     }
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-03-07 22:34 [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices jeffd
  2022-04-13 14:21 ` Thomas Monjalon
  2022-04-14  1:31 ` Wang, Haiyue
@ 2022-05-26 20:43 ` Jeff Daly
  2022-05-29 22:49   ` Zhang, Qi Z
  2 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-26 20:43 UTC (permalink / raw)
  To: dev; +Cc: Stephen Douthit, Qiming Yang, Wenjun Wu

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**)
+
+  This option is used to allow the X550 devices to treat 1G Cu SFPs as
+  1G SX SFPs, since this usually works.  By default, 1G Cu SFPs are not
+  supported by Intel for the X550.
+
 VF Runtime Options
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/drivers/net/ixgbe/base/ixgbe_type.h b/drivers/net/ixgbe/base/ixgbe_type.h
index b7eec456358d..36d741dbfaad 100644
--- a/drivers/net/ixgbe/base/ixgbe_type.h
+++ b/drivers/net/ixgbe/base/ixgbe_type.h
@@ -4190,6 +4190,7 @@ struct ixgbe_hw {
 	bool allow_unsupported_sfp;
 	bool wol_enabled;
 	bool need_crosstalk_fix;
+	bool cu_sfp_as_sx;
 };
 
 #define ixgbe_call_func(hw, func, params, error) \
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index 8810d1658e91..7211b8c707d8 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1538,9 +1538,19 @@ 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:
 	case ixgbe_sfp_type_1g_cu_core0:
+		if (hw->cu_sfp_as_sx == 1) {
+			EWARN(hw, "WARNING: Treating Cu SFP modules as SX modules is unsupported by Intel and may cause unstable operation or damage to the module or the adapter.  Intel Corporation is not responsible for any harm caused by using Cu modules in this way with this adapter.\n");
+			*linear = false;
+			hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core0;
+		}
 	case ixgbe_sfp_type_1g_cu_core1:
+		if (hw->cu_sfp_as_sx == 1) {
+			EWARN(hw, "WARNING: Treating Cu SFP modules as SX modules is unsupported by Intel and may cause unstable operation or damage to the module or the adapter.  Intel Corporation is not responsible for any harm caused by using Cu modules in this way with this adapter.\n");
+			*linear = false;
+			hw->phy.sfp_type = ixgbe_sfp_type_1g_sx_core1;
+		}
+	case ixgbe_sfp_type_unknown:
 	default:
 		return IXGBE_ERR_SFP_NOT_SUPPORTED;
 	}
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 2da3f67bbc78..419342f34132 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -129,12 +129,18 @@
 #define IXGBE_DMATXCTL_VT_MASK                 0xFFFF0000
 
 #define IXGBEVF_DEVARG_PFLINK_FULLCHK		"pflink_fullchk"
+#define IXGBE_DEVARG_CU_SFP_AS_SX		"cu_sfp_as_sx"
 
 static const char * const ixgbevf_valid_arguments[] = {
 	IXGBEVF_DEVARG_PFLINK_FULLCHK,
 	NULL
 };
 
+static const char * const ixgbe_valid_arguments[] = {
+	IXGBE_DEVARG_CU_SFP_AS_SX,
+	NULL
+};
+
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params);
 static int eth_ixgbe_dev_uninit(struct rte_eth_dev *eth_dev);
 static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev);
@@ -185,6 +191,8 @@ static int ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
 static int ixgbe_dev_info_get(struct rte_eth_dev *dev,
 			      struct rte_eth_dev_info *dev_info);
 static const uint32_t *ixgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+static int devarg_handle_int(const char *key, const char *value,
+			     void *extra_args);
 static int ixgbevf_dev_info_get(struct rte_eth_dev *dev,
 				struct rte_eth_dev_info *dev_info);
 static int ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
@@ -1032,6 +1040,29 @@ ixgbe_swfw_lock_reset(struct ixgbe_hw *hw)
 	ixgbe_release_swfw_semaphore(hw, mask);
 }
 
+static void
+ixgbe_parse_devargs(struct ixgbe_adapter *adapter,
+		    struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist;
+	uint16_t cu_sfp_as_sx;
+
+	if (devargs == NULL)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, ixgbe_valid_arguments);
+	if (kvlist == NULL)
+		return;
+
+	if (rte_kvargs_count(kvlist, IXGBE_DEVARG_CU_SFP_AS_SX) == 1 &&
+	    rte_kvargs_process(kvlist, IXGBE_DEVARG_CU_SFP_AS_SX,
+			       devarg_handle_int, &cu_sfp_as_sx) == 0 &&
+	    cu_sfp_as_sx == 1)
+		adapter->cu_sfp_as_sx = 1;
+
+	rte_kvargs_free(kvlist);
+}
+
 /*
  * This function is based on code in ixgbe_attach() in base/ixgbe.c.
  * It returns 0 on success.
@@ -1095,6 +1126,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	}
 
 	rte_atomic32_clear(&ad->link_thread_running);
+	ixgbe_parse_devargs(eth_dev->data->dev_private,
+			    pci_dev->device.devargs);
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 	eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS;
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 69e0e82a5b1a..c29e14c120b0 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -498,6 +498,9 @@ struct ixgbe_adapter {
 	struct rte_timecounter      tx_tstamp_tc;
  	struct ixgbe_tm_conf        tm_conf;
 
+	/* For treating CU SFPs as SX (Unsupported by Intel) */
+	uint8_t cu_sfp_as_sx;
+
 	/* For RSS reta table update */
 	uint8_t rss_reta_updated;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-26 20:43 ` [PATCH v2] " Jeff Daly
@ 2022-05-29 22:49   ` Zhang, Qi Z
  2022-05-30 13:32     ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-29 22:49 UTC (permalink / raw)
  To: Daly, Jeff, dev; +Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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. 




^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-29 22:49   ` Zhang, Qi Z
@ 2022-05-30 13:32     ` Jeff Daly
  2022-05-30 13:50       ` Zhang, Qi Z
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-30 13:32 UTC (permalink / raw)
  To: Zhang, Qi Z, dev; +Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-30 13:32     ` Jeff Daly
@ 2022-05-30 13:50       ` Zhang, Qi Z
  2022-05-31 12:30         ` Jeff Daly
  0 siblings, 1 reply; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-30 13:50 UTC (permalink / raw)
  To: Daly, Jeff, dev; +Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-30 13:50       ` Zhang, Qi Z
@ 2022-05-31 12:30         ` Jeff Daly
  2022-05-31 13:38           ` Zhang, Qi Z
  0 siblings, 1 reply; 25+ messages in thread
From: Jeff Daly @ 2022-05-31 12:30 UTC (permalink / raw)
  To: Zhang, Qi Z, dev; +Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----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?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices
  2022-05-31 12:30         ` Jeff Daly
@ 2022-05-31 13:38           ` Zhang, Qi Z
  0 siblings, 0 replies; 25+ messages in thread
From: Zhang, Qi Z @ 2022-05-31 13:38 UTC (permalink / raw)
  To: Daly, Jeff, dev; +Cc: Stephen Douthit, Yang, Qiming, Wu, Wenjun1



> -----Original Message-----
> From: Jeff Daly <jeffd@silicom-usa.com>
> Sent: Tuesday, May 31, 2022 8:31 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: 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. 

No I didn't realize this

But now I realize allow_unsupported_sfp is hardcoded as 1.

The original implementation is wrapped by a compile option.

+#ifdef RTE_LIBRTE_IXGBE_ALLOW_UNSUPPORTED_SFP
+       hw->allow_unsupported_sfp = 1;
+#endif

I don't know why we decide to keep this feature already be on when we remove the #ifdef.  
It could be implemented as a devarg just like kernel driver does.

So can we take this chance to sync with kernel driver?  and the case of treating 1G Cu as 1G SX could be covered into this config.

> are
> you now suggesting that this patch functionality falls under the same option?
> meaning it will *always* be treating 1G Cu as 1G SX?




^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-05-31 13:38 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 22:34 [PATCH] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices jeffd
2022-04-13 14:21 ` Thomas Monjalon
2022-05-20  0:14   ` Zhang, Qi Z
2022-05-20 18:02     ` Jeff Daly
2022-05-23  5:36       ` Zhang, Qi Z
2022-05-23 14:13         ` Jeff Daly
2022-05-23 23:22           ` Zhang, Qi Z
2022-05-25 15:23             ` Jeff Daly
2022-05-26  0:28               ` Zhang, Qi Z
2022-04-14  1:31 ` Wang, Haiyue
2022-04-14  9:42   ` Thomas Monjalon
2022-04-14 12:13     ` Wang, Haiyue
2022-04-14 12:18       ` Thomas Monjalon
2022-04-14 15:11         ` Jeff Daly
2022-04-14 15:43           ` Stephen Hemminger
2022-04-14 17:06             ` Thomas Monjalon
2022-04-19  9:11               ` Morten Brørup
2022-04-19 12:32                 ` Wang, Haiyue
2022-04-15  1:10           ` Wang, Haiyue
2022-05-26 20:43 ` [PATCH v2] " Jeff Daly
2022-05-29 22:49   ` Zhang, Qi Z
2022-05-30 13:32     ` Jeff Daly
2022-05-30 13:50       ` Zhang, Qi Z
2022-05-31 12:30         ` Jeff Daly
2022-05-31 13:38           ` Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).