Skip to content

Commit

Permalink
netfilter: nf_tables: do not store rule in traceinfo structure
Browse files Browse the repository at this point in the history
pass it as argument instead.  This reduces size of traceinfo to
16 bytes.  Total stack usage:

 nf_tables_core.c:252 nft_do_chain    304     static

While its possible to also pass basechain as argument, doing so
increases nft_do_chaininfo function size.

Unlike pktinfo/verdict/rule the basechain info isn't used in
the expression evaluation path. gcc places it on the stack, which
results in extra push/pop when it gets passed to the trace helpers
as argument rather than as part of the traceinfo structure.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
  • Loading branch information
Florian Westphal authored and Pablo Neira Ayuso committed Apr 21, 2023
1 parent 0a20214 commit 46df417
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 16 deletions.
3 changes: 1 addition & 2 deletions include/net/netfilter/nf_tables.h
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,6 @@ void nft_unregister_flowtable_type(struct nf_flowtable_type *type);
* @skbid: hash of skb to be used as trace id
* @packet_dumped: packet headers sent in a previous traceinfo message
* @basechain: base chain currently processed
* @rule: rule that was evaluated
*/
struct nft_traceinfo {
bool trace;
Expand All @@ -1418,14 +1417,14 @@ struct nft_traceinfo {
enum nft_trace_types type:8;
u32 skbid;
const struct nft_base_chain *basechain;
const struct nft_rule_dp *rule;
};

void nft_trace_init(struct nft_traceinfo *info, const struct nft_pktinfo *pkt,
const struct nft_chain *basechain);

void nft_trace_notify(const struct nft_pktinfo *pkt,
const struct nft_verdict *verdict,
const struct nft_rule_dp *rule,
struct nft_traceinfo *info);

#define MODULE_ALIAS_NFT_CHAIN(family, name) \
Expand Down
15 changes: 7 additions & 8 deletions net/netfilter/nf_tables_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ static inline void nf_skip_indirect_calls_enable(void) { }

static noinline void __nft_trace_packet(const struct nft_pktinfo *pkt,
const struct nft_verdict *verdict,
const struct nft_rule_dp *rule,
struct nft_traceinfo *info,
enum nft_trace_types type)
{
Expand All @@ -51,7 +52,7 @@ static noinline void __nft_trace_packet(const struct nft_pktinfo *pkt,

info->type = type;

nft_trace_notify(pkt, verdict, info);
nft_trace_notify(pkt, verdict, rule, info);
}

static inline void nft_trace_packet(const struct nft_pktinfo *pkt,
Expand All @@ -62,8 +63,7 @@ static inline void nft_trace_packet(const struct nft_pktinfo *pkt,
{
if (static_branch_unlikely(&nft_trace_enabled)) {
info->nf_trace = pkt->skb->nf_trace;
info->rule = rule;
__nft_trace_packet(pkt, verdict, info, type);
__nft_trace_packet(pkt, verdict, rule, info, type);
}
}

Expand Down Expand Up @@ -110,6 +110,7 @@ static void nft_cmp16_fast_eval(const struct nft_expr *expr,

static noinline void __nft_trace_verdict(const struct nft_pktinfo *pkt,
struct nft_traceinfo *info,
const struct nft_rule_dp *rule,
const struct nft_regs *regs)
{
enum nft_trace_types type;
Expand All @@ -131,18 +132,16 @@ static noinline void __nft_trace_verdict(const struct nft_pktinfo *pkt,
break;
}

__nft_trace_packet(pkt, &regs->verdict, info, type);
__nft_trace_packet(pkt, &regs->verdict, rule, info, type);
}

static inline void nft_trace_verdict(const struct nft_pktinfo *pkt,
struct nft_traceinfo *info,
const struct nft_rule_dp *rule,
const struct nft_regs *regs)
{
if (static_branch_unlikely(&nft_trace_enabled)) {
info->rule = rule;
__nft_trace_verdict(pkt, info, regs);
}
if (static_branch_unlikely(&nft_trace_enabled))
__nft_trace_verdict(pkt, info, rule, regs);
}

static bool nft_payload_fast_eval(const struct nft_expr *expr,
Expand Down
14 changes: 8 additions & 6 deletions net/netfilter/nf_tables_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ static int nf_trace_fill_pkt_info(struct sk_buff *nlskb,

static int nf_trace_fill_rule_info(struct sk_buff *nlskb,
const struct nft_verdict *verdict,
const struct nft_rule_dp *rule,
const struct nft_traceinfo *info)
{
if (!info->rule || info->rule->is_last)
if (!rule || rule->is_last)
return 0;

/* a continue verdict with ->type == RETURN means that this is
Expand All @@ -140,7 +141,7 @@ static int nf_trace_fill_rule_info(struct sk_buff *nlskb,
return 0;

return nla_put_be64(nlskb, NFTA_TRACE_RULE_HANDLE,
cpu_to_be64(info->rule->handle),
cpu_to_be64(rule->handle),
NFTA_TRACE_PAD);
}

Expand All @@ -166,9 +167,9 @@ static bool nft_trace_have_verdict_chain(const struct nft_verdict *verdict,
return true;
}

static const struct nft_chain *nft_trace_get_chain(const struct nft_traceinfo *info)
static const struct nft_chain *nft_trace_get_chain(const struct nft_rule_dp *rule,
const struct nft_traceinfo *info)
{
const struct nft_rule_dp *rule = info->rule;
const struct nft_rule_dp_last *last;

if (!rule)
Expand All @@ -187,6 +188,7 @@ static const struct nft_chain *nft_trace_get_chain(const struct nft_traceinfo *i

void nft_trace_notify(const struct nft_pktinfo *pkt,
const struct nft_verdict *verdict,
const struct nft_rule_dp *rule,
struct nft_traceinfo *info)
{
const struct nft_chain *chain;
Expand All @@ -199,7 +201,7 @@ void nft_trace_notify(const struct nft_pktinfo *pkt,
if (!nfnetlink_has_listeners(nft_net(pkt), NFNLGRP_NFTRACE))
return;

chain = nft_trace_get_chain(info);
chain = nft_trace_get_chain(rule, info);

size = nlmsg_total_size(sizeof(struct nfgenmsg)) +
nla_total_size(strlen(chain->table->name)) +
Expand Down Expand Up @@ -248,7 +250,7 @@ void nft_trace_notify(const struct nft_pktinfo *pkt,
if (nla_put_string(skb, NFTA_TRACE_TABLE, chain->table->name))
goto nla_put_failure;

if (nf_trace_fill_rule_info(skb, verdict, info))
if (nf_trace_fill_rule_info(skb, verdict, rule, info))
goto nla_put_failure;

switch (info->type) {
Expand Down

0 comments on commit 46df417

Please sign in to comment.