Re: [PATCH v4] mbuf: fix of enabling all newly added RX error flags

2014-12-10 13:50, Zhang, Helin:

quoted

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
Sent: Wednesday, December 10, 2014 5:35 PM
To: Zhang, Helin
Cc: dev-VfR2kkLFssw@public.gmane.org
Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
error flags

2014-12-10 16:55, Helin Zhang:

quoted

Before redefining mbuf structure, there was lack of free bits in 'ol_flags'
(32 bits in total) for new RX or TX flags. So it tried to reuse
existant bits as most as possible, or even assigning 0 to some of bit
flags. After new mbuf structure defined, there are quite a lot of free
bits. So those newly added bit flags should be assigned with correct
and valid bit values, and getting their names should be enabled as
well. Note that 'RECIP' should be removed, as nowhere uses it.
'PKT_RX_ERR' is defined to replace all other error bit flags, e.g. MAC error,

Oversize error, header buffer overflow error.

quoted

Signed-off-by: Helin Zhang <redacted>
---
 lib/librte_mbuf/rte_mbuf.c      |  7 ++-----
 lib/librte_mbuf/rte_mbuf.h      |  9 +++------
 lib/librte_pmd_i40e/i40e_rxtx.c | 37
++++++++++++++++++++-----------------
 3 files changed, 25 insertions(+), 28 deletions(-)

v2 changes:
* Removed error flag of 'ECIPE' processing only in both i40e PMD and mbuf.

All

quoted

  other error flags were added back.
* Assigned error flags with correct and valid values, as their previous values
  were invalid.
* Enabled getting all error flag names.

v3 changes:
* 'PKT_RX_ERR' is defined to replace error bit flags of MAC error, Oversize
  error, header buffer overflow error.
* Removed assigning values to PKT_TX_* bit flags, as it has already been

done

quoted

  in another packet set.

v4 changes:
* Renamed 'PKT_RX_EIP_CKSUM_BAD' to 'PKT_RX_OUTER_IP_CKSUM_BAD'.
* Fixed the bug of checking error bits of 'Descriptor oversize' and
  'Header buffer oversize'.
* Added debug logs for each RX error.

[...]

quoted

--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h

@@ -83,12 +83,7 @@ extern "C" {
 #define PKT_RX_RSS_HASH      (1ULL << 1)  /**< RX packet with RSS

hash result. */

quoted

 #define PKT_RX_FDIR          (1ULL << 2)  /**< RX packet with FDIR

match indicate. */

quoted

 #define PKT_RX_L4_CKSUM_BAD  (1ULL << 3)  /**< L4 cksum of RX pkt.

is

quoted

not OK. */ -#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP cksum of
RX pkt. is not OK. */ -#define PKT_RX_EIP_CKSUM_BAD (0ULL << 0)  /**<

External IP header checksum error. */

quoted

-#define PKT_RX_OVERSIZE      (0ULL << 0)  /**< Num of desc of an RX

pkt oversize. */

quoted

-#define PKT_RX_HBUF_OVERFLOW (0ULL << 0)  /**< Header buffer

overflow. */

quoted

-#define PKT_RX_RECIP_ERR     (0ULL << 0)  /**< Hardware processing

error. */

quoted

-#define PKT_RX_MAC_ERR       (0ULL << 0)  /**< MAC error. */
+#define PKT_RX_IP_CKSUM_BAD  (1ULL << 4)  /**< IP (or inner IP)
+header checksum error. */

It can be also an outer IP header in case the device don't support tunneling or is
not configured to detect it.

For non-tunneling case, no outer/inner at all, it just has the IP header. The bit flag
indicates the IP header checksum error.
For tunneling case, this bit flag indicates the inner IP header checksum error, another
one for outer IP header checksum error.
So I don't think this bit can be treated as outer.

I think you didn't understand my comment.
I talk about NICs which don't have tunneling support.
In this case, the outer IP header is seen as a simple IP header.
So, depending on which port is receiving a tunneled packet, this
flag or the dedicated one can be used for outer IP checksum.

I just suggest to remove the part "(or inner IP)" of the comment.
Do you agree?

-- 
Thomas