[iptables,v2,01/10] nft: Fix selective chain compatibility checks

Since commit 80251bc2a56ed ("nft: remove cache build calls"), 'chain'
parameter passed to nft_chain_list_get() is no longer effective. To
still support running nft_is_chain_compatible() on specific chains only,
add a short path to nft_is_table_compatible().

Follow-up patches will kill nft_chain_list_get(), so don't bother
dropping the unused parameter from its signature.

Fixes: 80251bc2a56ed ("nft: remove cache build calls")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 6 ++++++
 1 file changed, 6 insertions(+)

On Wed, Sep 23, 2020 at 07:48:40PM +0200, Phil Sutter wrote:
> Since commit 80251bc2a56ed ("nft: remove cache build calls"), 'chain'
> parameter passed to nft_chain_list_get() is no longer effective. To
> still support running nft_is_chain_compatible() on specific chains only,
> add a short path to nft_is_table_compatible().
> 
> Follow-up patches will kill nft_chain_list_get(), so don't bother
> dropping the unused parameter from its signature.

This has a Fixes: tag.

What is precisely the problem? How does show from the iptables and
iptables-restore interface?

Not sure I understand the problem.

> Fixes: 80251bc2a56ed ("nft: remove cache build calls")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  iptables/nft.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 27bb98d184c7c..669e29d4cf88f 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -3453,6 +3453,12 @@ bool nft_is_table_compatible(struct nft_handle *h,
>  {
>  	struct nftnl_chain_list *clist;
>  
> +	if (chain) {
> +		struct nftnl_chain *c = nft_chain_find(h, table, chain);
> +
> +		return c && !nft_is_chain_compatible(c, h);
> +	}
> +
>  	clist = nft_chain_list_get(h, table, chain);
>  	if (clist == NULL)
>  		return false;
> -- 
> 2.28.0
>

Hi Pablo,

On Mon, Oct 12, 2020 at 01:54:24PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Sep 23, 2020 at 07:48:40PM +0200, Phil Sutter wrote:
> > Since commit 80251bc2a56ed ("nft: remove cache build calls"), 'chain'
> > parameter passed to nft_chain_list_get() is no longer effective. To
> > still support running nft_is_chain_compatible() on specific chains only,
> > add a short path to nft_is_table_compatible().
> > 
> > Follow-up patches will kill nft_chain_list_get(), so don't bother
> > dropping the unused parameter from its signature.
> 
> This has a Fixes: tag.
> 
> What is precisely the problem? How does show from the iptables and
> iptables-restore interface?
> 
> Not sure I understand the problem.

Before the big caching rework, nft_chain_list_get() could cause a
cache-fetch to populate table's chain list. If a chain name was passed
to it, that cache-fetch would retrieve only the specified chain (if not
in cache already).

In the current code, nft_is_table_compatible() happens in the second
stage where cache is present already and nft_chain_list_get() really
just returns the table's chain list again. This means that the
compatibility check happens for all chains in cache which belong to that
table and the original optimization (to check only the interesting
chain) is not effective anymore.

The "short path" (actually: short-cut) introduced in this patch allows
to limit compat check to a specific chain again. The impact is not as
big anymore as nft_chain_list_get() does no longer populate the cache,
but still there's no point in checking unintereresting chains'
compatibility.

Above all else though, this is fallout from the preparations to drop
nft_chain_list_get() and given the above, I found it makes sense to push
it as a separate commit.

Cheers, Phil