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' <rmel...@cisco.com>; Laurent Hardy > <laurent.ha...@6wind.com>; dev@dpdk.org > Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; Zhang, Helin > <helin.zh...@intel.com>; Ananyev, Konstantin > <konstantin.anan...@intel.com>; olivier.m...@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:rmel...@cisco.com] > > Sent: Tuesday, May 23, 2017 2:54 AM > > To: Laurent Hardy <laurent.ha...@6wind.com>; dev@dpdk.org; Dai, Wei > > <wei....@intel.com> > > Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; Zhang, Helin > > <helin.zh...@intel.com>; Ananyev, Konstantin > > <konstantin.anan...@intel.com>; olivier.m...@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.ha...@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 > > > > > > . > > >
