From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 7731A2C08
 for <dev@dpdk.org>; Fri, 21 Apr 2017 10:27:51 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id 3D25320C66;
 Fri, 21 Apr 2017 04:27:51 -0400 (EDT)
Received: from frontend2 ([10.202.2.161])
 by compute1.internal (MEProxy); Fri, 21 Apr 2017 04:27:51 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 cc:content-transfer-encoding:content-type:date:from:in-reply-to
 :message-id:mime-version:references:subject:to:x-me-sender
 :x-me-sender:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=00Ri2Hll91Nkfug
 w4EA+9emt0Y5mjtv5utoaI8KiN8w=; b=QChYTX5vbyd1KXb/p2patdD9WlhyEsL
 FFjX3O/W2tkOZXr/47fonmgj5yI5N+e0OIvze0YXQw9iHhwYz+e/yJdxxNX3bN1U
 75R+3gp0Y7GXgjtzEdwzbH0neVwKRqVyo5QNAnoktIbjHbRoLDS4ThpvYfe1tsh9
 i//6B+HE4WJM=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:content-type
 :date:from:in-reply-to:message-id:mime-version:references
 :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s=
 fm1; bh=00Ri2Hll91Nkfugw4EA+9emt0Y5mjtv5utoaI8KiN8w=; b=D8+PsmPe
 z5/f2wMYf+/1L6NadYWysF9Qqnhb4nFbFX1lbVfTRcUSxC7KGNyDshrlERLsH8DB
 RiV6F03syLj2iJxm0s7igclUMFWVDLpYbsfpB6hvRBbKwPds0/NOt7F+G476n0uW
 MzIkDToL6UCUl3v6SnDceTHYFjOSHU5MguK1pK/7hwc+rmlhrKT89Z8LYP3gkmXM
 DEI9/JO7vhXA9iVUv2T5LCww5AcJPrl/j+PX2CGDVoJaaGcVKPnQvtW1Obhb3Gfy
 ILj0/tXSLOizHYrsf7rFj15RPdsJv8NVMf90eWeToR9lGkZiUjmcuOn/DlC50o7v
 tUWoU72Ftmp1aw==
X-ME-Sender: <xms:h8L5WPh5_vbpJoq71Fhve3MjGYO2_AnMKG0Vq9rZetUqC1ewL0xzCw>
X-Sasl-enc: bBfrFIS7WN8iYzMcigHieXXZGbIhHYoo2VnlXXC8Afhe 1492763270
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id E0862240A5;
 Fri, 21 Apr 2017 04:27:50 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>, "Zhao1,
 Wei" <wei.zhao1@intel.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "Mcnamara,
 John" <john.mcnamara@intel.com>, dev@dpdk.org, "Lu,
 Wenzhuo" <wenzhuo.lu@intel.com>, "Liu, Yu Y" <yu.y.liu@intel.com>
Date: Fri, 21 Apr 2017 10:27:49 +0200
Message-ID: <1581788.2CBIGuQB5e@xps>
In-Reply-To: <20170421022747.GA11512@yliu-dev.sh.intel.com>
References: <1490866456-52241-1-git-send-email-wei.zhao1@intel.com>
 <A2573D2ACFCADC41BB3BE09C6DE313CA0207FFD0@PGSMSX103.gar.corp.intel.com>
 <20170421022747.GA11512@yliu-dev.sh.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v4 1/3] lib/librte_ether: add support for
	port reset
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 21 Apr 2017 08:27:52 -0000

21/04/2017 04:27, Yuanhan Liu:
> On Thu, Apr 20, 2017 at 09:17:24AM +0000, Zhao1, Wei wrote:
> > > > > Please explain exactly the responsibility of this function, and how
> > > > > it is different from calling stop/configure/start.
> > > > 
> > > > In this reset feature, reset function can do the calling
> > > > stop/configure/start process, but also It can also do some restore
> > > > work for the port, for example, it can restore the added parameters 
> > > > of
> > > 
> > > vlan,  mac_addrs, promisc_unicast_enabled falg and
> > > promisc_multicast_enabled flag.
> > > 
> > > > Maybe , I should add this explanation in the patch comments or
> > > > function
> > > 
> > > comments?
> > > 
> > > I'm curious why we have to do save & restore for a reset operation.
> > > Why some configures have to be saved and restored? Doesn't "reset"
> > > literally means reset everything?
> > 
> > Users maybe do not want to do a second configuration operation to waste
> > time after reset which lost all previous configuration. But he still want
> > these configuration valid after reset.
> > So, save & restore can help them to save this process time and effort.
> > 
> > > Even though, how do you tell what kind of configures need be restored
> > > and
> > > what should not? Again, even though, will all PMDs supports restoring
> > > the
> > > same set of configurations?
> > 
> > Yes, this is hard to say what may be need and what may be not for user.
> > Now, the kinds of supported is list in patch set comment. And only i40e
> > NIC support this feature.
> Why it's the configurations listed in patch 2? Because they are requested
> by customers?
> 
> Is that all could be saved? If not, are you going to save & restore all
> possible configurations?
> 
> Assuming the configurations saved & restored may differ from different
> PMD drivers, how could you keep the consistency then? And judging that the
> application has no idea about the difference (yet it knows nothing about
> what kind of configurations will be saved), how could the application know
> that some configurations are not saved & restored by the driver that it
> has to do re-configuration by itself?
> 
> > > While looking at your reset implementation for i40e, it looks more
> > > complex
> > > than necessary: just thinking we have to call "xxx_queue_setup"
> > > for all PMDs.
> > > 
> > > I'm thinking a simple hardware reset might be enough?
> > > 
> > >     /* literally reset the hardware: reset everything */
> > >     rte_eth_reset(port)
> > >     {
> > >     
> > >     	eth_dev->ops->reset();
> > >     
> > >     }
> > 
> > You mean just do a reset  and do not restore any configuration?
> > That may not meet the need for this feature from customer?
> 
> Right, I'm just aware of the configuration might be done by PF (but not
> only by the application), that the VF port may be not aware of those
> configurations. So the save & restore is needed. I don't quite like
> how it is done in this patch set though. I also don't think the API is
> well named: as said, reset should literally reset everything.
> 
> We may need think how to do it properly.
> 
> Thomas, Konstantin, what do you guys think of it?

I have the same concerns.
I think we should better document the current status of start/stop
and which configuration parameters are lost or saved.