From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <helin.zhang@intel.com>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 55C1A6A95
 for <dev@dpdk.org>; Wed, 10 Dec 2014 23:29:33 +0100 (CET)
Received: from orsmga001.jf.intel.com ([10.7.209.18])
 by orsmga102.jf.intel.com with ESMTP; 10 Dec 2014 14:27:42 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.07,554,1413270000"; d="scan'208";a="621846799"
Received: from kmsmsx153.gar.corp.intel.com ([172.21.73.88])
 by orsmga001.jf.intel.com with ESMTP; 10 Dec 2014 14:29:04 -0800
Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by
 KMSMSX153.gar.corp.intel.com (172.21.73.88) with Microsoft SMTP Server (TLS)
 id 14.3.195.1; Thu, 11 Dec 2014 06:29:04 +0800
Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.182]) by
 shsmsx102.ccr.corp.intel.com ([169.254.2.216]) with mapi id 14.03.0195.001;
 Thu, 11 Dec 2014 06:29:02 +0800
From: "Zhang, Helin" <helin.zhang@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH v4] mbuf: fix of enabling all newly added RX
 error flags
Thread-Index: AQHQFFzOtE3RTEW+NEmbeyO057Pa/JyI1wTQ//+Va4CAAPq8YA==
Date: Wed, 10 Dec 2014 22:29:02 +0000
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E70A7D0A78@SHSMSX104.ccr.corp.intel.com>
References: <1417829617-7185-1-git-send-email-helin.zhang@intel.com>
 <2233989.0bm00NiAWz@xps13>
 <F35DEAC7BCE34641BA9FAC6BCA4A12E70A7D086F@SHSMSX104.ccr.corp.intel.com>
 <17296025.Gs0n2xkUhX@xps13>
In-Reply-To: <17296025.Gs0n2xkUhX@xps13>
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" <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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 10 Dec 2014 22:29:34 -0000



> -----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
>=20
> 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 'o=
l_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 an=
d
> mbuf.
> > > All
> > > >   other error flags were added back.
> > > > * Assigned error flags with correct and valid values, as their prev=
ious
> 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.
>=20
> 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, i=
t always treats
it as a general IP packet. The "inner" IP is just part of its data. For thi=
s case, no outer
or inner at all, just an IP header.

>=20
> 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
>=20
> --
> Thomas