Thanks for the update, so we can assume all functions are available in 
secondary. If not supported it will return right error code and documentation 
is updated for the limitation too.

> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Monday, November 26, 2018 10:40 AM
> To: Varghese, Vipin <vipin.vargh...@intel.com>; Lu, Wenzhuo
> <wenzhuo...@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Yang, Qiming
> <qiming.y...@intel.com>; Wu, Jingjing <jingjing...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 02/19] net/ice: support device initialization
> 
> Hi
> 
> > 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?
> 
> Yes. We check. It is in [PATCH 16/19] net/ice: support basic RX/TX. Please see
> that.
> We roughly split the ice codes in different patch. So the file in only one 
> patch is
> not complete.
> 
> >
> > > + 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>

Reply via email to