From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tomaszx.kulasek@intel.com>
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
 by dpdk.org (Postfix) with ESMTP id 1A1CA2B88
 for <dev@dpdk.org>; Thu, 29 Sep 2016 17:12:31 +0200 (CEST)
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
 by orsmga104.jf.intel.com with ESMTP; 29 Sep 2016 08:12:31 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.30,415,1470726000"; d="scan'208";a="767220874"
Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31])
 by FMSMGA003.fm.intel.com with ESMTP; 29 Sep 2016 08:12:30 -0700
Received: from irsmsx102.ger.corp.intel.com ([169.254.2.198]) by
 IRSMSX106.ger.corp.intel.com ([169.254.8.209]) with mapi id 14.03.0248.002;
 Thu, 29 Sep 2016 16:12:28 +0100
From: "Kulasek, TomaszX" <tomaszx.kulasek@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
Thread-Topic: [PATCH v3 5/6] ixgbe: add Tx preparation
Thread-Index: AQHSGXlVRzocZ84inkKqOR0RsDxzqqCQQBSAgAA2+xA=
Date: Thu, 29 Sep 2016 15:12:28 +0000
Message-ID: <3042915272161B4EB253DA4D77EB373A14F33D37@IRSMSX102.ger.corp.intel.com>
References: <1473691487-10032-1-git-send-email-tomaszx.kulasek@intel.com>
 <20160928111052.9968-1-tomaszx.kulasek@intel.com>
 <20160928111052.9968-6-tomaszx.kulasek@intel.com>
 <2601191342CEEE43887BDE71AB9772583F0BC604@irsmsx105.ger.corp.intel.com>
In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0BC604@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 v3 5/6] ixgbe: 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 15:12:32 -0000

Hi Konstantin,

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, September 29, 2016 13:09
> To: Kulasek, TomaszX <tomaszx.kulasek@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH v3 5/6] ixgbe: add Tx preparation
>=20
> Hi Tomasz,
>=20
> > Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
> > ---

...

> > +*/
> > +uint16_t
> > +ixgbe_prep_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t
> > +nb_pkts) {
> > +	int i, ret;
> > +	struct rte_mbuf *m;
> > +	struct ixgbe_tx_queue *txq =3D (struct ixgbe_tx_queue *)tx_queue;
> > +
> > +	for (i =3D 0; i < nb_pkts; i++) {
> > +		m =3D tx_pkts[i];
> > +
> > +		/**
> > +		 * Check if packet meets requirements for number of
> segments
> > +		 *
> > +		 * NOTE: for ixgbe it's always (40 - WTHRESH) for both TSO
> and non-TSO
> > +		 */
> > +
> > +		if (m->nb_segs > IXGBE_TX_MAX_SEG - txq->wthresh) {
> > +			rte_errno =3D -EINVAL;
> > +			return i;
> > +		}
> > +
> > +		if (m->ol_flags & IXGBE_TX_OFFLOAD_NOTSUP_MASK) {
> > +			rte_errno =3D -EINVAL;
> > +			return i;
> > +		}
> > +
> > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +		ret =3D rte_validate_tx_offload(m);
> > +		if (ret !=3D 0) {
> > +			rte_errno =3D ret;
> > +			return i;
> > +		}
> > +#endif
> > +		ret =3D rte_phdr_cksum_fix(m);
> > +		if (ret !=3D 0) {
> > +			rte_errno =3D ret;
> > +			return i;
> > +		}
> > +	}
> > +
> > +	return i;
> > +}
> > +
> > +/* ixgbe simple path as well as vector TX doesn't support tx offloads
> > +*/ uint16_t ixgbe_prep_pkts_simple(void *tx_queue __rte_unused,
> > +struct rte_mbuf **tx_pkts,
> > +		uint16_t nb_pkts)
> > +{
> > +	int i;
> > +	struct rte_mbuf *m;
> > +	uint64_t ol_flags;
> > +
> > +	for (i =3D 0; i < nb_pkts; i++) {
> > +		m =3D tx_pkts[i];
> > +		ol_flags =3D m->ol_flags;
> > +
> > +		/* simple tx path doesn't support multi-segments */
> > +		if (m->nb_segs !=3D 1) {
> > +			rte_errno =3D -EINVAL;
> > +			return i;
> > +		}
> > +
> > +		/* For simple path (simple and vector) no tx offloads are
> supported */
> > +		if (ol_flags & PKT_TX_OFFLOAD_MASK) {
> > +			rte_errno =3D -EINVAL;
> > +			return i;
> > +		}
> > +	}
> > +
> > +	return i;
> > +}
>=20
> Just thought about it once again:
> As now inside rte_eth_tx_prep() we do now:
> +
> +	if (!dev->tx_pkt_prep)
> +		return nb_pkts;
>=20
> Then there might be a better approach to set
> dev->tx_pkt_prep =3D NULL
> for simple and vector TX functions?
>=20
> After all, prep_simple() does nothing but returns an error if conditions =
are
> not met.
> And if simple TX was already selected, then that means that user delibera=
tely
> disabled all HW TX offloads in favor of faster TX and there is no point t=
o slow
> him down with extra checks here.
> Same for i40e and fm10k.
> What is your opinion?
>=20
> Konstantin
>=20

Yes, if performance is a key, and, while the limitations of vector/simple p=
ath are quite well documented, these additional checks are a bit overzealou=
s. We may assume that to made tx offloads working, we need to configure dri=
ver in a right way, and this is a configuration issue if something doesn't =
work.

I will remove it.

Tomasz