From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga05.intel.com (mga05.intel.com [192.55.52.43])
by dpdk.org (Postfix) with ESMTP id 65E49695D
for <dev@dpdk.org>; Fri, 24 Mar 2017 12:43:44 +0100 (CET)
Received: from orsmga005.jf.intel.com ([10.7.209.41])
by fmsmga105.fm.intel.com with ESMTP; 24 Mar 2017 04:43:43 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.36,214,1486454400"; d="scan'208";a="78945806"
Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155])
by orsmga005.jf.intel.com with ESMTP; 24 Mar 2017 04:43:41 -0700
Received: from irsmsx109.ger.corp.intel.com ([169.254.13.175]) by
IRSMSX102.ger.corp.intel.com ([169.254.2.7]) with mapi id 14.03.0248.002;
Fri, 24 Mar 2017 11:43:41 +0000
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Hu, Jiayu" <jiayu.hu@intel.com>, Yuanhan Liu <yuanhan.liu@linux.intel.com>
CC: "Wiles, Keith" <keith.wiles@intel.com>, "Richardson, Bruce"
<bruce.richardson@intel.com>, Stephen Hemminger <stephen@networkplumber.org>,
"Yigit, Ferruh" <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
"Liang, Cunming" <cunming.liang@intel.com>, Thomas Monjalon
<thomas.monjalon@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
Thread-Index: AQHSou9jpQ4UJraET0mXNmgGJzKXbKGh9nLjgAAqOwCAABfzgIAAKzIggADheACAAEHWAIAAEcwAgAAMT4CAADY0gA==
Date: Fri, 24 Mar 2017 11:43:40 +0000
Message-ID: <2601191342CEEE43887BDE71AB9772583FAD80A6@IRSMSX109.ger.corp.intel.com>
References: <1B893F1B-4DA8-4F88-9583-8C0BAA570832@intel.com>
<20170323021502.GA114662@localhost.localdomain>
<C830A6FC-F440-4E68-AB4E-2FD502722E3F@intel.com>
<20170323062433.GA120139@localhost.localdomain>
<59AF69C657FD0841A61C55336867B5B066729E3F@IRSMSX103.ger.corp.intel.com>
<20170323102135.GA124301@localhost.localdomain>
<2601191342CEEE43887BDE71AB9772583FAD410A@IRSMSX109.ger.corp.intel.com>
<20170324022310.GA129105@localhost.localdomain>
<32FED22E-7116-4B6A-894E-C81CBF7B4BBE@intel.com>
<20170324072230.GU18844@yliu-dev.sh.intel.com>
<20170324080633.GA2865@localhost.localdomain>
In-Reply-To: <20170324080633.GA2865@localhost.localdomain>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [163.33.239.182]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 24 Mar 2017 11:43:45 -0000
> -----Original Message-----
> From: Hu, Jiayu
> Sent: Friday, March 24, 2017 8:07 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Cc: Wiles, Keith <keith.wiles@intel.com>; Ananyev, Konstantin <konstantin=
.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Stephen Hemminger <stephen@networkplumber.o=
rg>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> dev@dpdk.org; Liang, Cunming <cunming.liang@intel.com>; Thomas Monjalon <=
thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
>=20
> On Fri, Mar 24, 2017 at 03:22:30PM +0800, Yuanhan Liu wrote:
> > On Fri, Mar 24, 2017 at 06:18:48AM +0000, Wiles, Keith wrote:
> > > >> I think that having a separate library for GRO is a step in a righ=
t 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 parti=
cular PMD),
> > > >> he can use existing librte_gro for that.
> > > >
> > > > Agree. I think introducing more flexibility is an important thing f=
or applications.
> > >
> > > Creating a new library just for GRO is not a reasonable solution, but=
adding that support to an existing library like librte_net would
> be cleaner and not create yet another library.
> >
> > Librte_net seems like a good suggestion to me, especially when we are
> > considering to add GSO in future. The only concern to me is "net" may
> > be too generic. It maybe kind of hard to decide which should be in
> > librte_net, and which should be added as a standalone lib. For example,
> > shouldn't 'lpm' and 'ip_frag' also belong to librte_net?
About librte_gro vs librte_net:
Right now librte_net is quite lightweight one - it mostly contains a net pr=
otocols definitions
plus some extra helper functions: to parse the l2/l3 headers to determine p=
type, to calculate cksum, etc.
GRO code is quite different - it has to allocate and manage hash table(s), =
etc.
Again my understanding it would keep growing (with new proto support).
Again as mentioned above if GRO should go into librte_net, then librte_ipfr=
ag and future GSO should also be here.
Which would create quite a monstrous library. =20
So I think it is better to keep librte_net small and tidy and put GRO funct=
ionality into the new library.
> >
> > > Creating more flexibility is not the best goal as we really want to m=
ake GRO easy and simple for the developer to use for any
> device without having to change his applications to take advantage of the=
feature. Some times providing more flexibility just means
> making it more complexed and more APIs the developer needs to understand.=
Providing GRO as a offload feature is the better
> direction as it makes it simple for an application to use.
> > >
> > > If we provide GRO as a standard offload similar to the other offloads=
we currently have makes it easy for the developer. The best
> goal for a feature is the best performance for the application without ha=
ving the application make even more APIs calls along with
> simple and easy to use.
> >
> > In general, I'd agree with you, if no one is object to add a short piec=
e
> > of code at the end of rte_eth_rx_burst:
> >
> > + if (eth_gro_is_enabled(dev))
> > + nb_rx =3D rte_net_gro(...);
> > +
> > return nb_rx;
> > }
> >
> > Objections?
I'd better not to open that door.
If we'll allow that for GRO - we'll have to allow that for every other stuf=
f:
- ip reassembly
- l3/l4 cksum calculation if underlying HW doesn't support it
- SW ptype recognition
- etc.
Adding all these things into rx_burst() would most likely slow things down
(remember it is a data-path) and pretty soon would bring rx_burst() into
messy monster that would be hard to maintain and debug. =20
My preference would be to keep rte_ethdev data-path as small and tidy as po=
ssible.
If in future we'd really like to introduce all these SW things into dev lay=
er -
my preference would be to create some sort of new abstraction on top of cur=
rent ethdev:
rte_eth_smartdev or so.
So it would be:
rte_eth_smartdev_rx_burst(....)
{
nb_rx =3D rte_eth_rx_burst(...);
/* apply GRO, reassembly, etc. */
...
}=20
Something similar with what 6Wind trying to introduce with their failsafe d=
ev concept.
> >
> > But one way or another, we need put the gro code at somewhere and we
> > need introduce a generic API for that. It could be librte_net as you
> > suggested. So the good thing is that we all at least come an agreement
> > that it should be implemented in lib, right? The only controversy is
> > should we export it to application and let them to invoke it, or hide
> > it inside rte_eth_rx_burst.
> >
> > Though it may take some time for all of us to come an agreement on that=
,
> > but the good thing is that it would be a very trivial change once it's
> > done. Agree?
>=20
> Agree.
>=20
> >
> > Thus I'd suggest Jiayu to focus on the the GRO code developement, such
> > as making it generic enough and adding other protocols support. And I
> > would like to ask you guys to help review them. Makes sense to all?
> >
>=20
> Agree again. No matter where to put GRO code, the apis should be generic
> and extensible. And more protocols should be supported.
Yep, that's what my take from the beginning:
Let's develop a librte_gro first and make it successful, then we can think =
should
we (and how) put into ethdev layer.
Konstantin
>=20
> Thanks,
> Jiayu
>=20
> > Thanks.
> >
> > --yliu
> >
> >
> > > >> I didn't have a closer look yet, but I think that caught my atten=
tion:
> > > >> API fir the lib seems too IPv4/TCP oriented -
> > > >> though I understand that the most common case and should be implem=
ented 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 an=
d if yes setup things accordingly.
> > > >
> > > > Indeed, current apis are too specific. It's not very friendly to ap=
plications.
> > > > 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 reassemb=
ly function.
> > > > Both of them perform different operations according to the macro va=
lue inputted
> > > > by the application.
> > > >
> > > >> BTW, that's for 17.08, right?
> > > >
> > > > Yes, it's for 17.08.
> > > >
> > > > Jiayu
> > > >>
> > > >> Konstantin
> > > >>
> > > >>
> > >
> > > Regards,
> > > Keith