From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 46D8EA00C5; Mon, 11 May 2020 12:57:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1D6361C1F3; Mon, 11 May 2020 12:57:16 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by dpdk.org (Postfix) with ESMTP id E06031C1EB for ; Mon, 11 May 2020 12:57:14 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 04BAt63x024790; Mon, 11 May 2020 03:57:11 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pfpt0818; bh=sMgUE0wFDLDX/mNweEBYfS80l/IUIF0AxHHl3FEY8+U=; b=rjm+z+Bo4UJ4he9OKmBEZvKzs7DSGf8r+yEC303IxXOnsSF7sgpuiacI8z/Ht7xB69eC 2Op+/CdQ7bmaisJudD4jpNWdP/v8/J5B5NDFPGPOFC1Fj6Bwi4h9JrOmDFDg8B/U9LzN iQl8sPqhyBnlDLki6i2kOChmTtJfV90r+WuFXoIzBRJCs+JmiRC7F1c3SsqJX5NNo/yv /ra7bPF77+1CiieC9NjQ/FvJDXC6QeGrs8SF8PEzfSnvtLAotlWiou2vID6XKqmJVsxr nl2OUiqjsORvTgq5RjzZeALgmyEi/X2i4BDmCELTMtDkTtY4YtGxd994iyBQm3VMGZzJ Hg== Received: from sc-exch02.marvell.com ([199.233.58.182]) by mx0a-0016f401.pphosted.com with ESMTP id 30wsvqed85-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Mon, 11 May 2020 03:57:10 -0700 Received: from SC-EXCH01.marvell.com (10.93.176.81) by SC-EXCH02.marvell.com (10.93.176.82) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Mon, 11 May 2020 03:57:09 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.175) by SC-EXCH01.marvell.com (10.93.176.81) with Microsoft SMTP Server (TLS) id 15.0.1497.2 via Frontend Transport; Mon, 11 May 2020 03:57:08 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q7WmXOUo0sFrIYAnZJK+56lOKCun2Oywzih9IQahs/RatJqdaSutAetTQkgqzKwdkYRO3fuUJKgJ7tqhA2afcojowpB8gNdRcsOwX/Guic0g0PzBcSHT+X8IAT4D4NeshZBCjtXuG1sUJWFzXvN4rf0AIJi/+ScjEwBcZV15aPGzPVaCPO0KgumMmazOT5hM0xeepbu8Oh2/iI/bgwXW4LYLnvnSOZz5kQ3fBPBGLoudgBWH4RDVZixqPXF3zPcK4lMkFe5uHKjzgHp2PZXiF+/Uyg2yG1uYmvsTg6m6MlyTkMHKdxJhwiIpDy1+H6erNP8qW9Yk7mXrWmSanAFRsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sMgUE0wFDLDX/mNweEBYfS80l/IUIF0AxHHl3FEY8+U=; b=be/i2wEuAsoQJsRAC/hfX+w30Rm6isHc7lhi7FMWmxSrYgV9iUB0yi9MbxmUi9A9VELZW5Tp2XZuIUIzlf0xbFpNCf+pKYieOq0p/zeHIpA3Vcq8oREj5tNb5irvLjZqkkf8RVl+7xC1/cjv9dflR866t/PwNRprTDmTjmDycMPrF7swg8eeDynBTY7LBX4gYg3sUCXoIEdW5gPISWevCVK5p55TwifF5wYcHZ+gbEk3BD5FcQQZNi0py9YYg+PoaVYEvaleKZYNA1mbw55QBqbCoCQKO0OibEbrDMVa/hiatQilmd91dVer9y4thcQDLm75HwX4kSlYdMeNty1f+Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=sMgUE0wFDLDX/mNweEBYfS80l/IUIF0AxHHl3FEY8+U=; b=grwBthBBDPIc4UK1JChyn1bXLOWYE9bxL4HMTV6mH3PqHWGtfkSboSm/pLdgoCsjmdcRzcdG9NHItuc+9YmKIa7fM0d7bMjJFvR1FVJDld+mQUqCyq+0UDO/iAekK6OqT6d8C43m9VmTeJIHLWmr+ukMh4sSS2j8ve9qOF0D3bA= Received: from BYAPR18MB2518.namprd18.prod.outlook.com (2603:10b6:a03:13b::18) by BYAPR18MB3062.namprd18.prod.outlook.com (2603:10b6:a03:106::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2979.34; Mon, 11 May 2020 10:57:07 +0000 Received: from BYAPR18MB2518.namprd18.prod.outlook.com ([fe80::fca4:5e00:46d9:a289]) by BYAPR18MB2518.namprd18.prod.outlook.com ([fe80::fca4:5e00:46d9:a289%3]) with mapi id 15.20.2979.033; Mon, 11 May 2020 10:57:07 +0000 From: Pavan Nikhilesh Bhagavatula To: "Ananyev, Konstantin" , "Jerin Jacob Kollanukkaran" , "thomas@monjalon.net" , "Wang, Yipeng1" , "Gobriel, Sameh" , "Richardson, Bruce" , Ruifeng Wang CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [RFC] hash: unify crc32 API header for x86 and ARM Thread-Index: AQHWJTgHo7IAeERCXE2KuklKRB3ozKih8AfggAC3mICAAAZJoIAABUUAgAABptA= Date: Mon, 11 May 2020 10:57:07 +0000 Message-ID: References: <20200429180515.5704-1-pbhagavatula@marvell.com> In-Reply-To: Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=marvell.com; x-originating-ip: [223.226.86.58] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 298f52d7-21d0-44e9-2e1d-08d7f59a0c35 x-ms-traffictypediagnostic: BYAPR18MB3062: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 04004D94E2 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: IP1qz2FdOMYz/kx7LULE+ufE4Mt6Akdqg9s6Vd0WOqHVMhYohP+bYkzlHn0Lwsl/OYr0/vJIW3PLgsjZGuN5xFK+fNj+cyBJ131gmE8pkJ4aKg5nj36LpabsUnsTXgWwVSkBFLLqaUx/AcAKQPjzrDbYx+AZGyU3+G2HDQ99C+6VFh1t8FieUC5Xk1od99t0gnCGKTZircaQ5r0MvuzGfFan9arPR/smGGAaN0Ktfw2THHyZssET3lY2CTaEMKif/RahxyrqEzH75YSvVtrhBniqI5caMTlUjlxIcrJX7wC/LGmvwQNo2IMMeLhlH/Nxrtf+DKBDZ1xSLSwthfNgOuAeujD9sIRC1Io7jWbJEcFcgm/0HHbDs8Q9vuqleu7HmHqWcKsvXgIJ15wImaVpEq+1zZsgXKsEMqy4NW7wcRcQSmhOAWhkQTJANpZkRUsS/7vlysNbjzaryQScWl6GicN535eSJhh7mGxrKbtSkZlURREntV1ySUj4EPn7+N2OKpdwmdCVbpFEwEpj4Yd0YcOx3TBd5woqbBXM8f8z6OY= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR18MB2518.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(366004)(396003)(376002)(39850400004)(346002)(136003)(33430700001)(66946007)(55016002)(316002)(86362001)(66476007)(66556008)(64756008)(66446008)(110136005)(9686003)(52536014)(2906002)(4326008)(76116006)(33656002)(33440700001)(8936002)(8676002)(71200400001)(5660300002)(7696005)(6506007)(478600001)(55236004)(186003)(26005)(921003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: 9nAJcPsBB46YQ5vuXd+yLgUjl7EhJcp+QLBwpAxSzCvnP/LUVN+HGhBqqXGB4/T/vnUPfPqJ9AqaketoE4PkuRxIXDs5mMCATldBA4MYFDCVjphrzPq0AJCyISbPYTtulPZhKBLpi330yzqLxLuoBE9coH/H8EXxZVpE28qmeaXyYkhMnPMOjXuyfRAkP36IebMJ3E0XXUa0HWkKszTR2r4ocA5gZvW3ulcIwBF3Avn+E0gSQ+RGdMsQpm4+whpkutFLFhyIXTrrmJBquRtx1qtf7XGzrevlG/MEJl0mPdz3Ht4e4Zv0SDv5gifYumCBm1dGZehGdtst6AOc8Y0ADOsl6OE9+uS4piR7XBvvpO1xOOTR6NksvgQPxJQa5BJLtswjb+ZWzIXTOqBcfmLK81xjj0cfzePexBOcAB+5Pe/cg/Ya52PGDFvsazwG9v7S53V5MwIJl2DACvNbVPwrEw4Nc7+BCvD8FNqq+ToTevc= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 298f52d7-21d0-44e9-2e1d-08d7f59a0c35 X-MS-Exchange-CrossTenant-originalarrivaltime: 11 May 2020 10:57:07.3355 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 2cKyBUfUC+/uH+KnDCufwO4ZkgWXJqJ0M6SN+C26F1r385nw4bXwKDthswoayxp7TWbjQ2IM0m0rR4ib0f+aJm7g724gD/0150t+aV/w71s= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR18MB3062 X-OriginatorOrg: marvell.com X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.216, 18.0.676 definitions=2020-05-11_04:2020-05-11, 2020-05-11 signatures=0 Subject: Re: [dpdk-dev] [RFC] hash: unify crc32 API header for x86 and ARM 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >> >> >> From: Pavan Nikhilesh >> >> >> >> >> >> Merge crc32 hash calculation public API headers for x86 and >ARM, >> >> >> split implementations of x86 and ARM into their respective >private >> >> >> headers. >> >> >> This reduces the ifdef code clutter while keeping current ABI >> >intact. >> >> >> >> >> >> Although we install `rte_crc_arm64.h` it is not used in any of the >lib >> >or >> >> >> drivers layers. All the libs and drivers use `rte_hash_crc.h` whic= h >> >falls >> >> >> back to SW crc32 calculation for ARM platform. >> >> >> >> >> >> Signed-off-by: Pavan Nikhilesh >> >> >> --- >> >> >> >> >> >> Currently, if application incorrectly sets CRC32_ARM64 as crc32 >> >> >algorithm >> >> >> through `rte_hash_crc_set_alg()` on x86 or vice-versa we >fallback >> >to >> >> >algorithm >> >> >> set previously via `rte_hash_crc_set_alg()` instead of setting >the >> >best >> >> >> available. >> >> >> This behaviour should probably change to setting the best >> >available >> >> >algorithm >> >> >> and is up for discussion. >> >> >> >> >> >> app/test/test_hash.c | 6 + >> >> >> lib/librte_hash/Makefile | 5 - >> >> >> lib/librte_hash/crc_arm64.h | 67 +++++++++++ >> >> >> lib/librte_hash/crc_x86.h | 68 +++++++++++ >> >> >> lib/librte_hash/meson.build | 3 +- >> >> >> lib/librte_hash/rte_crc_arm64.h | 183 ---------------------------= --- >> >> >> lib/librte_hash/rte_hash_crc.h | 193 +++++++++++++----------- >--- >> >---- >> >> >- >> >> >> 7 files changed, 219 insertions(+), 306 deletions(-) >> >> >> create mode 100644 lib/librte_hash/crc_arm64.h >> >> >> create mode 100644 lib/librte_hash/crc_x86.h >> >> >> delete mode 100644 lib/librte_hash/rte_crc_arm64.h >> >> >> >> >> >> diff --git a/app/test/test_hash.c b/app/test/test_hash.c >> >> >> index afa3a1a3c..7bd457dac 100644 >> >> >> --- a/app/test/test_hash.c >> >> >> +++ b/app/test/test_hash.c >> >> >> @@ -195,7 +195,13 @@ test_crc32_hash_alg_equiv(void) >> >> >> } >> >> >> >> >> >> /* Resetting to best available algorithm */ >> >> >> +#if defined RTE_ARCH_X86 >> >> >> rte_hash_crc_set_alg(CRC32_SSE42_x64); >> >> >> +#elif defined RTE_ARCH_ARM64 >> >> >> + rte_hash_crc_set_alg(CRC32_ARM64); >> >> >> +#else >> >> >> + rte_hash_crc_set_alg(CRC32_SW); >> >> >> +#endif >> >> >> >> >> >> if (i =3D=3D CRC32_ITERATIONS) >> >> >> return 0; >> >> >> diff --git a/lib/librte_hash/Makefile b/lib/librte_hash/Makefile >> >> >> index ec9f86499..f640afc42 100644 >> >> >> --- a/lib/librte_hash/Makefile >> >> >> +++ b/lib/librte_hash/Makefile >> >> >> @@ -19,11 +19,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) +=3D >> >> >rte_fbk_hash.c >> >> >> # install this header file >> >> >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include :=3D rte_hash.h >> >> >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >> >rte_hash_crc.h >> >> >> -ifeq ($(CONFIG_RTE_ARCH_ARM64),y) >> >> >> -ifneq ($(findstring >RTE_MACHINE_CPUFLAG_CRC32,$(CFLAGS)),) >> >> >> -SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >> >rte_crc_arm64.h >> >> >> -endif >> >> >> -endif >> >> >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >rte_jhash.h >> >> >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >rte_thash.h >> >> >> SYMLINK-$(CONFIG_RTE_LIBRTE_HASH)-include +=3D >> >rte_fbk_hash.h >> >> >> diff --git a/lib/librte_hash/crc_arm64.h >> >b/lib/librte_hash/crc_arm64.h >> >> >> new file mode 100644 >> >> >> index 000000000..8e75f8297 >> >> > >> >> >Wouldn't that break 'make install T=3D...'? >> >> >> >> My bad I verified with meson and it was building fine. >> >> >> >> >As now rte_hash_crc.h includes not public headers (crc_x86.h, >etc.). >> >> >Same question about external apps, where they would get from >> >these >> >> >headers? >> >> >> >> I think in the next version we can directly have the arch specific >> >functions >> >> Implemented in rte_hash_crc.h. Since its pretty stable code and >> >overhead of extra >> >> ~120 lines. >> > >> >Ok... but why not then just leave arch specific headers, as they are >right >> >now? >> >What is wrong with current approach? >> >> The problem is if any application directly includes only >rte_crc_arm64.h >> (completely legal) it will break the build. > >But we can probably mark rte_crc_arm64.h as internal, and warn users >not to >include it directly (same for rte_crc_x86.h and any other arch specific >headers). Yes but I think merging them would be a cleaner, number of constructors wou= ld be=20 one and maybe we could select the best available algorithm on a given platf= orm when=20 application requests unsupported one. As Yipeng mentioned do you thing having a indirect call instead of runtime = branch be=20 depreciative in terms of performance? > >> >> Example: >> >> diff --git a/lib/librte_efd/rte_efd.c b/lib/librte_efd/rte_efd.c >> index 6a799556d..318670940 100644 >> --- a/lib/librte_efd/rte_efd.c >> +++ b/lib/librte_efd/rte_efd.c >> @@ -19,7 +19,7 @@ >> #include >> #include >> #include >> -#include >> +#include >> #include >> >> #include "rte_efd.h" >> (END) >> >> Causes: >> >> ../lib/librte_hash/rte_crc_arm64.h: In function >'rte_hash_crc_set_alg': >> ../lib/librte_hash/rte_crc_arm64.h:77:7: error: 'CRC32_ARM64' >undeclared (first use in this function) >> 77 | case CRC32_ARM64: >> | ^~~~~~~~~~~ >> ../lib/librte_hash/rte_crc_arm64.h:77:7: note: each undeclared >identifier is reported only once for each function it appears in >> ../lib/librte_hash/rte_crc_arm64.h:79:10: error: 'CRC32_SW' >undeclared (first use in this function) >> 79 | alg =3D CRC32_SW; >> | ^~~~~~~~ >> ../lib/librte_hash/rte_crc_arm64.h:82:3: error: 'crc32_alg' undeclared >(first use in this function) >> 82 | crc32_alg =3D alg; >> | ^~~~~~~~~ >> ../lib/librte_hash/rte_crc_arm64.h: In function >'rte_hash_crc_init_alg': >> ../lib/librte_hash/rte_crc_arm64.h:92:23: error: 'CRC32_ARM64' >undeclared (first use in this function) >> 92 | rte_hash_crc_set_alg(CRC32_ARM64); >> >> Thanks, >> Pavan. >> >>