netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement
@ 2012-11-01 11:38 Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 1/4] nf_tables: Change chain's name to be fixed sized Tomasz Bursztyka
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Tomasz Bursztyka @ 2012-11-01 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Tomasz Bursztyka

Hi,

Here a second version of the previous patchset, now taking care of chain's name issue. The name is fixed,
same size as in iptables.

About an alternative, it could be possible to store a variable chain name in a separate allocated memory,
kept in hash table: not the chain object itself, just the name. The key would be the chain's object pointer.
At least, from execution point of view it would not affect anything since the name is necessary only for
the user. The only issue is the memory occupied by such hash table.

I was looking quickly at the linux helpers, could not find a hashtable/map api, is there any? If so, I could
try this idea of names stored like that.

Tomasz Bursztyka (4):
  nf_tables: Change chain's name to be fixed sized
  nf_tables: Add missing policy for NFTA_CHAIN_USE
  nf_tables: Add support for changing users chain's name
  nf_tables: Add support for replacing a rule by another one.

 include/linux/netfilter/nf_tables.h |  3 ++
 include/net/netfilter/nf_tables.h   |  2 +-
 net/netfilter/nf_tables_api.c       | 87 +++++++++++++++++++++++++++++++------
 3 files changed, 77 insertions(+), 15 deletions(-)

-- 
1.7.12.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [nf-next/nf_tables-experiments - v2 PATCH 1/4] nf_tables: Change chain's name to be fixed sized
  2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
