Re: [nf-next PATCH] net: nftables: Respect hash set backend features

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux