[dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[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.

This patch adds the device configuration before getting
info in the DCB configuration process. So the right info
can be got depending on the configuration.

Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 app/test-pmd/testpmd.c | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1428974..ba41bea 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1962,17 +1962,30 @@ 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;
+	port_conf.rxmode.hw_vlan_filter = 1;
+
+	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);
+	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;
@@ -1982,13 +1995,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;
@@ -1997,16 +2010,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);
-- 
1.9.3

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Iremonger, Bernard]

Hi Wenzhuo,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Friday, August 5, 2016 6:20 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH] app/testpmd: 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.
> 
> This patch adds the device configuration before getting info in the DCB
> configuration process. So the right info can be got depending on the
> configuration.
> 
> Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> queues)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> ---
>  app/test-pmd/testpmd.c | 39 +++++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 1428974..ba41bea 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1962,17 +1962,30 @@ 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;
> +	port_conf.rxmode.hw_vlan_filter = 1;
> +
> +	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);

The return value of rte_eth_dev_configure() should be checked.
Calling rte_eth_dev_configure() with  parameters nb_rx_q and nb_tx_q equal to 0 returns -EINVAL, and does nothing.
Should the values of nb_rx_q and nb_tx_q be non zero?

> +	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;
> @@ -1982,13 +1995,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;
> @@ -1997,16 +2010,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);
> --
> 1.9.3

Regards,

Bernard.

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Iremonger, Bernard]

Hi Wenzhuo,

<snip>

> > Subject: [dpdk-dev] [PATCH] app/testpmd: 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.
> >
> > This patch adds the device configuration before getting info in the
> > DCB configuration process. So the right info can be got depending on
> > the configuration.
> >
> > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> > queues)

As the fix in this patch is to testpmd, I don't think the fixes line is correct.

> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 39 +++++++++++++++++++++------------------
> >  1 file changed, 21 insertions(+), 18 deletions(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 1428974..ba41bea 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -1962,17 +1962,30 @@ 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;
> > +	port_conf.rxmode.hw_vlan_filter = 1;
> > +
> > +	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);
> 
> The return value of rte_eth_dev_configure() should be checked.
> Calling rte_eth_dev_configure() with  parameters nb_rx_q and nb_tx_q
> equal to 0 returns -EINVAL, and does nothing.
> Should the values of nb_rx_q and nb_tx_q be non zero?

The call to rte_eth_dev_configure() may not be necessary, as it is also called when the port is started.
 
> > +	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;
> > @@ -1982,13 +1995,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;

If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues respectively, there is a failure when the port is started in ixgbe_check_mq_mode() at line 1990 in ixgbe_ethdev.c.
SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or equal to 1.

nb_rxq and nb_txq are equal to 1 at this point, if they are not changed, port start completes successfully.
 

> >  	} 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;
> > @@ -1997,16 +2010,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);
> > --
> > 1.9.3
Regards,

Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Lu, Wenzhuo]

Hi Bernard,

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Wednesday, August 24, 2016 11:22 PM
> To: Iremonger, Bernard; Lu, Wenzhuo; dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
> 
> Hi Wenzhuo,
> 
> <snip>
> 
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: 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.
> > >
> > > This patch adds the device configuration before getting info in the
> > > DCB configuration process. So the right info can be got depending on
> > > the configuration.
> > >
> > > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx
> > > queues)
> 
> As the fix in this patch is to testpmd, I don't think the fixes line is correct.
The bug is introduced by this patch. Before this patch, APP need not care about the multi-queue mode when it want to get the max queue number.

> 
> > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > ---
> > >  app/test-pmd/testpmd.c | 39 +++++++++++++++++++++------------------
> > >  1 file changed, 21 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 1428974..ba41bea 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -1962,17 +1962,30 @@ 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;
> > > +	port_conf.rxmode.hw_vlan_filter = 1;
> > > +
> > > +	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);
> >
> > The return value of rte_eth_dev_configure() should be checked.
> > Calling rte_eth_dev_configure() with  parameters nb_rx_q and nb_tx_q
> > equal to 0 returns -EINVAL, and does nothing.
> > Should the values of nb_rx_q and nb_tx_q be non zero?
The 0 is used on purpose. Because I don't want to configure the queues. The only purpose is to  make all the configuration to write into the device. And that's why the return value is ignored by (void).

> 
> The call to rte_eth_dev_configure() may not be necessary, as it is also called
> when the port is started.
In rte_ethdev.h, we can see it's said that rte_eth_dev_configure should be called before any other function of ethernet API. As the DCB configuration is changed here. We have to call rte_eth_dev_configure to make the configuration is right before we can use rte_eth_dev_info_get.

