From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kavanagh, Mark B" Subject: Re: [PATCH v6 2/6] gso: add TCP/IPv4 GSO support Date: Wed, 4 Oct 2017 14:59:29 +0000 Message-ID: References: <1506636833-25851-1-git-send-email-mark.b.kavanagh@intel.com> <1506962749-106779-1-git-send-email-mark.b.kavanagh@intel.com> <1506962749-106779-3-git-send-email-mark.b.kavanagh@intel.com> <2601191342CEEE43887BDE71AB9772585FAA3D5A@IRSMSX103.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772585FAA3E3A@IRSMSX103.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "Hu, Jiayu" , "Tan, Jianfeng" , "Yigit, Ferruh" , "thomas@monjalon.net" To: "Ananyev, Konstantin" , "dev@dpdk.org" Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id EED3A1B67E for ; Wed, 4 Oct 2017 17:00:37 +0200 (CEST) In-Reply-To: <2601191342CEEE43887BDE71AB9772585FAA3E3A@IRSMSX103.ger.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >From: Ananyev, Konstantin >Sent: Wednesday, October 4, 2017 3:49 PM >To: Kavanagh, Mark B ; dev@dpdk.org >Cc: Hu, Jiayu ; Tan, Jianfeng = ; >Yigit, Ferruh ; thomas@monjalon.net >Subject: RE: [PATCH v6 2/6] gso: add TCP/IPv4 GSO support > >> >> int >> >> rte_gso_segment(struct rte_mbuf *pkt, >> >> @@ -41,12 +46,53 @@ >> >> struct rte_mbuf **pkts_out, >> >> uint16_t nb_pkts_out) >> >> { >> >> + struct rte_mempool *direct_pool, *indirect_pool; >> >> + struct rte_mbuf *pkt_seg; >> >> + uint64_t ol_flags; >> >> + uint16_t gso_size; >> >> + uint8_t ipid_delta; >> >> + int ret =3D 1; >> >> + >> >> if (pkt =3D=3D NULL || pkts_out =3D=3D NULL || gso_ctx =3D=3D NULL = || >> >> nb_pkts_out < 1) >> >> return -EINVAL; >> >> >> >> - pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >> >> - pkts_out[0] =3D pkt; >> >> + if ((gso_ctx->gso_size >=3D pkt->pkt_len) || (gso_ctx->gso_types & >> >> + DEV_TX_OFFLOAD_TCP_TSO) !=3D >> >> + gso_ctx->gso_types) { >> >> + pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >> >> + pkts_out[0] =3D pkt; >> >> + return 1; >> >> + } >> >> + >> >> + 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 RTE_GSO_IPID_FIXED); >> >> + ol_flags =3D pkt->ol_flags; >> >> + >> >> + if (IS_IPV4_TCP(pkt->ol_flags)) { >> >> + pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >> >> + ret =3D gso_tcp4_segment(pkt, gso_size, ipid_delta, >> >> + direct_pool, indirect_pool, >> >> + pkts_out, nb_pkts_out); >> >> + } else { >> >> + pkt->ol_flags &=3D (~PKT_TX_TCP_SEG); >> > >> >Not sure why do you clean this flag if you don't support that packet ty= pe >> >and no action was perfomed? >> >Suppose you have a mix ipv4 and ipv6 packets - gso lib would do ipv4 an= d >> >someone else >> >(HW?) can do ipv4 segmentation. >> >> I can't say for definite, since I didn't implement this change. However,= I >can only presume that the assumption here is that since >> segmentation is being done in S/W that the underlying H/W does not suppo= rt >TSO. >> Since the underlying HW can't segment the packet in HW, we should clear = the >flag; otherwise, if an mbuf marked for TCP segmentation is >> passed to the driver of a NIC that does not support/understand that feat= ure, >the behavior is undefined. >> Is this a fair assumption in your opinion, or is it the case that the pa= cket >would simply be transmitted un-segmented in that case, and so we >> shouldn't clear the flag? > >Yes, I think if we shouldn't clear the flag if we didn't do any segmentati= on >(we just encounter a packet type that we don't support). >Konstantin Okay, thanks for clarifying - I'll update the code accordingly. -Mark > >> >> Thanks again, >> Mark >> >> >BTW, did you notice that building of shared target fails? >> >Konstantin >> >> I didn't, but I'll take a look right now - thanks for the catch! >> >> > >> > >> >> + pkts_out[0] =3D pkt; >> >> + RTE_LOG(WARNING, GSO, "Unsupported packet type\n"); >> >> + return 1; >> >> + } >> >> + >> >> + if (ret > 1) { >> >> + pkt_seg =3D pkt; >> >> + while (pkt_seg) { >> >> + rte_mbuf_refcnt_update(pkt_seg, -1); >> >> + pkt_seg =3D pkt_seg->next; >> >> + } >> >> + } else if (ret < 0) { >> >> + /* Revert the ol_flags in the event of failure. */ >> >> + pkt->ol_flags =3D ol_flags; >> >> + } >> >> >> >> - return 1; >> >> + return ret; >> >> } >> >> -- >> >> 1.9.3