From 9bc2289fe5ea771f62412fce41c22d9ed7c31e67 Mon Sep 17 00:00:00 2001 From: Chengwen Feng Date: Fri, 9 Apr 2021 12:45:21 +0800 Subject: [PATCH] net/hns3: refactor VF LSC event report Currently, VF driver periodically obtains link status from PF kernel driver, and reports lsc event when detects link status change. Because the period is 1 second, it's probably too late to report especially in such as bonding scenario. To solve this problem we use the following scheme: 1. PF kernel driver support immediate push link status to all VFs when it detects the link status changes. 2. VF driver will detect PF kernel driver whether support push link status in device init stage by sending request link info mailbox message to PF, PF then tell VF the push capability by extend HNS3_MBX_LINK_STAT_CHANGE mailbox message. 3. VF driver marks RTE_PCI_DRV_INTR_LSC in rte_pci_driver by default, when it detects PF doesn't support push link status then it will clear RTE_ETH_DEV_INTR_LSC flag. So if PF kernel driver supports push link status to VF, then VF driver will have RTE_ETH_DEV_INTR_LSC capability. Signed-off-by: Chengwen Feng Signed-off-by: Min Hu (Connor) --- doc/guides/nics/features/hns3_vf.ini | 1 + drivers/net/hns3/hns3_ethdev.h | 27 +++++ drivers/net/hns3/hns3_ethdev_vf.c | 171 +++++++++++++++++++++------ drivers/net/hns3/hns3_intr.c | 30 +++++ drivers/net/hns3/hns3_intr.h | 2 + drivers/net/hns3/hns3_mbx.c | 3 + 6 files changed, 199 insertions(+), 35 deletions(-) diff --git a/doc/guides/nics/features/hns3_vf.ini b/doc/guides/nics/features/hns3_vf.ini index 1640669a98..92383506c3 100644 --- a/doc/guides/nics/features/hns3_vf.ini +++ b/doc/guides/nics/features/hns3_vf.ini @@ -5,6 +5,7 @@ ; [Features] Link status = Y +Link status event = Y Rx interrupt = Y Queue start/stop = Y Runtime Rx queue setup = Y diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h index 4efa4033e6..c20d8c950d 100644 --- a/drivers/net/hns3/hns3_ethdev.h +++ b/drivers/net/hns3/hns3_ethdev.h @@ -763,8 +763,26 @@ struct hns3_pf { struct hns3_tm_conf tm_conf; }; +enum { + HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED, + HNS3_PF_PUSH_LSC_CAP_SUPPORTED, + HNS3_PF_PUSH_LSC_CAP_UNKNOWN +}; + struct hns3_vf { struct hns3_adapter *adapter; + + /* Whether PF support push link status change to VF */ + uint16_t pf_push_lsc_cap; + + /* + * If PF support push link status change, VF still need send request to + * get link status in some cases (such as reset recover stage), so use + * the req_link_info_cnt to control max request count. + */ + uint16_t req_link_info_cnt; + + uint16_t poll_job_started; /* whether poll job is started */ }; struct hns3_adapter { @@ -849,6 +867,8 @@ enum { (&((struct hns3_adapter *)adapter)->hw) #define HNS3_DEV_PRIVATE_TO_PF(adapter) \ (&((struct hns3_adapter *)adapter)->pf) +#define HNS3_DEV_PRIVATE_TO_VF(adapter) \ + (&((struct hns3_adapter *)adapter)->vf) #define HNS3_DEV_HW_TO_ADAPTER(hw) \ container_of(hw, struct hns3_adapter, hw) @@ -858,6 +878,12 @@ static inline struct hns3_pf *HNS3_DEV_HW_TO_PF(struct hns3_hw *hw) return &adapter->pf; } +static inline struct hns3_vf *HNS3_DEV_HW_TO_VF(struct hns3_hw *hw) +{ + struct hns3_adapter *adapter = HNS3_DEV_HW_TO_ADAPTER(hw); + return &adapter->vf; +} + #define hns3_set_field(origin, mask, shift, val) \ do { \ (origin) &= (~(mask)); \ @@ -1003,6 +1029,7 @@ int hns3_dev_infos_get(struct rte_eth_dev *eth_dev, void hns3vf_update_link_status(struct hns3_hw *hw, uint8_t link_status, uint32_t link_speed, uint8_t link_duplex); void hns3_parse_devargs(struct rte_eth_dev *dev); +void hns3vf_update_push_lsc_cap(struct hns3_hw *hw, bool supported); int hns3_restore_ptp(struct hns3_adapter *hns); int hns3_mbuf_dyn_rx_timestamp_register(struct rte_eth_dev *dev, struct rte_eth_conf *conf); diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c index 5f8a460f35..061ba2756f 100644 --- a/drivers/net/hns3/hns3_ethdev_vf.c +++ b/drivers/net/hns3/hns3_ethdev_vf.c @@ -44,6 +44,9 @@ static int hns3vf_add_mc_mac_addr(struct hns3_hw *hw, struct rte_ether_addr *mac_addr); static int hns3vf_remove_mc_mac_addr(struct hns3_hw *hw, struct rte_ether_addr *mac_addr); +static int hns3vf_dev_link_update(struct rte_eth_dev *eth_dev, + __rte_unused int wait_to_complete); + /* set PCI bus mastering */ static int hns3vf_set_bus_master(const struct rte_pci_device *device, bool op) @@ -1191,6 +1194,65 @@ hns3vf_query_dev_specifications(struct hns3_hw *hw) return hns3vf_check_dev_specifications(hw); } +void +hns3vf_update_push_lsc_cap(struct hns3_hw *hw, bool supported) +{ + uint16_t val = supported ? HNS3_PF_PUSH_LSC_CAP_SUPPORTED : + HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED; + uint16_t exp = HNS3_PF_PUSH_LSC_CAP_UNKNOWN; + struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw); + + if (vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_UNKNOWN) + __atomic_compare_exchange(&vf->pf_push_lsc_cap, &exp, &val, 0, + __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE); +} + +static void +hns3vf_get_push_lsc_cap(struct hns3_hw *hw) +{ +#define HNS3_CHECK_PUSH_LSC_CAP_TIMEOUT_MS 500 + + struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id]; + int32_t remain_ms = HNS3_CHECK_PUSH_LSC_CAP_TIMEOUT_MS; + uint16_t val = HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED; + uint16_t exp = HNS3_PF_PUSH_LSC_CAP_UNKNOWN; + struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw); + + __atomic_store_n(&vf->pf_push_lsc_cap, HNS3_PF_PUSH_LSC_CAP_UNKNOWN, + __ATOMIC_RELEASE); + + (void)hns3_send_mbx_msg(hw, HNS3_MBX_GET_LINK_STATUS, 0, NULL, 0, false, + NULL, 0); + + while (remain_ms > 0) { + rte_delay_ms(HNS3_POLL_RESPONE_MS); + if (__atomic_load_n(&vf->pf_push_lsc_cap, __ATOMIC_ACQUIRE) != + HNS3_PF_PUSH_LSC_CAP_UNKNOWN) + break; + remain_ms--; + } + + /* + * When exit above loop, the pf_push_lsc_cap could be one of the three + * state: unknown (means pf not ack), not_supported, supported. + * Here config it as 'not_supported' when it's 'unknown' state. + */ + __atomic_compare_exchange(&vf->pf_push_lsc_cap, &exp, &val, 0, + __ATOMIC_ACQUIRE, __ATOMIC_ACQUIRE); + + if (__atomic_load_n(&vf->pf_push_lsc_cap, __ATOMIC_ACQUIRE) == + HNS3_PF_PUSH_LSC_CAP_SUPPORTED) { + hns3_info(hw, "detect PF support push link status change!"); + } else { + /* + * Framework already set RTE_ETH_DEV_INTR_LSC bit because driver + * declared RTE_PCI_DRV_INTR_LSC in drv_flags. So here cleared + * the RTE_ETH_DEV_INTR_LSC capability. + */ + dev->data->dev_flags &= ~RTE_ETH_DEV_INTR_LSC; + } +} + static int hns3vf_get_capability(struct hns3_hw *hw) { @@ -1404,6 +1466,8 @@ hns3vf_get_configuration(struct hns3_hw *hw) return ret; } + hns3vf_get_push_lsc_cap(hw); + /* Get queue configuration from PF */ ret = hns3vf_get_queue_info(hw); if (ret) @@ -1451,15 +1515,28 @@ hns3vf_set_tc_queue_mapping(struct hns3_adapter *hns, uint16_t nb_rx_q, static void hns3vf_request_link_info(struct hns3_hw *hw) { + struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw); uint8_t resp_msg; + bool send_req; int ret; if (__atomic_load_n(&hw->reset.resetting, __ATOMIC_RELAXED)) return; + + send_req = vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_NOT_SUPPORTED || + vf->req_link_info_cnt > 0; + if (!send_req) + return; + ret = hns3_send_mbx_msg(hw, HNS3_MBX_GET_LINK_STATUS, 0, NULL, 0, false, &resp_msg, sizeof(resp_msg)); - if (ret) - hns3_err(hw, "Failed to fetch link status from PF: %d", ret); + if (ret) { + hns3_err(hw, "failed to fetch link status, ret = %d", ret); + return; + } + + if (vf->req_link_info_cnt > 0) + vf->req_link_info_cnt--; } void @@ -1467,34 +1544,30 @@ hns3vf_update_link_status(struct hns3_hw *hw, uint8_t link_status, uint32_t link_speed, uint8_t link_duplex) { struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id]; + struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw); struct hns3_mac *mac = &hw->mac; - bool report_lse; - bool changed; - - changed = mac->link_status != link_status || - mac->link_speed != link_speed || - mac->link_duplex != link_duplex; - if (!changed) - return; + int ret; /* - * VF's link status/speed/duplex were updated by polling from PF driver, - * because the link status/speed/duplex may be changed in the polling - * interval, so driver will report lse (lsc event) once any of the above - * thress variables changed. - * But if the PF's link status is down and driver saved link status is - * also down, there are no need to report lse. + * PF kernel driver may push link status when VF driver is in resetting, + * driver will stop polling job in this case, after resetting done + * driver will start polling job again. + * When polling job started, driver will get initial link status by + * sending request to PF kernel driver, then could update link status by + * process PF kernel driver's link status mailbox message. */ - report_lse = true; - if (link_status == ETH_LINK_DOWN && link_status == mac->link_status) - report_lse = false; + if (!__atomic_load_n(&vf->poll_job_started, __ATOMIC_RELAXED)) + return; + + if (hw->adapter_state != HNS3_NIC_STARTED) + return; mac->link_status = link_status; mac->link_speed = link_speed; mac->link_duplex = link_duplex; - - if (report_lse) - rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL); + ret = hns3vf_dev_link_update(dev, 0); + if (ret == 0 && dev->data->dev_conf.intr_conf.lsc != 0) + hns3_start_report_lse(dev); } static int @@ -1710,8 +1783,8 @@ hns3vf_service_handler(void *param) /* * The query link status and reset processing are executed in the - * interrupt thread.When the IMP reset occurs, IMP will not respond, - * and the query operation will time out after 30ms. In the case of + * interrupt thread. When the IMP reset occurs, IMP will not respond, + * and the query operation will timeout after 30ms. In the case of * multiple PF/VFs, each query failure timeout causes the IMP reset * interrupt to fail to respond within 100ms. * Before querying the link status, check whether there is a reset @@ -1726,6 +1799,31 @@ hns3vf_service_handler(void *param) eth_dev); } +static void +hns3vf_start_poll_job(struct rte_eth_dev *dev) +{ +#define HNS3_REQUEST_LINK_INFO_REMAINS_CNT 3 + + struct hns3_vf *vf = HNS3_DEV_PRIVATE_TO_VF(dev->data->dev_private); + + if (vf->pf_push_lsc_cap == HNS3_PF_PUSH_LSC_CAP_SUPPORTED) + vf->req_link_info_cnt = HNS3_REQUEST_LINK_INFO_REMAINS_CNT; + + __atomic_store_n(&vf->poll_job_started, 1, __ATOMIC_RELAXED); + + hns3vf_service_handler(dev); +} + +static void +hns3vf_stop_poll_job(struct rte_eth_dev *dev) +{ + struct hns3_vf *vf = HNS3_DEV_PRIVATE_TO_VF(dev->data->dev_private); + + rte_eal_alarm_cancel(hns3vf_service_handler, dev); + + __atomic_store_n(&vf->poll_job_started, 0, __ATOMIC_RELAXED); +} + static int hns3_query_vf_resource(struct hns3_hw *hw) { @@ -2032,7 +2130,8 @@ hns3vf_dev_stop(struct rte_eth_dev *dev) hw->adapter_state = HNS3_NIC_CONFIGURED; } hns3_rx_scattered_reset(dev); - rte_eal_alarm_cancel(hns3vf_service_handler, dev); + hns3vf_stop_poll_job(dev); + hns3_stop_report_lse(dev); rte_spinlock_unlock(&hw->lock); return 0; @@ -2302,19 +2401,17 @@ hns3vf_dev_start(struct rte_eth_dev *dev) hns3_rx_scattered_calc(dev); hns3_set_rxtx_function(dev); hns3_mp_req_start_rxtx(dev); - hns3vf_service_handler(dev); hns3vf_restore_filter(dev); /* Enable interrupt of all rx queues before enabling queues */ hns3_dev_all_rx_queue_intr_enable(hw, true); - - /* - * After finished the initialization, start all tqps to receive/transmit - * packets and refresh all queue status. - */ hns3_start_tqps(hw); + if (dev->data->dev_conf.intr_conf.lsc != 0) + hns3vf_dev_link_update(dev, 0); + hns3vf_start_poll_job(dev); + return ret; start_all_rxqs_fail: @@ -2454,9 +2551,13 @@ hns3vf_stop_service(struct hns3_adapter *hns) eth_dev = &rte_eth_devices[hw->data->port_id]; if (hw->adapter_state == HNS3_NIC_STARTED) { - rte_eal_alarm_cancel(hns3vf_service_handler, eth_dev); + /* + * Make sure call update link status before hns3vf_stop_poll_job + * because update link status depend on polling job exist. + */ hns3vf_update_link_status(hw, ETH_LINK_DOWN, hw->mac.link_speed, - hw->mac.link_duplex); + hw->mac.link_duplex); + hns3vf_stop_poll_job(eth_dev); } hw->mac.link_status = ETH_LINK_DOWN; @@ -2497,7 +2598,7 @@ hns3vf_start_service(struct hns3_adapter *hns) hns3_set_rxtx_function(eth_dev); hns3_mp_req_start_rxtx(eth_dev); if (hw->adapter_state == HNS3_NIC_STARTED) { - hns3vf_service_handler(eth_dev); + hns3vf_start_poll_job(eth_dev); /* Enable interrupt of all rx queues before enabling queues */ hns3_dev_all_rx_queue_intr_enable(hw, true); @@ -2968,7 +3069,7 @@ static const struct rte_pci_id pci_id_hns3vf_map[] = { static struct rte_pci_driver rte_hns3vf_pmd = { .id_table = pci_id_hns3vf_map, - .drv_flags = RTE_PCI_DRV_NEED_MAPPING, + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC, .probe = eth_hns3vf_pci_probe, .remove = eth_hns3vf_pci_remove, }; diff --git a/drivers/net/hns3/hns3_intr.c b/drivers/net/hns3/hns3_intr.c index 39a16a0e91..668f598791 100644 --- a/drivers/net/hns3/hns3_intr.c +++ b/drivers/net/hns3/hns3_intr.c @@ -2614,3 +2614,33 @@ hns3_reset_abort(struct hns3_adapter *hns) reset_string[hw->reset.level], tv.tv_sec, tv.tv_usec); } } + +static void +hns3_report_lse(void *arg) +{ + struct rte_eth_dev *dev = (struct rte_eth_dev *)arg; + struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private); + + if (hw->adapter_state == HNS3_NIC_STARTED) + rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL); +} + +void +hns3_start_report_lse(struct rte_eth_dev *dev) +{ +#define DELAY_REPORT_LSE_US 1 + /* + * When this function called, the context may hold hns3_hw.lock, if + * report lse right now, in some application such as bonding, it will + * trigger call driver's ops which may acquire hns3_hw.lock again, so + * lead to deadlock. + * Here we use delay report to avoid the deadlock. + */ + rte_eal_alarm_set(DELAY_REPORT_LSE_US, hns3_report_lse, dev); +} + +void +hns3_stop_report_lse(struct rte_eth_dev *dev) +{ + rte_eal_alarm_cancel(hns3_report_lse, dev); +} diff --git a/drivers/net/hns3/hns3_intr.h b/drivers/net/hns3/hns3_intr.h index 31f42be27f..cb5a0fead6 100644 --- a/drivers/net/hns3/hns3_intr.h +++ b/drivers/net/hns3/hns3_intr.h @@ -115,5 +115,7 @@ int hns3_reset_req_hw_reset(struct hns3_adapter *hns); int hns3_reset_process(struct hns3_adapter *hns, enum hns3_reset_level reset_level); void hns3_reset_abort(struct hns3_adapter *hns); +void hns3_start_report_lse(struct rte_eth_dev *dev); +void hns3_stop_report_lse(struct rte_eth_dev *dev); #endif /* _HNS3_INTR_H_ */ diff --git a/drivers/net/hns3/hns3_mbx.c b/drivers/net/hns3/hns3_mbx.c index 8ea8998ac7..0cd51e7476 100644 --- a/drivers/net/hns3/hns3_mbx.c +++ b/drivers/net/hns3/hns3_mbx.c @@ -177,6 +177,7 @@ hns3_mbx_handler(struct hns3_hw *hw) { enum hns3_reset_level reset_level; uint8_t link_status, link_duplex; + uint8_t support_push_lsc; uint32_t link_speed; uint16_t *msg_q; uint8_t opcode; @@ -196,6 +197,8 @@ hns3_mbx_handler(struct hns3_hw *hw) link_duplex = (uint8_t)rte_le_to_cpu_16(msg_q[4]); hns3vf_update_link_status(hw, link_status, link_speed, link_duplex); + support_push_lsc = (*(uint8_t *)&msg_q[5]) & 1u; + hns3vf_update_push_lsc_cap(hw, support_push_lsc); break; case HNS3_MBX_ASSERTING_RESET: /* PF has asserted reset hence VF should go in pending -- 2.20.1