On Wed, Sep 18, 2019 at 04:42:35PM +0200, Florian Westphal wrote:
> Laura Garcia <nevola@xxxxxxxxx> wrote:
> > > Following example loads fine:
> > > table ip NAT {
> > > set set1 {
> > > type ipv4_addr
> > > size 64
> > > flags dynamic,timeout
> > > timeout 1m
> > > }
> > >
> > > chain PREROUTING {
> > > type nat hook prerouting priority -101; policy accept;
> > > }
> > > }
> > >
> > > But adding/using this set doesn't work:
> > > nft -- add rule NAT PREROUTING tcp dport 80 ip saddr @set1 counter
> > > Error: Could not process rule: Operation not supported
> >
> > If this set is only for matching, 'dynamic' is not required.
>
> I know, and the example above works if the 'dynamic' flag is omitted.
Looks like a kernel bug, kernel is selecting the fixed size hash with
the dynamic flag. That should not happen.
> > > This is because the 'dynamic' flag sets NFT_SET_EVAL.
> > >
> > > According to kernel comment, that flag means:
> > > * @NFT_SET_EVAL: set can be updated from the evaluation path
> > >
> > > The rule add is rejected from the lookup expression (nft_lookup_init)
> > > which has:
> > >
> > > if (set->flags & NFT_SET_EVAL)
> > > return -EOPNOTSUPP;
> > >
> > > From looking at the git history, the NFT_SET_EVAL flag means that the
> > > set contains expressions (i.e., a meter).
> > >
> > > And I can see why doing a lookup on meters isn't meaningful.
> > >
> > > Can someone please explain the exact precise meaning of 'dynamic'?
> > > Was it supposed to mean 'set can be updated from packet path'?
> > > Or was it supposed to mean 'set contains expressions'?
> > >
> >
> > AFAIK, I traduce the 'dynamic' flag as a 'set that is updated from the
> > packet path using an expression', formerly 'meter'.
>
> That would mean the kernel comment quoted above is wrong and should be
> patched to say
>
> * @NFT_SET_EVAL: set can be updated from the packet path and stores expressions.
The comment is correct. NFT_SET_EVAL semantics is "this set can be
updated from the packet path", this helps the kernel selects a set
backend that implements the ->update interface.
The expression is option, if the netlink attribute to describe the
extension is set, then it used.
> Unfortunately, this seems to contradict various nftables.git changelog
> messages which seem to imply that 'dynamic' means
>
> @NFT_SET_EVAL: set can be updated from the evaluation path
>
> i.e., make sure that set provides an ->update() implementation so
>
> 'add @set1 { ip saddr }' and the like work.
>
> The core issue is that when saying
>
> set set1 {
> type ipv4_addr
> size 64
> flags timeout
> timeout 1m
> }
>
> The kernel has no information on how this set is going to be used.
> For 'ip saddr @set1' this will just work as all sets implement
> ->lookup().
>
> But will this work reliably:
> nft add rule ... add @set1 { ip saddr }
No, because the dynamic flag is not set.
> At this time, it looks like when specifiying the mandatory 'timeout'
> flag, the kernel happens to pick the rhashtable backend so it will work.
Probably it would be good to implicitly set on timeout flag if dynamic
flag is set on, just like it happens with set size. To relax the
control plane interface a bit (IIRC the user currently gets an error
if you forget to set the timeout flag in a set that is updated from
the packet path).
> But I wonder if we're just lucky or if this is intentional, i.e.
> 'timeout' means that the set can be altered from the packet path.
I think we're just being lucky :-)
> In any case, this needs to be documented in nft.8, its telling that I
> can't be 100% about the intent of dynamic/EVAL even after reading both
> nftables.git and kernel implementation.
Sure.
