Re: [dpdk-dev] [PATCH 2/3] net/kni: fix rewritten return value

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

On 4/22/2021 4:56 AM, Min Hu (Connor) wrote:
> Return value of function 'eth_kni_dev_stop' passed to 'ret' is
> rewritten later, and this is unreasonable.
> 
> This patch fixes it.
> 
> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/kni/rte_eth_kni.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/kni/rte_eth_kni.c b/drivers/net/kni/rte_eth_kni.c
> index 9ce74e5..067584c 100644
> --- a/drivers/net/kni/rte_eth_kni.c
> +++ b/drivers/net/kni/rte_eth_kni.c
> @@ -211,6 +211,11 @@ eth_kni_close(struct rte_eth_dev *eth_dev)
>  		return 0;
>  
>  	ret = eth_kni_dev_stop(eth_dev);
> +	if (ret) {
> +		PMD_LOG(WARNING, "Not able to stop kni for %s",
> +			eth_dev->data->name);
> +		return ret;
> +	}
>  

'eth_kni_close()' is called by 'eth_kni_remove()', and returning here without
setting 'eth_dev->data->mac_addrs' to NULL, 'eth_kni_remove()' may crash.

And the close ops is to not use the device anymore, there is not much to do if
stop() fails here. So what do you think to log but not return on the
'eth_kni_dev_stop()' failure?


>  	/* mac_addrs must not be freed alone because part of dev_private */
>  	eth_dev->data->mac_addrs = NULL;
>