@ 2012-11-01 11:38 ` Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 2/4] nf_tables: Add missing policy for NFTA_CHAIN_USE Tomasz Bursztyka
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Tomasz Bursztyka @ 2012-11-01 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Tomasz Bursztyka

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/linux/netfilter/nf_tables.h |  2 ++
 include/net/netfilter/nf_tables.h   |  2 +-
 net/netfilter/nf_tables_api.c       | 17 +++++++++--------
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 0115a2f..8962657 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -1,6 +1,8 @@
 #ifndef _LINUX_NF_TABLES_H
 #define _LINUX_NF_TABLES_H
 
+#define NFT_CHAIN_MAXNAMELEN 32
+
 enum nft_registers {
 	NFT_REG_VERDICT,
 	NFT_REG_1,
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 8ce0db4..74b8b770 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -357,7 +357,7 @@ struct nft_chain {
 	u16				use;
 	u16				level;
 	u16				hgenerator;
-	char				name[];
+	char				name[NFT_CHAIN_MAXNAMELEN];
 };
 
 /**
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e0e4616..a04139c 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -478,7 +478,8 @@ static struct nft_chain *nf_tables_chain_lookup(const struct nft_table *table,
 }
 
 static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
-	[NFTA_CHAIN_NAME]	= { .type = NLA_STRING },
+	[NFTA_CHAIN_NAME]	= { .type = NLA_STRING,
+				    .len = NFT_CHAIN_MAXNAMELEN - 1 },
 	[NFTA_CHAIN_TABLE]	= { .type = NLA_STRING },
 	[NFTA_CHAIN_HOOK]	= { .type = NLA_NESTED },
 	[NFTA_CHAIN_POLICY]	= { .type = NLA_U32 },
@@ -687,7 +688,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	struct nft_chain *chain;
 	struct nft_base_chain *basechain = NULL;
 	struct nlattr *ha[NFTA_HOOK_MAX + 1];
-	unsigned int size;
 	int family = nfmsg->nfgen_family;
 	int err;
 	bool create;
@@ -723,7 +723,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		return 0;
 	}
 
-	size = nla_len(name);
 	if (nla[NFTA_CHAIN_HOOK]) {
 		struct nf_hook_ops *ops;
 
@@ -737,7 +736,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		if (ntohl(nla_get_be32(ha[NFTA_HOOK_HOOKNUM])) >= afi->nhooks)
 			return -EINVAL;
 
-		basechain = kzalloc(sizeof(*basechain) + size, GFP_KERNEL);
+		basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
 		if (basechain == NULL)
 			return -ENOMEM;
 		chain = &basechain->chain;
@@ -764,7 +763,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 			}
 		}
 	} else {
-		chain = kzalloc(sizeof(*chain) + size, GFP_KERNEL);
+		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
 		if (chain == NULL)
 			return -ENOMEM;
 
@@ -772,7 +771,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	}
 
 	INIT_LIST_HEAD(&chain->rules);
-	nla_strlcpy(chain->name, name, size);
+	nla_strlcpy(chain->name, name, NFT_CHAIN_MAXNAMELEN);
 
 	list_add_tail(&chain->list, &table->chains);
 
@@ -1054,7 +1053,8 @@ static u16 nf_tables_rule_alloc_handle(struct nft_chain *chain)
 
 static const struct nla_policy nft_rule_policy[NFTA_RULE_MAX + 1] = {
 	[NFTA_RULE_TABLE]	= { .type = NLA_STRING },
-	[NFTA_RULE_CHAIN]	= { .type = NLA_STRING },
+	[NFTA_RULE_CHAIN]	= { .type = NLA_STRING,
+				    .len = NFT_CHAIN_MAXNAMELEN - 1 },
 	[NFTA_RULE_HANDLE]	= { .type = NLA_U16 },
 	[NFTA_RULE_EXPRESSIONS]	= { .type = NLA_NESTED },
 };
@@ -2454,7 +2454,8 @@ EXPORT_SYMBOL_GPL(nft_validate_data_load);
 
 static const struct nla_policy nft_verdict_policy[NFTA_VERDICT_MAX + 1] = {
 	[NFTA_VERDICT_CODE]	= { .type = NLA_U32 },
-	[NFTA_VERDICT_CHAIN]	= { .type = NLA_STRING },
+	[NFTA_VERDICT_CHAIN]	= { .type = NLA_STRING,
+				    .len = NFT_CHAIN_MAXNAMELEN - 1 },
 };
 
 static int nft_verdict_init(const struct nft_ctx *ctx, struct nft_data *data,
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [nf-next/nf_tables-experiments - v2 PATCH 2/4] nf_tables: Add missing policy for NFTA_CHAIN_USE
  2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 1/4] nf_tables: Change chain's name to be fixed sized Tomasz Bursztyka
@ 2012-11-01 11:38 ` Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 3/4] nf_tables: Add support for changing users chain's name Tomasz Bursztyka
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Tomasz Bursztyka @ 2012-11-01 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Tomasz Bursztyka

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 net/netfilter/nf_tables_api.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index a04139c..14ae11d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -483,6 +483,7 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
 	[NFTA_CHAIN_TABLE]	= { .type = NLA_STRING },
 	[NFTA_CHAIN_HOOK]	= { .type = NLA_NESTED },
 	[NFTA_CHAIN_POLICY]	= { .type = NLA_U32 },
+	[NFTA_CHAIN_USE]	= { .type = NLA_U32 },
 };
 
 static const struct nla_policy nft_hook_policy[NFTA_HOOK_MAX + 1] = {
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [nf-next/nf_tables-experiments - v2 PATCH 3/4] nf_tables: Add support for changing users chain's name
  2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 1/4] nf_tables: Change chain's name to be fixed sized Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 2/4] nf_tables: Add missing policy for NFTA_CHAIN_USE Tomasz Bursztyka
