Re: [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails to be restored

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

On 4/22/2021 12:27 PM, Min Hu (Connor) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> After restoring the remote states, the return value of ioctl() is not
> checked. Therefore, users cannot know whether the remote state is
> restored successfully.
> 
> This patch add log for restoring failure.
> 
> Fixes: 4810d3af8343 ("net/tap: restore state of remote device when closing")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  drivers/net/tap/rte_eth_tap.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 68baa18..6007c78 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1101,6 +1101,7 @@ tap_dev_close(struct rte_eth_dev *dev)
>  	struct pmd_internals *internals = dev->data->dev_private;
>  	struct pmd_process_private *process_private = dev->process_private;
>  	struct rx_queue *rxq;
> +	int ret;
>  
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>  		rte_free(dev->process_private);
> @@ -1133,8 +1134,11 @@ tap_dev_close(struct rte_eth_dev *dev)
>  
>  	if (internals->remote_if_index) {
>  		/* Restore initial remote state */
> -		ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
> +		ret = ioctl(internals->ioctl_sock, SIOCSIFFLAGS,
>  				&internals->remote_initial_flags);
> +		if (ret)
> +			TAP_LOG(ERR, "restore remote state failed: %d", ret);
> +

'ret' is used only in this scope, can you please move the variable declaration
at the beginning of the this if block?
You can do something like "int ret = ioctl(...."


>  	}
>  
>  	rte_mempool_free(internals->gso_ctx_mp);
>