From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 1/4] ethdev: add support for PMD-tuned
	Tx/Rx parameters
Date: Fri, 30 Mar 2018 17:40:08 +0200
Message-ID: <1867154.rqMg12d5dS@xps>
References: <20180321142749.27520-1-remy.horton@intel.com>
 <20180321142749.27520-2-remy.horton@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, John McNamara <john.mcnamara@intel.com>,
 Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
 Qi Zhang <qi.z.zhang@intel.com>, Beilei Xing <beilei.xing@intel.com>,
 Shreyansh Jain <shreyansh.jain@nxp.com>, ferruh.yigit@intel.com
To: Remy Horton <remy.horton@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id 62E267CFC
 for <dev@dpdk.org>; Fri, 30 Mar 2018 17:40:41 +0200 (CEST)
In-Reply-To: <20180321142749.27520-2-remy.horton@intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

I know that this patch is already applied,
but I have some comments below.
Please try to address them.

21/03/2018 15:27, Remy Horton:
> +	/* If number of queues specified by application for both Rx and Tx is
> +	 * zero, use driver preferred values. This cannot be done individually
> +	 * as it is valid for either Tx or Rx (but not both) to be zero.
> +	 * If driver does not provide any preferred valued, fall back on
> +	 * EAL defaults.
> +	 */

The fallback could be set in rte_eth_dev_info_get() and call this function
instead of calling directly dev_ops->dev_infos_get.
It would require to never return -ENOTSUP.

At least, some default values should be set in rte_eth_dev_info_get().

> @@ -1029,6 +1048,10 @@ struct rte_eth_dev_info {
>  	/** Configured number of rx/tx queues */
>  	uint16_t nb_rx_queues; /**< Number of RX queues. */
>  	uint16_t nb_tx_queues; /**< Number of TX queues. */
> +
> +	/** Tx/Rx parameter recommendations */
> +	struct rte_eth_dev_portconf default_rxportconf;
> +	struct rte_eth_dev_portconf default_txportconf;

Some formatting comments:
Usually, there is no blank line in structs.
The doxygen comment must apply to each field.
Have you checked what is the doxygen output?