Hello Pablo,
On Mon, Mar 12, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> Hi Varsha,
>
> On Sat, Feb 17, 2018 at 12:19:18AM +0530, Varsha Rao wrote:
> > In nf_tables_set_alloc_name function, remove get_zeroed_page
> > find_first_zero_bit and set_bit functions. Instead use ida_simple_get
> > function as it simplifies the code.
> >
> > Signed-off-by: Varsha Rao <rvarsha016@xxxxxxxxx>
> > ---
> > net/netfilter/nf_tables_api.c | 27 +++++++++++----------------
> > 1 file changed, 11 insertions(+), 16 deletions(-)
> >
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 0791813a1e7d..08fbb5ffab3a 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -2653,18 +2653,14 @@ static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
> > {
> > const struct nft_set *i;
> > const char *p;
> > - unsigned long *inuse;
> > - unsigned int n = 0, min = 0;
> > + int n = 0;
> > + 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;
> >
> > @@ -2672,22 +2668,21 @@ static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
> > continue;
> > if (!sscanf(i->name, name, &tmp))
> > continue;
> > - if (tmp < min || tmp >= min + BITS_PER_BYTE * PAGE_SIZE)
> > + if (tmp < 0 || tmp >= BITS_PER_BYTE * PAGE_SIZE)
> > continue;
> >
> > - set_bit(tmp - min, inuse);
> > + n = ida_simple_get(&inuse, tmp, BITS_PER_BYTE * PAGE_SIZE, GFP_KERNEL);
> > + if (n < 0)
> > + return n;
> > }
> > + n = ida_simple_get(&inuse, 0, BITS_PER_BYTE * PAGE_SIZE, GFP_KERNEL);
> > + if (n < 0)
> > + return n;
> >
> > - 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);
> > + ida_destroy(&inuse);
>
> I think after this patch, we end up having an upper limit of PAGE_SIZE
> sets. This limit was not there before.
>
I will change it back.
>
> BTW, could we just do a full conversion to ida? I mean, we could store
> the struct ida in struct nft_table, so we don't need to rebuild this
> bitmap everytime we need to allocate a set.
>
> It will consume more memory but that's reasonable.
I think it is better to construct rather than add ida to struct nf_table.
As we are destroying the structure after getting the required id. It is
okay to allocate id while traversing the list and after completion of list
traversal.
Thanks,
Varsha
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
