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 8867BA04B5; Tue, 3 Dec 2019 14:19:38 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E439E2B8D; Tue, 3 Dec 2019 14:19:37 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 3C45D235 for ; Tue, 3 Dec 2019 14:19:36 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Dec 2019 05:19:35 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,273,1571727600"; d="scan'208";a="412177787" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.96]) ([10.237.221.96]) by fmsmga006.fm.intel.com with ESMTP; 03 Dec 2019 05:19:34 -0800 From: Ferruh Yigit To: Hongbo Zheng , "Wei Hu (Xavier)" References: <20191202025126.73423-1-xavier.huwei@tom.com> <20191202025126.73423-3-xavier.huwei@tom.com> <01debb84-b7de-e9fa-10f0-7fbf85dddc17@intel.com> Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJUBBMBCgA+AhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEAFiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl1meboFCQlupOoACgkQ+TPrQ98T YR9ACBAAv2tomhyxY0Tp9Up7mNGLfEdBu/7joB/vIdqMRv63ojkwr9orQq5V16V/25+JEAD0 60cKodBDM6HdUvqLHatS8fooWRueSXHKYwJ3vxyB2tWDyZrLzLI1jxEvunGodoIzUOtum0Ce gPynnfQCelXBja0BwLXJMplM6TY1wXX22ap0ZViC0m714U5U4LQpzjabtFtjT8qOUR6L7hfy YQ72PBuktGb00UR/N5UrR6GqB0x4W41aZBHXfUQnvWIMmmCrRUJX36hOTYBzh+x86ULgg7H2 1499tA4o6rvE13FiGccplBNWCAIroAe/G11rdoN5NBgYVXu++38gTa/MBmIt6zRi6ch15oLA Ln2vHOdqhrgDuxjhMpG2bpNE36DG/V9WWyWdIRlz3NYPCDM/S3anbHlhjStXHOz1uHOnerXM 1jEjcsvmj1vSyYoQMyRcRJmBZLrekvgZeh7nJzbPHxtth8M7AoqiZ/o/BpYU+0xZ+J5/szWZ aYxxmIRu5ejFf+Wn9s5eXNHmyqxBidpCWvcbKYDBnkw2+Y9E5YTpL0mS0dCCOlrO7gca27ux ybtbj84aaW1g0CfIlUnOtHgMCmz6zPXThb+A8H8j3O6qmPoVqT3qnq3Uhy6GOoH8Fdu2Vchh TWiF5yo+pvUagQP6LpslffufSnu+RKAagkj7/RSuZV25Ag0EV9ZMvgEQAKc0Db17xNqtSwEv mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/ vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi 6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh HwUCXWZ5wAUJB3FgggAKCRD5M+tD3xNhH2O+D/9OEz62YuJQLuIuOfL67eFTIB5/1+0j8Tsu o2psca1PUQ61SZJZOMl6VwNxpdvEaolVdrpnSxUF31kPEvR0Igy8HysQ11pj8AcgH0a9FrvU /8k2Roccd2ZIdpNLkirGFZR7LtRw41Kt1Jg+lafI0efkiHKMT/6D/P1EUp1RxOBNtWGV2hrd 0Yg9ds+VMphHHU69fDH02SwgpvXwG8Qm14Zi5WQ66R4CtTkHuYtA63sS17vMl8fDuTCtvfPF HzvdJLIhDYN3Mm1oMjKLlq4PUdYh68Fiwm+boJoBUFGuregJFlO3hM7uHBDhSEnXQr5mqpPM 6R/7Q5BjAxrwVBisH0yQGjsWlnysRWNfExAE2sRePSl0or9q19ddkRYltl6X4FDUXy2DTXa9 a+Fw4e1EvmcF3PjmTYs9IE3Vc64CRQXkhujcN4ZZh5lvOpU8WgyDxFq7bavFnSS6kx7Tk29/ wNJBp+cf9qsQxLbqhW5kfORuZGecus0TLcmpZEFKKjTJBK9gELRBB/zoN3j41hlEl7uTUXTI JQFLhpsFlEdKLujyvT/aCwP3XWT+B2uZDKrMAElF6ltpTxI53JYi22WO7NH7MR16Fhi4R6vh FHNBOkiAhUpoXRZXaCR6+X4qwA8CwHGqHRBfYFSU/Ulq1ZLR+S3hNj2mbnSx0lBs1eEqe2vh cA== Cc: dev@dpdk.org Message-ID: Date: Tue, 3 Dec 2019 13:19:33 +0000 MIME-Version: 1.0 In-Reply-To: <01debb84-b7de-e9fa-10f0-7fbf85dddc17@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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. > > >> >> 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 >> >