From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 15072959 for ; Fri, 12 Dec 2014 02:27:30 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga102.fm.intel.com with ESMTP; 11 Dec 2014 17:27:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,862,1389772800"; d="scan'208";a="428010936" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by FMSMGA003.fm.intel.com with ESMTP; 11 Dec 2014 17:16:40 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by PGSMSX108.gar.corp.intel.com (10.221.44.103) with Microsoft SMTP Server (TLS) id 14.3.195.1; Fri, 12 Dec 2014 09:27:26 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.240]) with mapi id 14.03.0195.001; Fri, 12 Dec 2014 09:27:26 +0800 From: "Zhang, Helin" To: Olivier MATZ , Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX error flags Thread-Index: AQHQFFzOtE3RTEW+NEmbeyO057Pa/JyI1wTQ//+Va4CAAPq8YIAAUbUAgAFuNIA= Date: Fri, 12 Dec 2014 01:27:25 +0000 Message-ID: References: <1417829617-7185-1-git-send-email-helin.zhang@intel.com> <2233989.0bm00NiAWz@xps13> <17296025.Gs0n2xkUhX@xps13> <54897CF6.2020509@6wind.com> In-Reply-To: <54897CF6.2020509@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX error flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Dec 2014 01:27:32 -0000 > -----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 >=20 > Hi Helin, >=20 > 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 inn= er 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. >=20 > I still don't understand how to use that flag. Let's imagine an applicati= on that > processes an IP packet: >=20 > 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? */ > ... >=20 > 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 rea= l users use it. I think the real users always want to have more error information at runtim= e, 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 o= n itself. >=20 > In my understanding, with your patch, if you receive a tunnel packet on i= 40e, > 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 anymor= e. > 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 I= P could be the real IP packet which is wanted to be processed. >=20 > By the way (it's a bit out of topic), as we already noticed on the list s= ome 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 somewhe= re else? Or can it adds a bit flag like PKT_RX_IP_CKSUM_NOT_OFFLOADED? Regards, Helin >=20 > Regards, > Olivier