bfd: Fix for missing BFD client regs/deregs from quagga clients

Ticket: CM-11256
Signed-off-by: Radhika Mahankali <radhika@cumulusnetworks.com>
Reviewed-by: Donald Sharp <sharpd@cumulusnetworks.com>
Reviewed-by: Kanna Rajagopal <kanna@cumulusnetworks.com>
Testing: Unit, PTM smoke, OSPF smoke, BGP Smoke

Issue:
BFD client registrations are not being sent to PTM from BGP/OSPF clients when the quagga clients have no BFD configuration. This can create stale BFD sessions in PTM when BFD is removed from quagga configuration before quagga is restarted.

BFD client de-registrations from BGP/OSPF also go missing sometimes when quagga is restarted. This also will cause stale BFD sessions in PTM.

Root Cause:
BFD client registrations were being sent at the time of BGP/OSPF daemon initialization. But, they were being sent to zebra before the socket connection between zebra and BGP/OSPF was established. This causes the missing BFD client registrations.

BFD client de-registrations are sent from zebra when zebra detects socket close for BGP/OSPF daemons. Based on the timing, the de-registrations may happen after socket between PTM and zebra is closed. This will result in missing de-registrations.

Fix:
Moved sending of BFD client registration messages to zebra connected callback to make sure that they are sent after the BGP/OSPF daemons connect with zebra.

Added BFD client de-registrations for BGP/OSPF to be also sent when zebra daemon gets restart signal. They are sent from the signal handler only if it was not already handled in zebra client socket close callback.

diff --git a/bgpd/bgp_bfd.c b/bgpd/bgp_bfd.c
index b64fbee04d978a9401e91259319c61c1487cff06..b8b005369529e4f62f29c2c70d37f3554394ea73 100644 (file)
--- a/bgpd/bgp_bfd.c
+++ b/bgpd/bgp_bfd.c
@@ -711,7 +711,4 @@ bgp_bfd_init(void)
   install_element (BGP_NODE, &no_neighbor_bfd_cmd);
   install_element (BGP_NODE, &no_neighbor_bfd_val_cmd);
   install_element (BGP_NODE, &no_neighbor_bfd_type_cmd);
-
-  /* Send the client registration */
-  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
 }

diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 457f865beababbc765c56e386601a8e5129c23a5..102e38e4bef786de65d8c063b1101b3e4de23d56 100644 (file)
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -32,6 +32,7 @@ Boston, MA 02111-1307, USA.  */
 #include "queue.h"
 #include "memory.h"
 #include "lib/json.h"
+#include "lib/bfd.h"
 
 #include "bgpd/bgpd.h"
 #include "bgpd/bgp_route.h"
@@ -43,7 +44,6 @@ Boston, MA 02111-1307, USA.  */
 #include "bgpd/bgp_mpath.h"
 #include "bgpd/bgp_nexthop.h"
 #include "bgpd/bgp_nht.h"
-#include "bgpd/bgp_bfd.h"
 
 /* All information about zebra. */
 struct zclient *zclient = NULL;
@@ -2053,6 +2053,9 @@ bgp_zebra_connected (struct zclient *zclient)
 
   bgp_zebra_instance_register (bgp);
 
+  /* Send the client registration */
+  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
+
   /* TODO - What if we have peers and networks configured, do we have to
    * kick-start them?
    */

diff --git a/ospf6d/ospf6_bfd.c b/ospf6d/ospf6_bfd.c
index 503c7e0cd71a69cc7640fe7c906e3f8f5e765788..f9bb6f003126497cf0a93ba5a90f0f8e4da98ce2 100644 (file)
--- a/ospf6d/ospf6_bfd.c
+++ b/ospf6d/ospf6_bfd.c
@@ -420,7 +420,4 @@ ospf6_bfd_init(void)
   install_element (INTERFACE_NODE, &ipv6_ospf6_bfd_cmd);
   install_element (INTERFACE_NODE, &ipv6_ospf6_bfd_param_cmd);
   install_element (INTERFACE_NODE, &no_ipv6_ospf6_bfd_cmd);
-
-  /* Send the client registration */
-  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
 }

diff --git a/ospf6d/ospf6_zebra.c b/ospf6d/ospf6_zebra.c
index 9c738233b4d91b5601fd006e312a6ac2789e2bbc..3137a33c500c4da797200d08fba6b71a34d4248e 100644 (file)
--- a/ospf6d/ospf6_zebra.c
+++ b/ospf6d/ospf6_zebra.c
@@ -28,6 +28,7 @@
 #include "stream.h"
 #include "zclient.h"
 #include "memory.h"
