[dpdk-dev,RFC] hash/lpm: return NULL if the object exists

Message ID 1458044745-32764-1-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Olivier Matz March 15, 2016, 12:25 p.m. UTC
  Seen by trying to fix the func_reentrancy autotest. The test
was doing the following on several cores in parallel:

  name = "common_name";
  do several times {
      obj = allocate_an_object(name)   // obj = ring, mempool, hash, lpm, ...
      if (obj == NULL && lookup(name) == NULL)
          return TEST_FAIL;
  }

Issues:

1/ rings, mempools, hashs API are not coherent
   rings and mempool return NULL if the object does not exist
   hash and lpm return an object that was allocated allocated if
   it already was allocated

2/ The hash/lpm API looks dangerous: when an object is returned,
   the user does not know if it should be freed or not (no refcnt)

3/ There are some possible race conditions in cuckoo_hash as the
   lock is not held in rte_hash_create(). We could find some cases
   where NULL is returned when the object already exists (ex: when
   rte_ring_create() fails).

This patch tries to rationalize the APIs of lpm and hash.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test/test_func_reentrancy.c   | 31 +++++++++++++++++++++----------
 app/test/test_lpm6.c              |  2 +-
 lib/librte_hash/rte_cuckoo_hash.c |  2 +-
 lib/librte_hash/rte_fbk_hash.c    |  4 +++-
 lib/librte_lpm/rte_lpm.c          |  8 ++++++--
 lib/librte_lpm/rte_lpm6.c         |  4 +++-
 6 files changed, 35 insertions(+), 16 deletions(-)
  

Comments

Olivier Matz March 25, 2016, 10:32 a.m. UTC | #1
Hi Bruce,

On 03/15/2016 01:25 PM, Olivier Matz wrote:
> Seen by trying to fix the func_reentrancy autotest. The test
> was doing the following on several cores in parallel:
> 
>   name = "common_name";
>   do several times {
>       obj = allocate_an_object(name)   // obj = ring, mempool, hash, lpm, ...
>       if (obj == NULL && lookup(name) == NULL)
>           return TEST_FAIL;
>   }
> 
> Issues:
> 
> 1/ rings, mempools, hashs API are not coherent
>    rings and mempool return NULL if the object does not exist
>    hash and lpm return an object that was allocated allocated if
>    it already was allocated
> 
> 2/ The hash/lpm API looks dangerous: when an object is returned,
>    the user does not know if it should be freed or not (no refcnt)
> 
> 3/ There are some possible race conditions in cuckoo_hash as the
>    lock is not held in rte_hash_create(). We could find some cases
>    where NULL is returned when the object already exists (ex: when
>    rte_ring_create() fails).
> 
> This patch tries to rationalize the APIs of lpm and hash.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Sorry, I forgot to CC you in the first mail. Do you have any opinion
about this rfc patch?

Thanks,
Olivier
  
Bruce Richardson March 25, 2016, 10:45 a.m. UTC | #2
On Fri, Mar 25, 2016 at 11:32:47AM +0100, Olivier Matz wrote:
> Hi Bruce,
> 
> On 03/15/2016 01:25 PM, Olivier Matz wrote:
> > Seen by trying to fix the func_reentrancy autotest. The test
> > was doing the following on several cores in parallel:
> > 
> >   name = "common_name";
> >   do several times {
> >       obj = allocate_an_object(name)   // obj = ring, mempool, hash, lpm, ...
> >       if (obj == NULL && lookup(name) == NULL)
> >           return TEST_FAIL;
> >   }
> > 
> > Issues:
> > 
> > 1/ rings, mempools, hashs API are not coherent
> >    rings and mempool return NULL if the object does not exist
> >    hash and lpm return an object that was allocated allocated if
> >    it already was allocated
> > 
> > 2/ The hash/lpm API looks dangerous: when an object is returned,
> >    the user does not know if it should be freed or not (no refcnt)
> > 
> > 3/ There are some possible race conditions in cuckoo_hash as the
> >    lock is not held in rte_hash_create(). We could find some cases
> >    where NULL is returned when the object already exists (ex: when
> >    rte_ring_create() fails).
> > 
> > This patch tries to rationalize the APIs of lpm and hash.
> > 
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Sorry, I forgot to CC you in the first mail. Do you have any opinion
> about this rfc patch?
> 
> Thanks,
> Olivier
Hi Olivier,

