From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Fri, 4 Oct 2024 00:37:24 +0200 (CEST) Received: by mail-pf1-f178.google.com with SMTP id d2e1a72fcca58-718816be6cbso1304381b3a.1 for ; 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 To: Thomas Monjalon Cc: dev@dpdk.org, Abdullah =?UTF-8?B?w5ZtZXIgWWFtYcOn?= , Yipeng Wang , Sameh Gobriel , Bruce Richardson , Vladimir Medvedkin , David Marchand , Abdullah =?UTF-8?B?w5ZtZXIgWWFtYcOn?= Subject: Re: [PATCH v2] lib/hash: new feature adding existing key Message-ID: <20241003153721.2eb45ebd@hermes.local> In-Reply-To: <5156487.LM0AJKV5NW@thomas> References: <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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, 16 Feb 2024 13:43:52 +0100 Thomas Monjalon 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 > >=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 > >=20 > > --- > > Cc: Yipeng Wang > > Cc: Sameh Gobriel > > Cc: Bruce Richardson > > Cc: Vladimir Medvedkin > > Cc: David Marchand > > --- > > 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.