Re: [PATCH] nft: Add support for inverted bitwise value list

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Jun 22, 2016 at 05:49:48PM +0200, Laura Garcia Liebana wrote:
> Add support for inverted state and status bitwise value list required in the
> ct match.
> 
> Before this patch, nft didn't support the rule:
> 
> $ nft add rule ip filter INPUT ct state != new,related counter accept
> <cmdline>:1:41-41: Error: syntax error, unexpected comma, expecting end of file or newline or semicolon
> add rule ip filter INPUT ct state != new,related counter accept
>                                         ^
> 
> This patch includes in the parser the ability to understand a list of
> bitwise values.
> 
> nft --debug=netlink add rule ip filter INPUT ct state != new,related,established,untracked counter accept
> ip filter INPUT
>   [ ct load state => reg 1 ]
>   [ cmp neq reg 1 0x0000004e ]
>   [ counter pkts 0 bytes 0 ]
>   [ immediate reg 0 accept ]

This bytecode looks incorrect.

nft --debug=netlink add rule ip filter INPUT ct state new,related,established,untracked
ip filter INPUT 
  [ ct load state => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0x0000004e ) ^ 0x00000000 ]
  [ cmp neq reg 1 0x00000000 ]

so I think the right bytecode should look like:

nft --debug=netlink add rule ip filter INPUT ct state new,related,established,untracked
ip filter INPUT 
  [ ct load state => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0x0000004e ) ^ 0x00000000 ]
  [ cmp eq reg 1 0x00000000 ] 

I guess something is missing from the expr_evaluate_relational(), I
can see:

        if (rel->op == OP_IMPLICIT) {
                switch (right->ops->type) {
                ...
                case EXPR_LIST:
                        rel->op = OP_FLAGCMP;

I guess rel->op is OP_NEQ for your case above, that's why it is
generating the wrong code.

Note that from netlink_linearize.c, it is netlink_gen_flagcmp() that
generates the bitwise + cmp when we see OP_FLAGCMP.

Instead of this, I would kill the OP_FLAGCMP and transform the left
hand side of the tree to get a bitwise from evaluate.c, so this looks
like:

        relational (OP_NEQ)
                / \
               /   \
              /     \
         bitwise   value
            /  \
           /    \
     ct state   mask

Then, we can kill netlink_gen_flagcmp() too since the
netlink_linearize.c will generate the right bytecode for us based on
that tree.