Re: [dpdk-dev] [PATCH v2] test/pmd_perf: fix the way to drain the port

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 2/18/2019 11:25 AM, Meunier, Julien (Nokia - FR/Paris-Saclay) wrote:
> Hi,
> 
> Sorry for the delay. Inline reply.
> 
> On 07/02/2019 13:28, Ferruh Yigit wrote:
>> On 2/3/2019 7:42 PM, Julien Meunier wrote:
>>> If the port has received less than ``pkt_per_port`` packets (for
>>> example, the port has missed some packets), the test is in an infinite
>>> loop.
>>>
>>> Instead of expecting a number of packet to receive, let the port to be
>>> drained by itself. If no more packets are received, the test can
>>> continue.
>>>
>>> Fixes: 002ade70e933 ("app/test: measure cycles per packet in Rx/Tx")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
>>> ---
>>> v2:
>>> * rename commit title
>>> * fix nb_free display
>>> ---
>>>   test/test/test_pmd_perf.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/test/test/test_pmd_perf.c b/test/test/test_pmd_perf.c
>>> index f5095c8..c7e2df3 100644
>>> --- a/test/test/test_pmd_perf.c
>>> +++ b/test/test/test_pmd_perf.c
>>> @@ -493,16 +493,16 @@ main_loop(__rte_unused void *args)
>>>   
>>>   	for (i = 0; i < conf->nb_ports; i++) {
>>>   		portid = conf->portlist[i];
>>> -		int nb_free = pkt_per_port;
>>> +		int nb_free = 0;
>>>   		do { /* dry out */
>>>   			nb_rx = rte_eth_rx_burst(portid, 0,
>>>   						 pkts_burst, MAX_PKT_BURST);
>>>   			nb_tx = 0;
>>>   			while (nb_tx < nb_rx)
>>>   				rte_pktmbuf_free(pkts_burst[nb_tx++]);
>>> -			nb_free -= nb_rx;
>>> -		} while (nb_free != 0);
>>> -		printf("free %d mbuf left in port %u\n", pkt_per_port, portid);
>>> +			nb_free += nb_rx;
>>> +		} while (nb_rx != 0);
>>> +		printf("free %d mbuf left in port %u\n", nb_free, portid);
>>
>>
>> In the test logic there is an expectation that 'pkt_per_port' packets will be
>> received.
>> We are losing that intention here with this update. What do you think updating
>> the log to include it, like:
>> "free %d (expected %d) mbuf left in port %u\n", nb_free, pkt_per_port, portid
>>
> 
> OK. But, after thinking, I should add a little timeout in order to drain 
> the port during N cycles (like it was already done in the function 
> poll_burst - timeout), just to be sure that all packets are dequeued.

Not sure if we need this, at this stage all packets should be in device Rx
queue, can rte_eth_rx_burst() return 0 when there are packets waiting in the queue?

Anyway, this is after measurement done, and to free to the packets, so adding a
timeout (retry) mechanism won't hurt if you prefer to add this.

Thanks,
ferruh

> 
> I will upload a new patch today.
> 
> Best regards,
> Julien Meunier
>