From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id DAA8AA04B7;
	Tue, 13 Oct 2020 12:45:01 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 9C8711DA36;
	Tue, 13 Oct 2020 12:41:10 +0200 (CEST)
Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113])
 by dpdk.org (Postfix) with ESMTP id 825121DA39
 for <dev@dpdk.org>; Tue, 13 Oct 2020 12:41:08 +0200 (CEST)
Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17])
 (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
 (No client certificate requested)
 by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 171757F513;
 Tue, 13 Oct 2020 13:41:07 +0300 (MSK)
DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 171757F513
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru;
 s=default; t=1602585667;
 bh=6U1KCS6zBFmB9mtgpxrwczNP+Lpd7bKbylWG0k8/q0c=;
 h=Subject:To:Cc:References:From:Date:In-Reply-To;
 b=Uli+v6qDwG6CdwbgqGLOpytB6unQh6J2Y+/R4zV0RmCa3/PLgjV4/1C7GGICWFNmK
 zz3ukjEIk9XoFTrYtoCUrNh5GXFzqEHXa2d790Vi788UiRzAiOBhWQX0VV57G+vM+n
 ojw2RhCjgFcf87U83TxWxT07D+Tph/RP3Boas18U=
To: Thomas Monjalon <thomas@monjalon.net>, dev@dpdk.org
Cc: ferruh.yigit@intel.com, Liron Himi <lironh@marvell.com>,
 Stephen Hemminger <stephen@networkplumber.org>, Ray Kinsella
 <mdr@ashroe.eu>, Neil Horman <nhorman@tuxdriver.com>,
 Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
 Gaetan Rivet <grive@u256.net>, Jakub Grajciar <jgrajcia@cisco.com>,
 Matan Azrad <matan@nvidia.com>, Shahaf Shuler <shahafs@nvidia.com>,
 Viacheslav Ovsiienko <viacheslavo@nvidia.com>, Zyta Szpak <zr@semihalf.com>,
 Stephen Hemminger <sthemmin@microsoft.com>,
 "K. Y. Srinivasan" <kys@microsoft.com>,
 Haiyang Zhang <haiyangz@microsoft.com>, Long Li <longli@microsoft.com>,
 Maxime Coquelin <maxime.coquelin@redhat.com>,
 Chenbo Xia <chenbo.xia@intel.com>, Zhihong Wang <zhihong.wang@intel.com>
References: <20200913220711.3768597-1-thomas@monjalon.net>
 <20201013100634.2482593-1-thomas@monjalon.net>
 <20201013100634.2482593-4-thomas@monjalon.net>
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Organization: OKTET Labs
Message-ID: <a66ed2d4-de0f-fb93-efa7-a56a2f948fb3@oktetlabs.ru>
Date: Tue, 13 Oct 2020 13:41:06 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.3.1
MIME-Version: 1.0
In-Reply-To: <20201013100634.2482593-4-thomas@monjalon.net>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [dpdk-dev] [PATCH v5 3/3] ethdev: allow close function to
	return an error
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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 10/13/20 1:06 PM, Thomas Monjalon wrote:
> The API function rte_eth_dev_close() was returning void.
> The return type is changed to int for notifying of errors.
> 
> If an error happens during a close operation,
> the status of the port is undefined,
> a maximum of resources having been freed.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Liron Himi <lironh@marvell.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

[snip]

> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 73c3d6f7fe..ad12d4f07a 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1716,19 +1716,25 @@ rte_eth_dev_set_link_down(uint16_t port_id)
>  	return eth_err(port_id, (*dev->dev_ops->dev_set_link_down)(dev));
>  }
>  
> -void
> +int
>  rte_eth_dev_close(uint16_t port_id)
>  {
>  	struct rte_eth_dev *dev;
> +	int firsterr, binerr;
> +	int *lasterr = &firsterr;
>  
> -	RTE_ETH_VALID_PORTID_OR_RET(port_id);
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  	dev = &rte_eth_devices[port_id];
>  
> -	RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close);
> -	(*dev->dev_ops->dev_close)(dev);
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_close, -ENOTSUP);
> +	*lasterr = (*dev->dev_ops->dev_close)(dev);
> +	if (*lasterr != 0)
> +		lasterr = &binerr;
>  
>  	rte_ethdev_trace_close(port_id);
> -	rte_eth_dev_release_port(dev);
> +	*lasterr = rte_eth_dev_release_port(dev);
> +
> +	return firsterr;

Shouldn't eth_err() be used here?