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? OTOH this makes me wonder whether
nft_hash_fast_ops could support smaller key lengths as well. (Does
this case exist at all?)
Cheers, Phil
--
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
