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: Fri, 2 Jun 2017 12:15:50 -0400 Message-ID: <77bceef0-2214-aac8-4713-ba4443211ce7@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> <22d3c322-9c61-9347-161c-140313123747@cisco.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 rcdn-iport-2.cisco.com (rcdn-iport-2.cisco.com [173.37.86.73]) by dpdk.org (Postfix) with ESMTP id CE89D7D04 for ; Fri, 2 Jun 2017 18:16:05 +0200 (CEST) In-Reply-To: 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, 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' ; 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 >>>>>> . >>>>>> >> >> > = > . >