the idea looks good, since an object already existing is an error condition on
create. One small change to the libs I'd suggest is to set rte_errno to 
EEXIST before exit, so that the error reason is known to the app.

Regards,
/Bruce
  
Olivier Matz March 30, 2016, 3:30 p.m. UTC | #3
Seen while trying to fix the func_reentrancy autotest. The
series addresses several issues:

1/ Hash and lpm return a pointer to an existing object if the user requests the
   creation with an already existing name. This look dangerous: when an object
   is returned, the user does not know if it should be freed or not.

2/ There is a race condition in cuckoo_hash as the lock is not held in
   rte_hash_create(). We could find some cases where NULL is returned when the
   object already exists (ex: when rte_ring_create() fails).

3/ There is a race condition func_reentrancy that can fail even if the tested
   API behaves correctly.


Changes since RFC:

- split the patch in 4 patches
- on error, set rte_errno to EEXIST when relevant
- fix locking in cuckoo_hash creation

Olivier Matz (4):
  lpm: allocation of an existing object should fail
  hash: allocation of an existing object should fail
  hash: keep the list locked at creation
  autotest: fix func reentrancy

 app/test/test_func_reentrancy.c   | 31 +++++++++++++++++++++----------
 app/test/test_lpm6.c              |  2 +-
 lib/librte_hash/rte_cuckoo_hash.c | 18 +++++++++++++-----
 lib/librte_hash/rte_fbk_hash.c    |  5 ++++-
 lib/librte_lpm/rte_lpm.c          | 10 ++++++++--
 lib/librte_lpm/rte_lpm6.c         |  5 ++++-
 6 files changed, 51 insertions(+), 20 deletions(-)
  
Olivier Matz March 31, 2016, 7:35 a.m. UTC | #4
On 03/30/2016 05:30 PM, Olivier Matz wrote:
> Seen while trying to fix the func_reentrancy autotest. The
> series addresses several issues:
> 
> 1/ Hash and lpm return a pointer to an existing object if the user requests the
>    creation with an already existing name. This look dangerous: when an object
>    is returned, the user does not know if it should be freed or not.
> 
> 2/ There is a race condition in cuckoo_hash as the lock is not held in
>    rte_hash_create(). We could find some cases where NULL is returned when the
>    object already exists (ex: when rte_ring_create() fails).
> 
> 3/ There is a race condition func_reentrancy that can fail even if the tested
>    API behaves correctly.
> 
> 
> Changes since RFC:
> 
> - split the patch in 4 patches
> - on error, set rte_errno to EEXIST when relevant
> - fix locking in cuckoo_hash creation
> 
> Olivier Matz (4):
>   lpm: allocation of an existing object should fail
>   hash: allocation of an existing object should fail
>   hash: keep the list locked at creation
>   autotest: fix func reentrancy
> 
>  app/test/test_func_reentrancy.c   | 31 +++++++++++++++++++++----------
>  app/test/test_lpm6.c              |  2 +-
>  lib/librte_hash/rte_cuckoo_hash.c | 18 +++++++++++++-----
>  lib/librte_hash/rte_fbk_hash.c    |  5 ++++-
>  lib/librte_lpm/rte_lpm.c          | 10 ++++++++--
>  lib/librte_lpm/rte_lpm6.c         |  5 ++++-
>  6 files changed, 51 insertions(+), 20 deletions(-)
> 

Self-nack, there is a typo in rte_cuckoo_hash.c breaking the
compilation.
  
Olivier Matz April 5, 2016, 7:35 a.m. UTC | #5
Seen while trying to fix the func_reentrancy autotest. The
series addresses several issues:

1/ Hash and lpm return a pointer to an existing object if the user requests the
   creation with an already existing name. This look dangerous: when an object
   is returned, the user does not know if it should be freed or not.

2/ There is a race condition in cuckoo_hash as the lock is not held in
   rte_hash_create(). We could find some cases where NULL is returned when the
   object already exists (ex: when rte_ring_create() fails).

3/ There is a race condition func_reentrancy that can fail even if the tested
   API behaves correctly.


RFC -> v1:

- split the patch in 4 patches
- on error, set rte_errno to EEXIST when relevant
- fix locking in cuckoo_hash creation

v1 -> v2:

- fix compilation issue in cuckoo hash
- update the hash test to conform to the new behavior
- rework locking modification in cuckoo_hash
- passed autotests: hash, lpm, lpm6, func_reentrancy

Olivier Matz (4):
  lpm: allocation of an existing object should fail
  hash: allocation of an existing object should fail
  hash: keep the list locked at creation
  autotest: fix func reentrancy

 app/test/test_func_reentrancy.c   | 31 +++++++++++------
 app/test/test_hash.c              | 65 +++++++++++++-----------------------
 app/test/test_lpm6.c              |  2 +-
 lib/librte_hash/rte_cuckoo_hash.c | 70 ++++++++++++++++++++++-----------------
 lib/librte_hash/rte_fbk_hash.c    |  5 ++-
 lib/librte_lpm/rte_lpm.c          | 10 ++++--
 lib/librte_lpm/rte_lpm6.c         |  5 ++-
 7 files changed, 101 insertions(+), 87 deletions(-)
  
Olivier Matz April 5, 2016, 11:53 a.m. UTC | #6
Seen while trying to fix the func_reentrancy autotest. The
series addresses several issues:

1/ Hash and lpm return a pointer to an existing object if the user requests the
   creation with an already existing name. This look dangerous: when an object
   is returned, the user does not know if it should be freed or not.

2/ There is a race condition in cuckoo_hash as the lock is not held in
   rte_hash_create(). We could find some cases where NULL is returned when the
   object already exists (ex: when rte_ring_create() fails).

3/ There is a race condition func_reentrancy that can fail even if the tested
   API behaves correctly.


RFC -> v1:

- split the patch in 4 patches
- on error, set rte_errno to EEXIST when relevant
- fix locking in cuckoo_hash creation

v1 -> v2:

- fix compilation issue in cuckoo hash
- update the hash test to conform to the new behavior
- rework locking modification in cuckoo_hash
- passed autotests: hash, lpm, lpm6, func_reentrancy

v2 -> v3:

- rebase against head
- add "Fixes:" in commit messages
- properly set lpm or hash pointers to NULL on error before returning

Olivier Matz (4):
  lpm: allocation of an existing object should fail
  hash: allocation of an existing object should fail
  hash: keep the list locked at creation
  autotest: fix func reentrancy

 app/test/test_func_reentrancy.c   | 31 +++++++++++------
 app/test/test_hash.c              | 65 +++++++++++++----------------------
 app/test/test_lpm6.c              |  2 +-
 lib/librte_hash/rte_cuckoo_hash.c | 72 +++++++++++++++++++++++----------------
 lib/librte_hash/rte_fbk_hash.c    |  5 ++-
 lib/librte_lpm/rte_lpm.c          | 10 ++++--
 lib/librte_lpm/rte_lpm6.c         |  5 ++-
 7 files changed, 103 insertions(+), 87 deletions(-)
  
Thomas Monjalon April 5, 2016, 3:51 p.m. UTC | #7
2016-04-05 13:53, Olivier Matz:
> Seen while trying to fix the func_reentrancy autotest. The
> series addresses several issues:
> 
> 1/ Hash and lpm return a pointer to an existing object if the user requests the
>    creation with an already existing name. This look dangerous: when an object
>    is returned, the user does not know if it should be freed or not.
> 
> 2/ There is a race condition in cuckoo_hash as the lock is not held in
>    rte_hash_create(). We could find some cases where NULL is returned when the
>    object already exists (ex: when rte_ring_create() fails).
> 
> 3/ There is a race condition func_reentrancy that can fail even if the tested
>    API behaves correctly.

Pablo, Bruce,
What do you think of these fixes for 16.04?
  
De Lara Guarch, Pablo April 6, 2016, 10:11 a.m. UTC | #8
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, April 05, 2016 4:52 PM
> To: Richardson, Bruce; De Lara Guarch, Pablo
> Cc: dev@dpdk.org; Olivier Matz
> Subject: Re: [dpdk-dev] [PATCH v3 0/4] fix creation of duplicate lpm and hash
> 
> 2016-04-05 13:53, Olivier Matz:
> > Seen while trying to fix the func_reentrancy autotest. The
> > series addresses several issues:
> >
> > 1/ Hash and lpm return a pointer to an existing object if the user requests
> the
> >    creation with an already existing name. This look dangerous: when an
> object
> >    is returned, the user does not know if it should be freed or not.
> >
> > 2/ There is a race condition in cuckoo_hash as the lock is not held in
> >    rte_hash_create(). We could find some cases where NULL is returned
> when the
> >    object already exists (ex: when rte_ring_create() fails).
> >
> > 3/ There is a race condition func_reentrancy that can fail even if the tested
> >    API behaves correctly.
> 
> Pablo, Bruce,
> What do you think of these fixes for 16.04?

I was reviewing them yesterday, but couldn't finish in time. I will do now.
  
De Lara Guarch, Pablo April 6, 2016, 10:32 a.m. UTC | #9
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Tuesday, April 05, 2016 12:54 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce; De Lara Guarch, Pablo
> Subject: [dpdk-dev] [PATCH v3 0/4] fix creation of duplicate lpm and hash
> 
> Seen while trying to fix the func_reentrancy autotest. The
> series addresses several issues:
> 
> 1/ Hash and lpm return a pointer to an existing object if the user requests the
>    creation with an already existing name. This look dangerous: when an
> object
>    is returned, the user does not know if it should be freed or not.
> 
> 2/ There is a race condition in cuckoo_hash as the lock is not held in
>    rte_hash_create(). We could find some cases where NULL is returned when
> the
>    object already exists (ex: when rte_ring_create() fails).
> 
> 3/ There is a race condition func_reentrancy that can fail even if the tested
>    API behaves correctly.
> 
> 
> RFC -> v1:
> 
> - split the patch in 4 patches
> - on error, set rte_errno to EEXIST when relevant
> - fix locking in cuckoo_hash creation
> 
> v1 -> v2:
> 
> - fix compilation issue in cuckoo hash
> - update the hash test to conform to the new behavior
> - rework locking modification in cuckoo_hash
> - passed autotests: hash, lpm, lpm6, func_reentrancy
> 
> v2 -> v3:
> 
> - rebase against head
> - add "Fixes:" in commit messages
> - properly set lpm or hash pointers to NULL on error before returning
> 
> Olivier Matz (4):
>   lpm: allocation of an existing object should fail
>   hash: allocation of an existing object should fail
>   hash: keep the list locked at creation
>   autotest: fix func reentrancy
> 
>  app/test/test_func_reentrancy.c   | 31 +++++++++++------
>  app/test/test_hash.c              | 65 +++++++++++++----------------------
>  app/test/test_lpm6.c              |  2 +-
>  lib/librte_hash/rte_cuckoo_hash.c | 72 +++++++++++++++++++++++------------
> ----
>  lib/librte_hash/rte_fbk_hash.c    |  5 ++-
>  lib/librte_lpm/rte_lpm.c          | 10 ++++--
>  lib/librte_lpm/rte_lpm6.c         |  5 ++-
>  7 files changed, 103 insertions(+), 87 deletions(-)
> 
> --
> 2.1.4

I wonder if you should include something in release notes.
We are fixing the API, so I guess we don't need to follow the deprecation process, but at least a note in the documentation?

Apart from that,

Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Thanks!
  
Olivier Matz April 6, 2016, 11:14 a.m. UTC | #10
On 04/06/2016 12:32 PM, De Lara Guarch, Pablo wrote:
> 
> I wonder if you should include something in release notes.
> We are fixing the API, so I guess we don't need to follow the deprecation process, but at least a note in the documentation?

