* [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key()
@ 2016-08-25 14:41 Pablo Neira Ayuso
2016-08-25 14:41 ` [PATCH nf-next,v2 2/2] netfilter: nf_tables: honor NLM_F_EXCL flag in set element insertion Pablo Neira Ayuso
2016-08-26 11:27 ` [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Herbert Xu
0 siblings, 2 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-25 14:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: herbert, tgraf, netdev
This patch modifies __rhashtable_insert_fast() so it returns the
existing object that clashes with the one that you want to insert.
In case the object is successfully inserted, NULL is returned.
Otherwise, you get an error via ERR_PTR().
This patch adapts the existing callers of __rhashtable_insert_fast()
so they handle this new logic, and it adds a new
rhashtable_lookup_get_insert_key() interface to fetch this existing
object.
nf_tables needs this change to improve handling of EEXIST cases via
honoring the NLM_F_EXCL flag and by checking if the data part of the
mapping matches what we have.
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Thomas Graf <tgraf@suug.ch>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: Address Herbert's feedback:
* Modify __rhashtable_insert_fast() so this returns the object if it
exists, NULL if it does not and the insertion was successful, and
an ERR_PTR otherwise.
* Update rhashtable_insert_slow() path too.
include/linux/rhashtable.h | 70 +++++++++++++++++++++++++++++++++++++---------
lib/rhashtable.c | 10 +++++--
2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 3eef080..26b7a05 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -343,7 +343,8 @@ int rhashtable_init(struct rhashtable *ht,
struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
const void *key,
struct rhash_head *obj,
- struct bucket_table *old_tbl);
+ struct bucket_table *old_tbl,
+ void **data);
int rhashtable_insert_rehash(struct rhashtable *ht, struct bucket_table *tbl);
int rhashtable_walk_init(struct rhashtable *ht, struct rhashtable_iter *iter,
@@ -563,8 +564,11 @@ restart:
return NULL;
}
-/* Internal function, please use rhashtable_insert_fast() instead */
-static inline int __rhashtable_insert_fast(
+/* Internal function, please use rhashtable_insert_fast() instead. This
+ * function returns the existing element already in hashes in there is a clash,
+ * otherwise it returns an error via ERR_PTR().
+ */
+static inline void *__rhashtable_insert_fast(
struct rhashtable *ht, const void *key, struct rhash_head *obj,
const struct rhashtable_params params)
{
@@ -577,6 +581,7 @@ static inline int __rhashtable_insert_fast(
spinlock_t *lock;
unsigned int elasticity;
unsigned int hash;
+ void *data = NULL;
int err;
restart:
@@ -601,11 +606,14 @@ restart:
new_tbl = rht_dereference_rcu(tbl->future_tbl, ht);
if (unlikely(new_tbl)) {
- tbl = rhashtable_insert_slow(ht, key, obj, new_tbl);
+ tbl = rhashtable_insert_slow(ht, key, obj, new_tbl, &data);
if (!IS_ERR_OR_NULL(tbl))
goto slow_path;
err = PTR_ERR(tbl);
+ if (err == -EEXIST)
+ err = 0;
+
goto out;
}
@@ -619,25 +627,25 @@ slow_path:
err = rhashtable_insert_rehash(ht, tbl);
rcu_read_unlock();
if (err)
- return err;
+ return ERR_PTR(err);
goto restart;
}
- err = -EEXIST;
+ err = 0;
elasticity = ht->elasticity;
rht_for_each(head, tbl, hash) {
if (key &&
unlikely(!(params.obj_cmpfn ?
params.obj_cmpfn(&arg, rht_obj(ht, head)) :
- rhashtable_compare(&arg, rht_obj(ht, head)))))
+ rhashtable_compare(&arg, rht_obj(ht, head))))) {
+ data = rht_obj(ht, head);
goto out;
+ }
if (!--elasticity)
goto slow_path;
}
- err = 0;
-
head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
RCU_INIT_POINTER(obj->next, head);
@@ -652,7 +660,7 @@ out:
spin_unlock_bh(lock);
rcu_read_unlock();
- return err;
+ return err ? ERR_PTR(err) : data;
}
/**
@@ -675,7 +683,13 @@ static inline int rhashtable_insert_fast(
struct rhashtable *ht, struct rhash_head *obj,
const struct rhashtable_params params)
{
- return __rhashtable_insert_fast(ht, NULL, obj, params);
+ void *ret;
+
+ ret = __rhashtable_insert_fast(ht, NULL, obj, params);
+ if (IS_ERR(ret))
+ return PTR_ERR(ret);
+
+ return ret == NULL ? 0 : -EEXIST;
}
/**
@@ -704,11 +718,15 @@ static inline int rhashtable_lookup_insert_fast(
const struct rhashtable_params params)
{
const char *key = rht_obj(ht, obj);
+ void *ret;
BUG_ON(ht->p.obj_hashfn);
- return __rhashtable_insert_fast(ht, key + ht->p.key_offset, obj,
- params);
+ ret = __rhashtable_insert_fast(ht, key + ht->p.key_offset, obj, params);
+ if (IS_ERR(ret))
+ return PTR_ERR(ret);
+
+ return ret == NULL ? 0 : -EEXIST;
}
/**
@@ -737,6 +755,32 @@ static inline int rhashtable_lookup_insert_key(
struct rhashtable *ht, const void *key, struct rhash_head *obj,
const struct rhashtable_params params)
{
+ void *ret;
+
+ BUG_ON(!ht->p.obj_hashfn || !key);
+
+ ret = __rhashtable_insert_fast(ht, key, obj, params);
+ if (IS_ERR(ret))
+ return PTR_ERR(ret);
+
+ return ret == NULL ? 0 : -EEXIST;
+}
+
+/**
+ * rhashtable_lookup_get_insert_key - lookup and insert object into hash table
+ * @ht: hash table
+ * @obj: pointer to hash head inside object
+ * @params: hash table parameters
+ * @data: pointer to element data already in hashes
+ *
+ * Just like rhashtable_lookup_insert_key(), but this function returns the
+ * object if it exists, NULL if it does not and the insertion was successful,
+ * and an ERR_PTR otherwise.
+ */
+static inline void *rhashtable_lookup_get_insert_key(
+ struct rhashtable *ht, const void *key, struct rhash_head *obj,
+ const struct rhashtable_params params)
+{
BUG_ON(!ht->p.obj_hashfn || !key);
return __rhashtable_insert_fast(ht, key, obj, params);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 5d845ff..7a940d9 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -438,7 +438,8 @@ EXPORT_SYMBOL_GPL(rhashtable_insert_rehash);
struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
const void *key,
struct rhash_head *obj,
- struct bucket_table *tbl)
+ struct bucket_table *tbl,
+ void **data)
{
struct rhash_head *head;
unsigned int hash;
@@ -449,8 +450,11 @@ struct bucket_table *rhashtable_insert_slow(struct rhashtable *ht,
spin_lock_nested(rht_bucket_lock(tbl, hash), SINGLE_DEPTH_NESTING);
err = -EEXIST;
- if (key && rhashtable_lookup_fast(ht, key, ht->p))
- goto exit;
+ if (key) {
+ *data = rhashtable_lookup_fast(ht, key, ht->p);
+ if (*data)
+ goto exit;
+ }
err = -E2BIG;
if (unlikely(rht_grow_above_max(ht, tbl)))
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH nf-next,v2 2/2] netfilter: nf_tables: honor NLM_F_EXCL flag in set element insertion
2016-08-25 14:41 [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Pablo Neira Ayuso
@ 2016-08-25 14:41 ` Pablo Neira Ayuso
2016-08-26 11:27 ` [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Herbert Xu
1 sibling, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-25 14:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: herbert, tgraf, netdev
If the NLM_F_EXCL flag is set, then new elements that clash with an
existing one return EEXIST. In case you try to add an element whose
data area differs from what we have, then this returns EBUSY. If no
flag is specified at all, then this returns success to userspace.
This patch also update the set insert operation so we can fetch the
existing element that clashes with the one you want to add, we need
this to make sure the element data doesn't differ.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: Adapt this patch to semantics changes in rhashtable_lookup_get_insert_key()
proposed by Herbert.
include/net/netfilter/nf_tables.h | 3 ++-
net/netfilter/nf_tables_api.c | 20 +++++++++++++++-----
net/netfilter/nft_set_hash.c | 17 +++++++++++++----
net/netfilter/nft_set_rbtree.c | 12 ++++++++----
4 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index f2f1339..8972468 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -251,7 +251,8 @@ struct nft_set_ops {
int (*insert)(const struct net *net,
const struct nft_set *set,
- const struct nft_set_elem *elem);
+ const struct nft_set_elem *elem,
+ struct nft_set_ext **ext);
void (*activate)(const struct net *net,
const struct nft_set *set,
const struct nft_set_elem *elem);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 221d27f..bd9715e 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3483,12 +3483,12 @@ static int nft_setelem_parse_flags(const struct nft_set *set,
}
static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
- const struct nlattr *attr)
+ const struct nlattr *attr, u32 nlmsg_flags)
{
struct nlattr *nla[NFTA_SET_ELEM_MAX + 1];
struct nft_data_desc d1, d2;
struct nft_set_ext_tmpl tmpl;
- struct nft_set_ext *ext;
+ struct nft_set_ext *ext, *ext2;
struct nft_set_elem elem;
struct nft_set_binding *binding;
struct nft_userdata *udata;
@@ -3615,9 +3615,19 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
goto err4;
ext->genmask = nft_genmask_cur(ctx->net) | NFT_SET_ELEM_BUSY_MASK;
- err = set->ops->insert(ctx->net, set, &elem);
- if (err < 0)
+ err = set->ops->insert(ctx->net, set, &elem, &ext2);
+ if (err) {
+ if (err == -EEXIST) {
+ if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA) &&
+ nft_set_ext_exists(ext2, NFT_SET_EXT_DATA) &&
+ memcmp(nft_set_ext_data(ext),
+ nft_set_ext_data(ext2), set->dlen) != 0)
+ err = -EBUSY;
+ else if (!(nlmsg_flags & NLM_F_EXCL))
+ err = 0;
+ }
goto err5;
+ }
nft_trans_elem(trans) = elem;
list_add_tail(&trans->list, &ctx->net->nft.commit_list);
@@ -3673,7 +3683,7 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
!atomic_add_unless(&set->nelems, 1, set->size + set->ndeact))
return -ENFILE;
- err = nft_add_set_elem(&ctx, set, attr);
+ err = nft_add_set_elem(&ctx, set, attr, nlh->nlmsg_flags);
if (err < 0) {
atomic_dec(&set->nelems);
break;
diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index 564fa79..3794cb2 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -126,7 +126,8 @@ err1:
}
static int nft_hash_insert(const struct net *net, const struct nft_set *set,
- const struct nft_set_elem *elem)
+ const struct nft_set_elem *elem,
+ struct nft_set_ext **ext)
{
struct nft_hash *priv = nft_set_priv(set);
struct nft_hash_elem *he = elem->priv;
@@ -135,9 +136,17 @@ static int nft_hash_insert(const struct net *net, const struct nft_set *set,
.set = set,
.key = elem->key.val.data,
};
-
- return rhashtable_lookup_insert_key(&priv->ht, &arg, &he->node,
- nft_hash_params);
+ struct nft_hash_elem *prev;
+
+ prev = rhashtable_lookup_get_insert_key(&priv->ht, &arg, &he->node,
+ nft_hash_params);
+ if (IS_ERR(prev))
+ return PTR_ERR(prev);
+ if (prev) {
+ *ext = &prev->ext;
+ return -EEXIST;
+ }
+ return 0;
}
static void nft_hash_activate(const struct net *net, const struct nft_set *set,
diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 6473936..038682d 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -94,7 +94,8 @@ out:
}
static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
- struct nft_rbtree_elem *new)
+ struct nft_rbtree_elem *new,
+ struct nft_set_ext **ext)
{
struct nft_rbtree *priv = nft_set_priv(set);
u8 genmask = nft_genmask_next(net);
@@ -122,8 +123,10 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
else if (!nft_rbtree_interval_end(rbe) &&
nft_rbtree_interval_end(new))
p = &parent->rb_right;
- else
+ else {
+ *ext = &rbe->ext;
return -EEXIST;
+ }
}
}
}
@@ -133,13 +136,14 @@ static int __nft_rbtree_insert(const struct net *net, const struct nft_set *set,
}
static int nft_rbtree_insert(const struct net *net, const struct nft_set *set,
- const struct nft_set_elem *elem)
+ const struct nft_set_elem *elem,
+ struct nft_set_ext **ext)
{
struct nft_rbtree_elem *rbe = elem->priv;
int err;
spin_lock_bh(&nft_rbtree_lock);
- err = __nft_rbtree_insert(net, set, rbe);
+ err = __nft_rbtree_insert(net, set, rbe, ext);
spin_unlock_bh(&nft_rbtree_lock);
return err;
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key()
2016-08-25 14:41 [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Pablo Neira Ayuso
2016-08-25 14:41 ` [PATCH nf-next,v2 2/2] netfilter: nf_tables: honor NLM_F_EXCL flag in set element insertion Pablo Neira Ayuso
@ 2016-08-26 11:27 ` Herbert Xu
2016-08-26 12:15 ` Pablo Neira Ayuso
1 sibling, 1 reply; 4+ messages in thread
From: Herbert Xu @ 2016-08-26 11:27 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, tgraf, netdev
On Thu, Aug 25, 2016 at 04:41:26PM +0200, Pablo Neira Ayuso wrote:
> This patch modifies __rhashtable_insert_fast() so it returns the
> existing object that clashes with the one that you want to insert.
> In case the object is successfully inserted, NULL is returned.
> Otherwise, you get an error via ERR_PTR().
>
> This patch adapts the existing callers of __rhashtable_insert_fast()
> so they handle this new logic, and it adds a new
> rhashtable_lookup_get_insert_key() interface to fetch this existing
> object.
>
> nf_tables needs this change to improve handling of EEXIST cases via
> honoring the NLM_F_EXCL flag and by checking if the data part of the
> mapping matches what we have.
>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Thomas Graf <tgraf@suug.ch>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key()
2016-08-26 11:27 ` [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Herbert Xu
@ 2016-08-26 12:15 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-26 12:15 UTC (permalink / raw)
To: Herbert Xu, davem; +Cc: netfilter-devel, tgraf, netdev
On Fri, Aug 26, 2016 at 07:27:36PM +0800, Herbert Xu wrote:
> On Thu, Aug 25, 2016 at 04:41:26PM +0200, Pablo Neira Ayuso wrote:
> > This patch modifies __rhashtable_insert_fast() so it returns the
> > existing object that clashes with the one that you want to insert.
> > In case the object is successfully inserted, NULL is returned.
> > Otherwise, you get an error via ERR_PTR().
> >
> > This patch adapts the existing callers of __rhashtable_insert_fast()
> > so they handle this new logic, and it adds a new
> > rhashtable_lookup_get_insert_key() interface to fetch this existing
> > object.
> >
> > nf_tables needs this change to improve handling of EEXIST cases via
> > honoring the NLM_F_EXCL flag and by checking if the data part of the
> > mapping matches what we have.
> >
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Cc: Thomas Graf <tgraf@suug.ch>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks Herbert!
@David, would you be OK if I get this rhashtable update through
nf-next given that I have a follow up patch that depending on this?
Will be sending a pull request asap so we don't get out of sync.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-08-26 12:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-25 14:41 [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Pablo Neira Ayuso
2016-08-25 14:41 ` [PATCH nf-next,v2 2/2] netfilter: nf_tables: honor NLM_F_EXCL flag in set element insertion Pablo Neira Ayuso
2016-08-26 11:27 ` [PATCH nf-next,v2 1/2] rhashtable: add rhashtable_lookup_get_insert_key() Herbert Xu
2016-08-26 12:15 ` Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).