Re: [dpdk-dev] [PATCH v2 1/2] pmd: Modified dev_info structure to include default RX/TX configuration

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Oct 01, 2014 at 09:00:57AM +0100, Pablo de Lara wrote:
> Many sample apps use duplicated code to set rte_eth_txconf and rte_eth_rxconf
> structures. This patch allows the user to get a default optimal RX/TX configuration
> through rte_eth_dev_info get, and still any parameters may be tweaked as wished,
> before setting up queues.
> 
> Besides, if a NULL pointer is passed to rte_eth_rx_queue_setup or
> rte_eth_tx_queue_setup, these functions get internally the default RX/TX
> configuration for the user.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c       |   28 +++++++++++++++++++++-----
>  lib/librte_ether/rte_ethdev.h       |    2 +
>  lib/librte_pmd_e1000/igb_ethdev.c   |   32 ++++++++++++++++++++++++++++++-
>  lib/librte_pmd_i40e/i40e_ethdev.c   |   33 ++++++++++++++++++++++++++++++++
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c |   36 +++++++++++++++++++++++++++++++++++
>  5 files changed, 124 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index fd1010a..2f9eea2 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -928,6 +928,7 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  	struct rte_eth_dev *dev;
>  	struct rte_pktmbuf_pool_private *mbp_priv;
>  	struct rte_eth_dev_info dev_info;
> +	const struct rte_eth_rxconf *conf;
>  
>  	/* This function is only safe when called from the primary process
>  	 * in a multi-process setup*/
> @@ -937,6 +938,7 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
>  		return (-EINVAL);
>  	}
> +
>  	dev = &rte_eth_devices[port_id];
>  	if (rx_queue_id >= dev->data->nb_rx_queues) {
>  		PMD_DEBUG_TRACE("Invalid RX queue_id=%d\n", rx_queue_id);
> @@ -980,8 +982,12 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  		return (-EINVAL);
>  	}
>  
> +	conf = rx_conf;
> +	if (conf == NULL)
> +		conf = &dev_info.default_rxconf;
> +
>  	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
> -					      socket_id, rx_conf, mp);
> +					      socket_id, conf, mp);
>  	if (!ret) {
>  		if (!dev->data->min_rx_buf_size ||
>  		    dev->data->min_rx_buf_size > mbp_buf_size)
> @@ -997,6 +1003,8 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
>  		       const struct rte_eth_txconf *tx_conf)
>  {
>  	struct rte_eth_dev *dev;
> +	struct rte_eth_dev_info dev_info;
> +	const struct rte_eth_txconf *conf;
>  
>  	/* This function is only safe when called from the primary process
>  	 * in a multi-process setup*/
> @@ -1006,6 +1014,7 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
>  		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
>  		return (-EINVAL);
>  	}
> +
>  	dev = &rte_eth_devices[port_id];
>  	if (tx_queue_id >= dev->data->nb_tx_queues) {
>  		PMD_DEBUG_TRACE("Invalid TX queue_id=%d\n", tx_queue_id);
> @@ -1018,9 +1027,17 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
>  		return -EBUSY;
>  	}
>  
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
>  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_queue_setup, -ENOTSUP);
> +
> +	(*dev->dev_ops->dev_infos_get)(dev, &dev_info);
> +
> +	conf = tx_conf;
> +	if (conf == NULL)
> +		conf = &dev_info.default_txconf;
> +
>  	return (*dev->dev_ops->tx_queue_setup)(dev, tx_queue_id, nb_tx_desc,
> -					       socket_id, tx_conf);
> +					       socket_id, conf);
>  }
>  
>  void
> @@ -1249,10 +1266,9 @@ rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info)
>  	}
>  	dev = &rte_eth_devices[port_id];
>  
> -	/* Default device offload capabilities to zero */
> -	dev_info->rx_offload_capa = 0;
> -	dev_info->tx_offload_capa = 0;
> -	dev_info->if_index = 0;
> +	/* Reset dev info structure */
> +	memset(dev_info, 0, sizeof(struct rte_eth_dev_info));
> +
>  	FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get);
>  	(*dev->dev_ops->dev_infos_get)(dev, dev_info);
>  	dev_info->pci_dev = dev->pci_dev;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 50df654..f63d38a 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -906,6 +906,8 @@ struct rte_eth_dev_info {
>  	uint16_t max_vmdq_pools; /**< Maximum number of VMDq pools. */
>  	uint32_t rx_offload_capa; /**< Device RX offload capabilities. */
>  	uint32_t tx_offload_capa; /**< Device TX offload capabilities. */
> +	struct rte_eth_rxconf default_rxconf; /**< Default RX configuration */
> +	struct rte_eth_txconf default_txconf; /**< Default TX configuration */
>  };
>  
>  struct rte_eth_dev;

In this file (rte_ethdev.h), you also need to update the API documentation 
for the rte_eth_rx_queue_setup and rte_eth_tx_queue_setup functions to state 
that passing in a NULL value for the conf parameter is allowed, and that it 
just means to use the defaults.

/Bruce