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 B6216A04F1; Sat, 14 Dec 2019 11:13:07 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C6D381BF95; Sat, 14 Dec 2019 11:13:06 +0100 (CET) Received: from incedge.chinasoftinc.com (unknown [114.113.233.8]) by dpdk.org (Postfix) with ESMTP id 302F41BF92 for ; Sat, 14 Dec 2019 11:13:04 +0100 (CET) X-ASG-Debug-ID: 1576318381-0a3dd1322c5ee60001-TfluYd Received: from mail.chinasoftinc.com (inccas002.ito.icss [10.168.0.52]) by incedge.chinasoftinc.com with ESMTP id 1JaHUI2CkqDEE4Ga (version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 14 Dec 2019 18:13:01 +0800 (CST) X-Barracuda-Envelope-From: huwei013@chinasoftinc.com X-Barracuda-RBL-Trusted-Forwarder: 10.168.0.52 X-ASG-Whitelist: Client Received: from [192.168.1.199] (139.159.243.11) by INCCAS002.ito.icss (10.168.0.60) with Microsoft SMTP Server id 14.3.439.0; Sat, 14 Dec 2019 18:13:00 +0800 X-Barracuda-RBL-IP: 192.168.1.199 To: Ferruh Yigit X-ASG-Orig-Subj: Re: [dpdk-dev] [PATCH 2/9] net/hns3: get link state change through mailbox CC: References: <20191202025126.73423-1-xavier.huwei@tom.com> <20191202025126.73423-3-xavier.huwei@tom.com> <01debb84-b7de-e9fa-10f0-7fbf85dddc17@intel.com> From: "Wei Hu (Xavier)" Message-ID: <176a8f09-698c-312f-2c12-a4fb8c55827d@chinasoftinc.com> Date: Sat, 14 Dec 2019 18:12:59 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [139.159.243.11] X-Barracuda-Connect: inccas002.ito.icss[10.168.0.52] X-Barracuda-Start-Time: 1576318381 X-Barracuda-Encrypted: ECDHE-RSA-AES256-SHA X-Barracuda-URL: https://spam.chinasoftinc.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at chinasoftinc.com X-Barracuda-Scan-Msg-Size: 6717 Subject: Re: [dpdk-dev] [PATCH 2/9] net/hns3: get link state change through mailbox 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Ferruh Yigit On 2019/12/3 21:19, Ferruh Yigit wrote: > On 12/3/2019 1:16 PM, Ferruh Yigit wrote: >> On 12/2/2019 2:51 AM, Wei Hu (Xavier) wrote: >>> From: Hongbo Zheng >>> >>> When link down occurs, firmware adds the function of sending message >>> to PF driver through mailbox, hns3 PMD driver can recognize link state >>> change faster through the message. >> >> Hi Xavier, >> >> As far as I can see the 'link_update' dev_ops (hns3_dev_link_update()), is just >> copying data from internal structure to "struct rte_eth_link". And you have >> timers set to regularly (ever second?) update the internal structure for link >> status. >> >> Instead, unless you are not using or need those status internally, you can pull >> the internal link status in the 'hns3_dev_link_update()', this way it can be >> possible to get rid of the timers. Also in current implementation, when used >> asked for the link status, it can be outdated regarding the status of the timers. >> >> >> In this patch, interrupt seems used to update the internal link status, this >> fixes the problem above that user is getting out of date link status. But >> instead of this, what do you think updating "struct rte_eth_link" too in the >> interrupt handler and advertising 'RTE_ETH_DEV_INTR_LSC' capability ("Link >> status event" feature in .ini file), so user can enable the lsc interrupt >> ('dev_conf.intr_conf.lsc') and gets the link status quicker because of the >> support in the API ('rte_eth_link_get')? >> >> Thanks, >> ferruh > > 'Wei Hu (Xavier) ' email address is failing, not sure what > it is, adding the @huawei.com addresses. > Sorry, there is something wrong for the email service: xavier.huwei@tom.com, Now I use the current mailbox that can provide relatively stable services as an alternative. Thanks, Xavier >> >> >>> >>> Signed-off-by: Hongbo Zheng >>> Signed-off-by: Wei Hu (Xavier) >>> --- >>> drivers/net/hns3/hns3_ethdev.c | 8 ++++++-- >>> drivers/net/hns3/hns3_ethdev.h | 1 + >>> drivers/net/hns3/hns3_mbx.c | 37 ++++++++++++++++++++++++++++++++++ >>> drivers/net/hns3/hns3_mbx.h | 8 ++++++++ >>> 4 files changed, 52 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c >>> index bf0ab458f..3c591be51 100644 >>> --- a/drivers/net/hns3/hns3_ethdev.c >>> +++ b/drivers/net/hns3/hns3_ethdev.c >>> @@ -218,6 +218,8 @@ hns3_interrupt_handler(void *param) >>> hns3_schedule_reset(hns); >>> } else if (event_cause == HNS3_VECTOR0_EVENT_RST) >>> hns3_schedule_reset(hns); >>> + else if (event_cause == HNS3_VECTOR0_EVENT_MBX) >>> + hns3_dev_handle_mbx_msg(hw); >>> else >>> hns3_err(hw, "Received unknown event"); >>> >>> @@ -3806,14 +3808,16 @@ hns3_get_mac_link_status(struct hns3_hw *hw) >>> return !!link_status; >>> } >>> >>> -static void >>> +void >>> hns3_update_link_status(struct hns3_hw *hw) >>> { >>> int state; >>> >>> state = hns3_get_mac_link_status(hw); >>> - if (state != hw->mac.link_status) >>> + if (state != hw->mac.link_status) { >>> hw->mac.link_status = state; >>> + hns3_warn(hw, "Link status change to %s!", state ? "up" : "down"); >>> + } >>> } >>> >>> static void >>> diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h >>> index e9a3fe410..004cd75a9 100644 >>> --- a/drivers/net/hns3/hns3_ethdev.h >>> +++ b/drivers/net/hns3/hns3_ethdev.h >>> @@ -631,6 +631,7 @@ int hns3_dev_filter_ctrl(struct rte_eth_dev *dev, >>> enum rte_filter_op filter_op, void *arg); >>> bool hns3_is_reset_pending(struct hns3_adapter *hns); >>> bool hns3vf_is_reset_pending(struct hns3_adapter *hns); >>> +void hns3_update_link_status(struct hns3_hw *hw); >>> >>> static inline bool >>> is_reset_pending(struct hns3_adapter *hns) >>> diff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c >>> index c1647af4b..26807bc4b 100644 >>> --- a/drivers/net/hns3/hns3_mbx.c >>> +++ b/drivers/net/hns3/hns3_mbx.c >>> @@ -282,6 +282,40 @@ hns3_update_resp_position(struct hns3_hw *hw, uint32_t resp_msg) >>> resp->tail = tail; >>> } >>> >>> +static void >>> +hns3_link_fail_parse(struct hns3_hw *hw, uint8_t link_fail_code) >>> +{ >>> + switch (link_fail_code) { >>> + case HNS3_MBX_LF_NORMAL: >>> + break; >>> + case HNS3_MBX_LF_REF_CLOCK_LOST: >>> + hns3_warn(hw, "Reference clock lost!"); >>> + break; >>> + case HNS3_MBX_LF_XSFP_TX_DISABLE: >>> + hns3_warn(hw, "SFP tx is disabled!"); >>> + break; >>> + case HNS3_MBX_LF_XSFP_ABSENT: >>> + hns3_warn(hw, "SFP is absent!"); >>> + break; >>> + default: >>> + hns3_warn(hw, "Unknown fail code:%u!", link_fail_code); >>> + break; >>> + } >>> +} >>> + >>> +static void >>> +hns3_handle_link_change_event(struct hns3_hw *hw, >>> + struct hns3_mbx_pf_to_vf_cmd *req) >>> +{ >>> +#define LINK_STATUS_OFFSET 1 >>> +#define LINK_FAIL_CODE_OFFSET 2 >>> + >>> + if (!req->msg[LINK_STATUS_OFFSET]) >>> + hns3_link_fail_parse(hw, req->msg[LINK_FAIL_CODE_OFFSET]); >>> + >>> + hns3_update_link_status(hw); >>> +} >>> + >>> void >>> hns3_dev_handle_mbx_msg(struct hns3_hw *hw) >>> { >>> @@ -335,6 +369,9 @@ hns3_dev_handle_mbx_msg(struct hns3_hw *hw) >>> >>> hns3_mbx_handler(hw); >>> break; >>> + case HNS3_MBX_PUSH_LINK_STATUS: >>> + hns3_handle_link_change_event(hw, req); >>> + break; >>> default: >>> hns3_err(hw, >>> "VF received unsupported(%d) mbx msg from PF", >>> diff --git a/drivers/net/hns3/hns3_mbx.h b/drivers/net/hns3/hns3_mbx.h >>> index d1a6bfead..3722c8760 100644 >>> --- a/drivers/net/hns3/hns3_mbx.h >>> +++ b/drivers/net/hns3/hns3_mbx.h >>> @@ -41,6 +41,7 @@ enum HNS3_MBX_OPCODE { >>> HNS3_MBX_GET_QID_IN_PF, /* (VF -> PF) get queue id in pf */ >>> >>> HNS3_MBX_HANDLE_VF_TBL = 38, /* (VF -> PF) store/clear hw cfg tbl */ >>> + HNS3_MBX_PUSH_LINK_STATUS = 201, /* (IMP -> PF) get port link status */ >>> }; >>> >>> /* below are per-VF mac-vlan subcodes */ >>> @@ -64,6 +65,13 @@ enum hns3_mbx_tbl_cfg_subcode { >>> HNS3_MBX_VPORT_LIST_CLEAR = 0, >>> }; >>> >>> +enum hns3_mbx_link_fail_subcode { >>> + HNS3_MBX_LF_NORMAL = 0, >>> + HNS3_MBX_LF_REF_CLOCK_LOST, >>> + HNS3_MBX_LF_XSFP_TX_DISABLE, >>> + HNS3_MBX_LF_XSFP_ABSENT, >>> +}; >>> + >>> #define HNS3_MBX_MAX_MSG_SIZE 16 >>> #define HNS3_MBX_MAX_RESP_DATA_SIZE 8 >>> #define HNS3_MBX_RING_MAP_BASIC_MSG_NUM 3 >>> >>