diff mbox

[dpdk-dev,v2,2/2] mbuf: assign valid bit values for some RX and TX flags

Message ID 1417743988-15604-3-git-send-email-helin.zhang@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Helin Zhang Dec. 5, 2014, 1:46 a.m. UTC
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.

Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
 lib/librte_mbuf/rte_mbuf.c |  9 ++++-----
 lib/librte_mbuf/rte_mbuf.h | 19 +++++++++----------
 2 files changed, 13 insertions(+), 15 deletions(-)

v2 changes:
* Removed error flag of 'ECIPE' processing only in 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.

Comments

Ananyev, Konstantin Dec. 5, 2014, 10:49 a.m. UTC | #1
Hi Helin,

> -----Original Message-----
> From: Zhang, Helin
> Sent: Friday, December 05, 2014 1:46 AM
> To: dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang; Ananyev, Konstantin; olivier.matz@6wind.com; Zhang, Helin
> Subject: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX 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.
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  lib/librte_mbuf/rte_mbuf.c |  9 ++++-----
>  lib/librte_mbuf/rte_mbuf.h | 19 +++++++++----------
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> v2 changes:
> * Removed error flag of 'ECIPE' processing only in 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.
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 87c2963..3ce7c8d 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -210,11 +210,10 @@ 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_OVERSIZE: return "PKT_RX_OVERSIZE";
> +	case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW";
> +	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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 2e5fce5..c9591c0 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,10 @@ 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_OVERSIZE      (1ULL << 16)  /**< Num of desc of an RX pkt oversize. */
> +#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer overflow. */
> +#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
>  /* add new RX flags here */

I still think there is no point to have several flags to indicate HW error for the packet.
As I suggested before we can collapse 3 of them (OVERSIZE, HBUF_OVERFLOW, MAC_ERR) into one. 
As I remember, Oliver even suggested to drop such packets.
As was said above - if is not a whole packet SW can't do much with it anyway.
The only thing such bad packets can probably be used for - some sort of debugging.
So we probably can combine both things: 
- in normal operation just drop such packet 
- if PMD_DEBUG_RX is enabled, then write a log record, set RX_HW_ERR and deliver a packet to the upper layer.

