From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 6797C1B199 for ; Thu, 14 Sep 2017 00:10:48 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2017 15:10:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,389,1500966000"; d="scan'208";a="151557270" Received: from irsmsx152.ger.corp.intel.com ([163.33.192.66]) by fmsmga006.fm.intel.com with ESMTP; 13 Sep 2017 15:10:38 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.75]) by IRSMSX152.ger.corp.intel.com ([169.254.6.87]) with mapi id 14.03.0319.002; Wed, 13 Sep 2017 23:10:38 +0100 From: "Ananyev, Konstantin" To: "Hu, Jiayu" CC: "dev@dpdk.org" , "Kavanagh, Mark B" , "Tan, Jianfeng" Thread-Topic: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support Thread-Index: AQHTK3CNNYr96WyXIECiBIjbMlkDdaKxEVgwgAA6OTCAAUc9gIAAyrWA Date: Wed, 13 Sep 2017 22:10:37 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772584F24AACB@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> In-Reply-To: <20170913104407.GA57844@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: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTJhMDJkNjMtNTc3Ny00Y2JmLWJmMzYtYWFiNmQ0Nzk2MzliIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IlwveHJKNTNqQUZHVnpyY0U1ZDNPM2hldmpOeFZob0pxNVJtdUI4R2VFMElZPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Sep 2017 22:10:49 -0000 Hi Jiayu, > > > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Tuesday, September 12, 2017 12:18 PM > > > To: Hu, Jiayu ; dev@dpdk.org > > > Cc: Kavanagh, Mark B ; Tan, Jianfeng > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > > > result, when all of its GSOed segments are freed, the packet is fre= ed > > > > 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 > > > > > > > > +#include > > > > + > > > > #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_TX_OF= FLOAD_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_IPV4 &= & > > (gso_ctx->gso_types & DEV_TX_OFFLOAD_TCP_TSO) !=3D 0) {... >=20 > I don't quite understand why the GSO library should be aware if the TSO > flag is set or not. Applications can query device TSO capability before > they call the GSO library. Do I misundertsand anything? >=20 > Additionally, we don't need to check if the packet is a TCP/IPv4 packet h= ere? Well, right now PMD we doesn't rely on ptype to figure out what type of pa= cket and what TX offload have to be performed. Instead it looks at TX part of ol_flags, and=20 My thought was that as what we doing is actually TSO in SW, it would be goo= d 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 p= erformed 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 probably= packet_type definitions. =20 So from my perspective first variant (use HW TSO API) is more plausible. Wonder what is your and Mark opinions here? Konstantin