From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 2458E4C90 for ; Mon, 10 Sep 2018 17:07:11 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20180910150710euoutp01abe55715b3c45eced51ebced2e28fdee~TEk9kqiKq1060910609euoutp01q for ; Mon, 10 Sep 2018 15:07:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20180910150710euoutp01abe55715b3c45eced51ebced2e28fdee~TEk9kqiKq1060910609euoutp01q DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1536592030; bh=C3StVkvdMusGgjyy+7j3r476im4C1RZtw5Cn23p/Xsw=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=IEPBrCIqF2bqwYiF/uROhILNHRuLJUpKqWLrCOwi+XcfaXsrEjD+ELUWeiA9ppQmj QrB36w0ActeKrU4nZGMXneXtYj0zisxWPCHGllezl9FD+0K1Fhsuh1+s/uOLZ4E9gI Gu8A7d5mASkHrQjaxUBGSjPitMeE/ELVBsEHD210= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20180910150709eucas1p2378a9f10c00b3214cecb4bf7ca9bf2a9~TEk8-HfPb1712217122eucas1p2R; Mon, 10 Sep 2018 15:07:09 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 9F.17.04806.D98869B5; Mon, 10 Sep 2018 16:07:09 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20180910150708eucas1p220c16857c4277b311ddc000b9337d9cb~TEk8KngQJ1365413654eucas1p2a; Mon, 10 Sep 2018 15:07:08 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20180910150708eusmtrp2998f0a065df7a8b8bce827044b1fea76~TEk73g-wH0194401944eusmtrp2s; Mon, 10 Sep 2018 15:07:08 +0000 (GMT) X-AuditID: cbfec7f5-367ff700000012c6-26-5b96889dda84 Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 4F.79.04128.B98869B5; Mon, 10 Sep 2018 16:07:07 +0100 (BST) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20180910150707eusmtip26bff1162c2edadd671b0c037493a2945~TEk7FMYHc1511715117eusmtip2j; Mon, 10 Sep 2018 15:07:07 +0000 (GMT) To: "Zhang, Qi Z" , "dev@dpdk.org" Cc: "Lu, Wenzhuo" , "Ananyev, Konstantin" , Laurent Hardy , "Dai, Wei" , "stable@dpdk.org" From: Ilya Maximets Date: Mon, 10 Sep 2018 18:08:49 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <039ED4275CED7440929022BC67E706115327FA5E@SHSMSX103.ccr.corp.intel.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrDKsWRmVeSWpSXmKPExsWy7djP87pzO6ZFG9xcxGLx7tN2Josr7T/Z Ld7/AXIvn5nOYjF9Qz+bxb+OP+wWB161sFhsPfOX0YHD42L/HUaPXwuWsnos3vOSyaNvyyrG AJYoLpuU1JzMstQifbsEroytz2azFKzSrWh9do29gfGQahcjJ4eEgInErf+PmLoYuTiEBFYw SrS8388O4XxhlPh+7CVU5jOjxJzl39hgWladWckKkVjOKPHi3Faoqo+MEhf6X7GDVAkLhEq8 +zaZGcQWEXCXeN7awApiMwtcYJTofuIPYrMJ6EicWn2EEcRmEVCVeP93DdgGUYEIiSMPFoLF eQUEJU7OfMICYnMKhEi8WP6GCWKOuETTl5VQM+Ultr+dwwxyhITALnaJna9/MkM0l0ms/3SV FeJsF4n9TQfZIWxhiVfHt0DZMhL/d85ngrDrJe63vGSEGNTBKDH90D+ohL3EltfngBo4gLZp SqzfpQ8RdpR43QayiwPI5pO48VYQ4h4+iUnbpkOFeSU62oQgqlUkfh9czgxhS0ncfPeZfQKj 0iwkX85C8tksJJ/NQti7gJFlFaN4amlxbnpqsXFearlecWJucWleul5yfu4mRmAaOv3v+Ncd jPv+JB1iFOBgVOLhvZA1LVqINbGsuDL3EKMEB7OSCO8uHaAQb0piZVVqUX58UWlOavEhRmkO FiVxXj6ttGghgfTEktTs1NSC1CKYLBMHp1QDo+P89datDp4X/Pcd3PLzRHaoiYsKH3/uy4NV ce1LGFa1FIidmtXXt++9kFfrDdWg8paK20euWnw89T1Dfd6Zb9cFp5gvzPV1Yf3nJNP53d9k BsOxk3fNr+UcMH2ZdadqHWPGVi2jBV3XXmh8ai2/b36j7LekOHs5Z37B7XUNUVlvHhieuaQc qMRSnJFoqMVcVJwIAKKgwTc/AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrHIsWRmVeSWpSXmKPExsVy+t/xe7qzO6ZFG5ybb2bx7tN2Josr7T/Z Ld7/WcRicfnMdBaL6Rv62Sz+dfxhtzjwqoXFYuuZv4wOHB4X++8wevxasJTVY/Gel0wefVtW MQawROnZFOWXlqQqZOQXl9gqRRtaGOkZWlroGZlY6hkam8daGZkq6dvZpKTmZJalFunbJehl bH02m6VglW5F67Nr7A2Mh1S7GDk5JARMJFadWckKYgsJLGWUWLowCCIuJfHj1wVWCFtY4s+1 LrYuRi6gmveMEj/nrWYHSQgLhEq8+zaZGcQWEXCXeN7awApSxCxwiVFiWX83O8TUKUwSm1sL QGw2AR2JU6uPMILYvAJ2Eh8OzwCzWQRUJd7/XcMGYosKREisXv6CFaJGUOLkzCcsIDanQIjE i+VvmEBsZgF1iT/zLjFD2OISTV8gPmAWkJfY/nYO8wRGoVlI2mchaZmFpGUWkpYFjCyrGEVS S4tz03OLjfSKE3OLS/PS9ZLzczcxAuNu27GfW3Ywdr0LPsQowMGoxMN7IWtatBBrYllxZe4h RgkOZiUR3l06QCHelMTKqtSi/Pii0pzU4kOMpkDPTWSWEk3OB6aEvJJ4Q1NDcwtLQ3Njc2Mz CyVx3vMGlVFCAumJJanZqakFqUUwfUwcnFINjLvqm3qW1F+euLNcedaz5a0XuYva3hk3XCxt XB1/W1Nf+d7XB4bNT9nnXJvHfkpYQ/jFtI45VyqET3a8LVnI17n42M+X7fVnX0/Pyly9JGne 0hnxItXOX7xcvz29WbJ/25ISwagPT+ceOW558/kqoS1f0g1NHN2iE16x2zd6XWgxPH1C6Vxn /VUlluKMREMt5qLiRACxgMJo0QIAAA== Message-Id: <20180910150708eucas1p220c16857c4277b311ddc000b9337d9cb~TEk8KngQJ1365413654eucas1p2a@eucas1p2.samsung.com> X-CMS-MailID: 20180910150708eucas1p220c16857c4277b311ddc000b9337d9cb X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20180831123824eucas1p1cd2981c716c4764703e24c3eeb4d33c7 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20180831123824eucas1p1cd2981c716c4764703e24c3eeb4d33c7 References: <20180831123824eucas1p1cd2981c716c4764703e24c3eeb4d33c7~P_GOOSRuf0867908679eucas1p1K@eucas1p1.samsung.com> <039ED4275CED7440929022BC67E706115327FA5E@SHSMSX103.ccr.corp.intel.com> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while fiber link update X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 10 Sep 2018 15:07:11 -0000 On 04.09.2018 09:08, Zhang, Qi Z wrote: > Hi Ilya: > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets >> Sent: Friday, August 31, 2018 8:40 PM >> To: dev@dpdk.org >> Cc: Lu, Wenzhuo ; Ananyev, Konstantin >> ; Laurent Hardy >> ; Dai, Wei ; Ilya Maximets >> ; stable@dpdk.org >> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix busy polling while fiber link update >> >> If the multispeed fiber link is in DOWN state, ixgbe_setup_link could take >> around a second of busy polling. This is highly inconvenient for the case where >> single thread periodically checks the link statuses. For example, OVS main >> thread periodically updates the link statuses and hangs for a really long time >> busy waiting on ixgbe_setup_link() for a DOWN fiber ports. For case with 3 >> down ports it hangs for a 3 seconds and unable to do anything including >> packet processing. >> Fix that by shifting that workaround to a separate thread by alarm handler that >> will try to set up link if it is DOWN. > > Does that mean we will block the interrupt thread for 3 seconds? Three times for one second. Other work could be scheduled between. IMHO, it's much better than blocking usual caller for 3 seconds. > Also, can we guarantee there will not be any race condition if we call ixgbe_setup_link at another thread, the base code API is not assumed to be thread-safe as I know. The only user of 'ixgbe_setup_link' is 'ixgbe_dev_start', but it could be called only if device stopped. 'ixgbe_dev_stop' cancels the alarm. Race with 'link_update' avoided by 'IXGBE_FLAG_NEED_LINK_CONFIG' flag. > > Regards > Qi > >> >> Fixes: c12d22f65b13 ("net/ixgbe: ensure link status is updated") >> CC: stable@dpdk.org >> >> Signed-off-by: Ilya Maximets >> --- >> drivers/net/ixgbe/ixgbe_ethdev.c | 43 ++++++++++++++++++++++++-------- >> 1 file changed, 32 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c >> b/drivers/net/ixgbe/ixgbe_ethdev.c >> index 26b192737..a33b9a6e8 100644 >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c >> @@ -221,6 +221,8 @@ static int ixgbe_dev_interrupt_action(struct >> rte_eth_dev *dev, >> struct rte_intr_handle *handle); static void >> ixgbe_dev_interrupt_handler(void *param); static void >> ixgbe_dev_interrupt_delayed_handler(void *param); >> +static void ixgbe_dev_setup_link_alarm_handler(void *param); >> + >> static int ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr >> *mac_addr, >> uint32_t index, uint32_t pool); >> static void ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index); @@ >> -2791,6 +2793,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev) >> >> PMD_INIT_FUNC_TRACE(); >> >> + rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev); >> + >> /* disable interrupts */ >> ixgbe_disable_intr(hw); >> >> @@ -3969,6 +3973,25 @@ ixgbevf_check_link(struct ixgbe_hw *hw, >> ixgbe_link_speed *speed, >> return ret_val; >> } >> >> +static void >> +ixgbe_dev_setup_link_alarm_handler(void *param) { >> + struct rte_eth_dev *dev = (struct rte_eth_dev *)param; >> + struct ixgbe_hw *hw = >> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); >> + struct ixgbe_interrupt *intr = >> + IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private); >> + u32 speed; >> + bool autoneg = false; >> + >> + speed = hw->phy.autoneg_advertised; >> + if (!speed) >> + ixgbe_get_link_capabilities(hw, &speed, &autoneg); >> + >> + ixgbe_setup_link(hw, speed, true); >> + >> + intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG; } >> + >> /* return 0 means link status changed, -1 means not changed */ int >> ixgbe_dev_link_update_share(struct rte_eth_dev *dev, @@ -3981,9 +4004,7 >> @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev, >> IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private); >> int link_up; >> int diag; >> - u32 speed = 0; >> int wait = 1; >> - bool autoneg = false; >> >> memset(&link, 0, sizeof(link)); >> link.link_status = ETH_LINK_DOWN; >> @@ -3993,13 +4014,8 @@ ixgbe_dev_link_update_share(struct rte_eth_dev >> *dev, >> >> hw->mac.get_link_status = true; >> >> - if ((intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) && >> - ixgbe_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); >> - } >> + if (intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG) >> + return rte_eth_linkstatus_set(dev, &link); >> >> /* 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) @@ >> -4017,11 +4033,14 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev, >> } >> >> if (link_up == 0) { >> - intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG; >> + if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) { >> + intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG; >> + rte_eal_alarm_set(10, >> + ixgbe_dev_setup_link_alarm_handler, dev); >> + } >> return rte_eth_linkstatus_set(dev, &link); >> } >> >> - intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG; >> link.link_status = ETH_LINK_UP; >> link.link_duplex = ETH_LINK_FULL_DUPLEX; >> >> @@ -5128,6 +5147,8 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev) >> >> PMD_INIT_FUNC_TRACE(); >> >> + rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev); >> + >> ixgbevf_intr_disable(dev); >> >> hw->adapter_stopped = 1; >> -- >> 2.17.1 >