diff mbox

[dpdk-dev,v2] net/ixgbe: ensure link status is updated

Message ID 1487262721-22370-1-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Olivier Matz Feb. 16, 2017, 4:32 p.m. UTC 
  From: Laurent Hardy <laurent.hardy@6wind.com>

In case of fiber and link speed set to 1Gb at peer side (with autoneg
or with defined speed), link status could be not properly updated at
time cable is plugged-in.
Indeed if cable was not plugged when device has been configured and
started then link status will not be updated properly with new speed
as no link setup will be triggered.

To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
setup each time link_update() is triggered and current link status is
down. When cable is plugged-in, link setup will be performed via
ixgbe_setup_link().

Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
---

v1 -> v2:
- rebase on top of head (change flag to 1<<4)
- fix regression with copper links: only update link for fiber links

 drivers/net/ixgbe/ixgbe_ethdev.c | 21 +++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
 2 files changed, 22 insertions(+)

Comments

Olivier Matz March 30, 2017, 9:19 a.m. UTC | #1 
Hi,

Can this patch be applied?

Thanks,
Olivier


On Thu, 16 Feb 2017 17:32:01 +0100, Olivier Matz <olivier.matz@6wind.com> wrote:
> From: Laurent Hardy <laurent.hardy@6wind.com>
> 
> In case of fiber and link speed set to 1Gb at peer side (with autoneg
> or with defined speed), link status could be not properly updated at
> time cable is plugged-in.
> Indeed if cable was not plugged when device has been configured and
> started then link status will not be updated properly with new speed
> as no link setup will be triggered.
> 
> To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
> setup each time link_update() is triggered and current link status is
> down. When cable is plugged-in, link setup will be performed via
> ixgbe_setup_link().
> 
> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> ---
> 
> v1 -> v2:
> - rebase on top of head (change flag to 1<<4)
> - fix regression with copper links: only update link for fiber links
> 
>  drivers/net/ixgbe/ixgbe_ethdev.c | 21 +++++++++++++++++++++
>  drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0b89598..1114106 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2357,6 +2357,7 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
>  
>  	/* set flag to update link status after init */
>  	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
> +	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>  
>  	/*
>  	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk
> @@ -3515,8 +3516,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct rte_eth_link link, old;
>  	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
> +	struct ixgbe_interrupt *intr =
> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
>  	int link_up;
>  	int diag;
> +	u32 speed = 0;
> +	bool autoneg = false;
>  
>  	link.link_status = ETH_LINK_DOWN;
>  	link.link_speed = 0;
> @@ -3526,6 +3531,20 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
>  
>  	hw->mac.get_link_status = true;
>  
> +	if ((intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) &&
> +		hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) {
> +		speed = hw->phy.autoneg_advertised;
> +		if (!speed) {
> +			ixgbe_get_link_capabilities(hw, &speed, &autoneg);
> +			/* setup the highest link when no autoneg */
> +			if (!autoneg) {
> +				if (speed & IXGBE_LINK_SPEED_10GB_FULL)
> +					speed = IXGBE_LINK_SPEED_10GB_FULL;
> +			}
> +		}
> +		ixgbe_setup_link(hw, speed, true);
> +	}
> +
>  	/* check if it needs to wait to complete, if lsc interrupt is enabled */
>  	if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0)
>  		diag = ixgbe_check_link(hw, &link_speed, &link_up, 0);
> @@ -3543,10 +3562,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
>  
>  	if (link_up == 0) {
>  		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
> +		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>  		if (link.link_status == old.link_status)
>  			return -1;
>  		return 0;
>  	}
> +	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
>  	link.link_status = ETH_LINK_UP;
>  	link.link_duplex = ETH_LINK_FULL_DUPLEX;
>  
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index aabbb00..bed4379 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -45,6 +45,7 @@
>  #define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
>  #define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
>  #define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
> +#define IXGBE_FLAG_NEED_LINK_CONFIG (uint32_t)(1 << 4)
>  
>  /*
>   * Defines that were not part of ixgbe_type.h as they are not used by the
Wei Dai March 30, 2017, 4:32 p.m. UTC | #2 
Hi, Olivier

Has anyone already tested this patch ?
Can you present some useful info on how to test it ?
Can I use ethtool with some argument to downgrade or upgrade the rate of peer port ?

I have just run testpmd with 82599, the hw->phy. autoneg_advertised is 0 after rte_eal_init() and rte_eth_dev_start().
So I think the codes in if (!speed) { ... } is likely to be run.
I agree with most of your codes.
But why to limit speed of NIC to 10Gbps if autoneg is false and 10Gbps is supported ?
In this case,  how about setting multiple speed ?

Thanks 
-Wei 


> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, March 30, 2017 5:20 PM
> To: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>; Dai, Wei <wei.dai@intel.com>
> Cc: laurent.hardy@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: ensure link status is updated
> 
> Hi,
> 
> Can this patch be applied?
> 
> Thanks,
> Olivier
> 
> 
> On Thu, 16 Feb 2017 17:32:01 +0100, Olivier Matz <olivier.matz@6wind.com>
> wrote:
> > From: Laurent Hardy <laurent.hardy@6wind.com>
> >
> > In case of fiber and link speed set to 1Gb at peer side (with autoneg
> > or with defined speed), link status could be not properly updated at
> > time cable is plugged-in.
> > Indeed if cable was not plugged when device has been configured and
> > started then link status will not be updated properly with new speed
> > as no link setup will be triggered.
> >
> > To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
> > setup each time link_update() is triggered and current link status is
> > down. When cable is plugged-in, link setup will be performed via
> > ixgbe_setup_link().
> >
> > Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
> > ---
> >
> > v1 -> v2:
> > - rebase on top of head (change flag to 1<<4)
> > - fix regression with copper links: only update link for fiber links
> >
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 21 +++++++++++++++++++++
> > drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 0b89598..1114106 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -2357,6 +2357,7 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
> >
> >  	/* set flag to update link status after init */
> >  	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
> > +	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> >
> >  	/*
> >  	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk @@
> > -3515,8 +3516,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int
> wait_to_complete)
> >  	struct ixgbe_hw *hw =
> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >  	struct rte_eth_link link, old;
> >  	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
> > +	struct ixgbe_interrupt *intr =
> > +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
> >  	int link_up;
> >  	int diag;
> > +	u32 speed = 0;
> > +	bool autoneg = false;
> >
> >  	link.link_status = ETH_LINK_DOWN;
> >  	link.link_speed = 0;
> > @@ -3526,6 +3531,20 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev,
> > int wait_to_complete)
> >
> >  	hw->mac.get_link_status = true;
> >
> > +	if ((intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) &&
> > +		hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) {
> > +		speed = hw->phy.autoneg_advertised;
> > +		if (!speed) {
> > +			ixgbe_get_link_capabilities(hw, &speed, &autoneg);
> > +			/* setup the highest link when no autoneg */
> > +			if (!autoneg) {
> > +				if (speed & IXGBE_LINK_SPEED_10GB_FULL)
> > +					speed = IXGBE_LINK_SPEED_10GB_FULL;
> > +			}
> > +		}
> > +		ixgbe_setup_link(hw, speed, true);
> > +	}
> > +
> >  	/* check if it needs to wait to complete, if lsc interrupt is enabled */
> >  	if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0)
> >  		diag = ixgbe_check_link(hw, &link_speed, &link_up, 0); @@ -3543,10
> > +3562,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int
> > wait_to_complete)
> >
> >  	if (link_up == 0) {
> >  		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
> > +		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> >  		if (link.link_status == old.link_status)
> >  			return -1;
> >  		return 0;
> >  	}
> > +	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
> >  	link.link_status = ETH_LINK_UP;
> >  	link.link_duplex = ETH_LINK_FULL_DUPLEX;
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> > b/drivers/net/ixgbe/ixgbe_ethdev.h
> > index aabbb00..bed4379 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > @@ -45,6 +45,7 @@
> >  #define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
> >  #define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
> >  #define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
> > +#define IXGBE_FLAG_NEED_LINK_CONFIG (uint32_t)(1 << 4)
> >
> >  /*
> >   * Defines that were not part of ixgbe_type.h as they are not used by
> > the
Laurent Hardy April 3, 2017, 1:23 p.m. UTC | #3 
Hi Wei,
Thanks for your reply.

If autoneg is false, then we set a default speed to the highest value 
before performing the link setup.
You are right, there is no relevant reason to keep this check on speed 
mask.
should be:
+                       if (!autoneg)
+                               speed = IXGBE_LINK_SPEED_10GB_FULL;

Patch has been tested using testpmd with following setups:

Autoneg setup:
=========
- On dut, both ports of 82599 are connected to a switch with 1Gb ports
- auto-negotiate option is enabled on switch

Defined speed setup:
============
- set link speed to 1Gb on both ports of the switch connected to the dut


Test plan:
======
- set down ports on switch before starting testpmd on dut
- start testpmd, then set up ports on switch
- show port on dut through testpmd

without patch:
---------------------
testpmd> show port info 0

********************* Infos for port 0  *********************
MAC address: 00:1B:21:C7:3B:84
Driver name: net_ixgbe
Connect to socket: 0
memory allocation on the socket: 0
Link status: down
Link speed: 0 Mbps
Link duplex: half-duplex
Promiscuous mode: enabled
Allmulticast mode: disabled
Maximum number of MAC addresses: 127
Maximum number of MAC addresses of hash filtering: 4096
VLAN offload:
   strip on
   filter on
   qinq(extend) off
Hash key size in bytes: 40
Redirection table size: 128
Supported flow types:
   ipv4
   ipv4-tcp
   ipv4-udp
   ipv6
   ipv6-tcp
   ipv6-udp
   unknown
   unknown
   unknown
Max possible RX queues: 128
Max possible number of RXDs per queue: 4096
Min possible number of RXDs per queue: 32
RXDs number alignment: 8
Max possible TX queues: 64
Max possible number of TXDs per queue: 4096
Min possible number of TXDs per queue: 32
TXDs number alignment: 8


With patch:
---------------
testpmd> show port info 0

********************* Infos for port 0  *********************
MAC address: 00:1B:21:C7:3B:84
Driver name: net_ixgbe
Connect to socket: 0
memory allocation on the socket: 0
Link status: up
Link speed: 1000 Mbps
Link duplex: full-duplex
Promiscuous mode: enabled
Allmulticast mode: disabled
Maximum number of MAC addresses: 127
Maximum number of MAC addresses of hash filtering: 4096
VLAN offload:
   strip on
   filter on
   qinq(extend) off
Hash key size in bytes: 40
Redirection table size: 128
Supported flow types:
   ipv4
   ipv4-tcp
   ipv4-udp
   ipv6
   ipv6-tcp
   ipv6-udp
   unknown
   unknown
   unknown
Max possible RX queues: 128
Max possible number of RXDs per queue: 4096
Min possible number of RXDs per queue: 32
RXDs number alignment: 8
Max possible TX queues: 64
Max possible number of TXDs per queue: 4096
Min possible number of TXDs per queue: 32
TXDs number alignment: 8


Thanks & regards,
Laurent

On 03/30/2017 06:32 PM, Dai, Wei wrote:
> Hi, Olivier
>
> Has anyone already tested this patch ?
> Can you present some useful info on how to test it ?
> Can I use ethtool with some argument to downgrade or upgrade the rate of peer port ?
>
> I have just run testpmd with 82599, the hw->phy. autoneg_advertised is 0 after rte_eal_init() and rte_eth_dev_start().
> So I think the codes in if (!speed) { ... } is likely to be run.
> I agree with most of your codes.
> But why to limit speed of NIC to 10Gbps if autoneg is false and 10Gbps is supported ?
> In this case,  how about setting multiple speed ?
>
> Thanks
> -Wei
>
>
>> -----Original Message-----
>> From: Olivier Matz [mailto:olivier.matz@6wind.com]
>> Sent: Thursday, March 30, 2017 5:20 PM
>> To: dev@dpdk.org; Zhang, Helin <helin.zhang@intel.com>; Ananyev,
>> Konstantin <konstantin.ananyev@intel.com>; Dai, Wei <wei.dai@intel.com>
>> Cc: laurent.hardy@6wind.com
>> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: ensure link status is updated
>>
>> Hi,
>>
>> Can this patch be applied?
>>
>> Thanks,
>> Olivier
>>
>>
>> On Thu, 16 Feb 2017 17:32:01 +0100, Olivier Matz <olivier.matz@6wind.com>
>> wrote:
>>> From: Laurent Hardy <laurent.hardy@6wind.com>
>>>
>>> In case of fiber and link speed set to 1Gb at peer side (with autoneg
>>> or with defined speed), link status could be not properly updated at
>>> time cable is plugged-in.
>>> Indeed if cable was not plugged when device has been configured and
>>> started then link status will not be updated properly with new speed
>>> as no link setup will be triggered.
>>>
>>> To avoid this issue, IXGBE_FLAG_NEED_LINK_CONFIG is set to try a link
>>> setup each time link_update() is triggered and current link status is
>>> down. When cable is plugged-in, link setup will be performed via
>>> ixgbe_setup_link().
>>>
>>> Signed-off-by: Laurent Hardy <laurent.hardy@6wind.com>
>>> ---
>>>
>>> v1 -> v2:
>>> - rebase on top of head (change flag to 1<<4)
>>> - fix regression with copper links: only update link for fiber links
>>>
>>>   drivers/net/ixgbe/ixgbe_ethdev.c | 21 +++++++++++++++++++++
>>> drivers/net/ixgbe/ixgbe_ethdev.h |  1 +
>>>   2 files changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> index 0b89598..1114106 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> @@ -2357,6 +2357,7 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
>>>
>>>   	/* set flag to update link status after init */
>>>   	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
>>> +	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>>>
>>>   	/*
>>>   	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk @@
>>> -3515,8 +3516,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int
>> wait_to_complete)
>>>   	struct ixgbe_hw *hw =
>> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>>   	struct rte_eth_link link, old;
>>>   	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
>>> +	struct ixgbe_interrupt *intr =
>>> +		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
>>>   	int link_up;
>>>   	int diag;
>>> +	u32 speed = 0;
>>> +	bool autoneg = false;
>>>
>>>   	link.link_status = ETH_LINK_DOWN;
>>>   	link.link_speed = 0;
>>> @@ -3526,6 +3531,20 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev,
>>> int wait_to_complete)
>>>
>>>   	hw->mac.get_link_status = true;
>>>
>>> +	if ((intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) &&
>>> +		hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) {
>>> +		speed = hw->phy.autoneg_advertised;
>>> +		if (!speed) {
>>> +			ixgbe_get_link_capabilities(hw, &speed, &autoneg);
>>> +			/* setup the highest link when no autoneg */
>>> +			if (!autoneg) {
>>> +				if (speed & IXGBE_LINK_SPEED_10GB_FULL)
>>> +					speed = IXGBE_LINK_SPEED_10GB_FULL;
>>> +			}
>>> +		}
>>> +		ixgbe_setup_link(hw, speed, true);
>>> +	}
>>> +
>>>   	/* check if it needs to wait to complete, if lsc interrupt is enabled */
>>>   	if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0)
>>>   		diag = ixgbe_check_link(hw, &link_speed, &link_up, 0); @@ -3543,10
>>> +3562,12 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, int
>>> wait_to_complete)
>>>
>>>   	if (link_up == 0) {
>>>   		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
>>> +		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>>>   		if (link.link_status == old.link_status)
>>>   			return -1;
>>>   		return 0;
>>>   	}
>>> +	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
>>>   	link.link_status = ETH_LINK_UP;
>>>   	link.link_duplex = ETH_LINK_FULL_DUPLEX;
>>>
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
>>> b/drivers/net/ixgbe/ixgbe_ethdev.h
>>> index aabbb00..bed4379 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>>> @@ -45,6 +45,7 @@
>>>   #define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
>>>   #define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
>>>   #define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
>>> +#define IXGBE_FLAG_NEED_LINK_CONFIG (uint32_t)(1 << 4)
>>>
>>>   /*
>>>    * Defines that were not part of ixgbe_type.h as they are not used by
>>> the
Ferruh Yigit April 10, 2017, 1:36 p.m. UTC | #4 
On 4/3/2017 2:23 PM, Laurent Hardy wrote:
> Hi Wei,
> Thanks for your reply.
> 
> If autoneg is false, then we set a default speed to the highest value 
> before performing the link setup.
> You are right, there is no relevant reason to keep this check on speed 
> mask> should be:
> +                       if (!autoneg)
> +                               speed = IXGBE_LINK_SPEED_10GB_FULL;

Hi Laurent,

Should we expect a new version of the patch with above update?

Thanks,
ferruh

> 
> Patch has been tested using testpmd with following setups:
> 
> Autoneg setup:
> =========
> - On dut, both ports of 82599 are connected to a switch with 1Gb ports
> - auto-negotiate option is enabled on switch
> 
> Defined speed setup:
> ============
> - set link speed to 1Gb on both ports of the switch connected to the dut
> 

<...>

> 
> 
> Thanks & regards,
> Laurent
> 
> On 03/30/2017 06:32 PM, Dai, Wei wrote:
>> Hi, Olivier
>>
>> Has anyone already tested this patch ?
>> Can you present some useful info on how to test it ?
>> Can I use ethtool with some argument to downgrade or upgrade the rate of peer port ?
>>
>> I have just run testpmd with 82599, the hw->phy. autoneg_advertised is 0 after rte_eal_init() and rte_eth_dev_start().
>> So I think the codes in if (!speed) { ... } is likely to be run.
>> I agree with most of your codes.
>> But why to limit speed of NIC to 10Gbps if autoneg is false and 10Gbps is supported ?
>> In this case,  how about setting multiple speed ?
>>
>> Thanks
>> -Wei
>>
>>
<...>
Laurent Hardy April 11, 2017, 12:13 p.m. UTC | #5 
On 04/10/2017 03:36 PM, Ferruh Yigit wrote:
> On 4/3/2017 2:23 PM, Laurent Hardy wrote:
>> Hi Wei,
>> Thanks for your reply.
>>
>> If autoneg is false, then we set a default speed to the highest value
>> before performing the link setup.
>> You are right, there is no relevant reason to keep this check on speed
>> mask> should be:
>> +                       if (!autoneg)
>> +                               speed = IXGBE_LINK_SPEED_10GB_FULL;
> Hi Laurent,
>
> Should we expect a new version of the patch with above update?
Hi Ferruh,
yes, I will send a new version today.

regards,
Laurent
> Thanks,
> ferruh
>
>> Patch has been tested using testpmd with following setups:
>>
>> Autoneg setup:
>> =========
>> - On dut, both ports of 82599 are connected to a switch with 1Gb ports
>> - auto-negotiate option is enabled on switch
>>
>> Defined speed setup:
>> ============
>> - set link speed to 1Gb on both ports of the switch connected to the dut
>>
> <...>
>
>>
>> Thanks & regards,
>> Laurent
>>
>> On 03/30/2017 06:32 PM, Dai, Wei wrote:
>>> Hi, Olivier
>>>
>>> Has anyone already tested this patch ?
>>> Can you present some useful info on how to test it ?
>>> Can I use ethtool with some argument to downgrade or upgrade the rate of peer port ?
>>>
>>> I have just run testpmd with 82599, the hw->phy. autoneg_advertised is 0 after rte_eal_init() and rte_eth_dev_start().
>>> So I think the codes in if (!speed) { ... } is likely to be run.
>>> I agree with most of your codes.
>>> But why to limit speed of NIC to 10Gbps if autoneg is false and 10Gbps is supported ?
>>> In this case,  how about setting multiple speed ?
>>>
>>> Thanks
>>> -Wei
>>>
>>>
> <...>
>
>
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0b89598..1114106 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2357,6 +2357,7 @@  ixgbe_dev_configure(struct rte_eth_dev *dev)
 
 	/* set flag to update link status after init */
 	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
