Re: [nf-next PATCH] netfilter: nf_tables: add support for inverted login in nft_lookup

[Arturo Borrero Gonzalez <arturo.borrero.glez@xxxxxxxxx>]

On 31 May 2016 at 17:50, Arturo Borrero Gonzalez
<arturo.borrero.glez@xxxxxxxxx> wrote:
> On 31 May 2016 at 16:44, Florian Westphal <fw@xxxxxxxxx> wrote:
>> Arturo Borrero Gonzalez <arturo.borrero.glez@xxxxxxxxx> wrote:
>>> -     if (set->ops->lookup(set, &regs->data[priv->sreg], &ext)) {
>>> +     if (set->ops->lookup(set, &regs->data[priv->sreg], &ext) ^
>>> +         priv->invert) {
>>>               if (set->flags & NFT_SET_MAP)
>>>                       nft_data_copy(&regs->data[priv->dreg],
>>>                                     nft_set_ext_data(ext), set->dlen);
>>
>> Whats the plan for SET_MAP here?
>> You enter 'lookup found a result' branch here in case we did not find
>> anything and invert is set.
>>
>> I think its better to use a
>>
>> } else if (priv->invert) {
>>         return;
>> }
>>

Not that easy, we should consider 4 cases:
 * lookup false, invert false -> NFT_BREAK
 * lookup false, invert true -> return w/o NFT_BREAK
 * lookup true, invert false -> return w/o NFT_BREAK
 * lookup true, invert true -> NFT_BREAK

The XOR approach in my original patch seems to over these cases, ie,
we enter the return branch only if lookup&invert are different.
I think the nft_data_copy() should not worry us if we prevent the
combination from happening in _init()
The only penalty seems the additional boolean check for NFT_SET_MAP
(which must always fail if priv->invert is true)

So, I'm updating the _init() function with the additional restrictions.

Am I missing something else?
-- 
Arturo Borrero González
--
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