netfilter: conntrack: add clash resolution stat counter
authorFlorian Westphal <fw@strlen.de>
Tue, 25 Aug 2020 22:52:44 +0000 (00:52 +0200)
committerPablo Neira Ayuso <pablo@netfilter.org>
Fri, 28 Aug 2020 17:51:26 +0000 (19:51 +0200)
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>
include/linux/netfilter/nf_conntrack_common.h
include/uapi/linux/netfilter/nfnetlink_conntrack.h
net/netfilter/nf_conntrack_core.c
net/netfilter/nf_conntrack_netlink.c
net/netfilter/nf_conntrack_standalone.c

index 96b90d7..0c7d8d1 100644 (file)
@@ -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;
index 3e47155..d8484be 100644 (file)
@@ -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)
index a111bcf..93e77ca 100644 (file)
@@ -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;
 }
@@ -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;
        }
 
@@ -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;
 }
 
@@ -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.
  */
index c64f23a..89d99f6 100644 (file)
@@ -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);
index b673a03..0ff3974 100644 (file)
@@ -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,