From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id CD4C6A0352;
Sun, 3 Nov 2019 12:38:36 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id B50FE1D417;
Sun, 3 Nov 2019 12:38:35 +0100 (CET)
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
by dpdk.org (Postfix) with ESMTP id 7254C1D414
for <dev@dpdk.org>; Sun, 3 Nov 2019 12:38:34 +0100 (CET)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga007.jf.intel.com ([10.7.209.58])
by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
03 Nov 2019 03:38:33 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.68,263,1569308400"; d="scan'208";a="191474848"
Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201])
by orsmga007.jf.intel.com with ESMTP; 03 Nov 2019 03:38:32 -0800
Received: from fmsmsx153.amr.corp.intel.com (10.18.125.6) by
FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS)
id 14.3.439.0; Sun, 3 Nov 2019 03:38:32 -0800
Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by
FMSMSX153.amr.corp.intel.com (10.18.125.6) with Microsoft SMTP Server (TLS)
id 14.3.439.0; Sun, 3 Nov 2019 03:38:32 -0800
Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by
SHSMSX103.ccr.corp.intel.com ([169.254.4.60]) with mapi id 14.03.0439.000;
Sun, 3 Nov 2019 19:38:30 +0800
From: "Wang, Haiyue" <haiyue.wang@intel.com>
To: Slava Ovsiienko <viacheslavo@mellanox.com>, "Liu, Yu Y"
<yu.y.liu@intel.com>, Thomas Monjalon <thomas@monjalon.net>
CC: "dev@dpdk.org" <dev@dpdk.org>, "arybchenko@solarflare.com"
<arybchenko@solarflare.com>, "Yigit, Ferruh" <ferruh.yigit@intel.com>,
"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>, "Ye, Xiaolong"
<xiaolong.ye@intel.com>, "Kinsella, Ray" <ray.kinsella@intel.com>, "Sun,
Chenmin" <chenmin.sun@intel.com>, "Damjan Marion (damarion)"
<damarion@cisco.com>
Thread-Topic: [PATCH v1 3/3] ethdev: enhance the API for getting burst mode
information
Thread-Index: AQHVkA85NRUGBSr/iU+DJBHluQAAgad2ZiSAgAD2BdD//5LDgIAAHLKAgAGgxyD///dfAIAAoh0Q
Date: Sun, 3 Nov 2019 11:38:29 +0000
Message-ID: <E3B9F2FDCB65864C82CD632F23D8AB8773D8DEE0@shsmsx102.ccr.corp.intel.com>
References: <20191031171139.105110-1-haiyue.wang@intel.com>
<20191031171139.105110-3-haiyue.wang@intel.com> <20693558.VL3dRorq05@xps>
<E3B9F2FDCB65864C82CD632F23D8AB8773D8DC86@shsmsx102.ccr.corp.intel.com>
<D9E7459352598649B9DF0563CBC90AEB778BC13A@SHSMSX101.ccr.corp.intel.com>
<AM4PR05MB3265E19B616FA4EF816C8DE6D27D0@AM4PR05MB3265.eurprd05.prod.outlook.com>
<E3B9F2FDCB65864C82CD632F23D8AB8773D8DE2D@shsmsx102.ccr.corp.intel.com>
<AM4PR05MB3265F1515F9E3D193B37B98AD27C0@AM4PR05MB3265.eurprd05.prod.outlook.com>
In-Reply-To: <AM4PR05MB3265F1515F9E3D193B37B98AD27C0@AM4PR05MB3265.eurprd05.prod.outlook.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDY4NTUyNDctMGIyNy00MWUzLTllNDYtZDVkZTM3ZDM3YjU1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiczRvZHUxSmdOV21YS1lVVUxnU3lBV1d0WEVvSFpSSXQ5Zk42aW56ZjdCSE9rd3IzbCtnMGRNOWFsS2ROUEhnViJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.2.0.6
dlp-reaction: no-action
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for getting
burst mode information
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Hi Slava,
> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo@mellanox.com>
> Sent: Sunday, November 3, 2019 16:59
> To: Wang, Haiyue <haiyue.wang@intel.com>; Liu, Yu Y <yu.y.liu@intel.com>;=
Thomas Monjalon
> <thomas@monjalon.net>
> Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh <ferruh.yigit@=
intel.com>;
> jerinjacobk@gmail.com; Ye, Xiaolong <xiaolong.ye@intel.com>; Kinsella, Ra=
y <ray.kinsella@intel.com>;
> Sun, Chenmin <chenmin.sun@intel.com>; Damjan Marion (damarion) <damarion@=
cisco.com>
> Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst mod=
e information
>=20
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Sunday, November 3, 2019 4:34
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>; Liu, Yu Y
> > <yu.y.liu@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; jerinjacobk@gmail.com; Ye, Xiaolong
> > <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>; Sun,
> > Chenmin <chenmin.sun@intel.com>; Damjan Marion (damarion)
> > <damarion@cisco.com>
> > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst m=
ode
> > information
> >
> > Hi Thomas, Slava,
> >
> > Please see the inline reply in one place.
> >
> > > -----Original Message-----
> > > From: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > Sent: Saturday, November 2, 2019 16:39
> > > To: Liu, Yu Y <yu.y.liu@intel.com>; Wang, Haiyue
> > > <haiyue.wang@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh
> > > <ferruh.yigit@intel.com>; jerinjacobk@gmail.com; Ye, Xiaolong
> > > <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>; Sun,
> > > Chenmin <chenmin.sun@intel.com>; Damjan Marion (damarion)
> > > <damarion@cisco.com>
> > > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst
> > > mode information
> > >
> > > Hi
> > > > -----Original Message-----
> > > > From: Liu, Yu Y <yu.y.liu@intel.com>
> > > > Sent: Saturday, November 2, 2019 8:56
> > > > To: Wang, Haiyue <haiyue.wang@intel.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>
> > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh
> > > > <ferruh.yigit@intel.com>; jerinjacobk@gmail.com; Ye, Xiaolong
> > > > <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> > > > Sun, Chenmin <chenmin.sun@intel.com>; Slava Ovsiienko
> > > > <viacheslavo@mellanox.com>; Damjan Marion (damarion)
> > > > <damarion@cisco.com>; Liu, Yu Y <yu.y.liu@intel.com>
> > > > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting
> > > > burst mode information
> > > >
> > > > Add Damjan from FD.io for awareness...
> > > >
> > > > Hi Thomas,
> > > >
> > > > Long time no see. Sorry I use outlook which is not friendly to
> > > > community email.
> > > >
> > > > >Anyway I will propose to replace this API in the next release.
> > > > Will your plan be affected by API/ABI stable plan?
> > > > BTW, if you propose new change in next release, it will make DPDK
> > > > consumer(FD.io) to change again.
> > > > So even if it is not affected to the API/ABI stable plan, do we
> > > > still have time to get a solution for everyone in DPDK 19.11 with
> > > > your contribution/acceleration?
> > > >
> > > > > I suspect a real hidden issue in Intel CPUs that you try to mitig=
ate.
> > > > Please be rest assured it is not the case.
> > > > This request is just from one FD.io project internal bug " tx/rx
> > > > burst function is shown as nil" reported by Chenmin.
> > >
> > > Why just the presenting string with function name (possible with suff=
ix) is
> > not enough?
> > > I would like to see this API (strings approach) in mlx5 either,
> > > dropping the entire feature does not look nice, as for me.
> > >
> > > We could consider some requirements for the name suffices to
> > > distinguish whether function uses vector instructions and which ones =
if any.
> > >
> > > > My understanding is DPDK behavior was taken as bug for someone in
> > > > FD.io project and potentially will mislead other DPDK consumer.
> > >
> > > Why does FD.io code want to know which vector extension is used by bu=
rst
> > routines?
> > > Is it going to share/preserve some resources (registers, etc.)? Is it=
robust ?
> > > Burst routines might not know whether vector extensions is used (they
> > > might call libraries, even rte_memcpy() can use vectors in implicit f=
ashion).
> > >
> >
> > 1.
> >
> > The original issue description is:
> > "VPP uses dladdr() to translate a function address to name, however, so=
me
> > tx/rx functions in DPDK are invisible for dladdr(), which is because t=
hey are
> > defined as static."
> >
> > 2.
> >
> > So the RFC design is: one function, one description, like:
> > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fpat=
ch
> > work.dpdk.org%2Fpatch%2F57644%2F&data=3D02%7C01%7Cviacheslavo
> > %40mellanox.com%7Ca99632b4e2444ec00b1f08d760065041%7Ca652971c7
> > d2e4d9ba6a4d149256f461b%7C0%7C0%7C637083452540980873&sdat
> > a=3D4re5GOXPSwGk5BTOYLglafzgjBzRLk1gXyWKT47o8o0%3D&reserved=3D
> > 0
> >
> > +#ifdef RTE_ARCH_X86
> > + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec_avx2)
> > + len =3D snprintf(buf, sz, "AVX2 Vector Scattered Rx");
> > + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec)
> > + len =3D snprintf(buf, sz, "Vector Scattered Rx");
> > + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec_avx2)
> > + len =3D snprintf(buf, sz, "AVX2 Vector Rx");
> > + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec)
> > + len =3D snprintf(buf, sz, "Vector Rx"); #endif
> The application gets the routine names with dladdr(). OK, it happens.
> It is not clear for me why instead of direct replacement/extension of dla=
ddr
> functionality some new names were introduced and then converted to flags.
>=20
Sorry, can you explain more ? Who 'direct replacement/extension of dladdr' =
?
VPP, or DPDK ?
> > 3. Since the main issue is as Damjan replied in another thread:
> > "people are reporting lower performance caused by DPDK deciding for
> > variety
> > of reasons to switch from vector PMD to scalar one."
> >
> > And Ferruh replied also:
> > "As I understand this is to let applications to give informed decis=
ion based
> > on what vectorization is used in the driver, currently this is not=
known by
> > the application.
> >
> > And as previously replied, the main target of the API is to define=
the vector
> > path, not all optimizations, so the number is limited."
> >
> > So we enhanced it with bit, example detail is (Yes, we defined a l=
it more,
> > so we removed it in this patch):
>=20
> There are might be a lot of various burst functions, vectorized or not,
> with various sets of supported offloads. Yes, identifying the engaged bur=
st
> routine is meaningful, but it is not clear for me, why the vectorizing ty=
pe
> should have dedicated means (flags) to identify ?
>=20
The new 'rte_eth_rx/tx_burst_mode_get' works like logging, but in fact, the=
log
message is something special, like "Vector Neon/AltiVec/SSE/AVX2" and the d=
evice
specific offloads as you said.
This kind of string "Vector Neon/AltiVec/SSE/AVX2" can be common, we not tr=
eat it
as 'flag', it is a normal bit like macro definition, and it will be transla=
ted into
string later. And we want to make PMD's string format life to be easy, don'=
t need
to call 'snprintf/sprintf' with the copied string format.
So now, the log message format is: device specific (if have) + "Vector ..."=
(if
have, this is not MUST, if the PMD doesn't use vector, but at least, this i=
s not
hardware specific, it is some common from arch: lib/librte_eal/common/arch/=
arm,ppc_64,x86).
Further, as a SDK, the API exposes these common bit data for application ea=
sily access
if it may need, DON'T NEED TO BREAK THE ABI/API. Compared to 'strstr/strcas=
estr',
'mode->options & RTE_ETH_BURST_VECTOR' is more friendly ?
> >
> > https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fpat=
ch
> > work.dpdk.org%2Fpatch%2F61196%2F&data=3D02%7C01%7Cviacheslavo
> > %40mellanox.com%7Ca99632b4e2444ec00b1f08d760065041%7Ca652971c7
> > d2e4d9ba6a4d149256f461b%7C0%7C0%7C637083452540980873&sdat
> > a=3Dnm80Pt0fFWqmmrJcKY6ks4qRTJ7cjGJWEG1Wv6gxfSw%3D&reserved
> > =3D0
> >
> > 4. And thanks Jerin's suggestion, I think his word can be more accurate=
: "This
> > would
> > help to reuse some of the flags to name conversion logic across all =
PMDs"
> > for the
> > reason we try to use bit to reduce some string format effort, it wil=
l be
> > handled
> > by the API internally "burst_mode_options_append(struct
> > rte_eth_burst_mode *mode)".
> > Now the new API will return the string finally:
> >
> > #define RTE_ETH_BURST_MODE_ALT_OPT_SIZE 1024 struct
> > rte_eth_burst_mode {
> > uint64_t options;
> >
> > /**< Each PMD can fill specific burst mode information into this, and
> > * ethdev APIs will append the 'options' string format at its end.
> > */
> > char alternate_options[RTE_ETH_BURST_MODE_ALT_OPT_SIZE];
> > };
> >
> > So MLX PMD can add 'full_empw', 'mtsc_empw' etc into 'alternate_options=
'
> > firstly, assign 'RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE' to 'options'
> > as needed, then finally, 'alternate_options' will be "full_empw, Vector=
SSE".
>=20
> For mlx5 tx_burst these flags have no meaning. All information regarding =
routine
> is encoded within the name, mtsc stands for:
> m - multisegment
> t - TSO
> s - software tunnel parser
> c - check sum
>=20
> There are no two versions of mtsc_empw - "mtsc_empw, Vector SSE", "mtsc_e=
mpw, Vector Neon".
> If we developed vectorized version, I would prefer "mtsc_empw_sse".
>=20
> To summarize:
> - application uses routine names, gets with dladdr(). Nice.
> - compatible API providing names of internal routines is proposed. Nice.
> - users now are able to identify the engaged burst routine. Nice.
> - proposed API is extended, some vector related flags were added. Hmmm...=
. Questionable.
> Why vector related only? Why do we change the string format? (name -> n=
ame, options)
Again, vector is not "only", it is just 'main' characteristic "tree ./drive=
rs/net/ | grep rxtx_".
we can design the Rx/Tx burst function mainly by vector type, it is straigh=
tforward.
Why 'name -> name' ?
1.) [v4,4/4] app/testpmd: show the Rx/Tx burst mode description
https://patchwork.dpdk.org/patch/61198/
This is handled by the application itself, not so friendly, many lines of c=
ode to show.
2). [PATCH v1 3/3] ethdev: enhance the API for getting burst mode informati=
on
if (rte_eth_tx_burst_mode_get(port_id, queue_id, &mode) =3D=3D 0)
printf("\nBurst mode: %s", mode.alternate_options);
This design may meet your question above if I understand correctly.
"It is not clear for me why instead of direct replacement/extension of dla=
ddr
functionality some new names were introduced and then converted to f=
lags".
Last, again, we define the bits 'RTE_ETH_BURST_XXX' for making the log mess=
age
generation process easily if you agree vector type is common, the vector ca=
n be
used to improve the performance. And if new burst design can be used for mo=
st
PMDs, use it as bit, the API helps to translate it to string. And the appli=
cation
can use the bit to do other kind of information display.
We define it a little more than 'simple string' for just making life easy. =
In fact,
the patch comes from "simple string", RFC v1, v2, v3, PATCH v1 v2 v3 v4.
>=20
>=20
> > Intel PMD can just assign "options", then finally, 'alternate_options' =
will be
> > "Vector SSE".
>=20
> As I see from initial patch, Intel PMD has dedicated routines with unique=
names for
> each type of vectorization. Is there some burst routine with single name=
which could
> operate with different vectorization types, depending on configuration?
>=20
> With best regards, Slava
>=20
> >
> > How about the design idea ? Again, this 'options' is not to do standard=
ization,
> > just want to reduce the duplicated name string things.
> >
> > > With best regards, Slava
> > >
> > > > Haiyue is working with Chenmin to address the issue and with your
> > > > support it will be even better.
> > > >
> > > > Your support will be highly appreciated!
> > > >
> > > > Thanks & Regards,
> > > > Yu Liu
> > > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Wang, Haiyue
> > > > Sent: Saturday, November 2, 2019 1:30 PM
> > > > To: Thomas Monjalon <thomas@monjalon.net>
> > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh
> > > > <ferruh.yigit@intel.com>; jerinjacobk@gmail.com; Ye, Xiaolong
> > > > <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> > > > Sun, Chenmin <chenmin.sun@intel.com>; Slava Ovsiienko
> > > > <viacheslavo@mellanox.com>
> > > > Subject: Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for
> > > > getting burst mode information
> > > >
> > > > > -----Original Message-----
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Saturday, November 2, 2019 06:46
> > > > > To: Wang, Haiyue <haiyue.wang@intel.com>
> > > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh
> > > > > <ferruh.yigit@intel.com>; jerinjacobk@gmail.com; Ye, Xiaolong
> > > > > <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> > > > > Sun, Chenmin <chenmin.sun@intel.com>; Slava Ovsiienko
> > > > > <viacheslavo@mellanox.com>
> > > > > Subject: Re: [PATCH v1 3/3] ethdev: enhance the API for getting
> > > > > burst mode information
> > > > >
> > > > > Thank you for trying to address comments done late.
> > > > >
> > > > > 31/10/2019 18:11, Haiyue Wang:
> > > > > > --- a/lib/librte_ethdev/rte_ethdev.h
> > > > > > +++ b/lib/librte_ethdev/rte_ethdev.h
> > > >
> > > >
> > > > > > +#define RTE_ETH_BURST_ALTIVEC (1ULL << 2)
> > > > > > +#define RTE_ETH_BURST_NEON (1ULL << 3)
> > > > > > +#define RTE_ETH_BURST_SSE (1ULL << 4)
> > > > > > +#define RTE_ETH_BURST_AVX2 (1ULL << 5)
> > > > > > +#define RTE_ETH_BURST_AVX512 (1ULL << 6)
> > > > >
> > > > > Of course, I still believe that giving a special treatment to
> > > > > vector instructions is wrong.
> > > > > You did not justify why it needs to be defined in bits instead of
> > > > > string. I am not asking again because anyway you don't really
> > > > > reply. I think you are executing an order you received and I don'=
t
> > > > > want to blame you more.
> > > > > I suspect a real hidden issue in Intel CPUs that you try to mitig=
ate.
> > > > > No need to reply to this comment.
> > > > > Anyway I will propose to replace this API in the next release.
> > > >
> > > > Never mind, if this design is truly ugly, drop it all now. I also
> > > > prefer to do the best, that's why open source is amazing, thanks!
> > > > ;-)