DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Helin" <helin.zhang@intel.com>
To: Olivier MATZ <olivier.matz@6wind.com>,
	Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX error flags
Date: Fri, 12 Dec 2014 01:27:25 +0000	[thread overview]
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E70A7D1438@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <54897CF6.2020509@6wind.com>



> -----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

      reply	other threads:[~2014-12-12  1:27 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-26  6:07 [dpdk-dev] [PATCH] i40e: Use one bit flag for all hardware detected RX packet errors Helin Zhang
2014-11-26 10:49 ` Ananyev, Konstantin
2014-11-26 11:22   ` Olivier MATZ
2014-11-26 13:38     ` Ananyev, Konstantin
2014-11-26 14:12       ` Olivier MATZ
2014-11-28  8:07         ` Zhang, Helin
2014-11-28  8:47           ` Olivier MATZ
2014-12-01  1:57             ` Zhang, Helin
2014-12-01  9:58               ` Olivier MATZ
2014-12-02  7:25                 ` Zhang, Helin
2014-12-05  1:46 ` [dpdk-dev] [PATCH v2 0/2] fix of enabling all newly added error flags Helin Zhang
2014-12-05  1:46   ` [dpdk-dev] [PATCH v2 1/2] i40e: remove checking rxd flag which is not public Helin Zhang
2014-12-05  1:46   ` [dpdk-dev] [PATCH v2 2/2] mbuf: assign valid bit values for some RX and TX flags Helin Zhang
2014-12-05 10:49     ` Ananyev, Konstantin
2014-12-06  0:42       ` Zhang, Helin
2014-12-06  1:07       ` Zhang, Helin
2014-12-08 10:55         ` Ananyev, Konstantin
2014-12-09  2:29           ` Zhang, Helin
2014-12-06  1:33   ` [dpdk-dev] [PATCH v3] mbuf: fix of enabling all newly added RX error flags Helin Zhang
2014-12-08 10:44     ` Ananyev, Konstantin
2014-12-09  2:23       ` Zhang, Helin
2014-12-08 10:50     ` Thomas Monjalon
2014-12-09  2:14       ` Zhang, Helin
2014-12-09  6:22         ` Thomas Monjalon
2014-12-10  8:55     ` [dpdk-dev] [PATCH v4] " Helin Zhang
2014-12-10  9:35       ` Thomas Monjalon
2014-12-10 13:50         ` Zhang, Helin
2014-12-10 15:26           ` Thomas Monjalon
2014-12-10 22:29             ` Zhang, Helin
2014-12-11 11:16               ` Olivier MATZ
2014-12-12  1:27                 ` Zhang, Helin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F35DEAC7BCE34641BA9FAC6BCA4A12E70A7D1438@SHSMSX104.ccr.corp.intel.com \
    --to=helin.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).