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 1D878A0567; Fri, 13 Mar 2020 08:04:13 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0623E1C011; Fri, 13 Mar 2020 08:04:12 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id ED3FD1C00E for ; Fri, 13 Mar 2020 08:04:09 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Mar 2020 00:04:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,547,1574150400"; d="scan'208";a="237121972" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by orsmga008.jf.intel.com with ESMTP; 13 Mar 2020 00:04:05 -0700 Date: Fri, 13 Mar 2020 15:01:28 +0800 From: Ye Xiaolong To: Haiyue Wang Cc: dev@dpdk.org, qi.z.zhang@intel.com, qiming.yang@intel.com, beilei.xing@intel.com, wei.zhao1@intel.com Message-ID: <20200313070128.GF44839@intel.com> References: <20200309141437.11800-1-haiyue.wang@intel.com> <20200310065029.40966-1-haiyue.wang@intel.com> <20200310065029.40966-8-haiyue.wang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200310065029.40966-8-haiyue.wang@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v2 7/7] net/ice: get the VF hardware index in DCF 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 03/10, Haiyue Wang wrote: >The DCF (Device Config Function) needs the hardware index of the VFs to >control the flow setting. And also if the VF resets, the index may be >changed, so it should handle this in VF reset event. > >Signed-off-by: Haiyue Wang >--- > drivers/net/ice/ice_dcf.c | 81 +++++++++++++++++++++++++++++++ > drivers/net/ice/ice_dcf.h | 4 ++ > drivers/net/ice/ice_dcf_parent.c | 83 +++++++++++++++++++++++++++++++- > 3 files changed, 167 insertions(+), 1 deletion(-) > >diff --git a/drivers/net/ice/ice_dcf.c b/drivers/net/ice/ice_dcf.c >index 480c449f5..1d3c8fa95 100644 >--- a/drivers/net/ice/ice_dcf.c >+++ b/drivers/net/ice/ice_dcf.c >@@ -269,6 +269,63 @@ ice_dcf_get_vf_resource(struct ice_dcf_hw *hw) > return 0; > } > >+static int >+ice_dcf_get_vf_vsi_map(struct ice_dcf_hw *hw) >+{ >+ struct virtchnl_dcf_vsi_map *vsi_map; >+ uint16_t len; >+ int err; >+ >+ err = ice_dcf_send_cmd_req_no_irq(hw, VIRTCHNL_OP_DCF_GET_VSI_MAP, >+ NULL, 0); >+ if (err) { >+ PMD_DRV_LOG(ERR, "Fail to send msg OP_DCF_GET_VSI_MAP"); Better to make the err log format consistent, either "Failed to xxx" or "Fail to xxxx", I prefer to "Failed to xxx". >+ return err; >+ } >+ >+ err = ice_dcf_recv_cmd_rsp_no_irq(hw, VIRTCHNL_OP_DCF_GET_VSI_MAP, >+ hw->arq_buf, ICE_DCF_AQ_BUF_SZ, >+ &len); >+ if (err) { >+ PMD_DRV_LOG(ERR, "Fail to get response of OP_DCF_GET_VSI_MAP"); >+ return err; >+ } >+ >+ vsi_map = (struct virtchnl_dcf_vsi_map *)hw->arq_buf; >+ if (len < sizeof(*vsi_map) || !vsi_map->num_vfs || >+ len < sizeof(*vsi_map) + >+ (vsi_map->num_vfs - 1) * sizeof(vsi_map->vf_vsi[0])) { Better to use a tmp variable to make the code more readable. And is the first len < sizeof(*vsi_map) redundant? >+ PMD_DRV_LOG(ERR, "invalid vf vsi map response with length %u", >+ len); >+ return -EINVAL; >+ } >+ >+ if (hw->num_vfs != 0 && hw->num_vfs != vsi_map->num_vfs) { >+ PMD_DRV_LOG(ERR, "The number VSI map (%u) doesn't match the number of VFs (%u)", >+ vsi_map->num_vfs, hw->num_vfs); >+ return -EINVAL; >+ } >+ >+ len = vsi_map->num_vfs * sizeof(vsi_map->vf_vsi[0]); >+ if (!hw->vf_vsi_map) { >+ hw->num_vfs = vsi_map->num_vfs; >+ hw->vf_vsi_map = rte_zmalloc("vf_vsi_ctx", len, 0); >+ } >+ >+ if (!hw->vf_vsi_map) { >+ PMD_DRV_LOG(ERR, "Fail to alloc memory for VSI context"); >+ return -ENOMEM; >+ } I think above two blocks can be combined with one if (!hw->vf_vsi_map). >+ >+ if (!memcmp(hw->vf_vsi_map, vsi_map->vf_vsi, len)) { >+ PMD_DRV_LOG(DEBUG, "VF VSI map doesn't change"); >+ return 1; >+ } >+ >+ rte_memcpy(hw->vf_vsi_map, vsi_map->vf_vsi, len); >+ return 0; >+} >+ > static int > ice_dcf_mode_disable(struct ice_dcf_hw *hw) > { >@@ -467,6 +524,23 @@ ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc, > return err; > } > >+int >+ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw) >+{ >+ int err = 0; >+ >+ rte_spinlock_lock(&hw->vc_cmd_send_lock); >+ ice_dcf_disable_irq0(hw); >+ >+ if (ice_dcf_get_vf_resource(hw) || ice_dcf_get_vf_vsi_map(hw)) >+ err = -1; >+ >+ ice_dcf_enable_irq0(hw); >+ rte_spinlock_unlock(&hw->vc_cmd_send_lock); >+ >+ return err; >+} >+ > int > ice_dcf_init_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw) > { >@@ -534,6 +608,12 @@ ice_dcf_init_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw) > goto err_alloc; > } > >+ if (ice_dcf_get_vf_vsi_map(hw) < 0) { >+ PMD_INIT_LOG(ERR, "Failed to get VF VSI map"); >+ ice_dcf_mode_disable(hw); >+ goto err_alloc; >+ } >+ > rte_intr_callback_register(&pci_dev->intr_handle, > ice_dcf_dev_interrupt_handler, hw); > rte_intr_enable(&pci_dev->intr_handle); >@@ -566,5 +646,6 @@ ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw) > iavf_shutdown_adminq(&hw->avf); > > rte_free(hw->arq_buf); >+ rte_free(hw->vf_vsi_map); > rte_free(hw->vf_res); > } >diff --git a/drivers/net/ice/ice_dcf.h b/drivers/net/ice/ice_dcf.h >index ecd6303a0..12bef4a2a 100644 >--- a/drivers/net/ice/ice_dcf.h >+++ b/drivers/net/ice/ice_dcf.h >@@ -41,6 +41,9 @@ struct ice_dcf_hw { > > uint8_t *arq_buf; > >+ uint16_t num_vfs; >+ uint16_t *vf_vsi_map; >+ > struct virtchnl_version_info virtchnl_version; > struct virtchnl_vf_resource *vf_res; /* VF resource */ > struct virtchnl_vsi_resource *vsi_res; /* LAN VSI */ >@@ -51,6 +54,7 @@ int ice_dcf_execute_virtchnl_cmd(struct ice_dcf_hw *hw, > struct dcf_virtchnl_cmd *cmd); > int ice_dcf_send_aq_cmd(void *dcf_hw, struct ice_aq_desc *desc, > void *buf, uint16_t buf_size); >+int ice_dcf_handle_vsi_update_event(struct ice_dcf_hw *hw); > int ice_dcf_init_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw); > void ice_dcf_uninit_hw(struct rte_eth_dev *eth_dev, struct ice_dcf_hw *hw); > >diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c >index 4c3bb68b1..2735221e9 100644 >--- a/drivers/net/ice/ice_dcf_parent.c >+++ b/drivers/net/ice/ice_dcf_parent.c >@@ -5,10 +5,76 @@ > #include > #include > >+#include >+ > #include "ice_dcf_ethdev.h" > >+#define ICE_DCF_VSI_UPDATE_SERVICE_INTERVAL 100000 /* us */ >+ >+static __rte_always_inline void >+ice_dcf_update_vsi_ctx(struct ice_hw *hw, uint16_t vsi_handle, >+ uint16_t vsi_map) >+{ >+ struct ice_vsi_ctx *vsi_ctx; >+ >+ if (unlikely(vsi_handle >= ICE_MAX_VSI)) { >+ PMD_DRV_LOG(ERR, "Invalid vsi handle %u", vsi_handle); >+ return; >+ } >+ >+ vsi_ctx = hw->vsi_ctx[vsi_handle]; >+ >+ if (vsi_map & VIRTCHNL_DCF_VF_VSI_VALID) { >+ if (!vsi_ctx) >+ vsi_ctx = ice_malloc(hw, sizeof(*vsi_ctx)); >+ >+ if (!vsi_ctx) { >+ PMD_DRV_LOG(ERR, "No memory for vsi context %u", >+ vsi_handle); >+ return; >+ } Above two blocks can be combined. >+ >+ vsi_ctx->vsi_num = (vsi_map & VIRTCHNL_DCF_VF_VSI_ID_M) >> >+ VIRTCHNL_DCF_VF_VSI_ID_S; >+ hw->vsi_ctx[vsi_handle] = vsi_ctx; >+ >+ PMD_DRV_LOG(DEBUG, "VF%u is assigned with vsi number %u", >+ vsi_handle, vsi_ctx->vsi_num); >+ } else { >+ hw->vsi_ctx[vsi_handle] = NULL; >+ >+ ice_free(hw, vsi_ctx); >+ >+ PMD_DRV_LOG(NOTICE, "VF%u is disabled", vsi_handle); >+ } >+} >+ >+static void >+ice_dcf_update_vf_vsi_map(struct ice_hw *hw, uint16_t num_vfs, >+ uint16_t *vf_vsi_map) >+{ >+ uint16_t vf_id; >+ >+ for (vf_id = 0; vf_id < num_vfs; vf_id++) >+ ice_dcf_update_vsi_ctx(hw, vf_id, vf_vsi_map[vf_id]); >+} >+ >+static void >+ice_dcf_vsi_update_service_handler(void *param) >+{ >+ struct ice_dcf_hw *hw = param; >+ >+ if (!ice_dcf_handle_vsi_update_event(hw)) { >+ struct ice_dcf_adapter *dcf_ad = >+ container_of(hw, struct ice_dcf_adapter, real_hw); >+ >+ ice_dcf_update_vf_vsi_map(&dcf_ad->parent.hw, >+ hw->num_vfs, hw->vf_vsi_map); >+ } >+} >+ > void >-ice_dcf_handle_pf_event_msg(__rte_unused struct ice_dcf_hw *dcf_hw, >+ice_dcf_handle_pf_event_msg(struct ice_dcf_hw *dcf_hw, > uint8_t *msg, uint16_t msglen) > { > struct virtchnl_pf_event *pf_msg = (struct virtchnl_pf_event *)msg; >@@ -21,6 +87,8 @@ ice_dcf_handle_pf_event_msg(__rte_unused struct ice_dcf_hw *dcf_hw, > switch (pf_msg->event) { > case VIRTCHNL_EVENT_RESET_IMPENDING: > PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event"); >+ rte_eal_alarm_set(ICE_DCF_VSI_UPDATE_SERVICE_INTERVAL * 2, >+ ice_dcf_vsi_update_service_handler, dcf_hw); Why * 2 for the VIRTCHNL_EVENT_RESET_IMPENDING event? > break; > case VIRTCHNL_EVENT_LINK_CHANGE: > PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event"); >@@ -28,6 +96,13 @@ ice_dcf_handle_pf_event_msg(__rte_unused struct ice_dcf_hw *dcf_hw, > case VIRTCHNL_EVENT_PF_DRIVER_CLOSE: > PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event"); > break; >+ case VIRTCHNL_EVENT_DCF_VSI_MAP_UPDATE: >+ PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_DCF_VSI_MAP_UPDATE event : VF%u with VSI num %u", >+ pf_msg->event_data.vf_vsi_map.vf_id, >+ pf_msg->event_data.vf_vsi_map.vsi_id); >+ rte_eal_alarm_set(ICE_DCF_VSI_UPDATE_SERVICE_INTERVAL, >+ ice_dcf_vsi_update_service_handler, dcf_hw); >+ break; > default: > PMD_DRV_LOG(ERR, "Unknown event received %u", pf_msg->event); > break; >@@ -235,6 +310,9 @@ ice_dcf_init_parent_adapter(struct rte_eth_dev *eth_dev) > } > parent_adapter->active_pkg_type = ice_load_pkg_type(parent_hw); > >+ ice_dcf_update_vf_vsi_map(parent_hw, >+ hw->num_vfs, hw->vf_vsi_map); >+ No need to split into 2 lines. > mac = (const struct rte_ether_addr *)hw->avf.mac.addr; > if (rte_is_valid_assigned_ether_addr(mac)) > rte_ether_addr_copy(mac, &parent_adapter->pf.dev_addr); >@@ -259,5 +337,8 @@ ice_dcf_uninit_parent_adapter(struct rte_eth_dev *eth_dev) > > eth_dev->data->mac_addrs = NULL; > >+ rte_eal_alarm_cancel(ice_dcf_vsi_update_service_handler, >+ &adapter->real_hw); >+ > ice_dcf_uninit_parent_hw(parent_hw); > } >-- >2.25.1 >