testpmd: add speed capability in device info
Checks
Commit Message
Comments
delay_us_sleep_autotest is failing on this patch. To replicate it, I
ran the same test on my system and it did not fail. Can this test be
re-run?
On Fri, Sep 4, 2020 at 11:23 AM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 30bee3324..8824ad174 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -518,6 +518,7 @@ 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];
>
> memset(&da, 0, sizeof(da));
> @@ -569,6 +570,90 @@ 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);
> + switch (dev_info.speed_capa) {
> + case ETH_LINK_SPEED_AUTONEG:
> + printf("\n\tDevice speed capability: %s",
> + "Autonegotiate (all speeds)");
> + break;
> + case ETH_LINK_SPEED_FIXED:
> + printf("\n\tDevice speed capability: %s",
> + "Disable autonegotiate (fixed speed)");
> + break;
> + case ETH_LINK_SPEED_10M_HD ...
> + ETH_LINK_SPEED_10M-1:
> + printf("\n\tDevice speed capability: %s",
> + "10 Mbps half-duplex");
> + break;
> + case ETH_LINK_SPEED_10M ...
> + ETH_LINK_SPEED_100M_HD-1:
> + printf("\n\tDevice speed capability: %s",
> + "10 Mbps full-duplex");
> + break;
> + case ETH_LINK_SPEED_100M_HD ...
> + ETH_LINK_SPEED_100M-1:
> + printf("\n\tDevice speed capability: %s",
> + "100 Mbps half-duplex");
> + break;
> + case ETH_LINK_SPEED_100M ...
> + ETH_LINK_SPEED_1G-1:
> + printf("\n\tDevice speed capability: %s",
> + "100 Mbps full-duplex");
> + break;
> + case ETH_LINK_SPEED_1G ...
> + ETH_LINK_SPEED_2_5G-1:
> + printf("\n\tDevice speed capability: %s",
> + "1 Gbps");
> + break;
> + case ETH_LINK_SPEED_2_5G ...
> + ETH_LINK_SPEED_5G-1:
> + printf("\n\tDevice speed capability: %s",
> + "2.5 Gbps");
> + break;
> + case ETH_LINK_SPEED_5G ...
> + ETH_LINK_SPEED_10G-1:
> + printf("\n\tDevice speed capability: %s",
> + "5 Gbps");
> + break;
> + case ETH_LINK_SPEED_10G ...
> + ETH_LINK_SPEED_20G-1:
> + printf("\n\tDevice speed capability: %s",
> + "10 Gbps");
> + break;
> + case ETH_LINK_SPEED_20G ...
> + ETH_LINK_SPEED_25G-1:
> + printf("\n\tDevice speed capability: %s",
> + "20 Gbps");
> + break;
> + case ETH_LINK_SPEED_25G ...
> + ETH_LINK_SPEED_50G-1:
> + printf("\n\tDevice speed capability: %s",
> + "25 Gbps");
> + break;
> + case ETH_LINK_SPEED_50G ...
> + ETH_LINK_SPEED_56G-1:
> + printf("\n\tDevice speed capability: %s",
> + "50 Gbps");
> + break;
> + case ETH_LINK_SPEED_56G ...
> + ETH_LINK_SPEED_100G-1:
> + printf("\n\tDevice speed capability: %s",
> + "56 Gbps");
> + break;
> + case ETH_LINK_SPEED_100G ...
> + ETH_LINK_SPEED_200G-1:
> + printf("\n\tDevice speed capability: %s",
> + "100 Gbps");
> + break;
> + case ETH_LINK_SPEED_200G:
> + printf("\n\tDevice speed capability: %s",
> + "200 Gbps");
> + break;
> + default:
> + printf("\n\tDevice speed capability: %s",
> + "not available");
> + break;
> + }
> printf("\n");
> }
> }
> --
> 2.25.1
>
On 9/8/2020 9:36 AM, Sarosh Arif wrote:
> delay_us_sleep_autotest is failing on this patch. To replicate it, I
> ran the same test on my system and it did not fail. Can this test be
> re-run?
cc'ed lab people.
I clicked the 'rebuild' button for the test, but I can't see if it queued or not...
>
> On Fri, Sep 4, 2020 at 11:23 AM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 30bee3324..8824ad174 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -518,6 +518,7 @@ 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];
>>
>> memset(&da, 0, sizeof(da));
>> @@ -569,6 +570,90 @@ 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);
>> + switch (dev_info.speed_capa) {
>> + case ETH_LINK_SPEED_AUTONEG:
>> + printf("\n\tDevice speed capability: %s",
>> + "Autonegotiate (all speeds)");
>> + break;
>> + case ETH_LINK_SPEED_FIXED:
>> + printf("\n\tDevice speed capability: %s",
>> + "Disable autonegotiate (fixed speed)");
>> + break;
>> + case ETH_LINK_SPEED_10M_HD ...
>> + ETH_LINK_SPEED_10M-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "10 Mbps half-duplex");
>> + break;
>> + case ETH_LINK_SPEED_10M ...
>> + ETH_LINK_SPEED_100M_HD-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "10 Mbps full-duplex");
>> + break;
>> + case ETH_LINK_SPEED_100M_HD ...
>> + ETH_LINK_SPEED_100M-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "100 Mbps half-duplex");
>> + break;
>> + case ETH_LINK_SPEED_100M ...
>> + ETH_LINK_SPEED_1G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "100 Mbps full-duplex");
>> + break;
>> + case ETH_LINK_SPEED_1G ...
>> + ETH_LINK_SPEED_2_5G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "1 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_2_5G ...
>> + ETH_LINK_SPEED_5G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "2.5 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_5G ...
>> + ETH_LINK_SPEED_10G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "5 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_10G ...
>> + ETH_LINK_SPEED_20G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "10 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_20G ...
>> + ETH_LINK_SPEED_25G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "20 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_25G ...
>> + ETH_LINK_SPEED_50G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "25 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_50G ...
>> + ETH_LINK_SPEED_56G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "50 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_56G ...
>> + ETH_LINK_SPEED_100G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "56 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_100G ...
>> + ETH_LINK_SPEED_200G-1:
>> + printf("\n\tDevice speed capability: %s",
>> + "100 Gbps");
>> + break;
>> + case ETH_LINK_SPEED_200G:
>> + printf("\n\tDevice speed capability: %s",
>> + "200 Gbps");
>> + break;
>> + default:
>> + printf("\n\tDevice speed capability: %s",
>> + "not available");
>> + break;
>> + }
>> printf("\n");
>> }
>> }
>> --
>> 2.25.1
>>
On 9/4/2020 7:23 AM, Sarosh Arif wrote:
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 30bee3324..8824ad174 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -518,6 +518,7 @@ 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];
This is for the testpmd command "show device info <identifier>|all",
not sure speed capabilities really fits the device info display.
"show port info <port_id>|all" command may be better fit, but before
that is there a specific need to see the speed capabilities of a port,
it may help figuring out right place.
>
> memset(&da, 0, sizeof(da));
> @@ -569,6 +570,90 @@ 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);
> + switch (dev_info.speed_capa) {
> + case ETH_LINK_SPEED_AUTONEG:
> + printf("\n\tDevice speed capability: %s",
> + "Autonegotiate (all speeds)");
> + break;
> + case ETH_LINK_SPEED_FIXED:
> + printf("\n\tDevice speed capability: %s",
> + "Disable autonegotiate (fixed speed)");
> + break;
> + case ETH_LINK_SPEED_10M_HD ...
> + ETH_LINK_SPEED_10M-1:
Why ranges are used, there can't be any value in between?
Also case range is not part of starndard, may be good to avoid for
portability, like the case -pendantic is used etc..
> + printf("\n\tDevice speed capability: %s",
> + "10 Mbps half-duplex");
> + break;
You should not break. 'speed_capa' is list of speeds that device
supports, so it won't be a single value, that is why breaking after
first hit is wrong.
Can you please confirm you intentions is not to display link speed, but
"speed capability"? Btw, link speed is already displayed in "show port
info ..."
> + case ETH_LINK_SPEED_10M ...
> + ETH_LINK_SPEED_100M_HD-1:
> + printf("\n\tDevice speed capability: %s",
> + "10 Mbps full-duplex");
Also no need to be this verbose, since there will be multiple values,
this makes to much noise, instead can be an list of speeds in single line.
On Thu, Sep 17, 2020 at 8:56 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 9/4/2020 7:23 AM, Sarosh Arif wrote:
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 30bee3324..8824ad174 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -518,6 +518,7 @@ 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];
>
> This is for the testpmd command "show device info <identifier>|all",
> not sure speed capabilities really fits the device info display.
>
> "show port info <port_id>|all" command may be better fit, but before
> that is there a specific need to see the speed capabilities of a port,
> it may help figuring out right place.
In bug 496 on bugzilla, it is requested to display speed capabilities
of a device,
in order to create a test to confirm the correct speed capabilities of a device.
Since speed_capa is property of the device it should be printed under
show device info.
>
> >
> > memset(&da, 0, sizeof(da));
> > @@ -569,6 +570,90 @@ 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);
> > + switch (dev_info.speed_capa) {
> > + case ETH_LINK_SPEED_AUTONEG:
> > + printf("\n\tDevice speed capability: %s",
> > + "Autonegotiate (all speeds)");
> > + break;
> > + case ETH_LINK_SPEED_FIXED:
> > + printf("\n\tDevice speed capability: %s",
> > + "Disable autonegotiate (fixed speed)");
> > + break;
> > + case ETH_LINK_SPEED_10M_HD ...
> > + ETH_LINK_SPEED_10M-1:
>
> Why ranges are used, there can't be any value in between?
>
It's a bit map so multiple bits can be one at a time resulting in
values in between.
> Also case range is not part of starndard, may be good to avoid for
> portability, like the case -pendantic is used etc..
I will submit v2 without using case range.
>
>
> > + printf("\n\tDevice speed capability: %s",
> > + "10 Mbps half-duplex");
> > + break;
>
> You should not break. 'speed_capa' is list of speeds that device
> supports, so it won't be a single value, that is why breaking after
> first hit is wrong.
I will correct this in v2
>
> Can you please confirm you intentions is not to display link speed, but
> "speed capability"? Btw, link speed is already displayed in "show port
> info ..."
The intention is to be able create a test to confirm the correct speed
capabilities of a device.
>
> > + case ETH_LINK_SPEED_10M ...
> > + ETH_LINK_SPEED_100M_HD-1:
> > + printf("\n\tDevice speed capability: %s",
> > + "10 Mbps full-duplex");
>
> Also no need to be this verbose, since there will be multiple values,
> this makes to much noise, instead can be an list of speeds in single line.
Okay
>
@@ -518,6 +518,7 @@ 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];
memset(&da, 0, sizeof(da));
@@ -569,6 +570,90 @@ 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);
+ switch (dev_info.speed_capa) {
+ case ETH_LINK_SPEED_AUTONEG:
+ printf("\n\tDevice speed capability: %s",
+ "Autonegotiate (all speeds)");
+ break;
+ case ETH_LINK_SPEED_FIXED:
+ printf("\n\tDevice speed capability: %s",
+ "Disable autonegotiate (fixed speed)");
+ break;
+ case ETH_LINK_SPEED_10M_HD ...
+ ETH_LINK_SPEED_10M-1:
+ printf("\n\tDevice speed capability: %s",
+ "10 Mbps half-duplex");
+ break;
+ case ETH_LINK_SPEED_10M ...
+ ETH_LINK_SPEED_100M_HD-1:
+ printf("\n\tDevice speed capability: %s",
+ "10 Mbps full-duplex");
+ break;
+ case ETH_LINK_SPEED_100M_HD ...
+ ETH_LINK_SPEED_100M-1:
+ printf("\n\tDevice speed capability: %s",
+ "100 Mbps half-duplex");
+ break;
+ case ETH_LINK_SPEED_100M ...
+ ETH_LINK_SPEED_1G-1:
+ printf("\n\tDevice speed capability: %s",
+ "100 Mbps full-duplex");
+ break;
+ case ETH_LINK_SPEED_1G ...
+ ETH_LINK_SPEED_2_5G-1:
+ printf("\n\tDevice speed capability: %s",
+ "1 Gbps");
+ break;
+ case ETH_LINK_SPEED_2_5G ...
+ ETH_LINK_SPEED_5G-1:
+ printf("\n\tDevice speed capability: %s",
+ "2.5 Gbps");
+ break;
+ case ETH_LINK_SPEED_5G ...
+ ETH_LINK_SPEED_10G-1:
+ printf("\n\tDevice speed capability: %s",
+ "5 Gbps");
+ break;
+ case ETH_LINK_SPEED_10G ...
+ ETH_LINK_SPEED_20G-1:
+ printf("\n\tDevice speed capability: %s",
+ "10 Gbps");
+ break;
+ case ETH_LINK_SPEED_20G ...
+ ETH_LINK_SPEED_25G-1:
+ printf("\n\tDevice speed capability: %s",
+ "20 Gbps");
+ break;
+ case ETH_LINK_SPEED_25G ...
+ ETH_LINK_SPEED_50G-1:
+ printf("\n\tDevice speed capability: %s",
+ "25 Gbps");
+ break;
+ case ETH_LINK_SPEED_50G ...
+ ETH_LINK_SPEED_56G-1:
+ printf("\n\tDevice speed capability: %s",
+ "50 Gbps");
+ break;
+ case ETH_LINK_SPEED_56G ...
+ ETH_LINK_SPEED_100G-1:
+ printf("\n\tDevice speed capability: %s",
+ "56 Gbps");
+ break;
+ case ETH_LINK_SPEED_100G ...
+ ETH_LINK_SPEED_200G-1:
+ printf("\n\tDevice speed capability: %s",
+ "100 Gbps");
+ break;
+ case ETH_LINK_SPEED_200G:
+ printf("\n\tDevice speed capability: %s",
+ "200 Gbps");
+ break;
+ default:
+ printf("\n\tDevice speed capability: %s",
+ "not available");
+ break;
+ }
printf("\n");
}
}