[dpdk-dev] [PATCH] app/test-pmd: fix testpmd log of start command

[dpdk-dev] [PATCH] app/test-pmd: fix testpmd log of start command

[Bernard Iremonger]

In the testpmd log of the start command, show the RX and TX
descriptor values from dev_info instead of the default
values.

Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")

Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1344016..5b00f7f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2308,7 +2308,7 @@ struct pmd_test_command {
 		if (rx_drop_en != RTE_PMD_PARAM_UNSET)
 			port->rx_conf[qid].rx_drop_en = rx_drop_en;
 
-		port->nb_rx_desc[qid] = nb_rxd;
+		port->nb_rx_desc[qid] = port->dev_info.rx_desc_lim.nb_max;
 	}
 
 	for (qid = 0; qid < nb_txq; qid++) {
@@ -2330,7 +2330,7 @@ struct pmd_test_command {
 		if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
 			port->tx_conf[qid].tx_free_thresh = tx_free_thresh;
 
-		port->nb_tx_desc[qid] = nb_txd;
+		port->nb_tx_desc[qid] = port->dev_info.tx_desc_lim.nb_max;
 	}
 }
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH] app/test-pmd: fix testpmd log of start command

[Ferruh Yigit]

On 5/15/2018 5:24 PM, Bernard Iremonger wrote:
> In the testpmd log of the start command, show the RX and TX
> descriptor values from dev_info instead of the default
> values.
> 
> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
> 
> Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/testpmd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 1344016..5b00f7f 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2308,7 +2308,7 @@ struct pmd_test_command {
>  		if (rx_drop_en != RTE_PMD_PARAM_UNSET)
>  			port->rx_conf[qid].rx_drop_en = rx_drop_en;
>  
> -		port->nb_rx_desc[qid] = nb_rxd;
> +		port->nb_rx_desc[qid] = port->dev_info.rx_desc_lim.nb_max;

Hi Bernard,

"rx_desc_lim.nb_max" is not the value we want here.

Also we shouldn't concern what dev_info returns but what is the number device is
configured.

>  	}
>  
>  	for (qid = 0; qid < nb_txq; qid++) {
> @@ -2330,7 +2330,7 @@ struct pmd_test_command {
>  		if (tx_free_thresh != RTE_PMD_PARAM_UNSET)
>  			port->tx_conf[qid].tx_free_thresh = tx_free_thresh;
>  
> -		port->nb_tx_desc[qid] = nb_txd;
> +		port->nb_tx_desc[qid] = port->dev_info.tx_desc_lim.nb_max;
>  	}
>  }
>  
> 

[dpdk-dev] [PATCH v2] app/testpmd: fix log of start command

[Bernard Iremonger]

Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
to update the number of rx and tx descriptors in the rte_port
variable.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/testpmd.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1d308f0..293b2a5 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1592,6 +1592,9 @@ static void eth_dev_event_callback(char *device_name,
 	struct rte_port *port;
 	struct ether_addr mac_addr;
 	enum rte_eth_event_type event_type;
+	struct rte_eth_rxq_info rx_qinfo;
+	struct rte_eth_txq_info tx_qinfo;
+	int32_t rc;
 
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return 0;
@@ -1706,8 +1709,19 @@ static void eth_dev_event_callback(char *device_name,
 					     &(port->rx_conf[qi]),
 					     mp);
 				}
