From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3ADEFA04DB; Fri, 16 Oct 2020 19:13:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 975D4A54B; Fri, 16 Oct 2020 19:13:25 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by dpdk.org (Postfix) with ESMTP id 67B1CA549 for ; Fri, 16 Oct 2020 19:13:24 +0200 (CEST) Received: from [192.168.1.192] (unknown [188.242.181.57]) (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 B14E97F4F3; Fri, 16 Oct 2020 20:13:22 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru B14E97F4F3 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1602868402; bh=D1/HBcRb0IcYsf3jEcbx+dj1zOkY3rIQc+Fj4db8iIo=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=EGoUvGJfCnvOTQwxJnsd892JAYEjb3B0GJlFYI9Tj4Qqx3sdR5BtgWsaJzYeYenMt fwyL9WxsUEh4ofOYMklynCTPvW7ZfohYCVUWDd/XiFU+cNVu66fjTFPLRt6EVzddos 8N2InsAWcdSQbO2nWQISpKr6Czn47kUml3ax0ioA= To: "Kinsella, Ray" , Andrew Rybchenko , Neil Horman , Thomas Monjalon , Ferruh Yigit Cc: dev@dpdk.org, Ivan Ilchenko References: <1602682146-4722-1-git-send-email-arybchenko@solarflare.com> <1602768646-13142-1-git-send-email-arybchenko@solarflare.com> <1602768646-13142-2-git-send-email-arybchenko@solarflare.com> From: Andrew Rybchenko Message-ID: Date: Fri, 16 Oct 2020 20:13:20 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2 01/11] ethdev: change eth dev stop function to return int X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/16/20 2:20 PM, Kinsella, Ray wrote: > > On 15/10/2020 14:30, Andrew Rybchenko wrote: >> From: Ivan Ilchenko >> >> Change rte_eth_dev_stop() return value from void to int >> and return negative errno values in case of error conditions. >> Also update the usage of the function in ethdev according to >> the new return type. >> >> Signed-off-by: Ivan Ilchenko >> Signed-off-by: Andrew Rybchenko >> Acked-by: Thomas Monjalon >> --- >> doc/guides/rel_notes/deprecation.rst | 1 - >> doc/guides/rel_notes/release_20_11.rst | 3 +++ >> lib/librte_ethdev/rte_ethdev.c | 27 +++++++++++++++++++------- >> lib/librte_ethdev/rte_ethdev.h | 5 ++++- >> 4 files changed, 27 insertions(+), 9 deletions(-) >> >> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst >> index d1f5ed39db..2e04e24374 100644 >> --- a/doc/guides/rel_notes/deprecation.rst >> +++ b/doc/guides/rel_notes/deprecation.rst >> @@ -127,7 +127,6 @@ Deprecation Notices >> negative errno values to indicate various error conditions (e.g. >> invalid port ID, unsupported operation, failed operation): >> >> - - ``rte_eth_dev_stop`` >> - ``rte_eth_dev_close`` >> >> * ethdev: New offload flags ``DEV_RX_OFFLOAD_FLOW_MARK`` will be added in 19.11. >> diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst >> index f8686a50db..c8c30937fa 100644 >> --- a/doc/guides/rel_notes/release_20_11.rst >> +++ b/doc/guides/rel_notes/release_20_11.rst >> @@ -355,6 +355,9 @@ API Changes >> * vhost: Add a new function ``rte_vhost_crypto_driver_start`` to be called >> instead of ``rte_vhost_driver_start`` by crypto applications. >> >> +* ethdev: changed ``rte_eth_dev_stop`` return value from ``void`` to >> + ``int`` to provide a way to report various error conditions. >> + >> >> ABI Changes >> ----------- >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >> index d9b82df073..b8cf04ef4d 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -1661,7 +1661,7 @@ rte_eth_dev_start(uint16_t port_id) >> struct rte_eth_dev *dev; >> struct rte_eth_dev_info dev_info; >> int diag; >> - int ret; >> + int ret, ret_stop; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL); >> >> @@ -1695,7 +1695,13 @@ rte_eth_dev_start(uint16_t port_id) >> RTE_ETHDEV_LOG(ERR, >> "Error during restoring configuration for device (port %u): %s\n", >> port_id, rte_strerror(-ret)); >> - rte_eth_dev_stop(port_id); >> + ret_stop = rte_eth_dev_stop(port_id); >> + if (ret_stop != 0) { >> + RTE_ETHDEV_LOG(ERR, >> + "Failed to stop device (port %u): %s\n", >> + port_id, rte_strerror(-ret_stop)); >> + } >> + >> return ret; >> } >> >> @@ -1708,26 +1714,28 @@ rte_eth_dev_start(uint16_t port_id) >> return 0; >> } >> >> -void >> +int >> rte_eth_dev_stop(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> >> - 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_stop); >> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_stop, -ENOTSUP); >> >> if (dev->data->dev_started == 0) { >> RTE_ETHDEV_LOG(INFO, >> "Device with port_id=%"PRIu16" already stopped\n", >> port_id); >> - return; >> + return 0; >> } >> >> dev->data->dev_started = 0; >> (*dev->dev_ops->dev_stop)(dev); >> rte_ethdev_trace_stop(port_id); >> + >> + return 0; >> } >> >> int >> @@ -1783,7 +1791,12 @@ rte_eth_dev_reset(uint16_t port_id) >> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_reset, -ENOTSUP); >> >> - rte_eth_dev_stop(port_id); >> + ret = rte_eth_dev_stop(port_id); >> + if (ret != 0) { >> + RTE_ETHDEV_LOG(ERR, >> + "Failed to stop device (port %u) before reset: %s - ignore\n", >> + port_id, rte_strerror(-ret)); > ABI change is 100%, > Just question the logic of continuing here to do a reset, if you failed to stop the device. In the case of reset I'm sure that we should ignore stop failure here. Typically reset is required to recover from bad state etc and stop failure in such condition could definitely happen.