> 
>  /* add new TX flags here */
> @@ -141,13 +140,13 @@ extern "C" {
>  #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt. computed by NIC. */
>  #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */
> 
> -/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
> -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> +/** Tell the NIC it's an IPv4 packet. */
> +#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4 packet. */
> 
> -/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
> -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> +/** Tell the NIC it's an IPv6 packet. */
> +#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6 packet. */
> 
> -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
> +#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a VLAN packet. */

I don't think these changes should be part of that patch.
They violate another patch that Frank sent before.

Konstantin

> 
>  /* Use final bit of flags to indicate a control mbuf */
>  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
> --
> 1.9.3
Helin Zhang Dec. 6, 2014, 12:42 a.m. UTC | #2
OK. I will send out another patch according to your comments. Thanks a lot!

Regards,
Helin

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, December 5, 2014 6:50 PM
> To: Zhang, Helin; dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> olivier.matz@6wind.com
> Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX
> flags
> 
> Hi Helin,
> 
> > -----Original Message-----
> > From: Zhang, Helin
> > Sent: Friday, December 05, 2014 1:46 AM
> > To: dev@dpdk.org
> > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang; Ananyev,
> > Konstantin; olivier.matz@6wind.com; Zhang, Helin
> > Subject: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and
> > TX 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.
> >
> > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > ---
> >  lib/librte_mbuf/rte_mbuf.c |  9 ++++-----  lib/librte_mbuf/rte_mbuf.h
> > | 19 +++++++++----------
> >  2 files changed, 13 insertions(+), 15 deletions(-)
> >
> > v2 changes:
> > * Removed error flag of 'ECIPE' processing only in 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.
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> > index 87c2963..3ce7c8d 100644
> > --- a/lib/librte_mbuf/rte_mbuf.c
> > +++ b/lib/librte_mbuf/rte_mbuf.c
> > @@ -210,11 +210,10 @@ 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_OVERSIZE: return "PKT_RX_OVERSIZE";
> > +	case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW";
> > +	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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
> > 2e5fce5..c9591c0 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,10 @@ 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_OVERSIZE      (1ULL << 16)  /**< Num of desc of an RX
> pkt oversize. */
> > +#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer
> overflow. */
> > +#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
> >  /* add new RX flags here */
> 
> I still think there is no point to have several flags to indicate HW error for the
> packet.
> As I suggested before we can collapse 3 of them (OVERSIZE, HBUF_OVERFLOW,
> MAC_ERR) into one.
> As I remember, Oliver even suggested to drop such packets.
> As was said above - if is not a whole packet SW can't do much with it anyway.
> The only thing such bad packets can probably be used for - some sort of
> debugging.
> So we probably can combine both things:
> - in normal operation just drop such packet
> - if PMD_DEBUG_RX is enabled, then write a log record, set RX_HW_ERR and
> deliver a packet to the upper layer.
> 
> >
> >  /* add new TX flags here */
> > @@ -141,13 +140,13 @@ extern "C" {
> >  #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt.
> computed by NIC. */
> >  #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of
> PKT_TX_IP_CKSUM. */
> >
> > -/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO.
> */
> > -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> > +/** Tell the NIC it's an IPv4 packet. */
> > +#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4
> packet. */
> >
> > -/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO.
> */
> > -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> > +/** Tell the NIC it's an IPv6 packet. */
> > +#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6
> packet. */
> >
> > -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q
> VLAN packet. */
> > +#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a VLAN
> packet. */
> 
> I don't think these changes should be part of that patch.
> They violate another patch that Frank sent before.
> 
> Konstantin
> 
> >
> >  /* Use final bit of flags to indicate a control mbuf */
> >  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control
> data */
> > --
> > 1.9.3
Helin Zhang Dec. 6, 2014, 1:07 a.m. UTC | #3
> -----Original Message-----
> From: Zhang, Helin
> Sent: Saturday, December 6, 2014 8:40 AM
> To: Ananyev, Konstantin; dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> olivier.matz@6wind.com
> Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX
> flags
> 
> OK. I will send out another patch according to your comments. Thanks a lot!
> 
> Regards,
> Helin
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Friday, December 5, 2014 6:50 PM
> > To: Zhang, Helin; dev@dpdk.org
> > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > olivier.matz@6wind.com
> > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX
> > and TX flags
> >
> > Hi Helin,
> >
> > > -----Original Message-----
> > > From: Zhang, Helin
> > > Sent: Friday, December 05, 2014 1:46 AM
> > > To: dev@dpdk.org
> > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang; Ananyev,
> > > Konstantin; olivier.matz@6wind.com; Zhang, Helin
> > > Subject: [PATCH v2 2/2] mbuf: assign valid bit values for some RX
> > > and TX 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.
> > >
> > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > > ---
> > >  lib/librte_mbuf/rte_mbuf.c |  9 ++++-----
> > > lib/librte_mbuf/rte_mbuf.h
> > > | 19 +++++++++----------
> > >  2 files changed, 13 insertions(+), 15 deletions(-)
> > >
> > > v2 changes:
> > > * Removed error flag of 'ECIPE' processing only in 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.
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> > > index 87c2963..3ce7c8d 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.c
> > > +++ b/lib/librte_mbuf/rte_mbuf.c
> > > @@ -210,11 +210,10 @@ 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_OVERSIZE: return "PKT_RX_OVERSIZE";
> > > +	case PKT_RX_HBUF_OVERFLOW: return
> "PKT_RX_HBUF_OVERFLOW";
> > > +	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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
> > > 2e5fce5..c9591c0 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,10 @@ 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_OVERSIZE      (1ULL << 16)  /**< Num of desc of an
> RX
> > pkt oversize. */
> > > +#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer
> > overflow. */
> > > +#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
> > >  /* add new RX flags here */
> >
> > I still think there is no point to have several flags to indicate HW
> > error for the packet.
> > As I suggested before we can collapse 3 of them (OVERSIZE,
> > HBUF_OVERFLOW,
> > MAC_ERR) into one.
> > As I remember, Oliver even suggested to drop such packets.
> > As was said above - if is not a whole packet SW can't do much with it anyway.
> > The only thing such bad packets can probably be used for - some sort
> > of debugging.
> > So we probably can combine both things:
> > - in normal operation just drop such packet
> > - if PMD_DEBUG_RX is enabled, then write a log record, set RX_HW_ERR
> > and deliver a packet to the upper layer.
I still do not want to drop the bad packet here, as it may affect vector processing.
At least it should be investigated how much impact on vector RX. I prefer to let
up-layer software do that.

> >
> > >
> > >  /* add new TX flags here */
> > > @@ -141,13 +140,13 @@ extern "C" {
> > >  #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt.
> > computed by NIC. */
> > >  #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of
> > PKT_TX_IP_CKSUM. */
> > >
> > > -/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or
> TSO.
> > */
> > > -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> > > +/** Tell the NIC it's an IPv4 packet. */
> > > +#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4
> > packet. */
> > >
> > > -/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or
> TSO.
> > */
> > > -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> > > +/** Tell the NIC it's an IPv6 packet. */
> > > +#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6
> > packet. */
> > >
> > > -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q
> > VLAN packet. */
> > > +#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a VLAN
> > packet. */
> >
> > I don't think these changes should be part of that patch.
> > They violate another patch that Frank sent before.
> >
> > Konstantin
> >
> > >
> > >  /* Use final bit of flags to indicate a control mbuf */
> > >  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains
> control
> > data */
> > > --
> > > 1.9.3
Ananyev, Konstantin Dec. 8, 2014, 10:55 a.m. UTC | #4
> -----Original Message-----
> From: Zhang, Helin
> Sent: Saturday, December 06, 2014 1:08 AM
> To: Ananyev, Konstantin; dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang; olivier.matz@6wind.com
> Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX flags
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Helin
> > Sent: Saturday, December 6, 2014 8:40 AM
> > To: Ananyev, Konstantin; dev@dpdk.org
> > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > olivier.matz@6wind.com
> > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX
> > flags
> >
> > OK. I will send out another patch according to your comments. Thanks a lot!
> >
> > Regards,
> > Helin
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Friday, December 5, 2014 6:50 PM
> > > To: Zhang, Helin; dev@dpdk.org
> > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > > olivier.matz@6wind.com
> > > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX
> > > and TX flags
> > >
> > > Hi Helin,
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Helin
> > > > Sent: Friday, December 05, 2014 1:46 AM
> > > > To: dev@dpdk.org
> > > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang; Ananyev,
> > > > Konstantin; olivier.matz@6wind.com; Zhang, Helin
> > > > Subject: [PATCH v2 2/2] mbuf: assign valid bit values for some RX
> > > > and TX 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.
> > > >
> > > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > > > ---
> > > >  lib/librte_mbuf/rte_mbuf.c |  9 ++++-----
> > > > lib/librte_mbuf/rte_mbuf.h
> > > > | 19 +++++++++----------
> > > >  2 files changed, 13 insertions(+), 15 deletions(-)
> > > >
> > > > v2 changes:
> > > > * Removed error flag of 'ECIPE' processing only in 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.
> > > >
> > > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> > > > index 87c2963..3ce7c8d 100644
> > > > --- a/lib/librte_mbuf/rte_mbuf.c
> > > > +++ b/lib/librte_mbuf/rte_mbuf.c
> > > > @@ -210,11 +210,10 @@ 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_OVERSIZE: return "PKT_RX_OVERSIZE";
> > > > +	case PKT_RX_HBUF_OVERFLOW: return
> > "PKT_RX_HBUF_OVERFLOW";
> > > > +	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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
> > > > 2e5fce5..c9591c0 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,10 @@ 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_OVERSIZE      (1ULL << 16)  /**< Num of desc of an
> > RX
> > > pkt oversize. */
> > > > +#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer
> > > overflow. */
> > > > +#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
> > > >  /* add new RX flags here */
> > >
> > > I still think there is no point to have several flags to indicate HW
> > > error for the packet.
> > > As I suggested before we can collapse 3 of them (OVERSIZE,
> > > HBUF_OVERFLOW,
> > > MAC_ERR) into one.
> > > As I remember, Oliver even suggested to drop such packets.
> > > As was said above - if is not a whole packet SW can't do much with it anyway.
> > > The only thing such bad packets can probably be used for - some sort
> > > of debugging.
> > > So we probably can combine both things:
> > > - in normal operation just drop such packet
> > > - if PMD_DEBUG_RX is enabled, then write a log record, set RX_HW_ERR
> > > and deliver a packet to the upper layer.
> I still do not want to drop the bad packet here, as it may affect vector processing.
> At least it should be investigated how much impact on vector RX. I prefer to let
> up-layer software do that.

But right now, i40e doesn't have any vector RX support.
You probably meant implications with scatter RX, no?
Another thing - if you HW can't receive packets normally, then something really wrong is going on.
In such situation, you probably wouldn't worry about your RX performance anyway :) 
But I suppose it is a good first step, let's just collapse 3 error flags into one for now.
Then  we can decide should we drop such packets or not. 
Konstantin

> 
> > >
> > > >
> > > >  /* add new TX flags here */
> > > > @@ -141,13 +140,13 @@ extern "C" {
> > > >  #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt.
> > > computed by NIC. */
> > > >  #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of
> > > PKT_TX_IP_CKSUM. */
> > > >
> > > > -/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or
> > TSO.
> > > */
> > > > -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> > > > +/** Tell the NIC it's an IPv4 packet. */
> > > > +#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4
> > > packet. */
> > > >
> > > > -/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or
> > TSO.
> > > */
> > > > -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> > > > +/** Tell the NIC it's an IPv6 packet. */
> > > > +#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6
> > > packet. */
> > > >
> > > > -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q
> > > VLAN packet. */
> > > > +#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a VLAN
> > > packet. */
> > >
> > > I don't think these changes should be part of that patch.
> > > They violate another patch that Frank sent before.
> > >
> > > Konstantin
> > >
> > > >
> > > >  /* Use final bit of flags to indicate a control mbuf */
> > > >  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains
> > control
> > > data */
> > > > --
> > > > 1.9.3
Helin Zhang Dec. 9, 2014, 2:29 a.m. UTC | #5
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Monday, December 8, 2014 6:55 PM
> To: Zhang, Helin; dev@dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> olivier.matz@6wind.com
> Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX
> flags
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Helin
> > Sent: Saturday, December 06, 2014 1:08 AM
> > To: Ananyev, Konstantin; dev@dpdk.org
> > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > olivier.matz@6wind.com
> > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some RX
> > and TX flags
> >
> >
> >
> > > -----Original Message-----
> > > From: Zhang, Helin
> > > Sent: Saturday, December 6, 2014 8:40 AM
> > > To: Ananyev, Konstantin; dev@dpdk.org
> > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > > olivier.matz@6wind.com
> > > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some
> > > RX and TX flags
> > >
> > > OK. I will send out another patch according to your comments. Thanks a lot!
> > >
> > > Regards,
> > > Helin
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Friday, December 5, 2014 6:50 PM
> > > > To: Zhang, Helin; dev@dpdk.org
> > > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > > > olivier.matz@6wind.com
> > > > Subject: RE: [PATCH v2 2/2] mbuf: assign valid bit values for some
> > > > RX and TX flags
> > > >
> > > > Hi Helin,
> > > >
> > > > > -----Original Message-----
> > > > > From: Zhang, Helin
> > > > > Sent: Friday, December 05, 2014 1:46 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Liu, Jijiang;
> > > > > Ananyev, Konstantin; olivier.matz@6wind.com; Zhang, Helin
> > > > > Subject: [PATCH v2 2/2] mbuf: assign valid bit values for some
> > > > > RX and TX 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.
> > > > >
> > > > > Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> > > > > ---
> > > > >  lib/librte_mbuf/rte_mbuf.c |  9 ++++-----
> > > > > lib/librte_mbuf/rte_mbuf.h
> > > > > | 19 +++++++++----------
> > > > >  2 files changed, 13 insertions(+), 15 deletions(-)
> > > > >
> > > > > v2 changes:
> > > > > * Removed error flag of 'ECIPE' processing only in 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.
> > > > >
> > > > > diff --git a/lib/librte_mbuf/rte_mbuf.c
> > > > > b/lib/librte_mbuf/rte_mbuf.c index 87c2963..3ce7c8d 100644
> > > > > --- a/lib/librte_mbuf/rte_mbuf.c
> > > > > +++ b/lib/librte_mbuf/rte_mbuf.c
> > > > > @@ -210,11 +210,10 @@ 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_OVERSIZE: return "PKT_RX_OVERSIZE";
> > > > > +	case PKT_RX_HBUF_OVERFLOW: return
> > > "PKT_RX_HBUF_OVERFLOW";
> > > > > +	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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
> > > > > 2e5fce5..c9591c0 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,10 @@ 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_OVERSIZE      (1ULL << 16)  /**< Num of desc of
> an
> > > RX
> > > > pkt oversize. */
> > > > > +#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer
> > > > overflow. */
> > > > > +#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
> > > > >  /* add new RX flags here */
> > > >
> > > > I still think there is no point to have several flags to indicate
> > > > HW error for the packet.
> > > > As I suggested before we can collapse 3 of them (OVERSIZE,
> > > > HBUF_OVERFLOW,
> > > > MAC_ERR) into one.
> > > > As I remember, Oliver even suggested to drop such packets.
> > > > As was said above - if is not a whole packet SW can't do much with it
> anyway.
> > > > The only thing such bad packets can probably be used for - some
> > > > sort of debugging.
> > > > So we probably can combine both things:
> > > > - in normal operation just drop such packet
> > > > - if PMD_DEBUG_RX is enabled, then write a log record, set
> > > > RX_HW_ERR and deliver a packet to the upper layer.
> > I still do not want to drop the bad packet here, as it may affect vector
> processing.
> > At least it should be investigated how much impact on vector RX. I
> > prefer to let up-layer software do that.
> 
> But right now, i40e doesn't have any vector RX support.
> You probably meant implications with scatter RX, no?
I was thinking we may need to add the similar things in igb and ixgbe (possibly vector).

> Another thing - if you HW can't receive packets normally, then something really
> wrong is going on.
> In such situation, you probably wouldn't worry about your RX performance
That may need an if() for each received packets without any error, though might
not affect performance number.

> anyway :) But I suppose it is a good first step, let's just collapse 3 error flags
> into one for now.
> Then  we can decide should we drop such packets or not.
> Konstantin
Agree to think more on this point!

> 
> >
> > > >
> > > > >
> > > > >  /* add new TX flags here */
> > > > > @@ -141,13 +140,13 @@ extern "C" {
> > > > >  #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX
> pkt.
> > > > computed by NIC. */
> > > > >  #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of
> > > > PKT_TX_IP_CKSUM. */
> > > > >
> > > > > -/** Tell the NIC it's an IPv4 packet. Required for L4 checksum
> > > > > offload or
> > > TSO.
> > > > */
> > > > > -#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> > > > > +/** Tell the NIC it's an IPv4 packet. */
> > > > > +#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4
> > > > packet. */
> > > > >
> > > > > -/** Tell the NIC it's an IPv6 packet. Required for L4 checksum
> > > > > offload or
> > > TSO.
> > > > */
> > > > > -#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> > > > > +/** Tell the NIC it's an IPv6 packet. */
> > > > > +#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6
> > > > packet. */
> > > > >
> > > > > -#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a
> 802.1q
> > > > VLAN packet. */
> > > > > +#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a
> VLAN
> > > > packet. */
> > > >
> > > > I don't think these changes should be part of that patch.
> > > > They violate another patch that Frank sent before.
> > > >
> > > > Konstantin
> > > >
> > > > >
> > > > >  /* Use final bit of flags to indicate a control mbuf */
> > > > >  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains
> > > control
> > > > data */
> > > > > --
> > > > > 1.9.3

Regards,
Helin
diff mbox

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 87c2963..3ce7c8d 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -210,11 +210,10 @@  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_OVERSIZE: return "PKT_RX_OVERSIZE";
+	case PKT_RX_HBUF_OVERFLOW: return "PKT_RX_HBUF_OVERFLOW";
+	case PKT_RX_MAC_ERR: return "PKT_RX_MAC_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 2e5fce5..c9591c0 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,10 @@  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_OVERSIZE      (1ULL << 16)  /**< Num of desc of an RX pkt oversize. */
+#define PKT_RX_HBUF_OVERFLOW (1ULL << 17)  /**< Header buffer overflow. */
+#define PKT_RX_MAC_ERR       (1ULL << 18)  /**< MAC error. */
 /* add new RX flags here */
 
 /* add new TX flags here */
@@ -141,13 +140,13 @@  extern "C" {
 #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt. computed by NIC. */
 #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */
 
-/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
-#define PKT_TX_IPV4          PKT_RX_IPV4_HDR
+/** Tell the NIC it's an IPv4 packet. */
+#define PKT_TX_IPV4          (1ULL << 55) /**< TX packet is a IPV4 packet. */
 
-/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
-#define PKT_TX_IPV6          PKT_RX_IPV6_HDR
+/** Tell the NIC it's an IPv6 packet. */
+#define PKT_TX_IPV6          (1ULL << 56) /**< TX packet is a IPV6 packet. */
 
-#define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
+#define PKT_TX_VLAN_PKT      (1ULL << 57) /**< TX packet is a VLAN packet. */
 
 /* Use final bit of flags to indicate a control mbuf */
 #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */