diff mbox series

net/ixgbe: 10GBASE-T SFP+ copper support

Message ID 1556105458-91997-1-git-send-email-ido@cgstowernetworks.com (mailing list archive)
State Rejected, archived
Delegated to: xiaolong ye
Headers
Series net/ixgbe: 10GBASE-T SFP+ copper support |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Ido Goshen April 24, 2019, 11:31 a.m. UTC 
  From: Ido Goshen <ido@cgstowernetworks.com>

10BASE-T SFP+ copper transceivers become cheaper and popular
So far those were blocked by ixgbe as “unsupported”.
e.g.
	eth_ixgbe_dev_init(): Unsupported SFP+ Module
	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
	EAL: Requested device 0000:0a:00.0 cannot be used

This patch expands the usage of allow_unsupported_sfp to be more general
and makes ixgbe more tolerant to unknown SFPs

Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
---
 drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
 drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Ananyev, Konstantin April 24, 2019, 11:47 a.m. UTC | #1 
> 
> From: Ido Goshen <ido@cgstowernetworks.com>
> 
> 10BASE-T SFP+ copper transceivers become cheaper and popular
> So far those were blocked by ixgbe as “unsupported”.
> e.g.
> 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> 	EAL: Requested device 0000:0a:00.0 cannot be used
> 
> This patch expands the usage of allow_unsupported_sfp to be more general
> and makes ixgbe more tolerant to unknown SFPs


I don't think it is a good idea to change the base code to blindly allow unknown SFPs.
Again in eth_ixgbe_dev_init() we do set 
hw->allow_unsupported_sfp = 1;
so the function below will return success anyway,

> 
> Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> ---
>  drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
>  drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
> index dd118f9..ff96afc 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> @@ -1527,18 +1527,9 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
>  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
>  				status = IXGBE_SUCCESS;
>  			} else {
> -				if (hw->allow_unsupported_sfp == true) {
> -					EWARN(hw,
> -						"WARNING: Intel (R) Network Connections are quality tested using Intel (R) Ethernet
> Optics. "
> -						"Using untested modules is not supported and may cause unstable operation or damage
> to the module or the adapter. "
> -						"Intel Corporation is not responsible for any harm caused by using untested modules.\n");
> -					status = IXGBE_SUCCESS;
> -				} else {
> -					DEBUGOUT("SFP+ module not supported\n");
> -					hw->phy.type =
> +				hw->phy.type =
>  						ixgbe_phy_sfp_unsupported;
> -					status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> -				}
> +				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
>  			}
>  		} else {
>  			status = IXGBE_SUCCESS;
> @@ -1546,6 +1537,15 @@ s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
>  	}
> 
>  out:
> +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> +			hw->allow_unsupported_sfp) {
> +		PMD_INIT_LOG(WARNING,
> +				"WARNING: Intel (R) Network Connections are quality tested using Intel (R) Ethernet Optics. "
> +				"Using untested modules is not supported and may cause unstable operation or damage to the module or
> the adapter. "
> +				"Intel Corporation is not responsible for any harm caused by using untested modules.\n");
> +		hw->phy.type = ixgbe_phy_unknown;
> +		status = IXGBE_SUCCESS;
> +	}
>  	return status;
> 
>  err_read_i2c_eeprom:
> diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
> index a920a14..212d9a0 100644
> --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> @@ -1539,6 +1539,9 @@ STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
>  		*linear = false;
>  		break;
>  	case ixgbe_sfp_type_unknown:
> +		if (hw->allow_unsupported_sfp)
> +			return IXGBE_SUCCESS;
> +		/* fall through */
>  	case ixgbe_sfp_type_1g_cu_core0:
>  	case ixgbe_sfp_type_1g_cu_core1:
>  	default:
> --
> 1.9.1
Ido Goshen April 24, 2019, 9:25 p.m. UTC | #2 
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Wednesday, April 24, 2019 2:47 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> 
> >
> > From: Ido Goshen <ido@cgstowernetworks.com>
> >
> > 10BASE-T SFP+ copper transceivers become cheaper and popular So far
> > those were blocked by ixgbe as “unsupported”.
> > e.g.
> > 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > 	EAL: Requested device 0000:0a:00.0 cannot be used
> >
> > This patch expands the usage of allow_unsupported_sfp to be more
> > general and makes ixgbe more tolerant to unknown SFPs
> 
> 
> I don't think it is a good idea to change the base code to blindly allow
> unknown SFPs.
> Again in eth_ixgbe_dev_init() we do set
> hw->allow_unsupported_sfp = 1;
> so the function below will return success anyway,

what's the reason to not allow unknown SFPs?  
as is they are explicitly blocked and not working anyway, why not give them a chance?

