From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Ananyev, Konstantin" Subject: Re: [PATCH v7 1/3] lib: add Generic Receive Offload API framework Date: Wed, 28 Jun 2017 17:41:40 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772583FB0FC29@IRSMSX109.ger.corp.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Tan, Jianfeng" , "stephen@networkplumber.org" , "yliu@fridaylinux.org" , "Wu, Jingjing" , "Yao, Lei A" , "Wiles, Keith" , "Bie, Tiwei" To: "Hu, Jiayu" Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 23CDD2C8 for ; Wed, 28 Jun 2017 19:41:44 +0200 (CEST) In-Reply-To: <20170628021710.GA4686@jiayu-Has> 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" Hi Jiayu, >=20 > > > > > + > > > +/** > > > + * GRO table, which is used to merge packets. It keeps many reassemb= ly > > > + * tables of desired GRO types. Applications need to create GRO tabl= es > > > + * 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. >=20 > 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. > > > + > > > +/** > > > + * Reassembly function, which tries to merge the inputted packet wit= h > > > + * one packet in a given GRO table. This function assumes the inputt= ed > > > + * packet is with correct checksums. And it won't update checksums i= f > > > + * two packets are merged. Besides, if the inputted packet is IP > > > + * fragmented, this function assumes it's a complete packet (i.e. wi= th > > > + * L4 header). > > > + * > > > + * If the inputted packet doesn't have data or it's with unsupported= GRO > > > + * type, function returns immediately. Otherwise, the inputted packe= t is > > > + * either merged or inserted into the table. If applications want ge= t > > > + * 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 fa= il > > > + * to merge, return zero. If the packet doesn't have data, or its G= RO > > > + * 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? >=20 > 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. >=20 > 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. >=20 > > > > > > > + > > > +/** > > > + * This function flushed packets from reassembly tables of desired G= RO > > > + * 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 pac= kets > > > + * 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. >=20 > rte_gro_flush flush all packets from table, but rte_gro_timeout_flush onl= y > flush timeout packets. They have different operations. But if we merge th= em > 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.