From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP; 28 Jun 2017 10:41:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,276,1496127600"; d="scan'208";a="872688824" Received: from irsmsx151.ger.corp.intel.com ([163.33.192.59]) by FMSMGA003.fm.intel.com with ESMTP; 28 Jun 2017 10:41:42 -0700 Received: from irsmsx112.ger.corp.intel.com (10.108.20.5) by IRSMSX151.ger.corp.intel.com (163.33.192.59) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 28 Jun 2017 18:41:41 +0100 Received: from irsmsx109.ger.corp.intel.com ([169.254.13.250]) by irsmsx112.ger.corp.intel.com ([169.254.1.42]) with mapi id 14.03.0319.002; Wed, 28 Jun 2017 18:41:41 +0100 From: "Ananyev, Konstantin" To: "Hu, Jiayu" CC: "dev@dpdk.org" , "Tan, Jianfeng" , "stephen@networkplumber.org" , "yliu@fridaylinux.org" , "Wu, Jingjing" , "Yao, Lei A" , "Wiles, Keith" , "Bie, Tiwei" Thread-Topic: [PATCH v7 1/3] lib: add Generic Receive Offload API framework Thread-Index: AQHS7kdhWIOXTzfKpkaGxL5+jW80/qI5XidwgAAd0gCAAQ82QA== 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> In-Reply-To: <20170628021710.GA4686@jiayu-Has> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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: Wed, 28 Jun 2017 17:41:45 -0000 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.