Re: [dpdk-dev] [PATCH v3 09/10] net/bnxt: release port upon close

[Somnath Kotur <somnath.kotur@broadcom.com>]

Ferruh,



On Thu, 16 Jan 2020, 16:50 Ferruh Yigit, <ferruh.yigit@intel.com> wrote:

> On 1/14/2020 2:49 PM, Somnath Kotur wrote:
> > Ferruh,
> > Will recheck and revert on
>
> Hi Ajit,
>
> this is blocking the next-net-brcm tree to be pulled. Would you want to
> drop
> this patch from your tree and continue with merge and deal with this patch
> later?
> Or if this patch is the critical for the series I can keep waiting.
>
Kalesh will be sending v4 of the this patch set addressing this patch as
well shortly ..few hours full now

Thanks
Som

>
> Thanks,
> ferruh
>
> >
> > Thanks
> > Som
> >
> > On Tue, 14 Jan 2020, 18:26 Ferruh Yigit, <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 1/14/2020 5:14 AM, Ajit Khaparde wrote:
> >     > From: Somnath Kotur <somnath.kotur@broadcom.com
> >     <mailto:somnath.kotur@broadcom.com>>
> >     >
> >     > 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: Somnath Kotur <somnath.kotur@broadcom.com
> >     <mailto:somnath.kotur@broadcom.com>>
> >     > Reviewed-by: Kalesh Anakkur Purayil <
> kalesh-anakkur.purayil@broadcom.com
> >     <mailto:kalesh-anakkur.purayil@broadcom.com>>
> >     > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com
> >     <mailto:ajit.khaparde@broadcom.com>>
> >     > ---
> >     >  drivers/net/bnxt/bnxt_ethdev.c | 5 +++++
> >     >  1 file changed, 5 insertions(+)
> >     >
> >     > diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> >     > index ba3f0a7d9..8acfade5f 100644
> >     > --- a/drivers/net/bnxt/bnxt_ethdev.c
> >     > +++ b/drivers/net/bnxt/bnxt_ethdev.c
> >     > @@ -4856,6 +4856,11 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> >     >       if (rc)
> >     >               goto error_free;
> >     >
> >     > +     /* Pass the information to the rte_eth_dev_close() that it
> should also
> >     > +      * release the private port resources.
> >     > +      */
> >     > +     eth_dev->data->dev_flags |= RTE_ETH_DEV_CLOSE_REMOVE;
> >     > +
> >
> >     When you give this flag, 'rte_eth_dev_close()' will release port
> resources, it
> >     will memset the 'eth_dev->data'.
> >
> >     In case user first closed the port, later removed it, remove path
> should be safe
> >     for it. I can see 'bnxt_dev_uninit()' operates on the values of
> 'eth_dev->data'
> >     which should crash in this case.
> >
> >     Can you please double check the return path, all following should be
> safe:
> >     - close the port
> >     - remove the port
> >     - close and remove the port
> >
>
>