-				if (diag == 0)
+				if (diag == 0) {
+					rc = rte_eth_rx_queue_info_get(pi, qi,
+						&rx_qinfo);
+					if (!rc)
+						port->nb_rx_desc[qi] =
+							rx_qinfo.nb_desc;
+					rc = rte_eth_tx_queue_info_get(pi, qi,
+						&tx_qinfo);
+					if (!rc)
+						port->nb_tx_desc[qi] =
+							tx_qinfo.nb_desc;
 					continue;
+				}
 
 				/* Fail to setup rx queue, return */
 				if (rte_atomic16_cmpset(&(port->port_status),
-- 
1.9.1

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix log of start command

[Ferruh Yigit]

On 5/21/2018 2:28 PM, Bernard Iremonger wrote:
> Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
> to update the number of rx and tx descriptors in the rte_port
> variable.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/testpmd.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 1d308f0..293b2a5 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1592,6 +1592,9 @@ static void eth_dev_event_callback(char *device_name,
>  	struct rte_port *port;
>  	struct ether_addr mac_addr;
>  	enum rte_eth_event_type event_type;
> +	struct rte_eth_rxq_info rx_qinfo;
> +	struct rte_eth_txq_info tx_qinfo;
> +	int32_t rc;
>  
>  	if (port_id_is_invalid(pid, ENABLED_WARN))
>  		return 0;
> @@ -1706,8 +1709,19 @@ static void eth_dev_event_callback(char *device_name,
>  					     &(port->rx_conf[qi]),
>  					     mp);
>  				}
> -				if (diag == 0)
> +				if (diag == 0) {
> +					rc = rte_eth_rx_queue_info_get(pi, qi,
> +						&rx_qinfo);
> +					if (!rc)
> +						port->nb_rx_desc[qi] =
> +							rx_qinfo.nb_desc;
> +					rc = rte_eth_tx_queue_info_get(pi, qi,
> +						&tx_qinfo);
> +					if (!rc)
> +						port->nb_tx_desc[qi] =
> +							tx_qinfo.nb_desc;

Hi Bernard,

port->nb_rx_desc[qi] and port->nb_tx_desc[qi] are intentionally set to zero, to
be able to use PMD provided values, assigning value to them will break that logic.

Instead of updating these values, what about using same information on print only?

Re: [dpdk-dev] [PATCH v2] app/testpmd: fix log of start command

[Iremonger, Bernard]

Hi Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, May 21, 2018 3:16 PM
> To: Iremonger, Bernard <bernard.iremonger@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v2] app/testpmd: fix log of start command
> 
> On 5/21/2018 2:28 PM, Bernard Iremonger wrote:
> > Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
> > to update the number of rx and tx descriptors in the rte_port
> > variable.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > ---
> >  app/test-pmd/testpmd.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 1d308f0..293b2a5 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -1592,6 +1592,9 @@ static void eth_dev_event_callback(char
> *device_name,
> >  	struct rte_port *port;
> >  	struct ether_addr mac_addr;
> >  	enum rte_eth_event_type event_type;
> > +	struct rte_eth_rxq_info rx_qinfo;
> > +	struct rte_eth_txq_info tx_qinfo;
> > +	int32_t rc;
> >
> >  	if (port_id_is_invalid(pid, ENABLED_WARN))
> >  		return 0;
> > @@ -1706,8 +1709,19 @@ static void eth_dev_event_callback(char
> *device_name,
> >  					     &(port->rx_conf[qi]),
> >  					     mp);
> >  				}
> > -				if (diag == 0)
> > +				if (diag == 0) {
> > +					rc = rte_eth_rx_queue_info_get(pi,
> qi,
> > +						&rx_qinfo);
> > +					if (!rc)
> > +						port->nb_rx_desc[qi] =
> > +							rx_qinfo.nb_desc;
> > +					rc = rte_eth_tx_queue_info_get(pi,
> qi,
> > +						&tx_qinfo);
> > +					if (!rc)
> > +						port->nb_tx_desc[qi] =
> > +							tx_qinfo.nb_desc;
> 
> Hi Bernard,
> 
> port->nb_rx_desc[qi] and port->nb_tx_desc[qi] are intentionally set to
> port->zero, to
> be able to use PMD provided values, assigning value to them will break that
> logic.
> 
> Instead of updating these values, what about using same information on
> print only?

I don't think this fix is breaking any logic, as the nb_rx_desc[qi] and nb_tx_desc[qi] values are not being updated until after the rx and tx queues have been configured using the original values.
The nb_rxd and nb_txd values assigned to nb_rx_desc[qi] and nb_tx_desc[qi]  are initialised to 0 but can be initialised to other vallues  from the command line.

I will investigate using the updated values in the print only to avoid any risk.

Regards,

Bernard.

[dpdk-dev] [PATCH v3] app/testpmd: fix log of start command

[Bernard Iremonger]

Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
to update the number of rx and tx descriptors.

Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4520084..339651f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1839,6 +1839,9 @@ struct igb_ring_desc_16_bytes {
 		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf[0];
 		uint16_t *nb_rx_desc = &ports[pid].nb_rx_desc[0];
 		uint16_t *nb_tx_desc = &ports[pid].nb_tx_desc[0];
+		struct rte_eth_rxq_info rx_qinfo;
+		struct rte_eth_txq_info tx_qinfo;
+		int32_t rc;
 
 		/* per port config */
 		printf("  port %d: RX queue number: %d Tx queue number: %d\n",
@@ -1850,6 +1853,10 @@ struct igb_ring_desc_16_bytes {
 
 		/* per rx queue config only for first queue to be less verbose */
 		for (qid = 0; qid < 1; qid++) {
+			rc = rte_eth_rx_queue_info_get(pid, qid, &rx_qinfo);
+			if (!rc)
+				nb_rx_desc[qid] = rx_qinfo.nb_desc;
+
 			printf("    RX queue: %d\n", qid);
 			printf("      RX desc=%d - RX free threshold=%d\n",
 				nb_rx_desc[qid], rx_conf[qid].rx_free_thresh);
@@ -1864,6 +1871,10 @@ struct igb_ring_desc_16_bytes {
 
 		/* per tx queue config only for first queue to be less verbose */
 		for (qid = 0; qid < 1; qid++) {
+			rc = rte_eth_tx_queue_info_get(pid, qid, &tx_qinfo);
+			if (!rc)
+				nb_tx_desc[qid] = tx_qinfo.nb_desc;
+
 			printf("    TX queue: %d\n", qid);
 			printf("      TX desc=%d - TX free threshold=%d\n",
 				nb_tx_desc[qid], tx_conf[qid].tx_free_thresh);
-- 
1.9.1

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix log of start command

[Ferruh Yigit]

On 5/22/2018 11:22 AM, Bernard Iremonger wrote:
> Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
> to update the number of rx and tx descriptors.
> 
> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  app/test-pmd/config.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 4520084..339651f 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1839,6 +1839,9 @@ struct igb_ring_desc_16_bytes {
>  		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf[0];
>  		uint16_t *nb_rx_desc = &ports[pid].nb_rx_desc[0];
>  		uint16_t *nb_tx_desc = &ports[pid].nb_tx_desc[0];
> +		struct rte_eth_rxq_info rx_qinfo;
> +		struct rte_eth_txq_info tx_qinfo;
> +		int32_t rc;
>  
>  		/* per port config */
>  		printf("  port %d: RX queue number: %d Tx queue number: %d\n",
> @@ -1850,6 +1853,10 @@ struct igb_ring_desc_16_bytes {
>  
>  		/* per rx queue config only for first queue to be less verbose */
>  		for (qid = 0; qid < 1; qid++) {
> +			rc = rte_eth_rx_queue_info_get(pid, qid, &rx_qinfo);
> +			if (!rc)
> +				nb_rx_desc[qid] = rx_qinfo.nb_desc;

Hi Bernard,

Isn't this doing same thing with previous patch, in a different location?

Do you think testpmd config variable should be updated?
Why not just use the value for print but not save it?

> +
>  			printf("    RX queue: %d\n", qid);
>  			printf("      RX desc=%d - RX free threshold=%d\n",
>  				nb_rx_desc[qid], rx_conf[qid].rx_free_thresh);
> @@ -1864,6 +1871,10 @@ struct igb_ring_desc_16_bytes {
>  
>  		/* per tx queue config only for first queue to be less verbose */
>  		for (qid = 0; qid < 1; qid++) {
> +			rc = rte_eth_tx_queue_info_get(pid, qid, &tx_qinfo);
> +			if (!rc)
> +				nb_tx_desc[qid] = tx_qinfo.nb_desc;
> +
>  			printf("    TX queue: %d\n", qid);
>  			printf("      TX desc=%d - TX free threshold=%d\n",
>  				nb_tx_desc[qid], tx_conf[qid].tx_free_thresh);
> 

Re: [dpdk-dev] [PATCH v3] app/testpmd: fix log of start command

[Iremonger, Bernard]

Hi  Ferruh,

<snip>

			nb_rx_desc[qid] = rx_qinfo.nb_desc;
> 
> Hi Bernard,
> 
> Isn't this doing same thing with previous patch, in a different location?
> 
> Do you think testpmd config variable should be updated?
> Why not just use the value for print but not save it?

Ok, I will send a v4

[dpdk-dev] [PATCH v4] app/testpmd: fix log of start command

[Bernard Iremonger]

Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
to update the number of rx and tx descriptors.

Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/config.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4520084..97020fb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1839,6 +1839,11 @@ struct igb_ring_desc_16_bytes {
 		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf[0];
 		uint16_t *nb_rx_desc = &ports[pid].nb_rx_desc[0];
 		uint16_t *nb_tx_desc = &ports[pid].nb_tx_desc[0];
+		uint16_t nb_rx_desc_tmp;
+		uint16_t nb_tx_desc_tmp;
+		struct rte_eth_rxq_info rx_qinfo;
+		struct rte_eth_txq_info tx_qinfo;
+		int32_t rc;
 
 		/* per port config */
 		printf("  port %d: RX queue number: %d Tx queue number: %d\n",
@@ -1850,9 +1855,15 @@ struct igb_ring_desc_16_bytes {
 
 		/* per rx queue config only for first queue to be less verbose */
 		for (qid = 0; qid < 1; qid++) {
+			rc = rte_eth_rx_queue_info_get(pid, qid, &rx_qinfo);
+			if (rc)
+				nb_rx_desc_tmp = nb_rx_desc[qid];
+			else
+				nb_rx_desc_tmp = rx_qinfo.nb_desc;
+
 			printf("    RX queue: %d\n", qid);
 			printf("      RX desc=%d - RX free threshold=%d\n",
-				nb_rx_desc[qid], rx_conf[qid].rx_free_thresh);
+				nb_rx_desc_tmp, rx_conf[qid].rx_free_thresh);
 			printf("      RX threshold registers: pthresh=%d hthresh=%d "
 				" wthresh=%d\n",
 				rx_conf[qid].rx_thresh.pthresh,
@@ -1864,9 +1875,15 @@ struct igb_ring_desc_16_bytes {
 
 		/* per tx queue config only for first queue to be less verbose */
 		for (qid = 0; qid < 1; qid++) {
+			rc = rte_eth_tx_queue_info_get(pid, qid, &tx_qinfo);
+			if (rc)
+				nb_tx_desc_tmp = nb_tx_desc[qid];
+			else
+				nb_tx_desc_tmp = tx_qinfo.nb_desc;
+
 			printf("    TX queue: %d\n", qid);
 			printf("      TX desc=%d - TX free threshold=%d\n",
-				nb_tx_desc[qid], tx_conf[qid].tx_free_thresh);
+				nb_tx_desc_tmp, tx_conf[qid].tx_free_thresh);
 			printf("      TX threshold registers: pthresh=%d hthresh=%d "
 				" wthresh=%d\n",
 				tx_conf[qid].tx_thresh.pthresh,
-- 
1.9.1

Re: [dpdk-dev] [PATCH v4] app/testpmd: fix log of start command

[Ferruh Yigit]

On 5/22/2018 4:10 PM, Bernard Iremonger wrote:
> Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
> to update the number of rx and tx descriptors.
> 
> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v4] app/testpmd: fix log of start command

[Ferruh Yigit]

On 5/22/2018 5:31 PM, Ferruh Yigit wrote:
> On 5/22/2018 4:10 PM, Bernard Iremonger wrote:
>> Call the rte_eth_rxq_info_get() and rte_eth_txq_info_get() functions
>> to update the number of rx and tx descriptors.
>>
>> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
>> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.