[dpdk-dev] [PATCH v4 0/6] update jhash function

[De Lara Guarch, Pablo]

Hi Neil,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Tuesday, May 12, 2015 4:33 PM
> To: De Lara Guarch, Pablo
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 0/6] update jhash function
> 
> On Tue, May 12, 2015 at 12:02:32PM +0100, Pablo de Lara wrote:
> > Jenkins hash function was developed originally in 1996,
> > and was integrated in first versions of DPDK.
> > The function has been improved in 2006,
> > achieving up to 60% better performance, compared to the original one.
> >
> > This patchset updates the current jhash in DPDK,
> > including two new functions that generate two hashes from a single key.
> >
> > It also separates the existing hash function performance tests to
> > another file, to make it quicker to run.
> >
> > changes in v4:
> > - Simplify key alignment checks
> > - Include missing x86 arch check
> >
> > changes in v3:
> >
> > - Update rte_jhash_1word, rte_jhash_2words and rte_jhash_3words
> >   functions
> >
> > changes in v2:
> >
> > - Split single commit in three commits, one that updates the existing
> functions
> >   and another that adds two new functions and use one of those functions
> >   as a base to be called by the other ones.
> > - Remove some unnecessary ifdefs in the code.
> > - Add new macros to help on the reutilization of constants
> > - Separate hash function performance tests to another file
> >   and improve cycle measurements.
> > - Rename existing function rte_jhash2 to rte_jhash_32b
> >   (something more meaninful) and mark rte_jhash2 as
> >   deprecated
> >
> > Pablo de Lara (6):
> >   test/hash: move hash function perf tests to separate file
> >   test/hash: improve accuracy on cycle measurements
> >   hash: update jhash function with the latest available
> >   hash: add two new functions to jhash library
> >   hash: remove duplicated code
> >   hash: rename rte_jhash2 to rte_jhash_32b
> >
> >  app/test/Makefile               |    1 +
> >  app/test/test_func_reentrancy.c |    2 +-
> >  app/test/test_hash.c            |    4 +-
> >  app/test/test_hash_func_perf.c  |  145 +++++++++++++++++
> >  app/test/test_hash_perf.c       |   71 +--------
> >  lib/librte_hash/rte_jhash.h     |  338 +++++++++++++++++++++++++++++-
> ---------
> >  6 files changed, 402 insertions(+), 159 deletions(-)
> >  create mode 100644 app/test/test_hash_func_perf.c
> >
> > --
> > 1.7.4.1
> >
> >
> did you run this through the ABI checker?  I see you're removing several
> symbols
> that will likely need to go through the ABI deprecation process.
> 
> Neil

I had not run it, but I just did. I see no problems on librte_hash
(but I see some on rte_ethdev.h, due to another commit).

Anyway, I renamed two functions to be more meaningful, but those functions are "static inline", 
so I am not sure exactly what the deprecation process is for those.
What I did was leaving the original function that calls the same function as the new renamed one,
but adds a line warning that the functions is deprecated.

Is that OK or should I do it differently?

Thanks!
Pablo