Re: [iptables PATCH v2 06/10] nft: Introduce struct nft_chain

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

On Wed, Sep 23, 2020 at 07:48:45PM +0200, Phil Sutter wrote:
> Preparing for ordered output of user-defined chains, introduce a local
> datatype wrapping nftnl_chain. In order to maintain the chain name hash
> table, introduce nft_chain_list as well and use it instead of
> nftnl_chain_list.
> 
> Put everything into a dedicated source file and provide a bunch of
> getters for attributes of the embedded libnftnl_chain object.
[...]
> diff --git a/iptables/nft-chain.h b/iptables/nft-chain.h
> new file mode 100644
> index 0000000000000..818bbf1f4b525
> --- /dev/null
> +++ b/iptables/nft-chain.h
> @@ -0,0 +1,87 @@
> +#ifndef _NFT_CHAIN_H_
> +#define _NFT_CHAIN_H_
> +
> +#include <libnftnl/chain.h>
> +#include <libiptc/linux_list.h>
> +
> +struct nft_handle;
> +
> +struct nft_chain {
> +	struct list_head	head;
> +	struct hlist_node	hnode;
> +	struct nftnl_chain	*nftnl;
> +};
> +
> +#define CHAIN_NAME_HSIZE	512
> +
> +struct nft_chain_list {
> +	struct list_head	list;
> +	struct hlist_head	names[CHAIN_NAME_HSIZE];
> +};
> +
> +struct nft_chain *nft_chain_alloc(struct nftnl_chain *nftnl);
> +void nft_chain_free(struct nft_chain *c);
> +
> +struct nft_chain_list *nft_chain_list_alloc(void);
> +void nft_chain_list_free(struct nft_chain_list *list);
> +void nft_chain_list_del(struct nft_chain *c);
> +
> +static inline const char *nft_chain_name(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_str(c->nftnl, NFTNL_CHAIN_NAME);
> +}
> +
> +static inline const char *nft_chain_table(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_str(c->nftnl, NFTNL_CHAIN_TABLE);
> +}
> +
> +static inline const char *nft_chain_type(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_str(c->nftnl, NFTNL_CHAIN_TYPE);
> +}
> +
> +static inline uint32_t nft_chain_prio(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u32(c->nftnl, NFTNL_CHAIN_PRIO);
> +}
> +
> +static inline uint32_t nft_chain_hooknum(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u32(c->nftnl, NFTNL_CHAIN_HOOKNUM);
> +}
> +
> +static inline uint64_t nft_chain_packets(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u64(c->nftnl, NFTNL_CHAIN_PACKETS);
> +}
> +
> +static inline uint64_t nft_chain_bytes(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u64(c->nftnl, NFTNL_CHAIN_BYTES);
> +}
> +
> +static inline bool nft_chain_has_policy(struct nft_chain *c)
> +{
> +	return nftnl_chain_is_set(c->nftnl, NFTNL_CHAIN_POLICY);
> +}
> +
> +static inline uint32_t nft_chain_policy(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u32(c->nftnl, NFTNL_CHAIN_POLICY);
> +}
> +
> +static inline uint32_t nft_chain_use(struct nft_chain *c)
> +{
> +	return nftnl_chain_get_u32(c->nftnl, NFTNL_CHAIN_USE);
> +}

Do you need this wrapper functions now? I mean, the intention is to
have a native nft_chain structure so nft_chain_use() become:

static inline uint32_t nft_chain_use(struct nft_chain *c)
{
	return c->use;
}

at some point?

Sorry but I don't see this is happening in this batch?

I remember the original intention was to support for sorting chains,
so the listing is predictable. But this batch is updating more things
than that and I don't see a clear connection with the goal.

Thanks Phil.