* [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits
2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list Pablo Neira Ayuso
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw)
To: netfilter-devel
Get rid of the extra struct list_head per rule. With this patch, a
temporary object is allocated to store the rule update information.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 15 ++++++-
net/netfilter/nf_tables_api.c | 83 ++++++++++++++++++++++++-------------
2 files changed, 67 insertions(+), 31 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 215edf5..fe08cf4 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -321,7 +321,6 @@ static inline void *nft_expr_priv(const struct nft_expr *expr)
* struct nft_rule - nf_tables rule
*
* @list: used internally
- * @dirty_list: this rule needs an update after new generation
* @rcu_head: used internally for rcu
* @handle: rule handle
* @genmask: generation mask
@@ -330,7 +329,6 @@ static inline void *nft_expr_priv(const struct nft_expr *expr)
*/
struct nft_rule {
struct list_head list;
- struct list_head dirty_list;
struct rcu_head rcu_head;
u64 handle:46,
genmask:2,
@@ -339,6 +337,19 @@ struct nft_rule {
__attribute__((aligned(__alignof__(struct nft_expr))));
};
+/**
+ * struct nft_rule_trans - nf_tables rule update in transaction
+ *
+ * @list: used internally
+ * @rule: rule that needs to be updated
+ * @family: family expressesed as AF_*
+ */
+struct nft_rule_trans {
+ struct list_head list;
+ struct nft_rule *rule;
+ u8 family;
+};
+
static inline struct nft_expr *nft_expr_first(const struct nft_rule *rule)
{
return (struct nft_expr *)&rule->data[0];
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index c5d0129..f099d0d 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1537,6 +1537,22 @@ static void nf_tables_rule_destroy(struct nft_rule *rule)
static struct nft_expr_info *info;
+static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
+{
+ struct nft_rule_trans *rupd;
+ struct nft_chain *chain = (struct nft_chain *)ctx->chain;
+
+ rupd = kmalloc(sizeof(struct nft_rule_trans), GFP_KERNEL);
+ if (rupd == NULL)
+ return -ENOMEM;
+
+ rupd->rule = rule;
+ rupd->family = ctx->afi->family;
+ list_add(&rupd->list, &chain->dirty_rules);
+
+ return 0;
+}
+
static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
@@ -1664,7 +1680,9 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
}
if (flags & NFT_RULE_F_COMMIT)
- list_add(&rule->dirty_list, &chain->dirty_rules);
+ err = nf_tables_trans_add(rule, &ctx);
+ if (err < 0)
+ goto err2;
else {
nf_tables_rule_notify(skb, nlh, table, chain, rule,
NFT_MSG_NEWRULE,
@@ -1683,14 +1701,14 @@ err1:
return err;
}
-static void
+static int
nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
{
- if (flags & NFT_RULE_F_COMMIT) {
- struct nft_chain *chain = (struct nft_chain *)ctx->chain;
+ int err = 0;
+ if (flags & NFT_RULE_F_COMMIT) {
nft_rule_disactivate_next(ctx->net, rule);
- list_add(&rule->dirty_list, &chain->dirty_rules);
+ err = nf_tables_trans_add(rule, ctx);
} else {
list_del_rcu(&rule->list);
nf_tables_rule_notify(ctx->skb, ctx->nlh, ctx->table,
@@ -1698,6 +1716,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
0, ctx->afi->family);
nf_tables_rule_destroy(rule);
}
+ return err;
}
static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
@@ -1710,7 +1729,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
const struct nft_table *table;
struct nft_chain *chain;
struct nft_rule *rule, *tmp;
- int family = nfmsg->nfgen_family;
+ int family = nfmsg->nfgen_family, err = 0;
struct nft_ctx ctx;
u32 flags = 0;
@@ -1740,14 +1759,17 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
if (IS_ERR(rule))
return PTR_ERR(rule);
- nf_tables_delrule_one(&ctx, rule, flags);
+ err = nf_tables_delrule_one(&ctx, rule, flags);
} else {
/* Remove all rules in this chain */
- list_for_each_entry_safe(rule, tmp, &chain->rules, list)
- nf_tables_delrule_one(&ctx, rule, flags);
+ list_for_each_entry_safe(rule, tmp, &chain->rules, list) {
+ err = nf_tables_delrule_one(&ctx, rule, flags);
+ if (err < 0)
+ break;
+ }
}
- return 0;
+ return err;
}
static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
@@ -1759,8 +1781,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
struct net *net = sock_net(skb->sk);
struct nft_table *table;
struct nft_chain *chain;
- struct nft_rule *rule, *tmp;
- int family = nfmsg->nfgen_family;
+ struct nft_rule_trans *rupd, *tmp;
bool create;
create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1782,31 +1803,33 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry(chain, &table->chains, list) {
- list_for_each_entry_safe(rule, tmp, &chain->dirty_rules, dirty_list) {
+ list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
/* Delete this rule from the dirty list */
- list_del(&rule->dirty_list);
+ list_del(&rupd->list);
/* This rule was inactive in the past and just
* became active. Clear the next bit of the
* genmask since its meaning has changed, now
* it is the future.
*/
- if (nft_rule_is_active(net, rule)) {
- nft_rule_clear(net, rule);
+ if (nft_rule_is_active(net, rupd->rule)) {
+ nft_rule_clear(net, rupd->rule);
nf_tables_rule_notify(skb, nlh, table,
- chain, rule,
+ chain, rupd->rule,
NFT_MSG_NEWRULE,
0,
- nfmsg->nfgen_family);
+ rupd->family);
+ kfree(rupd);
continue;
}
/* This rule is in the past, get rid of it */
- list_del_rcu(&rule->list);
+ list_del_rcu(&rupd->rule->list);
nf_tables_rule_notify(skb, nlh, table, chain,
- rule, NFT_MSG_DELRULE, 0,
- family);
- nf_tables_rule_destroy(rule);
+ rupd->rule, NFT_MSG_DELRULE, 0,
+ rupd->family);
+ nf_tables_rule_destroy(rupd->rule);
+ kfree(rupd);
}
}
}
@@ -1823,7 +1846,7 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
struct net *net = sock_net(skb->sk);
struct nft_table *table;
struct nft_chain *chain;
- struct nft_rule *rule, *tmp;
+ struct nft_rule_trans *rupd, *tmp;
bool create;
create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1834,18 +1857,20 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
list_for_each_entry(table, &afi->tables, list) {
list_for_each_entry(chain, &table->chains, list) {
- list_for_each_entry_safe(rule, tmp, &chain->dirty_rules, dirty_list) {
+ list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
/* Delete all rules from the dirty list */
- list_del(&rule->dirty_list);
+ list_del(&rupd->list);
- if (!nft_rule_is_active_next(net, rule)) {
- nft_rule_clear(net, rule);
+ if (!nft_rule_is_active_next(net, rupd->rule)) {
+ nft_rule_clear(net, rupd->rule);
+ kfree(rupd);
continue;
}
/* This rule is inactive, get rid of it */
- list_del_rcu(&rule->list);
- nf_tables_rule_destroy(rule);
+ list_del_rcu(&rupd->rule->list);
+ nf_tables_rule_destroy(rupd->rule);
+ kfree(rupd);
}
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list
2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw)
To: netfilter-devel
Instead of one list per chain.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 6 +-
include/net/netns/nftables.h | 1 +
net/netfilter/nf_tables_api.c | 109 +++++++++++++------------------------
3 files changed, 44 insertions(+), 72 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index fe08cf4..6b644c2 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -342,11 +342,15 @@ struct nft_rule {
*
* @list: used internally
* @rule: rule that needs to be updated
+ * @chain: chain that this rule belongs to
+ * @table: table for which this chain applies
* @family: family expressesed as AF_*
*/
struct nft_rule_trans {
struct list_head list;
struct nft_rule *rule;
+ const struct nft_chain *chain;
+ const struct nft_table *table;
u8 family;
};
@@ -383,7 +387,6 @@ enum nft_chain_flags {
* struct nft_chain - nf_tables chain
*
* @rules: list of rules in the chain
- * @dirty_rules: rules that need an update after next generation
* @list: used internally
* @rcu_head: used internally
* @net: net namespace that this chain belongs to
@@ -396,7 +399,6 @@ enum nft_chain_flags {
*/
struct nft_chain {
struct list_head rules;
- struct list_head dirty_rules;
struct list_head list;
struct rcu_head rcu_head;
struct net *net;
diff --git a/include/net/netns/nftables.h b/include/net/netns/nftables.h
index 9b35901..15d056d 100644
--- a/include/net/netns/nftables.h
+++ b/include/net/netns/nftables.h
@@ -7,6 +7,7 @@ struct nft_af_info;
struct netns_nftables {
struct list_head af_info;
+ struct list_head commit_list;
struct nft_af_info *ipv4;
struct nft_af_info *ipv6;
struct nft_af_info *arp;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index f099d0d..8c78d36 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -976,7 +976,6 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
}
INIT_LIST_HEAD(&chain->rules);
- INIT_LIST_HEAD(&chain->dirty_rules);
chain->handle = nf_tables_alloc_handle(table);
chain->net = net;
chain->table = table;
@@ -1540,15 +1539,16 @@ static struct nft_expr_info *info;
static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
{
struct nft_rule_trans *rupd;
- struct nft_chain *chain = (struct nft_chain *)ctx->chain;
rupd = kmalloc(sizeof(struct nft_rule_trans), GFP_KERNEL);
if (rupd == NULL)
return -ENOMEM;
+ rupd->chain = ctx->chain;
+ rupd->table = ctx->table;
rupd->rule = rule;
rupd->family = ctx->afi->family;
- list_add(&rupd->list, &chain->dirty_rules);
+ list_add(&rupd->list, &ctx->net->nft.commit_list);
return 0;
}
@@ -1776,19 +1776,8 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
- const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
- const struct nft_af_info *afi;
struct net *net = sock_net(skb->sk);
- struct nft_table *table;
- struct nft_chain *chain;
struct nft_rule_trans *rupd, *tmp;
- bool create;
-
- create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
-
- afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, create);
- if (IS_ERR(afi))
- return PTR_ERR(afi);
/* Bump generation counter, invalidate any dump in progress */
net->nft.genctr++;
@@ -1801,37 +1790,31 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
*/
synchronize_rcu();
- list_for_each_entry(table, &afi->tables, list) {
- list_for_each_entry(chain, &table->chains, list) {
- list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
- /* Delete this rule from the dirty list */
- list_del(&rupd->list);
-
- /* This rule was inactive in the past and just
- * became active. Clear the next bit of the
- * genmask since its meaning has changed, now
- * it is the future.
- */
- if (nft_rule_is_active(net, rupd->rule)) {
- nft_rule_clear(net, rupd->rule);
- nf_tables_rule_notify(skb, nlh, table,
- chain, rupd->rule,
- NFT_MSG_NEWRULE,
- 0,
- rupd->family);
- kfree(rupd);
- continue;
- }
+ list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
+ /* Delete this rule from the dirty list */
+ list_del(&rupd->list);
- /* This rule is in the past, get rid of it */
- list_del_rcu(&rupd->rule->list);
- nf_tables_rule_notify(skb, nlh, table, chain,
- rupd->rule, NFT_MSG_DELRULE, 0,
- rupd->family);
- nf_tables_rule_destroy(rupd->rule);
- kfree(rupd);
- }
+ /* This rule was inactive in the past and just became active.
+ * Clear the next bit of the genmask since its meaning has
+ * changed, now it is the future.
+ */
+ if (nft_rule_is_active(net, rupd->rule)) {
+ nft_rule_clear(net, rupd->rule);
+ nf_tables_rule_notify(skb, nlh, rupd->table,
+ rupd->chain, rupd->rule,
+ NFT_MSG_NEWRULE, 0,
+ rupd->family);
+ kfree(rupd);
+ continue;
}
+
+ /* This rule is in the past, get rid of it */
+ list_del_rcu(&rupd->rule->list);
+ nf_tables_rule_notify(skb, nlh, rupd->table, rupd->chain,
+ rupd->rule, NFT_MSG_DELRULE, 0,
+ rupd->family);
+ nf_tables_rule_destroy(rupd->rule);
+ kfree(rupd);
}
return 0;
@@ -1841,38 +1824,23 @@ static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
const struct nlmsghdr *nlh,
const struct nlattr * const nla[])
{
- const struct nfgenmsg *nfmsg = nlmsg_data(nlh);
- const struct nft_af_info *afi;
struct net *net = sock_net(skb->sk);
- struct nft_table *table;
- struct nft_chain *chain;
struct nft_rule_trans *rupd, *tmp;
- bool create;
-
- create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
-
- afi = nf_tables_afinfo_lookup(net, nfmsg->nfgen_family, create);
- if (IS_ERR(afi))
- return PTR_ERR(afi);
- list_for_each_entry(table, &afi->tables, list) {
- list_for_each_entry(chain, &table->chains, list) {
- list_for_each_entry_safe(rupd, tmp, &chain->dirty_rules, list) {
- /* Delete all rules from the dirty list */
- list_del(&rupd->list);
-
- if (!nft_rule_is_active_next(net, rupd->rule)) {
- nft_rule_clear(net, rupd->rule);
- kfree(rupd);
- continue;
- }
+ list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) {
+ /* Delete all rules from the dirty list */
+ list_del(&rupd->list);
- /* This rule is inactive, get rid of it */
- list_del_rcu(&rupd->rule->list);
- nf_tables_rule_destroy(rupd->rule);
- kfree(rupd);
- }
+ if (!nft_rule_is_active_next(net, rupd->rule)) {
+ nft_rule_clear(net, rupd->rule);
+ kfree(rupd);
+ continue;
}
+
+ /* This rule is inactive, get rid of it */
+ list_del_rcu(&rupd->rule->list);
+ nf_tables_rule_destroy(rupd->rule);
+ kfree(rupd);
}
return 0;
}
@@ -3224,6 +3192,7 @@ EXPORT_SYMBOL_GPL(nft_data_dump);
static int nf_tables_init_net(struct net *net)
{
INIT_LIST_HEAD(&net->nft.af_info);
+ INIT_LIST_HEAD(&net->nft.commit_list);
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables
2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 1/4] netfilter: nf_tables: get rid of per rule list_head for commits Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 2/4] netfilter: nf_tables: use per netns commit list Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
2013-09-17 10:43 ` [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw)
To: netfilter-devel
This patch adds batch support for nfnetlink, this allows you
to atomically handle a batch of netlink messages. This is used
by nf_tables to improve the existing atomic rule-set update
approach.
Two new nfnetlink control messages are added for this purpose,
they are:
* NFNL_MSG_BATCH_BEGIN, that indicates the beginning of a batch,
the nfgenmsg->res_id indicates the nfnetlink subsystem ID.
* NFNL_MSG_BATCH_END, that results in the invocation of the
ss->commit callback function. If not specified or an error
ocurred in the batch, the ss->abort function is invoked
instead.
This allows us to get rid of the NFT_MSG_COMMIT and NFT_MSG_ABORT
nf_tables message types.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/netfilter/nfnetlink.h | 2 +
include/net/netfilter/nf_tables.h | 2 +
include/uapi/linux/netfilter/nf_tables.h | 2 -
include/uapi/linux/netfilter/nfnetlink.h | 4 +
net/netfilter/nf_tables_api.c | 25 ++---
net/netfilter/nfnetlink.c | 171 +++++++++++++++++++++++++++++-
6 files changed, 182 insertions(+), 24 deletions(-)
diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index cadb740..0aab055 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -23,6 +23,8 @@ struct nfnetlink_subsystem {
__u8 subsys_id; /* nfnetlink subsystem ID */
__u8 cb_count; /* number of callbacks */
const struct nfnl_callback *cb; /* callback for individual types */
+ int (*commit)(struct sk_buff *skb);
+ int (*abort)(struct sk_buff *skb);
};
extern int nfnetlink_subsys_register(const struct nfnetlink_subsystem *n);
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 6b644c2..54c4a5c 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -344,6 +344,7 @@ struct nft_rule {
* @rule: rule that needs to be updated
* @chain: chain that this rule belongs to
* @table: table for which this chain applies
+ * @nlh: netlink header of the message that contain this update
* @family: family expressesed as AF_*
*/
struct nft_rule_trans {
@@ -351,6 +352,7 @@ struct nft_rule_trans {
struct nft_rule *rule;
const struct nft_chain *chain;
const struct nft_table *table;
+ const struct nlmsghdr *nlh;
u8 family;
};
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index d9bf8ea..cbb5c75 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -48,8 +48,6 @@ enum nf_tables_msg_types {
NFT_MSG_NEWSETELEM,
NFT_MSG_GETSETELEM,
NFT_MSG_DELSETELEM,
- NFT_MSG_COMMIT,
- NFT_MSG_ABORT,
NFT_MSG_MAX,
};
diff --git a/include/uapi/linux/netfilter/nfnetlink.h b/include/uapi/linux/netfilter/nfnetlink.h
index 2889594..596ddd4 100644
--- a/include/uapi/linux/netfilter/nfnetlink.h
+++ b/include/uapi/linux/netfilter/nfnetlink.h
@@ -57,4 +57,8 @@ struct nfgenmsg {
#define NFNL_SUBSYS_NFT_COMPAT 11
#define NFNL_SUBSYS_COUNT 12
+/* Reserved control nfnetlink messages */
+#define NFNL_MSG_BATCH_BEGIN NLMSG_MIN_TYPE
+#define NFNL_MSG_BATCH_END NLMSG_MIN_TYPE+1
+
#endif /* _UAPI_NFNETLINK_H */
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 8c78d36..ec2a566 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1548,6 +1548,7 @@ static int nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx)
rupd->table = ctx->table;
rupd->rule = rule;
rupd->family = ctx->afi->family;
+ rupd->nlh = ctx->nlh;
list_add(&rupd->list, &ctx->net->nft.commit_list);
return 0;
@@ -1772,9 +1773,7 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
return err;
}
-static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- const struct nlattr * const nla[])
+static int nf_tables_commit(struct sk_buff *skb)
{
struct net *net = sock_net(skb->sk);
struct nft_rule_trans *rupd, *tmp;
@@ -1800,7 +1799,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
*/
if (nft_rule_is_active(net, rupd->rule)) {
nft_rule_clear(net, rupd->rule);
- nf_tables_rule_notify(skb, nlh, rupd->table,
+ nf_tables_rule_notify(skb, rupd->nlh, rupd->table,
rupd->chain, rupd->rule,
NFT_MSG_NEWRULE, 0,
rupd->family);
@@ -1810,7 +1809,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
/* This rule is in the past, get rid of it */
list_del_rcu(&rupd->rule->list);
- nf_tables_rule_notify(skb, nlh, rupd->table, rupd->chain,
+ nf_tables_rule_notify(skb, rupd->nlh, rupd->table, rupd->chain,
rupd->rule, NFT_MSG_DELRULE, 0,
rupd->family);
nf_tables_rule_destroy(rupd->rule);
@@ -1820,9 +1819,7 @@ static int nf_tables_commit(struct sock *nlsk, struct sk_buff *skb,
return 0;
}
-static int nf_tables_abort(struct sock *nlsk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- const struct nlattr * const nla[])
+static int nf_tables_abort(struct sk_buff *skb)
{
struct net *net = sock_net(skb->sk);
struct nft_rule_trans *rupd, *tmp;
@@ -2804,16 +2801,6 @@ static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
.attr_count = NFTA_SET_ELEM_LIST_MAX,
.policy = nft_set_elem_list_policy,
},
- [NFT_MSG_COMMIT] = {
- .call = nf_tables_commit,
- .attr_count = NFTA_TABLE_MAX,
- .policy = nft_rule_policy,
- },
- [NFT_MSG_ABORT] = {
- .call = nf_tables_abort,
- .attr_count = NFTA_TABLE_MAX,
- .policy = nft_rule_policy,
- },
};
static const struct nfnetlink_subsystem nf_tables_subsys = {
@@ -2821,6 +2808,8 @@ static const struct nfnetlink_subsystem nf_tables_subsys = {
.subsys_id = NFNL_SUBSYS_NFTABLES,
.cb_count = NFT_MSG_MAX,
.cb = nf_tables_cb,
+ .commit = nf_tables_commit,
+ .abort = nf_tables_abort,
};
/*
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index 572d87d..8dde792 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -147,9 +147,6 @@ static int nfnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
const struct nfnetlink_subsystem *ss;
int type, err;
- if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
- return -EPERM;
-
/* All the messages must at least contain nfgenmsg */
if (nlmsg_len(nlh) < sizeof(struct nfgenmsg))
return 0;
@@ -217,9 +214,175 @@ replay:
}
}
+static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
+ u_int16_t subsys_id)
+{
+ struct sk_buff *nskb, *oskb = skb;
+ struct net *net = sock_net(skb->sk);
+ const struct nfnetlink_subsystem *ss;
+ const struct nfnl_callback *nc;
+ bool success = true, done = false;
+ int err;
+
+ if (subsys_id >= NFNL_SUBSYS_COUNT)
+ return netlink_ack(skb, nlh, -EINVAL);
+replay:
+ nskb = netlink_skb_clone(oskb, GFP_KERNEL);
+ if (!nskb)
+ return netlink_ack(oskb, nlh, -ENOMEM);
+
+ nskb->sk = oskb->sk;
+ skb = nskb;
+
+ nfnl_lock(subsys_id);
+ ss = rcu_dereference_protected(table[subsys_id].subsys,
+ lockdep_is_held(&table[subsys_id].mutex));
+ if (!ss) {
+#ifdef CONFIG_MODULES
+ nfnl_unlock(subsys_id);
+ request_module("nfnetlink-subsys-%d", subsys_id);
+ nfnl_lock(subsys_id);
+ ss = rcu_dereference_protected(table[subsys_id].subsys,
+ lockdep_is_held(&table[subsys_id].mutex));
+ if (!ss)
+#endif
+ {
+ nfnl_unlock(subsys_id);
+ kfree_skb(nskb);
+ return netlink_ack(skb, nlh, -EOPNOTSUPP);
+ }
+ }
+
+ if (!ss->commit || !ss->abort) {
+ nfnl_unlock(subsys_id);
+ kfree_skb(nskb);
+ return netlink_ack(skb, nlh, -EOPNOTSUPP);
+ }
+
+ while (skb->len >= nlmsg_total_size(0)) {
+ int msglen, type;
+
+ nlh = nlmsg_hdr(skb);
+ err = 0;
+
+ if (nlh->nlmsg_len < NLMSG_HDRLEN) {
+ err = -EINVAL;
+ goto ack;
+ }
+
+ /* Only requests are handled by the kernel */
+ if (!(nlh->nlmsg_flags & NLM_F_REQUEST)) {
+ err = -EINVAL;
+ goto ack;
+ }
+
+ type = nlh->nlmsg_type;
+ if (type == NFNL_MSG_BATCH_END) {
+ done = true;
+ goto done;
+ } else if (type < NLMSG_MIN_TYPE) {
+ err = -EINVAL;
+ goto ack;
+ }
+
+ /* We only accept a batch with messages for the same
+ * subsystem.
+ */
+ if (NFNL_SUBSYS_ID(type) != subsys_id) {
+ err = -EINVAL;
+ goto ack;
+ }
+
+ nc = nfnetlink_find_client(type, ss);
+ if (!nc) {
+ err = -EINVAL;
+ goto ack;
+ }
+
+ {
+ int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
+ u_int8_t cb_id = NFNL_MSG_TYPE(nlh->nlmsg_type);
+ struct nlattr *cda[ss->cb[cb_id].attr_count + 1];
+ struct nlattr *attr = (void *)nlh + min_len;
+ int attrlen = nlh->nlmsg_len - min_len;
+
+ err = nla_parse(cda, ss->cb[cb_id].attr_count,
+ attr, attrlen, ss->cb[cb_id].policy);
+ if (err < 0)
+ goto ack;
+
+ if (nc->call) {
+ err = nc->call(net->nfnl, skb, nlh,
+ (const struct nlattr **)cda);
+ }
+
+ /* The lock was released to autoload some module, we
+ * have to abort and start from scratch using the
+ * original skb.
+ */
+ if (err == -EAGAIN) {
+ ss->abort(skb);
+ nfnl_unlock(subsys_id);
+ kfree_skb(nskb);
+ goto replay;
+ }
+ }
+ack:
+ if (nlh->nlmsg_flags & NLM_F_ACK || err) {
+ /* We don't stop processing the batch on errors, thus,
+ * userspace gets all the errors that the batch
+ * triggers.
+ */
+ netlink_ack(skb, nlh, err);
+ if (err)
+ success = false;
+ }
+
+ msglen = NLMSG_ALIGN(nlh->nlmsg_len);
+ if (msglen > skb->len)
+ msglen = skb->len;
+ skb_pull(skb, msglen);
+ }
+done:
+ if (success && done)
+ ss->commit(skb);
+ else
+ ss->abort(skb);
+
+ nfnl_unlock(subsys_id);
+ kfree_skb(nskb);
+}
+
static void nfnetlink_rcv(struct sk_buff *skb)
{
- netlink_rcv_skb(skb, &nfnetlink_rcv_msg);
+ struct nlmsghdr *nlh = nlmsg_hdr(skb);
+ struct net *net = sock_net(skb->sk);
+ int msglen;
+
+ if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+ return netlink_ack(skb, nlh, -EPERM);
+
+ if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+ skb->len < nlh->nlmsg_len)
+ return;
+
+ if (nlh->nlmsg_type == NFNL_MSG_BATCH_BEGIN) {
+ struct nfgenmsg *nfgenmsg;
+
+ msglen = NLMSG_ALIGN(nlh->nlmsg_len);
+ if (msglen > skb->len)
+ msglen = skb->len;
+
+ if (nlh->nlmsg_len < NLMSG_HDRLEN ||
+ skb->len < NLMSG_HDRLEN + sizeof(struct nfgenmsg))
+ return;
+
+ nfgenmsg = nlmsg_data(nlh);
+ skb_pull(skb, msglen);
+ nfnetlink_rcv_batch(skb, nlh, nfgenmsg->res_id);
+ } else {
+ netlink_rcv_skb(skb, &nfnetlink_rcv_msg);
+ }
}
#ifdef CONFIG_MODULES
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH -nftables v2 4/4] netfilter: nf_tables: all rule updates are transactional
2013-09-17 10:43 [PATCH 0/4 -nftables v2] nf_tables atomic rule-set update Pablo Neira Ayuso
` (2 preceding siblings ...)
2013-09-17 10:43 ` [PATCH -nftables v2 3/4] netfilter: nfnetlink: add batch support and use it from nf_tables Pablo Neira Ayuso
@ 2013-09-17 10:43 ` Pablo Neira Ayuso
3 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2013-09-17 10:43 UTC (permalink / raw)
To: netfilter-devel
This patch makes all rule updates transactional, this simplifies
the ruleset update logic. Suggested by Patrick McHardy.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/uapi/linux/netfilter/nf_tables.h | 5 ---
net/netfilter/nf_tables_api.c | 62 ++++++------------------------
2 files changed, 12 insertions(+), 55 deletions(-)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index cbb5c75..b8cd62f 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -120,11 +120,6 @@ enum nft_chain_attributes {
};
#define NFTA_CHAIN_MAX (__NFTA_CHAIN_MAX - 1)
-enum {
- NFT_RULE_F_COMMIT = (1 << 0),
- NFT_RULE_F_MASK = NFT_RULE_F_COMMIT,
-};
-
/**
* enum nft_rule_attributes - nf_tables rule netlink attributes
*
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index ec2a566..c02a698 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1570,7 +1570,6 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
unsigned int size, i, n;
int err, rem;
bool create;
- u32 flags = 0;
u64 handle, pos_handle;
create = nlh->nlmsg_flags & NLM_F_CREATE ? true : false;
@@ -1639,14 +1638,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
if (rule == NULL)
goto err1;
- if (nla[NFTA_RULE_FLAGS]) {
- flags = ntohl(nla_get_be32(nla[NFTA_RULE_FLAGS]));
- if (flags & ~NFT_RULE_F_MASK)
- return -EINVAL;
-
- if (flags & NFT_RULE_F_COMMIT)
- nft_rule_activate_next(net, rule);
- }
+ nft_rule_activate_next(net, rule);
rule->handle = handle;
rule->dlen = size;
@@ -1661,13 +1653,8 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
}
if (nlh->nlmsg_flags & NLM_F_REPLACE) {
- if (flags & NFT_RULE_F_COMMIT) {
- nft_rule_disactivate_next(net, old_rule);
- list_add_tail_rcu(&rule->list, &chain->rules);
- } else {
- list_replace_rcu(&old_rule->list, &rule->list);
- nf_tables_rule_destroy(old_rule);
- }
+ nft_rule_disactivate_next(net, old_rule);
+ list_add_tail_rcu(&rule->list, &chain->rules);
} else if (nlh->nlmsg_flags & NLM_F_APPEND)
if (old_rule)
list_add_rcu(&rule->list, &old_rule->list);
@@ -1680,16 +1667,10 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
list_add_rcu(&rule->list, &chain->rules);
}
- if (flags & NFT_RULE_F_COMMIT)
- err = nf_tables_trans_add(rule, &ctx);
- if (err < 0)
- goto err2;
- else {
- nf_tables_rule_notify(skb, nlh, table, chain, rule,
- NFT_MSG_NEWRULE,
- nlh->nlmsg_flags & (NLM_F_APPEND | NLM_F_REPLACE),
- nfmsg->nfgen_family);
- }
+ err = nf_tables_trans_add(rule, &ctx);
+ if (err < 0)
+ goto err2;
+
return 0;
err2:
@@ -1703,21 +1684,10 @@ err1:
}
static int
-nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule, u32 flags)
+nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
{
- int err = 0;
-
- if (flags & NFT_RULE_F_COMMIT) {
- nft_rule_disactivate_next(ctx->net, rule);
- err = nf_tables_trans_add(rule, ctx);
- } else {
- list_del_rcu(&rule->list);
- nf_tables_rule_notify(ctx->skb, ctx->nlh, ctx->table,
- ctx->chain, rule, NFT_MSG_DELRULE,
- 0, ctx->afi->family);
- nf_tables_rule_destroy(rule);
- }
- return err;
+ nft_rule_disactivate_next(ctx->net, rule);
+ return nf_tables_trans_add(rule, ctx);
}
static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
@@ -1732,7 +1702,6 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
struct nft_rule *rule, *tmp;
int family = nfmsg->nfgen_family, err = 0;
struct nft_ctx ctx;
- u32 flags = 0;
afi = nf_tables_afinfo_lookup(net, family, false);
if (IS_ERR(afi))
@@ -1748,23 +1717,16 @@ static int nf_tables_delrule(struct sock *nlsk, struct sk_buff *skb,
nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
- if (nla[NFTA_RULE_FLAGS]) {
- flags = ntohl(nla_get_be32(nla[NFTA_RULE_FLAGS]));
-
- if (flags & ~NFT_RULE_F_MASK)
- return -EINVAL;
- }
-
if (nla[NFTA_RULE_HANDLE]) {
rule = nf_tables_rule_lookup(chain, nla[NFTA_RULE_HANDLE]);
if (IS_ERR(rule))
return PTR_ERR(rule);
- err = nf_tables_delrule_one(&ctx, rule, flags);
+ err = nf_tables_delrule_one(&ctx, rule);
} else {
/* Remove all rules in this chain */
list_for_each_entry_safe(rule, tmp, &chain->rules, list) {
- err = nf_tables_delrule_one(&ctx, rule, flags);
+ err = nf_tables_delrule_one(&ctx, rule);
if (err < 0)
break;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread