From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
by dpdk.org (Postfix) with ESMTP id 45FFB199B4
for <dev@dpdk.org>; Thu, 14 Sep 2017 10:47:03 +0200 (CEST)
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
by fmsmga105.fm.intel.com with ESMTP; 14 Sep 2017 01:47:02 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.42,391,1500966000"; d="scan'208";a="151203509"
Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23])
by fmsmga005.fm.intel.com with ESMTP; 14 Sep 2017 01:47:01 -0700
Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by
IRSMSX109.ger.corp.intel.com ([169.254.13.28]) with mapi id 14.03.0319.002;
Thu, 14 Sep 2017 09:47:00 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Hu, Jiayu" <jiayu.hu@intel.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, "Kavanagh, Mark B"
<mark.b.kavanagh@intel.com>, "Tan, Jianfeng" <jianfeng.tan@intel.com>
Thread-Topic: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support
Thread-Index: AQHTK3CNNYr96WyXIECiBIjbMlkDdaKxEVgwgAA6OTCAAUc9gIAAyrWAgAB6OYCAADvfkA==
Date: Thu, 14 Sep 2017 08:47:00 +0000
Message-ID: <2601191342CEEE43887BDE71AB9772584F24ADD2@irsmsx105.ger.corp.intel.com>
References: <1504598270-60080-1-git-send-email-jiayu.hu@intel.com>
<1505184211-36728-1-git-send-email-jiayu.hu@intel.com>
<1505184211-36728-3-git-send-email-jiayu.hu@intel.com>
<2601191342CEEE43887BDE71AB9772584F249FE8@irsmsx105.ger.corp.intel.com>
<20170913104407.GA57844@dpdk15.sh.intel.com>
<2601191342CEEE43887BDE71AB9772584F24AACB@irsmsx105.ger.corp.intel.com>
<20170914060705.GA60858@dpdk15.sh.intel.com>
In-Reply-To: <20170914060705.GA60858@dpdk15.sh.intel.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2VjMzkwNDctYWU2Ni00YTI0LThiYjItYWJiNjhjMmY5ZjJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IndGR2I4dDFiSjN3eFJnUlJweEh2Tm9YSDRrU215eDMzRXJVUXZTclJIMkU9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
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 2/5] gso: add TCP/IPv4 GSO support
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <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, 14 Sep 2017 08:47:04 -0000
Hi Jiayu,
> -----Original Message-----
> From: Hu, Jiayu
> Sent: Thursday, September 14, 2017 7:07 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Tan, Jian=
feng <jianfeng.tan@intel.com>
> Subject: Re: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support
>=20
> Hi Konstantin,
>=20
> On Thu, Sep 14, 2017 at 06:10:37AM +0800, Ananyev, Konstantin wrote:
> >
> > Hi Jiayu,
> >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ananyev, Konstantin
> > > > > Sent: Tuesday, September 12, 2017 12:18 PM
> > > > > To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org
> > > > > Cc: Kavanagh, Mark B <mark.b.kavanagh@intel.com>; Tan, Jianfeng <=
jianfeng.tan@intel.com>
> > > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support
> > > > >
> > > > > > result, when all of its GSOed segments are freed, the packet is=
freed
> > > > > > automatically.
> > > > > > diff --git a/lib/librte_gso/rte_gso.c b/lib/librte_gso/rte_gso.=
c
> > > > > > index dda50ee..95f6ea6 100644
> > > > > > --- a/lib/librte_gso/rte_gso.c
> > > > > > +++ b/lib/librte_gso/rte_gso.c
> > > > > > @@ -33,18 +33,53 @@
> > > > > >
> > > > > > #include <errno.h>
> > > > > >
> > > > > > +#include <rte_log.h>
> > > > > > +
> > > > > > #include "rte_gso.h"
> > > > > > +#include "gso_common.h"
> > > > > > +#include "gso_tcp4.h"
> > > > > >
> > > > > > int
> > > > > > rte_gso_segment(struct rte_mbuf *pkt,
> > > > > > - struct rte_gso_ctx gso_ctx __rte_unused,
> > > > > > + struct rte_gso_ctx gso_ctx,
> > > > > > struct rte_mbuf **pkts_out,
> > > > > > uint16_t nb_pkts_out)
> > > > > > {
> > > > > > + struct rte_mempool *direct_pool, *indirect_pool;
> > > > > > + struct rte_mbuf *pkt_seg;
> > > > > > + uint16_t gso_size;
> > > > > > + uint8_t ipid_delta;
> > > > > > + int ret =3D 1;
> > > > > > +
> > > > > > if (pkt =3D=3D NULL || pkts_out =3D=3D NULL || nb_pkts_out < =
1)
> > > > > > return -EINVAL;
> > > > > >
> > > > > > - pkts_out[0] =3D pkt;
> > > > > > + if (gso_ctx.gso_size >=3D pkt->pkt_len ||
> > > > > > + (pkt->packet_type & gso_ctx.gso_types) !=3D
> > > > > > + pkt->packet_type) {
> > > > > > + pkts_out[0] =3D pkt;
> > > > > > + return ret;
> > > > > > + }
> > > > > > +
> > > > > > + direct_pool =3D gso_ctx.direct_pool;
> > > > > > + indirect_pool =3D gso_ctx.indirect_pool;
> > > > > > + gso_size =3D gso_ctx.gso_size;
> > > > > > + ipid_delta =3D gso_ctx.ipid_flag =3D=3D RTE_GSO_IPID_INCREASE=
;
> > > > > > +
> > > > > > + if (is_ipv4_tcp(pkt->packet_type)) {
> > > > >
> > > > > Probably we need here:
> > > > > If (is_ipv4_tcp(pkt->packet_type) && (gso_ctx->gso_types & DEV_T=
X_OFFLOAD_TCP_TSO) !=3D 0) {...
> > > >
> > > > Sorry, actually it probably should be:
> > > > If (pkt->ol_flags & (PKT_TX_TCP_SEG | PKT_TX_IPV4) =3D=3D PKT_TX_IP=
V4 &&
> > > > (gso_ctx->gso_types & DEV_TX_OFFLOAD_TCP_TSO) !=3D 0) {...
> > >
> > > I don't quite understand why the GSO library should be aware if the T=
SO
> > > flag is set or not. Applications can query device TSO capability befo=
re
> > > they call the GSO library. Do I misundertsand anything?
> > >
> > > Additionally, we don't need to check if the packet is a TCP/IPv4 pack=
et here?
> >
> > Well, right now PMD we doesn't rely on ptype to figure out what type o=
f packet and
> > what TX offload have to be performed.
> > Instead it looks at TX part of ol_flags, and
> > My thought was that as what we doing is actually TSO in SW, it would be=
good
> > to use the same API here too.
> > Also with that approach, by setting ol_flags properly user can use the =
same gso_ctx and still
> > specify what segmentation to perform on a per-packet basis.
> >
> > Alternative way is to rely on ptype to distinguish should segmentation =
be performed on that package or not.
> > The only advantage I see here is that if someone would like to add GSO =
for some new protocol,
> > he wouldn't need to introduce new TX flag value for mbuf.ol_flags.
> > Though he still would need to update TX_OFFLOAD_* capabilities and prob=
ably packet_type definitions.
> >
> > So from my perspective first variant (use HW TSO API) is more plausible=
.
> > Wonder what is your and Mark opinions here?
>=20
> In the first choice, you mean:
> the GSO library uses gso_ctx->gso_types and mbuf->ol_flags to call a spec=
ific GSO
> segmentation function (e.g. gso_tcp4_segment(), gso_tunnel_xxx()) for eac=
h input packet.
> Applications should parse the packet type, and set an exactly correct DEV=
_TX_OFFLOAD_*_TSO
> flag to gso_types and ol_flags according to the packet type. That is, the=
value of gso_types
> is on a per-packet basis. Using gso_ctx->gso_types and mbuf->ol_flags at =
the same time
> is because that DEV_TX_OFFLOAD_*_TSO only tells tunnelling type and the i=
nner L4 type, and
> we need to know L3 type by ol_flags. With this design, HW segmentation an=
d SW segmentation
> are indeed consistent.
>=20
> If I understand it correctly, applications need to set 'ol_flags =3D PKT_=
TX_IPV4' and
> 'gso_types =3D DEV_TX_OFFLOAD_VXLAN_TNL_TSO' for a "ether+ipv4+udp+vxlan+=
ether+ipv4+
> tcp+payload" packet. But PKT_TX_IPV4 just present the inner L3 type for t=
unneled packet.
> How about the outer L3 type? Always assume the inner and the outer L3 typ=
e are the same?
It think that for that case you'll have to set in ol_flags:
PKT_TX_IPV4 | PKT_TX_OUTER_IPV4 | PKT_TX_TUNNEL_VXLAN | PKT_TX_TCP_SEG
Konstantin
>=20
> Jiayu
> > Konstantin