[19/21] net/ixgbe/base: remove default advertising for 2.5G and 5G

Message ID 20200612032410.20864-20-guinanx.sun@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series update ixgbe base code |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guinan Sun June 12, 2020, 3:24 a.m. UTC
  We are seeing interoperability issues with switches when 2.5G and 5G are
advertised by default, so default to off.

LINUX ONLY:

We will need to add a note to our README on how to enable 2.5G and 5G.
The netwmask is a combination of:

0x020	1000baseT Full
0x800000000000	2500baseT Full
0x1000000000000	5000baseT Full
0x1000	10000baseT Full

Combine the two into the advertisement flags:
ethtool -s advertise ethX <mask value>

Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
 drivers/net/ixgbe/base/ixgbe_phy.c | 4 ----
 1 file changed, 4 deletions(-)
  

Comments

Ferruh Yigit June 22, 2020, noon UTC | #1
On 6/12/2020 4:24 AM, Guinan Sun wrote:
> We are seeing interoperability issues with switches when 2.5G and 5G are
> advertised by default, so default to off.

This is only for 'X550' device, right? If so can you please clarify this in the
patch title and the commit log?

> 
> LINUX ONLY:
> 
> We will need to add a note to our README on how to enable 2.5G and 5G.
> The netwmask is a combination of:
> 
> 0x020	1000baseT Full
> 0x800000000000	2500baseT Full
> 0x1000000000000	5000baseT Full
> 0x1000	10000baseT Full
> 
> Combine the two into the advertisement flags:
> ethtool -s advertise ethX <mask value>

I can see this is Linux only so we can drop from commit log, but do we need to
document something similar for DPDK?

> 
> Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> ---
>  drivers/net/ixgbe/base/ixgbe_phy.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
> index f859b152e..8d4d9bbfe 100644
> --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> @@ -915,10 +915,6 @@ static s32 ixgbe_get_copper_speeds_supported(struct ixgbe_hw *hw)
>  		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_100_FULL;
>  
>  	switch (hw->mac.type) {
> -	case ixgbe_mac_X550:
> -		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_2_5GB_FULL;
> -		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_5GB_FULL;
> -		break;
>  	case ixgbe_mac_X550EM_x:
>  	case ixgbe_mac_X550EM_a:
>  		hw->phy.speeds_supported &= ~IXGBE_LINK_SPEED_100_FULL;
>
  
Guinan Sun July 1, 2020, 6:24 a.m. UTC | #2
Hi Ferruh

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, June 22, 2020 8:01 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Fujinaka, Todd <todd.fujinaka@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 19/21] net/ixgbe/base: remove default
> advertising for 2.5G and 5G
> 
> On 6/12/2020 4:24 AM, Guinan Sun wrote:
> > We are seeing interoperability issues with switches when 2.5G and 5G
> > are advertised by default, so default to off.
> 
> This is only for 'X550' device, right? If so can you please clarify this in the patch
> title and the commit log?
> 
> >
> > LINUX ONLY:
> >
> > We will need to add a note to our README on how to enable 2.5G and 5G.
> > The netwmask is a combination of:
> >
> > 0x020	1000baseT Full
> > 0x800000000000	2500baseT Full
> > 0x1000000000000	5000baseT Full
> > 0x1000	10000baseT Full
> >
> > Combine the two into the advertisement flags:
> > ethtool -s advertise ethX <mask value>
> 
> I can see this is Linux only so we can drop from commit log, but do we need to
> document something similar for DPDK?

The content related to linux only will be dropped from the commit log.
In addition, regarding the doc, this part is not the content of the limitation, just off the default value, 
it is no need to document something in DPDK.

> 
> >
> > Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
> > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > ---
> >  drivers/net/ixgbe/base/ixgbe_phy.c | 4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c
> > b/drivers/net/ixgbe/base/ixgbe_phy.c
> > index f859b152e..8d4d9bbfe 100644
> > --- a/drivers/net/ixgbe/base/ixgbe_phy.c
> > +++ b/drivers/net/ixgbe/base/ixgbe_phy.c
> > @@ -915,10 +915,6 @@ static s32
> ixgbe_get_copper_speeds_supported(struct ixgbe_hw *hw)
> >  		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_100_FULL;
> >
> >  	switch (hw->mac.type) {
> > -	case ixgbe_mac_X550:
> > -		hw->phy.speeds_supported |=
> IXGBE_LINK_SPEED_2_5GB_FULL;
> > -		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_5GB_FULL;
> > -		break;
> >  	case ixgbe_mac_X550EM_x:
> >  	case ixgbe_mac_X550EM_a:
> >  		hw->phy.speeds_supported &=
> ~IXGBE_LINK_SPEED_100_FULL;
> >
  

Patch

diff --git a/drivers/net/ixgbe/base/ixgbe_phy.c b/drivers/net/ixgbe/base/ixgbe_phy.c
index f859b152e..8d4d9bbfe 100644
--- a/drivers/net/ixgbe/base/ixgbe_phy.c
+++ b/drivers/net/ixgbe/base/ixgbe_phy.c
@@ -915,10 +915,6 @@  static s32 ixgbe_get_copper_speeds_supported(struct ixgbe_hw *hw)
 		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_100_FULL;
 
 	switch (hw->mac.type) {
-	case ixgbe_mac_X550:
-		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_2_5GB_FULL;
-		hw->phy.speeds_supported |= IXGBE_LINK_SPEED_5GB_FULL;
-		break;
 	case ixgbe_mac_X550EM_x:
 	case ixgbe_mac_X550EM_a:
 		hw->phy.speeds_supported &= ~IXGBE_LINK_SPEED_100_FULL;