* [PATCH 0/5] netfilter: nf_tables: restore context during destruction
@ 2014-03-07 18:08 Patrick McHardy
2014-03-07 18:08 ` [PATCH 1/5] netfilter: nf_tables: clean up nf_tables_trans_add() argument order Patrick McHardy
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
The following patches restore the context during expression destruction
in order to restore notifications for anonmyous set destruction and get
rid of some unnecessary members in the expressions' private structures.
Additionally they contain a couple of related minor cleanups.
Please apply.
Patrick McHardy (5):
netfilter: nf_tables: clean up nf_tables_trans_add() argument order
netfilter: nf_tables: restore context for expression destructors
netfilter: nf_tables: restore notifications for anonymous set destruction
netfilter: nft_ct: remove family from struct nft_ct
netfilter: nft_nat: fix family validation
include/net/netfilter/nf_tables.h | 13 ++++-------
net/netfilter/nf_tables_api.c | 45 +++++++++++++++++++--------------------
net/netfilter/nft_compat.c | 4 ++--
net/netfilter/nft_ct.c | 12 ++++-------
net/netfilter/nft_immediate.c | 3 ++-
net/netfilter/nft_log.c | 3 ++-
net/netfilter/nft_lookup.c | 5 +++--
net/netfilter/nft_nat.c | 22 +++++++++----------
8 files changed, 49 insertions(+), 58 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/5] netfilter: nf_tables: clean up nf_tables_trans_add() argument order 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy @ 2014-03-07 18:08 ` Patrick McHardy 2014-03-07 18:08 ` [PATCH 2/5] netfilter: nf_tables: restore context for expression destructors Patrick McHardy ` (4 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel The context argument logically comes first, and this is what every other function dealing with contexts does. Signed-off-by: Patrick McHardy <kaber@trash.net> --- net/netfilter/nf_tables_api.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 89d7ec4..a206405 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1557,7 +1557,7 @@ static void nf_tables_rule_destroy(struct nft_rule *rule) static struct nft_expr_info *info; static struct nft_rule_trans * -nf_tables_trans_add(struct nft_rule *rule, const struct nft_ctx *ctx) +nf_tables_trans_add(struct nft_ctx *ctx, struct nft_rule *rule) { struct nft_rule_trans *rupd; @@ -1683,7 +1683,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, if (nlh->nlmsg_flags & NLM_F_REPLACE) { if (nft_rule_is_active_next(net, old_rule)) { - repl = nf_tables_trans_add(old_rule, &ctx); + repl = nf_tables_trans_add(&ctx, old_rule); if (repl == NULL) { err = -ENOMEM; goto err2; @@ -1706,7 +1706,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb, list_add_rcu(&rule->list, &chain->rules); } - if (nf_tables_trans_add(rule, &ctx) == NULL) { + if (nf_tables_trans_add(&ctx, rule) == NULL) { err = -ENOMEM; goto err3; } @@ -1735,7 +1735,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule) { /* You cannot delete the same rule twice */ if (nft_rule_is_active_next(ctx->net, rule)) { - if (nf_tables_trans_add(rule, ctx) == NULL) + if (nf_tables_trans_add(ctx, rule) == NULL) return -ENOMEM; nft_rule_disactivate_next(ctx->net, rule); return 0; -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] netfilter: nf_tables: restore context for expression destructors 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy 2014-03-07 18:08 ` [PATCH 1/5] netfilter: nf_tables: clean up nf_tables_trans_add() argument order Patrick McHardy @ 2014-03-07 18:08 ` Patrick McHardy 2014-03-07 18:08 ` [PATCH 3/5] netfilter: nf_tables: restore notifications for anonymous set destruction Patrick McHardy ` (3 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel In order to fix set destruction notifications and get rid of unnecessary members in private data structures, pass the context to expressions' destructor functions again. In order to do so, replace various members in the nft_rule_trans structure by the full context. Signed-off-by: Patrick McHardy <kaber@trash.net> --- include/net/netfilter/nf_tables.h | 13 ++++--------- net/netfilter/nf_tables_api.c | 34 +++++++++++++++++----------------- net/netfilter/nft_compat.c | 4 ++-- net/netfilter/nft_ct.c | 3 ++- net/netfilter/nft_immediate.c | 3 ++- net/netfilter/nft_log.c | 3 ++- net/netfilter/nft_lookup.c | 3 ++- 7 files changed, 31 insertions(+), 32 deletions(-) diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 5af56da..e6bc14d 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -289,7 +289,8 @@ struct nft_expr_ops { int (*init)(const struct nft_ctx *ctx, const struct nft_expr *expr, const struct nlattr * const tb[]); - void (*destroy)(const struct nft_expr *expr); + void (*destroy)(const struct nft_ctx *ctx, + const struct nft_expr *expr); int (*dump)(struct sk_buff *skb, const struct nft_expr *expr); int (*validate)(const struct nft_ctx *ctx, @@ -343,19 +344,13 @@ struct nft_rule { * struct nft_rule_trans - nf_tables rule update in transaction * * @list: used internally + * @ctx: rule context * @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 { struct list_head list; + struct nft_ctx ctx; struct nft_rule *rule; - const struct nft_chain *chain; - const struct nft_table *table; - const struct nlmsghdr *nlh; - u8 family; }; static inline struct nft_expr *nft_expr_first(const struct nft_rule *rule) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index a206405..55541ce 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1253,10 +1253,11 @@ err1: return err; } -static void nf_tables_expr_destroy(struct nft_expr *expr) +static void nf_tables_expr_destroy(const struct nft_ctx *ctx, + struct nft_expr *expr) { if (expr->ops->destroy) - expr->ops->destroy(expr); + expr->ops->destroy(ctx, expr); module_put(expr->ops->type->owner); } @@ -1536,7 +1537,8 @@ err: return err; } -static void nf_tables_rule_destroy(struct nft_rule *rule) +static void nf_tables_rule_destroy(const struct nft_ctx *ctx, + struct nft_rule *rule) { struct nft_expr *expr; @@ -1546,7 +1548,7 @@ static void nf_tables_rule_destroy(struct nft_rule *rule) */ expr = nft_expr_first(rule); while (expr->ops && expr != nft_expr_last(rule)) { - nf_tables_expr_destroy(expr); + nf_tables_expr_destroy(ctx, expr); expr = nft_expr_next(expr); } kfree(rule); @@ -1565,11 +1567,8 @@ nf_tables_trans_add(struct nft_ctx *ctx, struct nft_rule *rule) if (rupd == NULL) return NULL; - rupd->chain = ctx->chain; - rupd->table = ctx->table; + rupd->ctx = *ctx; rupd->rule = rule; - rupd->family = ctx->afi->family; - rupd->nlh = ctx->nlh; list_add_tail(&rupd->list, &ctx->net->nft.commit_list); return rupd; @@ -1721,7 +1720,7 @@ err3: kfree(repl); } err2: - nf_tables_rule_destroy(rule); + nf_tables_rule_destroy(&ctx, rule); err1: for (i = 0; i < n; i++) { if (info[i].ops != NULL) @@ -1831,10 +1830,10 @@ static int nf_tables_commit(struct sk_buff *skb) */ if (nft_rule_is_active(net, rupd->rule)) { nft_rule_clear(net, rupd->rule); - nf_tables_rule_notify(skb, rupd->nlh, rupd->table, - rupd->chain, rupd->rule, - NFT_MSG_NEWRULE, 0, - rupd->family); + nf_tables_rule_notify(skb, rupd->ctx.nlh, + rupd->ctx.table, rupd->ctx.chain, + rupd->rule, NFT_MSG_NEWRULE, 0, + rupd->ctx.afi->family); list_del(&rupd->list); kfree(rupd); continue; @@ -1842,9 +1841,10 @@ static int nf_tables_commit(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, rupd->nlh, rupd->table, rupd->chain, + nf_tables_rule_notify(skb, rupd->ctx.nlh, + rupd->ctx.table, rupd->ctx.chain, rupd->rule, NFT_MSG_DELRULE, 0, - rupd->family); + rupd->ctx.afi->family); } /* Make sure we don't see any packet traversing old rules */ @@ -1852,7 +1852,7 @@ static int nf_tables_commit(struct sk_buff *skb) /* Now we can safely release unused old rules */ list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) { - nf_tables_rule_destroy(rupd->rule); + nf_tables_rule_destroy(&rupd->ctx, rupd->rule); list_del(&rupd->list); kfree(rupd); } @@ -1881,7 +1881,7 @@ static int nf_tables_abort(struct sk_buff *skb) synchronize_rcu(); list_for_each_entry_safe(rupd, tmp, &net->nft.commit_list, list) { - nf_tables_rule_destroy(rupd->rule); + nf_tables_rule_destroy(&rupd->ctx, rupd->rule); list_del(&rupd->list); kfree(rupd); } diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c index 82cb823..8a779be 100644 --- a/net/netfilter/nft_compat.c +++ b/net/netfilter/nft_compat.c @@ -192,7 +192,7 @@ err: } static void -nft_target_destroy(const struct nft_expr *expr) +nft_target_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { struct xt_target *target = expr->ops->data; @@ -379,7 +379,7 @@ err: } static void -nft_match_destroy(const struct nft_expr *expr) +nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { struct xt_match *match = expr->ops->data; diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c index e59b08f..65a2c7b 100644 --- a/net/netfilter/nft_ct.c +++ b/net/netfilter/nft_ct.c @@ -321,7 +321,8 @@ static int nft_ct_init(const struct nft_ctx *ctx, return 0; } -static void nft_ct_destroy(const struct nft_expr *expr) +static void nft_ct_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) { struct nft_ct *priv = nft_expr_priv(expr); diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c index f169501..810385e 100644 --- a/net/netfilter/nft_immediate.c +++ b/net/netfilter/nft_immediate.c @@ -70,7 +70,8 @@ err1: return err; } -static void nft_immediate_destroy(const struct nft_expr *expr) +static void nft_immediate_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) { const struct nft_immediate_expr *priv = nft_expr_priv(expr); return nft_data_uninit(&priv->data, nft_dreg_to_type(priv->dreg)); diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c index 26c5154..10cfb15 100644 --- a/net/netfilter/nft_log.c +++ b/net/netfilter/nft_log.c @@ -74,7 +74,8 @@ static int nft_log_init(const struct nft_ctx *ctx, return 0; } -static void nft_log_destroy(const struct nft_expr *expr) +static void nft_log_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) { struct nft_log *priv = nft_expr_priv(expr); diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c index bb4ef4c..953978e 100644 --- a/net/netfilter/nft_lookup.c +++ b/net/netfilter/nft_lookup.c @@ -89,7 +89,8 @@ static int nft_lookup_init(const struct nft_ctx *ctx, return 0; } -static void nft_lookup_destroy(const struct nft_expr *expr) +static void nft_lookup_destroy(const struct nft_ctx *ctx, + const struct nft_expr *expr) { struct nft_lookup *priv = nft_expr_priv(expr); -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] netfilter: nf_tables: restore notifications for anonymous set destruction 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy 2014-03-07 18:08 ` [PATCH 1/5] netfilter: nf_tables: clean up nf_tables_trans_add() argument order Patrick McHardy 2014-03-07 18:08 ` [PATCH 2/5] netfilter: nf_tables: restore context for expression destructors Patrick McHardy @ 2014-03-07 18:08 ` Patrick McHardy 2014-03-07 18:08 ` [PATCH 4/5] netfilter: nft_ct: remove family from struct nft_ct Patrick McHardy ` (2 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel Since we have the context available again, we can restore notifications for destruction of anonymous sets. Signed-off-by: Patrick McHardy <kaber@trash.net> --- net/netfilter/nf_tables_api.c | 3 +-- net/netfilter/nft_lookup.c | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 55541ce..2c22fa2 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -2447,8 +2447,7 @@ err1: static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set) { list_del(&set->list); - if (!(set->flags & NFT_SET_ANONYMOUS)) - nf_tables_set_notify(ctx, set, NFT_MSG_DELSET); + nf_tables_set_notify(ctx, set, NFT_MSG_DELSET); set->ops->destroy(set); module_put(set->ops->owner); diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c index 953978e..7fd2bea 100644 --- a/net/netfilter/nft_lookup.c +++ b/net/netfilter/nft_lookup.c @@ -94,7 +94,7 @@ static void nft_lookup_destroy(const struct nft_ctx *ctx, { struct nft_lookup *priv = nft_expr_priv(expr); - nf_tables_unbind_set(NULL, priv->set, &priv->binding); + nf_tables_unbind_set(ctx, priv->set, &priv->binding); } static int nft_lookup_dump(struct sk_buff *skb, const struct nft_expr *expr) -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] netfilter: nft_ct: remove family from struct nft_ct 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy ` (2 preceding siblings ...) 2014-03-07 18:08 ` [PATCH 3/5] netfilter: nf_tables: restore notifications for anonymous set destruction Patrick McHardy @ 2014-03-07 18:08 ` Patrick McHardy 2014-03-07 18:08 ` [PATCH 5/5] netfilter: nft_nat: fix family validation Patrick McHardy 2014-03-12 12:23 ` [PATCH 0/5] netfilter: nf_tables: restore context during destruction Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel Since we have the context available during destruction again, we can remove the family from the private structure. Signed-off-by: Patrick McHardy <kaber@trash.net> --- net/netfilter/nft_ct.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c index 65a2c7b..bd0d41e 100644 --- a/net/netfilter/nft_ct.c +++ b/net/netfilter/nft_ct.c @@ -24,11 +24,10 @@ struct nft_ct { enum nft_ct_keys key:8; enum ip_conntrack_dir dir:8; - union{ + union { enum nft_registers dreg:8; enum nft_registers sreg:8; }; - uint8_t family; }; static void nft_ct_get_eval(const struct nft_expr *expr, @@ -316,17 +315,13 @@ static int nft_ct_init(const struct nft_ctx *ctx, if (err < 0) return err; - priv->family = ctx->afi->family; - return 0; } static void nft_ct_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) { - struct nft_ct *priv = nft_expr_priv(expr); - - nft_ct_l3proto_module_put(priv->family); + nft_ct_l3proto_module_put(ctx->afi->family); } static int nft_ct_get_dump(struct sk_buff *skb, const struct nft_expr *expr) -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] netfilter: nft_nat: fix family validation 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy ` (3 preceding siblings ...) 2014-03-07 18:08 ` [PATCH 4/5] netfilter: nft_ct: remove family from struct nft_ct Patrick McHardy @ 2014-03-07 18:08 ` Patrick McHardy 2014-03-12 12:23 ` [PATCH 0/5] netfilter: nf_tables: restore context during destruction Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Patrick McHardy @ 2014-03-07 18:08 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel The family in the NAT expression is basically completely useless since we have it available during runtime anyway. Nevertheless it is used to decide the NAT family, so at least validate it properly. As we don't support cross-family NAT, it needs to match the family of the table the expression exists in. Unfortunately we can't remove it completely since we need to dump it for userspace (*sigh*), so at least reduce the memory waste. Additionally clean up the module init function by removing useless temporary variables. Signed-off-by: Patrick McHardy <kaber@trash.net> --- net/netfilter/nft_nat.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c index d3b1ffe..a0195d2 100644 --- a/net/netfilter/nft_nat.c +++ b/net/netfilter/nft_nat.c @@ -31,8 +31,8 @@ struct nft_nat { enum nft_registers sreg_addr_max:8; enum nft_registers sreg_proto_min:8; enum nft_registers sreg_proto_max:8; - int family; - enum nf_nat_manip_type type; + enum nf_nat_manip_type type:8; + u8 family; }; static void nft_nat_eval(const struct nft_expr *expr, @@ -88,6 +88,7 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, const struct nlattr * const tb[]) { struct nft_nat *priv = nft_expr_priv(expr); + u32 family; int err; if (tb[NFTA_NAT_TYPE] == NULL) @@ -107,9 +108,12 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr, if (tb[NFTA_NAT_FAMILY] == NULL) return -EINVAL; - priv->family = ntohl(nla_get_be32(tb[NFTA_NAT_FAMILY])); - if (priv->family != AF_INET && priv->family != AF_INET6) - return -EINVAL; + family = ntohl(nla_get_be32(tb[NFTA_NAT_FAMILY])); + if (family != AF_INET && family != AF_INET6) + return -EAFNOSUPPORT; + if (family != ctx->afi->family) + return -EOPNOTSUPP; + priv->family = family; if (tb[NFTA_NAT_REG_ADDR_MIN]) { priv->sreg_addr_min = ntohl(nla_get_be32( @@ -202,13 +206,7 @@ static struct nft_expr_type nft_nat_type __read_mostly = { static int __init nft_nat_module_init(void) { - int err; - - err = nft_register_expr(&nft_nat_type); - if (err < 0) - return err; - - return 0; + return nft_register_expr(&nft_nat_type); } static void __exit nft_nat_module_exit(void) -- 1.8.5.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/5] netfilter: nf_tables: restore context during destruction 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy ` (4 preceding siblings ...) 2014-03-07 18:08 ` [PATCH 5/5] netfilter: nft_nat: fix family validation Patrick McHardy @ 2014-03-12 12:23 ` Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2014-03-12 12:23 UTC (permalink / raw) To: Patrick McHardy; +Cc: netfilter-devel On Fri, Mar 07, 2014 at 07:08:28PM +0100, Patrick McHardy wrote: > The following patches restore the context during expression destruction > in order to restore notifications for anonmyous set destruction and get > rid of some unnecessary members in the expressions' private structures. > > Additionally they contain a couple of related minor cleanups. > > Please apply. All applied, thanks Patrick. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-12 12:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-07 18:08 [PATCH 0/5] netfilter: nf_tables: restore context during destruction Patrick McHardy 2014-03-07 18:08 ` [PATCH 1/5] netfilter: nf_tables: clean up nf_tables_trans_add() argument order Patrick McHardy 2014-03-07 18:08 ` [PATCH 2/5] netfilter: nf_tables: restore context for expression destructors Patrick McHardy 2014-03-07 18:08 ` [PATCH 3/5] netfilter: nf_tables: restore notifications for anonymous set destruction Patrick McHardy 2014-03-07 18:08 ` [PATCH 4/5] netfilter: nft_ct: remove family from struct nft_ct Patrick McHardy 2014-03-07 18:08 ` [PATCH 5/5] netfilter: nft_nat: fix family validation Patrick McHardy 2014-03-12 12:23 ` [PATCH 0/5] netfilter: nf_tables: restore context during destruction 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).