From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
Received: from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com
 [66.111.4.26]) by dpdk.org (Postfix) with ESMTP id 126962C8
 for <dev@dpdk.org>; Fri,  7 Jul 2017 10:36:19 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id B53592084E;
 Fri,  7 Jul 2017 04:36:18 -0400 (EDT)
Received: from frontend1 ([10.202.2.160])
 by compute1.internal (MEProxy); Fri, 07 Jul 2017 04:36:18 -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=Za9buJcWTIwKUAh
 u2ISIy15LoX+7rB5ToT395v0y9qE=; b=GQ3t5WCHQDfPhChe+wlya6tv7xUekf0
 Rjci+0bFbcqX+sY0MYpsqr0++i5fD4nEbqCgoZg6yWrYM+rIUxLWu8x0lN1B69Fx
 9vWC5JOQlIsviDMq7kjE/bc6V2tv/K12No6mpQyJQWYdcpagDracqspwo7Dk5su/
 oKOLcsJD1lm0=
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=Za9buJcWTIwKUAhu2ISIy15LoX+7rB5ToT395v0y9qE=; b=rsFqRwqz
 bBEIK1SiW+IeBAwGOGOspETfmqPPYJWLUstlqMPBIbyHFkdZwJ+rf5xfzgMCdl29
 9giXceZXTYnJoiOYS3gfjVZs5CW3H6kDeFqqaflPTRDS6LO2EEOS12MAzgiKi8yl
 oM+jZ2igGE309iD9BNS3elymJSKtY8pDtzs65r3RT6OVhvRQRrJxG9i1o2nJB+LY
 TdSUiz+zJ/O6hM9PCyw4Vsy6/3TkIPPdxXnsPwRBGlv01F1J2h6ac6S7sOVI0Mr9
 20Pf+A7zLV2KstSZMW8KugGLxomrC5k/tE/bhq7vHLYH8DNJOKpwqYaOJIJAt6Ww
 AM+Ulafs3e/KoQ==
X-ME-Sender: <xms:AkhfWUAGKwXHJTPMSbIZ1075jhozDvX6rOzJAhOyDzdA8H0jZbMNkg>
X-Sasl-enc: 67K0Cqs1ePyVzd4MMHR+9Mm2zhqMuW/boUNOWFpLcbhr 1499416578
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id 6F0FB7E2C1;
 Fri,  7 Jul 2017 04:36:18 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Wei Dai <wei.dai@intel.com>, ferruh.yigit@intel.com
Cc: dev@dpdk.org, wenzhuo.lu@intel.com, konstantin.ananyev@intel.com,
 jingjing.wu@intel.com, helin.zhang@intel.com, yuan.peng@intel.com
Date: Fri, 07 Jul 2017 10:36:17 +0200
Message-ID: <1715954.GxuFMxG21X@xps>
In-Reply-To: <9394960.Tm5hhKpHD9@xps>
References: <1498748282-69914-1-git-send-email-wei.dai@intel.com>
 <1498817556-64379-3-git-send-email-wei.dai@intel.com>
 <9394960.Tm5hhKpHD9@xps>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [PATCH v5 2/4] net/ixgbe: add support of 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, 07 Jul 2017 08:36:19 -0000

07/07/2017 10:25, Thomas Monjalon:
> Hi,
> 
> 30/06/2017 12:12, Wei Dai:
> > +/*
> > + * Reest PF device.
> > + */
> > +static int
> > +ixgbe_dev_reset(struct rte_eth_dev *dev)
> > +{
> > +       int ret;
> > +
> > +       /* To avoid unexpected behavior in VF, disable PF reset */
> > +       if (dev->data->sriov.active)
> > +               return -ENOTSUP;
> > +
> > +       ret = eth_ixgbe_dev_uninit(dev);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = eth_ixgbe_dev_init(dev);
> > +
> > +       return ret;
> > +}
> 
> rte_eth_dev_reset() just do
> +       rte_eth_dev_stop(port_id);
> +       ret = dev->dev_ops->dev_reset(dev);
> 
> and dev_reset() just do
> +       ret = eth_ixgbe_dev_uninit(dev);
> +       ret = eth_ixgbe_dev_init(dev);
> 
> It is doing one more thing, the check of SR-IOV.
> Unfortunately, this restriction is not documented.
> 
> This is the documentation of the new API:
> 
>  /**
> + * Reset a Ethernet device.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + */
> +int rte_eth_dev_reset(uint8_t port_id);
> 
> It is really really too short.
> From the beginning of this proposal we are asking you to better explain
> why this API is needed. It still does not appear in the doc.
> Are you adding it to offer a new service to DPDK application developpers?
> Or is it just a secret sauce that you will explain only to your customers?
> 
> This is what is expected to be documented:
> - why/when this API must be used
> - what the API will do
> - what is needed to do after

I would like to add that the description of the API must also help
other PMD maintainers to implement it.
Adding a new op means more work for PMD maintainers, that's why
they should understand the benefit and acknowledge it.
Ferruh, as the maintainer of next-net, please could you ask for feedbacks
from other PMD maintainers?