netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Giuseppe Longo <giuseppelng@gmail.com>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [xtables-arptables PATCH v2 3/5] nft: nft_xtables_config_load() called only in nft_init()
Date: Wed, 24 Jul 2013 10:56:49 +0200	[thread overview]
Message-ID: <20130724085649.GC16434@localhost> (raw)
In-Reply-To: <20130723161244.10040.57825.stgit@localhost>

On Tue, Jul 23, 2013 at 06:12:47PM +0200, Giuseppe Longo wrote:
> Signed-off-by: Giuseppe Longo <giuseppelng@gmail.com>
> ---
>  iptables/nft.c                |   33 ++++++++++++---------------------
>  iptables/nft.h                |    2 +-
>  iptables/xtables-config.c     |    5 ++---
>  iptables/xtables-restore.c    |   16 ++++++++--------
>  iptables/xtables-save.c       |   15 ++++++++-------
>  iptables/xtables-standalone.c |   14 +++-----------
>  iptables/xtables.c            |    5 +++++
>  7 files changed, 39 insertions(+), 51 deletions(-)
> 
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 07ca0f1..589cba7 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> @@ -373,7 +373,8 @@ static bool nft_chain_builtin(struct nft_chain *c)
>  	return nft_chain_attr_get(c, NFT_CHAIN_ATTR_HOOKNUM) != NULL;
>  }
>  
> -int nft_init(struct nft_handle *h, struct builtin_table *t)
> +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.

