[dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[A.McLoughlin]

The 'Accumulated forward statistics for all ports' incorrectly displayed
double the actual value for 'total_tx_dropped'. This was because 2
lines in the same function both incremented total_tx_dropped every time
a packet was dropped.  I removed one of these lines to fix this issue.

Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
Cc: david.marchand@redhat.com
Cc: stable@dpdk.org

Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
---
 app/test-pmd/testpmd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3ed3523b7..c41bada50 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1555,7 +1555,6 @@ fwd_stats_display(void)
 		total_recv += stats.ipackets;
 		total_xmit += stats.opackets;
 		total_rx_dropped += stats.imissed;
-		total_tx_dropped += ports_stats[pt_id].tx_dropped;
 		total_tx_dropped += stats.oerrors;
 		total_rx_nombuf  += stats.rx_nombuf;
 
-- 
2.17.1

--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Iremonger, Bernard]

Hi Aideen,


> -----Original Message-----
> From: Mcloughlin, Aideen
> Sent: Friday, July 12, 2019 9:32 AM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; Mcloughlin, Aideen <aideen.mcloughlin@intel.com>;
> david.marchand@redhat.com; stable@dpdk.org
> Subject: [PATCH] app/testpmd: fix doubling of 'total TX dropped'
> 
> The 'Accumulated forward statistics for all ports' incorrectly displayed double
> the actual value for 'total_tx_dropped'. This was because 2 lines in the same
> function both incremented total_tx_dropped every time a packet was
> dropped.  I removed one of these lines to fix this issue.
> 
> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on
> demand")
> Cc: david.marchand@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>

./devtools/check-git-log.sh -1
Wrong headline lowercase:
        app/testpmd: fix doubling of 'total TX dropped'

Otherwise

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Ferruh Yigit]

On 7/12/2019 9:32 AM, A.McLoughlin wrote:
> The 'Accumulated forward statistics for all ports' incorrectly displayed
> double the actual value for 'total_tx_dropped'. This was because 2
> lines in the same function both incremented total_tx_dropped every time
> a packet was dropped.  I removed one of these lines to fix this issue.
> 
> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
> Cc: david.marchand@redhat.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
> ---
>  app/test-pmd/testpmd.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 3ed3523b7..c41bada50 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
>  		total_recv += stats.ipackets;
>  		total_xmit += stats.opackets;
>  		total_rx_dropped += stats.imissed;
> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
>  		total_tx_dropped += stats.oerrors;
>  		total_rx_nombuf  += stats.rx_nombuf;
>  
> 

Hi Aideen,

Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,

in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
description it may be fair to say
"total_tx_dropped = oerrors + tx_dropped"

This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
reported failure for some reason, so these packets not transmitted to the medium.
'tx_dropped' is mostly calculated by application, driver returns packets that
can't able to sent to HW, so application can re-try to send or free them and
increase 'tx_dropped' counter.


The problem is in the virtual drivers, the packets not able to sent are
calculated as 'oerrors' and tx_burst functions returns the number of the
successfully sent packets which cause application calculate remaining ones as
'tx_dropped' which cause the duplication.

In virtual drivers we need to give a decision, if free the wrong packets and
increase the 'oerrors' or return packets backs to application without increasing
the 'oerrors', so that application can try to do something with packets or free
them increasing 'tx_dropped'.

@David, @Andrew, do you have a suggestion?

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Andrew Rybchenko]

On 7/15/19 5:53 PM, Ferruh Yigit wrote:
> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
>> The 'Accumulated forward statistics for all ports' incorrectly displayed
>> double the actual value for 'total_tx_dropped'. This was because 2
>> lines in the same function both incremented total_tx_dropped every time
>> a packet was dropped.  I removed one of these lines to fix this issue.
>>
>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
>> Cc: david.marchand@redhat.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
>> ---
>>   app/test-pmd/testpmd.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 3ed3523b7..c41bada50 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
>>   		total_recv += stats.ipackets;
>>   		total_xmit += stats.opackets;
>>   		total_rx_dropped += stats.imissed;
>> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
>>   		total_tx_dropped += stats.oerrors;
>>   		total_rx_nombuf  += stats.rx_nombuf;
>>   
>>
> 
> Hi Aideen,
> 
> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
> 
> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
> description it may be fair to say
> "total_tx_dropped = oerrors + tx_dropped"
> 
> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
> reported failure for some reason, so these packets not transmitted to the medium.
> 'tx_dropped' is mostly calculated by application, driver returns packets that
> can't able to sent to HW, so application can re-try to send or free them and
> increase 'tx_dropped' counter.
> 
> 
> The problem is in the virtual drivers, the packets not able to sent are
> calculated as 'oerrors' and tx_burst functions returns the number of the
> successfully sent packets which cause application calculate remaining ones as
> 'tx_dropped' which cause the duplication.

