[dpdk-dev,v4] mbuf: fix of enabling all newly added RX error flags
Commit Message
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.
Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
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
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.
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.
Comments
2014-12-10 16:55, 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 uses 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@intel.com>
> ---
> 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
> 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.
>
> 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.
[...]
> --- 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. */
> #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_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.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, December 10, 2014 5:35 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
> error flags
>
> 2014-12-10 16:55, 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 uses 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@intel.com>
> > ---
> > 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
> > 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.
> >
> > 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.
> [...]
> > --- 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. */
> > #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_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.
Regards,
Helin
> --
> Thomas
>
> > #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_OUTER_IP_CKSUM_BAD (1ULL << 15) /**< Outer 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 */
2014-12-10 13:50, Zhang, Helin:
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Wednesday, December 10, 2014 5:35 PM
> > To: Zhang, Helin
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
> > error flags
> >
> > 2014-12-10 16:55, 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 uses 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@intel.com>
> > > ---
> > > 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
> > > 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.
> > >
> > > 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.
> > [...]
> > > --- 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. */
> > > #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_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?
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, December 10, 2014 11:26 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
> error flags
>
> 2014-12-10 13:50, Zhang, Helin:
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > Sent: Wednesday, December 10, 2014 5:35 PM
> > > To: Zhang, Helin
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly
> > > added RX error flags
> > >
> > > 2014-12-10 16:55, 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
> uses 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@intel.com>
> > > > ---
> > > > 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
> > > > 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.
> > > >
> > > > 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.
> > > [...]
> > > > --- 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. */
> > > > #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_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 think I did understand your point. For those port which does not support tunneling,
if a 'tunneling' packet received, it never knows that's tunneling packet, it always treats
it as a general IP packet. The "inner" IP is just part of its data. For this case, no outer
or inner at all, just an IP header.
>
> I just suggest to remove the part "(or inner IP)" of the comment.
> Do you agree?
I got it, actually I wanted to describe it as (or inner IP for tunneling), as the macro name
does not tell it could be inner IP header checksum error for tunneling case.
Regards,
Helin
>
> --
> Thomas
Hi Helin,
On 12/10/2014 11:29 PM, Zhang, Helin wrote:
>>>>> --- 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. */
>>>>> #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_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 think I did understand your point. For those port which does not support tunneling,
> if a 'tunneling' packet received, it never knows that's tunneling packet, it always treats
> it as a general IP packet. The "inner" IP is just part of its data. For this case, no outer
> or inner at all, just an IP header.
>
>>
>> I just suggest to remove the part "(or inner IP)" of the comment.
>> Do you agree?
> I got it, actually I wanted to describe it as (or inner IP for tunneling), as the macro name
> does not tell it could be inner IP header checksum error for tunneling case.
I still don't understand how to use that flag. Let's imagine an
application that processes an IP packet:
ip_input(m) /* receive a packet after ethernet header is stripped */
{
if (m->ol_flags & PKT_RX_IP_CKSUM_BAD) {
log("packet dropped");
rte_pktmbuf_free(m);
return;
}
/* continue IP header processing,maybe route the packet? */
...
This kind of code works since a long time with dpdk on ixgbe, even
if you receive a tunnel packet.
In my understanding, with your patch, if you receive a tunnel packet on
i40e, the flag PKT_RX_IP_CKSUM_BAD is about the inner header, which
should not be checked by a router. This would make the code above
not working anymore. Am I correct?
By the way (it's a bit out of topic), as we already noticed on the
list some times, in the future we should add another flags
PKT_RX_IP_CKSUM_VERIFIED in addition to PKT_RX_IP_CKSUM_BAD because
many drivers do not support hardware checksum, or only supports it in
specific conditions (ex: no IP options, or no vlan, ...). We should
think about it for 2.0.
Regards,
Olivier
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Thursday, December 11, 2014 7:16 PM
> To: Zhang, Helin; Thomas Monjalon
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
> error flags
>
> Hi Helin,
>
> On 12/10/2014 11:29 PM, Zhang, Helin wrote:
> >>>>> --- 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. */
> >>>>> #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_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 think I did understand your point. For those port which does not
> > support tunneling, if a 'tunneling' packet received, it never knows
> > that's tunneling packet, it always treats it as a general IP packet.
> > The "inner" IP is just part of its data. For this case, no outer or inner at all,
> just an IP header.
> >
> >>
> >> I just suggest to remove the part "(or inner IP)" of the comment.
> >> Do you agree?
> > I got it, actually I wanted to describe it as (or inner IP for
> > tunneling), as the macro name does not tell it could be inner IP header
> checksum error for tunneling case.
>
> I still don't understand how to use that flag. Let's imagine an application that
> processes an IP packet:
>
> ip_input(m) /* receive a packet after ethernet header is stripped */
> {
> if (m->ol_flags & PKT_RX_IP_CKSUM_BAD) {
> log("packet dropped");
> rte_pktmbuf_free(m);
> return;
> }
> /* continue IP header processing,maybe route the packet? */
> ...
>
> This kind of code works since a long time with dpdk on ixgbe, even if you receive
> a tunnel packet.
I have the similar understanding of yours, though I am not sure how the real users use it.
I think the real users always want to have more error information at runtime, then they
know the root cause and how to deal with it.
For checksum errors, they are in packets which come from peer. If this type of errors is
detected, the end users can check what happens on the peer, but not debug on itself.
>
> In my understanding, with your patch, if you receive a tunnel packet on i40e,
> the flag PKT_RX_IP_CKSUM_BAD is about the inner header, which should not
> be checked by a router. This would make the code above not working anymore.
> Am I correct?
For a tunnel packet received, I think both inner and outer checksum errors should
be checked. And even the inner is more important than outer, as the inner IP could
be the real IP packet which is wanted to be processed.
>
> By the way (it's a bit out of topic), as we already noticed on the list some times,
> in the future we should add another flags PKT_RX_IP_CKSUM_VERIFIED in
> addition to PKT_RX_IP_CKSUM_BAD because many drivers do not support
> hardware checksum, or only supports it in specific conditions (ex: no IP options,
> or no vlan, ...). We should think about it for 2.0.
Good reason for new flags. But I think it may need another bit for outer IP checksum?
Is there any other choice to indicate the checksum is not offloaded somewhere else?
Or can it adds a bit flag like PKT_RX_IP_CKSUM_NOT_OFFLOADED?
Regards,
Helin
>
> Regards,
> Olivier
@@ -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_OUTER_IP_CKSUM_BAD: return "PKT_RX_OUTER_IP_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";
@@ -83,12 +83,7 @@ extern "C" {
#define PKT_RX_RSS_HASH (1ULL << 1) /**< RX packet with RSS hash result. */
#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_IP_CKSUM_BAD (1ULL << 4) /**< IP (or inner IP) header checksum 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_OUTER_IP_CKSUM_BAD (1ULL << 15) /**< Outer 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 */
@@ -68,6 +68,7 @@
#define I40E_TX_MAX_BURST 32
#define I40E_DMA_MEM_ALIGN 4096
+#define I40E_RX_ERR_MASK 0x3F
#define I40E_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
ETH_TXQ_FLAGS_NOOFFLOADS)
@@ -118,30 +119,32 @@ i40e_rxd_error_to_pkt_flags(uint64_t qword)
uint64_t flags = 0;
uint64_t error_bits = (qword >> I40E_RXD_QW1_ERROR_SHIFT);
-#define I40E_RX_ERR_BITS 0x3f
- if (likely((error_bits & I40E_RX_ERR_BITS) == 0))
+ if (likely((error_bits & I40E_RX_ERR_MASK) == 0))
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;
+ PMD_DRV_LOG(DEBUG, "Hardware MAC error");
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)))
+ 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)))
+ PMD_DRV_LOG(DEBUG, "IP or inner IP checksum error");
+ }
+ 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;
+ PMD_DRV_LOG(DEBUG, "L4 integrity error");
+ }
+ if (unlikely(error_bits & (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT))) {
+ flags |= PKT_RX_OUTER_IP_CKSUM_BAD;
+ PMD_DRV_LOG(DEBUG, "Outer IP checksum error");
+ }
+ if (unlikely(error_bits & ((1 << I40E_RX_DESC_ERROR_OVERSIZE_SHIFT) |
+ (1 << I40E_RX_DESC_ERROR_HBO_SHIFT)))) {
+ flags |= PKT_RX_ERR;
+ PMD_DRV_LOG(DEBUG, "Header buffer overflow, or descriptor "
+ "number oversize");
+ }
return flags;
}