Skip to content

Commit

Permalink
netfilter: conntrack: add clash resolution stat counter
Browse files Browse the repository at this point in the history
There is a misconception about what "insert_failed" means.

We increment this even when a clash got resolved, so it might not indicate
a problem.

Add a dedicated counter for clash resolution and only increment
insert_failed if a clash cannot be resolved.

For the old /proc interface, export this in place of an older stat
that got removed a while back.
For ctnetlink, export this with a new attribute.

Also correct an outdated comment that implies we add a duplicate tuple --
we only add the (unique) reply direction.

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 Aug 28, 2020
1 parent 4afc41d commit bc92470
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 6 deletions.
1 change: 1 addition & 0 deletions include/linux/netfilter/nf_conntrack_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct ip_conntrack_stat {
unsigned int invalid;
unsigned int insert;
unsigned int insert_failed;
unsigned int clash_resolve;
unsigned int drop;
unsigned int early_drop;
unsigned int error;
Expand Down
1 change: 1 addition & 0 deletions include/uapi/linux/netfilter/nfnetlink_conntrack.h
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ enum ctattr_stats_cpu {
CTA_STATS_EARLY_DROP,
CTA_STATS_ERROR,
CTA_STATS_SEARCH_RESTART,
CTA_STATS_CLASH_RESOLVE,
__CTA_STATS_MAX,
};
#define CTA_STATS_MAX (__CTA_STATS_MAX - 1)
Expand Down
9 changes: 5 additions & 4 deletions net/netfilter/nf_conntrack_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,6 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)

out:
nf_conntrack_double_unlock(hash, reply_hash);
NF_CT_STAT_INC(net, insert_failed);
local_bh_enable();
return -EEXIST;
}
Expand Down Expand Up @@ -934,7 +933,7 @@ static int __nf_ct_resolve_clash(struct sk_buff *skb,
nf_conntrack_put(&loser_ct->ct_general);
nf_ct_set(skb, ct, ctinfo);

NF_CT_STAT_INC(net, insert_failed);
NF_CT_STAT_INC(net, clash_resolve);
return NF_ACCEPT;
}

Expand Down Expand Up @@ -998,6 +997,8 @@ static int nf_ct_resolve_clash_harder(struct sk_buff *skb, u32 repl_idx)

hlist_nulls_add_head_rcu(&loser_ct->tuplehash[IP_CT_DIR_REPLY].hnnode,
&nf_conntrack_hash[repl_idx]);

NF_CT_STAT_INC(net, clash_resolve);
return NF_ACCEPT;
}

Expand Down Expand Up @@ -1027,10 +1028,10 @@ static int nf_ct_resolve_clash_harder(struct sk_buff *skb, u32 repl_idx)
*
* Failing that, the new, unconfirmed conntrack is still added to the table
* provided that the collision only occurs in the ORIGINAL direction.
* The new entry will be added after the existing one in the hash list,
* The new entry will be added only in the non-clashing REPLY direction,
* so packets in the ORIGINAL direction will continue to match the existing
* entry. The new entry will also have a fixed timeout so it expires --
* due to the collision, it will not see bidirectional traffic.
* due to the collision, it will only see reply traffic.
*
* Returns NF_DROP if the clash could not be resolved.
*/
Expand Down
4 changes: 3 additions & 1 deletion net/netfilter/nf_conntrack_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -2516,7 +2516,9 @@ ctnetlink_ct_stat_cpu_fill_info(struct sk_buff *skb, u32 portid, u32 seq,
nla_put_be32(skb, CTA_STATS_EARLY_DROP, htonl(st->early_drop)) ||
nla_put_be32(skb, CTA_STATS_ERROR, htonl(st->error)) ||
nla_put_be32(skb, CTA_STATS_SEARCH_RESTART,
htonl(st->search_restart)))
htonl(st->search_restart)) ||
nla_put_be32(skb, CTA_STATS_CLASH_RESOLVE,
htonl(st->clash_resolve)))
goto nla_put_failure;

nlmsg_end(skb, nlh);
Expand Down
2 changes: 1 addition & 1 deletion net/netfilter/nf_conntrack_standalone.c
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ static int ct_cpu_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "%08x %08x %08x %08x %08x %08x %08x %08x "
"%08x %08x %08x %08x %08x %08x %08x %08x %08x\n",
nr_conntracks,
0,
st->clash_resolve, /* was: searched */
st->found,
0,
st->invalid,
Expand Down

0 comments on commit bc92470

Please sign in to comment.