As you may notice, I have included a slightly modified version
of this patch in my series in order to cover the full picture:
https://patches.dpdk.org/patch/77559/
Feel free to continue improving your patch in this thread or the other,
as you prefer, as long as the secondary process issue is adressed.
Thanks
13/09/2020 10:53, Thomas Monjalon:
> Hi,
>
> SteveX Yang <stevex.y...@intel.com> wrote:
> > Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
> > for the port can be freed by rte_eth_dev_close().
> >
> > Signed-off-by: SteveX Yang <stevex.y...@intel.com>
>
> I guess the X is not part of your name.
>
> [...]
> > -static int
> > -iavf_dev_uninit(struct rte_eth_dev *dev)
> > -{
> > - struct iavf_info *vf =
> > IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> > -
> > - if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > - return -EPERM;
>
> All the code below is moved from iavf_dev_uninit() where it was
> restricted to the primary process, to iavf_dev_close() which can be
> called from the primary process.
> It looks inconsistent.
>
> >
> > dev->dev_ops = NULL;
> > dev->rx_pkt_burst = NULL;
> > dev->tx_pkt_burst = NULL;
> >
> > - iavf_dev_close(dev);
> > +
> > + if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
> > + if (vf->rss_lut) {
> > + rte_free(vf->rss_lut);
> > + vf->rss_lut = NULL;
> > + }
> > + if (vf->rss_key) {
> > + rte_free(vf->rss_key);
> > + vf->rss_key = NULL;
> > + }
> > + }
> >
> > rte_free(vf->vf_res);
> > vf->vsi_res = NULL;
> >
> > @@ -1449,15 +1456,15 @@ iavf_dev_uninit(struct rte_eth_dev *dev)
> >
> > rte_free(vf->aq_resp);
> > vf->aq_resp = NULL;
> >
> > +}
> >
> > - if (vf->rss_lut) {
> > - rte_free(vf->rss_lut);
> > - vf->rss_lut = NULL;
> > - }
> > - if (vf->rss_key) {
> > - rte_free(vf->rss_key);
> > - vf->rss_key = NULL;
> > - }
> > +static int
> > +iavf_dev_uninit(struct rte_eth_dev *dev)
> > +{
> > + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > + return -EPERM;
> > +
> > + iavf_dev_close(dev);
>
> Are you sure about what should be freed in the secondary process?
> If iavf_dev_close() should not be called by the secondary process,
> you should move the check inside the function,
> because iavf_dev_close() is also called by rte_eth_dev_close().
>
> >
> > return 0;
> >
> > }