+#include "lib/bfd.h"
 
 #include "ospf6_proto.h"
 #include "ospf6_top.h"
@@ -654,6 +655,9 @@ DEFUN (no_redistribute_ospf6,
 static void
 ospf6_zebra_connected (struct zclient *zclient)
 {
+  /* Send the client registration */
+  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
+
   zclient_send_reg_requests (zclient, VRF_DEFAULT);
 }
 

diff --git a/ospfd/ospf_bfd.c b/ospfd/ospf_bfd.c
index ec486c46882747383a0dc91db2aaa8cf054577c3..c87bcb0afbc8fd82714d1dc969190cfa109d93cb 100644 (file)
--- a/ospfd/ospf_bfd.c
+++ b/ospfd/ospf_bfd.c
@@ -452,7 +452,4 @@ ospf_bfd_init(void)
   install_element (INTERFACE_NODE, &ip_ospf_bfd_param_cmd);
   install_element (INTERFACE_NODE, &no_ip_ospf_bfd_cmd);
   install_element (INTERFACE_NODE, &no_ip_ospf_bfd_param_cmd);
-
-  /* Send the client registration */
-  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
 }

diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c
index ffdfd750d2bfbebe41d46224389e12770fbcfdfe..16cd2d2479eeb056f95737e35231ad15624ff40e 100644 (file)
--- a/ospfd/ospf_zebra.c
+++ b/ospfd/ospf_zebra.c
@@ -34,6 +34,7 @@
 #include "filter.h"
 #include "plist.h"
 #include "log.h"
+#include "lib/bfd.h"
 
 #include "ospfd/ospfd.h"
 #include "ospfd/ospf_interface.h"
@@ -1557,6 +1558,9 @@ ospf_distance_apply (struct prefix_ipv4 *p, struct ospf_route *or)
 static void
 ospf_zebra_connected (struct zclient *zclient)
 {
+  /* Send the client registration */
+  bfd_client_sendmsg(zclient, ZEBRA_BFD_CLIENT_REGISTER);
+
   zclient_send_reg_requests (zclient, VRF_DEFAULT);
 }
 

diff --git a/zebra/zebra_ptm.c b/zebra/zebra_ptm.c
index bf6afb021603b9162cf647fb3899e1208f75786d..931fc73e43bf7f6cb1c7e4ecf5aba16c6dd0e4bb 100644 (file)
--- a/zebra/zebra_ptm.c
+++ b/zebra/zebra_ptm.c
@@ -131,11 +131,13 @@ zebra_ptm_init (void)
 void
 zebra_ptm_finish(void)
 {
-  if (ptm_cb.ptm_sock != -1)
-    close(ptm_cb.ptm_sock);
+  int proto;
 
-  if (ptm_cb.wb)
-    buffer_free(ptm_cb.wb);
+  for (proto = 0; proto < ZEBRA_ROUTE_MAX; proto++)
+    if (CHECK_FLAG(ptm_cb.client_flags[proto], ZEBRA_PTM_BFD_CLIENT_FLAG_REG))
+      zebra_ptm_bfd_client_deregister(proto);
+
+  buffer_flush_all(ptm_cb.wb, ptm_cb.ptm_sock);
 
   if (ptm_cb.out_data)
     free(ptm_cb.out_data);
@@ -150,6 +152,12 @@ zebra_ptm_finish(void)
     thread_cancel (ptm_cb.t_write);
   if (ptm_cb.t_timer)
     thread_cancel (ptm_cb.t_timer);
+
+  if (ptm_cb.wb)
+    buffer_free(ptm_cb.wb);
+
+  if (ptm_cb.ptm_sock != -1)
+    close(ptm_cb.ptm_sock);
 }
 
 static int
@@ -999,24 +1007,26 @@ zebra_ptm_bfd_client_register (struct zserv *client, int sock, u_short length)
     zlog_debug ("%s: Sent message (%d) %s", __func__, data_len,
                   ptm_cb.out_data);
   zebra_ptm_send_message(ptm_cb.out_data, data_len);
+
+  SET_FLAG(ptm_cb.client_flags[client->proto], ZEBRA_PTM_BFD_CLIENT_FLAG_REG);
   return 0;
 }
 
 /* BFD client deregister */
 void
