I probably will change the name of these stats to 'invalid' and add a comment to the effect that the associated packets are usually dropped. I guess it would also be good to specify that only packet sanity invalids are tracked, which should be rare.
Thanks Darrell On Tue, Jun 26, 2018 at 4:16 PM, Darrell Ball <[email protected]> wrote: > Signed-off-by: Darrell Ball <[email protected]> > --- > NEWS | 2 + > lib/conntrack.c | 114 ++++++++++++++++++++++++++++++ > +++++++++++++++++++--- > lib/conntrack.h | 13 ++++++ > lib/ct-dpif.c | 24 +++++++++++ > lib/ct-dpif.h | 13 ++++++ > lib/dpctl.c | 62 ++++++++++++++++++++++++++++ > lib/dpctl.man | 2 +- > lib/dpif-netdev.c | 26 ++++++++++++ > lib/dpif-netlink.c | 1 + > lib/dpif-provider.h | 5 +++ > 10 files changed, 256 insertions(+), 6 deletions(-) > > diff --git a/NEWS b/NEWS > index cd15a33..04e6a53 100644 > --- a/NEWS > +++ b/NEWS > @@ -40,6 +40,8 @@ Post-v2.9.0 > ovs-appctl dpif-netdev/pmd-perf-show > * Supervision of PMD performance metrics and logging of suspicious > iterations > + * ovs-appctl dpctl/ct-stats-show now prints conntrack global drop > + statistics. > - ERSPAN: > * Implemented ERSPAN protocol (draft-foschiano-erspan-00.txt) for > both kernel datapath and userspace datapath. > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 825f1ad..ecc39de 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -228,6 +228,25 @@ long long ct_timeout_val[] = { > * are accepted; this is for CT_CONN_TYPE_DEFAULT connections. */ > #define DEFAULT_N_CONN_LIMIT 3000000 > > +/* IPv4 sanity drops. */ > +static atomic_count min_hdr_err_v4; > +static atomic_count size_err_v4; > +static atomic_count cksum_err_v4; > + > +/* IPv6 sanity drops. */ > +static atomic_count min_hdr_err_v6; > +static atomic_count hdr_parse_err_v6; > + > +/* L4 sanity drops. */ > +static atomic_count hdr_size_err_tcp; > +static atomic_count size_err_tcp; > +static atomic_count cksum_err_tcp; > +static atomic_count hdr_size_err_udp; > +static atomic_count size_err_udp; > +static atomic_count cksum_err_udp; > +static atomic_count cksum_err_icmp; > +static atomic_count cksum_err_icmp6; > + > /* Does a member by member comparison of two conn_keys; this > * function must be kept in sync with struct conn_key; returns 0 > * if the keys are equal or 1 if the keys are not equal. */ > @@ -339,6 +358,20 @@ conntrack_init(struct conntrack *ct) > ct->hash_basis = random_uint32(); > atomic_count_init(&ct->n_conn, 0); > atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); > + atomic_count_init(&min_hdr_err_v4, 0); > + atomic_count_init(&size_err_v4, 0); > + atomic_count_init(&cksum_err_v4, 0); > + atomic_count_init(&min_hdr_err_v6, 0); > + atomic_count_init(&hdr_parse_err_v6, 0); > + atomic_count_init(&hdr_size_err_tcp, 0); > + atomic_count_init(&size_err_tcp, 0); > + atomic_count_init(&cksum_err_tcp, 0); > + atomic_count_init(&hdr_size_err_udp, 0); > + atomic_count_init(&size_err_udp, 0); > + atomic_count_init(&cksum_err_udp, 0); > + atomic_count_init(&cksum_err_icmp, 0); > + atomic_count_init(&cksum_err_icmp6, 0); > + > latch_init(&ct->clean_thread_exit); > ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, > ct); > } > @@ -1502,6 +1535,7 @@ extract_l3_ipv4(const void *data, size_t size, bool > validate_checksum, > struct conn_key *key, const char **new_data) > { > if (OVS_UNLIKELY(size < IP_HEADER_LEN)) { > + atomic_count_inc(&min_hdr_err_v4); > return false; > } > > @@ -1509,14 +1543,17 @@ extract_l3_ipv4(const void *data, size_t size, > bool validate_checksum, > size_t ip_len = IP_IHL(ip->ip_ihl_ver) * 4; > > if (OVS_UNLIKELY(ip_len < IP_HEADER_LEN)) { > + atomic_count_inc(&min_hdr_err_v4); > return false; > } > > if (OVS_UNLIKELY(size < ip_len)) { > + atomic_count_inc(&size_err_v4); > return false; > } > > if (OVS_UNLIKELY(validate_checksum && csum(data, ip_len) != 0)) { > + atomic_count_inc(&cksum_err_v4); > return false; > } > > @@ -1542,6 +1579,7 @@ extract_l3_ipv6(const void *data, size_t size, > struct conn_key *key, > const struct ovs_16aligned_ip6_hdr *ip6 = data; > > if (OVS_UNLIKELY(size < sizeof *ip6)) { > + atomic_count_inc(&min_hdr_err_v6); > return false; > } > > @@ -1552,6 +1590,7 @@ extract_l3_ipv6(const void *data, size_t size, > struct conn_key *key, > > if (OVS_UNLIKELY(!parse_ipv6_ext_hdrs(&data, &size, &nw_proto, > &nw_frag))) { > + atomic_count_inc(&hdr_parse_err_v6); > return false; > } > > @@ -1595,15 +1634,24 @@ check_l4_tcp(const struct conn_key *key, const > void *data, size_t size, > { > const struct tcp_header *tcp = data; > if (size < sizeof *tcp) { > + atomic_count_inc(&hdr_size_err_tcp); > return false; > } > > size_t tcp_len = TCP_OFFSET(tcp->tcp_ctl) * 4; > if (OVS_UNLIKELY(tcp_len < TCP_HEADER_LEN || tcp_len > size)) { > + atomic_count_inc(&size_err_tcp); > return false; > } > > - return validate_checksum ? checksum_valid(key, data, size, l3) : true; > + if (!validate_checksum) { > + return true; > + } else if (checksum_valid(key, data, size, l3)) { > + return true; > + } else { > + atomic_count_inc(&cksum_err_tcp); > + return false; > + } > } > > static inline bool > @@ -1612,30 +1660,53 @@ check_l4_udp(const struct conn_key *key, const > void *data, size_t size, > { > const struct udp_header *udp = data; > if (size < sizeof *udp) { > + atomic_count_inc(&hdr_size_err_udp); > return false; > } > > size_t udp_len = ntohs(udp->udp_len); > if (OVS_UNLIKELY(udp_len < UDP_HEADER_LEN || udp_len > size)) { > + atomic_count_inc(&size_err_udp); > return false; > } > > /* Validation must be skipped if checksum is 0 on IPv4 packets */ > - return (udp->udp_csum == 0 && key->dl_type == htons(ETH_TYPE_IP)) > - || (validate_checksum ? checksum_valid(key, data, size, l3) : > true); > + if (!validate_checksum || > + (udp->udp_csum == 0 && key->dl_type == htons(ETH_TYPE_IP))) { > + return true; > + } else if (checksum_valid(key, data, size, l3)) { > + return true; > + } else { > + atomic_count_inc(&cksum_err_udp); > + return false; > + } > } > > static inline bool > check_l4_icmp(const void *data, size_t size, bool validate_checksum) > { > - return validate_checksum ? csum(data, size) == 0 : true; > + if (!validate_checksum) { > + return true; > + } else if (csum(data, size) == 0) { > + return true; > + } else { > + atomic_count_inc(&cksum_err_icmp); > + return false; > + } > } > > static inline bool > check_l4_icmp6(const struct conn_key *key, const void *data, size_t size, > const void *l3, bool validate_checksum) > { > - return validate_checksum ? checksum_valid(key, data, size, l3) : true; > + if (!validate_checksum) { > + return true; > + } else if (checksum_valid(key, data, size, l3)) { > + return true; > + } else { > + atomic_count_inc(&cksum_err_icmp6); > + return false; > + } > } > > static inline bool > @@ -1946,6 +2017,7 @@ conn_key_extract(struct conntrack *ct, struct > dp_packet *pkt, ovs_be16 dl_type, > bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt); > if (hwol_bad_l3_csum) { > ok = false; > + atomic_count_inc(&cksum_err_v4); > } else { > bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt); > /* Validate the checksum only when hwol is not supported. */ > @@ -2594,6 +2666,38 @@ conntrack_get_nconns(struct conntrack *ct, uint32_t > *nconns) > return 0; > } > > +int > +conntrack_get_drop_stats(unsigned int *ct_min_hdr_err_v4, > + unsigned int *ct_size_err_v4, > + unsigned int *ct_cksum_err_v4, > + unsigned int *ct_min_hdr_err_v6, > + unsigned int *ct_hdr_parse_err_v6, > + unsigned int *ct_hdr_size_err_tcp, > + unsigned int *ct_size_err_tcp, > + unsigned int *ct_cksum_err_tcp, > + unsigned int *ct_hdr_size_err_udp, > + unsigned int *ct_size_err_udp, > + unsigned int *ct_cksum_err_udp, > + unsigned int *ct_cksum_err_icmp, > + unsigned int *ct_cksum_err_icmp6) > +{ > + *ct_min_hdr_err_v4 = atomic_count_get(&min_hdr_err_v4); > + *ct_size_err_v4 = atomic_count_get(&size_err_v4); > + *ct_cksum_err_v4 = atomic_count_get(&cksum_err_v4); > + *ct_min_hdr_err_v6 = atomic_count_get(&min_hdr_err_v6); > + *ct_hdr_parse_err_v6 = atomic_count_get(&hdr_parse_err_v6); > + *ct_hdr_size_err_tcp = atomic_count_get(&hdr_size_err_tcp); > + *ct_size_err_tcp = atomic_count_get(&size_err_tcp); > + *ct_cksum_err_tcp = atomic_count_get(&cksum_err_tcp); > + *ct_hdr_size_err_udp = atomic_count_get(&hdr_size_err_udp); > + *ct_size_err_udp = atomic_count_get(&size_err_udp); > + *ct_cksum_err_udp = atomic_count_get(&cksum_err_udp); > + *ct_cksum_err_icmp = atomic_count_get(&cksum_err_icmp); > + *ct_cksum_err_icmp6 = atomic_count_get(&cksum_err_icmp6); > + > + return 0; > +} > + > /* This function must be called with the ct->resources read lock taken. */ > static struct alg_exp_node * > expectation_lookup(struct hmap *alg_expectations, const struct conn_key > *key, > diff --git a/lib/conntrack.h b/lib/conntrack.h > index e3a5dcc..e65c080 100644 > --- a/lib/conntrack.h > +++ b/lib/conntrack.h > @@ -122,6 +122,19 @@ int conntrack_flush_tuple(struct conntrack *, const > struct ct_dpif_tuple *, > int conntrack_set_maxconns(struct conntrack *ct, uint32_t maxconns); > int conntrack_get_maxconns(struct conntrack *ct, uint32_t *maxconns); > int conntrack_get_nconns(struct conntrack *ct, uint32_t *nconns); > +int conntrack_get_drop_stats(unsigned int *ct_min_hdr_err_v4, > + unsigned int *ct_size_err_v4, > + unsigned int *ct_cksum_err_v4, > + unsigned int *ct_min_hdr_err_v6, > + unsigned int *ct_hdr_parse_err_v6, > + unsigned int *ct_hdr_size_err_tcp, > + unsigned int *ct_size_err_tcp, > + unsigned int *ct_cksum_err_tcp, > + unsigned int *ct_hdr_size_err_udp, > + unsigned int *ct_size_err_udp, > + unsigned int *ct_cksum_err_udp, > + unsigned int *ct_cksum_err_icmp, > + unsigned int *ct_cksum_err_icmp6); > > /* 'struct ct_lock' is a wrapper for an adaptive mutex. It's useful to > try > * different types of locks (e.g. spinlocks) */ > diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c > index 5fa3a97..12a6987 100644 > --- a/lib/ct-dpif.c > +++ b/lib/ct-dpif.c > @@ -164,6 +164,30 @@ ct_dpif_get_nconns(struct dpif *dpif, uint32_t > *nconns) > : EOPNOTSUPP); > } > > +int > +ct_dpif_get_drop_stats(struct dpif *dpif, unsigned int *min_hdr_err_v4, > + unsigned int *size_err_v4, > + unsigned int *cksum_err_v4, > + unsigned int *min_hdr_err_v6, > + unsigned int *hdr_parse_err_v6, > + unsigned int *hdr_size_err_tcp, > + unsigned int *size_err_tcp, > + unsigned int *cksum_err_tcp, > + unsigned int *hdr_size_err_udp, > + unsigned int *size_err_udp, > + unsigned int *cksum_err_udp, > + unsigned int *cksum_err_icmp, > + unsigned int *cksum_err_icmp6) > +{ > + return (dpif->dpif_class->ct_get_drop_stats > + ? dpif->dpif_class->ct_get_drop_stats(dpif, min_hdr_err_v4, > + size_err_v4, cksum_err_v4, min_hdr_err_v6, > + hdr_parse_err_v6, hdr_size_err_tcp, size_err_tcp, > + cksum_err_tcp, hdr_size_err_udp, size_err_udp, > + cksum_err_udp, cksum_err_icmp, cksum_err_icmp6) > + : EOPNOTSUPP); > +} > + > void > ct_dpif_entry_uninit(struct ct_dpif_entry *entry) > { > diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h > index 09e7698..af010df 100644 > --- a/lib/ct-dpif.h > +++ b/lib/ct-dpif.h > @@ -200,6 +200,19 @@ int ct_dpif_flush(struct dpif *, const uint16_t *zone, > int ct_dpif_set_maxconns(struct dpif *dpif, uint32_t maxconns); > int ct_dpif_get_maxconns(struct dpif *dpif, uint32_t *maxconns); > int ct_dpif_get_nconns(struct dpif *dpif, uint32_t *nconns); > +int ct_dpif_get_drop_stats(struct dpif *dpif, unsigned int > *min_hdr_err_v4, > + unsigned int *size_err_v4, > + unsigned int *cksum_err_v4, > + unsigned int *min_hdr_err_v6, > + unsigned int *hdr_parse_err_v6, > + unsigned int *hdr_size_err_tcp, > + unsigned int *size_err_tcp, > + unsigned int *cksum_err_tcp, > + unsigned int *hdr_size_err_udp, > + unsigned int *size_err_udp, > + unsigned int *cksum_err_udp, > + unsigned int *cksum_err_icmp, > + unsigned int *cksum_err_icmp6); > void ct_dpif_entry_uninit(struct ct_dpif_entry *); > void ct_dpif_format_entry(const struct ct_dpif_entry *, struct ds *, > bool verbose, bool print_stats); > diff --git a/lib/dpctl.c b/lib/dpctl.c > index 4f1e443..1ef1a74 100644 > --- a/lib/dpctl.c > +++ b/lib/dpctl.c > @@ -1381,6 +1381,67 @@ error: > return error; > } > > +static void > +ct_print_drop_stats(struct dpctl_params *dpctl_p, struct dpif *dpif) > +{ > + if (dpif) { > + unsigned int min_hdr_err_v4; > + unsigned int size_err_v4; > + unsigned int cksum_err_v4; > + unsigned int min_hdr_err_v6; > + unsigned int hdr_parse_err_v6; > + unsigned int hdr_size_err_tcp; > + unsigned int size_err_tcp; > + unsigned int cksum_err_tcp; > + unsigned int hdr_size_err_udp; > + unsigned int size_err_udp; > + unsigned int cksum_err_udp; > + unsigned int cksum_err_icmp; > + unsigned int cksum_err_icmp6; > + int error = ct_dpif_get_drop_stats(dpif, &min_hdr_err_v4, > &size_err_v4, > + &cksum_err_v4, &min_hdr_err_v6, > + &hdr_parse_err_v6, > + &hdr_size_err_tcp, > &size_err_tcp, > + &cksum_err_tcp, > + &hdr_size_err_udp, > &size_err_udp, > + &cksum_err_udp, > &cksum_err_icmp, > + &cksum_err_icmp6); > + > + if (!error) { > + dpctl_print(dpctl_p, "\nConnTracker Drop Stats:\n"); > + dpctl_print(dpctl_p, "v4 minimum header errors: %u\n", > + min_hdr_err_v4); > + dpctl_print(dpctl_p, "v4 packet size errors: %u\n", > + size_err_v4); > + dpctl_print(dpctl_p, "v4 checksum errors: %u\n", > + cksum_err_v4); > + dpctl_print(dpctl_p, "v6 minimum header errors: %u\n", > + min_hdr_err_v6); > + dpctl_print(dpctl_p, "v6 parse errors: %u\n", > + hdr_parse_err_v6); > + dpctl_print(dpctl_p, "tcp header size errors: %u\n", > + cksum_err_tcp); > + dpctl_print(dpctl_p, "tcp size errors: %u\n", > + cksum_err_tcp); > + dpctl_print(dpctl_p, "tcp checksum errors: %u\n", > + cksum_err_tcp); > + dpctl_print(dpctl_p, "udp header size errors: %u\n", > + cksum_err_tcp); > + dpctl_print(dpctl_p, "udp size errors: %u\n", > + cksum_err_tcp); > + dpctl_print(dpctl_p, "udp checksum errors: %u\n", > + cksum_err_udp); > + dpctl_print(dpctl_p, "icmp checksum errors: %u\n", > + cksum_err_icmp); > + dpctl_print(dpctl_p, "icmp6 checksum errors: %u\n", > + cksum_err_icmp6); > + } else { > + dpctl_error(dpctl_p, error, > + "ct drop stats could not be retrieved"); > + } > + } > +} > + > static int > dpctl_ct_stats_show(int argc, const char *argv[], > struct dpctl_params *dpctl_p) > @@ -1514,6 +1575,7 @@ dpctl_ct_stats_show(int argc, const char *argv[], > } > > ct_dpif_dump_done(dump); > + ct_print_drop_stats(dpctl_p, dpif); > dpif_close(dpif); > return error; > } > diff --git a/lib/dpctl.man b/lib/dpctl.man > index 5d987e6..8b45a3a 100644 > --- a/lib/dpctl.man > +++ b/lib/dpctl.man > @@ -244,7 +244,7 @@ An example of an IPv6 TCP \fIct-tuple\fR: > Displays the number of connections grouped by protocol used by \fIdp\fR. > If \fBzone=\fIzone\fR is specified, numbers refer to the connections in > \fIzone\fR. With \fB\-\-more\fR, groups by connection state for each > -protocol. > +protocol. Prints global drop stats for the userspace connection tracker. > . > .TP > \*(DX\fBct\-bkts\fR [\fIdp\fR] [\fBgt=\fIthreshold\fR] > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 9390fff..fce36c3 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -6044,6 +6044,31 @@ dpif_netdev_ct_get_nconns(struct dpif *dpif, > uint32_t *nconns) > return conntrack_get_nconns(&dp->conntrack, nconns); > } > > +static int > +dpif_netdev_ct_get_drop_stats(struct dpif *dpif OVS_UNUSED, > + unsigned int *ct_min_hdr_err_v4, > + unsigned int *ct_size_err_v4, > + unsigned int *ct_cksum_err_v4, > + unsigned int *ct_min_hdr_err_v6, > + unsigned int *ct_hdr_parse_err_v6, > + unsigned int *ct_hdr_size_err_tcp, > + unsigned int *ct_size_err_tcp, > + unsigned int *ct_cksum_err_tcp, > + unsigned int *ct_hdr_size_err_udp, > + unsigned int *ct_size_err_udp, > + unsigned int *ct_cksum_err_udp, > + unsigned int *ct_cksum_err_icmp, > + unsigned int *ct_cksum_err_icmp6) > +{ > + return conntrack_get_drop_stats(ct_min_hdr_err_v4, ct_size_err_v4, > + ct_cksum_err_v4, ct_min_hdr_err_v6, > + ct_hdr_parse_err_v6, > ct_hdr_size_err_tcp, > + ct_size_err_tcp, ct_cksum_err_tcp, > + ct_hdr_size_err_udp, ct_size_err_udp, > + ct_cksum_err_udp, ct_cksum_err_icmp, > + ct_cksum_err_icmp6); > +} > + > const struct dpif_class dpif_netdev_class = { > "netdev", > dpif_netdev_init, > @@ -6092,6 +6117,7 @@ const struct dpif_class dpif_netdev_class = { > dpif_netdev_ct_set_maxconns, > dpif_netdev_ct_get_maxconns, > dpif_netdev_ct_get_nconns, > + dpif_netdev_ct_get_drop_stats, > dpif_netdev_meter_get_features, > dpif_netdev_meter_set, > dpif_netdev_meter_get, > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index aa9bbd9..a0a6e11 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -3006,6 +3006,7 @@ const struct dpif_class dpif_netlink_class = { > NULL, /* ct_set_maxconns */ > NULL, /* ct_get_maxconns */ > NULL, /* ct_get_nconns */ > + NULL, /* ct_get_drop_stats */ > dpif_netlink_meter_get_features, > dpif_netlink_meter_set, > dpif_netlink_meter_get, > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h > index 62b3598..b3ac02b 100644 > --- a/lib/dpif-provider.h > +++ b/lib/dpif-provider.h > @@ -443,6 +443,11 @@ struct dpif_class { > int (*ct_get_maxconns)(struct dpif *, uint32_t *maxconns); > /* Get number of connections tracked. */ > int (*ct_get_nconns)(struct dpif *, uint32_t *nconns); > + int (*ct_get_drop_stats)(struct dpif *, unsigned int *, > + unsigned int *, unsigned int *, unsigned int *, > + unsigned int *, unsigned int *, unsigned int *, > + unsigned int *, unsigned int *, unsigned int *, > + unsigned int *, unsigned int *, unsigned int *); > > /* Meters */ > > -- > 1.9.1 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
