From 89a634c1f83c63ed5656faf98a42f02f619414e8 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Mon, 13 Aug 2018 15:28:17 -0300 Subject: [PATCH 1/7] bfdd: turn repeatable code into functions Turn some code that will be repeated in the next commits into functions so we avoid that. Signed-off-by: Rafael Zalamena --- bfdd/bfdd_vty.c | 110 ++++++++++++++++++++++++++++++------------------ 1 file changed, 69 insertions(+), 41 deletions(-) diff --git a/bfdd/bfdd_vty.c b/bfdd/bfdd_vty.c index eedaec0a16..3745658fde 100644 --- a/bfdd/bfdd_vty.c +++ b/bfdd/bfdd_vty.c @@ -58,12 +58,19 @@ static int bfd_configure_peer(struct bfd_peer_cfg *bpc, bool mhop, const char *ifname, const char *vrfname, char *ebuf, size_t ebuflen); +static void _display_peer_header(struct vty *vty, struct bfd_session *bs); static struct json_object *__display_peer_json(struct bfd_session *bs); +static struct json_object *_peer_json_header(struct bfd_session *bs); static void _display_peer_json(struct vty *vty, struct bfd_session *bs); static void _display_peer(struct vty *vty, struct bfd_session *bs); static void _display_all_peers(struct vty *vty, bool use_json); static void _display_peer_iter(struct hash_backet *hb, void *arg); static void _display_peer_json_iter(struct hash_backet *hb, void *arg); +static struct bfd_session * +_find_peer_or_error(struct vty *vty, int argc, struct cmd_token **argv, + const char *label, const char *peer_str, + const char *local_str, const char *ifname, + const char *vrfname); /* @@ -353,11 +360,8 @@ DEFPY(bfd_no_peer, bfd_no_peer_cmd, /* * Show commands helper functions */ -static void _display_peer(struct vty *vty, struct bfd_session *bs) +static void _display_peer_header(struct vty *vty, struct bfd_session *bs) { - char buf[256]; - time_t now; - if (BFD_CHECK_FLAG(bs->flags, BFD_SESS_FLAG_MH)) { vty_out(vty, "\tpeer %s", satostr(&bs->mhop.peer)); vty_out(vty, " multihop"); @@ -377,6 +381,14 @@ static void _display_peer(struct vty *vty, struct bfd_session *bs) if (bs->pl) vty_out(vty, "\t\tlabel: %s\n", bs->pl->pl_label); +} + +static void _display_peer(struct vty *vty, struct bfd_session *bs) +{ + char buf[256]; + time_t now; + + _display_peer_header(vty, bs); vty_out(vty, "\t\tID: %u\n", bs->discrs.my_discr); vty_out(vty, "\t\tRemote ID: %u\n", bs->discrs.remote_discr); @@ -418,7 +430,7 @@ static void _display_peer(struct vty *vty, struct bfd_session *bs) vty_out(vty, "\t\t\tTransmission interval: %" PRIu32 "ms", bs->timers.desired_min_tx / 1000); if (bs->up_min_tx != bs->timers.desired_min_tx) - vty_out(vty, " (configured %" PRIu32 "ms)\n", + vty_out(vty, " (configured %" PRIu32 "ms)\n", bs->up_min_tx / 1000); else vty_out(vty, "\n"); @@ -441,7 +453,7 @@ static void _display_peer(struct vty *vty, struct bfd_session *bs) vty_out(vty, "\n"); } -static struct json_object *__display_peer_json(struct bfd_session *bs) +static struct json_object *_peer_json_header(struct bfd_session *bs) { struct json_object *jo = json_object_new_object(); @@ -465,6 +477,13 @@ static struct json_object *__display_peer_json(struct bfd_session *bs) if (bs->pl) json_object_string_add(jo, "label", bs->pl->pl_label); + return jo; +} + +static struct json_object *__display_peer_json(struct bfd_session *bs) +{ + struct json_object *jo = _peer_json_header(bs); + json_object_int_add(jo, "id", bs->discrs.my_discr); json_object_int_add(jo, "remote-id", bs->discrs.remote_discr); @@ -550,6 +569,7 @@ static void _display_all_peers(struct vty *vty, bool use_json) struct json_object *jo; if (use_json == false) { + vty_out(vty, "BFD Peers:\n"); bfd_id_iterate(_display_peer_iter, vty); return; } @@ -561,37 +581,11 @@ static void _display_all_peers(struct vty *vty, bool use_json) json_object_free(jo); } -DEFPY(bfd_show_peers, bfd_show_peers_cmd, "show bfd peers [json]", - SHOW_STR - "Bidirection Forwarding Detection\n" - "BFD peers status\n" - JSON_STR) -{ - bool json = use_json(argc, argv); - - if (json) { - _display_all_peers(vty, true); - } else { - vty_out(vty, "BFD Peers:\n"); - _display_all_peers(vty, false); - } - - return CMD_SUCCESS; -} - -DEFPY(bfd_show_peer, bfd_show_peer_cmd, - "show bfd peer $peer [{multihop|local-address $local|interface IFNAME$ifname|vrf NAME$vrfname}]> [json]", - SHOW_STR - "Bidirection Forwarding Detection\n" - "BFD peers status\n" - "Peer label\n" - PEER_IPV4_STR PEER_IPV6_STR - MHOP_STR - LOCAL_STR LOCAL_IPV4_STR LOCAL_IPV6_STR - INTERFACE_STR - LOCAL_INTF_STR - VRF_STR VRF_NAME_STR - JSON_STR) +static struct bfd_session * +_find_peer_or_error(struct vty *vty, int argc, struct cmd_token **argv, + const char *label, const char *peer_str, + const char *local_str, const char *ifname, + const char *vrfname) { int idx; bool mhop; @@ -608,7 +602,7 @@ DEFPY(bfd_show_peer, bfd_show_peer_cmd, bs = pl->pl_bs; } else { strtosa(peer_str, &psa); - if (local) { + if (local_str) { strtosa(local_str, &lsa); lsap = &lsa; } else @@ -622,7 +616,7 @@ DEFPY(bfd_show_peer, bfd_show_peer_cmd, != 0) { vty_out(vty, "%% Invalid peer configuration: %s\n", errormsg); - return CMD_WARNING_CONFIG_FAILED; + return NULL; } bs = bs_peer_find(&bpc); @@ -634,15 +628,49 @@ DEFPY(bfd_show_peer, bfd_show_peer_cmd, label ? label : peer_str); if (ifname) vty_out(vty, " interface %s", ifname); - if (local) + if (local_str) vty_out(vty, " local-address %s", local_str); if (vrfname) vty_out(vty, " vrf %s", vrfname); vty_out(vty, "'\n"); - return CMD_WARNING_CONFIG_FAILED; + return NULL; } + return bs; +} + + +/* + * Show commands. + */ +DEFPY(bfd_show_peers, bfd_show_peers_cmd, "show bfd peers [json]", + SHOW_STR + "Bidirection Forwarding Detection\n" + "BFD peers status\n" JSON_STR) +{ + _display_all_peers(vty, use_json(argc, argv)); + + return CMD_SUCCESS; +} + +DEFPY(bfd_show_peer, bfd_show_peer_cmd, + "show bfd peer $peer [{multihop|local-address $local|interface IFNAME$ifname|vrf NAME$vrfname}]> [json]", + SHOW_STR + "Bidirection Forwarding Detection\n" + "BFD peers status\n" + "Peer label\n" PEER_IPV4_STR PEER_IPV6_STR MHOP_STR LOCAL_STR + LOCAL_IPV4_STR LOCAL_IPV6_STR INTERFACE_STR LOCAL_INTF_STR VRF_STR + VRF_NAME_STR JSON_STR) +{ + struct bfd_session *bs; + + /* Look up the BFD peer. */ + bs = _find_peer_or_error(vty, argc, argv, label, peer_str, local_str, + ifname, vrfname); + if (bs == NULL) + return CMD_WARNING_CONFIG_FAILED; + if (use_json(argc, argv)) { _display_peer_json(vty, bs); } else { From 0684c9b12ca78d61c2ba5788175a6eaa9bb702f3 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Mon, 13 Aug 2018 17:47:21 -0300 Subject: [PATCH 2/7] bfdd: add new counters and command to show them Added 3 new counters to BFD sessions: * Session up events count; * Session down events count; * Zebra notifications count; In addition to previosly available counters: * Count of received control packets; * Count of transmitted control packets; * Count of received echo packets; * Count of transmitted echo packets; With this count we are able to visualize the BFD activity, bandwidth usage, interface/network flapping and excess of zebra notifications. Signed-off-by: Rafael Zalamena --- bfdd/bfd.c | 8 ++- bfdd/bfd.h | 3 + bfdd/bfdd_vty.c | 139 +++++++++++++++++++++++++++++++++++++++++++++ bfdd/ptm_adapter.c | 2 + 4 files changed, 150 insertions(+), 2 deletions(-) diff --git a/bfdd/bfd.c b/bfdd/bfd.c index 28b6beadcb..b3253a14d7 100644 --- a/bfdd/bfd.c +++ b/bfdd/bfd.c @@ -185,10 +185,12 @@ void ptm_bfd_ses_up(struct bfd_session *bfd) control_notify(bfd); - if (old_state != bfd->ses_state) + if (old_state != bfd->ses_state) { + bfd->stats.session_up++; log_info("state-change: [%s] %s -> %s", bs_to_string(bfd), state_list[old_state].str, state_list[bfd->ses_state].str); + } } void ptm_bfd_ses_dn(struct bfd_session *bfd, uint8_t diag) @@ -212,11 +214,13 @@ void ptm_bfd_ses_dn(struct bfd_session *bfd, uint8_t diag) if (BFD_CHECK_FLAG(bfd->flags, BFD_SESS_FLAG_ECHO_ACTIVE)) ptm_bfd_echo_stop(bfd, 0); - if (old_state != bfd->ses_state) + if (old_state != bfd->ses_state) { + bfd->stats.session_down++; log_info("state-change: [%s] %s -> %s reason:%s", bs_to_string(bfd), state_list[old_state].str, state_list[bfd->ses_state].str, get_diag_str(bfd->local_diag)); + } } static int ptm_bfd_get_vrf_name(char *port_name, char *vrf_name) diff --git a/bfdd/bfd.h b/bfdd/bfd.h index 40aecaa67b..d665448abf 100644 --- a/bfdd/bfd.h +++ b/bfdd/bfd.h @@ -186,6 +186,9 @@ struct bfd_session_stats { uint64_t tx_ctrl_pkt; uint64_t rx_echo_pkt; uint64_t tx_echo_pkt; + uint64_t session_up; + uint64_t session_down; + uint64_t znotification; }; struct bfd_session_vxlan_info { diff --git a/bfdd/bfdd_vty.c b/bfdd/bfdd_vty.c index 3745658fde..ae6081f01a 100644 --- a/bfdd/bfdd_vty.c +++ b/bfdd/bfdd_vty.c @@ -66,6 +66,12 @@ static void _display_peer(struct vty *vty, struct bfd_session *bs); static void _display_all_peers(struct vty *vty, bool use_json); static void _display_peer_iter(struct hash_backet *hb, void *arg); static void _display_peer_json_iter(struct hash_backet *hb, void *arg); +static void _display_peer_counter(struct vty *vty, struct bfd_session *bs); +static struct json_object *__display_peer_counters_json(struct bfd_session *bs); +static void _display_peer_counters_json(struct vty *vty, struct bfd_session *bs); +static void _display_peer_counter_iter(struct hash_backet *hb, void *arg); +static void _display_peer_counter_json_iter(struct hash_backet *hb, void *arg); +static void _display_peers_counter(struct vty *vty, bool use_json); static struct bfd_session * _find_peer_or_error(struct vty *vty, int argc, struct cmd_token **argv, const char *label, const char *peer_str, @@ -581,6 +587,89 @@ static void _display_all_peers(struct vty *vty, bool use_json) json_object_free(jo); } +static void _display_peer_counter(struct vty *vty, struct bfd_session *bs) +{ + _display_peer_header(vty, bs); + + vty_out(vty, "\t\tControl packet input: %" PRIu64 " packets\n", + bs->stats.rx_ctrl_pkt); + vty_out(vty, "\t\tControl packet output: %" PRIu64 " packets\n", + bs->stats.tx_ctrl_pkt); + vty_out(vty, "\t\tEcho packet input: %" PRIu64 " packets\n", + bs->stats.rx_echo_pkt); + vty_out(vty, "\t\tEcho packet output: %" PRIu64 " packets\n", + bs->stats.tx_echo_pkt); + vty_out(vty, "\t\tSession up events: %" PRIu64 "\n", + bs->stats.session_up); + vty_out(vty, "\t\tSession down events: %" PRIu64 "\n", + bs->stats.session_down); + vty_out(vty, "\t\tZebra notifications: %" PRIu64 "\n", + bs->stats.znotification); + vty_out(vty, "\n"); +} + +static struct json_object *__display_peer_counters_json(struct bfd_session *bs) +{ + struct json_object *jo = _peer_json_header(bs); + + json_object_int_add(jo, "control-packet-input", bs->stats.rx_ctrl_pkt); + json_object_int_add(jo, "control-packet-output", bs->stats.tx_ctrl_pkt); + json_object_int_add(jo, "echo-packet-input", bs->stats.rx_echo_pkt); + json_object_int_add(jo, "echo-packet-output", bs->stats.tx_echo_pkt); + json_object_int_add(jo, "session-up", bs->stats.session_up); + json_object_int_add(jo, "session-down", bs->stats.session_down); + json_object_int_add(jo, "zebra-notifications", bs->stats.znotification); + + return jo; +} + +static void _display_peer_counters_json(struct vty *vty, struct bfd_session *bs) +{ + struct json_object *jo = __display_peer_counters_json(bs); + + vty_out(vty, "%s\n", json_object_to_json_string_ext(jo, 0)); + json_object_free(jo); +} + +static void _display_peer_counter_iter(struct hash_backet *hb, void *arg) +{ + struct vty *vty = arg; + struct bfd_session *bs = hb->data; + + _display_peer_counter(vty, bs); +} + +static void _display_peer_counter_json_iter(struct hash_backet *hb, void *arg) +{ + struct json_object *jo = arg, *jon = NULL; + struct bfd_session *bs = hb->data; + + jon = __display_peer_counters_json(bs); + if (jon == NULL) { + log_warning("%s: not enough memory", __func__); + return; + } + + json_object_array_add(jo, jon); +} + +static void _display_peers_counter(struct vty *vty, bool use_json) +{ + struct json_object *jo; + + if (use_json == false) { + vty_out(vty, "BFD Peers:\n"); + bfd_id_iterate(_display_peer_counter_iter, vty); + return; + } + + jo = json_object_new_array(); + bfd_id_iterate(_display_peer_counter_json_iter, jo); + + vty_out(vty, "%s\n", json_object_to_json_string_ext(jo, 0)); + json_object_free(jo); +} + static struct bfd_session * _find_peer_or_error(struct vty *vty, int argc, struct cmd_token **argv, const char *label, const char *peer_str, @@ -681,6 +770,54 @@ DEFPY(bfd_show_peer, bfd_show_peer_cmd, return CMD_SUCCESS; } +DEFPY(bfd_show_peer_counters, bfd_show_peer_counters_cmd, + "show bfd peer $peer [{multihop|local-address $local|interface IFNAME$ifname|vrf NAME$vrfname}]> counters [json]", + SHOW_STR + "Bidirection Forwarding Detection\n" + "BFD peers status\n" + "Peer label\n" + PEER_IPV4_STR + PEER_IPV6_STR + MHOP_STR + LOCAL_STR + LOCAL_IPV4_STR + LOCAL_IPV6_STR + INTERFACE_STR + LOCAL_INTF_STR + VRF_STR + VRF_NAME_STR + "Show BFD peer counters information\n" + JSON_STR) +{ + struct bfd_session *bs; + + /* Look up the BFD peer. */ + bs = _find_peer_or_error(vty, argc, argv, label, peer_str, local_str, + ifname, vrfname); + if (bs == NULL) + return CMD_WARNING_CONFIG_FAILED; + + if (use_json(argc, argv)) + _display_peer_counters_json(vty, bs); + else + _display_peer_counter(vty, bs); + + return CMD_SUCCESS; +} + +DEFPY(bfd_show_peers_counters, bfd_show_peers_counters_cmd, + "show bfd peers counters [json]", + SHOW_STR + "Bidirection Forwarding Detection\n" + "BFD peers status\n" + "Show BFD peer counters information\n" + JSON_STR) +{ + _display_peers_counter(vty, use_json(argc, argv)); + + return CMD_SUCCESS; +} + /* * Function definitions. @@ -875,6 +1012,8 @@ struct cmd_node bfd_peer_node = { void bfdd_vty_init(void) { + install_element(ENABLE_NODE, &bfd_show_peers_counters_cmd); + install_element(ENABLE_NODE, &bfd_show_peer_counters_cmd); install_element(ENABLE_NODE, &bfd_show_peers_cmd); install_element(ENABLE_NODE, &bfd_show_peer_cmd); install_element(CONFIG_NODE, &bfd_enter_cmd); diff --git a/bfdd/ptm_adapter.c b/bfdd/ptm_adapter.c index 9a2b70d36a..3e528b0d10 100644 --- a/bfdd/ptm_adapter.c +++ b/bfdd/ptm_adapter.c @@ -155,6 +155,8 @@ int ptm_bfd_notify(struct bfd_session *bs) struct stream *msg; struct sockaddr_any sac; + bs->stats.znotification++; + /* * Message format: * - header: command, vrf From 123582b3c3d87aa3da61a4368d9ac665a4fcdecf Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 15 Aug 2018 16:04:13 -0300 Subject: [PATCH 3/7] bfdd: improve docs Improvements: * Show command line as code, to avoid confusion about single dash (`-`) and double dash; * Tell the user where the BGP BFD commands can be found; * Document OSPF/OSPF6/PIM BFD commands; * Document JSON commands; * Tell about session counters; Signed-off-by: Rafael Zalamena --- doc/user/bfd.rst | 105 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/doc/user/bfd.rst b/doc/user/bfd.rst index 0e0fd23ece..8eb9efe789 100644 --- a/doc/user/bfd.rst +++ b/doc/user/bfd.rst @@ -38,7 +38,7 @@ may also be specified (:ref:`common-invocation-options`). .. option:: --bfdctl Set the BFD daemon control socket location. If using a non-default - socket location. + socket location:: /usr/lib/frr/bfdd --bfdctl /tmp/bfdd.sock @@ -160,6 +160,8 @@ Peer Configurations BGP BFD Configuration --------------------- +The following commands are available inside the BGP configuration node. + .. index:: neighbor bfd .. clicmd:: neighbor bfd @@ -174,6 +176,66 @@ BGP BFD Configuration Removes any notification registration for this neighbor. +.. _bfd-ospf-peer-config: + +OSPF BFD Configuration +--------------------- + +The following commands are available inside the interface configuration node. + +.. index:: ip ospf bfd +.. clicmd:: ip ospf bfd + + Listen for BFD events on peers created on the interface. Every time + a new neighbor is found a BFD peer is created to monitor the link + status for fast convergence. + +.. index:: no ip ospf bfd +.. clicmd:: no ip ospf bfd + + Removes any notification registration for this interface peers. + + +.. _bfd-ospf6-peer-config: + +OSPF6 BFD Configuration +----------------------- + +The following commands are available inside the interface configuration node. + +.. index:: ipv6 ospf6 bfd +.. clicmd:: ipv6 ospf6 bfd + + Listen for BFD events on peers created on the interface. Every time + a new neighbor is found a BFD peer is created to monitor the link + status for fast convergence. + +.. index:: no ipv6 ospf6 bfd +.. clicmd:: no ipv6 ospf6 bfd + + Removes any notification registration for this interface peers. + + +.. _bfd-pim-peer-config: + +PIM BFD Configuration +--------------------- + +The following commands are available inside the interface configuration node. + +.. index:: ip pim bfd +.. clicmd:: ip pim bfd + + Listen for BFD events on peers created on the interface. Every time + a new neighbor is found a BFD peer is created to monitor the link + status for fast convergence. + +.. index:: no ip pim bfd +.. clicmd:: no ip pim bfd + + Removes any notification registration for this interface peers. + + .. _bfd-configuration: Configuration @@ -300,3 +362,44 @@ You can inspect the current BFD peer status with the following commands: Receive interval: 300ms Transmission interval: 300ms Echo transmission interval: 50ms + + frr# show bfd peer 192.168.0.1 json +{"multihop":false,"peer":"192.168.0.1","id":1,"remote-id":1,"status":"up","uptime":161,"diagnostic":"ok","remote-diagnostic":"ok","receive-interval":300,"transmit-interval":300,"echo-interval":50,"remote-receive-interval":300,"remote-transmit-interval":300,"remote-echo-interval":50} + + +You can also inspect peer session counters with the following commands: + +:: + + frr# show bfd peers counters + BFD Peers: + peer 192.168.2.1 interface r2-eth2 + Control packet input: 28 packets + Control packet output: 28 packets + Echo packet input: 0 packets + Echo packet output: 0 packets + Session up events: 1 + Session down events: 0 + Zebra notifications: 2 + + peer 192.168.0.1 + Control packet input: 54 packets + Control packet output: 103 packets + Echo packet input: 965 packets + Echo packet output: 966 packets + Session up events: 1 + Session down events: 0 + Zebra notifications: 4 + + frr# show bfd peer 192.168.0.1 counters + peer 192.168.0.1 + Control packet input: 126 packets + Control packet output: 247 packets + Echo packet input: 2409 packets + Echo packet output: 2410 packets + Session up events: 1 + Session down events: 0 + Zebra notifications: 4 + + frr# show bfd peer 192.168.0.1 counters json +{"multihop":false,"peer":"192.168.0.1","control-packet-input":348,"control-packet-output":685,"echo-packet-input":6815,"echo-packet-output":6816,"session-up":1,"session-down":0,"zebra-notifications":4} From ff98a58940d6d2bae8df2b9ee466d2388b9b3499 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 15 Aug 2018 18:00:24 -0300 Subject: [PATCH 4/7] bfdd: fix coverity scan issues (1472630, 1472623) Always initialize/santize string before calling the `read` function. It ensures that the debug function will always pick up the right thing. Signed-off-by: Rafael Zalamena --- bfdd/bfd_packet.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c index 8d74c5c234..543181a12a 100644 --- a/bfdd/bfd_packet.c +++ b/bfdd/bfd_packet.c @@ -718,12 +718,6 @@ static ssize_t bfd_recv_ipv4(int sd, bool is_mhop, char *port, size_t portlen, msghdr.msg_control = cmsgbuf; msghdr.msg_controllen = sizeof(cmsgbuf); - /* Sanitize input/output. */ - memset(port, 0, portlen); - memset(vrfname, 0, vrfnamelen); - memset(local, 0, sizeof(*local)); - memset(peer, 0, sizeof(*peer)); - mlen = recvmsg(sd, &msghdr, MSG_DONTWAIT); if (mlen == -1) { if (errno != EAGAIN) @@ -843,12 +837,6 @@ ssize_t bfd_recv_ipv6(int sd, bool is_mhop, char *port, size_t portlen, msghdr6.msg_control = cmsgbuf6; msghdr6.msg_controllen = sizeof(cmsgbuf6); - /* Sanitize input/output. */ - memset(port, 0, portlen); - memset(vrfname, 0, vrfnamelen); - memset(local, 0, sizeof(*local)); - memset(peer, 0, sizeof(*peer)); - mlen = recvmsg(sd, &msghdr6, MSG_DONTWAIT); if (mlen == -1) { if (errno != EAGAIN) @@ -983,6 +971,12 @@ int bfd_recv_cb(struct thread *t) return 0; } + /* Sanitize input/output. */ + memset(port, 0, sizeof(port)); + memset(vrfname, 0, sizeof(vrfname)); + memset(&local, 0, sizeof(local)); + memset(&peer, 0, sizeof(peer)); + /* Handle control packets. */ is_mhop = is_vxlan = false; if (sd == bglobal.bg_shop || sd == bglobal.bg_mhop) { From ae9f45a3ae64b028294376b5ff1329d345e9cde1 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Wed, 15 Aug 2018 20:40:40 -0300 Subject: [PATCH 5/7] bfdd: improve ptm_adapter log messages Make them look like the rest of the daemon: message begins with a unique descriptive message to help locate debug messages. Signed-off-by: Rafael Zalamena --- bfdd/ptm_adapter.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/bfdd/ptm_adapter.c b/bfdd/ptm_adapter.c index 3e528b0d10..4287891621 100644 --- a/bfdd/ptm_adapter.c +++ b/bfdd/ptm_adapter.c @@ -276,7 +276,7 @@ static void _ptm_msg_read_address(struct stream *msg, struct sockaddr_any *sa) return; default: - log_warning("%s: invalid family: %d", __func__, family); + log_warning("ptm-read-address: invalid family: %d", family); break; } @@ -331,7 +331,7 @@ static int _ptm_msg_read(struct stream *msg, int command, *pc = pc_new(pid); if (*pc == NULL) { - log_debug("%s: failed to allocate memory", __func__); + log_debug("ptm-read: failed to allocate memory"); return -1; } @@ -372,8 +372,8 @@ static int _ptm_msg_read(struct stream *msg, int command, * structure, otherwise fail. */ STREAM_GETC(msg, ifnamelen); - if (ifnamelen > sizeof(bpc->bpc_localif)) { - log_error("%s: interface name is too big", __func__); + if (ifnamelen >= sizeof(bpc->bpc_localif)) { + log_error("ptm-read: interface name is too big"); return -1; } @@ -388,8 +388,7 @@ static int _ptm_msg_read(struct stream *msg, int command, if (bpc->bpc_local.sa_sin.sin_family != 0 && (bpc->bpc_local.sa_sin.sin_family != bpc->bpc_peer.sa_sin.sin_family)) { - log_warning("%s: peer family doesn't match local type", - __func__); + log_warning("ptm-read: peer family doesn't match local type"); return -1; } @@ -417,7 +416,7 @@ static void bfdd_dest_register(struct stream *msg) if (bs == NULL) { bs = ptm_bfd_sess_new(&bpc); if (bs == NULL) { - log_debug("%s: failed to create BFD session", __func__); + log_debug("ptm-add-dest: failed to create BFD session"); return; } } else { @@ -430,7 +429,7 @@ static void bfdd_dest_register(struct stream *msg) /* Create client peer notification register. */ pcn = pcn_new(pc, bs); if (pcn == NULL) { - log_error("%s: failed to registrate notifications", __func__); + log_error("ptm-add-dest: failed to registrate notifications"); return; } @@ -453,7 +452,7 @@ static void bfdd_dest_deregister(struct stream *msg) /* Find or start new BFD session. */ bs = bs_peer_find(&bpc); if (bs == NULL) { - log_debug("%s: failed to create BFD session", __func__); + log_debug("ptm-del-dest: failed to find BFD session"); return; } @@ -476,14 +475,14 @@ static void bfdd_client_register(struct stream *msg) pc = pc_new(pid); if (pc == NULL) { - log_error("%s: failed to register client: %u", __func__, pid); + log_error("ptm-add-client: failed to register client: %u", pid); return; } return; stream_failure: - log_error("%s: failed to register client", __func__); + log_error("ptm-add-client: failed to register client"); } /* @@ -500,7 +499,7 @@ static void bfdd_client_deregister(struct stream *msg) pc = pc_lookup(pid); if (pc == NULL) { - log_debug("%s: failed to find client: %u", __func__, pid); + log_debug("ptm-del-client: failed to find client: %u", pid); return; } @@ -509,7 +508,7 @@ static void bfdd_client_deregister(struct stream *msg) return; stream_failure: - log_error("%s: failed to deregister client", __func__); + log_error("ptm-del-client: failed to deregister client"); } static int bfdd_replay(int cmd, struct zclient *zc, uint16_t len, vrf_id_t vid) @@ -535,14 +534,14 @@ static int bfdd_replay(int cmd, struct zclient *zc, uint16_t len, vrf_id_t vid) break; default: - log_debug("%s: invalid message type %u", __func__, rcmd); + log_debug("ptm-replay: invalid message type %u", rcmd); return -1; } return 0; stream_failure: - log_error("%s: failed to find command", __func__); + log_error("ptm-replay: failed to find command"); return -1; } From 7efe273ae4a30b7e617678a9a1933cbfcf8608e1 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 16 Aug 2018 15:23:13 -0300 Subject: [PATCH 6/7] bfdd: fix zebra_ptm adapter memory leak Memory leak detect with Address Sanitizer and topotests. Signed-off-by: Rafael Zalamena --- zebra/zebra_ptm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra/zebra_ptm.c b/zebra/zebra_ptm.c index cc2d5d411d..3f52514cdf 100644 --- a/zebra/zebra_ptm.c +++ b/zebra/zebra_ptm.c @@ -1283,6 +1283,7 @@ static void zebra_ptm_send_bfdd(struct stream *msg) } stream_free(msgc); + stream_free(msg); } static void zebra_ptm_send_clients(struct stream *msg) @@ -1323,6 +1324,7 @@ static void zebra_ptm_send_clients(struct stream *msg) } stream_free(msgc); + stream_free(msg); } static int _zebra_ptm_bfd_client_deregister(struct zserv *zs) From 3bcd76c4af51b9883207514fdc8ff42877680a89 Mon Sep 17 00:00:00 2001 From: Rafael Zalamena Date: Thu, 16 Aug 2018 15:47:34 -0300 Subject: [PATCH 7/7] bfdd: fix coverity scan issue (CID 1472622) Don't use the stack variable, but what we have recorded in our buffered data on the heap. Signed-off-by: Rafael Zalamena --- bfdd/control.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bfdd/control.c b/bfdd/control.c index 7e586dbb45..554a5a8d80 100644 --- a/bfdd/control.c +++ b/bfdd/control.c @@ -514,7 +514,7 @@ skip_header: if (bcb->bcb_left > 0) goto schedule_next_read; - switch (bcm.bcm_type) { + switch (bcb->bcb_bcm->bcm_type) { case BMT_REQUEST_ADD: control_handle_request_add(bcs, bcb->bcb_bcm); break; @@ -533,8 +533,8 @@ skip_header: default: log_debug("%s: unhandled message type: %d", __func__, - bcm.bcm_type); - control_response(bcs, bcm.bcm_id, BCM_RESPONSE_ERROR, + bcb->bcb_bcm->bcm_type); + control_response(bcs, bcb->bcb_bcm->bcm_id, BCM_RESPONSE_ERROR, "invalid message type"); break; }