> 
> > > +	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;
> > > @@ -1982,13 +1995,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;
> 
> If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues respectively,
> there is a failure when the port is started in ixgbe_check_mq_mode() at line
> 1990 in ixgbe_ethdev.c.
> SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or
> equal to 1.
I haven't hit this issue. Would you like to give more details about how to hit it? I'll check if I miss something.

> 
> nb_rxq and nb_txq are equal to 1 at this point, if they are not changed, port start
> completes successfully.
> 
> 
> > >  	} 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;
> > > @@ -1997,16 +2010,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);
> > > --
> > > 1.9.3
> Regards,
> 
> Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Iremonger, Bernard]

Hi Wenzhuo,

<snip>

> > > > Subject: [dpdk-dev] [PATCH] app/testpmd: 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.
> > > >
> > > > This patch adds the device configuration before getting info in
> > > > the DCB configuration process. So the right info can be got
> > > > depending on the configuration.
> > > >
> > > > Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported
> > > > Tx
> > > > queues)
> >
> > As the fix in this patch is to testpmd, I don't think the fixes line is correct.
> The bug is introduced by this patch. Before this patch, APP need not care
> about the multi-queue mode when it want to get the max queue number.
> 
> >
> > > > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> > > > ---
> > > >  app/test-pmd/testpmd.c | 39
> > > > +++++++++++++++++++++------------------
> > > >  1 file changed, 21 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > 1428974..ba41bea 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -1962,17 +1962,30 @@ 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;
> > > > +	port_conf.rxmode.hw_vlan_filter = 1;
> > > > +
> > > > +	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);
> > >
> > > The return value of rte_eth_dev_configure() should be checked.
> > > Calling rte_eth_dev_configure() with  parameters nb_rx_q and nb_tx_q
> > > equal to 0 returns -EINVAL, and does nothing.
> > > Should the values of nb_rx_q and nb_tx_q be non zero?
> The 0 is used on purpose. Because I don't want to configure the queues. The
> only purpose is to  make all the configuration to write into the device. And
> that's why the return value is ignored by (void).

It might be useful to add a comment to explain why rte_eth_dev_configure() is being called in this way.
 
> > The call to rte_eth_dev_configure() may not be necessary, as it is
> > also called when the port is started.
> In rte_ethdev.h, we can see it's said that rte_eth_dev_configure should be
> called before any other function of ethernet API. As the DCB configuration is
> changed here. We have to call rte_eth_dev_configure to make the
> configuration is right before we can use rte_eth_dev_info_get.
> 
> >
> > > > +	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;
> > > > @@ -1982,13 +1995,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;
> >
> > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > respectively, there is a failure when the port is started in
> > ixgbe_check_mq_mode() at line
> > 1990 in ixgbe_ethdev.c.
> > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > than or equal to 1.
> I haven't hit this issue. Would you like to give more details about how to hit
> it? I'll check if I miss something.

There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and Port 1 is the VF.
./testpmd -c 3f -l 1-5 -n 4 -- -i
testpmd> set corelist 2,3,4,5
testpmd> port stop 0  /* PF is 0 */
testpmd> port config 0 dcb vt on 4 pfc on
testpmd> port start 0 /* PF is 0 */
line 1990   ixgbe_ethdev.c
SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or equal to 1.
/* Works if nb_rx_q and nb_tx_q set to 1 */

testpmd> show port dcb_tc 0
================ DCB infos for port 0   ================
  TC NUMBER: 4  (showing 8 a lot of the time ??)   /* port start should be called first */ 
  TC :             0       1       2       3
  Priority :       0       1       2       3
  BW percent :    25%     25%     25%     25%
  RXQ base :       0       1       2       3
  RXQ number :     1       1       1       1
  TXQ base :       0       1       2       3
  TXQ number :     1       1       1       1 
testpmd> show port dcb_tc 1
 Failed to get dcb infos on port 1
 
> >
> > nb_rxq and nb_txq are equal to 1 at this point, if they are not
> > changed, port start completes successfully.
> >
> >
> > > >  	} 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;
> > > > @@ -1997,16 +2010,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);
> > > > --
> > > > 1.9.3
Regards,

Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Lu, Wenzhuo]

Hi Bernard,

> > > >
> > > > The return value of rte_eth_dev_configure() should be checked.
> > > > Calling rte_eth_dev_configure() with  parameters nb_rx_q and
> > > > nb_tx_q equal to 0 returns -EINVAL, and does nothing.
> > > > Should the values of nb_rx_q and nb_tx_q be non zero?
> > The 0 is used on purpose. Because I don't want to configure the
> > queues. The only purpose is to  make all the configuration to write
> > into the device. And that's why the return value is ignored by (void).
> 
> It might be useful to add a comment to explain why rte_eth_dev_configure() is
> being called in this way.
Thanks for the suggestion. I'll add it:)

