diff mbox series

[v2] testpmd: add speed capability in device info

Message ID 20200928100121.3332768-1-sarosh.arif@emumba.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] testpmd: add speed capability in device info |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Sarosh Arif Sept. 28, 2020, 10:01 a.m. UTC 
  Called rte_eth_dev_info_get() in testpmd, to get device info
so that speed capabilities can be printed under "show device info"
​
Bugzilla ID: 496
Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
 app/test-pmd/config.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

Comments

Asaf Penso Oct. 4, 2020, 8:51 a.m. UTC | #1 
>-----Original Message-----
>From: dev <dev-bounces@dpdk.org> On Behalf Of Sarosh Arif
>Sent: Monday, September 28, 2020 1:01 PM
>To: ferruh.yigit@intel.com
>Cc: dev@dpdk.org; Sarosh Arif <sarosh.arif@emumba.com>
>Subject: [dpdk-dev] [PATCH v2] testpmd: add speed capability in device info
>
>Called rte_eth_dev_info_get() in testpmd, to get device info so that speed
>capabilities can be printed under "show device info"
>​
>Bugzilla ID: 496
>Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
>---
> app/test-pmd/config.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
>diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>30bee3324..e2b3975a3 100644
>--- a/app/test-pmd/config.c
>+++ b/app/test-pmd/config.c
>@@ -518,7 +518,9 @@ device_infos_display(const char *identifier)
> 	struct rte_device *dev;
> 	struct rte_devargs da;
> 	portid_t port_id;
>+	struct rte_eth_dev_info dev_info;
> 	char devstr[128];
>+	uint32_t speed_capa;
>
> 	memset(&da, 0, sizeof(da));
> 	if (!identifier)
>@@ -569,6 +571,42 @@ device_infos_display(const char *identifier)
> 						      &mac_addr);
> 				rte_eth_dev_get_name_by_port(port_id,
>name);
> 				printf("\n\tDevice name: %s", name);

How about extracting the below to a dedicated helper function to make the code more readable?
Something like: void device_infos_display_speeds(uint32_t speed_capa)