+	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
 
 	/*
 	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk
@@ -3515,8 +3516,12 @@  ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_eth_link link, old;
 	ixgbe_link_speed link_speed = IXGBE_LINK_SPEED_UNKNOWN;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	int link_up;
 	int diag;
+	u32 speed = 0;
+	bool autoneg = false;
 
 	link.link_status = ETH_LINK_DOWN;
 	link.link_speed = 0;
@@ -3526,6 +3531,20 @@  ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 
 	hw->mac.get_link_status = true;
 
+	if ((intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) &&
+		hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) {
+		speed = hw->phy.autoneg_advertised;
+		if (!speed) {
+			ixgbe_get_link_capabilities(hw, &speed, &autoneg);
+			/* setup the highest link when no autoneg */
+			if (!autoneg) {
+				if (speed & IXGBE_LINK_SPEED_10GB_FULL)
+					speed = IXGBE_LINK_SPEED_10GB_FULL;
+			}
+		}
+		ixgbe_setup_link(hw, speed, true);
+	}
+
 	/* check if it needs to wait to complete, if lsc interrupt is enabled */
 	if (wait_to_complete == 0 || dev->data->dev_conf.intr_conf.lsc != 0)
 		diag = ixgbe_check_link(hw, &link_speed, &link_up, 0);
@@ -3543,10 +3562,12 @@  ixgbe_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 
 	if (link_up == 0) {
 		rte_ixgbe_dev_atomic_write_link_status(dev, &link);
+		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
 		if (link.link_status == old.link_status)
 			return -1;
 		return 0;
 	}
+	intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
 	link.link_status = ETH_LINK_UP;
 	link.link_duplex = ETH_LINK_FULL_DUPLEX;
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index aabbb00..bed4379 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -45,6 +45,7 @@ 
 #define IXGBE_FLAG_MAILBOX          (uint32_t)(1 << 1)
 #define IXGBE_FLAG_PHY_INTERRUPT    (uint32_t)(1 << 2)
 #define IXGBE_FLAG_MACSEC           (uint32_t)(1 << 3)
+#define IXGBE_FLAG_NEED_LINK_CONFIG (uint32_t)(1 << 4)
 
 /*
  * Defines that were not part of ixgbe_type.h as they are not used by the