@ 2012-11-01 11:38 ` Tomasz Bursztyka
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 4/4] nf_tables: Add support for replacing a rule by another one Tomasz Bursztyka
  2012-11-01 15:40 ` [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Pablo Neira Ayuso
  4 siblings, 0 replies; 6+ messages in thread
From: Tomasz Bursztyka @ 2012-11-01 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Tomasz Bursztyka

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 include/linux/netfilter/nf_tables.h |  1 +
 net/netfilter/nf_tables_api.c       | 45 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 8962657..63297b4 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -69,6 +69,7 @@ enum nft_chain_attributes {
 	NFTA_CHAIN_HOOK,
 	NFTA_CHAIN_POLICY,
 	NFTA_CHAIN_USE,
+	NFTA_CHAIN_NEW_NAME,
 	__NFTA_CHAIN_MAX
 };
 #define NFTA_CHAIN_MAX		(__NFTA_CHAIN_MAX - 1)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 14ae11d..43eccbb 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -484,6 +484,8 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
 	[NFTA_CHAIN_HOOK]	= { .type = NLA_NESTED },
 	[NFTA_CHAIN_POLICY]	= { .type = NLA_U32 },
 	[NFTA_CHAIN_USE]	= { .type = NLA_U32 },
+	[NFTA_CHAIN_NEW_NAME]	= { .type = NLA_STRING,
+				    .len = NFT_CHAIN_MAXNAMELEN - 1 },
 };
 
 static const struct nla_policy nft_hook_policy[NFTA_HOOK_MAX + 1] = {
@@ -678,6 +680,47 @@ nf_tables_chain_policy(struct nft_chain *chain, const struct nlattr *attr)
 	return 0;
 }
 
+static int nf_tables_mvchain(struct sk_buff *skb, const struct nlmsghdr *nlh,
+			     struct nft_table *table,
+			     struct nft_chain *chain,
+			     const struct nlattr * const nla[])
+{
+	const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
+	int family = nfmsg->nfgen_family;
+	struct nft_chain *new_chain;
+	struct nft_chain old_chain;
+
+	if (!nla[NFTA_CHAIN_NEW_NAME])
+		return -EINVAL;
+
+	if (chain->flags & NFT_CHAIN_BUILTIN ||
+	    chain->flags & NFT_BASE_CHAIN)
+		return -EOPNOTSUPP;
+
+	new_chain = nf_tables_chain_lookup(table, nla[NFTA_CHAIN_NEW_NAME]);
+	if (IS_ERR(new_chain)) {
+		if (PTR_ERR(new_chain) != -ENOENT)
+			return PTR_ERR(new_chain);
+		new_chain = NULL;
+	}
+
+	if (new_chain != NULL)
+		return -EEXIST;
+
+	new_chain = chain;
+
+	nla_strlcpy(old_chain.name,
+		    nla[NFTA_CHAIN_NAME], NFT_CHAIN_MAXNAMELEN);
+	nla_strlcpy(new_chain->name,
+		    nla[NFTA_CHAIN_NEW_NAME], NFT_CHAIN_MAXNAMELEN);
+
+	nf_tables_chain_notify(skb, nlh, table, &old_chain, NFT_MSG_DELCHAIN,
+			       family);
+	nf_tables_chain_notify(skb, nlh, table, new_chain, NFT_MSG_NEWCHAIN,
+			       family);
+	return 0;
+}
+
 static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 			      const struct nlmsghdr *nlh,
 			      const struct nlattr * const nla[])