Good idea, I'll send a v4 with the deprecation notice.

> Apart from that,
> 
> Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Thank you for the review.

Olivier
  
De Lara Guarch, Pablo April 6, 2016, 11:20 a.m. UTC | #11
> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, April 06, 2016 12:15 PM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v3 0/4] fix creation of duplicate lpm and hash
> 
> 
> 
> On 04/06/2016 12:32 PM, De Lara Guarch, Pablo wrote:
> >
> > I wonder if you should include something in release notes.
> > We are fixing the API, so I guess we don't need to follow the deprecation
> process, but at least a note in the documentation?
> 
> Good idea, I'll send a v4 with the deprecation notice.

Well, not sure if this needs a deprecation notice.
I mean, it is an API fix: yes, this is changing what the function returns
in a particular situation (when the hash/lpm already exists) ,
but it was going against the API documentation, so a deprecation notice should not be necessary.
(just my opinion, I could be quite wrong here :P).

I was thinking more on adding a note in Resolved issues.

Thanks,
Pablo
> 
> > Apart from that,
> >
> > Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Thank you for the review.
> 
> Olivier
  
Olivier Matz April 6, 2016, 11:57 a.m. UTC | #12
On 04/06/2016 01:20 PM, De Lara Guarch, Pablo wrote:
>> On 04/06/2016 12:32 PM, De Lara Guarch, Pablo wrote:
>>>
>>> I wonder if you should include something in release notes.
>>> We are fixing the API, so I guess we don't need to follow the deprecation
>> process, but at least a note in the documentation?
>>
>> Good idea, I'll send a v4 with the deprecation notice.
> 
> Well, not sure if this needs a deprecation notice.
> I mean, it is an API fix: yes, this is changing what the function returns
> in a particular situation (when the hash/lpm already exists) ,
> but it was going against the API documentation, so a deprecation notice should not be necessary.
> (just my opinion, I could be quite wrong here :P).
> 
> I was thinking more on adding a note in Resolved issues.

Yes, agree, it's a bug fix.

Another argument to not follow the API change process is that
the initial behavior was to return EEXIST, but it was changed
by this commit:

  http://dpdk.org/browse/dpdk/commit/?id=916e4f4f4e

By the way, the "Fixes:" line was not referencing this commit
in the v3, I'll also change that in v4.

Thanks,
Olivier
  
Olivier Matz April 6, 2016, 1:27 p.m. UTC | #13
Seen while trying to fix the func_reentrancy autotest. The
series addresses several issues:

1/ Hash and lpm return a pointer to an existing object if the user requests the
   creation with an already existing name. This look dangerous: when an object
   is returned, the user does not know if it should be freed or not.

2/ There is a race condition in cuckoo_hash as the lock is not held in
   rte_hash_create(). We could find some cases where NULL is returned when the
   object already exists (ex: when rte_ring_create() fails).

3/ There is a race condition func_reentrancy that can fail even if the tested
   API behaves correctly.


RFC -> v1:

- split the patch in 4 patches
- on error, set rte_errno to EEXIST when relevant
- fix locking in cuckoo_hash creation

v1 -> v2:

- fix compilation issue in cuckoo hash
- update the hash test to conform to the new behavior
- rework locking modification in cuckoo_hash
- passed autotests: hash, lpm, lpm6, func_reentrancy

v2 -> v3:

- rebase against head
- add "Fixes:" in commit messages
- properly set lpm or hash pointers to NULL on error before returning

v3 -> v4:

- add entries in the release note to explain the API changes
  in lpm and hash
- correct the "Fixes:" line in the first 2 patches