I don't understand how it is. Tx burst returns a number of owned packets
(either successfully transmitted or internally dropped/freed). If it is 
smaller than number of packets in request, other packets are either 
retried or calculated as tx_dropped.

> In virtual drivers we need to give a decision, if free the wrong packets and
> increase the 'oerrors' or return packets backs to application without increasing
> the 'oerrors', so that application can try to do something with packets or free
> them increasing 'tx_dropped'.
> 
> @David, @Andrew, do you have a suggestion? 

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Ferruh Yigit]

On 7/16/2019 1:23 PM, Andrew Rybchenko wrote:
> On 7/15/19 5:53 PM, Ferruh Yigit wrote:
>> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
>>> The 'Accumulated forward statistics for all ports' incorrectly displayed
>>> double the actual value for 'total_tx_dropped'. This was because 2
>>> lines in the same function both incremented total_tx_dropped every time
>>> a packet was dropped.  I removed one of these lines to fix this issue.
>>>
>>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
>>> Cc: david.marchand@redhat.com
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
>>> ---
>>>   app/test-pmd/testpmd.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index 3ed3523b7..c41bada50 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
>>>   		total_recv += stats.ipackets;
>>>   		total_xmit += stats.opackets;
>>>   		total_rx_dropped += stats.imissed;
>>> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
>>>   		total_tx_dropped += stats.oerrors;
>>>   		total_rx_nombuf  += stats.rx_nombuf;
>>>   
>>>
>>
>> Hi Aideen,
>>
>> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
>>
>> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
>> description it may be fair to say
>> "total_tx_dropped = oerrors + tx_dropped"
>>
>> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
>> reported failure for some reason, so these packets not transmitted to the medium.
>> 'tx_dropped' is mostly calculated by application, driver returns packets that
>> can't able to sent to HW, so application can re-try to send or free them and
>> increase 'tx_dropped' counter.
>>
>>
>> The problem is in the virtual drivers, the packets not able to sent are
>> calculated as 'oerrors' and tx_burst functions returns the number of the
>> successfully sent packets which cause application calculate remaining ones as
>> 'tx_dropped' which cause the duplication.
> 
> I don't understand how it is. Tx burst returns a number of owned packets
> (either successfully transmitted or internally dropped/freed). If it is 
> smaller than number of packets in request, other packets are either 
> retried or calculated as tx_dropped.

Virtual PMDs, at least the ones I checked, calculating not sent packets as
error, also application calculates them as tx_dropped.

Like:
https://git.dpdk.org/dpdk/tree/drivers/net/ring/rte_eth_ring.c?h=v19.08-rc1#n97

> 
>> In virtual drivers we need to give a decision, if free the wrong packets and
>> increase the 'oerrors' or return packets backs to application without increasing
>> the 'oerrors', so that application can try to do something with packets or free
>> them increasing 'tx_dropped'.
>>
>> @David, @Andrew, do you have a suggestion? 
> 

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Bruce Richardson]

