> -----Original Message----- > From: Ananyev, Konstantin > Sent: Monday, December 8, 2014 6:45 PM > To: Zhang, Helin; dev at dpdk.org > Cc: Cao, Waterman; Cao, Min; olivier.matz at 6wind.com > Subject: RE: [PATCH v3] mbuf: fix of enabling all newly added RX error flags > > Hi Helin, > > > -----Original Message----- > > From: Zhang, Helin > > Sent: Saturday, December 06, 2014 1:34 AM > > To: dev at dpdk.org > > Cc: Cao, Waterman; Cao, Min; olivier.matz at 6wind.com; Ananyev, > > Konstantin; Zhang, Helin > > Subject: [PATCH v3] mbuf: fix of enabling all newly added RX error > > flags > > > > 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 will use it. > > 'PKT_RX_ERR' is defined to replace all other error bit flags, e.g. MAC > > error, Oversize error, header buffer overflow error. > > > > Signed-off-by: Helin Zhang <helin.zhang at intel.com> > > --- > > lib/librte_mbuf/rte_mbuf.c | 7 ++----- > > lib/librte_mbuf/rte_mbuf.h | 7 ++----- > > lib/librte_pmd_i40e/i40e_rxtx.c | 15 ++++----------- > > 3 files changed, 8 insertions(+), 21 deletions(-) > > > > v2 changes: > > * Removed error flag of 'ECIPE' processing only in both i40e PMD and mbuf. > All > > 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 > > in another packet set. > > > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > > index 1b14e02..5e6b5d0 100644 > > --- a/lib/librte_mbuf/rte_mbuf.c > > +++ b/lib/librte_mbuf/rte_mbuf.c > > @@ -210,11 +210,8 @@ const char *rte_get_rx_ol_flag_name(uint64_t > mask) > > case PKT_RX_FDIR: return "PKT_RX_FDIR"; > > case PKT_RX_L4_CKSUM_BAD: return "PKT_RX_L4_CKSUM_BAD"; > > case PKT_RX_IP_CKSUM_BAD: return "PKT_RX_IP_CKSUM_BAD"; > > - /* case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD"; */ > > - /* case PKT_RX_OVERSIZE: return "PKT_RX_OVERSIZE"; */ > > - /* case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW"; > */ > > - /* case PKT_RX_RECIP_ERR: return "PKT_RX_RECIP_ERR"; */ > > - /* case PKT_RX_MAC_ERR: return "PKT_RX_MAC_ERR"; */ > > + case PKT_RX_EIP_CKSUM_BAD: return "PKT_RX_EIP_CKSUM_BAD"; > > + case PKT_RX_ERR: return "PKT_RX_ERR"; > > case PKT_RX_IPV4_HDR: return "PKT_RX_IPV4_HDR"; > > case PKT_RX_IPV4_HDR_EXT: return "PKT_RX_IPV4_HDR_EXT"; > > case PKT_RX_IPV6_HDR: return "PKT_RX_IPV6_HDR"; diff --git > > a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index > > efdefc4..5e647a9 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -84,11 +84,6 @@ extern "C" { > > #define PKT_RX_FDIR (1ULL << 2) /**< RX packet with FDIR > match indicate. */ > > #define PKT_RX_L4_CKSUM_BAD (1ULL << 3) /**< L4 cksum of RX pkt. > is > > 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. */ > > -#define PKT_RX_OVERSIZE (0ULL << 0) /**< Num of desc of an RX > pkt oversize. */ > > -#define PKT_RX_HBUF_OVERFLOW (0ULL << 0) /**< Header buffer > overflow. */ > > -#define PKT_RX_RECIP_ERR (0ULL << 0) /**< Hardware processing > error. */ > > -#define PKT_RX_MAC_ERR (0ULL << 0) /**< MAC error. */ > > #define PKT_RX_IPV4_HDR (1ULL << 5) /**< RX packet with IPv4 > header. */ > > #define PKT_RX_IPV4_HDR_EXT (1ULL << 6) /**< RX packet with > extended IPv4 header. */ > > #define PKT_RX_IPV6_HDR (1ULL << 7) /**< RX packet with IPv6 > header. */ > > @@ -99,6 +94,8 @@ extern "C" { > > #define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet > with IPv6 header. */ > > #define PKT_RX_FDIR_ID (1ULL << 13) /**< FD id reported if FDIR > match. */ > > #define PKT_RX_FDIR_FLX (1ULL << 14) /**< Flexible bytes reported > if FDIR match. */ > > +#define PKT_RX_EIP_CKSUM_BAD (1ULL << 15) /**< External IP header > checksum error. */ > > +#define PKT_RX_ERR (1ULL << 16) /**< Other errors, e.g. > MAC error. */ > > /* add new RX flags here */ > > > > /* add new TX flags here */ > > diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c > > b/lib/librte_pmd_i40e/i40e_rxtx.c index 2beae3c..5d99bd2 100644 > > --- a/lib/librte_pmd_i40e/i40e_rxtx.c > > +++ b/lib/librte_pmd_i40e/i40e_rxtx.c > > @@ -123,25 +123,18 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword) > > return flags; > > /* If RXE bit set, all other status bits are meaningless */ > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RXE_SHIFT))) { > > - flags |= PKT_RX_MAC_ERR; > > + flags |= PKT_RX_ERR; > > return flags; > > } > > - > > - /* If RECIPE bit set, all other status indications should be ignored */ > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_RECIPE_SHIFT))) { > > - flags |= PKT_RX_RECIP_ERR; > > - return flags; > > - } > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_HBO_SHIFT))) > > - flags |= PKT_RX_HBUF_OVERFLOW; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_IPE_SHIFT))) > > flags |= PKT_RX_IP_CKSUM_BAD; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_L4E_SHIFT))) > > flags |= PKT_RX_L4_CKSUM_BAD; > > if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT))) > > flags |= PKT_RX_EIP_CKSUM_BAD; > > - if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT))) > > - flags |= PKT_RX_OVERSIZE; > > + if (unlikely(error_bits & ((1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT) || > > + (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))) > > + flags |= PKT_RX_ERR; > > It should be: > error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT | 1 << > I40E_RX_DESC_ERROR_HBO_SHIFT) Ohm, stupid fault! I will fix it right now.
> > Another 2 questions: > - Why not to put all these 3 bits processing together: > error_bits & (1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT | 1 << > I40E_RX_DESC_ERROR_HBO_SHIFT | 1 << > I40E_RX_DESC_ERROR_RXE_SHIFT) ? If RECIPE, other bits are meaningless, said by EAS. If HBO or OVERSIZE, IPE, L4E, EIPE could be meaningful correct or not. > - Is there any reason why you don't want to put PMD_RX_LOG(DEBUG, ...) for > them? > I think it might be usefull for debugging. For all error bits or just HBO and OVERSIZE? > > Konstantin > > > > > > return flags; > > } > > -- > > 1.9.3 Regards, Helin