@@ -715,7 +758,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 		if (nlh->nlmsg_flags & NLM_F_EXCL)
 			return -EEXIST;
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
-			return -EOPNOTSUPP;
+			return nf_tables_mvchain(skb, nlh, table, chain, nla);
 
 		if ((chain->flags & NFT_BASE_CHAIN) && nla[NFTA_CHAIN_POLICY]) {
 			return nf_tables_chain_policy(chain,
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [nf-next/nf_tables-experiments - v2 PATCH 4/4] nf_tables: Add support for replacing a rule by another one.
  2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
                   ` (2 preceding siblings ...)
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 3/4] nf_tables: Add support for changing users chain's name Tomasz Bursztyka
@ 2012-11-01 11:38 ` Tomasz Bursztyka
  2012-11-01 15:40 ` [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Pablo Neira Ayuso
  4 siblings, 0 replies; 6+ messages in thread
From: Tomasz Bursztyka @ 2012-11-01 11:38 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Tomasz Bursztyka

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 net/netfilter/nf_tables_api.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 43eccbb..72881d2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1309,7 +1309,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 	const struct nft_af_info *afi;
 	const struct nft_table *table;
 	struct nft_chain *chain;
-	struct nft_rule *rule;
+	struct nft_rule *rule, *old_rule = NULL;
 	struct nft_expr_info info[NFT_RULE_MAXEXPRS];
 	struct nft_expr *expr;
 	struct nft_ctx ctx;
@@ -1345,9 +1345,11 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 		if (rule != NULL) {
 			if (nlh->nlmsg_flags & NLM_F_EXCL)
 				return -EEXIST;
-			if (nlh->nlmsg_flags & NLM_F_REPLACE)
-				return -EOPNOTSUPP;
-			return 0;
+			if (nlh->nlmsg_flags & NLM_F_REPLACE) {
+				old_rule = rule;
+				rule = NULL;
+			} else
+				return 0;
 		}
 	} else
 		handle = nf_tables_rule_alloc_handle(chain);
@@ -1390,7 +1392,19 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 		expr = nft_expr_next(expr);
 	}
 
-	if (nlh->nlmsg_flags & NLM_F_APPEND)
+	if (nlh->nlmsg_flags & NLM_F_REPLACE) {
+		if (old_rule == NULL)
+			goto err2;
+
+		list_replace_rcu(&old_rule->list, &rule->list);
+
+		// FIXME: this makes deletion performance *really* suck
+		synchronize_rcu();
+
+		nf_tables_rule_notify(skb, nlh, table, chain, old_rule,
+				      NFT_MSG_DELRULE, nfmsg->nfgen_family);
+		nf_tables_rule_destroy(&ctx, old_rule);
+	} else if (nlh->nlmsg_flags & NLM_F_APPEND)
 		list_add_tail_rcu(&rule->list, &chain->rules);
 	else
 		list_add_rcu(&rule->list, &chain->rules);
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement
  2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
                   ` (3 preceding siblings ...)
  2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 4/4] nf_tables: Add support for replacing a rule by another one Tomasz Bursztyka
@ 2012-11-01 15:40 ` Pablo Neira Ayuso
  4 siblings, 0 replies; 6+ messages in thread
From: Pablo Neira Ayuso @ 2012-11-01 15:40 UTC (permalink / raw)
  To: Tomasz Bursztyka; +Cc: netfilter-devel

Hi Tomasz,

On Thu, Nov 01, 2012 at 01:38:29PM +0200, Tomasz Bursztyka wrote:
> Hi,
> 
> Here a second version of the previous patchset, now taking care of
> chain's name issue. The name is fixed, same size as in iptables.
> 
> About an alternative, it could be possible to store a variable chain
> name in a separate allocated memory, kept in hash table: not the
> chain object itself, just the name. The key would be the chain's
> object pointer.  At least, from execution point of view it would not
> affect anything since the name is necessary only for the user. The
> only issue is the memory occupied by such hash table.

We can declare a const char * inside nft_chain and kmalloc the area
for the name dynamically. If renamed, we can reallocation it. It's
simple, but we increase memory fragmentation.

> I was looking quickly at the linux helpers, could not find a
> hashtable/map api, is there any? If so, I could try this idea of
> names stored like that.

There have been some discussion regarding generic hashtable API since
long time.

http://www.spinics.net/lists/linux-nfs/msg33526.html

It seems Linus is going to take it.

> Tomasz Bursztyka (4):
>   nf_tables: Change chain's name to be fixed sized
>   nf_tables: Add missing policy for NFTA_CHAIN_USE
>   nf_tables: Add support for changing users chain's name
>   nf_tables: Add support for replacing a rule by another one.

I've taken these three to nf_tables-experiments branch.

Thanks a lot.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-11-01 15:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-11-01 11:38 [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Tomasz Bursztyka
2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 1/4] nf_tables: Change chain's name to be fixed sized Tomasz Bursztyka
2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 2/4] nf_tables: Add missing policy for NFTA_CHAIN_USE Tomasz Bursztyka
2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 3/4] nf_tables: Add support for changing users chain's name Tomasz Bursztyka
2012-11-01 11:38 ` [nf-next/nf_tables-experiments - v2 PATCH 4/4] nf_tables: Add support for replacing a rule by another one Tomasz Bursztyka
2012-11-01 15:40 ` [nf-next/nf_tables-experiments - v2 PATCH 0/4] fixed chain name, chain rename and rule replacement Pablo Neira Ayuso

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