Olivier Matz (4):
  lpm: allocation of an existing object should fail
  hash: allocation of an existing object should fail
  hash: keep the list locked at creation
  autotest: fix func reentrancy

 app/test/test_func_reentrancy.c        | 31 ++++++++++-----
 app/test/test_hash.c                   | 65 +++++++++++-------------------
 app/test/test_lpm6.c                   |  2 +-
 doc/guides/rel_notes/release_16_04.rst | 17 ++++++++
 lib/librte_hash/rte_cuckoo_hash.c      | 72 ++++++++++++++++++++--------------
 lib/librte_hash/rte_fbk_hash.c         |  5 ++-
 lib/librte_lpm/rte_lpm.c               | 10 ++++-
 lib/librte_lpm/rte_lpm6.c              |  5 ++-
 8 files changed, 120 insertions(+), 87 deletions(-)
  
Thomas Monjalon April 6, 2016, 3:31 p.m. UTC | #14
2016-04-06 15:27, Olivier Matz:
> Seen while trying to fix the func_reentrancy autotest. The
> series addresses several issues:
> 
> 1/ Hash and lpm return a pointer to an existing object if the user requests the
>    creation with an already existing name. This look dangerous: when an object
>    is returned, the user does not know if it should be freed or not.
> 
> 2/ There is a race condition in cuckoo_hash as the lock is not held in
>    rte_hash_create(). We could find some cases where NULL is returned when the
>    object already exists (ex: when rte_ring_create() fails).
> 
> 3/ There is a race condition func_reentrancy that can fail even if the tested
>    API behaves correctly.

Applied, thanks
  

Patch

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 5d09296..300a3bc 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -83,6 +83,7 @@  typedef void (*case_clean_t)(unsigned lcore_id);
 
 #define MAX_LCORES	RTE_MAX_MEMZONE / (MAX_ITER_TIMES * 4U)
 
+static rte_atomic32_t obj_count = RTE_ATOMIC32_INIT(0);
 static rte_atomic32_t synchro = RTE_ATOMIC32_INIT(0);
 
 #define WAIT_SYNCHRO_FOR_SLAVES()   do{ \
@@ -100,6 +101,7 @@  test_eal_init_once(__attribute__((unused)) void *arg)
 
 	WAIT_SYNCHRO_FOR_SLAVES();
 
+	rte_atomic32_set(&obj_count, 1); /* silent the check in the caller */
 	if (rte_eal_init(0, NULL) != -1)
 		return -1;
 
@@ -122,8 +124,8 @@  ring_create_lookup(__attribute__((unused)) void *arg)
 	/* create the same ring simultaneously on all threads */
 	for (i = 0; i < MAX_ITER_TIMES; i++) {
 		rp = rte_ring_create("fr_test_once", 4096, SOCKET_ID_ANY, 0);
-		if ((NULL == rp) && (rte_ring_lookup("fr_test_once") == NULL))
-			return -1;
+		if (rp != NULL)
+			rte_atomic32_inc(&obj_count);
 	}
 
 	/* create/lookup new ring several times */
@@ -172,8 +174,8 @@  mempool_create_lookup(__attribute__((unused)) void *arg)
 					NULL, NULL,
 					my_obj_init, NULL,
 					SOCKET_ID_ANY, 0);
-		if ((NULL == mp) && (rte_mempool_lookup("fr_test_once") == NULL))
-			return -1;
+		if (mp != NULL)
+			rte_atomic32_inc(&obj_count);
 	}
 
 	/* create/lookup new ring several times */
@@ -238,8 +240,8 @@  hash_create_free(__attribute__((unused)) void *arg)
 	hash_params.name = "fr_test_once";
 	for (i = 0; i < MAX_ITER_TIMES; i++) {
 		handle = rte_hash_create(&hash_params);
-		if ((NULL == handle) && (rte_hash_find_existing("fr_test_once") == NULL))
-			return -1;
+		if (handle != NULL)
+			rte_atomic32_inc(&obj_count);
 	}
 
 	/* create mutiple times simultaneously */
@@ -306,8 +308,8 @@  fbk_create_free(__attribute__((unused)) void *arg)
 	fbk_params.name = "fr_test_once";
 	for (i = 0; i < MAX_ITER_TIMES; i++) {
 		handle = rte_fbk_hash_create(&fbk_params);
-		if ((NULL == handle) && (rte_fbk_hash_find_existing("fr_test_once") == NULL))
-			return -1;
+		if (handle != NULL)
+			rte_atomic32_inc(&obj_count);
 	}
 
 	/* create mutiple fbk tables simultaneously */
