Re: [PATCH dpdk v2 2/3] hash: free replaced data on overwrite when RCU is configured

["Robin Jarry" <rjarry@redhat.com>]

Hi Konstantin,

Konstantin Ananyev, Mar 04, 2026 at 12:40:
>> @@ -776,6 +778,8 @@ search_and_update(const struct rte_hash *h, void *data,
>> const void *key,
>>  			k = (struct rte_hash_key *) ((char *)keys +
>>  					bkt->key_idx[i] * h->key_entry_size);
>>  			if (rte_hash_cmp_eq(key, k->key, h) == 0) {
>> +				if (old_data != NULL)
>> +					*old_data = k->pdata;
>
> For consistency, shouldn't we use here instead of load/store:
> *old_data = rte_atomic_exchange_explicit(&k->pdata, data, ...);
> ?

Yes, that makes sense. I'll send that in a v3.

>> @@ -1263,6 +1304,11 @@ __rte_hash_add_key_with_hash(const struct rte_hash
>> *h, const void *key,
>>  	__hash_rw_writer_unlock(h);
>>  	return slot_id - 1;
>
> With multiple goto labels that function becomes even more complicated.
> Can we just add old_data as an extra parameter: 
> __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
>                    hash_sig_t sig, void *data, void **old_data)
> And make the callers to invoke __hash_rcu_auto_free_old_data()?

I'll see what I can do for v3.

Thanks for the review!