From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 107C0A04C2 for ; Thu, 14 Nov 2019 12:03:51 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BDA002AB; Thu, 14 Nov 2019 12:03:50 +0100 (CET) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by dpdk.org (Postfix) with ESMTP id DAC902AB; Thu, 14 Nov 2019 12:03:49 +0100 (CET) X-Originating-IP: 90.177.210.238 Received: from [192.168.1.110] (238.210.broadband10.iol.cz [90.177.210.238]) (Authenticated sender: i.maximets@ovn.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id EECB91BF207; Thu, 14 Nov 2019 11:03:47 +0000 (UTC) To: "Cui, LunyuanX" , Ilya Maximets , "dev@dpdk.org" Cc: "Lu, Wenzhuo" , "Yang, Qiming" , "stable@dpdk.org" , Laurent Hardy , "Ye, Xiaolong" References: <20191113125555.88840-1-lunyuanx.cui@intel.com> <20191113163441.90162-1-lunyuanx.cui@intel.com> <5b2db2f5-1d55-94b1-2648-8fe8ca9f7316@ovn.org> From: Ilya Maximets Message-ID: <3917fed0-ae35-8be7-277b-c9166174eeae@ovn.org> Date: Thu, 14 Nov 2019 12:03:46 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v2] net/ixgbe: fix link status X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 14.11.2019 4:45, Cui, LunyuanX wrote: > Hi, Ilya Maximets > > Before my patch, original treatment is as follows: > esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP); > if ((esdp_reg & IXGBE_ESDP_SDP3)) > link_up = 0; > > if (link_up == 0) { > 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); > } > > ixgbe_dev_setup_link_alarm_handler() can cause link status change from down to up. > When port stops, link status should always be down. When bug which you fixed occur, link status is down. > But this treatment can't confirm whether the reason is. I thank port status should be used as judgment. > If port stops, we don’t need to ensure link status is updated. > > If I change patch like this, will it affect your bug? At first, your 'Fixes' tag is not correct because patch 0408f47ba4d6 only changed direct ixgbe_setup_link() call to delayed alarm handler. You may add some more conditions in order to not schedule the handler if adapter is stopped, but don't remove the workaround completely. Best regards, Ilya Maximets. > > Looking forward to your reply. > Thanks > Lunyuan. > >> -----Original Message----- >> From: Ilya Maximets [mailto:i.maximets@ovn.org] >> Sent: Wednesday, November 13, 2019 11:07 PM >> To: Cui, LunyuanX ; dev@dpdk.org >> Cc: Lu, Wenzhuo ; Yang, Qiming >> ; stable@dpdk.org; Laurent Hardy >> ; Wei Dai ; Ye, Xiaolong >> >> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix link status >> >> On 13.11.2019 17:34, Cui LunyuanX wrote: >>> After ports reset, tx laser register will be reset. The link status >>> for 82599eb got from link status register was not correct. >>> Set tx laser disabled after ports reset. >>> >>> ixgbe_dev_setup_link_alarm_handler() will set tx laser enabled when >>> show port information. The purpose of the function has already >>> implemented in ixgbe_dev_start(). There is no need to reuse it in >>> ixgbe_dev_link_update_share(). >> >> The reason why the alarm handler stays there is the one described in >> following commit: >> >> commit c12d22f65b132c56db7b4fdbfd5ddce27d1e9572 >> Author: Laurent Hardy >> Date: Thu Apr 27 17:03:42 2017 +0200 >> >> 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 >> Acked-by: Wei Dai >> >> Does it fixed? >> If not, you should not touch the alarm handler or implement a different >> workaround. >> >> Best regards, Ilya Maximets.