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 dpdk.space (Postfix) with ESMTP id 288E1A0096
for <public@inbox.dpdk.org>; Tue, 9 Apr 2019 19:05:27 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id CC3784D3A;
Tue, 9 Apr 2019 19:05:26 +0200 (CEST)
Received: from mga06.intel.com (mga06.intel.com [134.134.136.31])
by dpdk.org (Postfix) with ESMTP id 3B12D4D27
for <dev@dpdk.org>; Tue, 9 Apr 2019 19:05:25 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from fmsmga002.fm.intel.com ([10.253.24.26])
by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
09 Apr 2019 10:05:24 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.60,330,1549958400"; d="scan'208";a="159635388"
Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31])
by fmsmga002.fm.intel.com with ESMTP; 09 Apr 2019 10:05:22 -0700
Received: from irsmsx103.ger.corp.intel.com ([169.254.3.30]) by
IRSMSX106.ger.corp.intel.com ([169.254.8.235]) with mapi id 14.03.0415.000;
Tue, 9 Apr 2019 18:05:21 +0100
From: "Richardson, Bruce" <bruce.richardson@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, Aaron Conole
<aconole@redhat.com>
CC: "dev@dpdk.org" <dev@dpdk.org>, Jerin Jacob <jerinj@marvell.com>, Gavin Hu
<gavin.hu@arm.com>, Michael Santana <msantana@redhat.com>
Thread-Topic: [PATCH 3/3] acl: adjust the tests
Thread-Index: AQHU7tRnFzUpsbeTZEqJJqSVVkAp/6Yz7P2AgAAh47A=
Date: Tue, 9 Apr 2019 17:05:21 +0000
Message-ID:
<59AF69C657FD0841A61C55336867B5B072787B4F@IRSMSX103.ger.corp.intel.com>
References: <20190408182420.4398-1-aconole@redhat.com>
<20190408182420.4398-4-aconole@redhat.com>
<2601191342CEEE43887BDE71AB9772580148A94A44@irsmsx105.ger.corp.intel.com>
<f7ty34jz60u.fsf@dhcp-25.97.bos.redhat.com>
<2601191342CEEE43887BDE71AB9772580148A94D87@irsmsx105.ger.corp.intel.com>
In-Reply-To: <2601191342CEEE43887BDE71AB9772580148A94D87@irsmsx105.ger.corp.intel.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTVlMDgyNjUtZmRlZC00NTcwLWIyYWQtZjQxYzc0NjI4OTczIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiXC9uVXlYZldcL2EzK1dVS28rZjdhZ2ZIWW5FOFJGREN0clVGbk5vZ3BBNGtZY1UyUEpmS1ZWbitIdlNOam05azRcLyJ9
x-ctpclassification: CTP_NT
dlp-product: dlpe-windows
dlp-version: 11.0.600.7
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH 3/3] acl: adjust the tests
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>
Message-ID: <20190409170521.nIEtksmKYApu167AegB6wHdaxMcPOf_sRr6L75Yfz7k@z>
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, April 9, 2019 5:03 PM
> To: Aaron Conole <aconole@redhat.com>
> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Gavin Hu
> <gavin.hu@arm.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> Michael Santana <msantana@redhat.com>
> Subject: RE: [PATCH 3/3] acl: adjust the tests
>=20
>=20
> > > Hi Aaron,
> > >
> > >>
> > >> This makes the tests pass, and also ensures that on platforms where
> > >> the testing is supported, we can properly test the implementation
> > >> specific code. One edge case is when we run on x86_64 systems that
> > >> don't support AVX2, but where the compiler can generate such
> > >> instructions. That could be an enhancement in the future, but for
> > >> now at least the tests will pass.
> > >>
> > >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> > >> ---
> > >> app/test/test_acl.c | 62 +++++++++++++-----------------=
-
> --
> > >> lib/librte_acl/Makefile | 1 +
> > >> lib/librte_acl/acl_run_notsup.c | 46 ++++++++++++++++++++++++
> > >> lib/librte_acl/meson.build | 4 +--
> > >> 4 files changed, 73 insertions(+), 40 deletions(-) create mode
> > >> 100644 lib/librte_acl/acl_run_notsup.c
> > >>
> > >> diff --git a/app/test/test_acl.c b/app/test/test_acl.c index
> > >> b1f75d1bc..c44faa251 100644
> > >> --- a/app/test/test_acl.c
> > >> +++ b/app/test/test_acl.c
> > >> @@ -408,6 +408,9 @@ test_classify(void)
> > >> return -1;
> > >> }
> > >>
> > >> + /* Always use the scalar testing for now. */
> > >> + rte_acl_set_ctx_classify(acx, RTE_ACL_CLASSIFY_SCALAR);
> > >> +
> > >> ret =3D 0;
> > >> for (i =3D 0; i !=3D TEST_CLASSIFY_ITER; i++) {
> > >>
> > >> @@ -547,6 +550,7 @@ test_build_ports_range(void)
> > >> for (i =3D 0; i !=3D RTE_DIM(test_data); i++)
> > >> data[i] =3D (uint8_t *)&test_data[i];
> > >>
> > >> + rte_acl_set_ctx_classify(acx, RTE_ACL_CLASSIFY_SCALAR);
> > >> for (i =3D 0; i !=3D RTE_DIM(test_rules); i++) {
> > >> rte_acl_reset(acx);
> > >> ret =3D test_classify_buid(acx, test_rules, i + 1); @@ -
> 911,6
> > >> +915,8 @@ test_convert_rules(const char *desc,
> > >> return -1;
> > >> }
> > >>
> > >> + rte_acl_set_ctx_classify(acx, RTE_ACL_CLASSIFY_SCALAR);
> > >> +
> > >
> > > As I understand here and above, on x86 you replaced default algo
> > > (SSE, AVX2) with scalar one, right?
> > > That looks like reduction of test coverage for x86.
> >
> > In one way, you're right. However, the tests weren't testing what
> > they purported anyway.
>=20
> Could you explain a bit more here?
> What I am seeing: tests were running bot sse(or avx2) and scalar
> classify() method.
> Now they always running scalar only.
> To me it definitely looks like reduction in coverage.
>=20
> > Actually, it's just a shift I think (previously, it would have tested
> > the AVX2 but I don't see AVX2 having a fallback into the SSE code -
> > unlike the SSE code falling back into scalar).
>=20
> Not sure I understand you here.
> What fallback for AVX2 you expect that you think is missing?
>=20
> >
> > The tests were failing for a number of reasons when built with meson,
>=20
> Ok, but with legacy build system (make) on x86 all tests passes, right?
> So the problem is in new build system, not in the test itself.
> Why we should compromise our test coverage to make it work with new tools=
?
> That just hides the problem without fixing it.
> Instead I think the build system needs to be fixed.
> Looking at it a bit closely, for .so meson+ninja generates code with
> correct version of the function:
>=20
> nm x86_64-native-linuxapp-gcc-meson/lib/librte_acl.so.2 | grep
> acl_classify_sse
> 000000000000fa50 t rte_acl_classify_sse
>=20
> So for 'meson -Ddefault_library=3Dshared'
> acl_autotest passes without the problem.
>=20
> Though for static lib we have both:
> nm x86_64-native-linuxapp-gcc-meson/lib/librte_acl.a | grep
> acl_classify_sse
> 0000000000000000 W rte_acl_classify_sse
> 0000000000004880 T rte_acl_classify_sse
>=20
> And then linker pickups the wrong one:
> nm x86_64-native-linuxapp-gcc-meson/app/test/dpdk-test | grep
> acl_classify_sse
> 00000000005f6100 W rte_acl_classify_sse
>=20
> While for make:
> $ nm x86_64-native-linuxapp-gcc-aesmb/lib/librte_acl.a | grep
> acl_classify_sse
> 0000000000000000 W rte_acl_classify_sse
> 0000000000004880 T rte_acl_classify_sse
> $ nm x86_64-native-linuxapp-gcc-aesmb/app/test | grep acl_classify_sse
> 0000000000240440 T rte_acl_classify_sse
>=20
> Linker pickups the right one.
>=20
I assume the same issues occurs for AVX2, but for SSE specifically why do w=
e even compile up two copies of the function for x86 platforms, since SSE w=
ill always be supported?
/Bruce