> -----Original Message----- > From: Honnappa Nagarahalli [mailto:[email protected]] > Sent: Sunday, September 30, 2018 11:33 PM > To: Van Haaren, Harry <[email protected]>; Richardson, Bruce > <[email protected]>; Wang, Yipeng1 <[email protected]> > Cc: De Lara Guarch, Pablo <[email protected]>; [email protected]; > Gavin Hu (Arm Technology China) <[email protected]>; Steve Capper > <[email protected]>; Ola Liljedahl <[email protected]>; nd > <[email protected]> > Subject: RE: [dpdk-dev] [PATCH 3/4] hash: fix rw concurrency while moving > keys > > > > > > > > > > > >Reader-writer concurrency issue, caused by moving the keys to their > > > > >alternative locations during key insert, is solved by introducing a > > > > >global counter(tbl_chng_cnt) indicating a change in table. > > > > <snip> > > > > > > > /** > > > > >@@ -200,7 +200,7 @@ rte_hash_add_key_with_hash_data(const struct > > > > >rte_hash > > > *h, const void *key, > > > > > * array of user data. This value is unique for this key. > > > > > */ > > > > > int32_t > > > > >-rte_hash_add_key(const struct rte_hash *h, const void *key); > > > > >+rte_hash_add_key(struct rte_hash *h, const void *key); > > > > > > > > > > /** > > > > > * Add a key to an existing hash table. > > > > >@@ -222,7 +222,7 @@ rte_hash_add_key(const struct rte_hash *h, > > > > >const void > > > *key); > > > > > * array of user data. This value is unique for this key. > > > > > */ > > > > > int32_t > > > > >-rte_hash_add_key_with_hash(const struct rte_hash *h, const void > > > > >*key, > > > hash_sig_t sig); > > > > >+rte_hash_add_key_with_hash(struct rte_hash *h, const void *key, > > > hash_sig_t sig); > > > > > > > > > > / > > > > > > > > I think the above changes will break ABI by changing the parameter > type? > > > Other people may know better on this. > > > > > > Just removing a const should not change the ABI, I believe, since the > > > const is just advisory hint to the compiler. Actual parameter size and > > > count remains unchanged so I don't believe there is an issue. > > > [ABI experts, please correct me if I'm wrong on this] > > > > > > [Certainly no ABI expert, but...] > > > > I think this is an API break, not ABI break. > > > > Given application code as follows, it will fail to compile - even though > running > > the new code as a .so wouldn't cause any issues (AFAIK). > > > > void do_hash_stuff(const struct rte_hash *h, ...) { > > /* parameter passed in is const, but updated function prototype is > non- > > const */ > > rte_hash_add_key_with_hash(h, ...); > > } > > > > This means that we can't recompile apps against latest patch without > > application code changes, if the app was passing a const rte_hash struct > as > > the first parameter. > > > Agree. Do we need to do anything for this?
I think we should try to avoid breaking API wherever possible. If we must, then I suppose we could follow the ABI process of a deprecation notice. >From my reading of the versioning docs, it doesn't document this case: https://doc.dpdk.org/guides/contributing/versioning.html I don't recall a similar situation in DPDK previously - so I suggest you ask Tech board for input here. Hope that helps! -Harry
