Hello Pablo,

On Mon, Mar 12, 2018 at 2:33 AM, Pablo Neira Ayuso <pa...@netfilter.org> 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 <rvarsha...@gmail.com>
> > ---
> >  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to