> > >
> > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > > respectively, there is a failure when the port is started in
> > > ixgbe_check_mq_mode() at line
> > > 1990 in ixgbe_ethdev.c.
> > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > > than or equal to 1.
> > I haven't hit this issue. Would you like to give more details about
> > how to hit it? I'll check if I miss something.
> 
> There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and Port 1 is the
> VF.
> ./testpmd -c 3f -l 1-5 -n 4 -- -i
> testpmd> set corelist 2,3,4,5
> testpmd> port stop 0  /* PF is 0 */
> testpmd> port config 0 dcb vt on 4 pfc on port start 0 /* PF is 0 */
> line 1990   ixgbe_ethdev.c
> SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or
> equal to 1.
> /* Works if nb_rx_q and nb_tx_q set to 1 */
To my opinion, it's a by-design limitation. After using the DCB configuration CLI, the queue number is set to a fix number which is the max number. But as you pointed, when SRIOV is active there's another requirement for the queue number.
We need to investigate deeper and find a solution for it. But I think it's another story. We need another patch for it.

> 
> testpmd> show port dcb_tc 0
> ================ DCB infos for port 0   ================
>   TC NUMBER: 4  (showing 8 a lot of the time ??)   /* port start should be called
> first */
>   TC :             0       1       2       3
>   Priority :       0       1       2       3
>   BW percent :    25%     25%     25%     25%
>   RXQ base :       0       1       2       3
>   RXQ number :     1       1       1       1
>   TXQ base :       0       1       2       3
>   TXQ number :     1       1       1       1
> testpmd> show port dcb_tc 1
>  Failed to get dcb infos on port 1
> 
> > >
> > > nb_rxq and nb_txq are equal to 1 at this point, if they are not
> > > changed, port start completes successfully.
> > >
> > >
> > > > >  	} 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; @@ -1997,16 +2010,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);
> > > > > --
> > > > > 1.9.3
> Regards,
> 
> Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Iremonger, Bernard]

Hi Wenzhuo,

<snip>

> > > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > > > respectively, there is a failure when the port is started in
> > > > ixgbe_check_mq_mode() at line
> > > > 1990 in ixgbe_ethdev.c.
> > > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > > > than or equal to 1.
> > > I haven't hit this issue. Would you like to give more details about
> > > how to hit it? I'll check if I miss something.
> >
> > There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and
> > Port 1 is the VF.
> > ./testpmd -c 3f -l 1-5 -n 4 -- -i
> > testpmd> set corelist 2,3,4,5
> > testpmd> port stop 0  /* PF is 0 */
> > testpmd> port config 0 dcb vt on 4 pfc on port start 0 /* PF is 0 */
> > line 1990   ixgbe_ethdev.c
> > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > than or equal to 1.
> > /* Works if nb_rx_q and nb_tx_q set to 1 */
> To my opinion, it's a by-design limitation. After using the DCB configuration
> CLI, the queue number is set to a fix number which is the max number. But
> as you pointed, when SRIOV is active there's another requirement for the
> queue number.
> We need to investigate deeper and find a solution for it. But I think it's
> another story. We need another patch for it.