>+				rte_eth_dev_info_get(port_id, &dev_info);
>+				speed_capa = dev_info.speed_capa;
>+
>+				printf("\n\tDevice speed capability:");
>+				if (speed_capa ==
>ETH_LINK_SPEED_AUTONEG)
>+					printf(" Autonegotiate (all speeds)");
>+				if (speed_capa & ETH_LINK_SPEED_FIXED)
>+					printf(" Disable autonegotiate (fixed
>speed)  ");
>+				if (speed_capa & ETH_LINK_SPEED_10M_HD)
>+					printf(" 10 Mbps half-duplex  ");
>+				if (speed_capa & ETH_LINK_SPEED_10M)
>+					printf(" 10 Mbps full-duplex  ");
>+				if (speed_capa & ETH_LINK_SPEED_100M_HD)
>+					printf(" 100 Mbps half-duplex  ");
>+				if (speed_capa & ETH_LINK_SPEED_100M)
>+					printf(" 100 Mbps full-duplex  ");
>+				if (speed_capa & ETH_LINK_SPEED_1G)
>+					printf(" 1 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_2_5G)
>+					printf(" 2.5 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_5G)
>+					printf(" 5 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_10G)
>+					printf(" 10 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_20G)
>+					printf(" 20 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_25G)
>+					printf(" 25 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_50G)
>+					printf(" 50 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_56G)
>+					printf(" 56 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_100G)
>+					printf(" 100 Gbps  ");
>+				if (speed_capa & ETH_LINK_SPEED_200G)
>+					printf(" 200 Gbps  ");
> 				printf("\n");
> 			}
> 		}
>--
>2.25.1

I'm ok also to leave is as-is.
Reviewed-By: Asaf Penso <asafp@nvidia.com>
Ferruh Yigit Oct. 7, 2020, 3:30 p.m. UTC | #2 
On 10/4/2020 9:51 AM, Asaf Penso wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Sarosh Arif
>> Sent: Monday, September 28, 2020 1:01 PM
>> To: ferruh.yigit@intel.com
>> Cc: dev@dpdk.org; Sarosh Arif <sarosh.arif@emumba.com>
>> Subject: [dpdk-dev] [PATCH v2] testpmd: add speed capability in device info
>>
>> Called rte_eth_dev_info_get() in testpmd, to get device info so that speed
>> capabilities can be printed under "show device info"
>> ​
>> Bugzilla ID: 496
>> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
>> ---
>> app/test-pmd/config.c | 38 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>> 30bee3324..e2b3975a3 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -518,7 +518,9 @@ device_infos_display(const char *identifier)
>> 	struct rte_device *dev;
>> 	struct rte_devargs da;
>> 	portid_t port_id;
>> +	struct rte_eth_dev_info dev_info;
>> 	char devstr[128];
>> +	uint32_t speed_capa;
>>
>> 	memset(&da, 0, sizeof(da));
>> 	if (!identifier)
>> @@ -569,6 +571,42 @@ device_infos_display(const char *identifier)
>> 						      &mac_addr);
>> 				rte_eth_dev_get_name_by_port(port_id,
>> name);
>> 				printf("\n\tDevice name: %s", name);
> 
> How about extracting the below to a dedicated helper function to make the code more readable?
> Something like: void device_infos_display_speeds(uint32_t speed_capa)
> 
>> +				rte_eth_dev_info_get(port_id, &dev_info);
>> +				speed_capa = dev_info.speed_capa;
>> +
>> +				printf("\n\tDevice speed capability:");
>> +				if (speed_capa ==
>> ETH_LINK_SPEED_AUTONEG)
>> +					printf(" Autonegotiate (all speeds)");
>> +				if (speed_capa & ETH_LINK_SPEED_FIXED)
>> +					printf(" Disable autonegotiate (fixed
>> speed)  ");
>> +				if (speed_capa & ETH_LINK_SPEED_10M_HD)
>> +					printf(" 10 Mbps half-duplex  ");
>> +				if (speed_capa & ETH_LINK_SPEED_10M)
>> +					printf(" 10 Mbps full-duplex  ");
>> +				if (speed_capa & ETH_LINK_SPEED_100M_HD)
>> +					printf(" 100 Mbps half-duplex  ");
>> +				if (speed_capa & ETH_LINK_SPEED_100M)
>> +					printf(" 100 Mbps full-duplex  ");
>> +				if (speed_capa & ETH_LINK_SPEED_1G)
>> +					printf(" 1 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_2_5G)
>> +					printf(" 2.5 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_5G)
>> +					printf(" 5 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_10G)
>> +					printf(" 10 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_20G)
>> +					printf(" 20 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_25G)
>> +					printf(" 25 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_50G)
>> +					printf(" 50 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_56G)
>> +					printf(" 56 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_100G)
>> +					printf(" 100 Gbps  ");
>> +				if (speed_capa & ETH_LINK_SPEED_200G)
>> +					printf(" 200 Gbps  ");
>> 				printf("\n");

Some speeds are missing, please add all.

And +1 to Asaf's comment to extract it into its own function.
	

>> 			}
>> 		}
>> --
>> 2.25.1
> 
> I'm ok also to leave is as-is.
> Reviewed-By: Asaf Penso <asafp@nvidia.com>
>
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee3324..e2b3975a3 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -518,7 +518,9 @@  device_infos_display(const char *identifier)
 	struct rte_device *dev;
 	struct rte_devargs da;
 	portid_t port_id;
+	struct rte_eth_dev_info dev_info;
 	char devstr[128];
+	uint32_t speed_capa;
 
 	memset(&da, 0, sizeof(da));
 	if (!identifier)
@@ -569,6 +571,42 @@  device_infos_display(const char *identifier)
 						      &mac_addr);
 				rte_eth_dev_get_name_by_port(port_id, name);
 				printf("\n\tDevice name: %s", name);
+				rte_eth_dev_info_get(port_id, &dev_info);
+				speed_capa = dev_info.speed_capa;
+
+				printf("\n\tDevice speed capability:");
+				if (speed_capa == ETH_LINK_SPEED_AUTONEG)
+					printf(" Autonegotiate (all speeds)");
+				if (speed_capa & ETH_LINK_SPEED_FIXED)
+					printf(" Disable autonegotiate (fixed speed)  ");
+				if (speed_capa & ETH_LINK_SPEED_10M_HD)
+					printf(" 10 Mbps half-duplex  ");
+				if (speed_capa & ETH_LINK_SPEED_10M)
+					printf(" 10 Mbps full-duplex  ");
+				if (speed_capa & ETH_LINK_SPEED_100M_HD)
+					printf(" 100 Mbps half-duplex  ");
+				if (speed_capa & ETH_LINK_SPEED_100M)
+					printf(" 100 Mbps full-duplex  ");
+				if (speed_capa & ETH_LINK_SPEED_1G)
+					printf(" 1 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_2_5G)
+					printf(" 2.5 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_5G)
+					printf(" 5 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_10G)
+					printf(" 10 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_20G)
+					printf(" 20 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_25G)
+					printf(" 25 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_50G)
+					printf(" 50 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_56G)
+					printf(" 56 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_100G)
+					printf(" 100 Gbps  ");
+				if (speed_capa & ETH_LINK_SPEED_200G)
+					printf(" 200 Gbps  ");
 				printf("\n");
 			}
 		}