linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Matthew Wilcox <willy@infradead.org>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	NetFilter <netfilter-devel@vger.kernel.org>,
	Linux-Next Mailing List <linux-next@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Varsha Rao <rvarsha016@gmail.com>
Subject: Re: linux-next: build failure after merge of the ida tree
Date: Wed, 18 Jul 2018 16:25:00 +0200	[thread overview]
Message-ID: <20180718142500.5ltqff7ajwmwidhp@salvia> (raw)
In-Reply-To: <20180718133126.GD4949@bombadil.infradead.org>

On Wed, Jul 18, 2018 at 06:31:26AM -0700, Matthew Wilcox wrote:
> On Wed, Jul 18, 2018 at 03:27:46PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Jul 18, 2018 at 06:14:46AM -0700, Matthew Wilcox wrote:
> > > So Varsha, if you would like to take a look at transforming table->sets
> > > from a LIST_HEAD to an IDR, I think that would be a great use of your
> > > time.
> > 
> > Please, don't do so, we don't need a radix tree datastructure, it's
> > just more complexity.
> 
> It's no more complex to use than the list_* macros.

Problem is that some of the sets that we place in that list may have
no ID.

We basically have two type of sets:

* Sets with names, they have no IDs as the user provides a meaningful
  name from the control plane that can be used to add/delete elements,
  eg. IP addresses.

* Anonymous sets, these are built-in into rules, eg.

  ip saddr { 1.1.1.1, 2.2.2.2 }

  so we generate an ID that we can use to refer to the set.

For our usecase, I'm thinking, if we don't have a simple way to
allocate IDs through this API, we could just simplify our existing
codebase by using an u64 and use incremental id, we don't need to
recycle IDs, so that's one posibility I stop bothering you ;-)

BTW, the anti-pattern we have in our codebase is the same logic that we
have to allocate identifiers with netdevice name, see __dev_alloc_name()
in net/core/dev.c. *Someone* copied + pasted + mangled that original code
to make it fit into netfilter. I guess that code may benefit from a
simple way to allocate IDs without locking dependencies. Just an idea,
not that this is a priority.

Thanks!

  reply	other threads:[~2018-07-18 14:25 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-18  6:54 linux-next: build failure after merge of the ida tree Stephen Rothwell
2018-07-18  9:24 ` Pablo Neira Ayuso
2018-07-18 11:59   ` Matthew Wilcox
2018-07-18 13:25     ` Pablo Neira Ayuso
2018-07-18 13:14   ` Matthew Wilcox
2018-07-18 13:27     ` Pablo Neira Ayuso
2018-07-18 13:31       ` Matthew Wilcox
2018-07-18 14:25         ` Pablo Neira Ayuso [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-08-10  6:36 Stephen Rothwell
2018-07-05  5:13 Stephen Rothwell
2018-07-05  4:36 Stephen Rothwell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180718142500.5ltqff7ajwmwidhp@salvia \
    --to=pablo@netfilter.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=rvarsha016@gmail.com \
    --cc=sfr@canb.auug.org.au \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).