From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shreyansh Jain Subject: Re: [PATCH 24/38] net/dpaa: add support for Tx and Rx queue setup Date: Fri, 30 Jun 2017 17:18:18 +0530 Message-ID: <7b4c8aa8-39ce-eff0-0124-dcaefc253488@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> <7865dc41-c509-cdec-8c24-c357330f294b@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: , To: Ferruh Yigit Return-path: Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0082.outbound.protection.outlook.com [104.47.41.82]) by dpdk.org (Postfix) with ESMTP id 46F925587 for ; Fri, 30 Jun 2017 13:39:11 +0200 (CEST) In-Reply-To: <7865dc41-c509-cdec-8c24-c357330f294b@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thursday 29 June 2017 09:11 PM, Ferruh Yigit wrote: > 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. Agree, I will fix the documentation and add information about this. > > <...> >>>> +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; >>>> +} >>> >>> <...> >>> >>> >> > >