From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by dpdk.org (Postfix, from userid 33) id 3DDFB5F0D; Tue, 30 Apr 2019 11:03:58 +0200 (CEST) From: bugzilla@dpdk.org To: dev@dpdk.org Date: Tue, 30 Apr 2019 09:03:58 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: DPDK X-Bugzilla-Component: other X-Bugzilla-Version: 18.11 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: zhongdahulinfan@163.com X-Bugzilla-Status: CONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: Normal X-Bugzilla-Assigned-To: dev@dpdk.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version rep_platform op_sys bug_status bug_severity priority component assigned_to reporter target_milestone Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://bugs.dpdk.org/ Auto-Submitted: auto-generated X-Auto-Response-Suppress: All MIME-Version: 1.0 Subject: [dpdk-dev] [Bug 261] DPDK 18.11 bug on rte_hash_free_key_with_position X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Apr 2019 09:03:58 -0000 https://bugs.dpdk.org/show_bug.cgi?id=3D261 Bug ID: 261 Summary: DPDK 18.11 bug on rte_hash_free_key_with_position Product: DPDK Version: 18.11 Hardware: All OS: All Status: CONFIRMED Severity: normal Priority: Normal Component: other Assignee: dev@dpdk.org Reporter: zhongdahulinfan@163.com Target Milestone: --- First let's see the definition of rte_hash_free_key_with_position in DPDK 18.11, as shown bellow:=20 int __rte_experimental rte_hash_free_key_with_position(const struct rte_hash *h, const int32_t position) { RETURN_IF_TRUE(((h =3D=3D NULL) || (position =3D=3D EMPTY_SLOT)), -EINV= AL); unsigned int lcore_id, n_slots; struct lcore_cache *cached_free_slots; const int32_t total_entries =3D h->num_buckets * RTE_HASH_BUCKET_ENTRIE= S; /* Out of bounds */ if (position >=3D total_entries) return -EINVAL; if (h->use_local_cache) { lcore_id =3D rte_lcore_id(); cached_free_slots =3D &h->local_free_slots[lcore_id]; /* Cache full, need to free it. */ if (cached_free_slots->len =3D=3D LCORE_CACHE_SIZE) { /* Need to enqueue the free slots in global ring. */ n_slots =3D rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); cached_free_slots->len -=3D n_slots; } /* Put index of new free slot in cache. */ cached_free_slots->objs[cached_free_slots->len] =3D (void *)((uintptr_t)position); cached_free_slots->len++; } else { rte_ring_sp_enqueue(h->free_slots, (void *)((uintptr_t)position)); } return 0; } There are two issues for this API. First, the input parameter 'position' is the key index of the hash table, w= hich is returned by rte_hash_add_key_xxx or rte_hash_del_key_xxx. Take a glance = look of rte_hash_del_key_with_hash for example, we see that it returns key_idx -= 1 if entry found and removed successfully. Hence rte_hash_free_key_with_posit= ion is not correct while it enqueues position into free_slots directly. It must increase position by one to get the right key index, before enqueues into free_slots. As comparision, remove_entry()enqueue key_idx directly, which is correct:=20 static inline void remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt, unsigne= d i) { unsigned lcore_id, n_slots; struct lcore_cache *cached_free_slots; if (h->use_local_cache) { lcore_id =3D rte_lcore_id(); cached_free_slots =3D &h->local_free_slots[lcore_id]; /* Cache full, need to free it. */ if (cached_free_slots->len =3D=3D LCORE_CACHE_SIZE) { /* Need to enqueue the free slots in global ring. */ n_slots =3D rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); cached_free_slots->len -=3D n_slots; } /* Put index of new free slot in cache. */ cached_free_slots->objs[cached_free_slots->len] =3D (void *)((uintptr_t)bkt->key_idx[i]); cached_free_slots->len++; } else { rte_ring_sp_enqueue(h->free_slots, (void *)((uintptr_t)bkt->key_idx[i])); } } Second, computation of total_entries is not correct. This should be the tot= al number of key slots.The number of key slots is seen as rte_hash_create, say (params->entries + (RTE_MAX_LCORE - 1) *(LCORE_CACHE_SIZE - 1) + 1) when use_local_cache is true, else (params->entries + 1) struct rte_hash * rte_hash_create(const struct rte_hash_parameters *params) { ... if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD) { use_local_cache =3D 1; writer_takes_lock =3D 1; } ... /* Store all keys and leave the first entry as a dummy entry for lookup_bulk */ if (use_local_cache) /* * Increase number of slots by total number of indices * that can be stored in the lcore caches * except for the first cache */ num_key_slots =3D params->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1; else num_key_slots =3D params->entries + 1; ... /* Populate free slots ring. Entry zero is reserved for key misses. */ for (i =3D 1; i < num_key_slots; i++) rte_ring_sp_enqueue(r, (void *)((uintptr_t) i)); ... } --=20 You are receiving this mail because: You are the assignee for the bug.= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 04A69A0679 for ; Tue, 30 Apr 2019 11:04:00 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 29A725F0D; Tue, 30 Apr 2019 11:03:59 +0200 (CEST) Received: by dpdk.org (Postfix, from userid 33) id 3DDFB5F0D; Tue, 30 Apr 2019 11:03:58 +0200 (CEST) From: bugzilla@dpdk.org To: dev@dpdk.org Date: Tue, 30 Apr 2019 09:03:58 +0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: new X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: DPDK X-Bugzilla-Component: other X-Bugzilla-Version: 18.11 X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: zhongdahulinfan@163.com X-Bugzilla-Status: CONFIRMED X-Bugzilla-Resolution: X-Bugzilla-Priority: Normal X-Bugzilla-Assigned-To: dev@dpdk.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_id short_desc product version rep_platform op_sys bug_status bug_severity priority component assigned_to reporter target_milestone Message-ID: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://bugs.dpdk.org/ Auto-Submitted: auto-generated X-Auto-Response-Suppress: All MIME-Version: 1.0 Subject: [dpdk-dev] [Bug 261] DPDK 18.11 bug on rte_hash_free_key_with_position X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190430090358.nnWzPm7m1M4ZLsNN9xDeq9pkXcF9EwGalw8mwdL2aOA@z> https://bugs.dpdk.org/show_bug.cgi?id=3D261 Bug ID: 261 Summary: DPDK 18.11 bug on rte_hash_free_key_with_position Product: DPDK Version: 18.11 Hardware: All OS: All Status: CONFIRMED Severity: normal Priority: Normal Component: other Assignee: dev@dpdk.org Reporter: zhongdahulinfan@163.com Target Milestone: --- First let's see the definition of rte_hash_free_key_with_position in DPDK 18.11, as shown bellow:=20 int __rte_experimental rte_hash_free_key_with_position(const struct rte_hash *h, const int32_t position) { RETURN_IF_TRUE(((h =3D=3D NULL) || (position =3D=3D EMPTY_SLOT)), -EINV= AL); unsigned int lcore_id, n_slots; struct lcore_cache *cached_free_slots; const int32_t total_entries =3D h->num_buckets * RTE_HASH_BUCKET_ENTRIE= S; /* Out of bounds */ if (position >=3D total_entries) return -EINVAL; if (h->use_local_cache) { lcore_id =3D rte_lcore_id(); cached_free_slots =3D &h->local_free_slots[lcore_id]; /* Cache full, need to free it. */ if (cached_free_slots->len =3D=3D LCORE_CACHE_SIZE) { /* Need to enqueue the free slots in global ring. */ n_slots =3D rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); cached_free_slots->len -=3D n_slots; } /* Put index of new free slot in cache. */ cached_free_slots->objs[cached_free_slots->len] =3D (void *)((uintptr_t)position); cached_free_slots->len++; } else { rte_ring_sp_enqueue(h->free_slots, (void *)((uintptr_t)position)); } return 0; } There are two issues for this API. First, the input parameter 'position' is the key index of the hash table, w= hich is returned by rte_hash_add_key_xxx or rte_hash_del_key_xxx. Take a glance = look of rte_hash_del_key_with_hash for example, we see that it returns key_idx -= 1 if entry found and removed successfully. Hence rte_hash_free_key_with_posit= ion is not correct while it enqueues position into free_slots directly. It must increase position by one to get the right key index, before enqueues into free_slots. As comparision, remove_entry()enqueue key_idx directly, which is correct:=20 static inline void remove_entry(const struct rte_hash *h, struct rte_hash_bucket *bkt, unsigne= d i) { unsigned lcore_id, n_slots; struct lcore_cache *cached_free_slots; if (h->use_local_cache) { lcore_id =3D rte_lcore_id(); cached_free_slots =3D &h->local_free_slots[lcore_id]; /* Cache full, need to free it. */ if (cached_free_slots->len =3D=3D LCORE_CACHE_SIZE) { /* Need to enqueue the free slots in global ring. */ n_slots =3D rte_ring_mp_enqueue_burst(h->free_slots, cached_free_slots->objs, LCORE_CACHE_SIZE, NULL); cached_free_slots->len -=3D n_slots; } /* Put index of new free slot in cache. */ cached_free_slots->objs[cached_free_slots->len] =3D (void *)((uintptr_t)bkt->key_idx[i]); cached_free_slots->len++; } else { rte_ring_sp_enqueue(h->free_slots, (void *)((uintptr_t)bkt->key_idx[i])); } } Second, computation of total_entries is not correct. This should be the tot= al number of key slots.The number of key slots is seen as rte_hash_create, say (params->entries + (RTE_MAX_LCORE - 1) *(LCORE_CACHE_SIZE - 1) + 1) when use_local_cache is true, else (params->entries + 1) struct rte_hash * rte_hash_create(const struct rte_hash_parameters *params) { ... if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD) { use_local_cache =3D 1; writer_takes_lock =3D 1; } ... /* Store all keys and leave the first entry as a dummy entry for lookup_bulk */ if (use_local_cache) /* * Increase number of slots by total number of indices * that can be stored in the lcore caches * except for the first cache */ num_key_slots =3D params->entries + (RTE_MAX_LCORE - 1) * (LCORE_CACHE_SIZE - 1) + 1; else num_key_slots =3D params->entries + 1; ... /* Populate free slots ring. Entry zero is reserved for key misses. */ for (i =3D 1; i < num_key_slots; i++) rte_ring_sp_enqueue(r, (void *)((uintptr_t) i)); ... } --=20 You are receiving this mail because: You are the assignee for the bug.=