-zebra_ptm_bfd_client_deregister (struct zserv *client)
+zebra_ptm_bfd_client_deregister (int proto)
 {
   void *out_ctxt;
   char tmp_buf[64];
   int data_len = ZEBRA_PTM_SEND_MAX_SOCKBUF;
 
-  if (client->proto != ZEBRA_ROUTE_OSPF && client->proto != ZEBRA_ROUTE_BGP
-      && client->proto != ZEBRA_ROUTE_OSPF6)
+  if (proto != ZEBRA_ROUTE_OSPF && proto != ZEBRA_ROUTE_BGP
+      && proto != ZEBRA_ROUTE_OSPF6)
     return;
 
   if (IS_ZEBRA_DEBUG_EVENT)
-    zlog_debug("bfd_client_deregister msg for client %s",
-                zebra_route_string(client->proto));
+    zlog_err("bfd_client_deregister msg for client %s",
+                zebra_route_string(proto));
 
   if (ptm_cb.ptm_sock == -1)
     {
@@ -1030,7 +1040,7 @@ zebra_ptm_bfd_client_deregister (struct zserv *client)
   sprintf(tmp_buf, "%s", ZEBRA_PTM_BFD_CLIENT_DEREG_CMD);
   ptm_lib_append_msg(ptm_hdl, out_ctxt, ZEBRA_PTM_CMD_STR, tmp_buf);
 
-  sprintf(tmp_buf, "%s", zebra_route_string(client->proto));
+  sprintf(tmp_buf, "%s", zebra_route_string(proto));
   ptm_lib_append_msg(ptm_hdl, out_ctxt, ZEBRA_PTM_BFD_CLIENT_FIELD,
                       tmp_buf);
 
@@ -1039,7 +1049,9 @@ zebra_ptm_bfd_client_deregister (struct zserv *client)
   if (IS_ZEBRA_DEBUG_SEND)
     zlog_debug ("%s: Sent message (%d) %s", __func__, data_len,
                   ptm_cb.out_data);
+
   zebra_ptm_send_message(ptm_cb.out_data, data_len);
+  UNSET_FLAG(ptm_cb.client_flags[proto], ZEBRA_PTM_BFD_CLIENT_FLAG_REG);
 }
 
 int

diff --git a/zebra/zebra_ptm.h b/zebra/zebra_ptm.h
index 27c0e42f99ef87e688398dc28124b43c74291280..71c85d90942a2d4167f6a59907cf9d7597205194 100644 (file)
--- a/zebra/zebra_ptm.h
+++ b/zebra/zebra_ptm.h
@@ -27,6 +27,8 @@ extern const char ZEBRA_PTM_SOCK_NAME[];
 #define ZEBRA_PTM_MAX_SOCKBUF 3200 /* 25B *128 ports */
 #define ZEBRA_PTM_SEND_MAX_SOCKBUF 512
 
+#define ZEBRA_PTM_BFD_CLIENT_FLAG_REG   (1 << 1) /* client registered with BFD */
+
 /* Zebra ptm context block */
 struct zebra_ptm_cb
 {
@@ -44,6 +46,7 @@ struct zebra_ptm_cb
 
   int ptm_enable;
   int pid;
+  u_int8_t client_flags[ZEBRA_ROUTE_MAX];
 };
 
 #define ZEBRA_PTM_STATUS_DOWN 0
@@ -72,5 +75,5 @@ int zebra_ptm_bfd_client_register (struct zserv *client, int sock,
 void zebra_ptm_if_init(struct zebra_if *zebra_ifp);
 void zebra_ptm_if_set_ptm_state(struct interface *ifp, struct zebra_if *zebra_ifp);
 void zebra_ptm_if_write (struct vty *vty, struct zebra_if *zebra_ifp);
-void zebra_ptm_bfd_client_deregister (struct zserv *client);
+void zebra_ptm_bfd_client_deregister (int proto);
 #endif

diff --git a/zebra/zserv.c b/zebra/zserv.c
index d6de0792e184302ac9f2def978d54c85221f1472..1529bc286cb80bbfbd48c58da87cb8e5866316d8 100644 (file)
--- a/zebra/zserv.c
+++ b/zebra/zserv.c
@@ -1752,7 +1752,7 @@ static void
 zebra_client_close (struct zserv *client)
 {
   /* Send client de-registration to BFD */
-  zebra_ptm_bfd_client_deregister(client);
+  zebra_ptm_bfd_client_deregister(client->proto);
 
   /* Cleanup any registered nexthops - across all VRFs. */
   zebra_client_close_cleanup_rnh (client);