From 56cab1850170603a138267a114a87379cfe5f88f Mon Sep 17 00:00:00 2001 From: Tamer Ahmed Date: Tue, 15 Sep 2020 15:27:36 -0700 Subject: [PATCH] [dhcpmon] Print Both Snapshot And Current Counters (#5374) Printing both snapshot and current counter sets will make it easier to pinpoint which message type(s) is/are not being relayed. This PR prints both counter sets. Also, this PR defines gnu11 as a C standard to compile with in order to avoid making changes when porting to 201811 branch. singed-off-by: Tamer Ahmed --- .../docker-dhcp-relay.supervisord.conf.j2 | 2 +- src/dhcpmon/debian/rules | 6 ++- src/dhcpmon/src/dhcp_device.c | 50 +++++++++++-------- src/dhcpmon/src/dhcp_device.h | 7 +-- src/dhcpmon/src/dhcp_devman.c | 22 +++++--- src/dhcpmon/src/dhcp_devman.h | 5 +- src/dhcpmon/src/dhcp_mon.c | 9 ++-- 7 files changed, 63 insertions(+), 38 deletions(-) diff --git a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 index 0ca31669b8..b883b69426 100644 --- a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 +++ b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 @@ -138,7 +138,7 @@ command=/usr/sbin/dhcpmon -id {{ vlan_name }} {% endfor %} {% if MGMT_INTERFACE %} {% for (name, prefix) in MGMT_INTERFACE|pfx_filter %} -{% if prefix | ipv4 %} -im {{ name }}{% endif %} +{% if prefix | ipv4 %} -im {{ name }}{% endif -%} {% endfor %} {% endif %} diff --git a/src/dhcpmon/debian/rules b/src/dhcpmon/debian/rules index 3995a26d7f..00c628b662 100755 --- a/src/dhcpmon/debian/rules +++ b/src/dhcpmon/debian/rules @@ -1,3 +1,7 @@ #!/usr/bin/make -f + +DEB_CFLAGS_APPEND=-std=gnu11 +export DEB_CFLAGS_APPEND + %: - dh $@ --with systemd + dh $@ --parallel diff --git a/src/dhcpmon/src/dhcp_device.c b/src/dhcpmon/src/dhcp_device.c index 84ec9be9b6..f5cb705ee2 100644 --- a/src/dhcpmon/src/dhcp_device.c +++ b/src/dhcpmon/src/dhcp_device.c @@ -25,7 +25,7 @@ #include "dhcp_device.h" /** Counter print width */ -#define DHCP_COUNTER_WIDTH 10 +#define DHCP_COUNTER_WIDTH 9 /** Start of Ether header of a captured frame */ #define ETHER_START_OFFSET 0 @@ -353,29 +353,39 @@ static dhcp_mon_status_t dhcp_device_check_health(dhcp_mon_check_t check_type, } /** - * @code dhcp_print_counters(vlan_intf, counters); + * @code dhcp_print_counters(vlan_intf, type, counters); * * @brief prints DHCP counters to sylsog. * + * @param vlan_intf vlan interface name + * @param type counter type * @param counters interface counter + * + * @return none */ -static void dhcp_print_counters(const char *vlan_intf, uint64_t counters[][DHCP_DIR_COUNT][DHCP_MESSAGE_TYPE_COUNT]) +static void dhcp_print_counters(const char *vlan_intf, + dhcp_counters_type_t type, + uint64_t counters[][DHCP_MESSAGE_TYPE_COUNT]) { - uint64_t *rx_counters = counters[DHCP_COUNTERS_CURRENT][DHCP_RX]; - uint64_t *tx_counters = counters[DHCP_COUNTERS_CURRENT][DHCP_TX]; + static const char *counter_desc[DHCP_COUNTERS_COUNT] = { + [DHCP_COUNTERS_CURRENT] = " Current", + [DHCP_COUNTERS_SNAPSHOT] = "Snapshot" + }; - syslog(LOG_NOTICE, - "[%*s] DHCP Discover rx/tx: %*lu/%*lu, Offer rx/tx: %*lu/%*lu, " - "Request rx/tx: %*lu/%*lu, ACK rx/tx: %*lu/%*lu\n", - IF_NAMESIZE, vlan_intf, - DHCP_COUNTER_WIDTH, rx_counters[DHCP_MESSAGE_TYPE_DISCOVER], - DHCP_COUNTER_WIDTH, tx_counters[DHCP_MESSAGE_TYPE_DISCOVER], - DHCP_COUNTER_WIDTH, rx_counters[DHCP_MESSAGE_TYPE_OFFER], - DHCP_COUNTER_WIDTH, tx_counters[DHCP_MESSAGE_TYPE_OFFER], - DHCP_COUNTER_WIDTH, rx_counters[DHCP_MESSAGE_TYPE_REQUEST], - DHCP_COUNTER_WIDTH, tx_counters[DHCP_MESSAGE_TYPE_REQUEST], - DHCP_COUNTER_WIDTH, rx_counters[DHCP_MESSAGE_TYPE_ACK], - DHCP_COUNTER_WIDTH, tx_counters[DHCP_MESSAGE_TYPE_ACK]); + syslog( + LOG_NOTICE, + "[%*s-%*s rx/tx] Discover: %*lu/%*lu, Offer: %*lu/%*lu, Request: %*lu/%*lu, ACK: %*lu/%*lu\n", + IF_NAMESIZE, vlan_intf, + (int) strlen(counter_desc[type]), counter_desc[type], + DHCP_COUNTER_WIDTH, counters[DHCP_RX][DHCP_MESSAGE_TYPE_DISCOVER], + DHCP_COUNTER_WIDTH, counters[DHCP_TX][DHCP_MESSAGE_TYPE_DISCOVER], + DHCP_COUNTER_WIDTH, counters[DHCP_RX][DHCP_MESSAGE_TYPE_OFFER], + DHCP_COUNTER_WIDTH, counters[DHCP_TX][DHCP_MESSAGE_TYPE_OFFER], + DHCP_COUNTER_WIDTH, counters[DHCP_RX][DHCP_MESSAGE_TYPE_REQUEST], + DHCP_COUNTER_WIDTH, counters[DHCP_TX][DHCP_MESSAGE_TYPE_REQUEST], + DHCP_COUNTER_WIDTH, counters[DHCP_RX][DHCP_MESSAGE_TYPE_ACK], + DHCP_COUNTER_WIDTH, counters[DHCP_TX][DHCP_MESSAGE_TYPE_ACK] + ); } /** @@ -624,13 +634,13 @@ void dhcp_device_update_snapshot(dhcp_device_context_t *context) } /** - * @code dhcp_device_print_status(context); + * @code dhcp_device_print_status(context, type); * * @brief prints status counters to syslog. */ -void dhcp_device_print_status(dhcp_device_context_t *context) +void dhcp_device_print_status(dhcp_device_context_t *context, dhcp_counters_type_t type) { if (context != NULL) { - dhcp_print_counters(context->intf, context->counters); + dhcp_print_counters(context->intf, type, context->counters[type]); } } diff --git a/src/dhcpmon/src/dhcp_device.h b/src/dhcpmon/src/dhcp_device.h index 9eee669101..133b9265a4 100644 --- a/src/dhcpmon/src/dhcp_device.h +++ b/src/dhcpmon/src/dhcp_device.h @@ -169,14 +169,15 @@ dhcp_mon_status_t dhcp_device_get_status(dhcp_mon_check_t check_type, dhcp_devic void dhcp_device_update_snapshot(dhcp_device_context_t *context); /** - * @code dhcp_device_print_status(context); + * @code dhcp_device_print_status(context, type); * * @brief prints status counters to syslog. If context is null, it will print aggregate status * - * @param context Device (interface) context + * @param context Device (interface) context + * @param counters_type Counter type to be printed * * @return none */ -void dhcp_device_print_status(dhcp_device_context_t *context); +void dhcp_device_print_status(dhcp_device_context_t *context, dhcp_counters_type_t type); #endif /* DHCP_DEVICE_H_ */ diff --git a/src/dhcpmon/src/dhcp_devman.c b/src/dhcpmon/src/dhcp_devman.c index e9a47ae581..35378a631c 100644 --- a/src/dhcpmon/src/dhcp_devman.c +++ b/src/dhcpmon/src/dhcp_devman.c @@ -198,25 +198,35 @@ dhcp_mon_status_t dhcp_devman_get_status(dhcp_mon_check_t check_type, dhcp_devic */ void dhcp_devman_update_snapshot(dhcp_device_context_t *context) { - dhcp_device_update_snapshot(context); + if (context == NULL) { + struct intf *int_ptr; + + LIST_FOREACH(int_ptr, &intfs, entry) { + dhcp_device_update_snapshot(int_ptr->dev_context); + } + + dhcp_device_update_snapshot(dhcp_devman_get_agg_dev()); + } else { + dhcp_device_update_snapshot(context); + } } /** - * @code dhcp_devman_print_status(context); + * @code dhcp_devman_print_status(context, type); * * @brief prints status counters to syslog, if context is null, it prints status counters for all interfaces */ -void dhcp_devman_print_status(dhcp_device_context_t *context) +void dhcp_devman_print_status(dhcp_device_context_t *context, dhcp_counters_type_t type) { if (context == NULL) { struct intf *int_ptr; LIST_FOREACH(int_ptr, &intfs, entry) { - dhcp_device_print_status(int_ptr->dev_context); + dhcp_device_print_status(int_ptr->dev_context, type); } - dhcp_device_print_status(dhcp_devman_get_agg_dev()); + dhcp_device_print_status(dhcp_devman_get_agg_dev(), type); } else { - dhcp_device_print_status(context); + dhcp_device_print_status(context, type); } } diff --git a/src/dhcpmon/src/dhcp_devman.h b/src/dhcpmon/src/dhcp_devman.h index 4d8a532657..bba7076902 100644 --- a/src/dhcpmon/src/dhcp_devman.h +++ b/src/dhcpmon/src/dhcp_devman.h @@ -97,14 +97,15 @@ dhcp_mon_status_t dhcp_devman_get_status(dhcp_mon_check_t check_type, dhcp_devic void dhcp_devman_update_snapshot(dhcp_device_context_t *context); /** - * @code dhcp_devman_print_status(context); + * @code dhcp_devman_print_status(context, type); * * @brief prints status counters to syslog * * @param context pointer to device (interface) context + * @param type Counter type to be printed * * @return none */ -void dhcp_devman_print_status(dhcp_device_context_t *context); +void dhcp_devman_print_status(dhcp_device_context_t *context, dhcp_counters_type_t type); #endif /* DHCP_DEVMAN_H_ */ diff --git a/src/dhcpmon/src/dhcp_mon.c b/src/dhcpmon/src/dhcp_mon.c index 8147b5057c..74d9869741 100644 --- a/src/dhcpmon/src/dhcp_mon.c +++ b/src/dhcpmon/src/dhcp_mon.c @@ -71,7 +71,7 @@ static dhcp_mon_state_t state_data[] = { static void signal_callback(evutil_socket_t fd, short event, void *arg) { syslog(LOG_ALERT, "Received signal: '%s'\n", strsignal(fd)); - dhcp_devman_print_status(NULL); + dhcp_devman_print_status(NULL, DHCP_COUNTERS_CURRENT); if ((fd == SIGTERM) || (fd == SIGINT)) { dhcp_mon_stop(); } @@ -96,7 +96,8 @@ static void check_dhcp_relay_health(dhcp_mon_state_t *state_data) case DHCP_MON_STATUS_UNHEALTHY: if (++state_data->count > dhcp_unhealthy_max_count) { syslog(LOG_ALERT, state_data->msg, state_data->count * window_interval_sec, context->intf); - dhcp_devman_print_status(context); + dhcp_devman_print_status(context, DHCP_COUNTERS_SNAPSHOT); + dhcp_devman_print_status(context, DHCP_COUNTERS_CURRENT); } break; case DHCP_MON_STATUS_HEALTHY: @@ -130,9 +131,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) check_dhcp_relay_health(&state_data[i]); } - for (uint8_t i = 0; i < sizeof(state_data) / sizeof(*state_data); i++) { - dhcp_devman_update_snapshot(state_data[i].get_context()); - } + dhcp_devman_update_snapshot(NULL); } /**