[prev in list] [next in list] [prev in thread] [next in thread] 

List:       netfilter-devel
Subject:    Re: [iptables PATCH v3 17/21] xtables: Optimize list command with given chain
From:       Phil Sutter <phil () nwl ! cc>
Date:       2018-12-30 12:10:13
Message-ID: 20181230121013.GM17224 () orbyte ! nwl ! cc
[Download RAW message or body]

On Thu, Dec 27, 2018 at 08:54:08PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Dec 20, 2018 at 04:09:18PM +0100, Phil Sutter wrote:
> > Make use of nftnl_chain_list_lookup_byname() even if not listing a
> > specific rule. Introduce __nft_print_header() to consolidate chain value
> > extraction for printing with ops->print_header().
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  iptables/nft.c | 78 +++++++++++++++++++++-----------------------------
> >  1 file changed, 32 insertions(+), 46 deletions(-)
> > 
> > diff --git a/iptables/nft.c b/iptables/nft.c
> > index 250cae0a34e37..b11c390edcc10 100644
> > --- a/iptables/nft.c
> > +++ b/iptables/nft.c
> > @@ -2247,6 +2247,24 @@ static int nft_rule_count(struct nft_handle *h, struct nftnl_chain *c)
> >  	return rule_ctr;
> >  }
> >  
> > +static void __nft_print_header(struct nft_handle *h,
> > +			       const struct nft_family_ops *ops,
> > +			       struct nftnl_chain *c, unsigned int format)
> > +{
> > +	const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
> > +	uint32_t policy = nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY);
> > +	bool basechain = !!nftnl_chain_get(c, NFTNL_CHAIN_HOOKNUM);
> > +	uint32_t refs = nftnl_chain_get_u32(c, NFTNL_CHAIN_USE);
> > +	uint32_t entries = nft_rule_count(h, c);
> > +	struct xt_counters ctrs = {
> > +		.pcnt = nftnl_chain_get_u64(c, NFTNL_CHAIN_PACKETS),
> > +		.bcnt = nftnl_chain_get_u64(c, NFTNL_CHAIN_BYTES),
> > +	};
> 
> Maybe we can introduce a container structure for this.
> 
> > +	ops->print_header(format, chain_name, policy_name[policy],
> > +			&ctrs, basechain, refs - entries, entries);
> 
> So we can pass it to ->print_header.
> 
> I would have preferred you add this in a initial patch, makes it
> harder to review. Please do so in the future.

Sorry for the inconvenience this caused.

I don't quite get your idea: Would you like to have a function
extracting the chain data into that container structure so
nft_rule_list() still calls ops->print_header() directly?

I can still do that, just let me know please.

Cheers, Phil
[prev in list] [next in list] [prev in thread] [next in thread]