From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 2BB01A0548;
Mon, 26 Apr 2021 17:30:12 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id EF8C141110;
Mon, 26 Apr 2021 17:30:10 +0200 (CEST)
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
by mails.dpdk.org (Postfix) with ESMTP id 76C1441104
for <dev@dpdk.org>; Mon, 26 Apr 2021 17:30:09 +0200 (CEST)
IronPort-SDR: eEP0apUYpduVa4NjW2ITGB0hUm+UUOtYAN9l7c25DrvjvlvzPT+c2Pw4M5vGyevzDYtIRF0D2+
XqHov251KfTA==
X-IronPort-AV: E=McAfee;i="6200,9189,9966"; a="193165996"
X-IronPort-AV: E=Sophos;i="5.82,252,1613462400"; d="scan'208";a="193165996"
Received: from orsmga008.jf.intel.com ([10.7.209.65])
by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
26 Apr 2021 08:30:07 -0700
IronPort-SDR: raVTHiBmq7bjEeSuIus0RAfvCYYjXXflLsicmfwsMlBE5f/16rVpJMUhKNPTwIrpGd6TdmoXbD
UDF4h94MrLWg==
X-IronPort-AV: E=Sophos;i="5.82,252,1613462400"; d="scan'208";a="429433690"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.241.96])
([10.213.241.96])
by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
26 Apr 2021 08:30:05 -0700
To: "Min Hu (Connor)" <humin29@huawei.com>, dev@dpdk.org
Cc: keith.wiles@intel.com
References: <1619090834-14643-1-git-send-email-humin29@huawei.com>
<1619090834-14643-2-git-send-email-humin29@huawei.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
X-User: ferruhy
Message-ID: <7a033aca-b3d4-fce8-91e0-6b7e6c4a5c7f@intel.com>
Date: Mon, 26 Apr 2021 16:30:02 +0100
MIME-Version: 1.0
In-Reply-To: <1619090834-14643-2-git-send-email-humin29@huawei.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH 1/2] net/tap: fix log loss when state fails
to be restored
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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);
>