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 7693AA04BC; Tue, 29 Sep 2020 18:02:12 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8051A1BED6; Tue, 29 Sep 2020 18:02:10 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7856C1BDC6 for ; Tue, 29 Sep 2020 18:02:07 +0200 (CEST) IronPort-SDR: JLS/IZF8jD5qcF5/2hA6ZELORXU+wWfKM2RUnRFIuELiE8gz5ZJjCU3T6lOg7FEkMbae/AEw5K /HMWNOwH77FA== X-IronPort-AV: E=McAfee;i="6000,8403,9759"; a="159556160" X-IronPort-AV: E=Sophos;i="5.77,319,1596524400"; d="scan'208";a="159556160" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2020 09:02:04 -0700 IronPort-SDR: hXPO0UsK4i5n2ri5Zqm+PfhcbB7h/4EeG28dzi5LpONOkfkQc8uLXUJFb0TgbH5MCH8HCiZnVW u3Vku4B6S6pw== X-IronPort-AV: E=Sophos;i="5.77,319,1596524400"; d="scan'208";a="491893290" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.220.198]) ([10.213.220.198]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Sep 2020 09:02:03 -0700 To: Thomas Monjalon , dev@dpdk.org Cc: arybchenko@solarflare.com References: <20200913220711.3768597-1-thomas@monjalon.net> <20200928231437.414489-1-thomas@monjalon.net> <20200928231437.414489-28-thomas@monjalon.net> From: Ferruh Yigit Message-ID: <7452c24f-b5ad-36b1-7e14-af375f902f75@intel.com> Date: Tue, 29 Sep 2020 17:01:59 +0100 MIME-Version: 1.0 In-Reply-To: <20200928231437.414489-28-thomas@monjalon.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 27/29] ethdev: remove forcing stopped state upon close 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 9/29/2020 12:14 AM, Thomas Monjalon wrote: > When closing a port, it is supposed to be already stopped, > and marked as such with "dev_started" state zeroed. > > Resetting "dev_started" before calling the driver close operation > was hiding the case of not properly stopped port being closed. > The flag "dev_started" is not changed anymore in "rte_eth_dev_close()". > > Signed-off-by: Thomas Monjalon > --- > lib/librte_ethdev/rte_ethdev.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index d7668114ca..0b8e8e3e8d 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1716,7 +1716,6 @@ rte_eth_dev_close(uint16_t port_id) > dev = &rte_eth_devices[port_id]; > > RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_close); > - dev->data->dev_started = 0; > (*dev->dev_ops->dev_close)(dev); > > rte_ethdev_trace_close(port_id); > The driver 'remove()' function may be calling the driver 'stop()' dev_ops internally, so the device will be stopped properly but the 'dev_started' status won't be updated because ethdev API is not called. This API assumes device stopped and updates the state accordingly, it is not good but removing it also won't be good for the case device already stopped. What do you think calling 'rte_eth_dev_stop()' from 'rte_eth_dev_close()'? Although not sure how to handle driver 'remove()' case.