From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Roger B. Melton" Subject: Re: 2nd try: problem with ixgbe_dev_link_update() for multi-speed fiber [was] Re: [PATCH v4] net/ixgbe: ensure link status is updated Date: Wed, 24 May 2017 10:40:08 -0400 Message-ID: <22d3c322-9c61-9347-161c-140313123747@cisco.com> References: <49759EB36A64CF4892C1AFEC9231E8D650A662DC@PGSMSX101.gar.corp.intel.com> <1493305422-10611-1-git-send-email-laurent.hardy@6wind.com> <1160d98e-93a7-c14c-82a9-0944a2df6e5c@cisco.com> <3aa30d0c-bd00-af5c-f8fa-c081e86adadb@cisco.com> <49759EB36A64CF4892C1AFEC9231E8D650A80C5B@PGSMSX106.gar.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "Yigit, Ferruh" , "Zhang, Helin" , "Ananyev, Konstantin" , "olivier.matz@6wind.com" To: "Dai, Wei" , Laurent Hardy , "dev@dpdk.org" Return-path: Received: from alln-iport-4.cisco.com (alln-iport-4.cisco.com [173.37.142.91]) by dpdk.org (Postfix) with ESMTP id 9174C7CD7 for ; Wed, 24 May 2017 16:40:11 +0200 (CEST) In-Reply-To: <49759EB36A64CF4892C1AFEC9231E8D650A80C5B@PGSMSX106.gar.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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' ; Laurent Hardy >> ; dev@dpdk.org >> Cc: Yigit, Ferruh ; Zhang, Helin >> ; Ananyev, Konstantin >> ; 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 ; dev@dpdk.org; Dai, Wei >>> >>> Cc: Yigit, Ferruh ; Zhang, Helin >>> ; Ananyev, Konstantin >>> ; 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 >>>>> --- >>>>> 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 >>>> . >>>> -- ____________________________________________________________________ |Roger B. Melton | | Cisco Systems | |CPP Software :|: :|: 7100 Kit Creek Rd | |+1.919.476.2332 phone :|||: :|||: RTP, NC 27709-4987 | |+1.919.392.1094 fax .:|||||||:..:|||||||:. rmelton@cisco.com | | | | This email may contain confidential and privileged material for the| | sole use of the intended recipient. Any review, use, distribution | | or disclosure by others is strictly prohibited. If you are not the | | intended recipient (or authorized to receive for the recipient), | | please contact the sender by reply email and delete all copies of | | this message. | | | | For corporate legal information go to: | | http://www.cisco.com/web/about/doing_business/legal/cri/index.html | |__________________________ http://www.cisco.com ____________________|