Re: [PATCH net-next 4/5] bnxt_en: Refactor __bnxt_vf_reps_destroy().

[Leon Romanovsky <leon@kernel.org>]

On Sun, Apr 11, 2021 at 08:18:14PM -0400, Michael Chan wrote:
> Add a new helper function __bnxt_free_one_vf_rep() to free one VF rep.
> We also reintialize the VF rep fields to proper initial values so that
> the function can be used without freeing the VF rep data structure.  This
> will be used in subsequent patches to free and recreate VF reps after
> error recovery.
> 
> Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
> Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c | 21 ++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
> index b5d6cd63bea7..a4ac11f5b0e5 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_vfr.c
> @@ -288,6 +288,21 @@ void bnxt_vf_reps_open(struct bnxt *bp)
>  		bnxt_vf_rep_open(bp->vf_reps[i]->dev);
>  }
>  
> +static void __bnxt_free_one_vf_rep(struct bnxt *bp, struct bnxt_vf_rep *vf_rep)
> +{
> +	if (!vf_rep)
> +		return;

How can it be NULL if you check that vf_rep != NULL when called to
__bnxt_free_one_vf_rep() ?

Thanks

> +
> +	if (vf_rep->dst) {
> +		dst_release((struct dst_entry *)vf_rep->dst);
> +		vf_rep->dst = NULL;
> +	}
> +	if (vf_rep->tx_cfa_action != CFA_HANDLE_INVALID) {
> +		hwrm_cfa_vfr_free(bp, vf_rep->vf_idx);
> +		vf_rep->tx_cfa_action = CFA_HANDLE_INVALID;
> +	}
> +}
> +
>  static void __bnxt_vf_reps_destroy(struct bnxt *bp)
>  {
>  	u16 num_vfs = pci_num_vf(bp->pdev);
> @@ -297,11 +312,7 @@ static void __bnxt_vf_reps_destroy(struct bnxt *bp)
>  	for (i = 0; i < num_vfs; i++) {
>  		vf_rep = bp->vf_reps[i];
>  		if (vf_rep) {
> -			dst_release((struct dst_entry *)vf_rep->dst);
> -
> -			if (vf_rep->tx_cfa_action != CFA_HANDLE_INVALID)
> -				hwrm_cfa_vfr_free(bp, vf_rep->vf_idx);
> -
> +			__bnxt_free_one_vf_rep(bp, vf_rep);
>  			if (vf_rep->dev) {
>  				/* if register_netdev failed, then netdev_ops
>  				 * would have been set to NULL
> -- 
> 2.18.1
>