On Tue, Jul 16, 2019 at 03:23:03PM +0100, Ferruh Yigit wrote:
> On 7/16/2019 1:23 PM, Andrew Rybchenko wrote:
> > On 7/15/19 5:53 PM, Ferruh Yigit wrote:
> >> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
> >>> The 'Accumulated forward statistics for all ports' incorrectly displayed
> >>> double the actual value for 'total_tx_dropped'. This was because 2
> >>> lines in the same function both incremented total_tx_dropped every time
> >>> a packet was dropped.  I removed one of these lines to fix this issue.
> >>>
> >>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
> >>> Cc: david.marchand@redhat.com
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
> >>> ---
> >>>   app/test-pmd/testpmd.c | 1 -
> >>>   1 file changed, 1 deletion(-)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>> index 3ed3523b7..c41bada50 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
> >>>   		total_recv += stats.ipackets;
> >>>   		total_xmit += stats.opackets;
> >>>   		total_rx_dropped += stats.imissed;
> >>> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
> >>>   		total_tx_dropped += stats.oerrors;
> >>>   		total_rx_nombuf  += stats.rx_nombuf;
> >>>   
> >>>
> >>
> >> Hi Aideen,
> >>
> >> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
> >>
> >> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
> >> description it may be fair to say
> >> "total_tx_dropped = oerrors + tx_dropped"
> >>
> >> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
> >> reported failure for some reason, so these packets not transmitted to the medium.
> >> 'tx_dropped' is mostly calculated by application, driver returns packets that
> >> can't able to sent to HW, so application can re-try to send or free them and
> >> increase 'tx_dropped' counter.
> >>
> >>
> >> The problem is in the virtual drivers, the packets not able to sent are
> >> calculated as 'oerrors' and tx_burst functions returns the number of the
> >> successfully sent packets which cause application calculate remaining ones as
> >> 'tx_dropped' which cause the duplication.
> > 
> > I don't understand how it is. Tx burst returns a number of owned packets
> > (either successfully transmitted or internally dropped/freed). If it is 
> > smaller than number of packets in request, other packets are either 
> > retried or calculated as tx_dropped.
> 
> Virtual PMDs, at least the ones I checked, calculating not sent packets as
> error, also application calculates them as tx_dropped.
> 
> Like:
> https://git.dpdk.org/dpdk/tree/drivers/net/ring/rte_eth_ring.c?h=v19.08-rc1#n97
> 
That is probably incorrect to do. Virtual PMDs should behave as real ones
do as far as possible. I think we should change them to not count as errors
any that are not handled by the driver, provided those are returned to the
app.

/Bruce

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Andrew Rybchenko]

On 7/16/19 5:28 PM, Bruce Richardson wrote:
> On Tue, Jul 16, 2019 at 03:23:03PM +0100, Ferruh Yigit wrote:
>> On 7/16/2019 1:23 PM, Andrew Rybchenko wrote:
>>> On 7/15/19 5:53 PM, Ferruh Yigit wrote:
>>>> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
>>>>> The 'Accumulated forward statistics for all ports' incorrectly displayed
>>>>> double the actual value for 'total_tx_dropped'. This was because 2
>>>>> lines in the same function both incremented total_tx_dropped every time
>>>>> a packet was dropped.  I removed one of these lines to fix this issue.
>>>>>
>>>>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
>>>>> Cc: david.marchand@redhat.com
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
>>>>> ---
>>>>>    app/test-pmd/testpmd.c | 1 -
>>>>>    1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>>> index 3ed3523b7..c41bada50 100644
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
>>>>>    		total_recv += stats.ipackets;
>>>>>    		total_xmit += stats.opackets;
>>>>>    		total_rx_dropped += stats.imissed;
>>>>> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
>>>>>    		total_tx_dropped += stats.oerrors;
>>>>>    		total_rx_nombuf  += stats.rx_nombuf;
>>>>>    
>>>>>
>>>>
>>>> Hi Aideen,
>>>>
>>>> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
>>>>
>>>> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
>>>> description it may be fair to say
>>>> "total_tx_dropped = oerrors + tx_dropped"
>>>>
>>>> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
>>>> reported failure for some reason, so these packets not transmitted to the medium.
>>>> 'tx_dropped' is mostly calculated by application, driver returns packets that
>>>> can't able to sent to HW, so application can re-try to send or free them and
>>>> increase 'tx_dropped' counter.
>>>>
>>>>
>>>> The problem is in the virtual drivers, the packets not able to sent are
>>>> calculated as 'oerrors' and tx_burst functions returns the number of the
>>>> successfully sent packets which cause application calculate remaining ones as
>>>> 'tx_dropped' which cause the duplication.
>>>
>>> I don't understand how it is. Tx burst returns a number of owned packets
>>> (either successfully transmitted or internally dropped/freed). If it is
>>> smaller than number of packets in request, other packets are either
>>> retried or calculated as tx_dropped.
>>
>> Virtual PMDs, at least the ones I checked, calculating not sent packets as
>> error, also application calculates them as tx_dropped.
>>
>> Like:
>> https://git.dpdk.org/dpdk/tree/drivers/net/ring/rte_eth_ring.c?h=v19.08-rc1#n97
>>
> That is probably incorrect to do. Virtual PMDs should behave as real ones
> do as far as possible. I think we should change them to not count as errors
> any that are not handled by the driver, provided those are returned to the
> app.

