diff mbox series

[nf-next,v4] net: netfilter: nf_tables_api: Use id allocation.

Message ID 20180613164143.19232-1-rvarsha016@gmail.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series [nf-next,v4] net: netfilter: nf_tables_api: Use id allocation. | expand

Commit Message

Varsha Rao June 13, 2018, 4:41 p.m. UTC 
In nf_tables_set_alloc_name function, remove get_zeroed_page
find_first_zero_bit and set_bit functions. Instead use ida_get_new_above
function as it simplifies the code.

Signed-off-by: Varsha Rao <rvarsha016@gmail.com>
---
Changes in v2:
- Modified the upper limit of page size.

Changes in v3:
- Used ida_get_new_above instead of ida_simple_get due to internal
  locking.
- Defined macro NFT_SET_IDA_SIZE.
- Modified commit message.

Changes in v4:
- Removed -EAGAIN return value.
- Updated NFT_SET_IDA_SIZE value.

 net/netfilter/nf_tables_api.c | 49 ++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 24 deletions(-)

Comments

Dan Carpenter June 18, 2018, 12:25 p.m. UTC | #1 
Hi Varsha,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Varsha-Rao/net-netfilter-nf_tables_api-Use-id-allocation/20180614-004233
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master

New smatch warnings:
net/netfilter/nf_tables_api.c:2962 nf_tables_set_alloc_name() error: uninitialized symbol 'id'.

Old smatch warnings:
net/netfilter/nf_tables_api.c:4314 nft_add_set_elem() error: uninitialized symbol 'ext2'.

# https://github.com/0day-ci/linux/commit/c19707b85d3d68e8c0fc2a91b2b401832676f224
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c19707b85d3d68e8c0fc2a91b2b401832676f224
vim +/id +2962 net/netfilter/nf_tables_api.c

c19707b8 Varsha Rao        2018-06-13  2923  
20a69341 Patrick McHardy   2013-10-11  2924  static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
20a69341 Patrick McHardy   2013-10-11  2925  				    const char *name)
20a69341 Patrick McHardy   2013-10-11  2926  {
20a69341 Patrick McHardy   2013-10-11  2927  	const struct nft_set *i;
20a69341 Patrick McHardy   2013-10-11  2928  	const char *p;
c19707b8 Varsha Rao        2018-06-13  2929  	int n = 0, id;
c19707b8 Varsha Rao        2018-06-13  2930  	DEFINE_IDA(inuse);
20a69341 Patrick McHardy   2013-10-11  2931  
38745490 Phil Sutter       2017-07-27  2932  	p = strchr(name, '%');
20a69341 Patrick McHardy   2013-10-11  2933  	if (p != NULL) {
20a69341 Patrick McHardy   2013-10-11  2934  		if (p[1] != 'd' || strchr(p + 2, '%'))
96518518 Patrick McHardy   2013-10-14  2935  			return -EINVAL;
20a69341 Patrick McHardy   2013-10-11  2936  
20a69341 Patrick McHardy   2013-10-11  2937  	list_for_each_entry(i, &ctx->table->sets, list) {
14662917 Daniel Borkmann   2013-12-31  2938  		int tmp;
14662917 Daniel Borkmann   2013-12-31  2939  
37a9cc52 Pablo Neira Ayuso 2016-06-12  2940  		if (!nft_is_active_next(ctx->net, set))
37a9cc52 Pablo Neira Ayuso 2016-06-12  2941  			continue;
14662917 Daniel Borkmann   2013-12-31  2942  		if (!sscanf(i->name, name, &tmp))
20a69341 Patrick McHardy   2013-10-11  2943  			continue;
c19707b8 Varsha Rao        2018-06-13  2944  		if (tmp < 0 || tmp >= NFT_SET_IDA_SIZE)
20a69341 Patrick McHardy   2013-10-11  2945  			continue;
14662917 Daniel Borkmann   2013-12-31  2946  
c19707b8 Varsha Rao        2018-06-13  2947  		n = ida_get_new_above(&inuse, tmp, &id);
c19707b8 Varsha Rao        2018-06-13  2948  		if ((n < 0) && (n != -EAGAIN))
c19707b8 Varsha Rao        2018-06-13  2949  			return n;
20a69341 Patrick McHardy   2013-10-11  2950  	}
c19707b8 Varsha Rao        2018-06-13  2951  	n = ida_get_new_above(&inuse, 0, &id);

Smatch is complaining that if n == -EAGAIN then id isn't initialized
which seems legit.

20a69341 Patrick McHardy   2013-10-11  2952  
c19707b8 Varsha Rao        2018-06-13  2953  	if ((n < 0)  && (n != -EAGAIN)) {
c19707b8 Varsha Rao        2018-06-13  2954  		if (n == -ENOSPC)
c19707b8 Varsha Rao        2018-06-13  2955  			ida_destroy(&inuse);
c19707b8 Varsha Rao        2018-06-13  2956  		return n;
60eb1894 Patrick McHardy   2014-03-07  2957  	}
c19707b8 Varsha Rao        2018-06-13  2958  
c19707b8 Varsha Rao        2018-06-13  2959  		ida_destroy(&inuse);
20a69341 Patrick McHardy   2013-10-11  2960  	}
20a69341 Patrick McHardy   2013-10-11  2961  
c19707b8 Varsha Rao        2018-06-13 @2962  	set->name = kasprintf(GFP_KERNEL, name, id);
38745490 Phil Sutter       2017-07-27  2963  	if (!set->name)
38745490 Phil Sutter       2017-07-27  2964  		return -ENOMEM;
38745490 Phil Sutter       2017-07-27  2965  
20a69341 Patrick McHardy   2013-10-11  2966  	list_for_each_entry(i, &ctx->table->sets, list) {
37a9cc52 Pablo Neira Ayuso 2016-06-12  2967  		if (!nft_is_active_next(ctx->net, i))
37a9cc52 Pablo Neira Ayuso 2016-06-12  2968  			continue;
e63aaaa6 Arvind Yadav      2017-09-20  2969  		if (!strcmp(set->name, i->name)) {
e63aaaa6 Arvind Yadav      2017-09-20  2970  			kfree(set->name);
20a69341 Patrick McHardy   2013-10-11  2971  			return -ENFILE;
20a69341 Patrick McHardy   2013-10-11  2972  		}
e63aaaa6 Arvind Yadav      2017-09-20  2973  	}
96518518 Patrick McHardy   2013-10-14  2974  	return 0;
96518518 Patrick McHardy   2013-10-14  2975  }
20a69341 Patrick McHardy   2013-10-11  2976  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ca4c4d994ddb..7f6e164607a9 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2919,46 +2919,47 @@  struct nft_set *nft_set_lookup_global(const struct net *net,
 }
 EXPORT_SYMBOL_GPL(nft_set_lookup_global);
 
