[dpdk-dev,v3,2/3] pmd: Modified dev_info structure to include default RX/TX configuration

Message ID 1412156945-6549-3-git-send-email-pablo.de.lara.guarch@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

De Lara Guarch, Pablo Oct. 1, 2014, 9:49 a.m. UTC
  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       |   11 ++++++++++
 lib/librte_ether/rte_ethdev.h       |    6 +++++
 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, 117 insertions(+), 1 deletions(-)
  

Comments

Thomas Monjalon Oct. 10, 2014, 11:14 a.m. UTC | #1
Pablo, I have some minor nits on this patchset.
If you agree, I can fix them before applying the patchset.

2014-10-01 10:49, Pablo de Lara:
> +/**
> + *  Default values for port configuration
> + */
> +#define IGB_DEFAULT_RX_FREE_THRESH  32
> +#define IGB_DEFAULT_RX_PTHRESH      8
> +#define IGB_DEFAULT_RX_HTHRESH      8
> +#define IGB_DEFAULT_RX_WTHRESH      0
> +
> +#define IGB_DEFAULT_TX_PTHRESH      32
> +#define IGB_DEFAULT_TX_HTHRESH      0
> +#define IGB_DEFAULT_TX_WTHRESH      0

Comments in .c files are not for doxygen. So /** should be replaced by /*

>  static int eth_igb_get_5tuple_filter(struct rte_eth_dev *dev,
>  			uint16_t index,
>  			struct rte_5tuple_filter *filter, uint16_t *rx_queue);
> -
>  /*
>   * Define VF Stats MACRO for Non "cleared on read" register
>   */

This blank line shouldn't be removed. Probably a typo?
  
De Lara Guarch, Pablo Oct. 10, 2014, 12:05 p.m. UTC | #2
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, October 10, 2014 12:14 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 2/3] pmd: Modified dev_info structure to
> include default RX/TX configuration
> 
> Pablo, I have some minor nits on this patchset.
> If you agree, I can fix them before applying the patchset.

Sure, those changes seem reasonable.

Many thanks,
Pablo
> 
> 2014-10-01 10:49, Pablo de Lara:
> > +/**
> > + *  Default values for port configuration
> > + */
> > +#define IGB_DEFAULT_RX_FREE_THRESH  32
> > +#define IGB_DEFAULT_RX_PTHRESH      8
> > +#define IGB_DEFAULT_RX_HTHRESH      8
> > +#define IGB_DEFAULT_RX_WTHRESH      0
> > +
> > +#define IGB_DEFAULT_TX_PTHRESH      32
> > +#define IGB_DEFAULT_TX_HTHRESH      0
> > +#define IGB_DEFAULT_TX_WTHRESH      0
> 
> Comments in .c files are not for doxygen. So /** should be replaced by /*
> 
> >  static int eth_igb_get_5tuple_filter(struct rte_eth_dev *dev,
> >  			uint16_t index,
> >  			struct rte_5tuple_filter *filter, uint16_t *rx_queue);
> > -
> >  /*
> >   * Define VF Stats MACRO for Non "cleared on read" register
> >   */
> 
> This blank line shouldn't be removed. Probably a typo?
> 
> --
> Thomas
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 3c09a23..ad4b973 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -980,6 +980,9 @@  rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 		return (-EINVAL);
 	}
 
+	if (rx_conf == NULL)
+		rx_conf = &dev_info.default_rxconf;
+
 	ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
 					      socket_id, rx_conf, mp);
 	if (!ret) {
@@ -997,6 +1000,7 @@  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;
 
 	/* This function is only safe when called from the primary process
 	 * in a multi-process setup*/
@@ -1018,7 +1022,14 @@  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);
+
+	if (tx_conf == NULL)
+		tx_conf = &dev_info.default_txconf;
+
 	return (*dev->dev_ops->tx_queue_setup)(dev, tx_queue_id, nb_tx_desc,
 					       socket_id, tx_conf);
 }
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 50df654..1dff97e 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;
@@ -1693,6 +1695,8 @@  extern int rte_eth_dev_configure(uint8_t port_id,
  *   the DMA memory allocated for the receive descriptors of the ring.
  * @param rx_conf
  *   The pointer to the configuration data to be used for the receive queue.
+ *   NULL value is allowed, in which case default RX configuration
+ *   will be used.
  *   The *rx_conf* structure contains an *rx_thresh* structure with the values
  *   of the Prefetch, Host, and Write-Back threshold registers of the receive
  *   ring.
@@ -1730,6 +1734,8 @@  extern int rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
  *   the DMA memory allocated for the transmit descriptors of the ring.
  * @param tx_conf
  *   The pointer to the configuration data to be used for the transmit queue.
+ *   NULL value is allowed, in which case default RX configuration
+ *   will be used.
  *   The *tx_conf* structure contains the following data:
  *   - The *tx_thresh* structure with the values of the Prefetch, Host, and
  *     Write-Back threshold registers of the transmit ring.
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 3187d92..4bf75f5 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -57,6 +57,18 @@ 
 #include "e1000/e1000_api.h"
 #include "e1000_ethdev.h"
 
+/**
+ *  Default values for port configuration
+ */
+#define IGB_DEFAULT_RX_FREE_THRESH  32
+#define IGB_DEFAULT_RX_PTHRESH      8
+#define IGB_DEFAULT_RX_HTHRESH      8
+#define IGB_DEFAULT_RX_WTHRESH      0
+
+#define IGB_DEFAULT_TX_PTHRESH      32
+#define IGB_DEFAULT_TX_HTHRESH      0
+#define IGB_DEFAULT_TX_WTHRESH      0
+
 static int  eth_igb_configure(struct rte_eth_dev *dev);
 static int  eth_igb_start(struct rte_eth_dev *dev);
 static void eth_igb_stop(struct rte_eth_dev *dev);
@@ -165,7 +177,6 @@  static int eth_igb_remove_5tuple_filter(struct rte_eth_dev *dev,
 static int eth_igb_get_5tuple_filter(struct rte_eth_dev *dev,
 			uint16_t index,
 			struct rte_5tuple_filter *filter, uint16_t *rx_queue);
-
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
  */
@@ -1331,6 +1342,25 @@  eth_igb_infos_get(struct rte_eth_dev *dev,
 		dev_info->max_tx_queues = 0;
 		dev_info->max_vmdq_pools = 0;
 	}
+
+	dev_info->default_rxconf = (struct rte_eth_rxconf) {
+		.rx_thresh = {
+			.pthresh = IGB_DEFAULT_RX_PTHRESH,
+			.hthresh = IGB_DEFAULT_RX_HTHRESH,
+			.wthresh = IGB_DEFAULT_RX_WTHRESH,
+		},
+		.rx_free_thresh = IGB_DEFAULT_RX_FREE_THRESH,
+		.rx_drop_en = 0,
+	};
+
+	dev_info->default_txconf = (struct rte_eth_txconf) {
+		.tx_thresh = {
+			.pthresh = IGB_DEFAULT_TX_PTHRESH,
+			.hthresh = IGB_DEFAULT_TX_HTHRESH,
+			.wthresh = IGB_DEFAULT_TX_WTHRESH,
+		},
+		.txq_flags = 0,
+	};
 }
 
 /* return 0 means link status changed, -1 means not changed */
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 9ed31b5..72faf95 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -58,6 +58,17 @@ 
 #include "i40e_rxtx.h"
 #include "i40e_pf.h"
 
+#define I40E_DEFAULT_RX_FREE_THRESH  32
+#define I40E_DEFAULT_RX_PTHRESH      8
+#define I40E_DEFAULT_RX_HTHRESH      8
+#define I40E_DEFAULT_RX_WTHRESH      0
+
+#define I40E_DEFAULT_TX_FREE_THRESH  32
+#define I40E_DEFAULT_TX_PTHRESH      32
+#define I40E_DEFAULT_TX_HTHRESH      0
+#define I40E_DEFAULT_TX_WTHRESH      0
+#define I40E_DEFAULT_TX_RSBIT_THRESH 32
+
 /* Maximun number of MAC addresses */
 #define I40E_NUM_MACADDR_MAX       64
 #define I40E_CLEAR_PXE_WAIT_MS     200
@@ -1231,6 +1242,28 @@  i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_TX_OFFLOAD_UDP_CKSUM |
 		DEV_TX_OFFLOAD_TCP_CKSUM |
 		DEV_TX_OFFLOAD_SCTP_CKSUM;
+
+	dev_info->default_rxconf = (struct rte_eth_rxconf) {
+		.rx_thresh = {
+			.pthresh = I40E_DEFAULT_RX_PTHRESH,
+			.hthresh = I40E_DEFAULT_RX_HTHRESH,
+			.wthresh = I40E_DEFAULT_RX_WTHRESH,
+		},
+		.rx_free_thresh = I40E_DEFAULT_RX_FREE_THRESH,
+		.rx_drop_en = 0,
+	};
+
+	dev_info->default_txconf = (struct rte_eth_txconf) {
+		.tx_thresh = {
+			.pthresh = I40E_DEFAULT_TX_PTHRESH,
+			.hthresh = I40E_DEFAULT_TX_HTHRESH,
+			.wthresh = I40E_DEFAULT_TX_WTHRESH,
+		},
+		.tx_free_thresh = I40E_DEFAULT_TX_FREE_THRESH,
+		.tx_rs_thresh = I40E_DEFAULT_TX_RSBIT_THRESH,
+		.txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS | ETH_TXQ_FLAGS_NOOFFLOADS,
+	};
+
 }
 
 static int
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 59122a1..3b1c156 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -92,6 +92,20 @@ 
 #define IXGBE_MMW_SIZE_DEFAULT        0x4
 #define IXGBE_MMW_SIZE_JUMBO_FRAME    0x14
 
+/**
+ *  Default values for RX/TX configuration
+ */
+#define IXGBE_DEFAULT_RX_FREE_THRESH  32
+#define IXGBE_DEFAULT_RX_PTHRESH      8
+#define IXGBE_DEFAULT_RX_HTHRESH      8
+#define IXGBE_DEFAULT_RX_WTHRESH      0
+
+#define IXGBE_DEFAULT_TX_FREE_THRESH  32
+#define IXGBE_DEFAULT_TX_PTHRESH      32
+#define IXGBE_DEFAULT_TX_HTHRESH      0
+#define IXGBE_DEFAULT_TX_WTHRESH      0
+#define IXGBE_DEFAULT_TX_RSBIT_THRESH 32
+
 #define IXGBEVF_PMD_NAME "rte_ixgbevf_pmd" /* PMD name */
 
 #define IXGBE_QUEUE_STAT_COUNTERS (sizeof(hw_stats->qprc) / sizeof(hw_stats->qprc[0]))
@@ -1948,6 +1962,28 @@  ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_TX_OFFLOAD_UDP_CKSUM   |
 		DEV_TX_OFFLOAD_TCP_CKSUM   |
 		DEV_TX_OFFLOAD_SCTP_CKSUM;
+
+	dev_info->default_rxconf = (struct rte_eth_rxconf) {
+			.rx_thresh = {
+				.pthresh = IXGBE_DEFAULT_RX_PTHRESH,
+				.hthresh = IXGBE_DEFAULT_RX_HTHRESH,
+				.wthresh = IXGBE_DEFAULT_RX_WTHRESH,
+			},
+			.rx_free_thresh = IXGBE_DEFAULT_RX_FREE_THRESH,
+			.rx_drop_en = 0,
+	};
+
+
+	 dev_info->default_txconf = (struct rte_eth_txconf) {
+			.tx_thresh = {
+				.pthresh = IXGBE_DEFAULT_TX_PTHRESH,
+				.hthresh = IXGBE_DEFAULT_TX_HTHRESH,
+				.wthresh = IXGBE_DEFAULT_TX_WTHRESH,
+			},
+			.tx_free_thresh = IXGBE_DEFAULT_TX_FREE_THRESH,
+			.tx_rs_thresh = IXGBE_DEFAULT_TX_RSBIT_THRESH,
+			.txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS | ETH_TXQ_FLAGS_NOOFFLOADS,
+	};
 }
 
 /* return 0 means link status changed, -1 means not changed */