From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
Subject: Re: [PATCH v2 1/2] librte_net: add crc init and
	compute	APIs
Date: Wed, 15 Mar 2017 19:09:25 +0000
Message-ID: <3EB4FA525960D640B5BDFFD6A3D89126527609CA@IRSMSX108.ger.corp.intel.com>
References: <1487969657-172541-2-git-send-email-jasvinder.singh@intel.com>
 <4909384.YypA4ZyVZF@xps13>
 <E115CCD9D858EF4F90C690B0DCB4D897476EABE5@IRSMSX108.ger.corp.intel.com>
 <6450918.dWRLMxV3EZ@xps13>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Cc: "Singh, Jasvinder" <jasvinder.singh@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>, "Doherty, Declan" <declan.doherty@intel.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>, "De Lara Guarch, Pablo"
 <pablo.de.lara.guarch@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
 by dpdk.org (Postfix) with ESMTP id 92FA61396
 for <dev@dpdk.org>; Wed, 15 Mar 2017 20:09:29 +0100 (CET)
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>



> -----Original Message-----
> From: Dumitrescu, Cristian
> Sent: Wednesday, March 15, 2017 7:04 PM
> To: 'Thomas Monjalon' <thomas.monjalon@6wind.com>; De Lara Guarch,
> Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Singh, Jasvinder <jasvinder.singh@intel.com>; dev@dpdk.org; Doherty,
> Declan <declan.doherty@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] librte_net: add crc init and compu=
te
> APIs
>=20
> ... <snip>
>=20
> > > > > > I think it should be in librte_hash.
> > > > > >
> > > > > > Please check lib/librte_hash/rte_hash_crc.h
> > > > >
> > > > > Is it good to include payload crc calculation in hash library as =
I see all
> > hash
> > > > related functionality there?
> > > >
> > > > I think yes. Pablo?
> > >
> > > I think this doesn't belong in the hash library. These new functions
> calculate
> > CRC, but not as a hash function.
> >
> > Can't we say that a CRC is a hash? What is a hash?
> > A function generating the same output bytes from given input bytes.
> >
> > I think you must separate hash functions and hash table management.
> >
>=20
> The fact that CRC32 instruction is opportunistically used to compute a ha=
sh
> digest/signature for load balancing (affinity-based) or hash tables (flow
> tables, ARP cache, etc) does not mean that all the code that uses CRC32
> instruction should be placed in librte_hash.
>=20
> The purpose of the hash functions in librte_hash is to compute a
> digest/signature for a given array of bytes (the key) as fast as possible=
. Any
> hash function that produces a hash signature with good uniform distributi=
on
> in a small amount of cycles belongs here, including those opportunistical=
ly
> using specialized CPU instructions such as CRC32 (or XOR, AESNI, etc).
>=20
> The code proposed in this patch is used to compute packet fields for vari=
ous
> protocols that have a CRC field, such as FCS of Ethernet frames, etc.
> according to the relevant standard (IEEE 802, others). It is an utility t=
o be used
> for implementing protocol-level functionality for various protocols from =
the
> network stack, similar to e.g. IP or UDP checksum. If we were to add an I=
P or
> UDCP checksum calculator, would you put it in librte_hash?
>=20
> The code from this patch is never going to be used to compute a fast
> signature/digest. Typically this CRC is computed over the entire frame/pa=
cket
> rather than just selected fields from the packet representing the applica=
tion-
> specific flow key. Also note that the signature produced by CRC32 hash
> function from librte_hash is actually not the correct Cyclic Redundancy C=
heck
> of that array of bytes (or, for math guys, of the associated polynomial),=
 it is
> just a partial/intermediate value.
>=20
> Therefore, I suggest placing this code into: librte_ether (given that it =
can be
> used to compute Ethernet FCS), or librte_net, or librte_crc. Definitely n=
ot in
> librte_hash.
>=20

Sorry, my bad on librte_ether: the rte_ether.h where Ethernet frame is defi=
ned is located in librte_net, not librte_ether, so librte_ether should not =
be on the above list. Therefore, my suggestion is: librte_net or a new libr=
ary librte_crc.

> Regards,
> Cristian