>  {
>  	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.

>  	return 0;
>  }
>  
> @@ -742,10 +753,6 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
>  	uint16_t flags = NLM_F_ACK|NLM_F_CREATE;
>  	int ret = 1;
>  
> -	/* If built-in chains don't exist for this table, create them */
> -	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
> -		nft_chain_builtin_init(h, table, chain, NF_ACCEPT);
> -
>  	nft_fn = nft_rule_append;
>  
>  	r = nft_rule_new(h, chain, table, cs);
> @@ -1316,10 +1323,6 @@ int nft_chain_user_add(struct nft_handle *h, const char *chain, const char *tabl
>  	struct nft_chain *c;
>  	int ret;
>  
> -	/* If built-in chains don't exist for this table, create them */
> -	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
> -		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
> -
>  	c = nft_chain_alloc();
>  	if (c == NULL)
>  		return 0;
> @@ -1472,10 +1475,6 @@ int nft_chain_user_rename(struct nft_handle *h,const char *chain,
>  	uint64_t handle;
>  	int ret;
>  
> -	/* If built-in chains don't exist for this table, create them */
> -	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
> -		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
> -
>  	/* Find the old chain to be renamed */
>  	c = nft_chain_find(h, table, chain);
>  	if (c == NULL) {
> @@ -2170,10 +2169,6 @@ int nft_rule_insert(struct nft_handle *h, const char *chain,
>  	struct nft_rule *r;
>  	uint64_t handle;
>  
> -	/* If built-in chains don't exist for this table, create them */
> -	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
> -		nft_chain_builtin_init(h, table, chain, NF_ACCEPT);
> -
>  	nft_fn = nft_rule_insert;
>  
>  	list = nft_rule_list_create(h);
> @@ -2521,10 +2516,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
>  	struct nft_chain *c;
>  	bool found = false;
>  
> -	/* If built-in chains don't exist for this table, create them */
> -	if (nft_xtables_config_load(h, XTABLES_CONFIG_DEFAULT, 0) < 0)
> -		nft_chain_builtin_init(h, table, NULL, NF_ACCEPT);
> -
>  	list = nft_chain_dump(h);
>  
>  	iter = nft_chain_list_iter_create(list);
> diff --git a/iptables/nft.h b/iptables/nft.h
> index e4d177e..abf0463 100644
> --- a/iptables/nft.h
> +++ b/iptables/nft.h
> @@ -33,7 +33,7 @@ struct nft_handle {
>  	struct builtin_table	*tables;
>  };
>  
> -int nft_init(struct nft_handle *h, struct builtin_table *t);
> +int nft_init(struct nft_handle *h, struct builtin_table *t, const char *filename);
>  void nft_fini(struct nft_handle *h);
>  
>  /*
> diff --git a/iptables/xtables-config.c b/iptables/xtables-config.c
> index bb87886..277e33e 100644
> --- a/iptables/xtables-config.c
> +++ b/iptables/xtables-config.c
> @@ -37,12 +37,11 @@ int xtables_config_main(int argc, char *argv[])
>  	else
>  		filename = argv[1];
>  
> -	if (nft_init(&h, tables) < 0) {
> +	if (nft_init(&h, tables, filename) < 0) {
>                  fprintf(stderr, "Failed to initialize nft: %s\n",
>  			strerror(errno));
>  		return EXIT_FAILURE;
>  	}
>  
> -	return nft_xtables_config_load(&h, filename, NFT_LOAD_VERBOSE) == 0 ?
> -						    EXIT_SUCCESS : EXIT_FAILURE;
> +	return EXIT_SUCCESS;
>  }
> diff --git a/iptables/xtables-restore.c b/iptables/xtables-restore.c
> index b894173..3893734 100644
> --- a/iptables/xtables-restore.c
> +++ b/iptables/xtables-restore.c
> @@ -194,14 +194,6 @@ xtables_restore_main(int argc, char *argv[])
>  	init_extensions4();
>  #endif
>  
> -	if (nft_init(&h, tables) < 0) {
> -		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
> -				xtables_globals.program_name,
> -				xtables_globals.program_version,
> -				strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> -
>  	while ((c = getopt_long(argc, argv, "bcvthnM:T:46", options, NULL)) != -1) {
>  		switch (c) {
>  			case 'b':
> @@ -239,6 +231,14 @@ xtables_restore_main(int argc, char *argv[])
>  		}
>  	}
>  
> +        if (nft_init(&h, tables, XTABLES_CONFIG_DEFAULT) < 0) {
> +                fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
> +                                xtables_globals.program_name,
> +                                xtables_globals.program_version,
> +                                strerror(errno));
> +                exit(EXIT_FAILURE);
> +        }
> +
>  	if (optind == argc - 1) {
>  		in = fopen(argv[optind], "re");
>  		if (!in) {
> diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
> index 8a5c991..897e805 100644
> --- a/iptables/xtables-save.c
> +++ b/iptables/xtables-save.c
> @@ -97,13 +97,6 @@ xtables_save_main(int argc, char *argv[])
>  	init_extensions();
>  	init_extensions4();
>  #endif
> -	if (nft_init(&h, tables) < 0) {
> -		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
> -				xtables_globals.program_name,
> -				xtables_globals.program_version,
> -				strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
>  
>  	while ((c = getopt_long(argc, argv, "bcdt:46", options, NULL)) != -1) {
>  		switch (c) {
> @@ -131,6 +124,14 @@ xtables_save_main(int argc, char *argv[])
>  		}
>  	}
>  
> +        if (nft_init(&h, tables, XTABLES_CONFIG_DEFAULT) < 0) {
> +                fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
> +                                xtables_globals.program_name,
> +                                xtables_globals.program_version,
> +                                strerror(errno));
> +                exit(EXIT_FAILURE);
> +        }
> +
>  	if (optind < argc) {
>  		fprintf(stderr, "Unknown arguments found on commandline\n");
>  		exit(1);
> diff --git a/iptables/xtables-standalone.c b/iptables/xtables-standalone.c
> index bd95ff8..212b293 100644
> --- a/iptables/xtables-standalone.c
> +++ b/iptables/xtables-standalone.c
> @@ -46,9 +46,9 @@ xtables_main(int argc, char *argv[])
>  {
>  	int ret;
>  	char *table = "filter";
> -	struct nft_handle h;
> -
> -	memset(&h, 0, sizeof(h));
> +	struct nft_handle h = {
> +		.family = AF_INET,
> +	};
>  
>  	xtables_globals.program_name = "xtables";
>  	ret = xtables_init_all(&xtables_globals, NFPROTO_IPV4);
> @@ -63,14 +63,6 @@ xtables_main(int argc, char *argv[])
>  	init_extensions4();
>  #endif
>  
> -	if (nft_init(&h, tables) < 0) {
> -		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
> -				xtables_globals.program_name,
> -				xtables_globals.program_version,
> -				strerror(errno));
> -		exit(EXIT_FAILURE);
> -	}
> -
>  	ret = do_commandx(&h, argc, argv, &table);
>  	if (!ret) {
>  		if (errno == EINVAL) {
> diff --git a/iptables/xtables.c b/iptables/xtables.c
> index 65e4882..d4b8709 100644
> --- a/iptables/xtables.c
> +++ b/iptables/xtables.c
> @@ -1100,6 +1100,11 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
>  	if (h->ops == NULL)
>  		xtables_error(PARAMETER_PROBLEM, "Unknown family");
>  
> +	if (h->tables == NULL) {
> +                if (nft_init(h, tables, XTABLES_CONFIG_DEFAULT) < 0)
> +                        xtables_error(OTHER_PROBLEM, "Could not initialize nftables layer.");
> +        }
> +
>  	h->ops->post_parse(command, &cs, &args);
>  
>  	if (command == CMD_REPLACE &&
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2013-07-24  8:56 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-23 16:11 [xtables-arptables PATCH v2 0/5] nft changes for xtables-arptables Giuseppe Longo
2013-07-23 16:12 ` [xtables-arptables PATCH v2 1/5] nft: add builtin_table pointer Giuseppe Longo
2013-07-24  8:50   ` Pablo Neira Ayuso
2013-07-23 16:12 ` [xtables-arptables PATCH v2 2/5] nft: search builtin tables via nft_handle tables pointer Giuseppe Longo
2013-07-24  8:51   ` Pablo Neira Ayuso
2013-07-23 16:12 ` [xtables-arptables PATCH v2 3/5] nft: nft_xtables_config_load() called only in nft_init() Giuseppe Longo
2013-07-24  8:56   ` Pablo Neira Ayuso [this message]
2013-07-24  9:36     ` Tomasz Bursztyka
2013-07-24 10:56       ` Pablo Neira Ayuso
2013-07-24 11:14         ` Tomasz Bursztyka
2013-07-24 11:55           ` Pablo Neira Ayuso
2013-07-24  9:01   ` Pablo Neira Ayuso
2013-07-23 16:12 ` [xtables-arptables PATCH v2 4/5] nft: make functions public Giuseppe Longo
2013-07-23 16:13 ` [xtables-arptables PATCH v2 5/5] nft: replace args.family Giuseppe Longo
2013-07-24  8:58   ` Pablo Neira Ayuso
2013-07-24  9:40     ` Tomasz Bursztyka
2013-07-23 16:16 ` [xtables-arptables PATCH v2 0/5] nft changes for xtables-arptables Tomasz Bursztyka

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=20130724085649.GC16434@localhost \
    --to=pablo@netfilter.org \
    --cc=giuseppelng@gmail.com \
    --cc=netfilter-devel@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).