Re: [PATCH nft 3/5] src: add cache level flags

[Phil Sutter <phil@nwl.cc>]

Hi,

On Mon, Jun 17, 2019 at 02:25:16PM +0200, Pablo Neira Ayuso wrote:
> The score approach based on command type is confusing.
> 
> This patch introduces cache level flags, each flag specifies what kind
> of object type is needed. These flags are set on/off depending on the
> list of commands coming in this batch.
> 
> cache_is_complete() now checks if the cache contains the objects that
> are needed in the cache through these new flags.

Thanks for committing to getting the cache stuff right!

[...]
> +enum cache_level_flags {
> +	NFT_CACHE_EMPTY		= 0,
> +	NFT_CACHE_TABLE		= (1 << 0),
> +	NFT_CACHE_CHAIN 	= (1 << 1),
> +	NFT_CACHE_SET 		= (1 << 2),
> +	NFT_CACHE_FLOWTABLE	= (1 << 3),
> +	NFT_CACHE_OBJECT	= (1 << 4),
> +	NFT_CACHE_SETELEM 	= (1 << 5),
> +	NFT_CACHE_RULE		= (1 << 6),
> +	NFT_CACHE_FULL		= (NFT_CACHE_TABLE	|
> +				   NFT_CACHE_CHAIN	|
> +				   NFT_CACHE_SET	|
> +				   NFT_CACHE_FLOWTABLE	|
> +				   NFT_CACHE_OBJECT	|
> +				   NFT_CACHE_SETELEM	|
> +				   NFT_CACHE_RULE),
> +};

I think we can do this in a way which reflects the implicit dependencies
when fetching ruleset elements. I think of something like:

| enum nft_cache_bits {
| 	NFT_CACHE_TABLE_BIT	= (1 << 0),
| 	NFT_CACHE_CHAIN_BIT	= (1 << 1),
| 	NFT_CACHE_SET_BIT	= (1 << 2),
| 	NFT_CACHE_FLOWTABLE_BIT	= (1 << 3),
| 	NFT_CACHE_OBJECT_BIT	= (1 << 4),
| 	NFT_CACHE_SETELEM_BIT	= (1 << 5),
| 	NFT_CACHE_RULE_BIT	= (1 << 6),
| 	__NFT_CACHE_MAX_BIT	= (1 << 7),
| };
| 
| enum cache_level_flags {
| 	NFT_CACHE_EMPTY		= 0,
| 	NFT_CACHE_TABLE		= NFT_CACHE_TABLE_BIT,
| 	NFT_CACHE_CHAIN		= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_CHAIN_BIT,
| 	NFT_CACHE_SET		= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_SET_BIT,
| 	NFT_CACHE_FLOWTABLE	= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_FLOWTABLE_BIT,
| 	NFT_CACHE_OBJECT	= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_OBJECT_BIT,
| 	NFT_CACHE_SETELEM	= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_SET_BIT
| 				| NFT_CACHE_SETELEM_BIT,
| 	NFT_CACHE_RULE		= NFT_CACHE_TABLE_BIT
| 				| NFT_CACHE_CHAIN_BIT
| 				| NFT_CACHE_SETELEM_BIT
| 				| NFT_CACHE_RULE_BIT,
| 	NFT_CACHE_FULL		= __NFT_CACHE_MAX_BIT - 1,
| };

This removes these dependency details from cache_evaluate() functions.
What do you think?

Cheers, Phil