From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH v6 1/4] lib/librte_ether: support device reset Date: Wed, 22 Jun 2016 11:18:21 +0200 Message-ID: <6984459.KfxJSr4bsX@xps13> References: <20160621133041.GA7509@localhost.localdomain> <6749084.arHGZf8DkE@xps13> <6A0DE07E22DDAD4C9103DF62FEBC09090348995A@shsmsx102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev@dpdk.org, "Ananyev, Konstantin" , Stephen Hemminger , "Richardson, Bruce" , "Chen, Jing D" , "Liang, Cunming" , "Wu, Jingjing" , "Zhang, Helin" To: "Lu, Wenzhuo" , Jerin Jacob Return-path: Received: from mail-lf0-f41.google.com (mail-lf0-f41.google.com [209.85.215.41]) by dpdk.org (Postfix) with ESMTP id C5291C3AA for ; Wed, 22 Jun 2016 11:18:26 +0200 (CEST) Received: by mail-lf0-f41.google.com with SMTP id l188so67018081lfe.2 for ; Wed, 22 Jun 2016 02:18:26 -0700 (PDT) In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09090348995A@shsmsx102.ccr.corp.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 2016-06-22 08:25, Lu, Wenzhuo: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > 2016-06-22 13:29, Jerin Jacob: > > > Thomas, > > > As a librte_ether maintainer any comments on this? > > > > +1 for adding details and make sure naming is good. > > I don't really need to comment here because I have already done this comment > > earlier: > > http://dpdk.org/ml/archives/dev/2016-June/041845.html > > Thank you for insisting. > I've add some details in this patch set. If it's not enough, please let me know. > And I think this discussion is about what the API name should be like. Actually I think all the existing name is describing what is done by the API not when and where it should be used, like dev_start/stop. You're right, I overlooked it: + * The API will stop the port, clear the rx/tx queues, re-setup the rx/tx + * queues, restart the port. Jerin, which detail do you think is needed? Wenzhuo, why this function is needed? All these actions are already possible independently. When looking at ixgbe implementation, I see: ixgbevf_dev_stats_reset() which is not documented in the API rte_delay_ms(1000); do {} while It looks to be some hacks. If you really need some workarounds to handle some tricky situations, maybe that the API is not detailed enough. > But anyway I'm open for changing the name. Is the name process_reset_intr you prefer? Thanks. Not sure. If you really intend to add a generic reset, maybe rte_eth_dev_reset() is a good name. We just need more justification. After reading the doc, the user can understand it is just a wrapper of existing functions. But it appears in the code that it does more and can help in some situations.