* [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