[dpdk-dev] [PATCH 02/19] net/ice: support device initialization

[Varghese, Vipin]

Hi Wenzhuo,

<snipped>

> Subject: [dpdk-dev] [PATCH 02/19] net/ice: support device initialization
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
> Signed-off-by: Qiming Yang <qiming.yang at intel.com>
> Signed-off-by: Xiaoyun Li <xiaoyun.li at intel.com>
> Signed-off-by: Jingjing Wu <jingjing.wu at intel.com>
> ---

<snipped>

> +static int
> +ice_dev_init(struct rte_eth_dev *dev)
> +{
> +	struct rte_pci_device *pci_dev;
> +	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +	int ret;
> +
> +	dev->dev_ops = &ice_eth_dev_ops;
> +
> +	pci_dev = RTE_DEV_TO_PCI(dev->device);
> +
> +	rte_eth_copy_pci_info(dev, pci_dev);
> +	pf->adapter = ICE_DEV_PRIVATE_TO_ADAPTER(dev->data-
> >dev_private);
> +	pf->adapter->eth_dev = dev;
> +	pf->dev_data = dev->data;
> +	hw->back = pf->adapter;
> +	hw->hw_addr = (uint8_t *)pci_dev->mem_resource[0].addr;
> +	hw->vendor_id = pci_dev->id.vendor_id;
> +	hw->device_id = pci_dev->id.device_id;
> +	hw->subsystem_vendor_id = pci_dev->id.subsystem_vendor_id;
> +	hw->subsystem_device_id = pci_dev->id.subsystem_device_id;
> +	hw->bus.device = pci_dev->addr.devid;
> +	hw->bus.func = pci_dev->addr.function;
> +
> +	ice_init_controlq_parameter(hw);
> +

Do we check if process is secondary and ICE PMD is already is initialized? If we do not check will we run to multi process reinitilization?

> +	ret = ice_init_hw(hw);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to initialize HW");
> +		return -EINVAL;
> +	}
> +
> +	PMD_INIT_LOG(INFO, "FW %d.%d.%05d API %d.%d",
> +		     hw->fw_maj_ver, hw->fw_min_ver, hw->fw_build,
> +		     hw->api_maj_ver, hw->api_min_ver);
> +
> +	ice_pf_sw_init(dev);
> +	ret = ice_init_mac_address(dev);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to initialize mac address");
> +		goto err_init_mac;
> +	}

Assuming in secondary multi process this will be skipped if primary has already initialized. Is this understanding correct?

> +
> +	ret = ice_res_pool_init(&pf->msix_pool, 1,
> +				hw-
> >func_caps.common_cap.num_msix_vectors - 1);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to init MSIX pool");
> +		goto err_msix_pool_init;
> +	}
> +
> +	ret = ice_pf_setup(pf);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to setup PF");
> +		goto err_pf_setup;
> +	}

Pool init and pf setup also for secondary skip if primary is done?

> +
> +	return 0;
> +
> +err_pf_setup:
> +	ice_res_pool_destroy(&pf->msix_pool);
> +err_msix_pool_init:
> +	rte_free(dev->data->mac_addrs);
> +err_init_mac:
> +	ice_sched_cleanup_all(hw);
> +	rte_free(hw->port_info);
> +	ice_shutdown_all_ctrlq(hw);
> +
> +	return ret;
> +}
> +
> +static int
> +ice_release_vsi(struct ice_vsi *vsi)
> +{
> +	struct ice_hw *hw;
> +	struct ice_vsi_ctx vsi_ctx;
> +	enum ice_status ret;
> +
> +	if (!vsi)
> +		return 0;

Should we check if process is secondary and primary sees the port, then skip the destroy?

> +
> +	hw = ICE_VSI_TO_HW(vsi);
> +
> +	memset(&vsi_ctx, 0, sizeof(vsi_ctx));
> +
> +	vsi_ctx.vsi_num = vsi->vsi_id;
> +	vsi_ctx.info = vsi->info;
> +	ret = ice_free_vsi(hw, vsi->idx, &vsi_ctx, false, NULL);
> +	if (ret != ICE_SUCCESS) {
> +		PMD_INIT_LOG(ERR, "Failed to free vsi by aq, %u", vsi->vsi_id);
> +		rte_free(vsi);
> +		return -1;
> +	}
> +
> +	rte_free(vsi);
> +	return 0;
> +}
> +
> +static int
> +ice_dev_uninit(struct rte_eth_dev *dev) {
> +	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +		return 0;
> +

Here we have check for secondary, but if the port is added in secondary and not primary is it valid to return 0?

> +	ice_dev_close(dev);
> +
> +	dev->dev_ops = NULL;
> +	dev->rx_pkt_burst = NULL;
> +	dev->tx_pkt_burst = NULL;
> +
> +	rte_free(dev->data->mac_addrs);
> +	dev->data->mac_addrs = NULL;
> +
> +	ice_release_vsi(pf->main_vsi);
> +	ice_sched_cleanup_all(hw);
> +	rte_free(hw->port_info);
> +	ice_shutdown_all_ctrlq(hw);
> +
> +	return 0;
> +}
> +

<snipped>

> +static void
> +ice_dev_close(struct rte_eth_dev *dev)
> +{
> +	struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +
> +	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
> +		return;
> +

Same as previous comment, if port is started in secondary it will not be seen in primary. Hence is it right to return 0 without checking?

> +	ice_res_pool_destroy(&pf->msix_pool);
> +	ice_release_vsi(pf->main_vsi);
> +
> +	ice_shutdown_all_ctrlq(hw);
> +}

<snipped>