From ca2cbc974615b4b0406ecbcada1c27266b3aabc7 Mon Sep 17 00:00:00 2001 From: Andrew Yourtchenko Date: Wed, 29 Mar 2017 16:47:10 +0200 Subject: [PATCH] acl-plugin: cleaner node bugfixes (VPP-675) - use the counters in a private struct rather than node error counters - ensure the timer for the non-idle connections is restarted - fix the deletion of conn at the current tail the list Change-Id: I632f63574d2ced95fb75c5e7fb588c78fb3cce1c Signed-off-by: Andrew Yourtchenko (cherry picked from commit 097051a3bd1f63a177c0728f15375afd84a68918) --- src/plugins/acl/acl.c | 13 +++++++ src/plugins/acl/acl.h | 16 ++++++++ src/plugins/acl/fa_node.c | 97 ++++++++++++++++++++++------------------------- 3 files changed, 75 insertions(+), 51 deletions(-) diff --git a/src/plugins/acl/acl.c b/src/plugins/acl/acl.c index 4a4dd43423e..5d0b6c25229 100644 --- a/src/plugins/acl/acl.c +++ b/src/plugins/acl/acl.c @@ -2120,10 +2120,15 @@ acl_show_aclplugin_fn (vlib_main_t * vm, u64 n_dels = sw_if_index < vec_len(am->fa_session_dels_by_sw_if_index) ? am->fa_session_dels_by_sw_if_index[sw_if_index] : 0; out0 = format(out0, "sw_if_index %d: add %lu - del %lu = %lu\n", sw_if_index, n_adds, n_dels, n_adds - n_dels); })); + out0 = format(out0, "\n\nConn cleaner thread counters:\n"); +#define _(cnt, desc) out0 = format(out0, " %20lu: %s\n", am->cnt, desc); + foreach_fa_cleaner_counter; +#undef _ vlib_cli_output(vm, "\n\n%s\n\n", out0); vlib_cli_output(vm, "Sessions per interval: min %lu max %lu increment: %f ms current: %f ms", am->fa_min_deleted_sessions_per_interval, am->fa_max_deleted_sessions_per_interval, am->fa_cleaner_wait_time_increment * 1000.0, ((f64)am->fa_current_cleaner_timer_wait_interval) * 1000.0/(f64)vm->clib_time.clocks_per_second); + vec_free(out0); } return error; @@ -2190,6 +2195,14 @@ acl_init (vlib_main_t * vm) am->fa_max_deleted_sessions_per_interval = ACL_FA_DEFAULT_MAX_DELETED_SESSIONS_PER_INTERVAL; am->fa_cleaner_wait_time_increment = ACL_FA_DEFAULT_CLEANER_WAIT_TIME_INCREMENT; + am->fa_cleaner_cnt_delete_by_sw_index = 0; + am->fa_cleaner_cnt_delete_by_sw_index_ok = 0; + am->fa_cleaner_cnt_unknown_event = 0; + am->fa_cleaner_cnt_deleted_sessions = 0; + am->fa_cleaner_cnt_timer_restarted = 0; + am->fa_cleaner_cnt_wait_with_timeout = 0; + + #define _(N, v, s) am->fa_ipv6_known_eh_bitmap = clib_bitmap_set(am->fa_ipv6_known_eh_bitmap, v, 1); foreach_acl_eh #undef _ diff --git a/src/plugins/acl/acl.h b/src/plugins/acl/acl.h index 47523636d48..f5a1fe0f934 100644 --- a/src/plugins/acl/acl.h +++ b/src/plugins/acl/acl.h @@ -209,6 +209,22 @@ typedef struct { u32 fa_conn_list_head[ACL_N_TIMEOUTS]; u32 fa_conn_list_tail[ACL_N_TIMEOUTS]; + /* Counters for the cleaner thread */ + +#define foreach_fa_cleaner_counter \ + _(fa_cleaner_cnt_delete_by_sw_index, "delete_by_sw_index events") \ + _(fa_cleaner_cnt_delete_by_sw_index_ok, "delete_by_sw_index handled ok") \ + _(fa_cleaner_cnt_unknown_event, "unknown events received") \ + _(fa_cleaner_cnt_deleted_sessions, "sessions deleted") \ + _(fa_cleaner_cnt_timer_restarted, "session idle timers restarted") \ + _(fa_cleaner_cnt_wait_with_timeout, "event wait with timeout called") \ + _(fa_cleaner_cnt_wait_without_timeout, "event wait w/o timeout called") \ + _(fa_cleaner_cnt_event_cycles, "total event cycles") \ + _(fa_cleaner_cnt_already_deleted, "try to delete already deleted conn") \ +/* end of counters */ +#define _(id, desc) u32 id; + foreach_fa_cleaner_counter +#undef _ /* convenience */ vlib_main_t * vlib_main; diff --git a/src/plugins/acl/fa_node.c b/src/plugins/acl/fa_node.c index ac619a7298b..1f9117a6321 100644 --- a/src/plugins/acl/fa_node.c +++ b/src/plugins/acl/fa_node.c @@ -533,6 +533,10 @@ acl_fa_conn_list_add_session (acl_main_t * am, u32 sess_id) if (~0 == am->fa_conn_list_head[list_id]) { am->fa_conn_list_head[list_id] = sess_id; + /* If it is a first conn in any list, kick off the cleaner */ + vlib_process_signal_event (am->vlib_main, am->fa_cleaner_node_index, + ACL_FA_CLEANER_RESCHEDULE, 0); + } } @@ -556,7 +560,7 @@ acl_fa_conn_list_delete_session (acl_main_t *am, u32 sess_id) am->fa_conn_list_head[sess->link_list_id] = sess->link_next_idx; } if (am->fa_conn_list_tail[sess->link_list_id] == sess_id) { - am->fa_conn_list_tail[sess->link_list_id] = sess->link_next_idx; + am->fa_conn_list_tail[sess->link_list_id] = sess->link_prev_idx; } } @@ -982,14 +986,6 @@ acl_out_ip4_fa_node_fn (vlib_main_t * vm, /* *INDENT-OFF* */ #define foreach_acl_fa_cleaner_error \ -_(EVENT_CYCLE, "event processing cycle") \ -_(TIMER_RESTARTED, "restarted session timers") \ -_(DELETED_SESSIONS, "deleted sessions") \ -_(ALREADY_DELETED, "timer event for already deleted session") \ -_(DELETE_BY_SW_IF_INDEX, "delete by sw_if_index event") \ -_(DELETE_BY_SW_IF_INDEX_OK, "delete by sw_if_index completed ok") \ -_(WAIT_WITHOUT_TIMEOUT, "process waits without timeout") \ -_(WAIT_WITH_TIMEOUT, "process waits with timeout") \ _(UNKNOWN_EVENT, "unknown event received") \ /* end of errors */ @@ -1067,7 +1063,7 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, f64 cpu_cps = vm->clib_time.clocks_per_second; u64 next_expire; /* We should call timer wheel at least twice a second */ - u64 max_timer_wait_interval = cpu_cps / 2; + u64 max_timer_wait_interval = cpu_cps / 2; am->fa_current_cleaner_timer_wait_interval = max_timer_wait_interval; u32 *expired = NULL; @@ -1079,10 +1075,24 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, { u32 count_deleted_sessions = 0; u32 count_already_deleted = 0; - u32 count_timer_restarted = 0; now = clib_cpu_time_now (); next_expire = now + am->fa_current_cleaner_timer_wait_interval; + int has_pending_conns = 0; + u8 tt; + for(tt = 0; tt < ACL_N_TIMEOUTS; tt++) + { + if (~0 != am->fa_conn_list_head[tt]) + has_pending_conns = 1; + } + /* If no pending connections then no point in timing out */ + if (!has_pending_conns) + { + am->fa_cleaner_cnt_wait_without_timeout++; + (void) vlib_process_wait_for_event (vm); + event_type = vlib_process_get_events (vm, &event_data); + } + else { f64 timeout = ((i64) next_expire - (i64) now) / cpu_cps; if (timeout <= 0) @@ -1095,11 +1105,7 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, /* Timing wheel code is happier if it is called regularly */ if (timeout > 0.5) timeout = 0.5; - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node. - index, - ACL_FA_CLEANER_ERROR_WAIT_WITH_TIMEOUT, - 1); + am->fa_cleaner_cnt_wait_with_timeout++; (void) vlib_process_wait_for_event_or_clock (vm, timeout); event_type = vlib_process_get_events (vm, &event_data); } @@ -1119,11 +1125,7 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, uword *sw_if_index0; vec_foreach (sw_if_index0, event_data) { - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node. - index, - ACL_FA_CLEANER_ERROR_DELETE_BY_SW_IF_INDEX, - 1); + am->fa_cleaner_cnt_delete_by_sw_index++; #ifdef FA_NODE_VERBOSE_DEBUG clib_warning ("ACL_FA_NODE_CLEAN: ACL_FA_CLEANER_DELETE_BY_SW_IF_INDEX: %d", @@ -1134,11 +1136,7 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, acl_fa_clean_sessions_by_sw_if_index (am, *sw_if_index0, &count); count_deleted_sessions += count; - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node. - index, - ACL_FA_CLEANER_ERROR_DELETE_BY_SW_IF_INDEX_OK, - result); + am->fa_cleaner_cnt_delete_by_sw_index_ok += result; } } break; @@ -1147,17 +1145,21 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, clib_warning ("ACL plugin connection cleaner: unknown event %u", event_type); #endif - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node. - index, - ACL_FA_CLEANER_ERROR_UNKNOWN_EVENT, 1); + vlib_node_increment_counter (vm, + acl_fa_session_cleaner_process_node. + index, + ACL_FA_CLEANER_ERROR_UNKNOWN_EVENT, 1); + am->fa_cleaner_cnt_unknown_event++; break; } { u8 tt = 0; for(tt = 0; tt < ACL_N_TIMEOUTS; tt++) { - while((vec_len(expired) < 2*am->fa_max_deleted_sessions_per_interval) && (~0 != am->fa_conn_list_head[tt]) && (acl_fa_conn_has_timed_out(am, now, am->fa_conn_list_head[tt]))) { + while((vec_len(expired) < 2*am->fa_max_deleted_sessions_per_interval) + && (~0 != am->fa_conn_list_head[tt]) + && (acl_fa_conn_has_timed_out(am, now, + am->fa_conn_list_head[tt]))) { u32 sess_id = am->fa_conn_list_head[tt]; vec_add1(expired, sess_id); acl_fa_conn_list_delete_session(am, sess_id); @@ -1165,7 +1167,6 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, } } - u32 *psid = NULL; vec_foreach (psid, expired) { @@ -1181,15 +1182,22 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, /* clib_warning ("ACL_FA_NODE_CLEAN: Restarting timer for session %d", (int) session_index); */ - /* Pretend we did this in the past, at last_active moment */ - count_timer_restarted++; + /* There was activity on the session, so the idle timeout + has not passed. Enqueue for another time period. */ + + acl_fa_conn_list_add_session(am, session_index); + + /* FIXME: When/if moving to timer wheel, + pretend we did this in the past, + at last_active moment, so the timer is accurate */ + am->fa_cleaner_cnt_timer_restarted++; } else { /* clib_warning ("ACL_FA_NODE_CLEAN: Deleting session %d", (int) session_index); */ acl_fa_delete_session (am, sw_if_index, session_index); - count_deleted_sessions++; + count_deleted_sessions++; } } else @@ -1210,22 +1218,9 @@ acl_fa_session_cleaner_process (vlib_main_t * vm, vlib_node_runtime_t * rt, if (am->fa_current_cleaner_timer_wait_interval < max_timer_wait_interval) am->fa_current_cleaner_timer_wait_interval += cpu_cps * am->fa_cleaner_wait_time_increment; } - - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node.index, - ACL_FA_CLEANER_ERROR_EVENT_CYCLE, 1); - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node.index, - ACL_FA_CLEANER_ERROR_TIMER_RESTARTED, - count_timer_restarted); - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node.index, - ACL_FA_CLEANER_ERROR_DELETED_SESSIONS, - count_deleted_sessions); - vlib_node_increment_counter (vm, - acl_fa_session_cleaner_process_node.index, - ACL_FA_CLEANER_ERROR_ALREADY_DELETED, - count_already_deleted); + am->fa_cleaner_cnt_event_cycles++; + am->fa_cleaner_cnt_deleted_sessions += count_deleted_sessions; + am->fa_cleaner_cnt_already_deleted += count_already_deleted; } /* NOT REACHED */ return 0; -- 2.16.6