On Wed, Mar 21, 2018 at 05:44:06PM +0100, Phil Sutter wrote:
> On Wed, Mar 21, 2018 at 12:34:53PM +0100, Pablo Neira Ayuso wrote:
> > Hi Phil,
> >
> > On Wed, Mar 21, 2018 at 12:07:53PM +0100, Phil Sutter wrote:
> > > Previously, creating a set of type ipv4_addr with timeout flag failed:
> > > nft_hash_select_ops() returned nft_hash_fast_ops despite that it doesn't
> > > support timeout feature. Fix this by making the given flags part of the
> > > selection process and return only backend ops which support all of them.
> > >
> > > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > > ---
> > > net/netfilter/nft_set_hash.c | 17 ++++++++++++++---
> > > 1 file changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
> > > index 3f1624ee056f9..3c8b10a556f7c 100644
> > > --- a/net/netfilter/nft_set_hash.c
> > > +++ b/net/netfilter/nft_set_hash.c
> > > @@ -670,6 +670,11 @@ static struct nft_set_ops nft_hash_fast_ops __read_mostly = {
> > > .features = NFT_SET_MAP | NFT_SET_OBJECT,
> > > };
> > >
> > > +static bool ops_have_features(const struct nft_set_ops *ops, u32 features)
> > > +{
> > > + return (ops->features & features) == features;
> > > +}
> > > +
> > > static const struct nft_set_ops *
> > > nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc,
> > > u32 flags)
> > > @@ -677,13 +682,19 @@ nft_hash_select_ops(const struct nft_ctx *ctx, const struct nft_set_desc *desc,
> > > if (desc->size) {
> > > switch (desc->klen) {
> > > case 4:
> > > - return &nft_hash_fast_ops;
> > > + if (ops_have_features(&nft_hash_fast_ops, flags))
> > > + return &nft_hash_fast_ops;
> > > + /* fall through */
> > > default:
> > > - return &nft_hash_ops;
> > > + if (ops_have_features(&nft_hash_ops, flags))
> > > + return &nft_hash_ops;
> > > }
> > > }
> > >
> > > - return &nft_rhash_ops;
> > > + if (ops_have_features(&nft_rhash_ops, flags))
> > > + return &nft_rhash_ops;
> > > +
> > > + return NULL;
> > > }
> >
> > This is clashing with existing fixes in nf.git.
> >
> > I think the backend selection needs a rework, this is the idea:
> >
> > 1) Register all set_ops in one single list (instead of the _type thing).
>
> Yes, that makes sense.
>
> > 2) Iterate over the list of _ops, select the one that fits better.
>
> I wonder how nft_hash_fast_ops' constraint on key length could be
> respected by the selection algorithm. Maybe introduce a new keylen
> attribute to struct nft_set_ops?
I think we can pass the keylen to ->select_ops().
> OTOH this makes me wonder whether nft_hash_fast_ops could support
> smaller key lengths as well. (Does this case exist at all?)
<= 2 bytes key is not worth, we have bitmaps. I think we should remove
modularity from sets and place them built-in.
--
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
