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 AFA91A04B7;
	Tue, 13 Oct 2020 19:54:16 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 117BE1DBC8;
	Tue, 13 Oct 2020 19:54:15 +0200 (CEST)
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 9C4EB1D941
 for <dev@dpdk.org>; Tue, 13 Oct 2020 19:54:12 +0200 (CEST)
IronPort-SDR: uq7rmAAMB35JiEvegCYarkiEALR2JPQdV8A/I6DbiV3xcAHpUGivQB5uMAUCeYG8NgARXHiOuj
 dvJgWY2t63nw==
X-IronPort-AV: E=McAfee;i="6000,8403,9773"; a="165165286"
X-IronPort-AV: E=Sophos;i="5.77,371,1596524400"; d="scan'208";a="165165286"
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga005.jf.intel.com ([10.7.209.41])
 by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 13 Oct 2020 10:54:11 -0700
IronPort-SDR: 8f2rYJ7+tp4KtpevOsF/6t1iFmGM20RxiqlOLBLurjOrVssnAuqk1iAzRDQ0fhcttTCJtrjToV
 6U3+kxvFRnsA==
X-IronPort-AV: E=Sophos;i="5.77,371,1596524400"; d="scan'208";a="530499511"
Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.214.64])
 ([10.213.214.64])
 by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
 13 Oct 2020 10:54:06 -0700
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
 Steven Webster <steven.webster@windriver.com>,
 Matt Peters <matt.peters@windriver.com>,
 Somalapuram Amaranath <asomalap@amd.com>, Rasesh Mody <rmody@marvell.com>,
 Shahed Shaikh <shshaikh@marvell.com>,
 Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
 Hemant Agrawal <hemant.agrawal@nxp.com>,
 Sachin Saxena <sachin.saxena@oss.nxp.com>, John Daley <johndale@cisco.com>,
 Hyong Youb Kim <hyonkim@cisco.com>, Qiming Yang <qiming.yang@intel.com>,
 Qi Zhang <qi.z.zhang@intel.com>, Alfredo Cardigliano <cardigliano@ntop.org>,
 Rosen Xu <rosen.xu@intel.com>, Matan Azrad <matan@nvidia.com>,
 Shahaf Shuler <shahafs@nvidia.com>,
 Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
 Liron Himi <lironh@marvell.com>, Heinrich Kuhn
 <heinrich.kuhn@netronome.com>, Harman Kalra <hkalra@marvell.com>,
 Jerin Jacob <jerinj@marvell.com>, Nithin Dabilpuram
 <ndabilpuram@marvell.com>, Kiran Kumar K <kirankumark@marvell.com>
References: <20200913220711.3768597-1-thomas@monjalon.net>
 <20201013100634.2482593-2-thomas@monjalon.net>
 <500095bc-470e-1bce-05d2-6fa67437dddd@intel.com> <4842858.jpJ4lIlKDh@thomas>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <66feb9dc-d08c-e01b-40c6-05dca3856c2f@intel.com>
Date: Tue, 13 Oct 2020 18:54:02 +0100
MIME-Version: 1.0
In-Reply-To: <4842858.jpJ4lIlKDh@thomas>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-dev] [PATCH v5 1/3] 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 <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/2020 1:51 PM, Thomas Monjalon wrote:
> 13/10/2020 14:45, Ferruh Yigit:
>> On 10/13/2020 11:06 AM, Thomas Monjalon wrote:
>>> When closing a port, it is supposed to be already stopped,
>>> and marked as such with "dev_started" state zeroed by the stop API.
>>>
>>> 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()".
>>>
>>> In case the "dev_stop" function is called from "dev_close",
>>> bypassing "rte_eth_dev_stop()" API,
>>> the "dev_started" state must be explicitly reset in the PMD
>>> in order to keep the same behaviour.
>>>
>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
>>> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> [...]
>> Following non-virtual PMDs doesn't call 'dev_stop()' from 'dev_close()',
>> maintainers of the PMDs cc'ed.
>>
>> avp
>> axgbe
>> bnx2x
>> cxgbe
>> dpaa2
>> enic
>> ice_dcf
>> ionic
>> ipn3ke
>> mlx4
>> mlx5
>> mvpp2
>> nfp
>> octeontx
>> octeontx2
>> sfc
>>
>> Can you please double check for your driver:
>>
>> 1) Device stopped properly before it has been closed?
>>
>> 2) The device stopped state ('dev->data->dev_started') is correct if device
>> closed without explicitly stopped first.
> 
> The application is supposed to call stop before close,
> so I don't know what has to be checked.
> The automatic stop in close is an extra feature per PMD choice.
> 

A device can be hot-removed without stop, that is why I was thinking 'close()' 
should stop the device (if it is not already).

Even current 'rte_eth_dev_close()' API doesn't check if the port stopped or not.

If application does stop -> close/remove in order, that would work, but for the 
case it didn't, PMD ensuring the device stopped before close is safer.