More inputs 
1. i40e already does support it (I didn't go deep into it but it just seems less strict on hw_init)
2. even with ixgbe it can work, because unsupported is only checked by ixgbe_init_hw 
     so if the SFP is inserted after the app has started it does work
     kind of inconsistent

> 
> >
> > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
> > drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
> >  2 files changed, 14 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index dd118f9..ff96afc 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -1527,18 +1527,9 @@ s32 ixgbe_identify_sfp_module_generic(struct
> ixgbe_hw *hw)
> >  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
> >  				status = IXGBE_SUCCESS;
> >  			} else {
> > -				if (hw->allow_unsupported_sfp == true) {
> > -					EWARN(hw,
> > -						"WARNING: Intel (R)
> Network Connections are quality tested using Intel (R) Ethernet
> > Optics. "
> > -						"Using untested modules is
> not supported and may cause unstable operation or damage
> > to the module or the adapter. "
> > -						"Intel Corporation is not
> responsible for any harm caused by using untested modules.\n");
> > -					status = IXGBE_SUCCESS;
> > -				} else {
> > -					DEBUGOUT("SFP+ module not
> supported\n");
> > -					hw->phy.type =
> > +				hw->phy.type =
> >  						ixgbe_phy_sfp_unsupported;
> > -					status =
> IXGBE_ERR_SFP_NOT_SUPPORTED;
> > -				}
> > +				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> >  			}
> >  		} else {
> >  			status = IXGBE_SUCCESS;
> > @@ -1546,6 +1537,15 @@ s32 ixgbe_identify_sfp_module_generic(struct
> ixgbe_hw *hw)
> >  	}
> >
> >  out:
> > +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > +			hw->allow_unsupported_sfp) {
> > +		PMD_INIT_LOG(WARNING,
> > +				"WARNING: Intel (R) Network Connections
> are quality tested using Intel (R) Ethernet Optics. "
> > +				"Using untested modules is not supported
> and may cause unstable
> > +operation or damage to the module or
> > the adapter. "
> > +				"Intel Corporation is not responsible for any
> harm caused by using untested modules.\n");
> > +		hw->phy.type = ixgbe_phy_unknown;
> > +		status = IXGBE_SUCCESS;
> > +	}
> >  	return status;
> >
> >  err_read_i2c_eeprom:
> > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > index a920a14..212d9a0 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > @@ -1539,6 +1539,9 @@ STATIC s32
> ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
> >  		*linear = false;
> >  		break;
> >  	case ixgbe_sfp_type_unknown:
> > +		if (hw->allow_unsupported_sfp)
> > +			return IXGBE_SUCCESS;
> > +		/* fall through */
> >  	case ixgbe_sfp_type_1g_cu_core0:
> >  	case ixgbe_sfp_type_1g_cu_core1:
> >  	default:
> > --
> > 1.9.1
Ananyev, Konstantin April 26, 2019, 12:13 p.m. UTC | #3 
> > > From: Ido Goshen <ido@cgstowernetworks.com>
> > >
> > > 10BASE-T SFP+ copper transceivers become cheaper and popular So far
> > > those were blocked by ixgbe as “unsupported”.
> > > e.g.
> > > 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > > 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > > 	EAL: Requested device 0000:0a:00.0 cannot be used
> > >
> > > This patch expands the usage of allow_unsupported_sfp to be more
> > > general and makes ixgbe more tolerant to unknown SFPs
> >
> >
> > I don't think it is a good idea to change the base code to blindly allow
> > unknown SFPs.
> > Again in eth_ixgbe_dev_init() we do set
> > hw->allow_unsupported_sfp = 1;
> > so the function below will return success anyway,
> 
> what's the reason to not allow unknown SFPs?
> as is they are explicitly blocked and not working anyway, why not give them a chance?

From my perspective the question should be opposite: why to allow it?
ixgbe base code is developed and maintained by Intel ND team for several platforms.
It should be some good reason to change it inside DPDK project only.
As I said,  in eth_ixgbe_dev_init() we already set hw->allow_unsupported_sfp = 1,
so unknown spf should be allowed by DPDK ixgbe PMD.
So what exact problem you are trying to solve here?
Konstantin

> 
> More inputs
> 1. i40e already does support it (I didn't go deep into it but it just seems less strict on hw_init)
> 2. even with ixgbe it can work, because unsupported is only checked by ixgbe_init_hw
>      so if the SFP is inserted after the app has started it does work
>      kind of inconsistent
> 
> >
> > >
> > > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > > ---
> > >  drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
> > > drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
> > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > index dd118f9..ff96afc 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > @@ -1527,18 +1527,9 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> > >  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
> > >  				status = IXGBE_SUCCESS;
> > >  			} else {
> > > -				if (hw->allow_unsupported_sfp == true) {
> > > -					EWARN(hw,
> > > -						"WARNING: Intel (R)
> > Network Connections are quality tested using Intel (R) Ethernet
> > > Optics. "
> > > -						"Using untested modules is
> > not supported and may cause unstable operation or damage
> > > to the module or the adapter. "
> > > -						"Intel Corporation is not
> > responsible for any harm caused by using untested modules.\n");
> > > -					status = IXGBE_SUCCESS;
> > > -				} else {
> > > -					DEBUGOUT("SFP+ module not
> > supported\n");
> > > -					hw->phy.type =
> > > +				hw->phy.type =
> > >  						ixgbe_phy_sfp_unsupported;
> > > -					status =
> > IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > -				}
> > > +				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> > >  			}
> > >  		} else {
> > >  			status = IXGBE_SUCCESS;
> > > @@ -1546,6 +1537,15 @@ s32 ixgbe_identify_sfp_module_generic(struct
> > ixgbe_hw *hw)
> > >  	}
> > >
> > >  out:
> > > +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > > +			hw->allow_unsupported_sfp) {
> > > +		PMD_INIT_LOG(WARNING,
> > > +				"WARNING: Intel (R) Network Connections
> > are quality tested using Intel (R) Ethernet Optics. "
> > > +				"Using untested modules is not supported
> > and may cause unstable
> > > +operation or damage to the module or
> > > the adapter. "
> > > +				"Intel Corporation is not responsible for any
> > harm caused by using untested modules.\n");
> > > +		hw->phy.type = ixgbe_phy_unknown;
> > > +		status = IXGBE_SUCCESS;
> > > +	}
> > >  	return status;
> > >
> > >  err_read_i2c_eeprom:
> > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > index a920a14..212d9a0 100644
> > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > @@ -1539,6 +1539,9 @@ STATIC s32
> > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
> > >  		*linear = false;
> > >  		break;
> > >  	case ixgbe_sfp_type_unknown:
> > > +		if (hw->allow_unsupported_sfp)
> > > +			return IXGBE_SUCCESS;
> > > +		/* fall through */
> > >  	case ixgbe_sfp_type_1g_cu_core0:
> > >  	case ixgbe_sfp_type_1g_cu_core1:
> > >  	default:
> > > --
> > > 1.9.1
Ido Goshen April 28, 2019, 6:54 a.m. UTC | #4 
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Friday, April 26, 2019 3:13 PM
> To: Ido Goshen <Ido@cgstowernetworks.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> 
> > > > From: Ido Goshen <ido@cgstowernetworks.com>
> > > >
> > > > 10BASE-T SFP+ copper transceivers become cheaper and popular So
> > > > far those were blocked by ixgbe as “unsupported”.
> > > > e.g.
> > > > 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > > > 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > > > 	EAL: Requested device 0000:0a:00.0 cannot be used
> > > >
> > > > This patch expands the usage of allow_unsupported_sfp to be more
> > > > general and makes ixgbe more tolerant to unknown SFPs
> > >
> > >
> > > I don't think it is a good idea to change the base code to blindly
> > > allow unknown SFPs.
> > > Again in eth_ixgbe_dev_init() we do set
> > > hw->allow_unsupported_sfp = 1;
> > > so the function below will return success anyway,
> >
> > what's the reason to not allow unknown SFPs?
> > as is they are explicitly blocked and not working anyway, why not give
> them a chance?
> 
> From my perspective the question should be opposite: why to allow it?
> ixgbe base code is developed and maintained by Intel ND team for several
> platforms.
> It should be some good reason to change it inside DPDK project only.
> As I said,  in eth_ixgbe_dev_init() we already set hw->allow_unsupported_sfp
> = 1, so unknown spf should be allowed by DPDK ixgbe PMD.
> So what exact problem you are trying to solve here?
> Konstantin

The problem is that 10GBASE-T copper transceivers are not working just because they are unknown
http://www.eoptolink.com/products/copper-10g-sfp

The hw->allow_unsupported_sfp is used too late in
https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/base/ixgbe_phy.c#n1530
And if we've already got out earlier in
https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/base/ixgbe_phy.c#n1507
The device cannot be used
The patch tries to make the hw->allow_unsupported_sfp more general and in case it is set (always in dpdk)
change any return status of IXGBE_ERR_SFP_NOT_SUPPORTED to IXGBE_SUCCESS with ixgbe_phy_unknown

Other suggestions how to make 10GBASE-T copper work?

> 
> >
> > More inputs
> > 1. i40e already does support it (I didn't go deep into it but it just
> > seems less strict on hw_init) 2. even with ixgbe it can work, because
> unsupported is only checked by ixgbe_init_hw
> >      so if the SFP is inserted after the app has started it does work
> >      kind of inconsistent
> >
> > >
> > > >
> > > > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > > > ---
> > > >  drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
> > > > drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
> > > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > index dd118f9..ff96afc 100644
> > > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > @@ -1527,18 +1527,9 @@ s32
> > > > ixgbe_identify_sfp_module_generic(struct
> > > ixgbe_hw *hw)
> > > >  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
> > > >  				status = IXGBE_SUCCESS;
> > > >  			} else {
> > > > -				if (hw->allow_unsupported_sfp == true) {
> > > > -					EWARN(hw,
> > > > -						"WARNING: Intel (R)
> > > Network Connections are quality tested using Intel (R) Ethernet
> > > > Optics. "
> > > > -						"Using untested modules is
> > > not supported and may cause unstable operation or damage
> > > > to the module or the adapter. "
> > > > -						"Intel Corporation is not
> > > responsible for any harm caused by using untested modules.\n");
> > > > -					status = IXGBE_SUCCESS;
> > > > -				} else {
> > > > -					DEBUGOUT("SFP+ module not
> > > supported\n");
> > > > -					hw->phy.type =
> > > > +				hw->phy.type =
> > > >  						ixgbe_phy_sfp_unsupported;
> > > > -					status =
> > > IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > -				}
> > > > +				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > >  			}
> > > >  		} else {
> > > >  			status = IXGBE_SUCCESS;
> > > > @@ -1546,6 +1537,15 @@ s32
> > > > ixgbe_identify_sfp_module_generic(struct
> > > ixgbe_hw *hw)
> > > >  	}
> > > >
> > > >  out:
> > > > +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > > > +			hw->allow_unsupported_sfp) {
> > > > +		PMD_INIT_LOG(WARNING,
> > > > +				"WARNING: Intel (R) Network Connections
> > > are quality tested using Intel (R) Ethernet Optics. "
> > > > +				"Using untested modules is not supported
> > > and may cause unstable
> > > > +operation or damage to the module or
> > > > the adapter. "
> > > > +				"Intel Corporation is not responsible for any
> > > harm caused by using untested modules.\n");
> > > > +		hw->phy.type = ixgbe_phy_unknown;
> > > > +		status = IXGBE_SUCCESS;
> > > > +	}
> > > >  	return status;
> > > >
> > > >  err_read_i2c_eeprom:
> > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > index a920a14..212d9a0 100644
> > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > @@ -1539,6 +1539,9 @@ STATIC s32
> > > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool
> > > *linear)
> > > >  		*linear = false;
> > > >  		break;
> > > >  	case ixgbe_sfp_type_unknown:
> > > > +		if (hw->allow_unsupported_sfp)
> > > > +			return IXGBE_SUCCESS;
> > > > +		/* fall through */
> > > >  	case ixgbe_sfp_type_1g_cu_core0:
> > > >  	case ixgbe_sfp_type_1g_cu_core1:
> > > >  	default:
> > > > --
> > > > 1.9.1
Ananyev, Konstantin May 1, 2019, 11:40 p.m. UTC | #5 
> -----Original Message-----
> From: Ido Goshen [mailto:Ido@cgstowernetworks.com]
> Sent: Sunday, April 28, 2019 7:54 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Sent: Friday, April 26, 2019 3:13 PM
> > To: Ido Goshen <Ido@cgstowernetworks.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> >
> >
> >
> > > > > From: Ido Goshen <ido@cgstowernetworks.com>
> > > > >
> > > > > 10BASE-T SFP+ copper transceivers become cheaper and popular So
> > > > > far those were blocked by ixgbe as “unsupported”.
> > > > > e.g.
> > > > > 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > > > > 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > > > > 	EAL: Requested device 0000:0a:00.0 cannot be used
> > > > >
> > > > > This patch expands the usage of allow_unsupported_sfp to be more
> > > > > general and makes ixgbe more tolerant to unknown SFPs
> > > >
> > > >
> > > > I don't think it is a good idea to change the base code to blindly
> > > > allow unknown SFPs.
> > > > Again in eth_ixgbe_dev_init() we do set
> > > > hw->allow_unsupported_sfp = 1;
> > > > so the function below will return success anyway,
> > >
> > > what's the reason to not allow unknown SFPs?
> > > as is they are explicitly blocked and not working anyway, why not give
> > them a chance?
> >
> > From my perspective the question should be opposite: why to allow it?
> > ixgbe base code is developed and maintained by Intel ND team for several
> > platforms.
> > It should be some good reason to change it inside DPDK project only.
> > As I said,  in eth_ixgbe_dev_init() we already set hw->allow_unsupported_sfp
> > = 1, so unknown spf should be allowed by DPDK ixgbe PMD.
> > So what exact problem you are trying to solve here?
> > Konstantin
> 
> The problem is that 10GBASE-T copper transceivers are not working just because they are unknown
> http://www.eoptolink.com/products/copper-10g-sfp
> 
> The hw->allow_unsupported_sfp is used too late in
> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/base/ixgbe_phy.c#n1530
> And if we've already got out earlier in
> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/base/ixgbe_phy.c#n1507

As I can read the code that check is for 1G SFPs.
So if you getting out here, then comp_codes_10g == 0 here,
and it means that given SFP is not recognized as 10G one.
I wonder why that happens?

As I can see comp_codes_10g should be initialized at line 1314:
status = hw->phy.ops.read_i2c_eeprom(hw,
                                                     IXGBE_SFF_10GBE_COMP_CODES,
                                                     &comp_codes_10g);

> The device cannot be used
> The patch tries to make the hw->allow_unsupported_sfp more general and in case it is set (always in dpdk)
> change any return status of IXGBE_ERR_SFP_NOT_SUPPORTED to IXGBE_SUCCESS with ixgbe_phy_unknown
> 
> Other suggestions how to make 10GBASE-T copper work?
> 
> >
> > >
> > > More inputs
> > > 1. i40e already does support it (I didn't go deep into it but it just
> > > seems less strict on hw_init) 2. even with ixgbe it can work, because
> > unsupported is only checked by ixgbe_init_hw
> > >      so if the SFP is inserted after the app has started it does work
> > >      kind of inconsistent
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > > > > ---
> > > > >  drivers/net/ixgbe/base/ixgbe_phy.c  | 22 +++++++++++-----------
> > > > > drivers/net/ixgbe/base/ixgbe_x550.c |  3 +++
> > > > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > index dd118f9..ff96afc 100644
> > > > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > @@ -1527,18 +1527,9 @@ s32
> > > > > ixgbe_identify_sfp_module_generic(struct
> > > > ixgbe_hw *hw)
> > > > >  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
> > > > >  				status = IXGBE_SUCCESS;
> > > > >  			} else {
> > > > > -				if (hw->allow_unsupported_sfp == true) {
> > > > > -					EWARN(hw,
> > > > > -						"WARNING: Intel (R)
> > > > Network Connections are quality tested using Intel (R) Ethernet
> > > > > Optics. "
> > > > > -						"Using untested modules is
> > > > not supported and may cause unstable operation or damage
> > > > > to the module or the adapter. "
> > > > > -						"Intel Corporation is not
> > > > responsible for any harm caused by using untested modules.\n");
> > > > > -					status = IXGBE_SUCCESS;
> > > > > -				} else {
> > > > > -					DEBUGOUT("SFP+ module not
> > > > supported\n");
> > > > > -					hw->phy.type =
> > > > > +				hw->phy.type =
> > > > >  						ixgbe_phy_sfp_unsupported;
> > > > > -					status =
> > > > IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > -				}
> > > > > +				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > >  			}
> > > > >  		} else {
> > > > >  			status = IXGBE_SUCCESS;
> > > > > @@ -1546,6 +1537,15 @@ s32
> > > > > ixgbe_identify_sfp_module_generic(struct
> > > > ixgbe_hw *hw)
> > > > >  	}
> > > > >
> > > > >  out:
> > > > > +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > > > > +			hw->allow_unsupported_sfp) {
> > > > > +		PMD_INIT_LOG(WARNING,
> > > > > +				"WARNING: Intel (R) Network Connections
> > > > are quality tested using Intel (R) Ethernet Optics. "
> > > > > +				"Using untested modules is not supported
> > > > and may cause unstable
> > > > > +operation or damage to the module or
> > > > > the adapter. "
> > > > > +				"Intel Corporation is not responsible for any
> > > > harm caused by using untested modules.\n");
> > > > > +		hw->phy.type = ixgbe_phy_unknown;
> > > > > +		status = IXGBE_SUCCESS;
> > > > > +	}
> > > > >  	return status;
> > > > >
> > > > >  err_read_i2c_eeprom:
> > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > index a920a14..212d9a0 100644
> > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > @@ -1539,6 +1539,9 @@ STATIC s32
> > > > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool
> > > > *linear)
> > > > >  		*linear = false;
> > > > >  		break;
> > > > >  	case ixgbe_sfp_type_unknown:
> > > > > +		if (hw->allow_unsupported_sfp)
> > > > > +			return IXGBE_SUCCESS;
> > > > > +		/* fall through */
> > > > >  	case ixgbe_sfp_type_1g_cu_core0:
> > > > >  	case ixgbe_sfp_type_1g_cu_core1:
> > > > >  	default:
> > > > > --
> > > > > 1.9.1
Ido Goshen May 2, 2019, 9:15 a.m. UTC | #6 
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, May 2, 2019 2:41 AM
> To: Ido Goshen <Ido@cgstowernetworks.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> 
> > -----Original Message-----
> > From: Ido Goshen [mailto:Ido@cgstowernetworks.com]
> > Sent: Sunday, April 28, 2019 7:54 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Sent: Friday, April 26, 2019 3:13 PM
> > > To: Ido Goshen <Ido@cgstowernetworks.com>; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>
> > > Cc: dev@dpdk.org
> > > Subject: RE: [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> > >
> > >
> > >
> > > > > > From: Ido Goshen <ido@cgstowernetworks.com>
> > > > > >
> > > > > > 10BASE-T SFP+ copper transceivers become cheaper and popular
> > > > > > So far those were blocked by ixgbe as “unsupported”.
> > > > > > e.g.
> > > > > > 	eth_ixgbe_dev_init(): Unsupported SFP+ Module
> > > > > > 	eth_ixgbe_dev_init(): Hardware Initialization Failure: -19
> > > > > > 	EAL: Requested device 0000:0a:00.0 cannot be used
> > > > > >
> > > > > > This patch expands the usage of allow_unsupported_sfp to be
> > > > > > more general and makes ixgbe more tolerant to unknown SFPs
> > > > >
> > > > >
> > > > > I don't think it is a good idea to change the base code to
> > > > > blindly allow unknown SFPs.
> > > > > Again in eth_ixgbe_dev_init() we do set
> > > > > hw->allow_unsupported_sfp = 1;
> > > > > so the function below will return success anyway,
> > > >
> > > > what's the reason to not allow unknown SFPs?
> > > > as is they are explicitly blocked and not working anyway, why not
> > > > give
> > > them a chance?
> > >
> > > From my perspective the question should be opposite: why to allow it?
> > > ixgbe base code is developed and maintained by Intel ND team for
> > > several platforms.
> > > It should be some good reason to change it inside DPDK project only.
> > > As I said,  in eth_ixgbe_dev_init() we already set
> > > hw->allow_unsupported_sfp = 1, so unknown spf should be allowed by
> DPDK ixgbe PMD.
> > > So what exact problem you are trying to solve here?
> > > Konstantin
> >
> > The problem is that 10GBASE-T copper transceivers are not working just
> > because they are unknown
> > http://www.eoptolink.com/products/copper-10g-sfp
> >
> > The hw->allow_unsupported_sfp is used too late in
> > https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/b
> > ase/ixgbe_phy.c#n1530 And if we've already got out earlier in
> > https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/b
> > ase/ixgbe_phy.c#n1507
> 
> As I can read the code that check is for 1G SFPs.
> So if you getting out here, then comp_codes_10g == 0 here, and it means
> that given SFP is not recognized as 10G one.
> I wonder why that happens?
> 
> As I can see comp_codes_10g should be initialized at line 1314:
> status = hw->phy.ops.read_i2c_eeprom(hw,
>                                                      IXGBE_SFF_10GBE_COMP_CODES,
>                                                      &comp_codes_10g);
> 

The samples I have (from 2 vendors) read 0 from the eeprom IXGBE_SFF_10GBE_COMP_CODES offset

SFF-8472 spec [https://members.snia.org/document/dl/25916] doesn't define a code value for 10GBASE-T 
	TABLE 5-3 TRANSCEIVER COMPLIANCE CODES	
	10G Ethernet Compliance Codes
	3 	7 	10G Base-ER 	 
	3 	6 	10G Base-LRM 
	3 	5 	10G Base-LR 
	3 	4 	10G Base-SR 
	Infiniband Compliance Codes
	3 	3 	1X SX
	3 	2 	1X LX
	3 	1 	1X Copper Active
	3 	0 	1X Copper Active
Seems they are right not to set any code from above, no?
 
Do you know any 10GBASE-T SFPs that does work? 
Any idea what does it return for this field?

> > The device cannot be used
> > The patch tries to make the hw->allow_unsupported_sfp more general and
> > in case it is set (always in dpdk) change any return status of
> > IXGBE_ERR_SFP_NOT_SUPPORTED to IXGBE_SUCCESS with
> ixgbe_phy_unknown
> >
> > Other suggestions how to make 10GBASE-T copper work?
> >
> > >
> > > >
> > > > More inputs
> > > > 1. i40e already does support it (I didn't go deep into it but it
> > > > just seems less strict on hw_init) 2. even with ixgbe it can work,
> > > > because
> > > unsupported is only checked by ixgbe_init_hw
> > > >      so if the SFP is inserted after the app has started it does work
> > > >      kind of inconsistent
> > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Ido Goshen <ido@cgstowernetworks.com>
> > > > > > ---
> > > > > >  drivers/net/ixgbe/base/ixgbe_phy.c  | 22
> > > > > > +++++++++++----------- drivers/net/ixgbe/base/ixgbe_x550.c |
> > > > > > 3 +++
> > > > > >  2 files changed, 14 insertions(+), 11 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > > index dd118f9..ff96afc 100644
> > > > > > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > > > > > @@ -1527,18 +1527,9 @@ s32
> > > > > > ixgbe_identify_sfp_module_generic(struct
> > > > > ixgbe_hw *hw)
> > > > > >  			if (hw->phy.type == ixgbe_phy_sfp_intel) {
> > > > > >  				status = IXGBE_SUCCESS;
> > > > > >  			} else {
> > > > > > -				if (hw->allow_unsupported_sfp ==
> true) {
> > > > > > -					EWARN(hw,
> > > > > > -						"WARNING: Intel (R)
> > > > > Network Connections are quality tested using Intel (R) Ethernet
> > > > > > Optics. "
> > > > > > -						"Using untested
> modules is
> > > > > not supported and may cause unstable operation or damage
> > > > > > to the module or the adapter. "
> > > > > > -						"Intel Corporation is
> not
> > > > > responsible for any harm caused by using untested modules.\n");
> > > > > > -					status = IXGBE_SUCCESS;
> > > > > > -				} else {
> > > > > > -					DEBUGOUT("SFP+ module
> not
> > > > > supported\n");
> > > > > > -					hw->phy.type =
> > > > > > +				hw->phy.type =
> > > > > >
> 	ixgbe_phy_sfp_unsupported;
> > > > > > -					status =
> > > > > IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > > -				}
> > > > > > +				status =
> IXGBE_ERR_SFP_NOT_SUPPORTED;
> > > > > >  			}
> > > > > >  		} else {
> > > > > >  			status = IXGBE_SUCCESS;
> > > > > > @@ -1546,6 +1537,15 @@ s32
> > > > > > ixgbe_identify_sfp_module_generic(struct
> > > > > ixgbe_hw *hw)
> > > > > >  	}
> > > > > >
> > > > > >  out:
> > > > > > +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
> > > > > > +			hw->allow_unsupported_sfp) {
> > > > > > +		PMD_INIT_LOG(WARNING,
> > > > > > +				"WARNING: Intel (R) Network
> Connections
> > > > > are quality tested using Intel (R) Ethernet Optics. "
> > > > > > +				"Using untested modules is not
> supported
> > > > > and may cause unstable
> > > > > > +operation or damage to the module or
> > > > > > the adapter. "
> > > > > > +				"Intel Corporation is not responsible
> for any
> > > > > harm caused by using untested modules.\n");
> > > > > > +		hw->phy.type = ixgbe_phy_unknown;
> > > > > > +		status = IXGBE_SUCCESS;
> > > > > > +	}
> > > > > >  	return status;
> > > > > >
> > > > > >  err_read_i2c_eeprom:
> > > > > > diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > index a920a14..212d9a0 100644
> > > > > > --- a/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > +++ b/drivers/net/ixgbe/base/ixgbe_x550.c
> > > > > > @@ -1539,6 +1539,9 @@ STATIC s32
> > > > > ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool
> > > > > *linear)
> > > > > >  		*linear = false;
> > > > > >  		break;
> > > > > >  	case ixgbe_sfp_type_unknown:
> > > > > > +		if (hw->allow_unsupported_sfp)
> > > > > > +			return IXGBE_SUCCESS;
> > > > > > +		/* fall through */
> > > > > >  	case ixgbe_sfp_type_1g_cu_core0:
> > > > > >  	case ixgbe_sfp_type_1g_cu_core1:
> > > > > >  	default:
> > > > > > --
> > > > > > 1.9.1
Igor Russkikh May 6, 2019, 11:22 a.m. UTC | #7 
>>>
>>> The hw->allow_unsupported_sfp is used too late in
>>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/b
>>> ase/ixgbe_phy.c#n1530 And if we've already got out earlier in
>>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe/b
>>> ase/ixgbe_phy.c#n1507
>>
>> As I can read the code that check is for 1G SFPs.
>> So if you getting out here, then comp_codes_10g == 0 here, and it means
>> that given SFP is not recognized as 10G one.
>> I wonder why that happens?
>>
>> As I can see comp_codes_10g should be initialized at line 1314:
>> status = hw->phy.ops.read_i2c_eeprom(hw,
>>                                                      IXGBE_SFF_10GBE_COMP_CODES,
>>                                                      &comp_codes_10g);
>>
> 
> The samples I have (from 2 vendors) read 0 from the eeprom IXGBE_SFF_10GBE_COMP_CODES offset
> 
> SFF-8472 spec [https://members.snia.org/document/dl/25916] doesn't define a code value for 10GBASE-T 
> 	TABLE 5-3 TRANSCEIVER COMPLIANCE CODES	
> 	10G Ethernet Compliance Codes
> 	3 	7 	10G Base-ER 	 
> 	3 	6 	10G Base-LRM 
> 	3 	5 	10G Base-LR 
> 	3 	4 	10G Base-SR 
> 	Infiniband Compliance Codes
> 	3 	3 	1X SX
> 	3 	2 	1X LX
> 	3 	1 	1X Copper Active
> 	3 	0 	1X Copper Active
> Seems they are right not to set any code from above, no?
>  
> Do you know any 10GBASE-T SFPs that does work? 
> Any idea what does it return for this field?
> 

I can confirm we saw the same issues using Aquantia SFP+ Copper modules with 550
MAC. Indeed there is no separate id for Base-T.

ixgbe logic both in kernel and dpdk discards such modules.

Regards,
  Igor
Stillwell Jr, Paul M May 7, 2019, 10:45 p.m. UTC | #8 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> Sent: Monday, May 6, 2019 4:22 AM
> To: Ido Goshen <Ido@cgstowernetworks.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> >>>
> >>> The hw->allow_unsupported_sfp is used too late in
> >>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe
> >>> /b
> >>> ase/ixgbe_phy.c#n1530 And if we've already got out earlier in
> >>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixgbe
> >>> /b
> >>> ase/ixgbe_phy.c#n1507
> >>
> >> As I can read the code that check is for 1G SFPs.
> >> So if you getting out here, then comp_codes_10g == 0 here, and it
> >> means that given SFP is not recognized as 10G one.
> >> I wonder why that happens?
> >>
> >> As I can see comp_codes_10g should be initialized at line 1314:
> >> status = hw->phy.ops.read_i2c_eeprom(hw,
> >>                                                      IXGBE_SFF_10GBE_COMP_CODES,
> >>
> >> &comp_codes_10g);
> >>
> >
> > The samples I have (from 2 vendors) read 0 from the eeprom
> > IXGBE_SFF_10GBE_COMP_CODES offset
> >
> > SFF-8472 spec [https://members.snia.org/document/dl/25916] doesn't
> define a code value for 10GBASE-T
> > 	TABLE 5-3 TRANSCEIVER COMPLIANCE CODES
> > 	10G Ethernet Compliance Codes
> > 	3 	7 	10G Base-ER
> > 	3 	6 	10G Base-LRM
> > 	3 	5 	10G Base-LR
> > 	3 	4 	10G Base-SR
> > 	Infiniband Compliance Codes
> > 	3 	3 	1X SX
> > 	3 	2 	1X LX
> > 	3 	1 	1X Copper Active
> > 	3 	0 	1X Copper Active
> > Seems they are right not to set any code from above, no?
> >
> > Do you know any 10GBASE-T SFPs that does work?
> > Any idea what does it return for this field?
> >
> 
> I can confirm we saw the same issues using Aquantia SFP+ Copper modules
> with 550 MAC. Indeed there is no separate id for Base-T.
> 
> ixgbe logic both in kernel and dpdk discards such modules.
> 
> Regards,
>   Igor

The issue is really that there are bad modules out there and Intel only supports the ones that have been tested in our labs and verified to consistently work. We get too many support tickets for modules that don't work and so the best thing for us to do is to test some set of modules, verify they work, mark them as supported, and move on.

Feel free to apply this patch to your local repo and move on, but we can't support these modules in the code and will not accept a patch to support them.

Paul
Igor Russkikh May 14, 2019, 12:21 p.m. UTC | #9 
>>> 	TABLE 5-3 TRANSCEIVER COMPLIANCE CODES
>>> 	10G Ethernet Compliance Codes
>>> 	3 	7 	10G Base-ER
>>> 	3 	6 	10G Base-LRM
>>> 	3 	5 	10G Base-LR
>>> 	3 	4 	10G Base-SR
>>> 	Infiniband Compliance Codes
>>> 	3 	3 	1X SX
>>> 	3 	2 	1X LX
>>> 	3 	1 	1X Copper Active
>>> 	3 	0 	1X Copper Active
>>> Seems they are right not to set any code from above, no?
>>>
>>> Do you know any 10GBASE-T SFPs that does work?
>>> Any idea what does it return for this field?
>>>
>>
>> I can confirm we saw the same issues using Aquantia SFP+ Copper modules
>> with 550 MAC. Indeed there is no separate id for Base-T.
>>
>> ixgbe logic both in kernel and dpdk discards such modules.
>>
>> Regards,
>>   Igor
> 
> The issue is really that there are bad modules out there and Intel only supports the ones that have been tested in our labs and verified to consistently work. We get too many support tickets for modules that don't work and so the best thing for us to do is to test some set of modules, verify they work, mark them as supported, and move on.
> 
> Feel free to apply this patch to your local repo and move on, but we can't support these modules in the code and will not accept a patch to support them.

That's reasonable, but then all the rationality of `hw->allow_unsupported_sfp`
goes away, as you do not allow the particular type (copper) of unsupported sfps.

Regards,
  Igor
Ido Goshen May 14, 2019, 5:10 p.m. UTC | #10 
> -----Original Message-----
> From: Stillwell Jr, Paul M <paul.m.stillwell.jr@intel.com>
> Sent: Wednesday, May 8, 2019 1:46 AM
> To: Igor Russkikh <Igor.Russkikh@aquantia.com>; Ido Goshen
> <Ido@cgstowernetworks.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: 10GBASE-T SFP+ copper support
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> > Sent: Monday, May 6, 2019 4:22 AM
> > To: Ido Goshen <Ido@cgstowernetworks.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: 10GBASE-T SFP+ copper
> > support
> >
> >
> > >>>
> > >>> The hw->allow_unsupported_sfp is used too late in
> > >>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixg
> > >>> be
> > >>> /b
> > >>> ase/ixgbe_phy.c#n1530 And if we've already got out earlier in
> > >>> https://git.dpdk.org/next/dpdk-next-net-intel/tree/drivers/net/ixg
> > >>> be
> > >>> /b
> > >>> ase/ixgbe_phy.c#n1507
> > >>
> > >> As I can read the code that check is for 1G SFPs.
> > >> So if you getting out here, then comp_codes_10g == 0 here, and it
> > >> means that given SFP is not recognized as 10G one.
> > >> I wonder why that happens?
> > >>
> > >> As I can see comp_codes_10g should be initialized at line 1314:
> > >> status = hw->phy.ops.read_i2c_eeprom(hw,
> > >>
> > >> IXGBE_SFF_10GBE_COMP_CODES,
> > >>
> > >> &comp_codes_10g);
> > >>
> > >
> > > The samples I have (from 2 vendors) read 0 from the eeprom
> > > IXGBE_SFF_10GBE_COMP_CODES offset
> > >
> > > SFF-8472 spec [https://members.snia.org/document/dl/25916] doesn't
> > define a code value for 10GBASE-T
> > > 	TABLE 5-3 TRANSCEIVER COMPLIANCE CODES
> > > 	10G Ethernet Compliance Codes
> > > 	3 	7 	10G Base-ER
> > > 	3 	6 	10G Base-LRM
> > > 	3 	5 	10G Base-LR
> > > 	3 	4 	10G Base-SR
> > > 	Infiniband Compliance Codes
> > > 	3 	3 	1X SX
> > > 	3 	2 	1X LX
> > > 	3 	1 	1X Copper Active
> > > 	3 	0 	1X Copper Active
> > > Seems they are right not to set any code from above, no?
> > >
> > > Do you know any 10GBASE-T SFPs that does work?
> > > Any idea what does it return for this field?
> > >
> >
> > I can confirm we saw the same issues using Aquantia SFP+ Copper
> > modules with 550 MAC. Indeed there is no separate id for Base-T.
> >
> > ixgbe logic both in kernel and dpdk discards such modules.
> >
> > Regards,
> >   Igor
> 
> The issue is really that there are bad modules out there and Intel only
> supports the ones that have been tested in our labs and verified to
> consistently work. We get too many support tickets for modules that don't
> work and so the best thing for us to do is to test some set of modules, verify
> they work, mark them as supported, and move on.

Doesn't it contradict https://git.dpdk.org/next/dpdk-next-net-intel/commit/?id=aa1a048729808c4c1797649b3863b00c661e5ee4?
Which say "No need to restrict usage of non Intel SFP."

> 
> Feel free to apply this patch to your local repo and move on, but we can't
> support these modules in the code and will not accept a patch to support
> them.
> 
> Paul
diff mbox series

Patch

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index dd118f9..ff96afc 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -1527,18 +1527,9 @@  s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 			if (hw->phy.type == ixgbe_phy_sfp_intel) {
 				status = IXGBE_SUCCESS;
 			} else {
-				if (hw->allow_unsupported_sfp == true) {
-					EWARN(hw,
-						"WARNING: Intel (R) Network Connections are quality tested using Intel (R) Ethernet Optics. "
-						"Using untested modules is not supported and may cause unstable operation or damage to the module or the adapter. "
-						"Intel Corporation is not responsible for any harm caused by using untested modules.\n");
-					status = IXGBE_SUCCESS;
-				} else {
-					DEBUGOUT("SFP+ module not supported\n");
-					hw->phy.type =
+				hw->phy.type =
 						ixgbe_phy_sfp_unsupported;
-					status = IXGBE_ERR_SFP_NOT_SUPPORTED;
-				}
+				status = IXGBE_ERR_SFP_NOT_SUPPORTED;
 			}
 		} else {
 			status = IXGBE_SUCCESS;
@@ -1546,6 +1537,15 @@  s32 ixgbe_identify_sfp_module_generic(struct ixgbe_hw *hw)
 	}
 
 out:
+	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED &&
+			hw->allow_unsupported_sfp) {
+		PMD_INIT_LOG(WARNING,
+				"WARNING: Intel (R) Network Connections are quality tested using Intel (R) Ethernet Optics. "
+				"Using untested modules is not supported and may cause unstable operation or damage to the module or the adapter. "
+				"Intel Corporation is not responsible for any harm caused by using untested modules.\n");
+		hw->phy.type = ixgbe_phy_unknown;
+		status = IXGBE_SUCCESS;
+	}
 	return status;
 
 err_read_i2c_eeprom:
diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c
index a920a14..212d9a0 100644
--- a/drivers/net/ixgbe/base/ixgbe_x550.c
+++ b/drivers/net/ixgbe/base/ixgbe_x550.c
@@ -1539,6 +1539,9 @@  STATIC s32 ixgbe_supported_sfp_modules_X550em(struct ixgbe_hw *hw, bool *linear)
 		*linear = false;
 		break;
 	case ixgbe_sfp_type_unknown:
+		if (hw->allow_unsupported_sfp)
+			return IXGBE_SUCCESS;
+		/* fall through */
 	case ixgbe_sfp_type_1g_cu_core0:
 	case ixgbe_sfp_type_1g_cu_core1:
 	default: