From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 51FA12B96 for ; Thu, 29 Jun 2017 17:41:53 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Jun 2017 08:41:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,281,1496127600"; d="scan'208";a="102792852" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.91]) ([10.237.220.91]) by orsmga004.jf.intel.com with ESMTP; 29 Jun 2017 08:41:18 -0700 To: Shreyansh Jain Cc: dev@dpdk.org, hemant.agrawal@nxp.com References: <1497591668-3320-1-git-send-email-shreyansh.jain@nxp.com> <1497591668-3320-25-git-send-email-shreyansh.jain@nxp.com> <0768ce6e-e23d-0562-fe34-b9e8dd908a45@nxp.com> From: Ferruh Yigit Message-ID: <7865dc41-c509-cdec-8c24-c357330f294b@intel.com> Date: Thu, 29 Jun 2017 16:41:18 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <0768ce6e-e23d-0562-fe34-b9e8dd908a45@nxp.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 24/38] net/dpaa: add support for Tx and Rx queue setup 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: , X-List-Received-Date: Thu, 29 Jun 2017 15:41:54 -0000 On 6/29/2017 3:55 PM, Shreyansh Jain wrote: > On Wednesday 28 June 2017 09:15 PM, Ferruh Yigit wrote: >> On 6/16/2017 6:40 AM, Shreyansh Jain wrote: >>> Signed-off-by: Hemant Agrawal >>> Signed-off-by: Shreyansh Jain >>> --- <...> >> >>> + >>> + /* Initialize Rx FQ's */ >>> + if (getenv("DPAA_NUM_RX_QUEUES")) >> >> I think this was disscussed before, should a PMD get config options from >> enviroment variable? Altough this works, I am for a more explicit >> method, like dev_args. > > Well, I do remember that discussion and still continued with it because > 1) I am not done with that dev_args changes and 2) I think this is more > non-intrusive as this is specific to DPAA without need for expanding it > towards dev_args (and impacting application arg list). > You think this is no-go? If so, I will fix this. Proving argument looks more clear to me, it is more visible, and for example if multiple process will be run, environment variables can be confusing. But this is not no-go, I would like to hear other comments. Also I recognized that mlx and ark drivers are also using this. But however this is implemented, this should be clearly documented, right now this is a hidden config. <...> >>> +uint16_t dpaa_eth_tx_drop_all(void *q __rte_unused, >>> + struct rte_mbuf **bufs __rte_unused, >>> + uint16_t nb_bufs __rte_unused) >>> +{ >>> + PMD_TX_LOG(DEBUG, "Drop all packets"); >> >> Should mbufs freed here? >> >>> + >>> + /* Drop all incoming packets. No need to free packets here >>> + * because the rte_eth f/w frees up the packets through tx_buffer >>> + * callback in case this functions returns count less than nb_bufs >>> + */ > > Ah, actually I was banking on logic that in case a driver doesn't > release memory, the API caller (on getting less than nb_bufs) would do > that. This is case for stopped interface. > > But, I agree, this is dirty fix. I will change this. I missed your logic here indeed, this looks a valid option too, its your call. > >>> + return 0; >>> +} >> >> <...> >> >> >