From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tomaszx.kulasek@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by dpdk.org (Postfix) with ESMTP id F3749532C
 for <dev@dpdk.org>; Mon, 24 Oct 2016 16:12:58 +0200 (CEST)
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
 by orsmga105.jf.intel.com with ESMTP; 24 Oct 2016 07:12:57 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.31,542,1473145200"; d="scan'208";a="23010930"
Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31])
 by fmsmga005.fm.intel.com with ESMTP; 24 Oct 2016 07:12:55 -0700
Received: from irsmsx102.ger.corp.intel.com ([169.254.2.226]) by
 IRSMSX106.ger.corp.intel.com ([169.254.8.99]) with mapi id 14.03.0248.002;
 Mon, 24 Oct 2016 15:12:54 +0100
From: "Kulasek, TomaszX" <tomaszx.kulasek@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
CC: "olivier.matz@6wind.com" <olivier.matz@6wind.com>
Thread-Topic: [PATCH v8 1/6] ethdev: add Tx preparation
Thread-Index: AQHSLfA4wLBbvj9PRU+x2H7f+gxpN6C3i/XA///ziQCAACWQkA==
Date: Mon, 24 Oct 2016 14:12:54 +0000
Message-ID: <3042915272161B4EB253DA4D77EB373A14F446C5@IRSMSX102.ger.corp.intel.com>
References: <1477057376-16224-1-git-send-email-tomaszx.kulasek@intel.com>
 <1477061177-15828-1-git-send-email-tomaszx.kulasek@intel.com>
 <1477061177-15828-2-git-send-email-tomaszx.kulasek@intel.com>
 <2601191342CEEE43887BDE71AB9772583F0CB086@irsmsx105.ger.corp.intel.com>
 <3042915272161B4EB253DA4D77EB373A14F44670@IRSMSX102.ger.corp.intel.com>
 <2601191342CEEE43887BDE71AB9772583F0CB102@irsmsx105.ger.corp.intel.com>
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0CB102@irsmsx105.ger.corp.intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v8 1/6] ethdev: add Tx preparation
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: Mon, 24 Oct 2016 14:12:59 -0000



> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Monday, October 24, 2016 14:57
> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com
> Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
>=20
>=20
>=20
> > -----Original Message-----
> > From: Kulasek, TomaszX
> > Sent: Monday, October 24, 2016 1:49 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> > Cc: olivier.matz@6wind.com
> > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
> >
> > Hi Konstantin,
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Monday, October 24, 2016 14:15
> > > To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> > > Cc: olivier.matz@6wind.com
> > > Subject: RE: [PATCH v8 1/6] ethdev: add Tx preparation
> > >
> > > Hi Tomasz,
> > >
> >
> > [...]
> >
> > > >
> > > > +/**
> > > > + * Fix pseudo header checksum
> > > > + *
> > > > + * This function fixes pseudo header checksum for TSO and non-TSO
> > > > +tcp/udp in
> > > > + * provided mbufs packet data.
> > > > + *
> > > > + * - for non-TSO tcp/udp packets full pseudo-header checksum is
> > > > +counted
> > > and set
> > > > + *   in packet data,
> > > > + * - for TSO the IP payload length is not included in pseudo
> header.
> > > > + *
> > > > + * This function expects that used headers are in the first data
> > > > +segment of
> > > > + * mbuf, and are not fragmented.
> > > > + *
> > > > + * @param m
> > > > + *   The packet mbuf to be validated.
> > > > + * @return
> > > > + *   0 if checksum is initialized properly
> > > > + */
> > > > +static inline int
> > > > +rte_phdr_cksum_fix(struct rte_mbuf *m) {
> > > > +	struct ipv4_hdr *ipv4_hdr;
> > > > +	struct ipv6_hdr *ipv6_hdr;
> > > > +	struct tcp_hdr *tcp_hdr;
> > > > +	struct udp_hdr *udp_hdr;
> > > > +	uint64_t ol_flags =3D m->ol_flags;
> > > > +	uint64_t inner_l3_offset =3D m->l2_len;
> > > > +
> > > > +	if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
> > > > +		inner_l3_offset +=3D m->outer_l2_len + m->outer_l3_len;
> > > > +
> > > > +	/* headers are fragmented */
> > > > +	if (unlikely(rte_pktmbuf_data_len(m) >=3D inner_l3_offset +
> > > > +m->l3_len
> > > +
> > > > +			m->l4_len))
> > >
> > > Might be better to move that check into rte_validate_tx_offload(),
> > > so it would be called only when TX_DEBUG is on.
> >
> > While unfragmented headers are not general requirements for Tx
> > offloads, and this requirement is for this particular implementation,
> maybe for performance reasons will be better to keep it here, and just ad=
d
> #if DEBUG to leave rte_validate_tx_offload more generic.
>=20
> Hmm and what is the advantage to pollute that code with more ifdefs?
> Again, why unfragmented headers are not general requirements?
> As long as DPDK pseudo-headear csum calculation routines can't handle
> fragmented case, it pretty much is a general requirement, no?
> Konstantin
>=20

Ok, you're right, if we assume that this is general requirement, it should =
be moved.

> >
> > > Another thing, shouldn't it be:
> > > if (rte_pktmbuf_data_len(m) < inner_l3_offset + m->l3_len + m->l4_len=
)
> ?
> >
> > Yes, it should.
> >
> > > Konstantin
> > >
> >
> > Tomasz