From 233e4681830bc2a9cd40deb4b5909b4e310d1a2a Mon Sep 17 00:00:00 2001 From: Ole Troan Date: Thu, 16 May 2019 15:01:34 +0200 Subject: [PATCH] stats: support multiple works for error counters The current code only allowed access to the main thread error counters. That is not so useful for a multi worker instance. No return a vector indexed by thread of counter_t values. Type: fix Change-Id: Ie322c8889c0c8175e1116e71de04a2cf453b9ed7 Signed-off-by: Ole Troan --- extras/vom/vom/stat_client.cpp | 6 +- extras/vom/vom/stat_client.hpp | 4 +- src/vlib/counter.c | 14 +--- src/vlib/error.c | 20 +----- src/vlib/stat_weak_inlines.h | 61 +++++++++++++++++ src/vlib/threads.c | 27 ++++---- src/vnet/interface_api.c | 16 ----- src/vnet/ip/ip_api.c | 24 ------- src/vnet/mpls/mpls_api.c | 7 -- src/vnet/pg/pg_api.c | 3 - src/vpp-api/client/stat_client.c | 15 ++-- src/vpp-api/client/stat_client.h | 4 +- src/vpp-api/python/vpp_papi/vpp_stats.py | 19 ++++-- src/vpp/app/vpp_get_stats.c | 8 ++- src/vpp/app/vpp_prometheus_export.c | 10 ++- src/vpp/stats/stat_segment.c | 16 +++-- src/vpp/stats/stat_segment.h | 1 + src/vpp/stats/stats.md | 3 +- test/framework.py | 5 ++ test/template_ipsec.py | 12 ++-- test/test_ipip.py | 10 +-- test/test_nat.py | 113 +++++++++++++++++-------------- test/test_punt.py | 2 +- test/test_stats_client.py | 6 ++ 24 files changed, 223 insertions(+), 183 deletions(-) create mode 100644 src/vlib/stat_weak_inlines.h diff --git a/extras/vom/vom/stat_client.cpp b/extras/vom/vom/stat_client.cpp index a413d6763f0..394c6eeee96 100644 --- a/extras/vom/vom/stat_client.cpp +++ b/extras/vom/vom/stat_client.cpp @@ -32,7 +32,7 @@ stat_client::stat_data_t::stat_data_t(const stat_segment_data_t& stat_seg_data) m_combined_counter_vec = stat_seg_data.combined_counter_vec; break; case STAT_DIR_TYPE_ERROR_INDEX: - m_error_value = stat_seg_data.error_value; + m_error_vec = stat_seg_data.error_vector; break; case STAT_DIR_TYPE_NAME_VECTOR: break; @@ -59,10 +59,10 @@ stat_client::stat_data_t::get_stat_segment_scalar_data() const return m_scalar_value; } -uint64_t +uint64_t* stat_client::stat_data_t::get_stat_segment_error_data() const { - return m_error_value; + return m_error_vec; } uint64_t** diff --git a/extras/vom/vom/stat_client.hpp b/extras/vom/vom/stat_client.hpp index 8e014da19b1..f1745c87b20 100644 --- a/extras/vom/vom/stat_client.hpp +++ b/extras/vom/vom/stat_client.hpp @@ -56,7 +56,7 @@ public: * Get pointer to actual data */ double get_stat_segment_scalar_data() const; - uint64_t get_stat_segment_error_data() const; + uint64_t* get_stat_segment_error_data() const; uint64_t** get_stat_segment_simple_counter_data() const; vlib_counter_t** get_stat_segment_combined_counter_data() const; @@ -77,7 +77,7 @@ public: union { double m_scalar_value; - uint64_t m_error_value; + counter_t* m_error_vec; counter_t** m_simple_counter_vec; vlib_counter_t** m_combined_counter_vec; }; diff --git a/src/vlib/counter.c b/src/vlib/counter.c index 5b73d5fa21a..faf106942b7 100644 --- a/src/vlib/counter.c +++ b/src/vlib/counter.c @@ -38,6 +38,7 @@ */ #include +#include void vlib_clear_simple_counters (vlib_simple_counter_main_t * cm) @@ -74,19 +75,6 @@ vlib_clear_combined_counters (vlib_combined_counter_main_t * cm) } } -void *vlib_stats_push_heap (void *) __attribute__ ((weak)); -void * -vlib_stats_push_heap (void *unused) -{ - return 0; -}; - -void vlib_stats_pop_heap (void *, void *, u32, int) __attribute__ ((weak)); -void -vlib_stats_pop_heap (void *notused, void *notused2, u32 i, int type) -{ -}; - void vlib_validate_simple_counter (vlib_simple_counter_main_t * cm, u32 index) { diff --git a/src/vlib/error.c b/src/vlib/error.c index a416649cfa7..7c61f319fc0 100644 --- a/src/vlib/error.c +++ b/src/vlib/error.c @@ -39,6 +39,7 @@ #include #include +#include uword vlib_error_drop_buffers (vlib_main_t * vm, @@ -109,20 +110,6 @@ vlib_error_drop_buffers (vlib_main_t * vm, return n_buffers; } -void vlib_stats_register_error_index (u8 *, u64 *, u64) - __attribute__ ((weak)); -void -vlib_stats_register_error_index (u8 * notused, u64 * notused2, u64 notused3) -{ -}; - -void vlib_stats_pop_heap2 (void *, u32, void *) __attribute__ ((weak)); -void -vlib_stats_pop_heap2 (void *notused, u32 notused2, void *notused3) -{ -}; - - /* Reserves given number of error codes for given node. */ void vlib_register_errors (vlib_main_t * vm, @@ -132,7 +119,6 @@ vlib_register_errors (vlib_main_t * vm, vlib_node_t *n = vlib_get_node (vm, node_index); uword l; void *oldheap; - void *vlib_stats_push_heap (void) __attribute__ ((weak)); ASSERT (vlib_get_thread_index () == 0); @@ -157,7 +143,7 @@ vlib_register_errors (vlib_main_t * vm, vec_validate (vm->error_elog_event_types, l - 1); /* Switch to the stats segment ... */ - oldheap = vlib_stats_push_heap (); + oldheap = vlib_stats_push_heap (0); /* Allocate a counter/elog type for each error. */ vec_validate (em->counters, l - 1); @@ -186,7 +172,7 @@ vlib_register_errors (vlib_main_t * vm, } /* (re)register the em->counters base address, switch back to main heap */ - vlib_stats_pop_heap2 (em->counters, vm->thread_index, oldheap); + vlib_stats_pop_heap2 (em->counters, vm->thread_index, oldheap, 1); { elog_event_type_t t; diff --git a/src/vlib/stat_weak_inlines.h b/src/vlib/stat_weak_inlines.h new file mode 100644 index 00000000000..5c04610ef13 --- /dev/null +++ b/src/vlib/stat_weak_inlines.h @@ -0,0 +1,61 @@ +/* + * Copyright (c) 2019 Cisco and/or its affiliates. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * NOTE: Only include this file from external components that require + * a loose coupling to the stats component. + */ + +#ifndef included_stat_weak_inlines_h +#define included_stat_weak_inlines_h +void *vlib_stats_push_heap (void *) __attribute__ ((weak)); +void * +vlib_stats_push_heap (void *unused) +{ + return 0; +}; + +void vlib_stats_pop_heap (void *, void *, u32, int) __attribute__ ((weak)); +void +vlib_stats_pop_heap (void *notused, void *notused2, u32 i, int type) +{ +}; +void vlib_stats_register_error_index (u8 *, u64 *, u64) + __attribute__ ((weak)); +void +vlib_stats_register_error_index (u8 * notused, u64 * notused2, u64 notused3) +{ +}; + +void vlib_stats_pop_heap2 (void *, u32, void *, int) __attribute__ ((weak)); +void +vlib_stats_pop_heap2 (void *notused, u32 notused2, void *notused3, + int notused4) +{ +}; + +void vlib_stat_segment_lock (void) __attribute__ ((weak)); +void +vlib_stat_segment_lock (void) +{ +} + +void vlib_stat_segment_unlock (void) __attribute__ ((weak)); +void +vlib_stat_segment_unlock (void) +{ +} + +#endif diff --git a/src/vlib/threads.c b/src/vlib/threads.c index 6a23bfd8a2f..22fa5f12ecd 100644 --- a/src/vlib/threads.c +++ b/src/vlib/threads.c @@ -23,6 +23,8 @@ #include #include +#include + DECLARE_CJ_GLOBAL_LOG; #define FRAME_QUEUE_NELTS 64 @@ -872,8 +874,13 @@ start_workers (vlib_main_t * vm) clib_mem_set_heap (oldheap); vec_add1_aligned (vlib_mains, vm_clone, CLIB_CACHE_LINE_BYTES); + /* Switch to the stats segment ... */ + void *oldheap = vlib_stats_push_heap (0); vm_clone->error_main.counters = vec_dup_aligned (vlib_mains[0]->error_main.counters, CLIB_CACHE_LINE_BYTES); + vlib_stats_pop_heap2 (vm_clone->error_main.counters, + worker_thread_index, oldheap, 1); + vm_clone->error_main.counters_last_clear = vec_dup_aligned (vlib_mains[0]->error_main.counters_last_clear, CLIB_CACHE_LINE_BYTES); @@ -1036,9 +1043,15 @@ vlib_worker_thread_node_refork (void) clib_memcpy_fast (&vm_clone->error_main, &vm->error_main, sizeof (vm->error_main)); j = vec_len (vm->error_main.counters) - 1; + + /* Switch to the stats segment ... */ + void *oldheap = vlib_stats_push_heap (0); vec_validate_aligned (old_counters, j, CLIB_CACHE_LINE_BYTES); - vec_validate_aligned (old_counters_all_clear, j, CLIB_CACHE_LINE_BYTES); vm_clone->error_main.counters = old_counters; + vlib_stats_pop_heap2 (vm_clone->error_main.counters, vm_clone->thread_index, + oldheap, 0); + + vec_validate_aligned (old_counters_all_clear, j, CLIB_CACHE_LINE_BYTES); vm_clone->error_main.counters_last_clear = old_counters_all_clear; nm_clone = &vm_clone->node_main; @@ -1466,18 +1479,6 @@ vlib_worker_thread_barrier_sync_int (vlib_main_t * vm, const char *func_name) } -void vlib_stat_segment_lock (void) __attribute__ ((weak)); -void -vlib_stat_segment_lock (void) -{ -} - -void vlib_stat_segment_unlock (void) __attribute__ ((weak)); -void -vlib_stat_segment_unlock (void) -{ -} - void vlib_worker_thread_barrier_release (vlib_main_t * vm) { diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c index 1b913823708..a91936f72de 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -392,18 +392,6 @@ done: REPLY_MACRO (VL_API_SW_INTERFACE_ADD_DEL_ADDRESS_REPLY); } -void stats_dslock_with_hint (int hint, int tag) __attribute__ ((weak)); -void -stats_dslock_with_hint (int hint, int tag) -{ -} - -void stats_dsunlock (void) __attribute__ ((weak)); -void -stats_dsunlock (void) -{ -} - static void vl_api_sw_interface_set_table_t_handler (vl_api_sw_interface_set_table_t * mp) { @@ -414,15 +402,11 @@ vl_api_sw_interface_set_table_t_handler (vl_api_sw_interface_set_table_t * mp) VALIDATE_SW_IF_INDEX (mp); - stats_dslock_with_hint (1 /* release hint */ , 4 /* tag */ ); - if (mp->is_ipv6) rv = ip_table_bind (FIB_PROTOCOL_IP6, sw_if_index, table_id, 1); else rv = ip_table_bind (FIB_PROTOCOL_IP4, sw_if_index, table_id, 1); - stats_dsunlock (); - BAD_SW_IF_INDEX_LABEL; REPLY_MACRO (VL_API_SW_INTERFACE_SET_TABLE_REPLY); diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c index 5a6053d1f42..f4e5d1776bd 100644 --- a/src/vnet/ip/ip_api.c +++ b/src/vnet/ip/ip_api.c @@ -119,9 +119,6 @@ _(IP_REASSEMBLY_ENABLE_DISABLE, ip_reassembly_enable_disable) \ _(IP_PUNT_REDIRECT_DUMP, ip_punt_redirect_dump) -extern void stats_dslock_with_hint (int hint, int tag); -extern void stats_dsunlock (void); - static vl_api_ip_neighbor_flags_t ip_neighbor_flags_encode (ip_neighbor_flags_t f) { @@ -704,8 +701,6 @@ vl_api_ip_neighbor_add_del_t_handler (vl_api_ip_neighbor_add_del_t * mp, VALIDATE_SW_IF_INDEX ((&mp->neighbor)); - stats_dslock_with_hint (1 /* release hint */ , 7 /* tag */ ); - flags = ip_neighbor_flags_decode (mp->neighbor.flags); type = ip_address_decode (&mp->neighbor.ip_address, &ip); mac_address_decode (mp->neighbor.mac_address, &mac); @@ -722,8 +717,6 @@ vl_api_ip_neighbor_add_del_t_handler (vl_api_ip_neighbor_add_del_t * mp, else rv = ip_neighbor_del (&ip, type, ntohl (mp->neighbor.sw_if_index)); - stats_dsunlock (); - BAD_SW_IF_INDEX_LABEL; /* *INDENT-OFF* */ @@ -879,8 +872,6 @@ add_del_route_t_handler (u8 is_multipath, path.frp_flags = path_flags; - stats_dslock_with_hint (1 /* release hint */ , 2 /* tag */ ); - if (is_drop || (is_local && (~0 == next_hop_sw_if_index)) || is_classify || is_unreach || is_prohibit) { @@ -910,7 +901,6 @@ add_del_route_t_handler (u8 is_multipath, if (pool_is_free_index (cm->tables, ntohl (classify_table_index))) { - stats_dsunlock (); return VNET_API_ERROR_NO_SUCH_TABLE; } @@ -920,7 +910,6 @@ add_del_route_t_handler (u8 is_multipath, } else { - stats_dsunlock (); return VNET_API_ERROR_NO_SUCH_TABLE; } @@ -964,7 +953,6 @@ add_del_route_t_handler (u8 is_multipath, } } - stats_dsunlock (); return (0); } @@ -1271,8 +1259,6 @@ mroute_add_del_handler (u8 is_add, { fib_node_index_t mfib_entry_index = ~0; - stats_dslock_with_hint (1 /* release hint */ , 2 /* tag */ ); - fib_route_path_t path = { .frp_sw_if_index = next_hop_sw_if_index, .frp_proto = nh_proto, @@ -1308,7 +1294,6 @@ mroute_add_del_handler (u8 is_add, } done: - stats_dsunlock (); return (mfib_entry_index); } @@ -2969,8 +2954,6 @@ vl_api_proxy_arp_add_del_t_handler (vl_api_proxy_arp_add_del_t * mp) u32 fib_index; int rv; - stats_dslock_with_hint (1 /* release hint */ , 6 /* tag */ ); - fib_index = fib_table_find (FIB_PROTOCOL_IP4, ntohl (mp->proxy.table_id)); if (~0 == fib_index) @@ -2985,7 +2968,6 @@ vl_api_proxy_arp_add_del_t_handler (vl_api_proxy_arp_add_del_t * mp) rv = vnet_proxy_arp_add_del (&lo, &hi, fib_index, mp->is_add == 0); out: - stats_dsunlock (); REPLY_MACRO (VL_API_PROXY_ARP_ADD_DEL_REPLY); } @@ -3166,8 +3148,6 @@ ip4_reset_fib_t_handler (vl_api_reset_fib_t * mp) int rv = VNET_API_ERROR_NO_SUCH_FIB; u32 target_fib_id = ntohl (mp->vrf_id); - stats_dslock_with_hint (1 /* release hint */ , 8 /* tag */ ); - /* *INDENT-OFF* */ pool_foreach (fib_table, im4->fibs, ({ @@ -3215,7 +3195,6 @@ ip4_reset_fib_t_handler (vl_api_reset_fib_t * mp) })); /* pool_foreach (fib) */ /* *INDENT-ON* */ - stats_dsunlock (); return rv; } @@ -3233,8 +3212,6 @@ ip6_reset_fib_t_handler (vl_api_reset_fib_t * mp) int rv = VNET_API_ERROR_NO_SUCH_FIB; u32 target_fib_id = ntohl (mp->vrf_id); - stats_dslock_with_hint (1 /* release hint */ , 9 /* tag */ ); - /* *INDENT-OFF* */ pool_foreach (fib_table, im6->fibs, ({ @@ -3273,7 +3250,6 @@ ip6_reset_fib_t_handler (vl_api_reset_fib_t * mp) })); /* pool_foreach (fib) */ /* *INDENT-ON* */ - stats_dsunlock (); return rv; } diff --git a/src/vnet/mpls/mpls_api.c b/src/vnet/mpls/mpls_api.c index 440dc9c4640..52434dae8aa 100644 --- a/src/vnet/mpls/mpls_api.c +++ b/src/vnet/mpls/mpls_api.c @@ -56,9 +56,6 @@ _(MPLS_TUNNEL_DUMP, mpls_tunnel_dump) \ _(SW_INTERFACE_SET_MPLS_ENABLE, sw_interface_set_mpls_enable) \ _(MPLS_FIB_DUMP, mpls_fib_dump) -extern void stats_dslock_with_hint (int hint, int tag); -extern void stats_dsunlock (void); - void mpls_table_delete (u32 table_id, u8 is_api) { @@ -318,8 +315,6 @@ vl_api_mpls_tunnel_add_del_t_handler (vl_api_mpls_tunnel_add_del_t * mp) clib_memset (&rpath, 0, sizeof (rpath)); - stats_dslock_with_hint (1 /* release hint */ , 5 /* tag */ ); - if (mp->mt_next_hop_proto_is_ip4) { rpath.frp_proto = DPO_PROTO_IP4; @@ -394,8 +389,6 @@ vl_api_mpls_tunnel_add_del_t_handler (vl_api_mpls_tunnel_add_del_t * mp) vec_free (rpaths); - stats_dsunlock (); - out: /* *INDENT-OFF* */ REPLY_MACRO2(VL_API_MPLS_TUNNEL_ADD_DEL_REPLY, diff --git a/src/vnet/pg/pg_api.c b/src/vnet/pg/pg_api.c index 49b661e7dc4..7775343a6f5 100644 --- a/src/vnet/pg/pg_api.c +++ b/src/vnet/pg/pg_api.c @@ -46,9 +46,6 @@ _(PG_CREATE_INTERFACE, pg_create_interface) \ _(PG_CAPTURE, pg_capture) \ _(PG_ENABLE_DISABLE, pg_enable_disable) -extern void stats_dslock_with_hint (int hint, int tag); -extern void stats_dsunlock (void); - static void vl_api_pg_create_interface_t_handler (vl_api_pg_create_interface_t * mp) { diff --git a/src/vpp-api/client/stat_client.c b/src/vpp-api/client/stat_client.c index a386eec30c3..8991806448a 100644 --- a/src/vpp-api/client/stat_client.c +++ b/src/vpp-api/client/stat_client.c @@ -231,7 +231,6 @@ copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm) int i; vlib_counter_t **combined_c; /* Combined counter */ counter_t **simple_c; /* Simple counter */ - counter_t *error_base; uint64_t *offset_vector; assert (sm->shared_header); @@ -275,10 +274,16 @@ copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm) break; case STAT_DIR_TYPE_ERROR_INDEX: - error_base = - stat_segment_pointer (sm->shared_header, - sm->shared_header->error_offset); - result.error_value = error_base[ep->index]; + /* Gather errors from all threads into a vector */ + offset_vector = stat_segment_pointer (sm->shared_header, + sm->shared_header->error_offset); + vec_validate (result.error_vector, vec_len (offset_vector) - 1); + for (i = 0; i < vec_len (offset_vector); i++) + { + counter_t *cb = + stat_segment_pointer (sm->shared_header, offset_vector[i]); + result.error_vector[i] = cb[ep->index]; + } break; case STAT_DIR_TYPE_NAME_VECTOR: diff --git a/src/vpp-api/client/stat_client.h b/src/vpp-api/client/stat_client.h index 1c76a938b30..901ec325522 100644 --- a/src/vpp-api/client/stat_client.h +++ b/src/vpp-api/client/stat_client.h @@ -18,7 +18,7 @@ #define included_stat_client_h #define STAT_VERSION_MAJOR 1 -#define STAT_VERSION_MINOR 1 +#define STAT_VERSION_MINOR 2 #include #include @@ -46,7 +46,7 @@ typedef struct union { double scalar_value; - uint64_t error_value; + counter_t *error_vector; counter_t **simple_counter_vec; vlib_counter_t **combined_counter_vec; uint8_t **name_vector; diff --git a/src/vpp-api/python/vpp_papi/vpp_stats.py b/src/vpp-api/python/vpp_papi/vpp_stats.py index c90aa349434..86a80ddc328 100644 --- a/src/vpp-api/python/vpp_papi/vpp_stats.py +++ b/src/vpp-api/python/vpp_papi/vpp_stats.py @@ -40,7 +40,7 @@ typedef struct union { double scalar_value; - uint64_t error_value; + counter_t *error_vector; counter_t **simple_counter_vec; vlib_counter_t **combined_counter_vec; uint8_t **name_vector; @@ -126,6 +126,12 @@ def combined_counter_vec_list(api, e): vec.append(if_per_thread) return vec +def error_vec_list(api, e): + vec = [] + for thread in range(api.stat_segment_vec_len(e)): + vec.append(e[thread]) + return vec + def name_vec_list(api, e): return [ffi.string(e[i]).decode('utf-8') for i in range(api.stat_segment_vec_len(e)) if e[i] != ffi.NULL] @@ -138,7 +144,7 @@ def stat_entry_to_python(api, e): if e.type == 3: return combined_counter_vec_list(api, e.combined_counter_vec) if e.type == 4: - return e.error_value + return error_vec_list(api, e.error_vector) if e.type == 5: return name_vec_list(api, e.name_vector) raise NotImplementedError() @@ -248,6 +254,11 @@ class VPPStats(object): return None retries += 1 + def get_err_counter(self, name): + """Get an error counter. The errors from each worker thread + are summed""" + return sum(self.get_counter(name)) + def disconnect(self): self.api.stat_segment_disconnect_r(self.client) self.api.stat_client_free(self.client) @@ -265,8 +276,8 @@ class VPPStats(object): return None retries += 1 - return {k: error_counters[k] - for k in error_counters.keys() if error_counters[k]} + return {k: sum(error_counters[k]) + for k in error_counters.keys() if sum(error_counters[k])} def set_errors_str(self): '''Return all errors counters > 0 pretty printed''' diff --git a/src/vpp/app/vpp_get_stats.c b/src/vpp/app/vpp_get_stats.c index 1d878029dda..c00fb835d1c 100644 --- a/src/vpp/app/vpp_get_stats.c +++ b/src/vpp/app/vpp_get_stats.c @@ -80,7 +80,9 @@ stat_poll_loop (u8 ** patterns) break; case STAT_DIR_TYPE_ERROR_INDEX: - fformat (stdout, "%llu %s\n", res[i].error_value, res[i].name); + for (j = 0; j < vec_len (res[i].error_vector); j++) + fformat (stdout, "%llu %s\n", res[i].error_vector[j], + res[i].name); break; case STAT_DIR_TYPE_SCALAR_INDEX: @@ -213,7 +215,9 @@ reconnect: break; case STAT_DIR_TYPE_ERROR_INDEX: - fformat (stdout, "%llu %s\n", res[i].error_value, res[i].name); + for (j = 0; j < vec_len (res[i].error_vector); j++) + fformat (stdout, "[@%d] %llu %s\n", j, res[i].error_vector[j], + res[i].name); break; case STAT_DIR_TYPE_SCALAR_INDEX: diff --git a/src/vpp/app/vpp_prometheus_export.c b/src/vpp/app/vpp_prometheus_export.c index e2fdd7150e5..06f1a9169cd 100644 --- a/src/vpp/app/vpp_prometheus_export.c +++ b/src/vpp/app/vpp_prometheus_export.c @@ -97,9 +97,13 @@ retry: } break; case STAT_DIR_TYPE_ERROR_INDEX: - fformat (stream, "# TYPE %s counter\n", prom_string (res[i].name)); - fformat (stream, "%s{thread=\"0\"} %lld\n", - prom_string (res[i].name), res[i].error_value); + for (j = 0; j < vec_len (res[i].error_vector); j++) + { + fformat (stream, "# TYPE %s counter\n", + prom_string (res[i].name)); + fformat (stream, "%s{thread=\"%d\"} %lld\n", + prom_string (res[i].name), j, res[i].error_vector[j]); + } break; case STAT_DIR_TYPE_SCALAR_INDEX: diff --git a/src/vpp/stats/stat_segment.c b/src/vpp/stats/stat_segment.c index bb2ffad8f6d..4cd00a22641 100644 --- a/src/vpp/stats/stat_segment.c +++ b/src/vpp/stats/stat_segment.c @@ -22,6 +22,7 @@ #undef HAVE_MEMFD_CREATE #include #include + stat_segment_main_t stat_segment_main; /* @@ -200,22 +201,29 @@ stat_validate_counter_vector (stat_segment_directory_entry_t * ep, u32 max) } void -vlib_stats_pop_heap2 (u64 * error_vector, u32 thread_index, void *oldheap) +vlib_stats_pop_heap2 (u64 * error_vector, u32 thread_index, void *oldheap, + int lock) { stat_segment_main_t *sm = &stat_segment_main; stat_segment_shared_header_t *shared_header = sm->shared_header; ASSERT (shared_header); - vlib_stat_segment_lock (); + if (lock) + vlib_stat_segment_lock (); /* Reset the client hash table pointer, since it WILL change! */ - shared_header->error_offset = + vec_validate (sm->error_vector, thread_index); + sm->error_vector[thread_index] = stat_segment_offset (shared_header, error_vector); + + shared_header->error_offset = + stat_segment_offset (shared_header, sm->error_vector); shared_header->directory_offset = stat_segment_offset (shared_header, sm->directory_vector); - vlib_stat_segment_unlock (); + if (lock) + vlib_stat_segment_unlock (); clib_mem_set_heap (oldheap); } diff --git a/src/vpp/stats/stat_segment.h b/src/vpp/stats/stat_segment.h index fd7ce79cabe..b9ffedf2169 100644 --- a/src/vpp/stats/stat_segment.h +++ b/src/vpp/stats/stat_segment.h @@ -110,6 +110,7 @@ typedef struct /* statistics segment */ uword *directory_vector_by_name; stat_segment_directory_entry_t *directory_vector; + u64 *error_vector; u8 **interfaces; u8 **nodes; diff --git a/src/vpp/stats/stats.md b/src/vpp/stats/stats.md index 6a62ca6c8ad..20ca7909baa 100644 --- a/src/vpp/stats/stats.md +++ b/src/vpp/stats/stats.md @@ -106,7 +106,8 @@ int main (int argc, char **argv) { break; case STAT_DIR_TYPE_ERROR_INDEX: - fformat (stdout, "%llu %s\n", res[i].error_value, res[i].name); + for (j = 0; j < vec_len (res[i].error_vector); j++) + fformat (stdout, "[@%d] %llu %s\n", j, res[i].error_vector[j], res[i].name); break; case STAT_DIR_TYPE_SCALAR_INDEX: diff --git a/test/framework.py b/test/framework.py index 201892aea27..85bd6616c8b 100644 --- a/test/framework.py +++ b/test/framework.py @@ -1027,6 +1027,11 @@ class VppTestCase(unittest.TestCase): counter_value = int(results[0]) break + def assert_error_counter_equal(self, counter, expected_value): + counter_value = self.statistics.get_err_counter(counter) + self.assert_equal(counter_value, expected_value, + "error counter `%s'" % counter) + @classmethod def sleep(cls, timeout, remark=None): diff --git a/test/template_ipsec.py b/test/template_ipsec.py index 61ac0b5cd22..84b878440e9 100644 --- a/test/template_ipsec.py +++ b/test/template_ipsec.py @@ -293,7 +293,7 @@ class IpsecTra4(object): # replayed packets are dropped self.send_and_assert_no_replies(self.tra_if, pkt * 3) - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/SA replayed packet' % self.tra4_decrypt_node_name, 3) # the window size is 64 packets @@ -321,7 +321,7 @@ class IpsecTra4(object): seq_num=350)) self.send_and_assert_no_replies(self.tra_if, pkt * 17) - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/Integrity check failed' % self.tra4_decrypt_node_name, 17) # a malformed 'runt' packet @@ -337,7 +337,7 @@ class IpsecTra4(object): seq_num=350)) self.send_and_assert_no_replies(self.tra_if, pkt * 17) - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/undersized packet' % self.tra4_decrypt_node_name, 17) # which we can determine since this packet is still in the window @@ -361,12 +361,12 @@ class IpsecTra4(object): if use_esn: # an out of window error with ESN looks like a high sequence # wrap. but since it isn't then the verify will fail. - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/Integrity check failed' % self.tra4_decrypt_node_name, 34) else: - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/SA replayed packet' % self.tra4_decrypt_node_name, 20) @@ -411,7 +411,7 @@ class IpsecTra4(object): decrypted = p.vpp_tra_sa.decrypt(rx[0][IP]) else: self.send_and_assert_no_replies(self.tra_if, [pkt]) - self.assert_packet_counter_equal( + self.assert_error_counter_equal( '/err/%s/sequence number cycled' % self.tra4_encrypt_node_name, 1) diff --git a/test/test_ipip.py b/test/test_ipip.py index e5b9092a431..137e3c54dff 100644 --- a/test/test_ipip.py +++ b/test/test_ipip.py @@ -135,7 +135,7 @@ class TestIPIP(VppTestCase): for p in rx: self.validate(p[1], p4_reply) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/ipip4-input/packets decapsulated') self.assertEqual(err, 10) @@ -149,7 +149,7 @@ class TestIPIP(VppTestCase): for p in rx: self.validate(p[1], p6_reply) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/ipip4-input/packets decapsulated') self.assertEqual(err, 20) @@ -178,7 +178,7 @@ class TestIPIP(VppTestCase): for p in rx: self.validate(p[1], p4_reply) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/ipip4-input/packets decapsulated') self.assertEqual(err, 1020) @@ -426,7 +426,7 @@ class TestIPIP6(VppTestCase): is_ip6=1) # Send lots of fragments, verify reassembled packet - before_cnt = self.statistics.get_counter( + before_cnt = self.statistics.get_err_counter( '/err/ipip6-input/packets decapsulated') frags, p6_reply = self.generate_ip6_frags(3131, 1400) f = [] @@ -440,7 +440,7 @@ class TestIPIP6(VppTestCase): for p in rx: self.validate(p[1], p6_reply) - cnt = self.statistics.get_counter( + cnt = self.statistics.get_err_counter( '/err/ipip6-input/packets decapsulated') self.assertEqual(cnt, before_cnt + 1000) diff --git a/test/test_nat.py b/test/test_nat.py index 848d7225296..f7364747efd 100644 --- a/test/test_nat.py +++ b/test/test_nat.py @@ -1577,13 +1577,13 @@ class TestNAT44(MethodHolder): is_add=1) # in2out - tcpn = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/TCP packets') - udpn = self.statistics.get_counter( + udpn = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/UDP packets') - icmpn = self.statistics.get_counter( + icmpn = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/ICMP packets') - totaln = self.statistics.get_counter( + totaln = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/good in2out packets processed') pkts = self.create_stream_in(self.pg0, self.pg1) @@ -1593,24 +1593,25 @@ class TestNAT44(MethodHolder): capture = self.pg1.get_capture(len(pkts)) self.verify_capture_out(capture) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-in2out-slowpath/good in2out packets processed') self.assertEqual(err - totaln, 3) # out2in - tcpn = self.statistics.get_counter('/err/nat44-out2in/TCP packets') - udpn = self.statistics.get_counter('/err/nat44-out2in/UDP packets') - icmpn = self.statistics.get_counter('/err/nat44-out2in/ICMP packets') - totaln = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter('/err/nat44-out2in/TCP packets') + udpn = self.statistics.get_err_counter('/err/nat44-out2in/UDP packets') + icmpn = self.statistics.get_err_counter( + '/err/nat44-out2in/ICMP packets') + totaln = self.statistics.get_err_counter( '/err/nat44-out2in/good out2in packets processed') pkts = self.create_stream_out(self.pg1) @@ -1620,13 +1621,13 @@ class TestNAT44(MethodHolder): capture = self.pg0.get_capture(len(pkts)) self.verify_capture_in(capture, self.pg0) - err = self.statistics.get_counter('/err/nat44-out2in/TCP packets') + err = self.statistics.get_err_counter('/err/nat44-out2in/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter('/err/nat44-out2in/UDP packets') + err = self.statistics.get_err_counter('/err/nat44-out2in/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter('/err/nat44-out2in/ICMP packets') + err = self.statistics.get_err_counter('/err/nat44-out2in/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-out2in/good out2in packets processed') self.assertEqual(err - totaln, 3) @@ -3655,9 +3656,11 @@ class TestNAT44(MethodHolder): self.logger.error(ppp("Unexpected or invalid packet:", p)) raise - err = self.statistics.get_counter('/err/nat44-classify/next in2out') + err = self.statistics.get_err_counter( + '/err/nat44-classify/next in2out') self.assertEqual(err, 1) - err = self.statistics.get_counter('/err/nat44-classify/next out2in') + err = self.statistics.get_err_counter( + '/err/nat44-classify/next out2in') self.assertEqual(err, 1) def test_del_session(self): @@ -4349,7 +4352,7 @@ class TestNAT44(MethodHolder): stats = self.statistics.get_counter('/nat44/ha/del-event-recv') self.assertEqual(stats[0][0], 1) - stats = self.statistics.get_counter('/err/nat-ha/pkts-processed') + stats = self.statistics.get_err_counter('/err/nat-ha/pkts-processed') self.assertEqual(stats, 2) # send HA session refresh event to failover/passive @@ -4393,7 +4396,7 @@ class TestNAT44(MethodHolder): stats = self.statistics.get_counter('/nat44/ha/refresh-event-recv') self.assertEqual(stats[0][0], 1) - stats = self.statistics.get_counter('/err/nat-ha/pkts-processed') + stats = self.statistics.get_err_counter('/err/nat-ha/pkts-processed') self.assertEqual(stats, 3) # send packet to test session created by HA @@ -4734,13 +4737,13 @@ class TestNAT44EndpointDependent(MethodHolder): self.assertEqual(1, nat_config.endpoint_dependent) # in2out - tcpn = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/TCP packets') - udpn = self.statistics.get_counter( + udpn = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/UDP packets') - icmpn = self.statistics.get_counter( + icmpn = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/ICMP packets') - totaln = self.statistics.get_counter( + totaln = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/good in2out packets processed') pkts = self.create_stream_in(self.pg0, self.pg1) @@ -4750,25 +4753,27 @@ class TestNAT44EndpointDependent(MethodHolder): capture = self.pg1.get_capture(len(pkts)) self.verify_capture_out(capture) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-in2out-slowpath/good in2out packets processed') self.assertEqual(err - totaln, 3) # out2in - tcpn = self.statistics.get_counter('/err/nat44-ed-out2in/TCP packets') - udpn = self.statistics.get_counter('/err/nat44-ed-out2in/UDP packets') - icmpn = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter( + '/err/nat44-ed-out2in/TCP packets') + udpn = self.statistics.get_err_counter( + '/err/nat44-ed-out2in/UDP packets') + icmpn = self.statistics.get_err_counter( '/err/nat44-ed-out2in-slowpath/ICMP packets') - totaln = self.statistics.get_counter( + totaln = self.statistics.get_err_counter( '/err/nat44-ed-out2in/good out2in packets processed') pkts = self.create_stream_out(self.pg1) @@ -4778,14 +4783,16 @@ class TestNAT44EndpointDependent(MethodHolder): capture = self.pg0.get_capture(len(pkts)) self.verify_capture_in(capture, self.pg0) - err = self.statistics.get_counter('/err/nat44-ed-out2in/TCP packets') + err = self.statistics.get_err_counter( + '/err/nat44-ed-out2in/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter('/err/nat44-ed-out2in/UDP packets') + err = self.statistics.get_err_counter( + '/err/nat44-ed-out2in/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-out2in-slowpath/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat44-ed-out2in/good out2in packets processed') self.assertEqual(err - totaln, 2) @@ -7904,10 +7911,11 @@ class TestNAT64(MethodHolder): sw_if_index=self.pg1.sw_if_index) # in2out - tcpn = self.statistics.get_counter('/err/nat64-in2out/TCP packets') - udpn = self.statistics.get_counter('/err/nat64-in2out/UDP packets') - icmpn = self.statistics.get_counter('/err/nat64-in2out/ICMP packets') - totaln = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter('/err/nat64-in2out/TCP packets') + udpn = self.statistics.get_err_counter('/err/nat64-in2out/UDP packets') + icmpn = self.statistics.get_err_counter( + '/err/nat64-in2out/ICMP packets') + totaln = self.statistics.get_err_counter( '/err/nat64-in2out/good in2out packets processed') pkts = self.create_stream_in_ip6(self.pg0, self.pg1) @@ -7918,21 +7926,22 @@ class TestNAT64(MethodHolder): self.verify_capture_out(capture, nat_ip=self.nat_addr, dst_ip=self.pg1.remote_ip4) - err = self.statistics.get_counter('/err/nat64-in2out/TCP packets') + err = self.statistics.get_err_counter('/err/nat64-in2out/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter('/err/nat64-in2out/UDP packets') + err = self.statistics.get_err_counter('/err/nat64-in2out/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter('/err/nat64-in2out/ICMP packets') + err = self.statistics.get_err_counter('/err/nat64-in2out/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat64-in2out/good in2out packets processed') self.assertEqual(err - totaln, 3) # out2in - tcpn = self.statistics.get_counter('/err/nat64-out2in/TCP packets') - udpn = self.statistics.get_counter('/err/nat64-out2in/UDP packets') - icmpn = self.statistics.get_counter('/err/nat64-out2in/ICMP packets') - totaln = self.statistics.get_counter( + tcpn = self.statistics.get_err_counter('/err/nat64-out2in/TCP packets') + udpn = self.statistics.get_err_counter('/err/nat64-out2in/UDP packets') + icmpn = self.statistics.get_err_counter( + '/err/nat64-out2in/ICMP packets') + totaln = self.statistics.get_err_counter( '/err/nat64-out2in/good out2in packets processed') pkts = self.create_stream_out(self.pg1, dst_ip=self.nat_addr) @@ -7943,13 +7952,13 @@ class TestNAT64(MethodHolder): ip = IPv6(src=''.join(['64:ff9b::', self.pg1.remote_ip4])) self.verify_capture_in_ip6(capture, ip[IPv6].src, self.pg0.remote_ip6) - err = self.statistics.get_counter('/err/nat64-out2in/TCP packets') + err = self.statistics.get_err_counter('/err/nat64-out2in/TCP packets') self.assertEqual(err - tcpn, 1) - err = self.statistics.get_counter('/err/nat64-out2in/UDP packets') + err = self.statistics.get_err_counter('/err/nat64-out2in/UDP packets') self.assertEqual(err - udpn, 1) - err = self.statistics.get_counter('/err/nat64-out2in/ICMP packets') + err = self.statistics.get_err_counter('/err/nat64-out2in/ICMP packets') self.assertEqual(err - icmpn, 1) - err = self.statistics.get_counter( + err = self.statistics.get_err_counter( '/err/nat64-out2in/good out2in packets processed') self.assertEqual(err - totaln, 3) diff --git a/test/test_punt.py b/test/test_punt.py index f68a38f7515..77847fe21f6 100644 --- a/test/test_punt.py +++ b/test/test_punt.py @@ -757,7 +757,7 @@ class TestPunt(VppTestCase): # 2 - per-reason counters # 2, 3 are the index of the assigned punt reason # - stats = self.statistics.get_counter( + stats = self.statistics.get_err_counter( "/err/punt-dispatch/No registrations") self.assertEqual(stats, 2*NUM_PKTS) diff --git a/test/test_stats_client.py b/test/test_stats_client.py index 87c9efd4f1c..a0504fc45ab 100644 --- a/test/test_stats_client.py +++ b/test/test_stats_client.py @@ -19,6 +19,12 @@ class StatsClientTestCase(VppTestCase): def tearDownClass(cls): super(StatsClientTestCase, cls).tearDownClass() + def test_set_errors(self): + """Test set errors""" + self.assertEqual(self.statistics.set_errors(), {}) + self.assertEqual(self.statistics.get_counter('/err/ethernet-input/no'), + [0]) + def test_client_fd_leak(self): """Test file descriptor count - VPP-1486""" -- 2.16.6