2017-03-15 19:03, Dumitrescu, Cristian: > ... <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.
I agree with you Cristian that the protocol layer must be in librte_net. But I think most of this patch is not protocol level. I think you agree with me that the code computing a "digest/signature for a given array of bytes" must go to librte_hash?
