[dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 4953 bytes --]

Add support for forced ethernet speed setting.
Currently testpmd tries to configure the Ethernet port in autoneg mode.
It is not possible to set the Ethernet port to a specific speed while
starting testpmd. In some cases capability to configure a forced speed
for the Ethernet port during initialization may be necessary. This patch
tries to add this support.

The patch assumes full duplex setting and does not attempt to change that.
So speeds like 10M, 100M are not configurable using this method.

The command line to configure a forced speed of 10G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000

The command line to configure a forced speed of 50G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.c                |  4 +++
 app/test-pmd/testpmd.h                |  1 +
 doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
 4 files changed, 58 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c8acd5d1b7..e10f7d38fb 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -224,6 +224,7 @@ usage(char* progname)
 	printf("  --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n "
 	       "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
 	       "    0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
+	printf("  --eth-link-speed: forced link speed.\n");
 }
 
 #ifdef RTE_LIB_CMDLINE
@@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg, int enable)
 	return 0;
 }
 
+static int
+parse_link_speed(int n)
+{
+	uint32_t speed;
+
+	switch (n) {
+	case 1000:
+		speed = ETH_LINK_SPEED_1G;
+		break;
+	case 10000:
+		speed = ETH_LINK_SPEED_10G;
+		break;
+	case 25000:
+		speed = ETH_LINK_SPEED_25G;
+		break;
+	case 40000:
+		speed = ETH_LINK_SPEED_40G;
+		break;
+	case 50000:
+		speed = ETH_LINK_SPEED_50G;
+		break;
+	case 100000:
+		speed = ETH_LINK_SPEED_100G;
+		break;
+	case 200000:
+		speed = ETH_LINK_SPEED_200G;
+		break;
+	default:
+		speed = ETH_LINK_SPEED_AUTONEG;
+		break;
+	}
+
+	return speed;
+}
+
 void
 launch_args_parse(int argc, char** argv)
 {
@@ -605,6 +641,7 @@ launch_args_parse(int argc, char** argv)
 		{ "rx-mq-mode",                 1, 0, 0 },
 		{ "record-core-cycles",         0, 0, 0 },
 		{ "record-burst-stats",         0, 0, 0 },
+		{ "eth-link-speed",		1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1366,6 +1403,11 @@ launch_args_parse(int argc, char** argv)
 				record_core_cycles = 1;
 			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
 				record_burst_stats = 1;
+			if (!strcmp(lgopts[opt_idx].name, "eth-link-speed")) {
+				n = atoi(optarg);
+				if (n >= 0 && parse_link_speed(n) >= 0)
+					eth_link_speed = parse_link_speed(n);
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index caa711d6f3..9434e335a0 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -535,6 +535,8 @@ uint16_t gso_max_segment_size = RTE_ETHER_MAX_LEN - RTE_ETHER_CRC_LEN;
 /* Holds the registered mbuf dynamic flags names. */
 char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
 
+uint32_t eth_link_speed;
+
 /*
  * Helper function to check if socket is already discovered.
  * If yes, return positive value. If not, return zero.
@@ -1484,6 +1486,8 @@ init_config(void)
 			port->tx_conf[k].offloads =
 				port->dev_conf.txmode.offloads;
 
+		port->dev_conf.link_speeds = eth_link_speed;
+
 		/* set flag to initialize port/queue */
 		port->need_reconfig = 1;
 		port->need_reconfig_queues = 1;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 60ddeb8f13..a3cd4a0e16 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -351,6 +351,7 @@ extern bool setup_on_probe_event; /**< disabled by port setup-on iterator */
 extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
 extern int do_mlockall; /**< set by "--mlockall" or "--no-mlockall" parameter */
 extern uint8_t clear_ptypes; /**< disabled by set ptype cmd */
+extern uint32_t eth_link_speed;
 
 #ifdef RTE_LIBRTE_IXGBE_BYPASS
 extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog timeout */
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 6745072329..a856f52736 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -536,3 +536,14 @@ The command line options are:
     bit 1 - two hairpin ports paired
     bit 0 - two hairpin ports loop
     The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
+
+*   ``--eth-link-speed``
+
+    Set a forced link speed to the ethernet port.
+    1000 - 1Gbps
+    10000 - 10Gbps
+    25000 - 25Gbps
+    40000 - 40Gbps
+    50000 - 50Gbps
+    100000 - 100Gbps
+    200000 - 200Gbps
-- 
2.21.1 (Apple Git-122.3)

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
> Add support for forced ethernet speed setting.
> Currently testpmd tries to configure the Ethernet port in autoneg mode.
> It is not possible to set the Ethernet port to a specific speed while
> starting testpmd. In some cases capability to configure a forced speed
> for the Ethernet port during initialization may be necessary. This patch
> tries to add this support.
> 
> The patch assumes full duplex setting and does not attempt to change that.
> So speeds like 10M, 100M are not configurable using this method.
> 
> The command line to configure a forced speed of 10G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> 
> The command line to configure a forced speed of 50G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>   app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>   app/test-pmd/testpmd.c                |  4 +++
>   app/test-pmd/testpmd.h                |  1 +
>   doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>   4 files changed, 58 insertions(+)

Can you also update the release notes to document the new parameter?

> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index c8acd5d1b7..e10f7d38fb 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -224,6 +224,7 @@ usage(char* progname)
>   	printf("  --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n "
>   	       "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>   	       "    0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
> +	printf("  --eth-link-speed: forced link speed.\n");
>   }
>   
>   #ifdef RTE_LIB_CMDLINE
> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg, int enable)
>   	return 0;
>   }
>   
> +static int
> +parse_link_speed(int n)
> +{
> +	uint32_t speed;
> +
> +	switch (n) {

OK to not support "10M, 100M", not sure if anybody really uses them, but what do 
you think checking them and return an error?

> +	case 1000:
> +		speed = ETH_LINK_SPEED_1G;
> +		break;
> +	case 10000:
> +		speed = ETH_LINK_SPEED_10G;
> +		break;
> +	case 25000:
> +		speed = ETH_LINK_SPEED_25G;
> +		break;
> +	case 40000:
> +		speed = ETH_LINK_SPEED_40G;
> +		break;
> +	case 50000:
> +		speed = ETH_LINK_SPEED_50G;
> +		break;
> +	case 100000:
> +		speed = ETH_LINK_SPEED_100G;
> +		break;
> +	case 200000:
> +		speed = ETH_LINK_SPEED_200G;
> +		break;
> +	default:
> +		speed = ETH_LINK_SPEED_AUTONEG;
> +		break;

Isn't this function to set a fixed link speed, why falling back to autoneg?

Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?

> +	}
> +
> +	return speed;
> +}
> +
>   void
>   launch_args_parse(int argc, char** argv)
>   {
> @@ -605,6 +641,7 @@ launch_args_parse(int argc, char** argv)
>   		{ "rx-mq-mode",                 1, 0, 0 },
>   		{ "record-core-cycles",         0, 0, 0 },
>   		{ "record-burst-stats",         0, 0, 0 },
> +		{ "eth-link-speed",		1, 0, 0 },
>   		{ 0, 0, 0, 0 },
>   	};
>   
> @@ -1366,6 +1403,11 @@ launch_args_parse(int argc, char** argv)
>   				record_core_cycles = 1;
>   			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
>   				record_burst_stats = 1;
> +			if (!strcmp(lgopts[opt_idx].name, "eth-link-speed")) {
> +				n = atoi(optarg);
> +				if (n >= 0 && parse_link_speed(n) >= 0)
> +					eth_link_speed = parse_link_speed(n);
> +			}
>   			break;
>   		case 'h':
>   			usage(argv[0]);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index caa711d6f3..9434e335a0 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -535,6 +535,8 @@ uint16_t gso_max_segment_size = RTE_ETHER_MAX_LEN - RTE_ETHER_CRC_LEN;
>   /* Holds the registered mbuf dynamic flags names. */
>   char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
>   
> +uint32_t eth_link_speed;
> +

Can you please move this a little up, at the beginning of the 'testpmd.c' there 
are global config variables, this can go next to them.

And can you please add a single line comment to the variable, as done in all 
other ones.

>   /*
>    * Helper function to check if socket is already discovered.
>    * If yes, return positive value. If not, return zero.
> @@ -1484,6 +1486,8 @@ init_config(void)
>   			port->tx_conf[k].offloads =
>   				port->dev_conf.txmode.offloads;
>   
> +		port->dev_conf.link_speeds = eth_link_speed;
> +

This is set even user doesn't provide '--eth-link-speed' at all, in that case I 
assume it relies on the fact that variable default value (0) is the same as 
'AUTONEG' value (0). So it sets speed to autoneg in that case.
But what do you think to be more explicit, and set this only if the user 
provided the device argument, like:
	if (eth_link_speed)
		port->dev_conf.link_speeds = eth_link_speed;


This sets the same link speed for all ports, do you think may we need capability 
to set link speed per port? Does it worth the complexity it brings?

>   		/* set flag to initialize port/queue */
>   		port->need_reconfig = 1;
>   		port->need_reconfig_queues = 1;
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 60ddeb8f13..a3cd4a0e16 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -351,6 +351,7 @@ extern bool setup_on_probe_event; /**< disabled by port setup-on iterator */
>   extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
>   extern int do_mlockall; /**< set by "--mlockall" or "--no-mlockall" parameter */
>   extern uint8_t clear_ptypes; /**< disabled by set ptype cmd */
> +extern uint32_t eth_link_speed;
>   
>   #ifdef RTE_LIBRTE_IXGBE_BYPASS
>   extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog timeout */
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
> index 6745072329..a856f52736 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -536,3 +536,14 @@ The command line options are:
>       bit 1 - two hairpin ports paired
>       bit 0 - two hairpin ports loop
>       The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
> +
> +*   ``--eth-link-speed``
> +

Should this document that "10M, 100M" is not supported?

> +    Set a forced link speed to the ethernet port.
> +    1000 - 1Gbps
> +    10000 - 10Gbps
> +    25000 - 25Gbps
> +    40000 - 40Gbps
> +    50000 - 50Gbps
> +    100000 - 100Gbps
> +    200000 - 200Gbps

What do you think to put something like "..." to very bottom, to clarify this is 
not full list, so that we won't need to update this each time we add a new speed?

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Andrew Boyer]

