From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 01ED945AA4;
Fri, 4 Oct 2024 00:37:28 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id DF6F64060B;
Fri, 4 Oct 2024 00:37:27 +0200 (CEST)
Received: from mail-pf1-f178.google.com (mail-pf1-f178.google.com
[209.85.210.178])
by mails.dpdk.org (Postfix) with ESMTP id B150F402B3
for <dev@dpdk.org>; Fri, 4 Oct 2024 00:37:24 +0200 (CEST)
Received: by mail-pf1-f178.google.com with SMTP id
d2e1a72fcca58-718816be6cbso1304381b3a.1
for <dev@dpdk.org>; Thu, 03 Oct 2024 15:37:24 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1727995044;
x=1728599844; darn=dpdk.org;
h=content-transfer-encoding:mime-version:references:in-reply-to
:message-id:subject:cc:to:from:date:from:to:cc:subject:date
:message-id:reply-to;
bh=JFjlXRmOG2FRPU2dQylzSdCK6Z7FBx9yKJrujakty10=;
b=MlGptQVWhz84W+VLlqgaPm+AxI9iqDLEyTFwefA8AyTbkqhw6OcOIO+q48xpMi2VrX
LEcqKE919JKyOi/qVRRJhQl62dOZweU1KV2kqtolJdq0misjAI907UV0x36DBx2jBx3m
XEZZOOiWv1bhzs46H8jbpAF2B0svgkMtp46enchyVXhLmzxj1/Br2mVAFVGUwQT149LQ
iLQyyO1lwbEPU5WY7zP0hcO6wr3Iu15RkPpL+7W5N8wFtOAe/ratY37MYTEEF452crnd
rOhaDm6iU+dJ75r1xb1ASRJHIQs7I4dxXqj3/oJ18Jv7eB5+FSJjgyly4mDghIAAWPsY
Vt9w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1727995044; x=1728599844;
h=content-transfer-encoding:mime-version:references:in-reply-to
:message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=JFjlXRmOG2FRPU2dQylzSdCK6Z7FBx9yKJrujakty10=;
b=is+rA+UFbLYn2wQDJc1bD1ALlkzCm/oUuLOkJE+BLyvy3XjLqHe16JK/WsvMuFPQbZ
waLKTYdC0i2y4isgLiEZQxGLSFZzHRkrksFGKgQzIcpHYKNpPiEIdTWzeABcWfGVelbg
SuBow10AbVYPCS6Chigm/+wg0GjCHcSPPYBSo3WJ69VJhPtrmeh1z2uUmQsqF4RLrKZr
cjDcUb0atY5uYRClROBBxudNVzISzrWf4l8FKs2fR2OBX46AxpKwts2sZtLPQzg3Wkvb
ZdjcP4lEPoJpDXVnGfzKQR36VI7e6tpzgHmzc723ETj/zNrwys9mfg23wEt4BWwhq+4h
+EpQ==
X-Gm-Message-State: AOJu0Yxom3N5xfiBGhnW0yL2N19gl2Z/RJf92wx46CxK2GHPqxaAdlA+
PJ9dJvdYELEOSVXJPjbg0ulzsJvbsBtcCNNOf9lDCOC0QuQhnw73nKiZIpejM3M=
X-Google-Smtp-Source: AGHT+IHTs9UsznqdfM7t6KxtT3rXqWSX3zOHSHkGINyNc7LrEL/6Q3/C64G5GZfYGas4Pa/k1sETuA==
X-Received: by 2002:aa7:88cb:0:b0:70b:a46:7db7 with SMTP id
d2e1a72fcca58-71de23e8eaemr1080396b3a.16.1727995043818;
Thu, 03 Oct 2024 15:37:23 -0700 (PDT)
Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226])
by smtp.gmail.com with ESMTPSA id
d2e1a72fcca58-71dd9d9c0b3sm1960744b3a.92.2024.10.03.15.37.23
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Thu, 03 Oct 2024 15:37:23 -0700 (PDT)
Date: Thu, 3 Oct 2024 15:37:21 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: dev@dpdk.org, Abdullah =?UTF-8?B?w5ZtZXIgWWFtYcOn?=
<omer.yamac@ceng.metu.edu.tr>, Yipeng Wang <yipeng1.wang@intel.com>, Sameh
Gobriel <sameh.gobriel@intel.com>, Bruce Richardson
<bruce.richardson@intel.com>, Vladimir Medvedkin
<vladimir.medvedkin@intel.com>, David Marchand <david.marchand@redhat.com>,
Abdullah =?UTF-8?B?w5ZtZXIgWWFtYcOn?= <aomeryamac@gmail.com>
Subject: Re: [PATCH v2] lib/hash: new feature adding existing key
Message-ID: <20241003153721.2eb45ebd@hermes.local>
In-Reply-To: <5156487.LM0AJKV5NW@thomas>
References: <CAJFAV8z-+CWhUeOf7ieo82EqZcvTKuqPczfWuEGok6UcPAxm2A@mail.gmail.com>
<20231023082925.256141-1-aomeryamac@gmail.com>
<5156487.LM0AJKV5NW@thomas>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Fri, 16 Feb 2024 13:43:52 +0100
Thomas Monjalon <thomas@monjalon.net> wrote:
> Any review please?
> If maintainers agree with the idea, we should announce the ABI change.
>=20
>=20
> 23/10/2023 10:29, Abdullah =C3=96mer Yama=C3=A7:
> > From: Abdullah =C3=96mer Yama=C3=A7 <omer.yamac@ceng.metu.edu.tr>
> >=20
> > In some use cases inserting data with the same key shouldn't be
> > overwritten. We use a new flag in this patch to disable overwriting
> > data for the same key.
> >=20
> > Signed-off-by: Abdullah =C3=96mer Yama=C3=A7 <omer.yamac@ceng.metu.edu.=
tr>
> >=20
> > ---
> > Cc: Yipeng Wang <yipeng1.wang@intel.com>
> > Cc: Sameh Gobriel <sameh.gobriel@intel.com>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>
> > Cc: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
> > Cc: David Marchand <david.marchand@redhat.com>
> > ---
> > lib/hash/rte_cuckoo_hash.c | 10 +++++++++-
> > lib/hash/rte_cuckoo_hash.h | 2 ++
> > lib/hash/rte_hash.h | 4 ++++
> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >=20
> > diff --git a/lib/hash/rte_cuckoo_hash.c b/lib/hash/rte_cuckoo_hash.c
> > index 19b23f2a97..fe8f21bee4 100644
> > --- a/lib/hash/rte_cuckoo_hash.c
> > +++ b/lib/hash/rte_cuckoo_hash.c
> > @@ -32,7 +32,8 @@
> > RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | \
> > RTE_HASH_EXTRA_FLAGS_EXT_TABLE | \
> > RTE_HASH_EXTRA_FLAGS_NO_FREE_ON_DEL | \
> > - RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)
> > + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF | \
> > + RTE_HASH_EXTRA_FLAGS_DISABLE_UPDATE_EXISTING_KEY)
> > =20
> > #define FOR_EACH_BUCKET(CURRENT_BKT, START_BUCKET) =
\
> > for (CURRENT_BKT =3D START_BUCKET; =
\
> > @@ -148,6 +149,7 @@ rte_hash_create(const struct rte_hash_parameters *p=
arams)
> > unsigned int readwrite_concur_support =3D 0;
> > unsigned int writer_takes_lock =3D 0;
> > unsigned int no_free_on_del =3D 0;
> > + unsigned int no_update_data =3D 0;
> > uint32_t *ext_bkt_to_free =3D NULL;
> > uint32_t *tbl_chng_cnt =3D NULL;
> > struct lcore_cache *local_free_slots =3D NULL;
> > @@ -216,6 +218,9 @@ rte_hash_create(const struct rte_hash_parameters *p=
arams)
> > no_free_on_del =3D 1;
> > }
> > =20
> > + if (params->extra_flag & RTE_HASH_EXTRA_FLAGS_DISABLE_UPDATE_EXISTING=
_KEY)
> > + no_update_data =3D 1;
> > +
> > /* Store all keys and leave the first entry as a dummy entry for look=
up_bulk */
> > if (use_local_cache)
> > /*
> > @@ -428,6 +433,7 @@ rte_hash_create(const struct rte_hash_parameters *p=
arams)
> > h->ext_table_support =3D ext_table_support;
> > h->writer_takes_lock =3D writer_takes_lock;
> > h->no_free_on_del =3D no_free_on_del;
> > + h->no_update_data =3D no_update_data;
> > h->readwrite_concur_lf_support =3D readwrite_concur_lf_support;
> > =20
> > #if defined(RTE_ARCH_X86)
> > @@ -707,6 +713,8 @@ search_and_update(const struct rte_hash *h, void *d=
ata, const void *key,
> > k =3D (struct rte_hash_key *) ((char *)keys +
> > bkt->key_idx[i] * h->key_entry_size);
> > if (rte_hash_cmp_eq(key, k->key, h) =3D=3D 0) {
> > + if (h->no_update_data =3D=3D 1)
> > + return -EINVAL;
This is buggy, the caller assumes -1 on error in several places.
See:
ret =3D search_and_update(h, data, key, prim_bkt, sig);
if (ret !=3D -1) {
__hash_rw_writer_unlock(h);
*ret_val =3D ret;
return 1;
}
These paths would exercised when table had to expand.
Also any new functionality like this would need tests in functional test.