On Thu, Sep 19, 2019 at 01:56:36PM +0200, Florian Westphal wrote: [...] > My conclusion is that meters are anon sets with expressions attached to them. > So, I don't think they should be deprecated. At least, meters with names should be I think. There is now a way to represent them > I would propose to: > > 1. add this kernel patch: > > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c > --- a/net/netfilter/nf_tables_api.c > +++ b/net/netfilter/nf_tables_api.c > @@ -3562,8 +3562,11 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, > NFT_SET_OBJECT)) > return -EINVAL; > /* Only one of these operations is supported */ > - if ((flags & (NFT_SET_MAP | NFT_SET_EVAL | NFT_SET_OBJECT)) == > - (NFT_SET_MAP | NFT_SET_EVAL | NFT_SET_OBJECT)) > + if ((flags & (NFT_SET_MAP | NFT_SET_OBJECT)) == > + (NFT_SET_MAP | NFT_SET_OBJECT)) That's fine by now. But look, combining map and objects should be fine in the future. A user might want to combine this by specifying an IP address as the right hand side of a mapping and a stateful counter (with a name) to be updated when matching on that element. This is not supported yet though. > + return -EOPNOTSUPP; > + if ((flags & (NFT_SET_EVAL | NFT_SET_OBJECT)) == > + (NFT_SET_EVAL | NFT_SET_OBJECT)) This looks fine. > return -EOPNOTSUPP; > } > > diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c > --- a/net/netfilter/nft_lookup.c > +++ b/net/netfilter/nft_lookup.c > @@ -73,9 +73,6 @@ static int nft_lookup_init(const struct nft_ctx *ctx, > if (IS_ERR(set)) > return PTR_ERR(set); > > - if (set->flags & NFT_SET_EVAL) > - return -EOPNOTSUPP; > - > priv->sreg = nft_parse_register(tb[NFTA_LOOKUP_SREG]); > err = nft_validate_register_load(priv->sreg, set->klen); > if (err < 0) > > 2. avoid depreaction of 'meter', since thats what is documented everywhere > and appears to work fine OK, but only for anonymous meters. We have a better way, more aligned to set/map definitions, to represent updates to dynamic named sets from the packet path now. We can probably introduce a syntax more aligned to the existing set/map syntax for the _anonymous_ dynamic set/map case, so we don't need this 'meter' keyword syntactic sugar. > 3. patch nft to hide normal sets from 'nft list meters' output This makes no sense for anonymous meters. Since the kernel picks the name, I don't think nft should expose them to the user. > What to do with dynamic, I fear we have to remove it, i.e. just ignore > the "dynamic" flag when talking to the kernel. Otherwise sets using dynamic flag > will only work on future/very recent kernels. I would not go that path. You are just hitting a limitation on the existing implementation. People cannot make lookups on a dynamic set on existing kernels, that's all. There is no NFT_SET_EXPR support for nft_lookup either, is this a bug? This infrastructure is just incomplete and just need to be finished. There is no way to combine NFT_SET_MAP with NFT_SET_OBJ, but that is also artificial, this is just happening again because the code is incomplete and needs an extension.