> On Feb 22, 2021, at 2:18 PM, Ajit Khaparde <ajit.khaparde@broadcom.com> wrote:
> 
> Add support for forced ethernet speed setting.
> Currently testpmd tries to configure the Ethernet port in autoneg mode.
> It is not possible to set the Ethernet port to a specific speed while
> starting testpmd. In some cases capability to configure a forced speed
> for the Ethernet port during initialization may be necessary. This patch
> tries to add this support.
> 
> The patch assumes full duplex setting and does not attempt to change that.
> So speeds like 10M, 100M are not configurable using this method.
> 
> The command line to configure a forced speed of 10G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> 
> The command line to configure a forced speed of 50G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
...
> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg, int enable)
> 	return 0;
> }
> 
> +static int
> +parse_link_speed(int n)
> +{
> +	uint32_t speed;
> +
> +	switch (n) {
> +	case 1000:
> +		speed = ETH_LINK_SPEED_1G;
> +		break;
> +	case 10000:
> +		speed = ETH_LINK_SPEED_10G;
> +		break;
> +	case 25000:
> +		speed = ETH_LINK_SPEED_25G;
> +		break;
> +	case 40000:
> +		speed = ETH_LINK_SPEED_40G;
> +		break;
> +	case 50000:
> +		speed = ETH_LINK_SPEED_50G;
> +		break;
> +	case 100000:
> +		speed = ETH_LINK_SPEED_100G;
> +		break;
> +	case 200000:
> +		speed = ETH_LINK_SPEED_200G;
> +		break;

Shouldn’t all of these fixed values be OR’d with ETH_LINK_SPEED_FIXED?

The testpmd command to change speed is also missing it.

-Andrew

...

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Andrew Rybchenko]

On 2/25/21 9:25 PM, Ferruh Yigit wrote:
> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
>> Add support for forced ethernet speed setting.
>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
>> It is not possible to set the Ethernet port to a specific speed while
>> starting testpmd. In some cases capability to configure a forced speed
>> for the Ethernet port during initialization may be necessary. This patch
>> tries to add this support.
>>
>> The patch assumes full duplex setting and does not attempt to change
>> that.
>> So speeds like 10M, 100M are not configurable using this method.
>>
>> The command line to configure a forced speed of 10G:
>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
>>
>> The command line to configure a forced speed of 50G:
>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
>>
>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> ---
>>   app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>>   app/test-pmd/testpmd.c                |  4 +++
>>   app/test-pmd/testpmd.h                |  1 +
>>   doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>>   4 files changed, 58 insertions(+)
> 
> Can you also update the release notes to document the new parameter?
> 
>>
>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>> index c8acd5d1b7..e10f7d38fb 100644
>> --- a/app/test-pmd/parameters.c
>> +++ b/app/test-pmd/parameters.c
>> @@ -224,6 +224,7 @@ usage(char* progname)
>>       printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
>> mode.\n "
>>              "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>>              "    0x01 - hairpin ports loop, 0x00 - hairpin port
>> self\n");
>> +    printf("  --eth-link-speed: forced link speed.\n");
>>   }
>>     #ifdef RTE_LIB_CMDLINE
>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
>> int enable)
>>       return 0;
>>   }
>>   +static int
>> +parse_link_speed(int n)
>> +{
>> +    uint32_t speed;
>> +
>> +    switch (n) {
> 
> OK to not support "10M, 100M", not sure if anybody really uses them, but
> what do you think checking them and return an error?
> 
>> +    case 1000:
>> +        speed = ETH_LINK_SPEED_1G;
>> +        break;
>> +    case 10000:
>> +        speed = ETH_LINK_SPEED_10G;
>> +        break;
>> +    case 25000:
>> +        speed = ETH_LINK_SPEED_25G;
>> +        break;
>> +    case 40000:
>> +        speed = ETH_LINK_SPEED_40G;
>> +        break;
>> +    case 50000:
>> +        speed = ETH_LINK_SPEED_50G;
>> +        break;
>> +    case 100000:
>> +        speed = ETH_LINK_SPEED_100G;
>> +        break;
>> +    case 200000:
>> +        speed = ETH_LINK_SPEED_200G;
>> +        break;
>> +    default:
>> +        speed = ETH_LINK_SPEED_AUTONEG;
>> +        break;
> 
> Isn't this function to set a fixed link speed, why falling back to autoneg?
> 
> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?

It should. Previous time I've tried to fix corresponding
bug in CLI commands, it ended up with rollback because
of Intel drivers do not handle it correctly.

See "app/testpmd: set fixed flag for exact link speed" and
corresponding revert.

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 2/26/2021 6:43 AM, Andrew Rybchenko wrote:
> On 2/25/21 9:25 PM, Ferruh Yigit wrote:
>> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
>>> Add support for forced ethernet speed setting.
>>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
>>> It is not possible to set the Ethernet port to a specific speed while
>>> starting testpmd. In some cases capability to configure a forced speed
>>> for the Ethernet port during initialization may be necessary. This patch
>>> tries to add this support.
>>>
>>> The patch assumes full duplex setting and does not attempt to change
>>> that.
>>> So speeds like 10M, 100M are not configurable using this method.
>>>
>>> The command line to configure a forced speed of 10G:
>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
>>>
>>> The command line to configure a forced speed of 50G:
>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
>>>
>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>>    app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>>>    app/test-pmd/testpmd.c                |  4 +++
>>>    app/test-pmd/testpmd.h                |  1 +
>>>    doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>>>    4 files changed, 58 insertions(+)
>>
>> Can you also update the release notes to document the new parameter?
>>
>>>
>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>> index c8acd5d1b7..e10f7d38fb 100644
>>> --- a/app/test-pmd/parameters.c
>>> +++ b/app/test-pmd/parameters.c
>>> @@ -224,6 +224,7 @@ usage(char* progname)
>>>        printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
>>> mode.\n "
>>>               "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>>>               "    0x01 - hairpin ports loop, 0x00 - hairpin port
>>> self\n");
>>> +    printf("  --eth-link-speed: forced link speed.\n");
>>>    }
>>>      #ifdef RTE_LIB_CMDLINE
>>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
>>> int enable)
>>>        return 0;
>>>    }
>>>    +static int
>>> +parse_link_speed(int n)
>>> +{
>>> +    uint32_t speed;
>>> +
>>> +    switch (n) {
>>
>> OK to not support "10M, 100M", not sure if anybody really uses them, but
>> what do you think checking them and return an error?
>>
>>> +    case 1000:
>>> +        speed = ETH_LINK_SPEED_1G;
>>> +        break;
>>> +    case 10000:
>>> +        speed = ETH_LINK_SPEED_10G;
>>> +        break;
>>> +    case 25000:
>>> +        speed = ETH_LINK_SPEED_25G;
>>> +        break;
>>> +    case 40000:
>>> +        speed = ETH_LINK_SPEED_40G;
>>> +        break;
>>> +    case 50000:
>>> +        speed = ETH_LINK_SPEED_50G;
>>> +        break;
>>> +    case 100000:
>>> +        speed = ETH_LINK_SPEED_100G;
>>> +        break;
>>> +    case 200000:
>>> +        speed = ETH_LINK_SPEED_200G;
>>> +        break;
>>> +    default:
>>> +        speed = ETH_LINK_SPEED_AUTONEG;
>>> +        break;
>>
>> Isn't this function to set a fixed link speed, why falling back to autoneg?
>>
>> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
> 
> It should. Previous time I've tried to fix corresponding
> bug in CLI commands, it ended up with rollback because
> of Intel drivers do not handle it correctly.
> 
> See "app/testpmd: set fixed flag for exact link speed" and
> corresponding revert.
> 

Thanks for the reminder Andrew, you have a good memory :)
For reference: 
http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/

It seems that patch reverted with the pressure of the release, what do you think 
applying it again while we have enough time to fix the PMDs before release?

 From previous discussions, long term actions listed as:
"
1) Implement 'fixed' link speed support in the missing drivers.
2) Send a new version of the testpmd patch with a "fixed" argument, so that we
can support all three above
"

Not sure having (2) explicitly is required, we have already "auto" speed, not 
having it implies the fixed speed.
So we can just re-apply your old patch.

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Andrew Boyer]

> On Feb 26, 2021, at 6:21 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 2/26/2021 6:43 AM, Andrew Rybchenko wrote:
>> On 2/25/21 9:25 PM, Ferruh Yigit wrote:
>>> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
>>>> Add support for forced ethernet speed setting.
>>>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
>>>> It is not possible to set the Ethernet port to a specific speed while
>>>> starting testpmd. In some cases capability to configure a forced speed
>>>> for the Ethernet port during initialization may be necessary. This patch
>>>> tries to add this support.
>>>> 
>>>> The patch assumes full duplex setting and does not attempt to change
>>>> that.
>>>> So speeds like 10M, 100M are not configurable using this method.
>>>> 
>>>> The command line to configure a forced speed of 10G:
>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
>>>> 
>>>> The command line to configure a forced speed of 50G:
>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
>>>> 
>>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> ---
>>>>   app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>>>>   app/test-pmd/testpmd.c                |  4 +++
>>>>   app/test-pmd/testpmd.h                |  1 +
>>>>   doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>>>>   4 files changed, 58 insertions(+)
>>> 
>>> Can you also update the release notes to document the new parameter?
>>> 
>>>> 
>>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>>> index c8acd5d1b7..e10f7d38fb 100644
>>>> --- a/app/test-pmd/parameters.c
>>>> +++ b/app/test-pmd/parameters.c
>>>> @@ -224,6 +224,7 @@ usage(char* progname)
>>>>       printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
>>>> mode.\n "
>>>>              "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>>>>              "    0x01 - hairpin ports loop, 0x00 - hairpin port
>>>> self\n");
>>>> +    printf("  --eth-link-speed: forced link speed.\n");
>>>>   }
>>>>     #ifdef RTE_LIB_CMDLINE
>>>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
>>>> int enable)
>>>>       return 0;
>>>>   }
>>>>   +static int
>>>> +parse_link_speed(int n)
>>>> +{
>>>> +    uint32_t speed;
>>>> +
>>>> +    switch (n) {
>>> 
>>> OK to not support "10M, 100M", not sure if anybody really uses them, but
>>> what do you think checking them and return an error?
>>> 
>>>> +    case 1000:
>>>> +        speed = ETH_LINK_SPEED_1G;
>>>> +        break;
>>>> +    case 10000:
>>>> +        speed = ETH_LINK_SPEED_10G;
>>>> +        break;
>>>> +    case 25000:
>>>> +        speed = ETH_LINK_SPEED_25G;
>>>> +        break;
>>>> +    case 40000:
>>>> +        speed = ETH_LINK_SPEED_40G;
>>>> +        break;
>>>> +    case 50000:
>>>> +        speed = ETH_LINK_SPEED_50G;
>>>> +        break;
>>>> +    case 100000:
>>>> +        speed = ETH_LINK_SPEED_100G;
>>>> +        break;
>>>> +    case 200000:
>>>> +        speed = ETH_LINK_SPEED_200G;
>>>> +        break;
>>>> +    default:
>>>> +        speed = ETH_LINK_SPEED_AUTONEG;
>>>> +        break;
>>> 
>>> Isn't this function to set a fixed link speed, why falling back to autoneg?
>>> 
>>> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
>> It should. Previous time I've tried to fix corresponding
>> bug in CLI commands, it ended up with rollback because
>> of Intel drivers do not handle it correctly.
>> See "app/testpmd: set fixed flag for exact link speed" and
>> corresponding revert.
> 
> Thanks for the reminder Andrew, you have a good memory :)
> For reference: http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/
> 
> It seems that patch reverted with the pressure of the release, what do you think applying it again while we have enough time to fix the PMDs before release?
> 
> From previous discussions, long term actions listed as:
> "
> 1) Implement 'fixed' link speed support in the missing drivers.
> 2) Send a new version of the testpmd patch with a "fixed" argument, so that we
> can support all three above
> "
> 
> Not sure having (2) explicitly is required, we have already "auto" speed, not having it implies the fixed speed.
> So we can just re-apply your old patch.


Please also see the message I sent back in November: http://inbox.dpdk.org/dev/F041DE53-0ABF-4A0A-974A-16167967ABD5@pensando.io/

I added the FIXED flag to fix my local tree and found that it causes intermittent failures in link_bonding_autotest. (In version 20.02, haven’t tested in latest branch.)

-Other Andrew

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 4857 bytes --]

On Fri, Feb 26, 2021 at 3:21 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 2/26/2021 6:43 AM, Andrew Rybchenko wrote:
> > On 2/25/21 9:25 PM, Ferruh Yigit wrote:
> >> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
> >>> Add support for forced ethernet speed setting.
> >>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
> >>> It is not possible to set the Ethernet port to a specific speed while
> >>> starting testpmd. In some cases capability to configure a forced speed
> >>> for the Ethernet port during initialization may be necessary. This patch
> >>> tries to add this support.
> >>>
> >>> The patch assumes full duplex setting and does not attempt to change
> >>> that.
> >>> So speeds like 10M, 100M are not configurable using this method.
> >>>
> >>> The command line to configure a forced speed of 10G:
> >>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> >>>
> >>> The command line to configure a forced speed of 50G:
> >>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> >>>
> >>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>> ---
> >>>    app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
> >>>    app/test-pmd/testpmd.c                |  4 +++
> >>>    app/test-pmd/testpmd.h                |  1 +
> >>>    doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
> >>>    4 files changed, 58 insertions(+)
> >>
> >> Can you also update the release notes to document the new parameter?
> >>
> >>>
> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> >>> index c8acd5d1b7..e10f7d38fb 100644
> >>> --- a/app/test-pmd/parameters.c
> >>> +++ b/app/test-pmd/parameters.c
> >>> @@ -224,6 +224,7 @@ usage(char* progname)
> >>>        printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
> >>> mode.\n "
> >>>               "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
> >>>               "    0x01 - hairpin ports loop, 0x00 - hairpin port
> >>> self\n");
> >>> +    printf("  --eth-link-speed: forced link speed.\n");
> >>>    }
> >>>      #ifdef RTE_LIB_CMDLINE
> >>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
> >>> int enable)
> >>>        return 0;
> >>>    }
> >>>    +static int
> >>> +parse_link_speed(int n)
> >>> +{
> >>> +    uint32_t speed;
> >>> +
> >>> +    switch (n) {
> >>
> >> OK to not support "10M, 100M", not sure if anybody really uses them, but
> >> what do you think checking them and return an error?
> >>
> >>> +    case 1000:
> >>> +        speed = ETH_LINK_SPEED_1G;
> >>> +        break;
> >>> +    case 10000:
> >>> +        speed = ETH_LINK_SPEED_10G;
> >>> +        break;
> >>> +    case 25000:
> >>> +        speed = ETH_LINK_SPEED_25G;
> >>> +        break;
> >>> +    case 40000:
> >>> +        speed = ETH_LINK_SPEED_40G;
> >>> +        break;
> >>> +    case 50000:
> >>> +        speed = ETH_LINK_SPEED_50G;
> >>> +        break;
> >>> +    case 100000:
> >>> +        speed = ETH_LINK_SPEED_100G;
> >>> +        break;
> >>> +    case 200000:
> >>> +        speed = ETH_LINK_SPEED_200G;
> >>> +        break;
> >>> +    default:
> >>> +        speed = ETH_LINK_SPEED_AUTONEG;
> >>> +        break;
> >>
> >> Isn't this function to set a fixed link speed, why falling back to autoneg?
> >>
> >> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
> >
> > It should. Previous time I've tried to fix corresponding
> > bug in CLI commands, it ended up with rollback because
> > of Intel drivers do not handle it correctly.
I can set the ETH_LINK_SPEED_FIXED bit while setting the fixed speed.

> >
> > See "app/testpmd: set fixed flag for exact link speed" and
> > corresponding revert.
I took a look at the patch.
Looks like we were setting ETH_LINK_SPEED_FIXED in
parse_and_check_speed_duplex(). Which is an existing function.

I am adding the fixed speed capability during application init itself.
So it is a little different than what was done earlier.
I will send a v2 with the suggestions I got so far.

> >
>
> Thanks for the reminder Andrew, you have a good memory :)
> For reference:
> http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/
>
> It seems that patch reverted with the pressure of the release, what do you think
> applying it again while we have enough time to fix the PMDs before release?
>
>  From previous discussions, long term actions listed as:
> "
> 1) Implement 'fixed' link speed support in the missing drivers.
> 2) Send a new version of the testpmd patch with a "fixed" argument, so that we
> can support all three above
> "
>
> Not sure having (2) explicitly is required, we have already "auto" speed, not
> having it implies the fixed speed.
> So we can just re-apply your old patch.

I think that's a good idea. If there are any issues with the drivers,
there is time to address them.

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 2/26/2021 4:18 PM, Andrew Boyer wrote:
> 
> 
>> On Feb 26, 2021, at 6:21 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 2/26/2021 6:43 AM, Andrew Rybchenko wrote:
>>> On 2/25/21 9:25 PM, Ferruh Yigit wrote:
>>>> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
>>>>> Add support for forced ethernet speed setting.
>>>>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
>>>>> It is not possible to set the Ethernet port to a specific speed while
>>>>> starting testpmd. In some cases capability to configure a forced speed
>>>>> for the Ethernet port during initialization may be necessary. This patch
>>>>> tries to add this support.
>>>>>
>>>>> The patch assumes full duplex setting and does not attempt to change
>>>>> that.
>>>>> So speeds like 10M, 100M are not configurable using this method.
>>>>>
>>>>> The command line to configure a forced speed of 10G:
>>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
>>>>>
>>>>> The command line to configure a forced speed of 50G:
>>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
>>>>>
>>>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>> ---
>>>>>    app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>>>>>    app/test-pmd/testpmd.c                |  4 +++
>>>>>    app/test-pmd/testpmd.h                |  1 +
>>>>>    doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>>>>>    4 files changed, 58 insertions(+)
>>>>
>>>> Can you also update the release notes to document the new parameter?
>>>>
>>>>>
>>>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>>>> index c8acd5d1b7..e10f7d38fb 100644
>>>>> --- a/app/test-pmd/parameters.c
>>>>> +++ b/app/test-pmd/parameters.c
>>>>> @@ -224,6 +224,7 @@ usage(char* progname)
>>>>>        printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
>>>>> mode.\n "
>>>>>               "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>>>>>               "    0x01 - hairpin ports loop, 0x00 - hairpin port
>>>>> self\n");
>>>>> +    printf("  --eth-link-speed: forced link speed.\n");
>>>>>    }
>>>>>      #ifdef RTE_LIB_CMDLINE
>>>>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
>>>>> int enable)
>>>>>        return 0;
>>>>>    }
>>>>>    +static int
>>>>> +parse_link_speed(int n)
>>>>> +{
>>>>> +    uint32_t speed;
>>>>> +
>>>>> +    switch (n) {
>>>>
>>>> OK to not support "10M, 100M", not sure if anybody really uses them, but
>>>> what do you think checking them and return an error?
>>>>
>>>>> +    case 1000:
>>>>> +        speed = ETH_LINK_SPEED_1G;
>>>>> +        break;
>>>>> +    case 10000:
>>>>> +        speed = ETH_LINK_SPEED_10G;
>>>>> +        break;
>>>>> +    case 25000:
>>>>> +        speed = ETH_LINK_SPEED_25G;
>>>>> +        break;
>>>>> +    case 40000:
>>>>> +        speed = ETH_LINK_SPEED_40G;
>>>>> +        break;
>>>>> +    case 50000:
>>>>> +        speed = ETH_LINK_SPEED_50G;
>>>>> +        break;
>>>>> +    case 100000:
>>>>> +        speed = ETH_LINK_SPEED_100G;
>>>>> +        break;
>>>>> +    case 200000:
>>>>> +        speed = ETH_LINK_SPEED_200G;
>>>>> +        break;
>>>>> +    default:
>>>>> +        speed = ETH_LINK_SPEED_AUTONEG;
>>>>> +        break;
>>>>
>>>> Isn't this function to set a fixed link speed, why falling back to autoneg?
>>>>
>>>> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
>>> It should. Previous time I've tried to fix corresponding
>>> bug in CLI commands, it ended up with rollback because
>>> of Intel drivers do not handle it correctly.
>>> See "app/testpmd: set fixed flag for exact link speed" and
>>> corresponding revert.
>>
>> Thanks for the reminder Andrew, you have a good memory :)
>> For reference: http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/
>>
>> It seems that patch reverted with the pressure of the release, what do you think applying it again while we have enough time to fix the PMDs before release?
>>
>>  From previous discussions, long term actions listed as:
>> "
>> 1) Implement 'fixed' link speed support in the missing drivers.
>> 2) Send a new version of the testpmd patch with a "fixed" argument, so that we
>> can support all three above
>> "
>>
>> Not sure having (2) explicitly is required, we have already "auto" speed, not having it implies the fixed speed.
>> So we can just re-apply your old patch.
> 
> 
> Please also see the message I sent back in November: http://inbox.dpdk.org/dev/F041DE53-0ABF-4A0A-974A-16167967ABD5@pensando.io/
> 
> I added the FIXED flag to fix my local tree and found that it causes intermittent failures in link_bonding_autotest. (In version 20.02, haven’t tested in latest branch.)
> 

Why updates in the testpmd may be affecting bonding link autotest, what am I 
missing?

> -Other Andrew
> 

:)

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 7997 bytes --]

On Thu, Feb 25, 2021 at 10:25 AM Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
> > Add support for forced ethernet speed setting.
> > Currently testpmd tries to configure the Ethernet port in autoneg mode.
> > It is not possible to set the Ethernet port to a specific speed while
> > starting testpmd. In some cases capability to configure a forced speed
> > for the Ethernet port during initialization may be necessary. This patch
> > tries to add this support.
> >
> > The patch assumes full duplex setting and does not attempt to change
> that.
> > So speeds like 10M, 100M are not configurable using this method.
> >
> > The command line to configure a forced speed of 10G:
> > dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> >
> > The command line to configure a forced speed of 50G:
> > dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> >
> > Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >   app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
> >   app/test-pmd/testpmd.c                |  4 +++
> >   app/test-pmd/testpmd.h                |  1 +
> >   doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
> >   4 files changed, 58 insertions(+)
>
> Can you also update the release notes to document the new parameter?
>
Done


>
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index c8acd5d1b7..e10f7d38fb 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -224,6 +224,7 @@ usage(char* progname)
> >       printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
> mode.\n "
> >              "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
> >              "    0x01 - hairpin ports loop, 0x00 - hairpin port
> self\n");
> > +     printf("  --eth-link-speed: forced link speed.\n");
> >   }
> >
> >   #ifdef RTE_LIB_CMDLINE
> > @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg, int
> enable)
> >       return 0;
> >   }
> >
> > +static int
> > +parse_link_speed(int n)
> > +{
> > +     uint32_t speed;
> > +
> > +     switch (n) {
>
> OK to not support "10M, 100M", not sure if anybody really uses them, but
> what do
> you think checking them and return an error?
>
Ok. Added this.


>
> > +     case 1000:
> > +             speed = ETH_LINK_SPEED_1G;
> > +             break;
> > +     case 10000:
> > +             speed = ETH_LINK_SPEED_10G;
> > +             break;
> > +     case 25000:
> > +             speed = ETH_LINK_SPEED_25G;
> > +             break;
> > +     case 40000:
> > +             speed = ETH_LINK_SPEED_40G;
> > +             break;
> > +     case 50000:
> > +             speed = ETH_LINK_SPEED_50G;
> > +             break;
> > +     case 100000:
> > +             speed = ETH_LINK_SPEED_100G;
> > +             break;
> > +     case 200000:
> > +             speed = ETH_LINK_SPEED_200G;
> > +             break;
> > +     default:
> > +             speed = ETH_LINK_SPEED_AUTONEG;
> > +             break;
>
> Isn't this function to set a fixed link speed, why falling back to autoneg?
>
Good point. I removed it.


>
> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
>
Yes. Done in v2.


>
> > +     }
> > +
> > +     return speed;
> > +}
> > +
> >   void
> >   launch_args_parse(int argc, char** argv)
> >   {
> > @@ -605,6 +641,7 @@ launch_args_parse(int argc, char** argv)
> >               { "rx-mq-mode",                 1, 0, 0 },
> >               { "record-core-cycles",         0, 0, 0 },
> >               { "record-burst-stats",         0, 0, 0 },
> > +             { "eth-link-speed",             1, 0, 0 },
> >               { 0, 0, 0, 0 },
> >       };
> >
> > @@ -1366,6 +1403,11 @@ launch_args_parse(int argc, char** argv)
> >                               record_core_cycles = 1;
> >                       if (!strcmp(lgopts[opt_idx].name,
> "record-burst-stats"))
> >                               record_burst_stats = 1;
> > +                     if (!strcmp(lgopts[opt_idx].name,
> "eth-link-speed")) {
> > +                             n = atoi(optarg);
> > +                             if (n >= 0 && parse_link_speed(n) >= 0)
> > +                                     eth_link_speed =
> parse_link_speed(n);
> > +                     }
> >                       break;
> >               case 'h':
> >                       usage(argv[0]);
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> > index caa711d6f3..9434e335a0 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -535,6 +535,8 @@ uint16_t gso_max_segment_size = RTE_ETHER_MAX_LEN -
> RTE_ETHER_CRC_LEN;
> >   /* Holds the registered mbuf dynamic flags names. */
> >   char dynf_names[64][RTE_MBUF_DYN_NAMESIZE];
> >
> > +uint32_t eth_link_speed;
> > +
>
> Can you please move this a little up, at the beginning of the 'testpmd.c'
> there
> are global config variables, this can go next to them.
>
Done in v2.


