[dpdk-dev,v4] net/ixgbe: ensure link status is updated
Checks
Commit Message
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>
---
Hi Wei, please find enclosed patch v4, tested using testpmd.
v1 -> v2:
- rebase on top of head (change flag to 1<<4)
- fix regression with copper links: only update link for fiber links
v2 -> v3:
- remove unnescessary check on speed mask if autoneg is false
v3 -> v4:
- remove default speed set to 10Gb if autoneg is false, rely on
ixgbe_get_link_capabilities( ) instead.
---
drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
2 files changed, 15 insertions(+)
Comments
> -----Original Message-----
> From: Laurent Hardy [mailto:laurent.hardy@6wind.com]
> Sent: Thursday, April 27, 2017 11:04 PM
> To: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
> Subject: [PATCH v4] net/ixgbe: ensure link status is updated
>
> 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>
Acked-by: Wei Dai <wei.dai@intel.com>
> ---
> Hi Wei, please find enclosed patch v4, tested using testpmd.
>
> v1 -> v2:
> - rebase on top of head (change flag to 1<<4)
> - fix regression with copper links: only update link for fiber links
>
> v2 -> v3:
> - remove unnescessary check on speed mask if autoneg is false
>
> v3 -> v4:
> - remove default speed set to 10Gb if autoneg is false, rely on
> ixgbe_get_link_capabilities( ) instead.
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7b856bb..8a0c0a7 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3782,8 +3782,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;
> @@ -3793,6 +3797,14 @@ 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);
> + 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);
> @@ -3810,10 +3822,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 5176b02..b576a6f 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
> --
> 2.1.4
On 4/28/2017 1:43 AM, Dai, Wei wrote:
>> -----Original Message-----
>> From: Laurent Hardy [mailto:laurent.hardy@6wind.com]
>> Sent: Thursday, April 27, 2017 11:04 PM
>> To: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>> <helin.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>> Subject: [PATCH v4] net/ixgbe: ensure link status is updated
>>
>> 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>
> Acked-by: Wei Dai <wei.dai@intel.com>
Applied to dpdk-next-net/master, thanks.
Hi Laurent/Wei,
As I continue to integrate DPDK 17.05 into my application, I have hit
another issue with this patch while testing in a VM with multispeed
fiber ixgbe PCI passthrough. My application periodically invokes
rte_eth_link_get_nowait() to detect link state changes. If the link is
down (no cable or far end disabled), ixgbe_setup_link() will not return
for ~1.3seconds due to the link setup algorithm in
ixgbe_common.c:ixgbe_multispeed_fiber():
+ 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);
+ ixgbe_setup_link(hw, speed, true); + }
+
I have two questions:
* Shouldn't we avoid the link setup cost if the caller has specified
not to wait_to_complete?
* If the concern is speed may not be properly configured, shouldn't
the link setup be deferred until state changes link up thus
minimizing the delays enforced in ixgbe_multispeed_fiber()?
Regards,
-Roger
On 4/27/17 11:03 AM, Laurent Hardy wrote:
> 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>
> ---
> Hi Wei, please find enclosed patch v4, tested using testpmd.
>
> v1 -> v2:
> - rebase on top of head (change flag to 1<<4)
> - fix regression with copper links: only update link for fiber links
>
> v2 -> v3:
> - remove unnescessary check on speed mask if autoneg is false
>
> v3 -> v4:
> - remove default speed set to 10Gb if autoneg is false, rely on
> ixgbe_get_link_capabilities( ) instead.
> ---
> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 7b856bb..8a0c0a7 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3782,8 +3782,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;
> @@ -3793,6 +3797,14 @@ 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);
> + 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);
> @@ -3810,10 +3822,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 5176b02..b576a6f 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
Hi Laurent/Wei,
Any thoughts?
Regards,
Roger
On 5/17/17 9:31 AM, Roger B Melton wrote:
> Hi Laurent/Wei,
>
> As I continue to integrate DPDK 17.05 into my application, I have hit
> another issue with this patch while testing in a VM with multispeed
> fiber ixgbe PCI passthrough. My application periodically invokes
> rte_eth_link_get_nowait() to detect link state changes. If the link
> is down (no cable or far end disabled), ixgbe_setup_link() will not
> return for ~1.3seconds due to the link setup algorithm in
> ixgbe_common.c:ixgbe_multispeed_fiber():
>
> + 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);
> + ixgbe_setup_link(hw, speed, true); + }
> +
>
> I have two questions:
>
> * Shouldn't we avoid the link setup cost if the caller has specified
> not to wait_to_complete?
> * If the concern is speed may not be properly configured, shouldn't
> the link setup be deferred until state changes link up thus
> minimizing the delays enforced in ixgbe_multispeed_fiber()?
>
>
> Regards,
> -Roger
>
>
>
>
> On 4/27/17 11:03 AM, Laurent Hardy wrote:
>> 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>
>> ---
>> Hi Wei, please find enclosed patch v4, tested using testpmd.
>>
>> v1 -> v2:
>> - rebase on top of head (change flag to 1<<4)
>> - fix regression with copper links: only update link for fiber links
>>
>> v2 -> v3:
>> - remove unnescessary check on speed mask if autoneg is false
>>
>> v3 -> v4:
>> - remove default speed set to 10Gb if autoneg is false, rely on
>> ixgbe_get_link_capabilities( ) instead.
>> ---
>> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
>> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index 7b856bb..8a0c0a7 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -3782,8 +3782,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;
>> @@ -3793,6 +3797,14 @@ 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);
>> + 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);
>> @@ -3810,10 +3822,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 5176b02..b576a6f 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
>
> .
>
Hi, Roger
Sorry for late response as we are busy with other higher priority task.
ixgbe_setup_mac_link_multispeed_fiber( ) in ixgbe_common.c calls ixgbe_setup_mac_link( ) which some functions defined in ixgbe/base .
Would you please give us more info to narrow down this issue ?
What device id did you use to trigger this issue ?
To get more info, would you please change "CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n" to " CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=y" in config/common_base
And run your application with EAL parameter --log-level=7 to get trace path of ixgbe functions for us.
Thanks & Best Regards
-Wei
> -----Original Message-----
> From: Roger B. Melton [mailto:rmelton@cisco.com]
> Sent: Tuesday, May 23, 2017 2:54 AM
> To: Laurent Hardy <laurent.hardy@6wind.com>; dev@dpdk.org; Dai, Wei
> <wei.dai@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
> Subject: 2nd try: problem with ixgbe_dev_link_update() for multi-speed fiber
> [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is updated
>
> Hi Laurent/Wei,
>
> Any thoughts?
>
> Regards,
> Roger
>
>
> On 5/17/17 9:31 AM, Roger B Melton wrote:
> > Hi Laurent/Wei,
> >
> > As I continue to integrate DPDK 17.05 into my application, I have hit
> > another issue with this patch while testing in a VM with multispeed
> > fiber ixgbe PCI passthrough. My application periodically invokes
> > rte_eth_link_get_nowait() to detect link state changes. If the link
> > is down (no cable or far end disabled), ixgbe_setup_link() will not
> > return for ~1.3seconds due to the link setup algorithm in
> > ixgbe_common.c:ixgbe_multispeed_fiber():
> >
> > + 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);
> > + ixgbe_setup_link(hw, speed, true); + }
> > +
> >
> > I have two questions:
> >
> > * Shouldn't we avoid the link setup cost if the caller has specified
> > not to wait_to_complete?
> > * If the concern is speed may not be properly configured, shouldn't
> > the link setup be deferred until state changes link up thus
> > minimizing the delays enforced in ixgbe_multispeed_fiber()?
> >
> >
> > Regards,
> > -Roger
> >
> >
> >
> >
> > On 4/27/17 11:03 AM, Laurent Hardy wrote:
> >> 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>
> >> ---
> >> Hi Wei, please find enclosed patch v4, tested using testpmd.
> >>
> >> v1 -> v2:
> >> - rebase on top of head (change flag to 1<<4)
> >> - fix regression with copper links: only update link for fiber links
> >>
> >> v2 -> v3:
> >> - remove unnescessary check on speed mask if autoneg is false
> >>
> >> v3 -> v4:
> >> - remove default speed set to 10Gb if autoneg is false, rely on
> >> ixgbe_get_link_capabilities( ) instead.
> >> ---
> >> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
> >> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
> >> 2 files changed, 15 insertions(+)
> >>
> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> index 7b856bb..8a0c0a7 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> @@ -3782,8 +3782,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;
> >> @@ -3793,6 +3797,14 @@ 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);
> >> + 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); @@
> >> -3810,10 +3822,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 5176b02..b576a6f 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
> >
> > .
> >
Hi, Roger
Sorry for typo error. You should set --log-level=8
Regards
-Wei
> -----Original Message-----
> From: Dai, Wei
> Sent: Tuesday, May 23, 2017 3:24 PM
> To: 'Roger B. Melton' <rmelton@cisco.com>; Laurent Hardy
> <laurent.hardy@6wind.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
> <helin.zhang@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
> Subject: RE: 2nd try: problem with ixgbe_dev_link_update() for multi-speed
> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is updated
>
> Hi, Roger
> Sorry for late response as we are busy with other higher priority task.
> ixgbe_setup_mac_link_multispeed_fiber( ) in ixgbe_common.c calls
> ixgbe_setup_mac_link( ) which some functions defined in ixgbe/base .
> Would you please give us more info to narrow down this issue ?
>
> What device id did you use to trigger this issue ?
>
> To get more info, would you please change
> "CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n" to "
> CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=y" in config/common_base And
> run your application with EAL parameter --log-level=7 to get trace path of
> ixgbe functions for us.
>
> Thanks & Best Regards
> -Wei
>
>
> > -----Original Message-----
> > From: Roger B. Melton [mailto:rmelton@cisco.com]
> > Sent: Tuesday, May 23, 2017 2:54 AM
> > To: Laurent Hardy <laurent.hardy@6wind.com>; dev@dpdk.org; Dai, Wei
> > <wei.dai@intel.com>
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
> > <helin.zhang@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
> > Subject: 2nd try: problem with ixgbe_dev_link_update() for multi-speed
> > fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is
> > updated
> >
> > Hi Laurent/Wei,
> >
> > Any thoughts?
> >
> > Regards,
> > Roger
> >
> >
> > On 5/17/17 9:31 AM, Roger B Melton wrote:
> > > Hi Laurent/Wei,
> > >
> > > As I continue to integrate DPDK 17.05 into my application, I have
> > > hit another issue with this patch while testing in a VM with
> > > multispeed fiber ixgbe PCI passthrough. My application periodically
> > > invokes
> > > rte_eth_link_get_nowait() to detect link state changes. If the link
> > > is down (no cable or far end disabled), ixgbe_setup_link() will not
> > > return for ~1.3seconds due to the link setup algorithm in
> > > ixgbe_common.c:ixgbe_multispeed_fiber():
> > >
> > > + 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);
> > > + ixgbe_setup_link(hw, speed, true); + }
> > > +
> > >
> > > I have two questions:
> > >
> > > * Shouldn't we avoid the link setup cost if the caller has specified
> > > not to wait_to_complete?
> > > * If the concern is speed may not be properly configured, shouldn't
> > > the link setup be deferred until state changes link up thus
> > > minimizing the delays enforced in ixgbe_multispeed_fiber()?
> > >
> > >
> > > Regards,
> > > -Roger
> > >
> > >
> > >
> > >
> > > On 4/27/17 11:03 AM, Laurent Hardy wrote:
> > >> 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>
> > >> ---
> > >> Hi Wei, please find enclosed patch v4, tested using testpmd.
> > >>
> > >> v1 -> v2:
> > >> - rebase on top of head (change flag to 1<<4)
> > >> - fix regression with copper links: only update link for fiber
> > >> links
> > >>
> > >> v2 -> v3:
> > >> - remove unnescessary check on speed mask if autoneg is false
> > >>
> > >> v3 -> v4:
> > >> - remove default speed set to 10Gb if autoneg is false, rely on
> > >> ixgbe_get_link_capabilities( ) instead.
> > >> ---
> > >> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
> > >> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
> > >> 2 files changed, 15 insertions(+)
> > >>
> > >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> index 7b856bb..8a0c0a7 100644
> > >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > >> @@ -3782,8 +3782,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;
> > >> @@ -3793,6 +3797,14 @@ 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);
> > >> + 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); @@
> > >> -3810,10 +3822,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 5176b02..b576a6f 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
> > >
> > > .
> > >
Hi Wei,
Thanks for getting back to me. I'm also dealing with various priorities
so it'll take me a bit to get the data, but I'll get back to you with
the details in the next day or so.
Regards,
Roger
On 5/23/17 3:41 AM, Dai, Wei wrote:
> Hi, Roger
> Sorry for typo error. You should set --log-level=8
>
> Regards
> -Wei
>
>> -----Original Message-----
>> From: Dai, Wei
>> Sent: Tuesday, May 23, 2017 3:24 PM
>> To: 'Roger B. Melton' <rmelton@cisco.com>; Laurent Hardy
>> <laurent.hardy@6wind.com>; dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>> <helin.zhang@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>> Subject: RE: 2nd try: problem with ixgbe_dev_link_update() for multi-speed
>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is updated
>>
>> Hi, Roger
>> Sorry for late response as we are busy with other higher priority task.
>> ixgbe_setup_mac_link_multispeed_fiber( ) in ixgbe_common.c calls
>> ixgbe_setup_mac_link( ) which some functions defined in ixgbe/base .
>> Would you please give us more info to narrow down this issue ?
>>
>> What device id did you use to trigger this issue ?
>>
>> To get more info, would you please change
>> "CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n" to"
>> CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=y" in config/common_base And
>> run your application with EAL parameter --log-level=7 to get trace path of
>> ixgbe functions for us.
>>
>> Thanks & Best Regards
>> -Wei
>>
>>
>>> -----Original Message-----
>>> From: Roger B. Melton [mailto:rmelton@cisco.com]
>>> Sent: Tuesday, May 23, 2017 2:54 AM
>>> To: Laurent Hardy <laurent.hardy@6wind.com>; dev@dpdk.org; Dai, Wei
>>> <wei.dai@intel.com>
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>>> <helin.zhang@intel.com>; Ananyev, Konstantin
>>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>>> Subject: 2nd try: problem with ixgbe_dev_link_update() for multi-speed
>>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is
>>> updated
>>>
>>> Hi Laurent/Wei,
>>>
>>> Any thoughts?
>>>
>>> Regards,
>>> Roger
>>>
>>>
>>> On 5/17/17 9:31 AM, Roger B Melton wrote:
>>>> Hi Laurent/Wei,
>>>>
>>>> As I continue to integrate DPDK 17.05 into my application, I have
>>>> hit another issue with this patch while testing in a VM with
>>>> multispeed fiber ixgbe PCI passthrough. My application periodically
>>>> invokes
>>>> rte_eth_link_get_nowait() to detect link state changes. If the link
>>>> is down (no cable or far end disabled), ixgbe_setup_link() will not
>>>> return for ~1.3seconds due to the link setup algorithm in
>>>> ixgbe_common.c:ixgbe_multispeed_fiber():
>>>>
>>>> + 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);
>>>> + ixgbe_setup_link(hw, speed, true); + }
>>>> +
>>>>
>>>> I have two questions:
>>>>
>>>> * Shouldn't we avoid the link setup cost if the caller has specified
>>>> not to wait_to_complete?
>>>> * If the concern is speed may not be properly configured, shouldn't
>>>> the link setup be deferred until state changes link up thus
>>>> minimizing the delays enforced in ixgbe_multispeed_fiber()?
>>>>
>>>>
>>>> Regards,
>>>> -Roger
>>>>
>>>>
>>>>
>>>>
>>>> On 4/27/17 11:03 AM, Laurent Hardy wrote:
>>>>> 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>
>>>>> ---
>>>>> Hi Wei, please find enclosed patch v4, tested using testpmd.
>>>>>
>>>>> v1 -> v2:
>>>>> - rebase on top of head (change flag to 1<<4)
>>>>> - fix regression with copper links: only update link for fiber
>>>>> links
>>>>>
>>>>> v2 -> v3:
>>>>> - remove unnescessary check on speed mask if autoneg is false
>>>>>
>>>>> v3 -> v4:
>>>>> - remove default speed set to 10Gb if autoneg is false, rely on
>>>>> ixgbe_get_link_capabilities( ) instead.
>>>>> ---
>>>>> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
>>>>> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
>>>>> 2 files changed, 15 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> index 7b856bb..8a0c0a7 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> @@ -3782,8 +3782,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;
>>>>> @@ -3793,6 +3797,14 @@ 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);
>>>>> + 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); @@
>>>>> -3810,10 +3822,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 5176b02..b576a6f 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
>>>> .
>>>>
Hi Wei,
I am using ixgbe:
00:02.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit
SFI/SFP+ Network Connection (rev 01)
The debug output you requested is below showing the results for both far
end link up and far end link down. In the case of far end link up, the
elapse time is too small for the granularity of my logging system. In
the case when the far end link is down, the elapsed time for
rte_eth_link_get_nowait() is ~1.4 seconds. You can really see those 40
and 100msec delays add up when the link is down.
Let me know if you require more information.
Regards,
Roger
Far end link up:
2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
rmeltonDBG Start rte_eth_link_get_nowait()
2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
PMD: ixgbe_check_mac_link_generic(): ixgbe_check_mac_link_generic
2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
rmeltonDBG :End rte_eth_link_get_nowait()
Far end link down:
2017/05/24 18:15:38.502 (debug): rmeltonDBG: Start rte_eth_link_get_nowait()
2017/05/24 18:15:38.502 (debug): PMD: ixgbe_get_media_type_82599():
ixgbe_get_media_type_82599
2017/05/24 18:15:38.502 (debug): PMD:
ixgbe_setup_mac_link_multispeed_fiber():
ixgbe_setup_mac_link_multispeed_fiber
2017/05/24 18:15:38.502 (debug): PMD:
ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
2017/05/24 18:15:38.542 (debug): PMD: ixgbe_setup_mac_link_82599():
ixgbe_setup_mac_link_82599
2017/05/24 18:15:38.542 (debug): PMD:
ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
2017/05/24 18:15:38.542 (debug): PMD:
ixgbe_flap_tx_laser_multispeed_fiber(): ixgbe_flap_tx_laser_multispeed_fiber
2017/05/24 18:15:38.542 (debug): PMD: ixgbe_check_reset_blocked():
ixgbe_check_reset_blocked
2017/05/24 18:15:38.643 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:38.743 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:38.843 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:38.943 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.043 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_setup_mac_link_82599():
ixgbe_setup_mac_link_82599
2017/05/24 18:15:39.083 (debug): PMD:
ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_check_reset_blocked():
ixgbe_check_reset_blocked
2017/05/24 18:15:39.083 (debug): PMD:
ixgbe_verify_lesm_fw_enabled_82599(): ixgbe_verify_lesm_fw_enabled_82599
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eeprom_82599():
ixgbe_read_eeprom_82599
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eerd_buffer_generic():
ixgbe_read_eerd_buffer_generic
2017/05/24 18:15:39.083 (debug): PMD:
ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_poll_eerd_eewr_done():
ixgbe_poll_eerd_eewr_done
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eeprom_82599():
ixgbe_read_eeprom_82599
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eerd_buffer_generic():
ixgbe_read_eerd_buffer_generic
2017/05/24 18:15:39.083 (debug): PMD:
ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
2017/05/24 18:15:39.083 (debug): PMD: ixgbe_poll_eerd_eewr_done():
ixgbe_poll_eerd_eewr_done
2017/05/24 18:15:39.165 (debug): PMD:
ixgbe_flap_tx_laser_multispeed_fiber(): ixgbe_flap_tx_laser_multispeed_fiber
2017/05/24 18:15:39.165 (debug): PMD: ixgbe_check_reset_blocked():
ixgbe_check_reset_blocked
2017/05/24 18:15:39.265 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.265 (debug): PMD:
ixgbe_setup_mac_link_multispeed_fiber():
ixgbe_setup_mac_link_multispeed_fiber
2017/05/24 18:15:39.265 (debug): PMD:
ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_setup_mac_link_82599():
ixgbe_setup_mac_link_82599
2017/05/24 18:15:39.305 (debug): PMD:
ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_check_reset_blocked():
ixgbe_check_reset_blocked
2017/05/24 18:15:39.305 (debug): PMD:
ixgbe_verify_lesm_fw_enabled_82599(): ixgbe_verify_lesm_fw_enabled_82599
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eeprom_82599():
ixgbe_read_eeprom_82599
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eerd_buffer_generic():
ixgbe_read_eerd_buffer_generic
2017/05/24 18:15:39.305 (debug): PMD:
ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_poll_eerd_eewr_done():
ixgbe_poll_eerd_eewr_done
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eeprom_82599():
ixgbe_read_eeprom_82599
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eerd_buffer_generic():
ixgbe_read_eerd_buffer_generic
2017/05/24 18:15:39.305 (debug): PMD:
ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
2017/05/24 18:15:39.305 (debug): PMD: ixgbe_poll_eerd_eewr_done():
ixgbe_poll_eerd_eewr_done
2017/05/24 18:15:39.387 (debug): PMD:
ixgbe_flap_tx_laser_multispeed_fiber(): ixgbe_flap_tx_laser_multispeed_fiber
2017/05/24 18:15:39.387 (debug): PMD: ixgbe_check_reset_blocked():
ixgbe_check_reset_blocked
2017/05/24 18:15:39.487 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.587 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.687 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.787 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.887 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.887 (debug): PMD: ixgbe_check_mac_link_generic():
ixgbe_check_mac_link_generic
2017/05/24 18:15:39.887 (debug): rmeltonDBG
posix_pmd_oper_state_get_by_id:End rte_eth_link_get_nowait()
On 5/24/17 10:40 AM, Roger B. Melton wrote:
> Hi Wei,
>
> Thanks for getting back to me. I'm also dealing with various
> priorities so it'll take me a bit to get the data, but I'll get back
> to you with the details in the next day or so.
>
> Regards,
> Roger
>
>
> On 5/23/17 3:41 AM, Dai, Wei wrote:
>> Hi, Roger
>> Sorry for typo error. You should set --log-level=8
>>
>> Regards
>> -Wei
>>
>>> -----Original Message-----
>>> From: Dai, Wei
>>> Sent: Tuesday, May 23, 2017 3:24 PM
>>> To: 'Roger B. Melton' <rmelton@cisco.com>; Laurent Hardy
>>> <laurent.hardy@6wind.com>; dev@dpdk.org
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>>> <helin.zhang@intel.com>; Ananyev, Konstantin
>>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>>> Subject: RE: 2nd try: problem with ixgbe_dev_link_update() for
>>> multi-speed
>>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status
>>> is updated
>>>
>>> Hi, Roger
>>> Sorry for late response as we are busy with other higher priority task.
>>> ixgbe_setup_mac_link_multispeed_fiber( ) in ixgbe_common.c calls
>>> ixgbe_setup_mac_link( ) which some functions defined in ixgbe/base .
>>> Would you please give us more info to narrow down this issue ?
>>>
>>> What device id did you use to trigger this issue ?
>>>
>>> To get more info, would you please change
>>> "CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n" to"
>>> CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=y" in config/common_base And
>>> run your application with EAL parameter --log-level=7 to get trace
>>> path of
>>> ixgbe functions for us.
>>>
>>> Thanks & Best Regards
>>> -Wei
>>>
>>>
>>>> -----Original Message-----
>>>> From: Roger B. Melton [mailto:rmelton@cisco.com]
>>>> Sent: Tuesday, May 23, 2017 2:54 AM
>>>> To: Laurent Hardy <laurent.hardy@6wind.com>; dev@dpdk.org; Dai, Wei
>>>> <wei.dai@intel.com>
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>>>> <helin.zhang@intel.com>; Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>>>> Subject: 2nd try: problem with ixgbe_dev_link_update() for multi-speed
>>>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status is
>>>> updated
>>>>
>>>> Hi Laurent/Wei,
>>>>
>>>> Any thoughts?
>>>>
>>>> Regards,
>>>> Roger
>>>>
>>>>
>>>> On 5/17/17 9:31 AM, Roger B Melton wrote:
>>>>> Hi Laurent/Wei,
>>>>>
>>>>> As I continue to integrate DPDK 17.05 into my application, I have
>>>>> hit another issue with this patch while testing in a VM with
>>>>> multispeed fiber ixgbe PCI passthrough. My application periodically
>>>>> invokes
>>>>> rte_eth_link_get_nowait() to detect link state changes. If the link
>>>>> is down (no cable or far end disabled), ixgbe_setup_link() will not
>>>>> return for ~1.3seconds due to the link setup algorithm in
>>>>> ixgbe_common.c:ixgbe_multispeed_fiber():
>>>>>
>>>>> + 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);
>>>>> + ixgbe_setup_link(hw, speed, true); + }
>>>>> +
>>>>>
>>>>> I have two questions:
>>>>>
>>>>> * Shouldn't we avoid the link setup cost if the caller has
>>>>> specified
>>>>> not to wait_to_complete?
>>>>> * If the concern is speed may not be properly configured, shouldn't
>>>>> the link setup be deferred until state changes link up thus
>>>>> minimizing the delays enforced in ixgbe_multispeed_fiber()?
>>>>>
>>>>>
>>>>> Regards,
>>>>> -Roger
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 4/27/17 11:03 AM, Laurent Hardy wrote:
>>>>>> 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>
>>>>>> ---
>>>>>> Hi Wei, please find enclosed patch v4, tested using testpmd.
>>>>>>
>>>>>> v1 -> v2:
>>>>>> - rebase on top of head (change flag to 1<<4)
>>>>>> - fix regression with copper links: only update link for fiber
>>>>>> links
>>>>>>
>>>>>> v2 -> v3:
>>>>>> - remove unnescessary check on speed mask if autoneg is false
>>>>>>
>>>>>> v3 -> v4:
>>>>>> - remove default speed set to 10Gb if autoneg is false, rely on
>>>>>> ixgbe_get_link_capabilities( ) instead.
>>>>>> ---
>>>>>> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
>>>>>> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
>>>>>> 2 files changed, 15 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>> index 7b856bb..8a0c0a7 100644
>>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>> @@ -3782,8 +3782,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;
>>>>>> @@ -3793,6 +3797,14 @@ 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);
>>>>>> + 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); @@
>>>>>> -3810,10 +3822,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 5176b02..b576a6f 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
>>>>> .
>>>>>
>
>
=
Hi Wei,
Have you had a chance to look at the debug output yet? Is there any
additional data you need?
Regards,
Roger
On 5/24/17 2:38 PM, Roger B. Melton wrote:
> Hi Wei,
>
> I am using ixgbe:
>
> 00:02.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit
> SFI/SFP+ Network Connection (rev 01)
>
> The debug output you requested is below showing the results for both
> far end link up and far end link down. In the case of far end link
> up, the elapse time is too small for the granularity of my logging
> system. In the case when the far end link is down, the elapsed time
> for rte_eth_link_get_nowait() is ~1.4 seconds. You can really see
> those 40 and 100msec delays add up when the link is down.
>
> Let me know if you require more information.
>
> Regards,
> Roger
>
>
> Far end link up:
>
> 2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
> rmeltonDBG Start rte_eth_link_get_nowait()
>
> 2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
> PMD: ixgbe_check_mac_link_generic(): ixgbe_check_mac_link_generic
>
> 2017/05/24 18:15:14.353 [ulord-dp]: [3706]: UUID: 0, ra: 0 (debug):
> rmeltonDBG :End rte_eth_link_get_nowait()
>
>
> Far end link down:
>
>
> 2017/05/24 18:15:38.502 (debug): rmeltonDBG: Start
> rte_eth_link_get_nowait()
>
> 2017/05/24 18:15:38.502 (debug): PMD: ixgbe_get_media_type_82599():
> ixgbe_get_media_type_82599
> 2017/05/24 18:15:38.502 (debug): PMD:
> ixgbe_setup_mac_link_multispeed_fiber():
> ixgbe_setup_mac_link_multispeed_fiber
> 2017/05/24 18:15:38.502 (debug): PMD:
> ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
> 2017/05/24 18:15:38.542 (debug): PMD: ixgbe_setup_mac_link_82599():
> ixgbe_setup_mac_link_82599
> 2017/05/24 18:15:38.542 (debug): PMD:
> ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
> 2017/05/24 18:15:38.542 (debug): PMD:
> ixgbe_flap_tx_laser_multispeed_fiber():
> ixgbe_flap_tx_laser_multispeed_fiber
> 2017/05/24 18:15:38.542 (debug): PMD: ixgbe_check_reset_blocked():
> ixgbe_check_reset_blocked
> 2017/05/24 18:15:38.643 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:38.743 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:38.843 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:38.943 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.043 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_setup_mac_link_82599():
> ixgbe_setup_mac_link_82599
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_check_reset_blocked():
> ixgbe_check_reset_blocked
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_verify_lesm_fw_enabled_82599(): ixgbe_verify_lesm_fw_enabled_82599
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eeprom_82599():
> ixgbe_read_eeprom_82599
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_read_eerd_buffer_generic(): ixgbe_read_eerd_buffer_generic
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_poll_eerd_eewr_done():
> ixgbe_poll_eerd_eewr_done
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_read_eeprom_82599():
> ixgbe_read_eeprom_82599
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_read_eerd_buffer_generic(): ixgbe_read_eerd_buffer_generic
> 2017/05/24 18:15:39.083 (debug): PMD:
> ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
> 2017/05/24 18:15:39.083 (debug): PMD: ixgbe_poll_eerd_eewr_done():
> ixgbe_poll_eerd_eewr_done
> 2017/05/24 18:15:39.165 (debug): PMD:
> ixgbe_flap_tx_laser_multispeed_fiber():
> ixgbe_flap_tx_laser_multispeed_fiber
> 2017/05/24 18:15:39.165 (debug): PMD: ixgbe_check_reset_blocked():
> ixgbe_check_reset_blocked
> 2017/05/24 18:15:39.265 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.265 (debug): PMD:
> ixgbe_setup_mac_link_multispeed_fiber():
> ixgbe_setup_mac_link_multispeed_fiber
> 2017/05/24 18:15:39.265 (debug): PMD:
> ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_setup_mac_link_82599():
> ixgbe_setup_mac_link_82599
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_get_link_capabilities_82599(): ixgbe_get_link_capabilities_82599
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_check_reset_blocked():
> ixgbe_check_reset_blocked
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_verify_lesm_fw_enabled_82599(): ixgbe_verify_lesm_fw_enabled_82599
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eeprom_82599():
> ixgbe_read_eeprom_82599
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_read_eerd_buffer_generic(): ixgbe_read_eerd_buffer_generic
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_poll_eerd_eewr_done():
> ixgbe_poll_eerd_eewr_done
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_read_eeprom_82599():
> ixgbe_read_eeprom_82599
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_read_eerd_buffer_generic(): ixgbe_read_eerd_buffer_generic
> 2017/05/24 18:15:39.305 (debug): PMD:
> ixgbe_init_eeprom_params_generic(): ixgbe_init_eeprom_params_generic
> 2017/05/24 18:15:39.305 (debug): PMD: ixgbe_poll_eerd_eewr_done():
> ixgbe_poll_eerd_eewr_done
> 2017/05/24 18:15:39.387 (debug): PMD:
> ixgbe_flap_tx_laser_multispeed_fiber():
> ixgbe_flap_tx_laser_multispeed_fiber
> 2017/05/24 18:15:39.387 (debug): PMD: ixgbe_check_reset_blocked():
> ixgbe_check_reset_blocked
> 2017/05/24 18:15:39.487 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.587 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.687 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.787 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.887 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
> 2017/05/24 18:15:39.887 (debug): PMD: ixgbe_check_mac_link_generic():
> ixgbe_check_mac_link_generic
>
> 2017/05/24 18:15:39.887 (debug): rmeltonDBG
> posix_pmd_oper_state_get_by_id:End rte_eth_link_get_nowait()
>
>
> On 5/24/17 10:40 AM, Roger B. Melton wrote:
>> Hi Wei,
>>
>> Thanks for getting back to me. I'm also dealing with various
>> priorities so it'll take me a bit to get the data, but I'll get back
>> to you with the details in the next day or so.
>>
>> Regards,
>> Roger
>>
>>
>> On 5/23/17 3:41 AM, Dai, Wei wrote:
>>> Hi, Roger
>>> Sorry for typo error. You should set --log-level=8
>>>
>>> Regards
>>> -Wei
>>>
>>>> -----Original Message-----
>>>> From: Dai, Wei
>>>> Sent: Tuesday, May 23, 2017 3:24 PM
>>>> To: 'Roger B. Melton' <rmelton@cisco.com>; Laurent Hardy
>>>> <laurent.hardy@6wind.com>; dev@dpdk.org
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>>>> <helin.zhang@intel.com>; Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>>>> Subject: RE: 2nd try: problem with ixgbe_dev_link_update() for
>>>> multi-speed
>>>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link status
>>>> is updated
>>>>
>>>> Hi, Roger
>>>> Sorry for late response as we are busy with other higher priority
>>>> task.
>>>> ixgbe_setup_mac_link_multispeed_fiber( ) in ixgbe_common.c calls
>>>> ixgbe_setup_mac_link( ) which some functions defined in ixgbe/base .
>>>> Would you please give us more info to narrow down this issue ?
>>>>
>>>> What device id did you use to trigger this issue ?
>>>>
>>>> To get more info, would you please change
>>>> "CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n" to"
>>>> CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=y" in config/common_base And
>>>> run your application with EAL parameter --log-level=7 to get trace
>>>> path of
>>>> ixgbe functions for us.
>>>>
>>>> Thanks & Best Regards
>>>> -Wei
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Roger B. Melton [mailto:rmelton@cisco.com]
>>>>> Sent: Tuesday, May 23, 2017 2:54 AM
>>>>> To: Laurent Hardy <laurent.hardy@6wind.com>; dev@dpdk.org; Dai, Wei
>>>>> <wei.dai@intel.com>
>>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Zhang, Helin
>>>>> <helin.zhang@intel.com>; Ananyev, Konstantin
>>>>> <konstantin.ananyev@intel.com>; olivier.matz@6wind.com
>>>>> Subject: 2nd try: problem with ixgbe_dev_link_update() for
>>>>> multi-speed
>>>>> fiber [was] Re: [dpdk-dev] [PATCH v4] net/ixgbe: ensure link
>>>>> status is
>>>>> updated
>>>>>
>>>>> Hi Laurent/Wei,
>>>>>
>>>>> Any thoughts?
>>>>>
>>>>> Regards,
>>>>> Roger
>>>>>
>>>>>
>>>>> On 5/17/17 9:31 AM, Roger B Melton wrote:
>>>>>> Hi Laurent/Wei,
>>>>>>
>>>>>> As I continue to integrate DPDK 17.05 into my application, I have
>>>>>> hit another issue with this patch while testing in a VM with
>>>>>> multispeed fiber ixgbe PCI passthrough. My application periodically
>>>>>> invokes
>>>>>> rte_eth_link_get_nowait() to detect link state changes. If the link
>>>>>> is down (no cable or far end disabled), ixgbe_setup_link() will not
>>>>>> return for ~1.3seconds due to the link setup algorithm in
>>>>>> ixgbe_common.c:ixgbe_multispeed_fiber():
>>>>>>
>>>>>> + 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);
>>>>>> + ixgbe_setup_link(hw, speed, true); + }
>>>>>> +
>>>>>>
>>>>>> I have two questions:
>>>>>>
>>>>>> * Shouldn't we avoid the link setup cost if the caller has
>>>>>> specified
>>>>>> not to wait_to_complete?
>>>>>> * If the concern is speed may not be properly configured,
>>>>>> shouldn't
>>>>>> the link setup be deferred until state changes link up thus
>>>>>> minimizing the delays enforced in ixgbe_multispeed_fiber()?
>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> -Roger
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 4/27/17 11:03 AM, Laurent Hardy wrote:
>>>>>>> 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>
>>>>>>> ---
>>>>>>> Hi Wei, please find enclosed patch v4, tested using testpmd.
>>>>>>>
>>>>>>> v1 -> v2:
>>>>>>> - rebase on top of head (change flag to 1<<4)
>>>>>>> - fix regression with copper links: only update link for fiber
>>>>>>> links
>>>>>>>
>>>>>>> v2 -> v3:
>>>>>>> - remove unnescessary check on speed mask if autoneg is false
>>>>>>>
>>>>>>> v3 -> v4:
>>>>>>> - remove default speed set to 10Gb if autoneg is false, rely on
>>>>>>> ixgbe_get_link_capabilities( ) instead.
>>>>>>> ---
>>>>>>> drivers/net/ixgbe/ixgbe_ethdev.c | 14 ++++++++++++++
>>>>>>> drivers/net/ixgbe/ixgbe_ethdev.h | 1 +
>>>>>>> 2 files changed, 15 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> index 7b856bb..8a0c0a7 100644
>>>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>>>> @@ -3782,8 +3782,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;
>>>>>>> @@ -3793,6 +3797,14 @@ 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);
>>>>>>> + 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); @@
>>>>>>> -3810,10 +3822,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 5176b02..b576a6f 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
>>>>>> .
>>>>>>
>>
>>
> =
> .
>
@@ -3782,8 +3782,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;
@@ -3793,6 +3797,14 @@ 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);
+ 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);
@@ -3810,10 +3822,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;
@@ -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