All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lib: fix DCB config issue on ixgbe
@ 2016-04-11  8:24 Wenzhuo Lu
  2016-04-11  9:52 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Wenzhuo Lu @ 2016-04-11  8:24 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

An issue is found that DCB cannot be configged on ixgbe
NICs. It's said the TX queue number is not right.
On ixgbe the max TX queue number is not fixed, it depends
on the multi-queue mode. The API rte_eth_dev_configure
should be used to config this mode. But the input of this
API includes TX queue number. The problem is before the
mode is configged, we cannot decide the TX queue number.

This patch adds an API to config RX & TX multi-queue mode
separately. After the mode is configged, the max RX & TX
queue number is decided. Then we can set the appropriate
RX & TX queue number.

Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/testpmd.c                 | 40 +++++++++++++++++++---------------
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 19 ++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  1 +
 4 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1398c6c..c726e1c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1925,17 +1925,31 @@ init_port_dcb_config(portid_t pid,
 		     uint8_t pfc_en)
 {
 	struct rte_eth_conf port_conf;
-	struct rte_eth_dev_info dev_info;
 	struct rte_port *rte_port;
 	int retval;
 	uint16_t i;
 
-	rte_eth_dev_info_get(pid, &dev_info);
+	rte_port = &ports[pid];
+
+	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
+	/* Enter DCB configuration status */
+	dcb_config = 1;
+
+	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
+	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
+	if (retval < 0)
+		return retval;
+
+	rte_eth_dev_mq_mode_set(pid,
+				port_conf.rxmode.mq_mode,
+				port_conf.txmode.mq_mode);
+	rte_eth_dev_info_get(pid, &rte_port->dev_info);
 
 	/* If dev_info.vmdq_pool_base is greater than 0,
 	 * the queue id of vmdq pools is started after pf queues.
 	 */
-	if (dcb_mode == DCB_VT_ENABLED && dev_info.vmdq_pool_base > 0) {
+	if (dcb_mode == DCB_VT_ENABLED &&
+	    rte_port->dev_info.vmdq_pool_base > 0) {
 		printf("VMDQ_DCB multi-queue mode is nonsensical"
 			" for port %d.", pid);
 		return -1;
@@ -1945,13 +1959,13 @@ init_port_dcb_config(portid_t pid,
 	 * and has the same number of rxq and txq in dcb mode
 	 */
 	if (dcb_mode == DCB_VT_ENABLED) {
-		nb_rxq = dev_info.max_rx_queues;
-		nb_txq = dev_info.max_tx_queues;
+		nb_rxq = rte_port->dev_info.max_rx_queues;
+		nb_txq = rte_port->dev_info.max_tx_queues;
 	} else {
 		/*if vt is disabled, use all pf queues */
-		if (dev_info.vmdq_pool_base == 0) {
-			nb_rxq = dev_info.max_rx_queues;
-			nb_txq = dev_info.max_tx_queues;
+		if (rte_port->dev_info.vmdq_pool_base == 0) {
+			nb_rxq = rte_port->dev_info.max_rx_queues;
+			nb_txq = rte_port->dev_info.max_tx_queues;
 		} else {
 			nb_rxq = (queueid_t)num_tcs;
 			nb_txq = (queueid_t)num_tcs;
@@ -1960,16 +1974,6 @@ init_port_dcb_config(portid_t pid,
 	}
 	rx_free_thresh = 64;
 
-	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
-	/* Enter DCB configuration status */
-	dcb_config = 1;
-
-	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
-	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
-	if (retval < 0)
-		return retval;
-
-	rte_port = &ports[pid];
 	memcpy(&rte_port->dev_conf, &port_conf, sizeof(struct rte_eth_conf));
 
 	rxtx_port_config(rte_port);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a31018e..b4eaa29 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3369,3 +3369,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	dev->data->dev_conf.rxmode.mq_mode = rx_mq_mode;
+	dev->data->dev_conf.txmode.mq_mode = tx_mq_mode;
+
+	return 0;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 022733e..852acca 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -4279,6 +4279,25 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				  uint32_t mask,
 				  uint8_t en);
 
+/**
+ * Set RX & TX multi_queue mode.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param rx_mq_mode
+ *   RX multi_queue mode.
+ * @param tx_mq_mode
+ *   TX multi_queue mode.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ */
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 214ecc7..8adea31 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -130,5 +130,6 @@ DPDK_16.04 {
 	rte_eth_tx_buffer_drop_callback;
 	rte_eth_tx_buffer_init;
 	rte_eth_tx_buffer_set_err_callback;
+	rte_eth_dev_mq_mode_set;
 
 } DPDK_2.2;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] lib: fix DCB config issue on ixgbe
  2016-04-11  8:24 [PATCH] lib: fix DCB config issue on ixgbe Wenzhuo Lu
@ 2016-04-11  9:52 ` Thomas Monjalon
  2016-04-12  0:39   ` Lu, Wenzhuo
  2016-05-04 21:47 ` [PATCH v2] " Wenzhuo Lu
  2016-05-05 21:33 ` [PATCH v3] ethdev: " Wenzhuo Lu
  2 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2016-04-11  9:52 UTC (permalink / raw)
  To: Wenzhuo Lu; +Cc: dev

2016-04-11 16:24, Wenzhuo Lu:
> An issue is found that DCB cannot be configged on ixgbe
> NICs. It's said the TX queue number is not right.
> On ixgbe the max TX queue number is not fixed, it depends
> on the multi-queue mode. The API rte_eth_dev_configure
> should be used to config this mode. But the input of this
> API includes TX queue number. The problem is before the
> mode is configged, we cannot decide the TX queue number.
> 
> This patch adds an API to config RX & TX multi-queue mode
> separately. After the mode is configged, the max RX & TX
> queue number is decided. Then we can set the appropriate
> RX & TX queue number.
> 
> Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  app/test-pmd/testpmd.c                 | 40 +++++++++++++++++++---------------
>  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++
>  lib/librte_ether/rte_ethdev.h          | 19 ++++++++++++++++
>  lib/librte_ether/rte_ether_version.map |  1 +
>  4 files changed, 59 insertions(+), 18 deletions(-)

Obviously, it will be considered for 16.07.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] lib: fix DCB config issue on ixgbe
  2016-04-11  9:52 ` Thomas Monjalon
@ 2016-04-12  0:39   ` Lu, Wenzhuo
  0 siblings, 0 replies; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-04-12  0:39 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, April 11, 2016 5:52 PM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib: fix DCB config issue on ixgbe
> 
> 2016-04-11 16:24, Wenzhuo Lu:
> > An issue is found that DCB cannot be configged on ixgbe NICs. It's
> > said the TX queue number is not right.
> > On ixgbe the max TX queue number is not fixed, it depends on the
> > multi-queue mode. The API rte_eth_dev_configure should be used to
> > config this mode. But the input of this API includes TX queue number.
> > The problem is before the mode is configged, we cannot decide the TX
> > queue number.
> >
> > This patch adds an API to config RX & TX multi-queue mode separately.
> > After the mode is configged, the max RX & TX queue number is decided.
> > Then we can set the appropriate RX & TX queue number.
> >
> > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> > queues)
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >  app/test-pmd/testpmd.c                 | 40 +++++++++++++++++++---------------
> >  lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++
> >  lib/librte_ether/rte_ethdev.h          | 19 ++++++++++++++++
> >  lib/librte_ether/rte_ether_version.map |  1 +
> >  4 files changed, 59 insertions(+), 18 deletions(-)
> 
> Obviously, it will be considered for 16.07.
OK. I've got some feedback from the users that DCB is working, so I think it's not a critical issue and maybe only testpmd is impacted.
I'll send a new version later. As rte_ether_version.map should be change for 16.07.
Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] lib: fix DCB config issue on ixgbe
  2016-04-11  8:24 [PATCH] lib: fix DCB config issue on ixgbe Wenzhuo Lu
  2016-04-11  9:52 ` Thomas Monjalon
@ 2016-05-04 21:47 ` Wenzhuo Lu
  2016-05-05 21:33 ` [PATCH v3] ethdev: " Wenzhuo Lu
  2 siblings, 0 replies; 14+ messages in thread
From: Wenzhuo Lu @ 2016-05-04 21:47 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

An issue is found that DCB cannot be configged on ixgbe
NICs. It's said the TX queue number is not right.
On ixgbe the max TX queue number is not fixed, it depends
on the multi-queue mode. The API rte_eth_dev_configure
should be used to config this mode. But the input of this
API includes TX queue number. The problem is before the
mode is configged, we cannot decide the TX queue number.

This patch adds an API to config RX & TX multi-queue mode
separately. After the mode is configged, the max RX & TX
queue number is decided. Then we can set the appropriate
RX & TX queue number.

v2:
- Changed the release to 16.07.

Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/testpmd.c                 | 40 +++++++++++++++++++---------------
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 19 ++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 ++++++
 4 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 26a174c..733f760 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1924,17 +1924,31 @@ init_port_dcb_config(portid_t pid,
 		     uint8_t pfc_en)
 {
 	struct rte_eth_conf port_conf;
-	struct rte_eth_dev_info dev_info;
 	struct rte_port *rte_port;
 	int retval;
 	uint16_t i;
 
-	rte_eth_dev_info_get(pid, &dev_info);
+	rte_port = &ports[pid];
+
+	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
+	/* Enter DCB configuration status */
+	dcb_config = 1;
+
+	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
+	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
+	if (retval < 0)
+		return retval;
+
+	rte_eth_dev_mq_mode_set(pid,
+				port_conf.rxmode.mq_mode,
+				port_conf.txmode.mq_mode);
+	rte_eth_dev_info_get(pid, &rte_port->dev_info);
 
 	/* If dev_info.vmdq_pool_base is greater than 0,
 	 * the queue id of vmdq pools is started after pf queues.
 	 */
-	if (dcb_mode == DCB_VT_ENABLED && dev_info.vmdq_pool_base > 0) {
+	if (dcb_mode == DCB_VT_ENABLED &&
+	    rte_port->dev_info.vmdq_pool_base > 0) {
 		printf("VMDQ_DCB multi-queue mode is nonsensical"
 			" for port %d.", pid);
 		return -1;
@@ -1944,13 +1958,13 @@ init_port_dcb_config(portid_t pid,
 	 * and has the same number of rxq and txq in dcb mode
 	 */
 	if (dcb_mode == DCB_VT_ENABLED) {
-		nb_rxq = dev_info.max_rx_queues;
-		nb_txq = dev_info.max_tx_queues;
+		nb_rxq = rte_port->dev_info.max_rx_queues;
+		nb_txq = rte_port->dev_info.max_tx_queues;
 	} else {
 		/*if vt is disabled, use all pf queues */
-		if (dev_info.vmdq_pool_base == 0) {
-			nb_rxq = dev_info.max_rx_queues;
-			nb_txq = dev_info.max_tx_queues;
+		if (rte_port->dev_info.vmdq_pool_base == 0) {
+			nb_rxq = rte_port->dev_info.max_rx_queues;
+			nb_txq = rte_port->dev_info.max_tx_queues;
 		} else {
 			nb_rxq = (queueid_t)num_tcs;
 			nb_txq = (queueid_t)num_tcs;
@@ -1959,16 +1973,6 @@ init_port_dcb_config(portid_t pid,
 	}
 	rx_free_thresh = 64;
 
-	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
-	/* Enter DCB configuration status */
-	dcb_config = 1;
-
-	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
-	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
-	if (retval < 0)
-		return retval;
-
-	rte_port = &ports[pid];
 	memcpy(&rte_port->dev_conf, &port_conf, sizeof(struct rte_eth_conf));
 
 	rxtx_port_config(rte_port);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a31018e..b4eaa29 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3369,3 +3369,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	dev->data->dev_conf.rxmode.mq_mode = rx_mq_mode;
+	dev->data->dev_conf.txmode.mq_mode = tx_mq_mode;
+
+	return 0;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 2757510..3015cec 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -4253,6 +4253,25 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				  uint32_t mask,
 				  uint8_t en);
 
+/**
+ * Set RX & TX multi_queue mode.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param rx_mq_mode
+ *   RX multi_queue mode.
+ * @param tx_mq_mode
+ *   TX multi_queue mode.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ */
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 214ecc7..c53ce80 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -132,3 +132,10 @@ DPDK_16.04 {
 	rte_eth_tx_buffer_set_err_callback;
 
 } DPDK_2.2;
+
+DPDK_16.07 {
+	global:
+
+	rte_eth_dev_mq_mode_set;
+
+} DPDK_16.04;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-04-11  8:24 [PATCH] lib: fix DCB config issue on ixgbe Wenzhuo Lu
  2016-04-11  9:52 ` Thomas Monjalon
  2016-05-04 21:47 ` [PATCH v2] " Wenzhuo Lu
@ 2016-05-05 21:33 ` Wenzhuo Lu
  2016-06-17 11:21   ` De Lara Guarch, Pablo
  2016-06-22 17:01   ` Thomas Monjalon
  2 siblings, 2 replies; 14+ messages in thread
From: Wenzhuo Lu @ 2016-05-05 21:33 UTC (permalink / raw)
  To: dev; +Cc: Wenzhuo Lu

An issue is found that DCB cannot be configured on ixgbe
NICs. It's said the TX queue number is not right.
On ixgbe the max TX queue number is not fixed, it depends
on the multi-queue mode. The API rte_eth_dev_configure
should be used to configure this mode. But the input of
this API includes TX queue number. The problem is before
the mode is configured, we cannot decide the TX queue
number.

This patch adds an API to configure RX & TX multi-queue mode
separately. After the mode is configured, the max RX & TX
queue number is decided. Then we can set the appropriate
RX & TX queue number.

Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v2:
- Changed the release to 16.07.

v3:
- Changed the title prefix to ethdev.
- Reworded the commit log.

 app/test-pmd/testpmd.c                 | 40 +++++++++++++++++++---------------
 lib/librte_ether/rte_ethdev.c          | 17 +++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 19 ++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  7 ++++++
 4 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 26a174c..733f760 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1924,17 +1924,31 @@ init_port_dcb_config(portid_t pid,
 		     uint8_t pfc_en)
 {
 	struct rte_eth_conf port_conf;
-	struct rte_eth_dev_info dev_info;
 	struct rte_port *rte_port;
 	int retval;
 	uint16_t i;
 
-	rte_eth_dev_info_get(pid, &dev_info);
+	rte_port = &ports[pid];
+
+	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
+	/* Enter DCB configuration status */
+	dcb_config = 1;
+
+	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
+	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
+	if (retval < 0)
+		return retval;
+
+	rte_eth_dev_mq_mode_set(pid,
+				port_conf.rxmode.mq_mode,
+				port_conf.txmode.mq_mode);
+	rte_eth_dev_info_get(pid, &rte_port->dev_info);
 
 	/* If dev_info.vmdq_pool_base is greater than 0,
 	 * the queue id of vmdq pools is started after pf queues.
 	 */
-	if (dcb_mode == DCB_VT_ENABLED && dev_info.vmdq_pool_base > 0) {
+	if (dcb_mode == DCB_VT_ENABLED &&
+	    rte_port->dev_info.vmdq_pool_base > 0) {
 		printf("VMDQ_DCB multi-queue mode is nonsensical"
 			" for port %d.", pid);
 		return -1;
@@ -1944,13 +1958,13 @@ init_port_dcb_config(portid_t pid,
 	 * and has the same number of rxq and txq in dcb mode
 	 */
 	if (dcb_mode == DCB_VT_ENABLED) {
-		nb_rxq = dev_info.max_rx_queues;
-		nb_txq = dev_info.max_tx_queues;
+		nb_rxq = rte_port->dev_info.max_rx_queues;
+		nb_txq = rte_port->dev_info.max_tx_queues;
 	} else {
 		/*if vt is disabled, use all pf queues */
-		if (dev_info.vmdq_pool_base == 0) {
-			nb_rxq = dev_info.max_rx_queues;
-			nb_txq = dev_info.max_tx_queues;
+		if (rte_port->dev_info.vmdq_pool_base == 0) {
+			nb_rxq = rte_port->dev_info.max_rx_queues;
+			nb_txq = rte_port->dev_info.max_tx_queues;
 		} else {
 			nb_rxq = (queueid_t)num_tcs;
 			nb_txq = (queueid_t)num_tcs;
@@ -1959,16 +1973,6 @@ init_port_dcb_config(portid_t pid,
 	}
 	rx_free_thresh = 64;
 
-	memset(&port_conf, 0, sizeof(struct rte_eth_conf));
-	/* Enter DCB configuration status */
-	dcb_config = 1;
-
-	/*set configuration of DCB in vt mode and DCB in non-vt mode*/
-	retval = get_eth_dcb_conf(&port_conf, dcb_mode, num_tcs, pfc_en);
-	if (retval < 0)
-		return retval;
-
-	rte_port = &ports[pid];
 	memcpy(&rte_port->dev_conf, &port_conf, sizeof(struct rte_eth_conf));
 
 	rxtx_port_config(rte_port);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index a31018e..b4eaa29 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3369,3 +3369,20 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				-ENOTSUP);
 	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
 }
+
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	dev = &rte_eth_devices[port_id];
+
+	dev->data->dev_conf.rxmode.mq_mode = rx_mq_mode;
+	dev->data->dev_conf.txmode.mq_mode = tx_mq_mode;
+
+	return 0;
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 2757510..3015cec 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -4253,6 +4253,25 @@ rte_eth_dev_l2_tunnel_offload_set(uint8_t port_id,
 				  uint32_t mask,
 				  uint8_t en);
 
+/**
+ * Set RX & TX multi_queue mode.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param rx_mq_mode
+ *   RX multi_queue mode.
+ * @param tx_mq_mode
+ *   TX multi_queue mode.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ */
+int
+rte_eth_dev_mq_mode_set(uint8_t port_id,
+			enum rte_eth_rx_mq_mode rx_mq_mode,
+			enum rte_eth_tx_mq_mode tx_mq_mode);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 214ecc7..c53ce80 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -132,3 +132,10 @@ DPDK_16.04 {
 	rte_eth_tx_buffer_set_err_callback;
 
 } DPDK_2.2;
+
+DPDK_16.07 {
+	global:
+
+	rte_eth_dev_mq_mode_set;
+
+} DPDK_16.04;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-05-05 21:33 ` [PATCH v3] ethdev: " Wenzhuo Lu
@ 2016-06-17 11:21   ` De Lara Guarch, Pablo
  2016-06-22 17:01   ` Thomas Monjalon
  1 sibling, 0 replies; 14+ messages in thread
From: De Lara Guarch, Pablo @ 2016-06-17 11:21 UTC (permalink / raw)
  To: Lu, Wenzhuo, dev; +Cc: Lu, Wenzhuo



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Thursday, May 05, 2016 10:34 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> An issue is found that DCB cannot be configured on ixgbe
> NICs. It's said the TX queue number is not right.
> On ixgbe the max TX queue number is not fixed, it depends
> on the multi-queue mode. The API rte_eth_dev_configure
> should be used to configure this mode. But the input of
> this API includes TX queue number. The problem is before
> the mode is configured, we cannot decide the TX queue
> number.
> 
> This patch adds an API to configure RX & TX multi-queue mode
> separately. After the mode is configured, the max RX & TX
> queue number is decided. Then we can set the appropriate
> RX & TX queue number.
> 
> Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> queues)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-05-05 21:33 ` [PATCH v3] ethdev: " Wenzhuo Lu
  2016-06-17 11:21   ` De Lara Guarch, Pablo
@ 2016-06-22 17:01   ` Thomas Monjalon
  2016-06-23  1:04     ` Lu, Wenzhuo
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2016-06-22 17:01 UTC (permalink / raw)
  To: Wenzhuo Lu; +Cc: dev

2016-05-06 05:33, Wenzhuo Lu:
> An issue is found that DCB cannot be configured on ixgbe
> NICs. It's said the TX queue number is not right.
> On ixgbe the max TX queue number is not fixed, it depends
> on the multi-queue mode. The API rte_eth_dev_configure
> should be used to configure this mode. But the input of
> this API includes TX queue number. The problem is before
> the mode is configured, we cannot decide the TX queue
> number.
> 
> This patch adds an API to configure RX & TX multi-queue mode
> separately. After the mode is configured, the max RX & TX
> queue number is decided. Then we can set the appropriate
> RX & TX queue number.
[...]
> +/**
> + * Set RX & TX multi_queue mode.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param rx_mq_mode
> + *   RX multi_queue mode.
> + * @param tx_mq_mode
> + *   TX multi_queue mode.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENODEV) if port identifier is invalid.
> + */
> +int
> +rte_eth_dev_mq_mode_set(uint8_t port_id,
> +			enum rte_eth_rx_mq_mode rx_mq_mode,
> +			enum rte_eth_tx_mq_mode tx_mq_mode);

I've really tried to think about it and I think it is more or less
a hack.
First, it is not explained in the doc when we should use
rte_eth_dev_mq_mode_set() instead of a simple call to
rte_eth_dev_configure().
Second, I don't understand why having a function which configures the
"multiqueue modes" without configuring properly RSS/VMDq/DCB.
Last, it is said that rte_eth_dev_configure() "must be invoked first
before any other function in the Ethernet API".

My opinion is that the primary goal of rte_eth_dev_configure() was
"Embedding all configuration information in a single data structure"
but it is currently configuring only speed and some flow steering
(only RSS, VMDq, DCB and flow director).
This bug and the state of the ethdev API clearly shows that we must
have one function per feature (or group of features) and drop
rte_eth_dev_configure().

You can argue it is a just a personal feeling and this comment comes
late, but I promise it is not easy to give a negative opinion because
of design perspective.
I strongly feel we must stop workarounding the ethdev API issues
and start really fixing it.

Hope you understand and agree to work on a new API.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-22 17:01   ` Thomas Monjalon
@ 2016-06-23  1:04     ` Lu, Wenzhuo
  2016-06-23 12:21       ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-06-23  1:04 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 23, 2016 1:02 AM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> 2016-05-06 05:33, Wenzhuo Lu:
> > An issue is found that DCB cannot be configured on ixgbe NICs. It's
> > said the TX queue number is not right.
> > On ixgbe the max TX queue number is not fixed, it depends on the
> > multi-queue mode. The API rte_eth_dev_configure should be used to
> > configure this mode. But the input of this API includes TX queue
> > number. The problem is before the mode is configured, we cannot decide
> > the TX queue number.
> >
> > This patch adds an API to configure RX & TX multi-queue mode
> > separately. After the mode is configured, the max RX & TX queue number
> > is decided. Then we can set the appropriate RX & TX queue number.
> [...]
> > +/**
> > + * Set RX & TX multi_queue mode.
> > + *
> > + * @param port_id
> > + *   The port identifier of the Ethernet device.
> > + * @param rx_mq_mode
> > + *   RX multi_queue mode.
> > + * @param tx_mq_mode
> > + *   TX multi_queue mode.
> > + *
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENODEV) if port identifier is invalid.
> > + */
> > +int
> > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> 
> I've really tried to think about it and I think it is more or less a hack.
> First, it is not explained in the doc when we should use
> rte_eth_dev_mq_mode_set() instead of a simple call to rte_eth_dev_configure().
> Second, I don't understand why having a function which configures the
> "multiqueue modes" without configuring properly RSS/VMDq/DCB.
> Last, it is said that rte_eth_dev_configure() "must be invoked first before any
> other function in the Ethernet API".
Sorry, didn't notice this announcement.

> 
> My opinion is that the primary goal of rte_eth_dev_configure() was "Embedding
> all configuration information in a single data structure"
> but it is currently configuring only speed and some flow steering (only RSS,
> VMDq, DCB and flow director).
> This bug and the state of the ethdev API clearly shows that we must have one
> function per feature (or group of features) and drop rte_eth_dev_configure().
> 
> You can argue it is a just a personal feeling and this comment comes late, but I
> promise it is not easy to give a negative opinion because of design perspective.
> I strongly feel we must stop workarounding the ethdev API issues and start really
> fixing it.
> 
> Hope you understand and agree to work on a new API.
I have the same feeling with you. There's some problem with rte_eth_dev_configure. So this patch is a workaround more than a real fix.
But the problem is this API has already been used. What I think is could we take this workaround as a first step. It need not ask the APP to change too much.
Then we can discuss how could we rework on a new API or APIs. We all know the change in rte layer is not easy and need to be very careful :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-23  1:04     ` Lu, Wenzhuo
@ 2016-06-23 12:21       ` Thomas Monjalon
  2016-06-24  0:45         ` Lu, Wenzhuo
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2016-06-23 12:21 UTC (permalink / raw)
  To: Lu, Wenzhuo; +Cc: dev

2016-06-23 01:04, Lu, Wenzhuo:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-05-06 05:33, Wenzhuo Lu:
> > > +int
> > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > 
> > I've really tried to think about it and I think it is more or less a hack.
> > First, it is not explained in the doc when we should use
> > rte_eth_dev_mq_mode_set() instead of a simple call to rte_eth_dev_configure().
> > Second, I don't understand why having a function which configures the
> > "multiqueue modes" without configuring properly RSS/VMDq/DCB.
> > Last, it is said that rte_eth_dev_configure() "must be invoked first before any
> > other function in the Ethernet API".
> Sorry, didn't notice this announcement.
> 
> > My opinion is that the primary goal of rte_eth_dev_configure() was "Embedding
> > all configuration information in a single data structure"
> > but it is currently configuring only speed and some flow steering (only RSS,
> > VMDq, DCB and flow director).
> > This bug and the state of the ethdev API clearly shows that we must have one
> > function per feature (or group of features) and drop rte_eth_dev_configure().
> > 
> > You can argue it is a just a personal feeling and this comment comes late, but I
> > promise it is not easy to give a negative opinion because of design perspective.
> > I strongly feel we must stop workarounding the ethdev API issues and start really
> > fixing it.
> > 
> > Hope you understand and agree to work on a new API.
> I have the same feeling with you. There's some problem with rte_eth_dev_configure. So this patch is a workaround more than a real fix.
> But the problem is this API has already been used. What I think is could we take this workaround as a first step. It need not ask the APP to change too much.
> Then we can discuss how could we rework on a new API or APIs. We all know the change in rte layer is not easy and need to be very careful :)

We probably need more opinions.
I think it is not a good idea to introduce a new API only to workaround
another one and keep confusion in place.
A similar approach which looks better is to introduce a new API which will
partly replace the old one and will remain a good one when the old API
will be completely removed.
In other words, we should introduce a good API for flow steering as soon
as possible and deprecate rte_eth_dev_configure().

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-23 12:21       ` Thomas Monjalon
@ 2016-06-24  0:45         ` Lu, Wenzhuo
  2016-06-30  1:40           ` Lu, Wenzhuo
  0 siblings, 1 reply; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-06-24  0:45 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 23, 2016 8:22 PM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> 2016-06-23 01:04, Lu, Wenzhuo:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-05-06 05:33, Wenzhuo Lu:
> > > > +int
> > > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > >
> > > I've really tried to think about it and I think it is more or less a hack.
> > > First, it is not explained in the doc when we should use
> > > rte_eth_dev_mq_mode_set() instead of a simple call to
> rte_eth_dev_configure().
> > > Second, I don't understand why having a function which configures
> > > the "multiqueue modes" without configuring properly RSS/VMDq/DCB.
> > > Last, it is said that rte_eth_dev_configure() "must be invoked first
> > > before any other function in the Ethernet API".
> > Sorry, didn't notice this announcement.
> >
> > > My opinion is that the primary goal of rte_eth_dev_configure() was
> > > "Embedding all configuration information in a single data structure"
> > > but it is currently configuring only speed and some flow steering
> > > (only RSS, VMDq, DCB and flow director).
> > > This bug and the state of the ethdev API clearly shows that we must
> > > have one function per feature (or group of features) and drop
> rte_eth_dev_configure().
> > >
> > > You can argue it is a just a personal feeling and this comment comes
> > > late, but I promise it is not easy to give a negative opinion because of design
> perspective.
> > > I strongly feel we must stop workarounding the ethdev API issues and
> > > start really fixing it.
> > >
> > > Hope you understand and agree to work on a new API.
> > I have the same feeling with you. There's some problem with
> rte_eth_dev_configure. So this patch is a workaround more than a real fix.
> > But the problem is this API has already been used. What I think is could we take
> this workaround as a first step. It need not ask the APP to change too much.
> > Then we can discuss how could we rework on a new API or APIs. We all
> > know the change in rte layer is not easy and need to be very careful
> > :)
> 
> We probably need more opinions.
> I think it is not a good idea to introduce a new API only to workaround another
> one and keep confusion in place.
> A similar approach which looks better is to introduce a new API which will partly
> replace the old one and will remain a good one when the old API will be
> completely removed.
> In other words, we should introduce a good API for flow steering as soon as
> possible and deprecate rte_eth_dev_configure().
I think you're right. The workaround can make things confusing. Better to introduce a new API to replace rte_eth_dev_configure.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-24  0:45         ` Lu, Wenzhuo
@ 2016-06-30  1:40           ` Lu, Wenzhuo
  2016-06-30  7:41             ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-06-30  1:40 UTC (permalink / raw)
  To: Lu, Wenzhuo, Thomas Monjalon; +Cc: dev

Hi Thomas,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo
> Sent: Friday, June 24, 2016 8:46 AM
> To: Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> Hi Thomas,
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, June 23, 2016 8:22 PM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on
> > ixgbe
> >
> > 2016-06-23 01:04, Lu, Wenzhuo:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2016-05-06 05:33, Wenzhuo Lu:
> > > > > +int
> > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > > >
> > > > I've really tried to think about it and I think it is more or less a hack.
> > > > First, it is not explained in the doc when we should use
> > > > rte_eth_dev_mq_mode_set() instead of a simple call to
> > rte_eth_dev_configure().
> > > > Second, I don't understand why having a function which configures
> > > > the "multiqueue modes" without configuring properly RSS/VMDq/DCB.
> > > > Last, it is said that rte_eth_dev_configure() "must be invoked
> > > > first before any other function in the Ethernet API".
After checking the code, Honestly I'm confused. I don't find this description. And on the contrary, I find rte_eth_dev_info_get is always called before rte_eth_dev_configure. I believe it's the problem. As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get the right info. That why I have to add a API to set the mq_mode before rte_eth_dev_info_get.
Does that mean all the related examples are wrong? Any opinion? Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-30  1:40           ` Lu, Wenzhuo
@ 2016-06-30  7:41             ` Thomas Monjalon
  2016-06-30  8:23               ` Lu, Wenzhuo
  2016-06-30  8:47               ` Lu, Wenzhuo
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Monjalon @ 2016-06-30  7:41 UTC (permalink / raw)
  To: Lu, Wenzhuo; +Cc: dev

2016-06-30 01:40, Lu, Wenzhuo:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2016-06-23 01:04, Lu, Wenzhuo:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2016-05-06 05:33, Wenzhuo Lu:
> > > > > > +int
> > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > > > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > > > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > > > >
> > > > > I've really tried to think about it and I think it is more or less a hack.
> > > > > First, it is not explained in the doc when we should use
> > > > > rte_eth_dev_mq_mode_set() instead of a simple call to
> > > rte_eth_dev_configure().
> > > > > Second, I don't understand why having a function which configures
> > > > > the "multiqueue modes" without configuring properly RSS/VMDq/DCB.
> > > > > Last, it is said that rte_eth_dev_configure() "must be invoked
> > > > > first before any other function in the Ethernet API".
> After checking the code, Honestly I'm confused. I don't find this description.

It's in the description of rte_eth_dev_configure():
	http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1904

> And on the contrary, I find rte_eth_dev_info_get is always called before
> rte_eth_dev_configure. I believe it's the problem.
> As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get the right info.
> That why I have to add a API to set the mq_mode before rte_eth_dev_info_get.
> Does that mean all the related examples are wrong? Any opinion? Thanks.

My opinion is that this area needs a good cleanup and easy API :)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-30  7:41             ` Thomas Monjalon
@ 2016-06-30  8:23               ` Lu, Wenzhuo
  2016-06-30  8:47               ` Lu, Wenzhuo
  1 sibling, 0 replies; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-06-30  8:23 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, June 30, 2016 3:42 PM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> 2016-06-30 01:40, Lu, Wenzhuo:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2016-06-23 01:04, Lu, Wenzhuo:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > 2016-05-06 05:33, Wenzhuo Lu:
> > > > > > > +int
> > > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > > > > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > > > > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > > > > >
> > > > > > I've really tried to think about it and I think it is more or less a hack.
> > > > > > First, it is not explained in the doc when we should use
> > > > > > rte_eth_dev_mq_mode_set() instead of a simple call to
> > > > rte_eth_dev_configure().
> > > > > > Second, I don't understand why having a function which
> > > > > > configures the "multiqueue modes" without configuring properly
> RSS/VMDq/DCB.
> > > > > > Last, it is said that rte_eth_dev_configure() "must be invoked
> > > > > > first before any other function in the Ethernet API".
> > After checking the code, Honestly I'm confused. I don't find this description.
> 
> It's in the description of rte_eth_dev_configure():
> 	http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1904
The bad news is this rule is not followed. I think the reason is something has to be done before configure.

> 
> > And on the contrary, I find rte_eth_dev_info_get is always called
> > before rte_eth_dev_configure. I believe it's the problem.
> > As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get the
> right info.
> > That why I have to add a API to set the mq_mode before
> rte_eth_dev_info_get.
> > Does that mean all the related examples are wrong? Any opinion? Thanks.
> 
> My opinion is that this area needs a good cleanup and easy API :)
OK. My solution is
1, Remove the description " rte_eth_dev_configure() must be invoked first before any other function in the Ethernet API ". I think it's reasonable to execute the rte_eth_dev_info_get before rte_eth_dev_configure, because we need to get some NIC specific limitation to check if the configuration is right.
2, Add one more argument, " const struct rte_eth_conf *eth_conf ", for rte_eth_dev_info_get, like this,
void rte_eth_dev_info_get(uint8_t port_id, struct rte_eth_dev_info *dev_info, const struct rte_eth_conf *eth_conf);
because getting the right info depends on the configuration.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] ethdev: fix DCB config issue on ixgbe
  2016-06-30  7:41             ` Thomas Monjalon
  2016-06-30  8:23               ` Lu, Wenzhuo
@ 2016-06-30  8:47               ` Lu, Wenzhuo
  1 sibling, 0 replies; 14+ messages in thread
From: Lu, Wenzhuo @ 2016-06-30  8:47 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi Thomas,


> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Thursday, June 30, 2016 4:23 PM
> To: 'Thomas Monjalon'
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
> 
> Hi Thomas,
> 
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Thursday, June 30, 2016 3:42 PM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on
> > ixgbe
> >
> > 2016-06-30 01:40, Lu, Wenzhuo:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2016-06-23 01:04, Lu, Wenzhuo:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > 2016-05-06 05:33, Wenzhuo Lu:
> > > > > > > > +int
> > > > > > > > +rte_eth_dev_mq_mode_set(uint8_t port_id,
> > > > > > > > +			enum rte_eth_rx_mq_mode rx_mq_mode,
> > > > > > > > +			enum rte_eth_tx_mq_mode tx_mq_mode);
> > > > > > >
> > > > > > > I've really tried to think about it and I think it is more or less a hack.
> > > > > > > First, it is not explained in the doc when we should use
> > > > > > > rte_eth_dev_mq_mode_set() instead of a simple call to
> > > > > rte_eth_dev_configure().
> > > > > > > Second, I don't understand why having a function which
> > > > > > > configures the "multiqueue modes" without configuring
> > > > > > > properly
> > RSS/VMDq/DCB.
> > > > > > > Last, it is said that rte_eth_dev_configure() "must be
> > > > > > > invoked first before any other function in the Ethernet API".
> > > After checking the code, Honestly I'm confused. I don't find this description.
> >
> > It's in the description of rte_eth_dev_configure():
> > 	http://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1904
> The bad news is this rule is not followed. I think the reason is something has to
> be done before configure.
> 
> >
> > > And on the contrary, I find rte_eth_dev_info_get is always called
> > > before rte_eth_dev_configure. I believe it's the problem.
> > > As rte_eth_dev_configure is not run, rte_eth_dev_info_get cannot get
> > > the
> > right info.
> > > That why I have to add a API to set the mq_mode before
> > rte_eth_dev_info_get.
> > > Does that mean all the related examples are wrong? Any opinion? Thanks.
> >
> > My opinion is that this area needs a good cleanup and easy API :)
> OK. My solution is
> 1, Remove the description " rte_eth_dev_configure() must be invoked first
> before any other function in the Ethernet API ". I think it's reasonable to execute
> the rte_eth_dev_info_get before rte_eth_dev_configure, because we need to
> get some NIC specific limitation to check if the configuration is right.
> 2, Add one more argument, " const struct rte_eth_conf *eth_conf ", for
> rte_eth_dev_info_get, like this, void rte_eth_dev_info_get(uint8_t port_id,
> struct rte_eth_dev_info *dev_info, const struct rte_eth_conf *eth_conf);
> because getting the right info depends on the configuration.
And I'm a little lost, my patch has nothing to do with rte_eth_dev_configure. I just try to fix the issue that rte_eth_dev_info_get has dependency with configuration. If the configure is not right, we cannot get the right info.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-06-30  8:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-11  8:24 [PATCH] lib: fix DCB config issue on ixgbe Wenzhuo Lu
2016-04-11  9:52 ` Thomas Monjalon
2016-04-12  0:39   ` Lu, Wenzhuo
2016-05-04 21:47 ` [PATCH v2] " Wenzhuo Lu
2016-05-05 21:33 ` [PATCH v3] ethdev: " Wenzhuo Lu
2016-06-17 11:21   ` De Lara Guarch, Pablo
2016-06-22 17:01   ` Thomas Monjalon
2016-06-23  1:04     ` Lu, Wenzhuo
2016-06-23 12:21       ` Thomas Monjalon
2016-06-24  0:45         ` Lu, Wenzhuo
2016-06-30  1:40           ` Lu, Wenzhuo
2016-06-30  7:41             ` Thomas Monjalon
2016-06-30  8:23               ` Lu, Wenzhuo
2016-06-30  8:47               ` Lu, Wenzhuo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.