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 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 <dev@dpdk.org>; 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" <konstantin.ananyev@intel.com>
To: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>, "Jerin Jacob
 Kollanukkaran" <jerinj@marvell.com>, "thomas@monjalon.net"
 <thomas@monjalon.net>, "Wang, Yipeng1" <yipeng1.wang@intel.com>, "Gobriel,
 Sameh" <sameh.gobriel@intel.com>, "Richardson, Bruce"
 <bruce.richardson@intel.com>, Ruifeng Wang <ruifeng.wang@arm.com>
CC: "dev@dpdk.org" <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: <BYAPR11MB33015C0908201FB6F122C14E9AA10@BYAPR11MB3301.namprd11.prod.outlook.com>
References: <20200429180515.5704-1-pbhagavatula@marvell.com>
 <BYAPR11MB3301068EFA50ACA315A20A709AA20@BYAPR11MB3301.namprd11.prod.outlook.com>
 <BYAPR18MB2518F63F062EB5D02362457DDEA00@BYAPR18MB2518.namprd18.prod.outlook.com>
 <BYAPR11MB3301F33F0AE517A687A1496D9AA10@BYAPR11MB3301.namprd11.prod.outlook.com>
 <BYAPR18MB2518C998EB024C61A8923AF8DEA10@BYAPR18MB2518.namprd18.prod.outlook.com>
In-Reply-To: <BYAPR18MB2518C998EB024C61A8923AF8DEA10@BYAPR18MB2518.namprd18.prod.outlook.com>
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: <BYAPR11MB3479CC9A1DB5924FE75B8A259AA10@BYAPR11MB3479.namprd11.prod.outlook.com>
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 <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>


>=20
> >> >> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
> >> >>
> >> >> 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 <pbhagavatula@marvell.com>
> >> >> ---
> >> >>
> >> >>  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 <rte_memcpy.h>
>  #include <rte_ring.h>
>  #include <rte_jhash.h>
> -#include <rte_hash_crc.h>
> +#include <rte_crc_arm64.h>
>  #include <rte_tailq.h>
>=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