From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 6423729CA for ; Thu, 29 Jun 2017 03:18:27 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2017 18:18:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,278,1496127600"; d="scan'208";a="118662769" Received: from dpdk15.sh.intel.com ([10.67.111.77]) by orsmga005.jf.intel.com with ESMTP; 28 Jun 2017 18:18:24 -0700 Date: Thu, 29 Jun 2017 09:19:50 +0800 From: Jiayu Hu To: "Ananyev, Konstantin" Cc: "dev@dpdk.org" , "Tan, Jianfeng" , "stephen@networkplumber.org" , "yliu@fridaylinux.org" , "Wu, Jingjing" , "Yao, Lei A" , "Wiles, Keith" , "Bie, Tiwei" Message-ID: <20170629011950.GA123467@dpdk15.sh.intel.com> References: <1498229000-94867-1-git-send-email-jiayu.hu@intel.com> <1498459430-116048-1-git-send-email-jiayu.hu@intel.com> <1498459430-116048-2-git-send-email-jiayu.hu@intel.com> <2601191342CEEE43887BDE71AB9772583FB0F4DA@IRSMSX109.ger.corp.intel.com> <20170628021710.GA4686@jiayu-Has> <2601191342CEEE43887BDE71AB9772583FB0FC29@IRSMSX109.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB9772583FB0FC29@IRSMSX109.ger.corp.intel.com> User-Agent: Mutt/1.7.1 (2016-10-04) Subject: Re: [dpdk-dev] [PATCH v7 1/3] lib: add Generic Receive Offload API framework 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: Thu, 29 Jun 2017 01:18:29 -0000 Hi Konstantin, On Thu, Jun 29, 2017 at 01:41:40AM +0800, Ananyev, Konstantin wrote: > > Hi Jiayu, > > > > > > > > > > + > > > > +/** > > > > + * GRO table, which is used to merge packets. It keeps many reassembly > > > > + * tables of desired GRO types. Applications need to create GRO tables > > > > + * before using rte_gro_reassemble to perform GRO. > > > > + */ > > > > +struct rte_gro_tbl { > > > > + uint64_t desired_gro_types; /**< GRO types to perform */ > > > > + /* max TTL measured in nanosecond */ > > > > + uint64_t max_timeout_cycles; > > > > + /* max length of merged packet measured in byte */ > > > > + uint32_t max_packet_size; > > > > + /* reassebly tables of desired GRO types */ > > > > + void *tbls[GRO_TYPE_MAX_NUM]; > > > > +}; > > > > > > Not sure why do you need to define that structure here. > > > As I understand it is internal to the library. > > > Just declaration should be enough. > > > > This structure defines a GRO table, which is used by rte_gro_reassemble > > to merge packets. Applications need to create this table before calling > > rte_gro_reassemble. So I define it in rte_gro.h. > > Yes, application has to call gro_table_create(). > But application don't need to access contents of struct rte_gro_tbl, > which means at it can (and should) treat it as opaque pointer. Thanks, I will modify it. > > > > > + > > > > +/** > > > > + * Reassembly function, which tries to merge the inputted packet with > > > > + * one packet in a given GRO table. This function assumes the inputted > > > > + * packet is with correct checksums. And it won't update checksums if > > > > + * two packets are merged. Besides, if the inputted packet is IP > > > > + * fragmented, this function assumes it's a complete packet (i.e. with > > > > + * L4 header). > > > > + * > > > > + * If the inputted packet doesn't have data or it's with unsupported GRO > > > > + * type, function returns immediately. Otherwise, the inputted packet is > > > > + * either merged or inserted into the table. If applications want get > > > > + * packets in the table, they need to call flush APIs. > > > > + * > > > > + * @param pkt > > > > + * packet to reassemble. > > > > + * @param gro_tbl > > > > + * a pointer points to a GRO table. > > > > + * @return > > > > + * if merge the packet successfully, return a positive value. If fail > > > > + * to merge, return zero. If the packet doesn't have data, or its GRO > > > > + * type is unsupported, return a negative value. > > > > + */ > > > > +int rte_gro_reassemble(struct rte_mbuf *pkt, > > > > + struct rte_gro_tbl *gro_tbl); > > > > > > > > > Ok, and why tbl one can't do bursts? > > > > In current design, if applications want to do bursts, they don't need to > > create gro_tbl. rte_gro_reassemble_burst will create a temporary table > > in stack. So when do bursts (we call it lightweight mode), the operations > > of applications is very simple: calling rte_gro_reassemble_burst. And > > after rte_gro_reassemble_burst returns, applications can get all merged > > packets. rte_gro_reassemble is another mode API, called heavyweight mode. > > The gro_tbl is just used in rte_gro_reassemble. rte_gro_reassemble just > > processes one packet at a time. > > > > So you mean: we should enable rte_gro_reassemble to merge N inputted > > packets with the packets in a given gro_tbl? > > Yes, I suppose that will be faster. Thanks, I will enable it to process N packets at a time. > > > > > > > > > > > > > + > > > > +/** > > > > + * This function flushed packets from reassembly tables of desired GRO > > > > + * types. It won't re-calculate checksums for merged packets in the > > > > + * tables. That is, the returned packets may be with wrong checksums. > > > > + * > > > > + * @param gro_tbl > > > > + * a pointer points to a GRO table object. > > > > + * @param desired_gro_types > > > > + * GRO types whose packets will be flushed. > > > > + * @param out > > > > + * a pointer array that is used to keep flushed packets. > > > > + * @param nb_out > > > > + * the size of out. > > > > + * @return > > > > + * the number of flushed packets. If no packets are flushed, return 0. > > > > + */ > > > > +uint16_t rte_gro_flush(struct rte_gro_tbl *gro_tbl, > > > > + uint64_t desired_gro_types, > > > > + struct rte_mbuf **out, > > > > + const uint16_t max_nb_out); > > > > + > > > > +/** > > > > + * This function flushes the timeout packets from reassembly tables of > > > > + * desired GRO types. It won't re-calculate checksums for merged packets > > > > + * in the tables. That is, the returned packets may be with wrong > > > > + * checksums. > > > > + * > > > > + * @param gro_tbl > > > > + * a pointer points to a GRO table object. > > > > + * @param desired_gro_types > > > > + * rte_gro_timeout_flush only processes packets which belong to the > > > > + * GRO types specified by desired_gro_types. > > > > + * @param out > > > > + * a pointer array that is used to keep flushed packets. > > > > + * @param nb_out > > > > + * the size of out. > > > > + * @return > > > > + * the number of flushed packets. If no packets are flushed, return 0. > > > > + */ > > > > +uint16_t rte_gro_timeout_flush(struct rte_gro_tbl *gro_tbl, > > > > + uint64_t desired_gro_types, > > > > + struct rte_mbuf **out, > > > > + const uint16_t max_nb_out); > > > > > > No point to have 2 flush() functions. > > > I suggest to merge them together. > > > > rte_gro_flush flush all packets from table, but rte_gro_timeout_flush only > > flush timeout packets. They have different operations. But if we merge them > > together, we need to flush all or only timeout ones? > > We can specify that if timeout is zero (or less then current time) then > we flush all packets. Thanks, I will merge them together.