+#define NFT_SET_IDA_SIZE (8 * _BITUL(12))
+
 static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
 				    const char *name)
 {
 	const struct nft_set *i;
 	const char *p;
-	unsigned long *inuse;
-	unsigned int n = 0, min = 0;
+	int n = 0, id;
+	DEFINE_IDA(inuse);
 
 	p = strchr(name, '%');
 	if (p != NULL) {
 		if (p[1] != 'd' || strchr(p + 2, '%'))
 			return -EINVAL;
 
-		inuse = (unsigned long *)get_zeroed_page(GFP_KERNEL);
-		if (inuse == NULL)
-			return -ENOMEM;
-cont:
-		list_for_each_entry(i, &ctx->table->sets, list) {
-			int tmp;
+	list_for_each_entry(i, &ctx->table->sets, list) {
+		int tmp;
 
-			if (!nft_is_active_next(ctx->net, set))
-				continue;
-			if (!sscanf(i->name, name, &tmp))
-				continue;
-			if (tmp < min || tmp >= min + BITS_PER_BYTE * PAGE_SIZE)
-				continue;
+		if (!nft_is_active_next(ctx->net, set))
+			continue;
+		if (!sscanf(i->name, name, &tmp))
+			continue;
+		if (tmp < 0 || tmp >= NFT_SET_IDA_SIZE)
+			continue;
 
-			set_bit(tmp - min, inuse);
-		}
+		n = ida_get_new_above(&inuse, tmp, &id);
+		if ((n < 0) && (n != -EAGAIN))
+			return n;
+	}
+	n = ida_get_new_above(&inuse, 0, &id);
 
-		n = find_first_zero_bit(inuse, BITS_PER_BYTE * PAGE_SIZE);
-		if (n >= BITS_PER_BYTE * PAGE_SIZE) {
-			min += BITS_PER_BYTE * PAGE_SIZE;
-			memset(inuse, 0, PAGE_SIZE);
-			goto cont;
-		}
-		free_page((unsigned long)inuse);
+	if ((n < 0)  && (n != -EAGAIN)) {
+		if (n == -ENOSPC)
+			ida_destroy(&inuse);
+		return n;
+	}
+
+		ida_destroy(&inuse);
 	}
 
-	set->name = kasprintf(GFP_KERNEL, name, min + n);
+	set->name = kasprintf(GFP_KERNEL, name, id);
 	if (!set->name)
 		return -ENOMEM;