@@ -372,8 +374,8 @@  lpm_create_free(__attribute__((unused)) void *arg)
 	/* create the same lpm simultaneously on all threads */
 	for (i = 0; i < MAX_ITER_TIMES; i++) {
 		lpm = rte_lpm_create("fr_test_once",  SOCKET_ID_ANY, &config);
-		if ((NULL == lpm) && (rte_lpm_find_existing("fr_test_once") == NULL))
-			return -1;
+		if (lpm != NULL)
+			rte_atomic32_inc(&obj_count);
 	}
 
 	/* create mutiple fbk tables simultaneously */
@@ -432,10 +434,12 @@  launch_test(struct test_case *pt_case)
 	unsigned lcore_id;
 	unsigned cores_save = rte_lcore_count();
 	unsigned cores = RTE_MIN(cores_save, MAX_LCORES);
+	unsigned count;
 
 	if (pt_case->func == NULL)
 		return -1;
 
+	rte_atomic32_set(&obj_count, 0);
 	rte_atomic32_set(&synchro, 0);
 
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
@@ -462,6 +466,13 @@  launch_test(struct test_case *pt_case)
 			pt_case->clean(lcore_id);
 	}
 
+	count = rte_atomic32_read(&obj_count);
+	if (count != 1) {
+		printf("%s: common object allocated %d times (should be 1)\n",
+			pt_case->name, count);
+		ret = -1;
+	}
+
 	return ret;
 }
 
diff --git a/app/test/test_lpm6.c b/app/test/test_lpm6.c
index 1f88d7a..b464342 100644
--- a/app/test/test_lpm6.c
+++ b/app/test/test_lpm6.c
@@ -222,7 +222,7 @@  test1(void)
 
 	/* rte_lpm6_create: lpm name == LPM2 */
 	lpm3 = rte_lpm6_create("LPM1", SOCKET_ID_ANY, &config);
-	TEST_LPM_ASSERT(lpm3 == lpm1);
+	TEST_LPM_ASSERT(lpm3 == NULL);
 
 	rte_lpm6_free(lpm1);
 	rte_lpm6_free(lpm2);
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 71b5b76..857e152 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -231,7 +231,7 @@  rte_hash_create(const struct rte_hash_parameters *params)
 	/* Guarantee there's no existing */
 	h = rte_hash_find_existing(params->name);
 	if (h != NULL)
-		return h;
+		return NULL;
 
 	te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
 	if (te == NULL) {
diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c
index 8752a47..ea854f1 100644
--- a/lib/librte_hash/rte_fbk_hash.c
+++ b/lib/librte_hash/rte_fbk_hash.c
@@ -140,8 +140,10 @@  rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
 		if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)
 			break;
 	}
-	if (te != NULL)
+	if (te != NULL) {
+		ht = NULL;
 		goto exit;
+	}
 
 	te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0);
 	if (te == NULL) {
diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index ccaaa2a..dd62f9b 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -209,8 +209,10 @@  rte_lpm_create_v20(const char *name, int socket_id, int max_rules,
 		if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
 			break;
 	}
-	if (te != NULL)
+	if (te != NULL) {
+		lpm = NULL;
 		goto exit;
+	}
 
 	/* allocate tailq entry */
 	te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
@@ -280,8 +282,10 @@  rte_lpm_create_v1604(const char *name, int socket_id,
 		if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
 			break;
 	}
-	if (te != NULL)
+	if (te != NULL) {
+		lpm = NULL;
 		goto exit;
+	}
 
 	/* allocate tailq entry */
 	te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index 6c2b293..4e9f2d0 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -182,8 +182,10 @@  rte_lpm6_create(const char *name, int socket_id,
 		if (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)
 			break;
 	}
-	if (te != NULL)
+	if (te != NULL) {
+		lpm = NULL;
 		goto exit;
+	}
 
 	/* allocate tailq entry */
 	te = rte_zmalloc("LPM6_TAILQ_ENTRY", sizeof(*te), 0);