Re: [dpdk-dev] [PATCH v2 02/20] net/ice: support device initialization

["Varghese, Vipin" <vipin.varghese@intel.com>]

Snipped

> +	/* Set the info.ingress_table and info.egress_table
> +	 * for UP translate table. Now just set it to 1:1 map by default
> +	 * -- 0b 111 110 101 100 011 010 001 000 == 0xFAC688
> +	 */
> +	info->ingress_table  = rte_cpu_to_le_32(0x00FAC688);
> +	info->egress_table   = rte_cpu_to_le_32(0x00FAC688);
> +	info->outer_up_table = rte_cpu_to_le_32(0x00FAC688);

Can we use MACRO instead of exact values for ingress, egress and outer_up table.

> +	return 0;
> +}
> +

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);
> +
> +	ret = ice_init_hw(hw);
> +	if (ret) {
> +		PMD_INIT_LOG(ERR, "Failed to initialize HW");
> +		return -EINVAL;
> +	}

Definition for ice_init_hw in patch 01/20 does not check for primary-secondary. Are we allowing secondary to invoke ice_init_hw if it is initialized by primary?

> +
> +	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);
> +

Snipped

> +
> +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);
> +
> +	ICE_PROC_SECONDARY_CHECK_RET_0;

Should not we check if primary is alive and NIC is used or initialized by primary then ' ICE_PROC_SECONDARY_CHECK_RET_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);
> +
> +	ICE_PROC_SECONDARY_CHECK_NO_ERR;
> +

I am just wondering in a multi process (primary-secondary) if primary is killed or exited, then if we try to stop the secondary due to this check the vsi, pool and shutdown is not called. Should not we check if primary is still alive, if yes then 
ICE_PROC_SECONDARY_CHECK_NO_ERR?

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

snipped