From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiayu Hu Subject: Re: [PATCH v3 2/5] gso: add TCP/IPv4 GSO support Date: Wed, 13 Sep 2017 18:44:07 +0800 Message-ID: <20170913104407.GA57844@dpdk15.sh.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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "dev@dpdk.org" , "Kavanagh, Mark B" , "Tan, Jianfeng" To: "Ananyev, Konstantin" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 28BA47CFD for ; Wed, 13 Sep 2017 12:41:19 +0200 (CEST) Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB9772584F249FE8@irsmsx105.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Konstantin, On Tue, Sep 12, 2017 at 10:17:27PM +0800, Ananyev, Konstantin wrote: > > > > -----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 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 = 1; > > > + > > > if (pkt == NULL || pkts_out == NULL || nb_pkts_out < 1) > > > return -EINVAL; > > > > > > - pkts_out[0] = pkt; > > > + if (gso_ctx.gso_size >= pkt->pkt_len || > > > + (pkt->packet_type & gso_ctx.gso_types) != > > > + pkt->packet_type) { > > > + pkts_out[0] = pkt; > > > + return ret; > > > + } > > > + > > > + direct_pool = gso_ctx.direct_pool; > > > + indirect_pool = gso_ctx.indirect_pool; > > > + gso_size = gso_ctx.gso_size; > > > + ipid_delta = gso_ctx.ipid_flag == 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) != 0) {... > > Sorry, actually it probably should be: > If (pkt->ol_flags & (PKT_TX_TCP_SEG | PKT_TX_IPV4) == PKT_TX_IPV4 && > (gso_ctx->gso_types & DEV_TX_OFFLOAD_TCP_TSO) != 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 TSO capability before they call the GSO library. Do I misundertsand anything? Additionally, we don't need to check if the packet is a TCP/IPv4 packet here? Thanks, Jiayu > > Konstantin > > > > > > + ret = gso_tcp4_segment(pkt, gso_size, ipid_delta, > > > + direct_pool, indirect_pool, > > > + pkts_out, nb_pkts_out); > > > + } else > > > + RTE_LOG(WARNING, GSO, "Unsupported packet type\n"); > > > > Shouldn't we do pkt_out[0] = pkt; here? > > > > > + > > > + if (ret > 1) { > > > + pkt_seg = pkt; > > > + while (pkt_seg) { > > > + rte_mbuf_refcnt_update(pkt_seg, -1); > > > + pkt_seg = pkt_seg->next; > > > + } > > > + } > > > > > > - return 1; > > > + return ret; > > > } > > > -- > > > 2.7.4