Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [iptables PATCH v2 02/10] nft: Implement nft_chain_foreach()
Date: Mon, 12 Oct 2020 14:01:18 +0200
Message-ID: <20201012120118.GB26845@salvia> (raw)
In-Reply-To: <20200923174849.5773-3-phil@nwl.cc>

On Wed, Sep 23, 2020 at 07:48:41PM +0200, Phil Sutter wrote:
> This is just a fancy wrapper around nftnl_chain_list_foreach() with the
> added benefit of detecting invalid table names or uninitialized chain
> lists. This in turn allows to drop the checks in flush_rule_cache() and
> ignore the return code of nft_chain_foreach() as it fails only if the
> dropped checks had failed, too.

At quick glance, this is reducing the LoC.

However, I'm not sure this is better, before this code:

1) You fetch the list
2) You use it from several spots in the function

with this patch you might look up for the chain list several times in
the same function.

+int nft_chain_foreach(struct nft_handle *h, const char *table,                
+                   int (*cb)(struct nftnl_chain *c, void *data),              
+                   void *data)                                                
+{                                                                             
+     const struct builtin_table *t;                                           
+                                                                              
+     t = nft_table_builtin_find(h, table);                                    
+     if (!t)                                                                  
+             return -1;                                                       
+                                                                              
+     if (!h->cache->table[t->type].chains)                                    
+             return -1;                                                       
+                                                                              
+     return nftnl_chain_list_foreach(h->cache->table[t->type].chains,         
+                                     cb, data);                               
+}

I can also see calls to:

nft_chain_find(h, table, chain);

and

nft_chain_foreach(...)

from the same function.

This patch also updates paths in very different ways, there is no
common idiom being replaced.

  reply index

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23 17:48 [iptables PATCH v2 00/10] nft: Sorted chain listing et al Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 01/10] nft: Fix selective chain compatibility checks Phil Sutter
2020-10-12 11:54   ` Pablo Neira Ayuso
2020-10-13  9:29     ` Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 02/10] nft: Implement nft_chain_foreach() Phil Sutter
2020-10-12 12:01   ` Pablo Neira Ayuso [this message]
2020-10-13  9:40     ` Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 03/10] nft: cache: Introduce nft_cache_add_chain() Phil Sutter
2020-10-12 12:02   ` Pablo Neira Ayuso
2020-09-23 17:48 ` [iptables PATCH v2 04/10] nft: Eliminate nft_chain_list_get() Phil Sutter
2020-10-12 12:03   ` Pablo Neira Ayuso
2020-10-13  9:44     ` Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 05/10] nft: cache: Move nft_chain_find() over Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 06/10] nft: Introduce struct nft_chain Phil Sutter
2020-10-12 12:08   ` Pablo Neira Ayuso
2020-10-13  9:56     ` Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 07/10] nft: Introduce a dedicated base chain array Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 08/10] nft: cache: Sort custom chains by name Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 09/10] tests: shell: Drop any dump sorting in place Phil Sutter
2020-09-23 17:48 ` [iptables PATCH v2 10/10] nft: Avoid pointless table/chain creation Phil Sutter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201012120118.GB26845@salvia \
    --to=pablo@netfilter.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git