I agree with Bruce. It is a bug in ring PMD. If so, too fast attempts to 
transmit packets will blow up err_pkts counter.

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[Ferruh Yigit]

On 7/16/2019 4:00 PM, Andrew Rybchenko wrote:
> On 7/16/19 5:28 PM, Bruce Richardson wrote:
>> On Tue, Jul 16, 2019 at 03:23:03PM +0100, Ferruh Yigit wrote:
>>> On 7/16/2019 1:23 PM, Andrew Rybchenko wrote:
>>>> On 7/15/19 5:53 PM, Ferruh Yigit wrote:
>>>>> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
>>>>>> The 'Accumulated forward statistics for all ports' incorrectly displayed
>>>>>> double the actual value for 'total_tx_dropped'. This was because 2
>>>>>> lines in the same function both incremented total_tx_dropped every time
>>>>>> a packet was dropped.  I removed one of these lines to fix this issue.
>>>>>>
>>>>>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
>>>>>> Cc: david.marchand@redhat.com
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
>>>>>> ---
>>>>>>    app/test-pmd/testpmd.c | 1 -
>>>>>>    1 file changed, 1 deletion(-)
>>>>>>
>>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>>>> index 3ed3523b7..c41bada50 100644
>>>>>> --- a/app/test-pmd/testpmd.c
>>>>>> +++ b/app/test-pmd/testpmd.c
>>>>>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
>>>>>>    		total_recv += stats.ipackets;
>>>>>>    		total_xmit += stats.opackets;
>>>>>>    		total_rx_dropped += stats.imissed;
>>>>>> -		total_tx_dropped += ports_stats[pt_id].tx_dropped;
>>>>>>    		total_tx_dropped += stats.oerrors;
>>>>>>    		total_rx_nombuf  += stats.rx_nombuf;
>>>>>>    
>>>>>>
>>>>>
>>>>> Hi Aideen,
>>>>>
>>>>> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
>>>>>
>>>>> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
>>>>> description it may be fair to say
>>>>> "total_tx_dropped = oerrors + tx_dropped"
>>>>>
>>>>> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
>>>>> reported failure for some reason, so these packets not transmitted to the medium.
>>>>> 'tx_dropped' is mostly calculated by application, driver returns packets that
>>>>> can't able to sent to HW, so application can re-try to send or free them and
>>>>> increase 'tx_dropped' counter.
>>>>>
>>>>>
>>>>> The problem is in the virtual drivers, the packets not able to sent are
>>>>> calculated as 'oerrors' and tx_burst functions returns the number of the
>>>>> successfully sent packets which cause application calculate remaining ones as
>>>>> 'tx_dropped' which cause the duplication.
>>>>
>>>> I don't understand how it is. Tx burst returns a number of owned packets
>>>> (either successfully transmitted or internally dropped/freed). If it is
>>>> smaller than number of packets in request, other packets are either
>>>> retried or calculated as tx_dropped.
>>>
>>> Virtual PMDs, at least the ones I checked, calculating not sent packets as
>>> error, also application calculates them as tx_dropped.
>>>
>>> Like:
>>> https://git.dpdk.org/dpdk/tree/drivers/net/ring/rte_eth_ring.c?h=v19.08-rc1#n97
>>>
>> That is probably incorrect to do. Virtual PMDs should behave as real ones
>> do as far as possible. I think we should change them to not count as errors
>> any that are not handled by the driver, provided those are returned to the
>> app.
> 
> I agree with Bruce. It is a bug in ring PMD. If so, too fast attempts to 
> transmit packets will blow up err_pkts counter.
> 

+1, as far as I can see following PMDs requires fixing:
ring
kni
pcap
tap

Re: [dpdk-dev] [PATCH] app/testpmd: fix doubling of 'total TX dropped'

[David Marchand]

On Tue, Jul 16, 2019 at 5:29 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 7/16/2019 4:00 PM, Andrew Rybchenko wrote:
> > On 7/16/19 5:28 PM, Bruce Richardson wrote:
> >> On Tue, Jul 16, 2019 at 03:23:03PM +0100, Ferruh Yigit wrote:
> >>> On 7/16/2019 1:23 PM, Andrew Rybchenko wrote:
> >>>> On 7/15/19 5:53 PM, Ferruh Yigit wrote:
> >>>>> On 7/12/2019 9:32 AM, A.McLoughlin wrote:
> >>>>>> The 'Accumulated forward statistics for all ports' incorrectly displayed
> >>>>>> double the actual value for 'total_tx_dropped'. This was because 2
> >>>>>> lines in the same function both incremented total_tx_dropped every time
> >>>>>> a packet was dropped.  I removed one of these lines to fix this issue.
> >>>>>>
> >>>>>> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand")
> >>>>>> Cc: david.marchand@redhat.com
> >>>>>> Cc: stable@dpdk.org
> >>>>>>
> >>>>>> Signed-off-by: A.McLoughlin <aideen.mcloughlin@intel.com>
> >>>>>> ---
> >>>>>>    app/test-pmd/testpmd.c | 1 -
> >>>>>>    1 file changed, 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>>>>> index 3ed3523b7..c41bada50 100644
> >>>>>> --- a/app/test-pmd/testpmd.c
> >>>>>> +++ b/app/test-pmd/testpmd.c
> >>>>>> @@ -1555,7 +1555,6 @@ fwd_stats_display(void)
> >>>>>>                  total_recv += stats.ipackets;
> >>>>>>                  total_xmit += stats.opackets;
> >>>>>>                  total_rx_dropped += stats.imissed;
> >>>>>> -                total_tx_dropped += ports_stats[pt_id].tx_dropped;
> >>>>>>                  total_tx_dropped += stats.oerrors;
> >>>>>>                  total_rx_nombuf  += stats.rx_nombuf;
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Hi Aideen,
> >>>>>
> >>>>> Indeed 'rte_eth_stats->oerrors' and 'tx_dropped' are different values,
> >>>>>
> >>>>> in testpmd, 'TX-total' is taken as "total_xmit + total_tx_dropped", from this
> >>>>> description it may be fair to say
> >>>>> "total_tx_dropped = oerrors + tx_dropped"
> >>>>>
> >>>>> This is easier to see in HW devices, 'oerrors' is the packets sent to HW but HW
> >>>>> reported failure for some reason, so these packets not transmitted to the medium.
> >>>>> 'tx_dropped' is mostly calculated by application, driver returns packets that
> >>>>> can't able to sent to HW, so application can re-try to send or free them and
> >>>>> increase 'tx_dropped' counter.
> >>>>>
> >>>>>
> >>>>> The problem is in the virtual drivers, the packets not able to sent are
> >>>>> calculated as 'oerrors' and tx_burst functions returns the number of the
> >>>>> successfully sent packets which cause application calculate remaining ones as
> >>>>> 'tx_dropped' which cause the duplication.
> >>>>
> >>>> I don't understand how it is. Tx burst returns a number of owned packets
> >>>> (either successfully transmitted or internally dropped/freed). If it is
> >>>> smaller than number of packets in request, other packets are either
> >>>> retried or calculated as tx_dropped.
> >>>
> >>> Virtual PMDs, at least the ones I checked, calculating not sent packets as
> >>> error, also application calculates them as tx_dropped.
> >>>
> >>> Like:
> >>> https://git.dpdk.org/dpdk/tree/drivers/net/ring/rte_eth_ring.c?h=v19.08-rc1#n97
> >>>
> >> That is probably incorrect to do. Virtual PMDs should behave as real ones
> >> do as far as possible. I think we should change them to not count as errors
> >> any that are not handled by the driver, provided those are returned to the
> >> app.
> >
> > I agree with Bruce. It is a bug in ring PMD. If so, too fast attempts to
> > transmit packets will blow up err_pkts counter.
> >
>
> +1, as far as I can see following PMDs requires fixing:
> ring
> kni
> pcap
> tap

I already have fixes for some of them (found when I started to look at
the stats).


-- 
David Marchand