[dpdk-dev] [PATCH 24/38] net/dpaa: add support for Tx and Rx queue setup

Ferruh Yigit ferruh.yigit at intel.com
Thu Jun 29 17:41:18 CEST 2017


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 at nxp.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain at 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;
>>> +}
>>
>> <...>
>>
>>
> 



More information about the dev mailing list