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

[Ferruh Yigit <ferruh.yigit@intel.com>]

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.

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
>