From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
by dpdk.org (Postfix) with ESMTP id 32A862B93
for <dev@dpdk.org>; Thu, 29 Sep 2016 15:57:41 +0200 (CEST)
Received: from orsmga002.jf.intel.com ([10.7.209.21])
by fmsmga103.fm.intel.com with ESMTP; 29 Sep 2016 06:57:39 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,415,1470726000"; d="scan'208";a="1058284044"
Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25])
by orsmga002.jf.intel.com with ESMTP; 29 Sep 2016 06:57:26 -0700
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.196]) by
irsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id 14.03.0248.002;
Thu, 29 Sep 2016 14:57:23 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Kulasek, TomaszX" <tomaszx.kulasek@intel.com>, "dev@dpdk.org"
<dev@dpdk.org>
Thread-Topic: [PATCH v3 1/6] ethdev: add Tx preparation
Thread-Index: AQHSGXlhCES3CNrMHk+9VvxQQTGWNKCQRZ4QgAAaoYCAAB8gcA==
Date: Thu, 29 Sep 2016 13:57:22 +0000
Message-ID: <2601191342CEEE43887BDE71AB9772583F0BC720@irsmsx105.ger.corp.intel.com>
References: <1473691487-10032-1-git-send-email-tomaszx.kulasek@intel.com>
<20160928111052.9968-1-tomaszx.kulasek@intel.com>
<20160928111052.9968-2-tomaszx.kulasek@intel.com>
<2601191342CEEE43887BDE71AB9772583F0BC5E0@irsmsx105.ger.corp.intel.com>
<3042915272161B4EB253DA4D77EB373A14F33C87@IRSMSX102.ger.corp.intel.com>
In-Reply-To: <3042915272161B4EB253DA4D77EB373A14F33C87@IRSMSX102.ger.corp.intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [163.33.239.180]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v3 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: Thu, 29 Sep 2016 13:57:41 -0000
> -----Original Message-----
> From: Kulasek, TomaszX
> Sent: Thursday, September 29, 2016 2:04 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v3 1/6] ethdev: add Tx preparation
>=20
> Hi Konstantin,
>=20
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Thursday, September 29, 2016 12:41
> > To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> > Subject: RE: [PATCH v3 1/6] ethdev: add Tx preparation
> >
> > Hi Tomasz,
> >
> > ....
> >
> > > diff --git a/lib/librte_net/rte_pkt.h b/lib/librte_net/rte_pkt.h new =
file
> > mode 100644 index 0000000..72903ac
> > > --- /dev/null
> > > +++ b/lib/librte_net/rte_pkt.h
> > > @@ -0,0 +1,133 @@
> > > +/*-
> > > + * BSD LICENSE
> > > + *
> > > + * Copyright(c) 2016 Intel Corporation. All rights reserved.
> > > + * All rights reserved.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or with=
out
> > > + * modification, are permitted provided that the following conditi=
ons
> > > + * are met:
> > > + *
> > > + * * Redistributions of source code must retain the above copyri=
ght
> > > + * notice, this list of conditions and the following disclaime=
r.
> > > + * * Redistributions in binary form must reproduce the above cop=
yright
> > > + * notice, this list of conditions and the following disclaime=
r in
> > > + * the documentation and/or other materials provided with the
> > > + * distribution.
> > > + * * Neither the name of Intel Corporation nor the names of its
> > > + * contributors may be used to endorse or promote products der=
ived
> > > + * from this software without specific prior written permissio=
n.
> > > + *
> > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> > CONTRIBUTORS
> > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> > NOT
> > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > FITNESS FOR
> > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> > COPYRIGHT
> > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> > INCIDENTAL,
> > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> > BUT NOT
> > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> > LOSS OF USE,
> > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> > AND ON ANY
> > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> > TORT
> > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> > OF THE USE
> > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> > DAMAGE.
> > > + */
> > > +
> > > +#ifndef _RTE_PKT_H_
> > > +#define _RTE_PKT_H_
> > > +
> > > +#include <rte_ip.h>
> > > +#include <rte_udp.h>
> > > +#include <rte_tcp.h>
> > > +#include <rte_sctp.h>
> > > +
> > > +/**
> > > + * Validate general requirements for tx offload in packet.
> > > + */
> > > +static inline int
> > > +rte_validate_tx_offload(struct rte_mbuf *m) {
> > > + uint64_t ol_flags =3D m->ol_flags;
> > > +
> > > + /* Does packet set any of available offloads? */
> > > + if (!(ol_flags & PKT_TX_OFFLOAD_MASK))
> > > + return 0;
> > > +
> > > + /* IP checksum can be counted only for IPv4 packet */
> > > + if ((ol_flags & PKT_TX_IP_CKSUM) && (ol_flags & PKT_TX_IPV6))
> > > + return -EINVAL;
> > > +
> > > + if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))
> >
> > Not sure what you are trying to test here?
> > Is that PKT_TX_TCP_SEG is set?
> > If so, then the test condition doesn't look correct to me.
> >
> > > + /* IP type not set */
> > > + if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
> > > + return -EINVAL;
> > > +
>=20
> if (ol_flags & (PKT_TX_L4_MASK | PKT_TX_TCP_SEG))
> /* IP type not set */
> if (!(ol_flags & (PKT_TX_IPV4 | PKT_TX_IPV6)))
> return -EINVAL;
>=20
> For L4 checksums (L4_MASK =3D=3D TCP_CSUM|UDP_CSUM|SCTP_CSUM) as well as =
for TCP_SEG, the flag PKT_TX_IPV4 or PKT_TX_IPV6
> must be set. L4_MASK doesn't include TCP_SEG bit, so I added it to have o=
ne condition for all these cases (check if IPV4/6 flag is set
> when required).
>=20
> More detailed check, only for TCP_SEG is below (tso_segsz and IP_CSUM fla=
g for IPV4):
>=20
> > > + if (ol_flags & PKT_TX_TCP_SEG)
> > > + /* PKT_TX_IP_CKSUM offload not set for IPv4 TSO packet */
> > > + if ((m->tso_segsz =3D=3D 0) ||
> > > + ((ol_flags & PKT_TX_IPV4) && !(ol_flags &
> > PKT_TX_IP_CKSUM)))
> > > + return -EINVAL;
> > > +
> >
> > Why not just:
> > If ((ol_flags & PKT_TX_L4_MASK) =3D=3D PKT_TX_TCP_SEG) {
>=20
> PKT_TX_L4_MASK doesn't include PKT_TX_TCP_SEG, so it will always be false=
.
Ah yes, your right.
By some reason I thought that it does.
Looks good to me then and sorry for the noise.
Konstantin
>=20
> >
> > uint64_t f =3D ol_flags & PKT_TX_L4_MASK;
> >
> > if ((f & (PKT_TX_IPV4 | PKT_TX_IPV6)) =3D=3D 0 || f =3D=3D PK=
T_TX_IPV4 || m-
> > >tso_segsz =3D=3D 0)
> > return -EINVAL;
> > }
> >
> > Instead of 2 ifs around TCP_SEG above?
> >
> > Konstantin
> >
>=20
> Tomasz