From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 058252BBE for ; Fri, 15 Sep 2017 10:38:29 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP; 15 Sep 2017 01:38:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,396,1500966000"; d="scan'208";a="149502348" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga005.jf.intel.com with ESMTP; 15 Sep 2017 01:38:27 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 15 Sep 2017 01:38:24 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 15 Sep 2017 01:38:23 -0700 Received: from shsmsx151.ccr.corp.intel.com ([169.254.3.98]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.219]) with mapi id 14.03.0319.002; Fri, 15 Sep 2017 16:38:21 +0800 From: "Hu, Jiayu" To: "Ananyev, Konstantin" , "Kavanagh, Mark B" CC: "dev@dpdk.org" , "Tan, Jianfeng" Thread-Topic: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support Thread-Index: AQHTLTYKKvGbzhXn4Uirw+Dd+ZTvGqKxEVgwgAA6OTCAAUc9gIAAyrWAgAB6OYCAADvfkP///JqAgAARR4D///eegIAAX0SAgABB4iCAAM3fAIAAFiXggAAAoID///5zUA== Date: Fri, 15 Sep 2017 08:38:20 +0000 Message-ID: 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> <2601191342CEEE43887BDE71AB9772584F24ADD2@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772584F24AE4D@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772584F24B081@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772584F24B2E9@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772584F24B2E9@irsmsx105.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiM2VjMzkwNDctYWU2Ni00YTI0LThiYjItYWJiNjhjMmY5ZjJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IndGR2I4dDFiSjN3eFJnUlJweEh2Tm9YSDRrU215eDMzRXJVUXZTclJIMkU9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] 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: Fri, 15 Sep 2017 08:38:31 -0000 > -----Original Message----- > From: Ananyev, Konstantin > Sent: Friday, September 15, 2017 4:17 PM > To: Hu, Jiayu ; Kavanagh, Mark B > > Cc: dev@dpdk.org; Tan, Jianfeng > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support >=20 >=20 >=20 > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Friday, September 15, 2017 9:16 AM > > To: Hu, Jiayu ; Kavanagh, Mark B > > > Cc: dev@dpdk.org; Tan, Jianfeng > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > Hi Jiayu, > > > > > -----Original Message----- > > > From: Hu, Jiayu > > > Sent: Friday, September 15, 2017 8:55 AM > > > To: Ananyev, Konstantin ; Kavanagh, > Mark B > > > Cc: dev@dpdk.org; Tan, Jianfeng > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > > Hi Konstantin, > > > > > > > -----Original Message----- > > > > From: Ananyev, Konstantin > > > > Sent: Friday, September 15, 2017 2:39 AM > > > > To: Kavanagh, Mark B ; Hu, Jiayu > > > > > > > > Cc: dev@dpdk.org; Tan, Jianfeng > > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Kavanagh, Mark B > > > > > Sent: Thursday, September 14, 2017 4:42 PM > > > > > To: Hu, Jiayu ; Ananyev, Konstantin > > > > > > > > > Cc: dev@dpdk.org; Tan, Jianfeng > > > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > > > > > > >From: Hu, Jiayu > > > > > >Sent: Thursday, September 14, 2017 11:01 AM > > > > > >To: Ananyev, Konstantin ; > Kavanagh, > > > > Mark B > > > > > > > > > > > >Cc: dev@dpdk.org; Tan, Jianfeng > > > > > >Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > > > > > > > >Hi Konstantin and Mark, > > > > > > > > > > > >> -----Original Message----- > > > > > >> From: Ananyev, Konstantin > > > > > >> Sent: Thursday, September 14, 2017 5:36 PM > > > > > >> To: Hu, Jiayu > > > > > >> Cc: dev@dpdk.org; Kavanagh, Mark B > ; > > > > Tan, > > > > > >> Jianfeng > > > > > >> Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > >> > > > > > >> > > > > > >> > > > > > >> > -----Original Message----- > > > > > >> > From: Hu, Jiayu > > > > > >> > Sent: Thursday, September 14, 2017 10:29 AM > > > > > >> > To: Ananyev, Konstantin > > > > > >> > Cc: dev@dpdk.org; Kavanagh, Mark B > ; > > > > Tan, > > > > > >> Jianfeng > > > > > >> > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > >> > > > > > > >> > Hi Konstantin, > > > > > >> > > > > > > >> > > -----Original Message----- > > > > > >> > > From: Ananyev, Konstantin > > > > > >> > > Sent: Thursday, September 14, 2017 4:47 PM > > > > > >> > > To: Hu, Jiayu > > > > > >> > > Cc: dev@dpdk.org; Kavanagh, Mark B > > > > ; > > > > > >> Tan, > > > > > >> > > Jianfeng > > > > > >> > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support > > > > > >> > > > > > > > >> > > Hi Jiayu, > > > > > >> > > > > > > > >> > > > -----Original Message----- > > > > > >> > > > From: Hu, Jiayu > > > > > >> > > > Sent: Thursday, September 14, 2017 7:07 AM > > > > > >> > > > To: Ananyev, Konstantin > > > > > >> > > > Cc: dev@dpdk.org; Kavanagh, Mark B > > > > ; > > > > > >> Tan, > > > > > >> > > Jianfeng > > > > > >> > > > Subject: Re: [PATCH v3 2/5] gso: add TCP/IPv4 GSO suppor= t > > > > > >> > > > > > > > > >> > > > Hi Konstantin, > > > > > >> > > > > > > > > >> > > > 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 ; dev@dpdk.org > > > > > >> > > > > > > > Cc: Kavanagh, Mark B = ; > Tan, > > > > > >> Jianfeng > > > > > >> > > > > > > > >> > > > > > > > Subject: RE: [PATCH v3 2/5] gso: add TCP/IPv4 GS= O > support > > > > > >> > > > > > > > > > > > > >> > > > > > > > > result, when all of its GSOed segments are fre= ed, 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 > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > +#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_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_IPV4 > > > > > >> > > && > > > > > >> > > > > > > (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 > > > > > >> TSO > > > > > >> > > > > > flag is set or not. Applications can query device TS= O > capability > > > > > >> before > > > > > >> > > > > > they call the GSO library. Do I misundertsand anythi= ng? > > > > > >> > > > > > > > > > > >> > > > > > Additionally, we don't need to check if the packet i= s a > TCP/IPv4 > > > > > >> packet > > > > > >> > > here? > > > > > >> > > > > > > > > > >> > > > > Well, right now PMD we doesn't rely on ptype to figur= e out > what > > > > > >type > > > > > >> of > > > > > >> > > 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 i= n 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 sho= uld > > > > > >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 > > > > > >> > > probably packet_type definitions. > > > > > >> > > > > > > > > > >> > > > > So from my perspective first variant (use HW TSO API) = is > more > > > > > >> plausible. > > > > > >> > > > > Wonder what is your and Mark opinions here? > > > > > >> > > > > > > > > >> > > > In the first choice, you mean: > > > > > >> > > > the GSO library uses gso_ctx->gso_types and mbuf->ol_fla= gs > to call > > > > a > > > > > >> > > specific GSO > > > > > >> > > > segmentation function (e.g. gso_tcp4_segment(), > gso_tunnel_xxx()) > > > > for > > > > > >> > > each input packet. > > > > > >> > > > Applications should parse the packet type, and set an ex= actly > > > > correct > > > > > >> > > DEV_TX_OFFLOAD_*_TSO > > > > > >> > > > flag to gso_types and ol_flags according to the packet t= ype. > That is, > > > > > >the > > > > > >> > > value of gso_types > > > > > >> > > > is on a per-packet basis. Using gso_ctx->gso_types and m= buf- > > > > >ol_flags > > > > > >> at > > > > > >> > > the same time > > > > > >> > > > is because that DEV_TX_OFFLOAD_*_TSO only tells tunnelli= ng > type > > > > and > > > > > >> the > > > > > >> > > inner L4 type, and > > > > > >> > > > we need to know L3 type by ol_flags. With this design, H= W > > > > > >> segmentation > > > > > >> > > and SW segmentation > > > > > >> > > > are indeed consistent. > > > > > >> > > > > > > > > >> > > > 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 in= ner > L3 > > > > type > > > > > >for > > > > > >> > > tunneled packet. > > > > > >> > > > How about the outer L3 type? Always assume the inner and > the > > > > outer L3 > > > > > >> > > type 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 > > > > > >> > > > > > > >> > OK, so it means PKT_TX_TCP_SEG is also used for tunneled TSO= . > The > > > > > >> > GSO library doesn't need gso_types anymore. > > > > > >> > > > > > >> You still might need gso_ctx.gso_types to let user limit what = types > of > > > > > >> segmentation > > > > > >> that particular gso_ctx supports. > > > > > >> An alternative would be to assume that each gso_ctx supports a= ll > > > > > >> currently implemented segmentations. > > > > > >> This is possible too, but probably not very convenient to the = user. > > > > > > > > > > > >Hmm, make sense. > > > > > > > > > > > >One thing to confirm: the value of gso_types should be > > > > DEV_TX_OFFLOAD_*_TSO, > > > > > >or new macros? > > > > > > > > > > Hi Jiayu, Konstantin, > > > > > > > > > > I think that the existing macros are fine, as they provide a cons= istent > view > > > > of segmentation capabilities to the application/user. > > > > > > > > +1 > > > > I also think it is better to re-use DEV_TX_OFFLOAD_*_TSO. > > > > > > There might be an 'issue', if we use 'PKT_TX_TCP_SEG' to tell the > > > GSO library to segment a packet or not. Given the scenario that > > > an application only wants to do GSO and doesn't want to use TSO. > > > The application sets 'mbuf->ol_flags=3DPKT_TX_TCP_SEG' and doesn't > > > set mbuf->tso_segsz. Then the GSO library segments the packet, and > > > all output GSO segments have the same ol_flags as the input packet > > > (in current GSO library design). Then the output GSO segments are > > > transmitted to rte_eth_tx_prepare(). If the NIC is i40e, its TX prepa= re > function, > > > i40e_prep_pkts, checks if mbuf->tso_segsz is in the range of > I40E_MIN_TSO_MSS > > > and I40E_MAX_TSO_MSS, when PKT_TX_TCP_SEG is set. So an error > happens in > > > this scenario, since tso_segsz is 0. > > > > > > In fact, it may confuse the PMD driver when set PKT_TX_TCP_SEG but > don't want > > > to do TSO. One solution is that the GSO library removes the > PKT_TX_TCP_SEG flag > > > for all GSO segments after finishes segmenting. > > > > Yes, that was my thought too: after successful segmentation we probably > > need to cleanup related ol_flags. >=20 > In fact, we just don't need to set these flags in our newly created segme= nts. +1. PKT_TX_TCP_SEG is not needed, but others, like PKT_TX_IPV4, should be kept, since they may also be used by other HW offloadings, like csum. Thanks, Jiayu >=20 > > Konstantin > > > > > Wonder you and Mark's opinion. > > > > > > Thanks, > > > Jiayu > > > > > > > > > > > > > > I was initially concerned that they might be too coarse-grained (= i.e. > only > > > > IPv4 is currently supported, and not IPv6), but as per Konstantin's > > > > > previous example, the DEV_TX_OFFLOAD_*_TSO macros can be used > in > > > > concert with the packet type to determine whether a packet should > > > > > be fragmented or not. > > > > > > > > > > Thanks, > > > > > Mark > > > > > > > > > > > > > > > > >Jiayu > > > > > >> Konstantin > > > > > >> > > > > > >> > > > > > > >> > The first choice makes HW and SW segmentation are totally th= e > same. > > > > > >> > Applications just need to parse the packet and set proper ol= _flags, > and > > > > > >> > the GSO library uses ol_flags to decide which segmentation > function to > > > > > >use. > > > > > >> > I think it's better than the second choice which depending o= n > ptype to > > > > > >> > choose segmentation function. > > > > > >> > > > > > > >> > Jiayu > > > > > >> > > > > > > > >> > > Konstantin > > > > > >> > > > > > > > >> > > > > > > > > >> > > > Jiayu > > > > > >> > > > > Konstantin