> -----Original Message----- > From: Dumitrescu, Cristian > Sent: Wednesday, March 15, 2017 7:04 PM > To: 'Thomas Monjalon' <thomas.monja...@6wind.com>; De Lara Guarch, > Pablo <pablo.de.lara.gua...@intel.com> > Cc: Singh, Jasvinder <jasvinder.si...@intel.com>; dev@dpdk.org; Doherty, > Declan <declan.dohe...@intel.com> > Subject: RE: [dpdk-dev] [PATCH v2 1/2] librte_net: add crc init and compute > APIs > > ... <snip> > > > > > > > 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. > > > > The fact that CRC32 instruction is opportunistically used to compute a hash > 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. > > 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 distribution > in a small amount of cycles belongs here, including those opportunistically > using specialized CPU instructions such as CRC32 (or XOR, AESNI, etc). > > The code proposed in this patch is used to compute packet fields for various > 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 to 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 IP or > UDCP checksum calculator, would you put it in librte_hash? > > 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/packet > rather than just selected fields from the packet representing the application- > specific flow key. Also note that the signature produced by CRC32 hash > function from librte_hash is actually not the correct Cyclic Redundancy Check > of that array of bytes (or, for math guys, of the associated polynomial), it > is > just a partial/intermediate value. > > 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 not in > librte_hash. >
Sorry, my bad on librte_ether: the rte_ether.h where Ethernet frame is defined 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 library librte_crc. > Regards, > Cristian
