On Thu, Mar 23, 2017 at 09:12:56PM +0800, Ananyev, Konstantin wrote: > Hi everyone, > > > > > > > > > > > -----Original Message----- > > > > From: Hu, Jiayu > > > > Sent: Thursday, March 23, 2017 6:25 AM > > > > To: Wiles, Keith <keith.wi...@intel.com> > > > > Cc: Yuanhan Liu <yuanhan....@linux.intel.com>; Richardson, Bruce > > > > <bruce.richard...@intel.com>; Yigit, Ferruh <ferruh.yi...@intel.com>; > > > > Ananyev, Konstantin <konstantin.anan...@intel.com> > > > > Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support > > > > > > > > On Thu, Mar 23, 2017 at 01:29:06PM +0800, Wiles, Keith wrote: > > > > > > > > > > > On Mar 22, 2017, at 9:15 PM, Hu, Jiayu <jiayu...@intel.com> wrote: > > > > > > > > > > > > On Wed, Mar 22, 2017 at 10:19:41PM +0800, Wiles, Keith wrote: > > > > > >> Off list. > > > > > >> > > > > > >> This support for GRO, seems it needs to be a feature for all > > > > > >> ethernet > > > > devices and some way the developer can enable this feature like the > > > > other > > > > offloads in DPDK. The GRO support should be set by the developer and > > > > then > > > > the apis are called within ethdev or the PMD to process the packets. The > > > > code is generic and creating a library is not the best overall solution > > > > IMO. > > > > > > > > > > > > Indeed, in this patchset, GRO is just proposed as a application-used > > > > library. > > > > > > But actually, these GRO APIs can also be used by ethdev and PMD. For > > > > > > example, register rte_gro_tcp4_reassemble_burst as a rx callback. > > > > > > Therefore, maybe we can support GRO in two ways. The one is a > > > > > > application-used library, the other is a device feature. > > > > > > Applications > > > > decide which one to use. > > > > > > > > > > > > How do you think of it? > > > > > > > > > > I would prefer to use it only in a offload design, meaning the GRO is > > > > just another ethernet offload the user can turn on. Using something > > > > like a > > > > RX callback to handle the GRO for the developer. This way he just turns > > > > it > > > > on in via a ethdev offload support feature and then setup the RX > > > > callback > > > > via ethdev. The developer only needs to enable the feature and never > > > > calls > > > > GRO APIs. > > > > > > > > The advantage of providing an application-used GRO library can enable > > > > more > > > > flexibility to applications. Applications can flexibly use GRO functions > > > > according to own realistic scenario. Therefore, I think it makes sense > > > > to > > > > provide an application-used GRO library. > > > > > > > > > > Adding a new GRO library may not get much support and having a whole > > > > library for GRO seems a bit odd. > > > > > > > > In my opinion, we just need to provide one GRO library. But it can be > > > > used > > > > by ethernet devices and applications at the same time. Ethernet devices > > > > use it to provide an offload feature. If applications want more > > > > flexibility, they can just turn off this device feature, and use GRO > > > > APIs > > > > directly. > > > > > > > > +Konstantin > > > > > > > > > > [Apologies for the basic questions, I haven't studied the patchset in > > > detail] > > > Rather than adding a whole new library for it, can it just fit into > > > librte_net or an existing lib? Are we planning a sample to show off > > tighter integration with ethdev or changes to the ethdev library to > > transparently use the library when needed? > > > > Currently, we have an individual library, librte_ip_frag, which provides IP > > fragment > > and ressembly abilities. Similiarly, DPDK GRO will provide reassembly > > ability for > > various of protocols, not only TCP. So I think it's good to make a new > > library for > > this feature. > > > > About GRO, we had a discussion two monthes ago. You can see it in > > http://dpdk.org/ml/archives/dev/2017-January/056276.html > > In that discussion, we agree to support GRO in two steps. The first is to > > implement > > GRO as a standalone library, and see how much performance we can get. The > > second > > step is to discuss how to integrate GRO into DPDK. Therefore, if we agree > > to support > > GRO as a device feature, we need to discuss how to enable/disable this > > device feature. > > Once we reach an agreement, there will be a sample to demonstrate the > > integration. > > I think that having a separate library for GRO is a step in a right direction. > >From my perspective - it provides a clean and flexible way to use that > >feature. > If later someone would like to put GRO into ethdev layer (or particular PMD), > he can use existing librte_gro for that.
Agree. I think introducing more flexibility is an important thing for applications. > I didn't have a closer look yet, but I think that caught my attention: > API fir the lib seems too IPv4/TCP oriented - > though I understand that the most common case and should be implemented first. > I wonder can we have it a bit more generic and extendable, so user can > specify what combination of protocols > he is interested in (let say: ipv4/tcp, ipv6/tcp, etc.). > Even if right now we'll have it implemented only for ipv4/tcp. > Then internally we can have some check is that supported or not and if yes > setup things accordingly. Indeed, current apis are too specific. It's not very friendly to applications. Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4 and GRO_UDP_IPV6; and provide a generic setup function and reassembly function. Both of them perform different operations according to the macro value inputted by the application. > BTW, that's for 17.08, right? Yes, it's for 17.08. Jiayu > > Konstantin > >
