Re: [PATCH 1/1] test/pmd_perf: handling of unknown connection speed

[Heinrich Schuchardt <heinrich.schuchardt@canonical.com>]

On 6/26/22 17:15, Thomas Monjalon wrote:
> 11/05/2022 18:33, Heinrich Schuchardt:
>> When running DPDK in QEMU it cannot determine the connection speed.
>> pmd_perf_autotest treats this as if the connection speed where
>> UNIT32_MAX Mbps:
>>
>>      RTE>>pmd_perf_autotest
>>      Start PMD RXTX cycles cost test.
>>      Allocated mbuf pool on socket 0
>>      CONFIG RXD=1024 TXD=1024
>>      Performance test runs on lcore 1 socket 0
>>      Port 0 Address:52:54:00:12:34:57
>>      Port 1 Address:52:54:00:12:34:58
>>      Checking link statuses...
>>      Port 0 Link up at Unknown FDX Autoneg
>>      Port 1 Link up at Unknown FDX Autoneg
>>      IPv4 pktlen 46
>>      UDP pktlen 26
>>      Generate 4096 packets @socket 0
>>      inject 2048 packet to port 0
>>      inject 2048 packet to port 1
>>      Total packets inject to prime ports = 4096
>>      Each port will do 6391320379464 packets per second
>>      Test will stop after at least 25565281517856 packets received
>>
>> This will not allow the test to terminate in a reasonable timespan.
>> Just assume 10 Gbps in this case instead:
>>
>>      ...
>>      Test will stop after at least 59523808 packets received
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> ---
>>   app/test/test_pmd_perf.c | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test/test_pmd_perf.c b/app/test/test_pmd_perf.c
>> index 25611bfe9b..ee08c8aade 100644
>> --- a/app/test/test_pmd_perf.c
>> +++ b/app/test/test_pmd_perf.c
>> @@ -486,10 +486,17 @@ main_loop(__rte_unused void *args)
>>   	}
>>   	printf("Total packets inject to prime ports = %u\n", idx);
>>   
>> -	packets_per_second = (link_mbps * 1000 * 1000) /
>> -		((PACKET_SIZE + FRAME_GAP + MAC_PREAMBLE) * CHAR_BIT);
>> -	printf("Each port will do %"PRIu64" packets per second\n",
>> -	       packets_per_second);
>> +	if (link_mbps != RTE_ETH_SPEED_NUM_UNKNOWN) {
>> +		packets_per_second = (link_mbps * 1000 * 1000) /
>> +			((PACKET_SIZE + FRAME_GAP + MAC_PREAMBLE) * CHAR_BIT);
>> +		printf("Each port will do %"PRIu64" packets per second\n",
>> +		       packets_per_second);
>> +		total_packets = RTE_TEST_DURATION * conf->nb_ports * packets_per_second;

Yes this line should be removed.

> 
> This is redundant with below.
> 
>> +	} else {
>> +		/* We don't know the speed. Pretend it is 10G */
>> +		packets_per_second = ((uint64_t)RTE_ETH_SPEED_NUM_10G * 1000 * 1000) /
>> +			((PACKET_SIZE + FRAME_GAP + MAC_PREAMBLE) * CHAR_BIT);
>> +	}
>>   
>>   	total_packets = RTE_TEST_DURATION * conf->nb_ports * packets_per_second;
> 
> Why not just inserting this:
> 
> if (link_mbps == RTE_ETH_SPEED_NUM_UNKNOWN)
> 	link_mbps = RTE_ETH_SPEED_NUM_10G;

Following your suggestion the message "Each port will do %"PRIu64" 
packets per second\n" would provide misleading information to the user. 
This should be avoided.

Best regards

Heinrich