On 12/3/2019 1:16 PM, Ferruh Yigit wrote:
> On 12/2/2019 2:51 AM, Wei Hu (Xavier) wrote:
>> From: Hongbo Zheng <zhenghong...@huawei.com>
>>
>> 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) <xavier.hu...@tom.com>' email address is failing, not sure what
it is, adding the @huawei.com addresses.
>
>
>>
>> Signed-off-by: Hongbo Zheng <zhenghong...@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
>> ---
>> 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
>>
>
