From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <ferruh.yigit@intel.com>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 51FA12B96
 for <dev@dpdk.org>; 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 <shreyansh.jain@nxp.com>
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>
 <bfc41380-a6d9-107a-b1f2-593ddd9675d6@intel.com>
 <0768ce6e-e23d-0562-fe34-b9e8dd908a45@nxp.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
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 <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: 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 <hemant.agrawal@nxp.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> ---

<...>

>>
>>> +
>>> +	/* 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;
>>> +}
>>
>> <...>
>>
>>
>