All of lore.kernel.org
 help / color / mirror / Atom feed
From: Olivier MATZ <olivier.matz@6wind.com>
To: "Tahhan, Maryam" <maryam.tahhan@intel.com>,
	Andriy Berestovskyy <aber@semihalf.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: ixgbe: account more Rx errors Issue
Date: Mon, 07 Sep 2015 10:30:26 +0200	[thread overview]
Message-ID: <55ED4B22.8020708@6wind.com> (raw)
In-Reply-To: <1A27633A6DA49C4A92FCD5D4312DBF536A50A7FE@IRSMSX109.ger.corp.intel.com>

Hi,

On 09/06/2015 07:15 PM, Tahhan, Maryam wrote:
>> From: Andriy Berestovskyy [mailto:aber@semihalf.com]
>> Sent: Friday, September 4, 2015 5:59 PM
>> To: Tahhan, Maryam
>> Cc: dev@dpdk.org; Olivier MATZ
>> Subject: Re: ixgbe: account more Rx errors Issue
>>
>> Hi Maryam,
>> Please see below.
>>
>>> XEC counts the Number of receive IPv4, TCP, UDP or SCTP XSUM errors
>>
>> Please note than UDP checksum is optional for IPv4, but UDP packets with
>> zero checksum hit XEC.
>>
> 
> I understand, but this is what the hardware register is picking up and what I included previously is the definitions of the registers from the datasheet.
> 
>>> And general crc errors counts Counts the number of receive packets with
>> CRC errors.
>>
>> Let me explain you with an example.
>>
>> DPDK 2.0 behavior:
>> host A sends 10M IPv4 UDP packets (no checksum) to host B host B stats: 9M
>> ipackets + 1M ierrors (missed) = 10M
>>
>> DPDK 2.1 behavior:
>> host A sends 10M IPv4 UDP packets (no checksum) to host B host B stats: 9M
>> ipackets + 11M in ierrors (1M missed + 10M XEC) = 20M?
> 
> Because it's hitting the 2 error registers. If you had packets with multiple errors that are added up as part of ierrors you'll still be getting more than 10M errors which is why I asked for feedback on the 3 suggestions below. What I'm saying is the number of errors being > the number of received packets will be seen if you hit multiple error registers on the NIC.
> 
>>
>>> So our options are we can:
>>> 1. Add only one of these into the error stats.
>>> 2. We can introduce some cooking of stats in this scenario, so only add
>> either or if they are equal or one is higher than the other.
>>> 3. Add them all which means you can have more errors than the number of
>> received packets, but TBH this is going to be the case if your packets have
>> multiple errors anyway.
>>
>> 4. ierrors should reflect NIC drops only.
> 
> I may have misinterpreted this, but ierrors in rte_ethdev.h ierrors is defined as the Total number of erroneous received packets.
> Maybe we need a clear definition or a separate drop counter as I see uint64_t q_errors defined as: Total number of queue packets received that are dropped.
> 
>> XEC does not count drops, so IMO it should be removed from ierrors.
> 
> While it's picking up the 0 checksum as an error (which it shouldn't necessarily be doing), removing it could mean missing other valid L3/L4 checksum errors... Let me experiment some more with L3/L4 checksum errors and crcerrs to see if we can cook the stats around this register in particular. I would hate to remove it and miss genuine errors 

For me, the definition that looks the most straightforward is:

ipackets = packets successfully received by hardware
imissed = packets dropped by hardware because the software does
  not poll fast enough (= queue full)
ierrors = packets dropped by hardware (malformed packets, ...)

These 3 stats never count twice the same packet.

If we want more statistics, they could go in xstats. For instance,
a counter for invalid checksum. The definition of these stats would
be pmd-specific.

I agree we should clarify and have a consensus on the definitions
before going further.


Regards,
Olivier



> 
>>
>> Please note that we still can access the XEC using rte_eth_xstats_get()
>>
>>
>> Regards,
>> Andriy

  reply	other threads:[~2015-09-07  8:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-04  9:38 ixgbe: account more Rx errors Issue Andriy Berestovskyy
2015-09-04 12:44 ` Tahhan, Maryam
2015-09-04 16:58   ` Andriy Berestovskyy
2015-09-06 17:15     ` Tahhan, Maryam
2015-09-07  8:30       ` Olivier MATZ [this message]
2015-09-07 11:44         ` Tahhan, Maryam
2015-09-09 17:43           ` Kyle Larose
2015-09-14  9:50             ` Tahhan, Maryam
2015-10-22  8:21               ` Martin Weiser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55ED4B22.8020708@6wind.com \
    --to=olivier.matz@6wind.com \
    --cc=aber@semihalf.com \
    --cc=dev@dpdk.org \
    --cc=maryam.tahhan@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.