* [PATCH 0/3] netfilter: nf_tables cleanups
@ 2015-03-03 20:10 Patrick McHardy
2015-03-03 20:10 ` [PATCH 1/3] netfilter: nf_tables: minor tracing cleanups Patrick McHardy
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Patrick McHardy @ 2015-03-03 20:10 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
These patches for -next contain some cleanups which are not directly
related to the following set patches, so I'm sending them seperately
for easier review:
* move the reoccuring checks for skb->nf_trace to an inline function
* consolidate similar invocations of nft_trace_packet()
* rearrange nf_tables.h so related things are next to each other
Please apply, thanks!
Patrick McHardy (3):
netfilter: nf_tables: minor tracing cleanups
netfilter: nf_tables: consolidate tracing invocations
netfilter: nf_tables: cleanup nf_tables.h
include/net/netfilter/nf_tables.h | 174 +++++++++++++++++++-------------------
net/netfilter/nf_tables_core.c | 105 +++++++++++------------
2 files changed, 138 insertions(+), 141 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] netfilter: nf_tables: minor tracing cleanups
2015-03-03 20:10 [PATCH 0/3] netfilter: nf_tables cleanups Patrick McHardy
@ 2015-03-03 20:10 ` Patrick McHardy
2015-03-03 20:10 ` [PATCH 2/3] netfilter: nf_tables: consolidate tracing invocations Patrick McHardy
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2015-03-03 20:10 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
The tracing code is squeezed between multiple related parts of the
evaluation code, move it out. Also add an inline wrapper for the
reoccuring test for skb->nf_trace.
Small code savings in nft_do_chain():
nft_trace_packet | -137
nft_do_chain | -8
2 functions changed, 145 bytes removed, diff: -145
net/netfilter/nf_tables_core.c:
__nft_trace_packet | +137
1 function changed, 137 bytes added, diff: +137
net/netfilter/nf_tables_core.o:
3 functions changed, 137 bytes added, 145 bytes removed, diff: -8
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nf_tables_core.c | 98 +++++++++++++++++++++---------------------
1 file changed, 49 insertions(+), 49 deletions(-)
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 3b90eb2..074067d 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -21,6 +21,48 @@
#include <net/netfilter/nf_tables.h>
#include <net/netfilter/nf_log.h>
+enum nft_trace {
+ NFT_TRACE_RULE,
+ NFT_TRACE_RETURN,
+ NFT_TRACE_POLICY,
+};
+
+static const char *const comments[] = {
+ [NFT_TRACE_RULE] = "rule",
+ [NFT_TRACE_RETURN] = "return",
+ [NFT_TRACE_POLICY] = "policy",
+};
+
+static struct nf_loginfo trace_loginfo = {
+ .type = NF_LOG_TYPE_LOG,
+ .u = {
+ .log = {
+ .level = 4,
+ .logflags = NF_LOG_MASK,
+ },
+ },
+};
+
+static void __nft_trace_packet(const struct nft_pktinfo *pkt,
+ const struct nft_chain *chain,
+ int rulenum, enum nft_trace type)
+{
+ struct net *net = dev_net(pkt->in ? pkt->in : pkt->out);
+
+ nf_log_packet(net, pkt->xt.family, pkt->ops->hooknum, pkt->skb, pkt->in,
+ pkt->out, &trace_loginfo, "TRACE: %s:%s:%s:%u ",
+ chain->table->name, chain->name, comments[type],
+ rulenum);
+}
+
+static inline void nft_trace_packet(const struct nft_pktinfo *pkt,
+ const struct nft_chain *chain,
+ int rulenum, enum nft_trace type)
+{
+ if (unlikely(pkt->skb->nf_trace))
+ __nft_trace_packet(pkt, chain, rulenum, type);
+}
+
static void nft_cmp_fast_eval(const struct nft_expr *expr,
struct nft_data data[NFT_REG_MAX + 1])
{
@@ -66,40 +108,6 @@ struct nft_jumpstack {
int rulenum;
};
-enum nft_trace {
- NFT_TRACE_RULE,
- NFT_TRACE_RETURN,
- NFT_TRACE_POLICY,
-};
-
-static const char *const comments[] = {
- [NFT_TRACE_RULE] = "rule",
- [NFT_TRACE_RETURN] = "return",
- [NFT_TRACE_POLICY] = "policy",
-};
-
-static struct nf_loginfo trace_loginfo = {
- .type = NF_LOG_TYPE_LOG,
- .u = {
- .log = {
- .level = 4,
- .logflags = NF_LOG_MASK,
- },
- },
-};
-
-static void nft_trace_packet(const struct nft_pktinfo *pkt,
- const struct nft_chain *chain,
- int rulenum, enum nft_trace type)
-{
- struct net *net = dev_net(pkt->in ? pkt->in : pkt->out);
-
- nf_log_packet(net, pkt->xt.family, pkt->ops->hooknum, pkt->skb, pkt->in,
- pkt->out, &trace_loginfo, "TRACE: %s:%s:%s:%u ",
- chain->table->name, chain->name, comments[type],
- rulenum);
-}
-
unsigned int
nft_do_chain(struct nft_pktinfo *pkt, const struct nf_hook_ops *ops)
{
@@ -146,8 +154,7 @@ next_rule:
data[NFT_REG_VERDICT].verdict = NFT_CONTINUE;
continue;
case NFT_CONTINUE:
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
+ nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
continue;
}
break;
@@ -157,16 +164,13 @@ next_rule:
case NF_ACCEPT:
case NF_DROP:
case NF_QUEUE:
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
-
+ nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
return data[NFT_REG_VERDICT].verdict;
}
switch (data[NFT_REG_VERDICT].verdict) {
case NFT_JUMP:
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
+ nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
BUG_ON(stackptr >= NFT_JUMP_STACK_SIZE);
jumpstack[stackptr].chain = chain;
@@ -176,18 +180,15 @@ next_rule:
chain = data[NFT_REG_VERDICT].chain;
goto do_chain;
case NFT_GOTO:
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
+ nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
chain = data[NFT_REG_VERDICT].chain;
goto do_chain;
case NFT_RETURN:
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RETURN);
+ nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RETURN);
break;
case NFT_CONTINUE:
- if (unlikely(pkt->skb->nf_trace && !(chain->flags & NFT_BASE_CHAIN)))
- nft_trace_packet(pkt, chain, ++rulenum, NFT_TRACE_RETURN);
+ nft_trace_packet(pkt, chain, ++rulenum, NFT_TRACE_RETURN);
break;
default:
WARN_ON(1);
@@ -201,8 +202,7 @@ next_rule:
goto next_rule;
}
- if (unlikely(pkt->skb->nf_trace))
- nft_trace_packet(pkt, basechain, -1, NFT_TRACE_POLICY);
+ nft_trace_packet(pkt, basechain, -1, NFT_TRACE_POLICY);
rcu_read_lock_bh();
stats = this_cpu_ptr(rcu_dereference(nft_base_chain(basechain)->stats));
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] netfilter: nf_tables: consolidate tracing invocations
2015-03-03 20:10 [PATCH 0/3] netfilter: nf_tables cleanups Patrick McHardy
2015-03-03 20:10 ` [PATCH 1/3] netfilter: nf_tables: minor tracing cleanups Patrick McHardy
@ 2015-03-03 20:10 ` Patrick McHardy
2015-03-03 20:10 ` [PATCH 3/3] netfilter: nf_tables: cleanup nf_tables.h Patrick McHardy
2015-03-06 0:13 ` [PATCH 0/3] netfilter: nf_tables cleanups Pablo Neira Ayuso
3 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2015-03-03 20:10 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
* JUMP and GOTO are equivalent except for JUMP pushing the current
context to the stack
* RETURN and implicit RETURN (CONTINUE) are equivalent except that
the logged rule number differs
Result:
nft_do_chain | -112
1 function changed, 112 bytes removed, diff: -112
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
net/netfilter/nf_tables_core.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 074067d..77165bf 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -170,26 +170,23 @@ next_rule:
switch (data[NFT_REG_VERDICT].verdict) {
case NFT_JUMP:
- nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
-
BUG_ON(stackptr >= NFT_JUMP_STACK_SIZE);
jumpstack[stackptr].chain = chain;
jumpstack[stackptr].rule = rule;
jumpstack[stackptr].rulenum = rulenum;
stackptr++;
- chain = data[NFT_REG_VERDICT].chain;
- goto do_chain;
+ /* fall through */
case NFT_GOTO:
nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RULE);
chain = data[NFT_REG_VERDICT].chain;
goto do_chain;
+ case NFT_CONTINUE:
+ rulenum++;
+ /* fall through */
case NFT_RETURN:
nft_trace_packet(pkt, chain, rulenum, NFT_TRACE_RETURN);
break;
- case NFT_CONTINUE:
- nft_trace_packet(pkt, chain, ++rulenum, NFT_TRACE_RETURN);
- break;
default:
WARN_ON(1);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] netfilter: nf_tables: cleanup nf_tables.h
2015-03-03 20:10 [PATCH 0/3] netfilter: nf_tables cleanups Patrick McHardy
2015-03-03 20:10 ` [PATCH 1/3] netfilter: nf_tables: minor tracing cleanups Patrick McHardy
2015-03-03 20:10 ` [PATCH 2/3] netfilter: nf_tables: consolidate tracing invocations Patrick McHardy
@ 2015-03-03 20:10 ` Patrick McHardy
2015-03-06 0:13 ` [PATCH 0/3] netfilter: nf_tables cleanups Pablo Neira Ayuso
3 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2015-03-03 20:10 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel
The transaction related definitions are squeezed in between the rule
and expression definitions, which are closely related and should be
next to each other. The transaction definitions actually don't belong
into that file at all since it defines the global objects and API and
transactions are internal to nf_tables_api, but for now simply move
them to a seperate section.
Similar, the chain types are in between a set of registration functions,
they belong to the chain section.
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
include/net/netfilter/nf_tables.h | 174 +++++++++++++++++++-------------------
1 file changed, 87 insertions(+), 87 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index decb9a0..2c6a2e2 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -409,74 +409,6 @@ struct nft_rule {
__attribute__((aligned(__alignof__(struct nft_expr))));
};
-/**
- * struct nft_trans - nf_tables object update in transaction
- *
- * @list: used internally
- * @msg_type: message type
- * @ctx: transaction context
- * @data: internal information related to the transaction
- */
-struct nft_trans {
- struct list_head list;
- int msg_type;
- struct nft_ctx ctx;
- char data[0];
-};
-
-struct nft_trans_rule {
- struct nft_rule *rule;
-};
-
-#define nft_trans_rule(trans) \
- (((struct nft_trans_rule *)trans->data)->rule)
-
-struct nft_trans_set {
- struct nft_set *set;
- u32 set_id;
-};
-
-#define nft_trans_set(trans) \
- (((struct nft_trans_set *)trans->data)->set)
-#define nft_trans_set_id(trans) \
- (((struct nft_trans_set *)trans->data)->set_id)
-
-struct nft_trans_chain {
- bool update;
- char name[NFT_CHAIN_MAXNAMELEN];
- struct nft_stats __percpu *stats;
- u8 policy;
-};
-
-#define nft_trans_chain_update(trans) \
- (((struct nft_trans_chain *)trans->data)->update)
-#define nft_trans_chain_name(trans) \
- (((struct nft_trans_chain *)trans->data)->name)
-#define nft_trans_chain_stats(trans) \
- (((struct nft_trans_chain *)trans->data)->stats)
-#define nft_trans_chain_policy(trans) \
- (((struct nft_trans_chain *)trans->data)->policy)
-
-struct nft_trans_table {
- bool update;
- bool enable;
-};
-
-#define nft_trans_table_update(trans) \
- (((struct nft_trans_table *)trans->data)->update)
-#define nft_trans_table_enable(trans) \
- (((struct nft_trans_table *)trans->data)->enable)
-
-struct nft_trans_elem {
- struct nft_set *set;
- struct nft_set_elem elem;
-};
-
-#define nft_trans_elem_set(trans) \
- (((struct nft_trans_elem *)trans->data)->set)
-#define nft_trans_elem(trans) \
- (((struct nft_trans_elem *)trans->data)->elem)
-
static inline struct nft_expr *nft_expr_first(const struct nft_rule *rule)
{
return (struct nft_expr *)&rule->data[0];
@@ -544,6 +476,25 @@ enum nft_chain_type {
NFT_CHAIN_T_MAX
};
+/**
+ * struct nf_chain_type - nf_tables chain type info
+ *
+ * @name: name of the type
+ * @type: numeric identifier
+ * @family: address family
+ * @owner: module owner
+ * @hook_mask: mask of valid hooks
+ * @hooks: hookfn overrides
+ */
+struct nf_chain_type {
+ const char *name;
+ enum nft_chain_type type;
+ int family;
+ struct module *owner;
+ unsigned int hook_mask;
+ nf_hookfn *hooks[NF_MAX_HOOKS];
+};
+
int nft_chain_validate_dependency(const struct nft_chain *chain,
enum nft_chain_type type);
int nft_chain_validate_hooks(const struct nft_chain *chain,
@@ -630,25 +581,6 @@ struct nft_af_info {
int nft_register_afinfo(struct net *, struct nft_af_info *);
void nft_unregister_afinfo(struct nft_af_info *);
-/**
- * struct nf_chain_type - nf_tables chain type info
- *
- * @name: name of the type
- * @type: numeric identifier
- * @family: address family
- * @owner: module owner
- * @hook_mask: mask of valid hooks
- * @hooks: hookfn overrides
- */
-struct nf_chain_type {
- const char *name;
- enum nft_chain_type type;
- int family;
- struct module *owner;
- unsigned int hook_mask;
- nf_hookfn *hooks[NF_MAX_HOOKS];
-};
-
int nft_register_chain_type(const struct nf_chain_type *);
void nft_unregister_chain_type(const struct nf_chain_type *);
@@ -673,4 +605,72 @@ void nft_unregister_expr(struct nft_expr_type *);
#define MODULE_ALIAS_NFT_SET() \
MODULE_ALIAS("nft-set")
+/**
+ * struct nft_trans - nf_tables object update in transaction
+ *
+ * @list: used internally
+ * @msg_type: message type
+ * @ctx: transaction context
+ * @data: internal information related to the transaction
+ */
+struct nft_trans {
+ struct list_head list;
+ int msg_type;
+ struct nft_ctx ctx;
+ char data[0];
+};
+
+struct nft_trans_rule {
+ struct nft_rule *rule;
+};
+
+#define nft_trans_rule(trans) \
+ (((struct nft_trans_rule *)trans->data)->rule)
+
+struct nft_trans_set {
+ struct nft_set *set;
+ u32 set_id;
+};
+
+#define nft_trans_set(trans) \
+ (((struct nft_trans_set *)trans->data)->set)
+#define nft_trans_set_id(trans) \
+ (((struct nft_trans_set *)trans->data)->set_id)
+
+struct nft_trans_chain {
+ bool update;
+ char name[NFT_CHAIN_MAXNAMELEN];
+ struct nft_stats __percpu *stats;
+ u8 policy;
+};
+
+#define nft_trans_chain_update(trans) \
+ (((struct nft_trans_chain *)trans->data)->update)
+#define nft_trans_chain_name(trans) \
+ (((struct nft_trans_chain *)trans->data)->name)
+#define nft_trans_chain_stats(trans) \
+ (((struct nft_trans_chain *)trans->data)->stats)
+#define nft_trans_chain_policy(trans) \
+ (((struct nft_trans_chain *)trans->data)->policy)
+
+struct nft_trans_table {
+ bool update;
+ bool enable;
+};
+
+#define nft_trans_table_update(trans) \
+ (((struct nft_trans_table *)trans->data)->update)
+#define nft_trans_table_enable(trans) \
+ (((struct nft_trans_table *)trans->data)->enable)
+
+struct nft_trans_elem {
+ struct nft_set *set;
+ struct nft_set_elem elem;
+};
+
+#define nft_trans_elem_set(trans) \
+ (((struct nft_trans_elem *)trans->data)->set)
+#define nft_trans_elem(trans) \
+ (((struct nft_trans_elem *)trans->data)->elem)
+
#endif /* _NET_NF_TABLES_H */
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] netfilter: nf_tables cleanups
2015-03-03 20:10 [PATCH 0/3] netfilter: nf_tables cleanups Patrick McHardy
` (2 preceding siblings ...)
2015-03-03 20:10 ` [PATCH 3/3] netfilter: nf_tables: cleanup nf_tables.h Patrick McHardy
@ 2015-03-06 0:13 ` Pablo Neira Ayuso
2015-03-06 0:41 ` Patrick McHardy
3 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2015-03-06 0:13 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netfilter-devel
On Tue, Mar 03, 2015 at 08:10:03PM +0000, Patrick McHardy wrote:
> These patches for -next contain some cleanups which are not directly
> related to the following set patches, so I'm sending them seperately
> for easier review:
>
> * move the reoccuring checks for skb->nf_trace to an inline function
> * consolidate similar invocations of nft_trace_packet()
> * rearrange nf_tables.h so related things are next to each other
Applied, thanks Patrick.
I'll send another net-next batch with accumulated patches just after
David gets the pending fixes. Then ask for net to go into net-next,
sorry I cannot go any faster.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] netfilter: nf_tables cleanups
2015-03-06 0:13 ` [PATCH 0/3] netfilter: nf_tables cleanups Pablo Neira Ayuso
@ 2015-03-06 0:41 ` Patrick McHardy
0 siblings, 0 replies; 6+ messages in thread
From: Patrick McHardy @ 2015-03-06 0:41 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 06.03, Pablo Neira Ayuso wrote:
> On Tue, Mar 03, 2015 at 08:10:03PM +0000, Patrick McHardy wrote:
> > These patches for -next contain some cleanups which are not directly
> > related to the following set patches, so I'm sending them seperately
> > for easier review:
> >
> > * move the reoccuring checks for skb->nf_trace to an inline function
> > * consolidate similar invocations of nft_trace_packet()
> > * rearrange nf_tables.h so related things are next to each other
>
> Applied, thanks Patrick.
>
> I'll send another net-next batch with accumulated patches just after
> David gets the pending fixes. Then ask for net to go into net-next,
> sorry I cannot go any faster.
Thanks Pablo, I expected this will take a while.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-06 0:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-03 20:10 [PATCH 0/3] netfilter: nf_tables cleanups Patrick McHardy
2015-03-03 20:10 ` [PATCH 1/3] netfilter: nf_tables: minor tracing cleanups Patrick McHardy
2015-03-03 20:10 ` [PATCH 2/3] netfilter: nf_tables: consolidate tracing invocations Patrick McHardy
2015-03-03 20:10 ` [PATCH 3/3] netfilter: nf_tables: cleanup nf_tables.h Patrick McHardy
2015-03-06 0:13 ` [PATCH 0/3] netfilter: nf_tables cleanups Pablo Neira Ayuso
2015-03-06 0:41 ` Patrick McHardy
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).