>
> And can you please add a single line comment to the variable, as done in
> all
> other ones.
>
Sure. Done in v2.


>
> >   /*
> >    * Helper function to check if socket is already discovered.
> >    * If yes, return positive value. If not, return zero.
> > @@ -1484,6 +1486,8 @@ init_config(void)
> >                       port->tx_conf[k].offloads =
> >                               port->dev_conf.txmode.offloads;
> >
> > +             port->dev_conf.link_speeds = eth_link_speed;
> > +
>
> This is set even user doesn't provide '--eth-link-speed' at all, in that
> case I
> assume it relies on the fact that variable default value (0) is the same
> as
> 'AUTONEG' value (0). So it sets speed to autoneg in that case.
> But what do you think to be more explicit, and set this only if the user
> provided the device argument, like:
>         if (eth_link_speed)
>                 port->dev_conf.link_speeds = eth_link_speed;
>
Done in v2.


>
>
> This sets the same link speed for all ports, do you think may we need
> capability
> to set link speed per port? Does it worth the complexity it brings?
>
It is turning out to be a bit complex. For this round I think let's keep it
simple.


>
> >               /* set flag to initialize port/queue */
> >               port->need_reconfig = 1;
> >               port->need_reconfig_queues = 1;
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> > index 60ddeb8f13..a3cd4a0e16 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -351,6 +351,7 @@ extern bool setup_on_probe_event; /**< disabled by
> port setup-on iterator */
> >   extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
> >   extern int do_mlockall; /**< set by "--mlockall" or "--no-mlockall"
> parameter */
> >   extern uint8_t clear_ptypes; /**< disabled by set ptype cmd */
> > +extern uint32_t eth_link_speed;
> >
> >   #ifdef RTE_LIBRTE_IXGBE_BYPASS
> >   extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog
> timeout */
> > diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> > index 6745072329..a856f52736 100644
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > @@ -536,3 +536,14 @@ The command line options are:
> >       bit 1 - two hairpin ports paired
> >       bit 0 - two hairpin ports loop
> >       The default value is 0. Hairpin will use single port mode and
> implicit Tx flow mode.
> > +
> > +*   ``--eth-link-speed``
> > +
>
> Should this document that "10M, 100M" is not supported?
>
Done in v2.


>
> > +    Set a forced link speed to the ethernet port.
> > +    1000 - 1Gbps
> > +    10000 - 10Gbps
> > +    25000 - 25Gbps
> > +    40000 - 40Gbps
> > +    50000 - 50Gbps
> > +    100000 - 100Gbps
> > +    200000 - 200Gbps
>
> What do you think to put something like "..." to very bottom, to clarify
> this is
> not full list, so that we won't need to update this each time we add a new
> speed?
>
Done.

[dpdk-dev] [PATCH v2] app/testpmd: add support for forced ethernet speed

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 6071 bytes --]

Add support for forced ethernet speed setting.
Currently testpmd tries to configure the Ethernet port in autoneg mode.
It is not possible to set the Ethernet port to a specific speed while
starting testpmd. In some cases capability to configure a forced speed
for the Ethernet port during initialization may be necessary. This patch
tries to add this support.

The patch assumes full duplex setting and does not attempt to change that.
So speeds like 10M, 100M are not configurable using this method.

The command line to configure a forced speed of 10G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000

The command line to configure a forced speed of 50G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
----
v1->v2:
 - Updated the release notes to document the new parameter.
 - Updated the user guide to indicate 10 and 100 Mbps are not supported.
 - Check and return error if 10 or 100Mbps speeds are requested.
 - Set ETH_LINK_SPEED_FIXED when requesting forced link speed.
 - Updated code based on other review comments.

Please apply.
---
 app/test-pmd/parameters.c              | 44 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.c                 |  8 +++++
 app/test-pmd/testpmd.h                 |  1 +
 doc/guides/rel_notes/release_21_05.rst |  3 ++
 doc/guides/testpmd_app_ug/run_app.rst  | 16 ++++++++++
 5 files changed, 72 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c8acd5d1b7..c5dac33b0f 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -224,6 +224,7 @@ usage(char* progname)
 	printf("  --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n "
 	       "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
 	       "    0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
+	printf("  --eth-link-speed: forced link speed.\n");
 }
 
 #ifdef RTE_LIB_CMDLINE
@@ -485,6 +486,43 @@ parse_event_printing_config(const char *optarg, int enable)
 	return 0;
 }
 
+static int
+parse_link_speed(int n)
+{
+	uint32_t speed = ETH_LINK_SPEED_FIXED;
+
+	switch (n) {
+	case 1000:
+		speed |= ETH_LINK_SPEED_1G;
+		break;
+	case 10000:
+		speed |= ETH_LINK_SPEED_10G;
+		break;
+	case 25000:
+		speed |= ETH_LINK_SPEED_25G;
+		break;
+	case 40000:
+		speed |= ETH_LINK_SPEED_40G;
+		break;
+	case 50000:
+		speed |= ETH_LINK_SPEED_50G;
+		break;
+	case 100000:
+		speed |= ETH_LINK_SPEED_100G;
+		break;
+	case 200000:
+		speed |= ETH_LINK_SPEED_200G;
+		break;
+	case 100:
+	case 10:
+	default:
+		printf("Unsupported fixed speed\n");
+		return 0;
+	}
+
+	return speed;
+}
+
 void
 launch_args_parse(int argc, char** argv)
 {
@@ -605,6 +643,7 @@ launch_args_parse(int argc, char** argv)
 		{ "rx-mq-mode",                 1, 0, 0 },
 		{ "record-core-cycles",         0, 0, 0 },
 		{ "record-burst-stats",         0, 0, 0 },
+		{ "eth-link-speed",		1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1366,6 +1405,11 @@ launch_args_parse(int argc, char** argv)
 				record_core_cycles = 1;
 			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
 				record_burst_stats = 1;
+			if (!strcmp(lgopts[opt_idx].name, "eth-link-speed")) {
+				n = atoi(optarg);
+				if (n >= 0 && parse_link_speed(n) > 0)
+					eth_link_speed = parse_link_speed(n);
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1a57324b1b..98c3248c01 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -513,6 +513,11 @@ uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
  */
 enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
 
+/*
+ * Used to set forced link speed
+ */
+uint32_t eth_link_speed;
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -1484,6 +1489,9 @@ init_config(void)
 			port->tx_conf[k].offloads =
 				port->dev_conf.txmode.offloads;
 
+		if (eth_link_speed)
+			port->dev_conf.link_speeds = eth_link_speed;
+
 		/* set flag to initialize port/queue */
 		port->need_reconfig = 1;
 		port->need_reconfig_queues = 1;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index ce83f31f0d..0f54bcc346 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -353,6 +353,7 @@ extern bool setup_on_probe_event; /**< disabled by port setup-on iterator */
 extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
 extern int do_mlockall; /**< set by "--mlockall" or "--no-mlockall" parameter */
 extern uint8_t clear_ptypes; /**< disabled by set ptype cmd */
+extern uint32_t eth_link_speed;
 
 #ifdef RTE_LIBRTE_IXGBE_BYPASS
 extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog timeout */
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index ca28d05d59..dde652dddf 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -64,6 +64,9 @@ New Features
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
 
+  * Added a command line option to configure forced speed for Ethernet port.
+    ``dpdk-testpmd -c 0xff  -- -i  --eth-link-speed N``
+
 
 Removed Items
 -------------
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 6745072329..2abff3019a 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -536,3 +536,19 @@ The command line options are:
     bit 1 - two hairpin ports paired
     bit 0 - two hairpin ports loop
     The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
+
+*   ``--eth-link-speed``
+
+    Set a forced link speed to the ethernet port.
+    1000 - 1Gbps
+    10000 - 10Gbps
+    25000 - 25Gbps
+    40000 - 40Gbps
+    50000 - 50Gbps
+    100000 - 100Gbps
+    200000 - 200Gbps
+    ...
+
+.. Note::
+
+   Setting 10Mbps and 100Mbps speeds is not supported.
-- 
2.21.1 (Apple Git-122.3)

Re: [dpdk-dev] [PATCH v2] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 3/5/2021 4:17 AM, Ajit Khaparde wrote:
> Add support for forced ethernet speed setting.
> Currently testpmd tries to configure the Ethernet port in autoneg mode.
> It is not possible to set the Ethernet port to a specific speed while
> starting testpmd. In some cases capability to configure a forced speed
> for the Ethernet port during initialization may be necessary. This patch
> tries to add this support.
> 
> The patch assumes full duplex setting and does not attempt to change that.
> So speeds like 10M, 100M are not configurable using this method.
> 
> The command line to configure a forced speed of 10G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> 
> The command line to configure a forced speed of 50G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

<...>

> @@ -536,3 +536,19 @@ The command line options are:
>       bit 1 - two hairpin ports paired
>       bit 0 - two hairpin ports loop
>       The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
> +
> +*   ``--eth-link-speed``
> +
> +    Set a forced link speed to the ethernet port.
> +    1000 - 1Gbps
> +    10000 - 10Gbps
> +    25000 - 25Gbps
> +    40000 - 40Gbps
> +    50000 - 50Gbps
> +    100000 - 100Gbps
> +    200000 - 200Gbps
> +    ...
> +

Line breaks are lost when converted to html, it becomes single line, '::' and 
indentation is required.

Except from documentation, looks good to me.

[dpdk-dev] [PATCH v3] app/testpmd: add support for forced ethernet speed

[Ajit Khaparde]

[-- Attachment #1: Type: text/plain, Size: 6163 bytes --]

Add support for forced ethernet speed setting.
Currently testpmd tries to configure the Ethernet port in autoneg mode.
It is not possible to set the Ethernet port to a specific speed while
starting testpmd. In some cases capability to configure a forced speed
for the Ethernet port during initialization may be necessary. This patch
tries to add this support.

The patch assumes full duplex setting and does not attempt to change that.
So speeds like 10M, 100M are not configurable using this method.

The command line to configure a forced speed of 10G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000

The command line to configure a forced speed of 50G:
dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
----
v1->v2:
 - Updated the release notes to document the new parameter.
 - Updated the user guide to indicate 10 and 100 Mbps are not supported.
 - Check and return error if 10 or 100Mbps speeds are requested.
 - Set ETH_LINK_SPEED_FIXED when requesting forced link speed.
 - Updated code based on other review comments.
v2>v3:
 - Updated the user guide to correct formatting issues.

Please apply.
---
 app/test-pmd/parameters.c              | 44 ++++++++++++++++++++++++++
 app/test-pmd/testpmd.c                 |  8 +++++
 app/test-pmd/testpmd.h                 |  1 +
 doc/guides/rel_notes/release_21_05.rst |  3 ++
 doc/guides/testpmd_app_ug/run_app.rst  | 15 +++++++++
 5 files changed, 71 insertions(+)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c8acd5d1b7..c5dac33b0f 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -224,6 +224,7 @@ usage(char* progname)
 	printf("  --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n "
 	       "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
 	       "    0x01 - hairpin ports loop, 0x00 - hairpin port self\n");
+	printf("  --eth-link-speed: forced link speed.\n");
 }
 
 #ifdef RTE_LIB_CMDLINE
@@ -485,6 +486,43 @@ parse_event_printing_config(const char *optarg, int enable)
 	return 0;
 }
 
+static int
+parse_link_speed(int n)
+{
+	uint32_t speed = ETH_LINK_SPEED_FIXED;
+
+	switch (n) {
+	case 1000:
+		speed |= ETH_LINK_SPEED_1G;
+		break;
+	case 10000:
+		speed |= ETH_LINK_SPEED_10G;
+		break;
+	case 25000:
+		speed |= ETH_LINK_SPEED_25G;
+		break;
+	case 40000:
+		speed |= ETH_LINK_SPEED_40G;
+		break;
+	case 50000:
+		speed |= ETH_LINK_SPEED_50G;
+		break;
+	case 100000:
+		speed |= ETH_LINK_SPEED_100G;
+		break;
+	case 200000:
+		speed |= ETH_LINK_SPEED_200G;
+		break;
+	case 100:
+	case 10:
+	default:
+		printf("Unsupported fixed speed\n");
+		return 0;
+	}
+
+	return speed;
+}
+
 void
 launch_args_parse(int argc, char** argv)
 {
@@ -605,6 +643,7 @@ launch_args_parse(int argc, char** argv)
 		{ "rx-mq-mode",                 1, 0, 0 },
 		{ "record-core-cycles",         0, 0, 0 },
 		{ "record-burst-stats",         0, 0, 0 },
+		{ "eth-link-speed",		1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1366,6 +1405,11 @@ launch_args_parse(int argc, char** argv)
 				record_core_cycles = 1;
 			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
 				record_burst_stats = 1;
+			if (!strcmp(lgopts[opt_idx].name, "eth-link-speed")) {
+				n = atoi(optarg);
+				if (n >= 0 && parse_link_speed(n) > 0)
+					eth_link_speed = parse_link_speed(n);
+			}
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1a57324b1b..98c3248c01 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -513,6 +513,11 @@ uint8_t gro_flush_cycles = GRO_DEFAULT_FLUSH_CYCLES;
  */
 enum rte_eth_rx_mq_mode rx_mq_mode = ETH_MQ_RX_VMDQ_DCB_RSS;
 
+/*
+ * Used to set forced link speed
+ */
+uint32_t eth_link_speed;
+
 /* Forward function declarations */
 static void setup_attached_port(portid_t pi);
 static void check_all_ports_link_status(uint32_t port_mask);
@@ -1484,6 +1489,9 @@ init_config(void)
 			port->tx_conf[k].offloads =
 				port->dev_conf.txmode.offloads;
 
+		if (eth_link_speed)
+			port->dev_conf.link_speeds = eth_link_speed;
+
 		/* set flag to initialize port/queue */
 		port->need_reconfig = 1;
 		port->need_reconfig_queues = 1;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index ce83f31f0d..0f54bcc346 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -353,6 +353,7 @@ extern bool setup_on_probe_event; /**< disabled by port setup-on iterator */
 extern uint8_t hot_plug; /**< enable by "--hot-plug" parameter */
 extern int do_mlockall; /**< set by "--mlockall" or "--no-mlockall" parameter */
 extern uint8_t clear_ptypes; /**< disabled by set ptype cmd */
+extern uint32_t eth_link_speed;
 
 #ifdef RTE_LIBRTE_IXGBE_BYPASS
 extern uint32_t bypass_timeout; /**< Store the NIC bypass watchdog timeout */
diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index ca28d05d59..dde652dddf 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -64,6 +64,9 @@ New Features
   * Added command to display Rx queue used descriptor count.
     ``show port (port_id) rxq (queue_id) desc used count``
 
+  * Added a command line option to configure forced speed for Ethernet port.
+    ``dpdk-testpmd -c 0xff  -- -i  --eth-link-speed N``
+
 
 Removed Items
 -------------
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 6745072329..3035118cdc 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -536,3 +536,18 @@ The command line options are:
     bit 1 - two hairpin ports paired
     bit 0 - two hairpin ports loop
     The default value is 0. Hairpin will use single port mode and implicit Tx flow mode.
+
+*   ``--eth-link-speed``
+
+    Set a forced link speed to the ethernet port::
+
+       10 - 10Mbps (not supported)
+       100 - 100Mbps (not supported)
+       1000 - 1Gbps
+       10000 - 10Gbps
+       25000 - 25Gbps
+       40000 - 40Gbps
+       50000 - 50Gbps
+       100000 - 100Gbps
+       200000 - 200Gbps
+       ...
-- 
2.21.1 (Apple Git-122.3)

Re: [dpdk-dev] [PATCH v3] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 3/5/2021 7:42 PM, Ajit Khaparde wrote:
> Add support for forced ethernet speed setting.
> Currently testpmd tries to configure the Ethernet port in autoneg mode.
> It is not possible to set the Ethernet port to a specific speed while
> starting testpmd. In some cases capability to configure a forced speed
> for the Ethernet port during initialization may be necessary. This patch
> tries to add this support.
> 
> The patch assumes full duplex setting and does not attempt to change that.
> So speeds like 10M, 100M are not configurable using this method.
> 
> The command line to configure a forced speed of 10G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
> 
> The command line to configure a forced speed of 50G:
> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

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

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

Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed

[Ferruh Yigit]

On 2/26/2021 11:21 AM, Ferruh Yigit wrote:
> On 2/26/2021 6:43 AM, Andrew Rybchenko wrote:
>> On 2/25/21 9:25 PM, Ferruh Yigit wrote:
>>> On 2/22/2021 7:18 PM, Ajit Khaparde wrote:
>>>> Add support for forced ethernet speed setting.
>>>> Currently testpmd tries to configure the Ethernet port in autoneg mode.
>>>> It is not possible to set the Ethernet port to a specific speed while
>>>> starting testpmd. In some cases capability to configure a forced speed
>>>> for the Ethernet port during initialization may be necessary. This patch
>>>> tries to add this support.
>>>>
>>>> The patch assumes full duplex setting and does not attempt to change
>>>> that.
>>>> So speeds like 10M, 100M are not configurable using this method.
>>>>
>>>> The command line to configure a forced speed of 10G:
>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  10000
>>>>
>>>> The command line to configure a forced speed of 50G:
>>>> dpdk-testpmd -c 0xff  -- -i  --eth-link-speed  50000
>>>>
>>>> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>> ---
>>>>    app/test-pmd/parameters.c             | 42 +++++++++++++++++++++++++++
>>>>    app/test-pmd/testpmd.c                |  4 +++
>>>>    app/test-pmd/testpmd.h                |  1 +
>>>>    doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++
>>>>    4 files changed, 58 insertions(+)
>>>
>>> Can you also update the release notes to document the new parameter?
>>>
>>>>
>>>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>>>> index c8acd5d1b7..e10f7d38fb 100644
>>>> --- a/app/test-pmd/parameters.c
>>>> +++ b/app/test-pmd/parameters.c
>>>> @@ -224,6 +224,7 @@ usage(char* progname)
>>>>        printf("  --hairpin-mode=0xXX: bitmask set the hairpin port
>>>> mode.\n "
>>>>               "    0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n"
>>>>               "    0x01 - hairpin ports loop, 0x00 - hairpin port
>>>> self\n");
>>>> +    printf("  --eth-link-speed: forced link speed.\n");
>>>>    }
>>>>      #ifdef RTE_LIB_CMDLINE
>>>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg,
>>>> int enable)
>>>>        return 0;
>>>>    }
>>>>    +static int
>>>> +parse_link_speed(int n)
>>>> +{
>>>> +    uint32_t speed;
>>>> +
>>>> +    switch (n) {
>>>
>>> OK to not support "10M, 100M", not sure if anybody really uses them, but
>>> what do you think checking them and return an error?
>>>
>>>> +    case 1000:
>>>> +        speed = ETH_LINK_SPEED_1G;
>>>> +        break;
>>>> +    case 10000:
>>>> +        speed = ETH_LINK_SPEED_10G;
>>>> +        break;
>>>> +    case 25000:
>>>> +        speed = ETH_LINK_SPEED_25G;
>>>> +        break;
>>>> +    case 40000:
>>>> +        speed = ETH_LINK_SPEED_40G;
>>>> +        break;
>>>> +    case 50000:
>>>> +        speed = ETH_LINK_SPEED_50G;
>>>> +        break;
>>>> +    case 100000:
>>>> +        speed = ETH_LINK_SPEED_100G;
>>>> +        break;
>>>> +    case 200000:
>>>> +        speed = ETH_LINK_SPEED_200G;
>>>> +        break;
>>>> +    default:
>>>> +        speed = ETH_LINK_SPEED_AUTONEG;
>>>> +        break;
>>>
>>> Isn't this function to set a fixed link speed, why falling back to autoneg?
>>>
>>> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too?
>>
>> It should. Previous time I've tried to fix corresponding
>> bug in CLI commands, it ended up with rollback because
>> of Intel drivers do not handle it correctly.
>>
>> See "app/testpmd: set fixed flag for exact link speed" and
>> corresponding revert.
>>
> 
> Thanks for the reminder Andrew, you have a good memory :)
> For reference: 
> http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/ 
> 
> 
> It seems that patch reverted with the pressure of the release, what do you think 
> applying it again while we have enough time to fix the PMDs before release?
> 
>  From previous discussions, long term actions listed as:
> "
> 1) Implement 'fixed' link speed support in the missing drivers.
> 2) Send a new version of the testpmd patch with a "fixed" argument, so that we
> can support all three above
> "
> 
> Not sure having (2) explicitly is required, we have already "auto" speed, not 
> having it implies the fixed speed.
> So we can just re-apply your old patch.

Andrew, if you are OK with above, can you please send your old patch (the 
reverted one) on top of latest head?