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: sta...@dpdk.org
>>>
>>> Signed-off-by: Julien Meunier <julien.meun...@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
>
