[dpdk-dev] app/testpmd: print Rx/Tx offload values

Message ID 20180306162822.61791-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit March 6, 2018, 4:28 p.m. UTC
  It is not clear which per port offloads are enabled. Printing offloads
values at forwarding start.

CRC strip offload value was printed in more verbose manner, it is
removed since Rx/Tx offload values covers it and printing only CRC one
can cause confusion.

Hexadecimal offloads values are not very user friendly but preferred to
not create to much noise during forwarding start.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Shahaf Shuler <shahafs@mellanox.com>
---
 app/test-pmd/config.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
  

Comments

Yongseok Koh March 7, 2018, 9:36 p.m. UTC | #1
> On Mar 6, 2018, at 8:28 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> It is not clear which per port offloads are enabled. Printing offloads
> values at forwarding start.
> 
> CRC strip offload value was printed in more verbose manner, it is
> removed since Rx/Tx offload values covers it and printing only CRC one
> can cause confusion.
> 
> Hexadecimal offloads values are not very user friendly but preferred to
> not create to much noise during forwarding start.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Shahaf Shuler <shahafs@mellanox.com>
> ---
> app/test-pmd/config.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 4bb255c62..47845d0cb 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1682,10 +1682,9 @@ rxtx_config_display(void)
> 		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf;
> 
> 		printf("  port %d:\n", (unsigned int)pid);
> -		printf("  CRC stripping %s\n",
> -				(ports[pid].dev_conf.rxmode.offloads &
> -				 DEV_RX_OFFLOAD_CRC_STRIP) ?
> -				"enabled" : "disabled");
> +		printf("  Rx offloads=0x%"PRIx64" Tx Offloads=0x%"PRIx64"\n",
> +				ports[pid].dev_conf.rxmode.offloads,
> +				ports[pid].dev_conf.txmode.offloads);

So, if someone wants to know what these values mean, they should type
'show port cap all'.

BTW, tx_conf->offloads is printed a few lines later, then isn't it necessary to
print rx_conf->offloads as well? Or, is it supposed to be identical to
dev_conf.rxmode.offloads?

Thanks,
Yongseok
  
Ferruh Yigit March 9, 2018, 6:27 p.m. UTC | #2
On 3/7/2018 9:36 PM, Yongseok Koh wrote:
> 
>> On Mar 6, 2018, at 8:28 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> It is not clear which per port offloads are enabled. Printing offloads
>> values at forwarding start.
>>
>> CRC strip offload value was printed in more verbose manner, it is
>> removed since Rx/Tx offload values covers it and printing only CRC one
>> can cause confusion.
>>
>> Hexadecimal offloads values are not very user friendly but preferred to
>> not create to much noise during forwarding start.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Shahaf Shuler <shahafs@mellanox.com>
>> ---
>> app/test-pmd/config.c | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 4bb255c62..47845d0cb 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1682,10 +1682,9 @@ rxtx_config_display(void)
>> 		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf;
>>
>> 		printf("  port %d:\n", (unsigned int)pid);
>> -		printf("  CRC stripping %s\n",
>> -				(ports[pid].dev_conf.rxmode.offloads &
>> -				 DEV_RX_OFFLOAD_CRC_STRIP) ?
>> -				"enabled" : "disabled");
>> +		printf("  Rx offloads=0x%"PRIx64" Tx Offloads=0x%"PRIx64"\n",
>> +				ports[pid].dev_conf.rxmode.offloads,
>> +				ports[pid].dev_conf.txmode.offloads);
> 
> So, if someone wants to know what these values mean, they should type
> 'show port cap all'.

Indeed I forget about 'show port cap all', it is better way to display offloads,
still I believe it is good a have a brief info in the start, especially it
already has "tx_conf->offloads"

btw, 'show port cap all' is missing some offload values, it needs to be updated.

> 
> BTW, tx_conf->offloads is printed a few lines later, then isn't it necessary to
> print rx_conf->offloads as well? Or, is it supposed to be identical to
> dev_conf.rxmode.offloads?

They may be different, good idea to add rx_conf->offloads as well. I will add it
in v2.

Thanks,
ferruh

> 
> Thanks,
> Yongseok
>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4bb255c62..47845d0cb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1682,10 +1682,9 @@  rxtx_config_display(void)
 		struct rte_eth_txconf *tx_conf = &ports[pid].tx_conf;
 
 		printf("  port %d:\n", (unsigned int)pid);
-		printf("  CRC stripping %s\n",
-				(ports[pid].dev_conf.rxmode.offloads &
-				 DEV_RX_OFFLOAD_CRC_STRIP) ?
-				"enabled" : "disabled");
+		printf("  Rx offloads=0x%"PRIx64" Tx Offloads=0x%"PRIx64"\n",
+				ports[pid].dev_conf.rxmode.offloads,
+				ports[pid].dev_conf.txmode.offloads);
 		printf("  RX queues=%d - RX desc=%d - RX free threshold=%d\n",
 				nb_rxq, nb_rxd, rx_conf->rx_free_thresh);
 		printf("  RX threshold registers: pthresh=%d hthresh=%d "