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 A1559A0350; Mon, 11 May 2020 12:27:55 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7FE3D1C2DB; Mon, 11 May 2020 12:27:55 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 2EAE91C138 for ; Mon, 11 May 2020 12:27:53 +0200 (CEST) IronPort-SDR: GfmLR8YasiH6DzMu0NVlrfqtFOaTzk2QWrI6Lwt4KoAp+nTTX+mu+dyHFAGdzCwes6xDtq45Bd V/HwxNQBajeg== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 May 2020 03:27:47 -0700 IronPort-SDR: xK7hDhpYWjKK9hyhhFStmFIPP652Xn18zgLdLjabj82EXt8j2GglWIbPciT6v4hVe5250qShD9 huOHwpyvTgGA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,379,1583222400"; d="scan'208";a="463132210" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga005.fm.intel.com with ESMTP; 11 May 2020 03:27:47 -0700 Received: from orsmsx161.amr.corp.intel.com (10.22.240.84) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 May 2020 03:27:47 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by ORSMSX161.amr.corp.intel.com (10.22.240.84) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 May 2020 03:27:47 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.173) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 11 May 2020 03:27:46 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MP7HTR3wKBdP/uqjN/q+tjr0QgMGkWAcljycnAzufu1nSTpzK6UCJAPq7cgHDqdtC5eDiF+gAwJ19sBLobdoei1U72u2108102w7fjyg4x8H/HhbGF4x3MbsDkWqorxDi9vohkppO8u7suyNzDD3UFphV0Z8po0lkgcVnuvD7pPUO5k0WM11C462lcDeMJ1qLXq8WeNNHJolwB9aE+ud/sZ0fJ1dcCDJoSbKqIT8ZwkKowjjPh/oEp+KkppVGT1HsB6uYZDnapQ0y839YdVNUe0wSui5c6hSJP+ozFG5snoU4eqa1FY0T686rPvgI0/9dUg3GKGocPuMcqZDm1icUg== 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=Z3ofecQQ1k4Ew9J4UzCj7inNthlEl7NnUfC/1amfC4o=; b=LAwSp+xlr9wEtA3cqzx+IEN3uX8mz6WVte8KpsvTao+AKGKDT5wqfd2jlVU26c6aADlIAa9Kfvj+Xb07R3nchaDLHyDbiUwd9Pcg9zD+wGGd01YLMtE58wFR1gIWzzDYlTWnkDCeraalVPlrk0G6RcrTDvvC/ghGcXRQ8ZbuInTzucQoLg1HxjkMCrYf0OetAiY4OJuJkloucJYVXbwTYVutaSBYIhvxd64RQoAAOOBihuoeGqoD3Mnhv+yNhT5CSMpcnld2ivYzAIBmQlleJugCwCfLf0G9bywSQNTYm6wWDoY5C2PAoYOGfSbXRuaD21i1ZzKR7HqHBRMyw/wU3w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Z3ofecQQ1k4Ew9J4UzCj7inNthlEl7NnUfC/1amfC4o=; b=mBThiSFSVw25nDrNeT2yv+g834nDYFCRLjBqLUPDuEdzvmHM7LJoI9rbm/8OcmlOuwP9akhNp7P2pg0c3eQAucAhNEKWIPAusHxVZXs8PsTbaUYY+ojAl2XhClmsXuEMr/vebzfzRipzuwq3tcdpWafQk/Q4K5ZQOqEbS8WoNsY= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BYAPR11MB3479.namprd11.prod.outlook.com (2603:10b6:a03:1e::24) 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:27:45 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f8cb:58cd:e958:fff4%6]) with mapi id 15.20.2979.033; Mon, 11 May 2020 10:27:44 +0000 From: "Ananyev, Konstantin" To: Pavan Nikhilesh Bhagavatula , "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: AQHWHlDpPeIPCcs0i0ui77Z53FWYRaieMNWwgAPOQICAALYOIIAACp6AgAAAlhA= Date: Mon, 11 May 2020 10:27:44 +0000 Message-ID: References: <20200429180515.5704-1-pbhagavatula@marvell.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: marvell.com; dkim=none (message not signed) header.d=none;marvell.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.151.178] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3899eab7-acfa-46cc-a8ec-08d7f595f1ae x-ms-traffictypediagnostic: BYAPR11MB3479: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr 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: LbzDFimtNHKEhQQEJzwE5cVDJNL0W+Ceu8a45YE7f26QkxaU2Tk/xovz0XiswcsU0J3vfcGPYV7wlfrUTYvNqTbQBaYOCYPXfI3s7MSmgMVuwvol/oW7fhJ5VdZyeV/Sxoj/Y99cQPYjIHy8Dzkth/k9v06Q10Vtz6MaoLUss3IKxYSoPEmYk4HwzXYJZOITQpyn0le/8dtBHb+PvimLLn3MF+IlIOSCpZmxA/mxngyUhoYTdwrXcJZJTiBEjDdsDK8Vfa0EEIf5LXRrkeyUz5ezGDmM2GmBU/tjtgcrIAMoMytzty2BV2jAqUl7rNXQZ2R9NOPP9jHBZj93lE1/Zlk3J368y517qxnaFQ7xUNOuZAW5aMuIcWMlFirCxBUww3y7SQPsxFf2V0ZA8QGSV8LyMEir2ynhJlhNJg98+LR7ZDB8L1jIRRhYeDHp/Jf77sPfTeQfwdTxF/yNgwd4oRlB6ngtZiPFFI+ty3waQdNjSNbtbmAj9Ju4RA5ElMAFShNrPW+YihRmRRD9soc6CzZ/UZYzvTbpRS7ziGSFC04= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(396003)(39860400002)(136003)(376002)(366004)(346002)(33430700001)(316002)(7696005)(5660300002)(9686003)(55016002)(8676002)(76116006)(86362001)(33440700001)(8936002)(4326008)(52536014)(2906002)(110136005)(186003)(26005)(6506007)(66556008)(66446008)(478600001)(66946007)(66476007)(33656002)(71200400001)(64756008)(921003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: hxtk1I0zg/4W4jCNSQJRZ6UklKswIulhituGmHNRHU80k2EvyOC4vA10dAMH+WF5ZP3FJvr5+bHvTD77ZcjD0rwXo8kHh/zRwq6maVSgm6rt3QFWAVEMV4HuDmPcbFPgziAAkQRt6typ0BQVlQLiJsg9lsZWDKSekx0quN0niD0EiqfEIsgMsmxLE+Cr0/IacPWjrJnV/X+7AKa6bYNuVjmJfljAj2HsvBDWMXC3CJwzvpeh3O5c8qvl3TSVNkEDTd5wBnf7brt66rGvKtyB2ShwkCYkmAlf7RrFJ+EcbauVBO/T/O1NTcF1aIqzeYCGLWCJ7UBvCdAn29mDp8BLAlCTagTU5+WX0DJBV5DdIgmD2qeVVIxm5HAeqgCtU472PYrZmUVVqOgQymZk0I2gXd4N5anJNdcRQTaZXlStlzqgmqM36qMSD2LyWc9WbQxAdiCXmax1u2Y44wqZQflG+m4dV2euIXxejS24nY4qL4Mvm9CR923tTNcbw1jLdkGc Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 3899eab7-acfa-46cc-a8ec-08d7f595f1ae X-MS-Exchange-CrossTenant-originalarrivaltime: 11 May 2020 10:27:44.8523 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: udX+nb3W4+by1LdZzxtJxqUWNzVEFpVCGZhF3wdR+LnX64fuhK1IwKlvCGPhiqJvYYcpfWsxl/KKdsi2hkdiYDtw9YyDuhi1W6fqeS0G7u4= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3479 X-OriginatorOrg: intel.com 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" >=20 > >> >> 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` which > >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 rig= ht > >now? > >What is wrong with current approach? >=20 > 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 hea= ders).=20 >=20 > Example: >=20 > 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 >=20 > #include "rte_efd.h" > (END) >=20 > Causes: >=20 > ../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 (f= irst use in this function) > 79 | alg =3D CRC32_SW; > | ^~~~~~~~ > ../lib/librte_hash/rte_crc_arm64.h:82:3: error: 'crc32_alg' undeclared (f= irst 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); >=20 > Thanks, > Pavan. >=20 >=20