From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 32A862B93 for ; 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" To: "Kulasek, TomaszX" , "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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 ; 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 ; 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 > > > +#include > > > +#include > > > +#include > > > + > > > +/** > > > + * 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