Re: [PATCH V2 iptables] extensions: libxt_connlabel: Add translation to nft

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sat, Jul 16, 2016 at 08:12:51PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > On Sat, Jul 16, 2016 at 04:51:30PM +0200, Pablo Neira Ayuso wrote:
> > > > On Sat, Jul 16, 2016 at 06:42:24PM +0800, Liping Zhang wrote:
> > > > >   # iptables-translate -A INPUT -m connlabel ! --label bit40 --set
> > > > >   nft add rule ip filter INPUT ct label set bit40 ct label and bit40 != bit40 counter
> > > > 
> > > > I think this logic is inverted, I mean:
> > > > 
> > > > nft add rule ip filter INPUT ct label and bit40 != bit40 ct label set bit40 counter
> > > >                              ---------------------------
> > > > 
> > > > test should happen before set.
> > > 
> > > BTW, why not simply translate this to:
> > > 
> > >         nft add rule ip filter INPUT ct label set bit40 counter
> > 
> > Its not the same as the bloated version.
> > 
> > The set operation will only ever fail in case the conntrack doesn't have a label
> >
> > extension or is untracked/invalid, but if that is the case we get
> > different results:
> > 
> > nft add rule ip filter INPUT ct label set bit40 ct label and bit40 != bit40 counter
> > 
> > -> counter Increments for every packet that lacks a conntrack, or the
> > conntrack extension
> > 
> > nft add rule ip filter INPUT ct label set bit40 counter
> > 
> > -> counter Increments for every packet (we don't set NFT_BREAK anywhere
> > in the setter).
> 
> set operations are not expected to return anything at all, they must
> always evaluate true.

Yes.

> This behaviour is deviating from what we have in other set operations,
> this is clearly inconsistent.

How so?