2014-12-09 02:14, Zhang, Helin: > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > 2014-12-06 09:33, Helin Zhang: > > > 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> > > [...] > > > --- 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. */ > > > > Could PKT_RX_EIP_CKSUM_BAD be aggregated with > > PKT_RX_IP_CKSUM_BAD? > > I tend to say no, but I would listen to comments from others. > For tunneling case (e.g. IP over IP), it is a bit similar to the case of > L3/L4 (e.g. UDP over IP). > For L3/L4 case, we have PKT_RX_IP_CKSUM_BAD and PKT_RX_L4_CKSUM_BAD to > indicate the checksum error is in L3 or L4. > So I'd prefer to have PKT_RX_IP_CKSUM_BAD and PKT_RX_EIP_CKSUM_BAD to indicate > the checksum error is in outer or inner header.
I think OUTER_IP would be more consistent than EIP. > Otherwise we have no chance to know where the checksum error is, based on > mbuf. > > > The conclusion is the same: the packet is corrupted. > > And some hardwares could not detect the encapsulation and use > > PKT_RX_IP_CKSUM_BAD. > > If the hardware don't know it is a tunneling packet, it will just treat it as > an IP packet. But if > hardware supports tunneling, it would be better for apps to know that more > about the > packet which can be offloaded by hardware. > > > > > Another interesting improvement would be to have PKT_RX_IP_CKSUM_OK. > > I think we'll have to think about this kind of flag for next version. > > For checksum OK, if no 'BAD' indicated, we can assume it is OK. Any other > hints from you? No, having no BAD can indicate also that it hasn't been checked (i.e. check not enabled or not supported). > > Note that this patch is an API change and shouldn't be applied for 1.8.0. > > But we can do an exception as it has no impact on existing applications and > > fixes the 0 flags. > Agree with you! > > Thank you very much for thinking so much about this! > > Regards, > Helin -- Thomas
