Re: [PATCH v2 1/2] librte_net: add crc init and compute APIs

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2017-03-08 11:08, De Lara Guarch, Pablo:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2017-03-02 13:03, Singh, Jasvinder:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2017-02-28 12:08, Jasvinder Singh:
> > > > >  lib/librte_net/rte_net_crc.c       | 664 +++++++++++++++++++++++++++++++++++++
> > > > >  lib/librte_net/rte_net_crc.h       | 101 ++++++
> > > >
> > > > 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.

> Yes, CRC can be used as hash function (in fact, it is used as such in the hash library,
> the CRC32C version, and I assume that's why it is in there), but its use is much broader
> (its main purpose is not to be a hash function, but for data error detection, for any data).

The librte_hash has several hash functions, including CRC32C and Toeplitz.

> Therefore, I would suggest either creating a separate library for this, if we want to use this as a broader use,
> or leave it in net library, if we want to focus on calculating CRC for Ethernet frames.

I don't think Toeplitz should go in librte_net.
That's why I suggest to keep every kind of hash functions in librte_hash.

> Regarding to the CRC that we have in the hash library, if we go for a separate library,
> we could move that function there, but then it would have to follow the function prototype of a hash function,
> defined in the hash library.