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

[Varghese, Vipin]

snipped
> > > +	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?
> It's a patch split issue. We add the check in later patch. Will put it in this patch in
> the new version.
Suggestion in current patch if comment is kept it will be easier to understand that it is taken care in future patch. 

Example patch 2/20 has comment stating adding support in patch 5/20. Then in patch 5/20 it removes the ToDo it is easier to read and understand the flow

> 
> >
> > > +
> > > +	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'?
> I think it's not a critical issue if the process is terminate abnormally without uninit.
> Comparing with that, I have more concern about this scenario, if the primary
> process exit and uninit the resource, the secondary process is left alone.
Since primary is application which reserves the huge page memory (malloc, zmalloc, memzone). So when secondary is killed or stop whole huge pages are released. I am bit confused what is check suggested affecting?

 And also
> to me it looks not a good solution to change every PMD for this feature. 
I am not aware about why other PMD are done in specific way. In my humble opinion, if there is a right way let it be used rather than doing other way.

I don't
> see many PMD support it. Maybe we'd better not support it now and wait for a
> better whole picture.
I wait for others to comment to this approach. 

snipped