Re: [xtables-arptables PATCH v2 3/5] nft: nft_xtables_config_load() called only in nft_init()

[Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>]

Hi Pablo,

>> +int nft_init(struct nft_handle *h, struct builtin_table *t,
>> >+	     const char *filename)
>                           ^^^^^^^^
>
> why do we need this new parameter?
>
> The optional /etc/xtables.conf file should contain the definition for
> all the families, that includes IPv4, IPv6, bridge and ARP.

My mistake, I advised that. Imho, it's not relevant to load everything 
for all families if the user is using only xtables-iptables for instance.

Then ok, let's have one file, but we could make a change in 
nft_xtables_config_load() so it would load only the tables of the 
current family.
It's a quick fix to do actually.

>
>> >  {
>> >  	h->nl = mnl_socket_open(NETLINK_NETFILTER);
>> >  	if (h->nl == NULL) {
>> >@@ -388,6 +389,16 @@ int nft_init(struct nft_handle *h, struct builtin_table *t)
>> >  	h->portid = mnl_socket_get_portid(h->nl);
>> >  	h->tables = t;
>> >  
>> >+	/* If built-in chains don't exist for this table, create them */
>> >+	if (nft_xtables_config_load(h, filename, 0) < 0) {
>> >+		int i;
>> >+
>> >+		if (h->tables != NULL) {
>> >+			for (i=0; i<TABLES_MAX; i++)
>> >+				nft_chain_builtin_init(h, h->tables[i].name,
>> >+						       NULL, NF_ACCEPT);
>> >+		}
>> >+	}
> I don't see what we get by moving nft_xtables_config_load here.

Why not calling this at only one unique place instead of multiple ones?
You need to call that anyway currently as soon as you list/change the 
rule set.
It's relevant to move that here, makes code clearer.

Cheers,

Tomasz