Line 1997 in testpmd.c
1997	if (dcb_mode == DCB_VT_ENABLED) {
		nb_rxq = rte_port->dev_info.max_rx_queues;
		nb_txq = rte_port->dev_info.max_tx_queues;
	} else {
2001		/*if vt is disabled, use all pf 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;
		}
	}

The comment at line 2001 implies that when dcb_mode is DCB_VT_ENABLED all pf queues should not be used.
When dcb_mode is DCB_VT_ENABLED, setting nb_rxq and nb_txq equal to 1 works when the PF (port 0) is started.
When dcb_mode is DCB_VT_ENABLED, setting nb_rxq to max_rx_queues and nb_txq to max_tx_queues results in the following failure in ixgbe when the PF (port 0) is started.
SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or equal to 1.

<snip>
 
Regards,

Bernard

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Lu, Wenzhuo]

Hi Bernard,


> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Friday, August 26, 2016 6:04 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: De Lara Guarch, Pablo; Wu, Jingjing
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
> 
> Hi Wenzhuo,
> 
> <snip>
> 
> > > > > If nb_rxq and nb_txq are set to max_rx_queues and max_tx_queues
> > > > > respectively, there is a failure when the port is started in
> > > > > ixgbe_check_mq_mode() at line
> > > > > 1990 in ixgbe_ethdev.c.
> > > > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be
> > > > > less than or equal to 1.
> > > > I haven't hit this issue. Would you like to give more details
> > > > about how to hit it? I'll check if I miss something.
> > >
> > > There is a Niantic PF and VF bound to igb_uio. Port 0 is the PF and
> > > Port 1 is the VF.
> > > ./testpmd -c 3f -l 1-5 -n 4 -- -i
> > > testpmd> set corelist 2,3,4,5
> > > testpmd> port stop 0  /* PF is 0 */
> > > testpmd> port config 0 dcb vt on 4 pfc on port start 0 /* PF is 0 */
> > > line 1990   ixgbe_ethdev.c
> > > SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less
> > > than or equal to 1.
> > > /* Works if nb_rx_q and nb_tx_q set to 1 */
> > To my opinion, it's a by-design limitation. After using the DCB
> > configuration CLI, the queue number is set to a fix number which is
> > the max number. But as you pointed, when SRIOV is active there's
> > another requirement for the queue number.
> > We need to investigate deeper and find a solution for it. But I think
> > it's another story. We need another patch for it.
> 
> Line 1997 in testpmd.c
> 1997	if (dcb_mode == DCB_VT_ENABLED) {
> 		nb_rxq = rte_port->dev_info.max_rx_queues;
> 		nb_txq = rte_port->dev_info.max_tx_queues;
> 	} else {
> 2001		/*if vt is disabled, use all pf 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;
> 		}
> 	}
> 
> The comment at line 2001 implies that when dcb_mode is DCB_VT_ENABLED all
> pf queues should not be used.
> When dcb_mode is DCB_VT_ENABLED, setting nb_rxq and nb_txq equal to 1
> works when the PF (port 0) is started.
> When dcb_mode is DCB_VT_ENABLED, setting nb_rxq to max_rx_queues and
> nb_txq to max_tx_queues results in the following failure in ixgbe when the PF
> (port 0) is started.
> SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or
> equal to 1.
PF and its VF port cannot be used in the same APP. As VF depends on its PF. We must wait until PF is completed, then run VF. Normally, like use DPDK PF on host, then use DPDK VF on guest. If you use PF and VF on parallel, the behavior is unpredicted. I think that's why you see this prompt " SRIOV is active, nb_rx_q=128 nb_tx_q=128 queue number must be less than or equal to 1.".
I've tried PF only testpmd. The result is like " PMD: ixgbe_check_mq_mode(): SRIOV active, unsupported mq_mode rx 6.". It means DCB is not supported by DPDK PF.

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Thomas Monjalon]

What is the status of this patch?

2016-08-05 13:20, 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.
> 
> This patch adds the device configuration before getting
> info in the DCB configuration process. So the right info
> can be got depending on the configuration.
> 
> Fixes: 96c0450dff86 (ixgbe: fix dropping packets from unsupported Tx queues)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe

[Lu, Wenzhuo]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Saturday, September 24, 2016 2:25 AM
> To: Lu, Wenzhuo; De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix DCB config issue on ixgbe
> 
> What is the status of this patch?
I need to send a V2 to add more explanation in the code. I'll send it soon. 

[dpdk-dev] [PATCH v2] app/testpmd: fix DCB config issue

[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.

This patch adds the device configuration before getting
info in the DCB configuration process. So the right info
can be got depending on the configuration.

Fixes: 1a572499beb6 (app/testpmd: setup DCB forwarding based on traffic class)
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v2:
- add the explanation about the argument 0 of rte_eth_dev_configure.
- change the Fixes.

 app/test-pmd/testpmd.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1428974..36d36bd 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1962,17 +1962,36 @@ 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;
+	port_conf.rxmode.hw_vlan_filter = 1;
+
+	/**
+	 * Write the configuration into the device.
+	 * Set the numbers of RX & TX queues to 0, so
+	 * the RX & TX queues will not be setup.
+	 */
+	(void)rte_eth_dev_configure(pid, 0, 0, &port_conf);
+
+	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;
@@ -1982,13 +2001,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;
@@ -1997,16 +2016,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);
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix DCB config issue

[Iremonger, Bernard]

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Monday, September 26, 2016 2:11 AM
> To: dev@dpdk.org
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Iremonger,
> Bernard <bernard.iremonger@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: [PATCH v2] app/testpmd: fix DCB config issue
> 
> 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.
> 
> This patch adds the device configuration before getting info in the DCB
> configuration process. So the right info can be got depending on the
> configuration.
> 
> Fixes: 1a572499beb6 (app/testpmd: setup DCB forwarding based on traffic
> class)
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Acked-by: Bernard Iremonger <Bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix DCB config issue

[Thomas Monjalon]

> > 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.
> > 
> > This patch adds the device configuration before getting info in the DCB
> > configuration process. So the right info can be got depending on the
> > configuration.
> > 
> > Fixes: 1a572499beb6 (app/testpmd: setup DCB forwarding based on traffic
> > class)
> > Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> 
> Acked-by: Bernard Iremonger <Bernard.iremonger@intel.com>

Applied, thanks