isisd: fix ldp-sync configuration

YANG model and CLI commands allow user to configure LDP-sync per area.
But the actual implementation is incorrect - all commands are changing
the config for the whole VRF instead of a single area. This commit fixes
this issue by actually implementing per area configuration.

Fixes #8578.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>

10 files changed, 176 insertions, 321 deletions

diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c
index efc7fec54..7fd9c07ed 100644
--- a/isisd/isis_circuit.c
+++ b/isisd/isis_circuit.c
@@ -174,6 +174,9 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag)
 	isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL1);
 	isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL2);
 
+	circuit->ldp_sync_info = ldp_sync_info_create();
+	circuit->ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED;
+
 	QOBJ_REG(circuit, isis_circuit);
 
 	isis_circuit_if_bind(circuit, ifp);
@@ -196,6 +199,8 @@ void isis_circuit_del(struct isis_circuit *circuit)
 
 	QOBJ_UNREG(circuit);
 
+	ldp_sync_info_free(&circuit->ldp_sync_info);
+
 	circuit_mt_finish(circuit);
 	isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL1);
 	isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL2);
@@ -751,6 +756,8 @@ int isis_circuit_up(struct isis_circuit *circuit)
 	if (circuit->area->mta && circuit->area->mta->status)
 		isis_link_params_update(circuit, circuit->interface);
 
+	isis_if_ldp_sync_enable(circuit);
+
 #ifndef FABRICD
 	/* send northbound notification */
 	isis_notif_if_state_change(circuit, false);
@@ -766,6 +773,8 @@ void isis_circuit_down(struct isis_circuit *circuit)
 	isis_notif_if_state_change(circuit, true);
 #endif /* ifndef FABRICD */
 
+	isis_if_ldp_sync_disable(circuit);
+
 	/* log adjacency changes if configured to do so */
 	if (circuit->area->log_adj_changes) {
 		struct isis_adjacency *adj = NULL;
diff --git a/isisd/isis_ldp_sync.c b/isisd/isis_ldp_sync.c
index 585f76980..62d8b8334 100644
--- a/isisd/isis_ldp_sync.c
+++ b/isisd/isis_ldp_sync.c
@@ -65,28 +65,20 @@ int isis_ldp_sync_state_update(struct ldp_igp_sync_if_state state)
 	struct interface *ifp;
 	struct isis_circuit *circuit = NULL;
 	struct isis_area *area;
-	struct listnode *node;
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
-
-	/* if isis is not enabled or LDP-SYNC is not configured ignore */
-	if (!isis ||
-	    !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
-		return 0;
 
 	/* lookup circuit */
 	ifp = if_lookup_by_index(state.ifindex, VRF_DEFAULT);
 	if (ifp == NULL)
 		return 0;
 
-	for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) {
-		circuit = circuit_lookup_by_ifp(ifp, area->circuit_list);
-		if (circuit != NULL)
-			break;
-	}
+	circuit = ifp->info;
+	if (circuit == NULL)
+		return 0;
 
 	/* if isis is not enabled or LDP-SYNC is not configured ignore */
-	if (circuit == NULL ||
-	    !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+	area = circuit->area;
+	if (area == NULL
+	    || !CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
 		return 0;
 
 	/* received ldp-sync interface state from LDP */
@@ -103,15 +95,12 @@ int isis_ldp_sync_state_update(struct ldp_igp_sync_if_state state)
 int isis_ldp_sync_announce_update(struct ldp_igp_sync_announce announce)
 {
 	struct isis_area *area;
-	struct listnode *node;
-	struct vrf *vrf;
-	struct interface *ifp;
+	struct listnode *anode, *cnode;
 	struct isis_circuit *circuit;
 	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
 
-	/* if isis is not enabled or LDP-SYNC is not configured ignore */
-	if (!isis ||
-	    !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+	/* if isis is not enabled ignore */
+	if (!isis)
 		return 0;
 
 	if (announce.proto != ZEBRA_ROUTE_LDP)
@@ -123,15 +112,12 @@ int isis_ldp_sync_announce_update(struct ldp_igp_sync_announce announce)
 	 *  set cost to LSInfinity
 	 *  send request to LDP for LDP-SYNC state for each interface
 	 */
-	vrf = vrf_lookup_by_id(VRF_DEFAULT);
-	FOR_ALL_INTERFACES (vrf, ifp) {
-		for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) {
-			circuit = circuit_lookup_by_ifp(ifp,
-				area->circuit_list);
-			if (circuit == NULL)
-				continue;
+	for (ALL_LIST_ELEMENTS_RO(isis->area_list, anode, area)) {
+		if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+			continue;
+
+		for (ALL_LIST_ELEMENTS_RO(area->circuit_list, cnode, circuit))
 			isis_ldp_sync_if_start(circuit, true);
-		}
 	}
 
 	return 0;
@@ -157,32 +143,6 @@ void isis_ldp_sync_state_req_msg(struct isis_circuit *circuit)
 /*
  * LDP-SYNC general interface routines
  */
-void isis_ldp_sync_if_init(struct isis_circuit *circuit, struct isis *isis)
-{
-	struct ldp_sync_info *ldp_sync_info;
-	struct interface *ifp = circuit->interface;
-
-	/* called when ISIS is configured on an interface
-	 *  if LDP-IGP Sync is configured globally set state
-	 *  and if ptop interface LDP LDP-SYNC is enabled
-	 */
-	ils_debug("ldp_sync: init if %s ", ifp->name);
-	if (circuit->ldp_sync_info == NULL)
-		circuit->ldp_sync_info = ldp_sync_info_create();
-	ldp_sync_info = circuit->ldp_sync_info;
-
-	/* specifed on interface overrides global config. */
-	if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN))
-		ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown;
-
-	if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG))
-		ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED;
-
-	if ((circuit->circ_type == CIRCUIT_T_P2P || if_is_pointopoint(ifp)) &&
-	    ldp_sync_info->enabled == LDP_IGP_SYNC_ENABLED)
-		ldp_sync_info->state = LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP;
-}
-
 void isis_ldp_sync_if_start(struct isis_circuit *circuit,
 	bool send_state_req)
 {
@@ -251,49 +211,17 @@ void isis_ldp_sync_ldp_fail(struct isis_circuit *circuit)
 	}
 }
 
-void isis_ldp_sync_if_remove(struct isis_circuit *circuit, bool remove)
-{
-	struct ldp_sync_info *ldp_sync_info;
-
-	if (circuit->ldp_sync_info == NULL)
-		return;
-
-	ldp_sync_info = circuit->ldp_sync_info;
-
-	/* Stop LDP-SYNC on this interface:
-	 *  if holddown timer is running stop it
-	 *  delete ldp instance on interface
-	 *  restore metric
-	 */
-	ils_debug("ldp_sync: remove if %s", circuit->interface
-		  ? circuit->interface->name : "");
-
-	THREAD_OFF(ldp_sync_info->t_holddown);
-	ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED;
-	isis_ldp_sync_set_if_metric(circuit, true);
-	if (remove) {
-		/* ISIS instance being removed free ldp-sync info */
-		ldp_sync_info_free((struct ldp_sync_info **)&(ldp_sync_info));
-		circuit->ldp_sync_info = NULL;
-	}
-}
-
 static int isis_ldp_sync_adj_state_change(struct isis_adjacency *adj)
 {
 	struct isis_circuit *circuit = adj->circuit;
-	struct ldp_sync_info *ldp_sync_info;
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
+	struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info;
+	struct isis_area *area = circuit->area;
 
-	if (!isis ||
-	    !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) ||
-	    circuit->interface->vrf_id != VRF_DEFAULT ||
-	    if_is_loopback(circuit->interface))
+	if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)
+	    || circuit->interface->vrf_id != VRF_DEFAULT
+	    || if_is_loopback(circuit->interface))
 		return 0;
 
-	if (circuit->ldp_sync_info == NULL)
-		isis_ldp_sync_if_init(circuit, isis);
-	ldp_sync_info = circuit->ldp_sync_info;
-
 	if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED)
 		return 0;
 
@@ -329,16 +257,15 @@ bool isis_ldp_sync_if_metric_config(struct isis_circuit *circuit, int level,
 				    int metric)
 {
 	struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info;
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
+	struct isis_area *area = circuit->area;
 
 	/* configured interface metric has been changed:
 	 *   if LDP-IGP Sync is running and metric has been set to LSInfinity
 	 *   change saved value so when ldp-sync completes proper metric is
 	 *   restored
 	 */
-	if (isis &&
-	    CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) &&
-	    ldp_sync_info != NULL) {
+	if (area && CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)
+	    && ldp_sync_info != NULL) {
 
 		if (CHECK_FLAG(ldp_sync_info->flags,
 			       LDP_SYNC_FLAG_SET_METRIC)) {
@@ -471,15 +398,12 @@ void isis_ldp_sync_holddown_timer_add(struct isis_circuit *circuit)
 void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info)
 {
 	struct isis_area *area;
-	struct listnode *node;
+	struct listnode *anode, *cnode;
 	struct isis_circuit *circuit;
-	struct interface *ifp;
-	struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT);
 	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
 
-	/* if isis is not enabled or LDP-SYNC is not configured ignore */
-	if (!isis
-	    || !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+	/* if isis is not enabled ignore */
+	if (!isis)
 		return;
 
 	/* Check if the LDP main client session closed */
@@ -492,14 +416,12 @@ void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info)
 	 */
 	zlog_err("ldp_sync: LDP down");
 
-	FOR_ALL_INTERFACES (vrf, ifp) {
-		for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) {
-			circuit =
-				circuit_lookup_by_ifp(ifp, area->circuit_list);
-			if (circuit == NULL)
-				continue;
+	for (ALL_LIST_ELEMENTS_RO(isis->area_list, anode, area)) {
+		if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+			continue;
+
+		for (ALL_LIST_ELEMENTS_RO(area->circuit_list, cnode, circuit))
 			isis_ldp_sync_ldp_fail(circuit);
-		}
 	}
 }
 
@@ -507,110 +429,130 @@ void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info)
  * LDP-SYNC routes used by set commands.
  */
 
-void isis_if_set_ldp_sync_enable(struct isis_circuit *circuit)
+void isis_area_ldp_sync_enable(struct isis_area *area)
 {
-	struct ldp_sync_info *ldp_sync_info;
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
+	struct isis_circuit *circuit;
+	struct listnode *node;
+
+	if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) {
+		SET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE);
+
+		for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit))
+			isis_if_ldp_sync_enable(circuit);
+	}
+}
+
+void isis_area_ldp_sync_disable(struct isis_area *area)
+{
+	struct isis_circuit *circuit;
+	struct listnode *node;
+
+	if (CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) {
+		for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit))
+			isis_if_ldp_sync_disable(circuit);
+
+		UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE);
+
+		UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN);
+		area->ldp_sync_cmd.holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT;
+	}
+}
+
+void isis_area_ldp_sync_set_holddown(struct isis_area *area, uint16_t holddown)
+{
+	struct isis_circuit *circuit;
+	struct listnode *node;
+
+	if (holddown == LDP_IGP_SYNC_HOLDDOWN_DEFAULT)
+		UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN);
+	else
+		SET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN);
+
+	area->ldp_sync_cmd.holddown = holddown;
+
+	for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit))
+		isis_if_set_ldp_sync_holddown(circuit);
+}
+
+void isis_if_ldp_sync_enable(struct isis_circuit *circuit)
+{
+	struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info;
+	struct isis_area *area = circuit->area;
 
 	/* called when setting LDP-SYNC at the global level:
 	 *  specifed on interface overrides global config
 	 *  if ptop link send msg to LDP indicating ldp-sync enabled
  	 */
-	if (!isis || if_is_loopback(circuit->interface))
+	if (if_is_loopback(circuit->interface))
 		return;
 
-	if (CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) {
-		if (circuit->ldp_sync_info == NULL)
-			isis_ldp_sync_if_init(circuit, isis);
-		ldp_sync_info = circuit->ldp_sync_info;
+	ils_debug("ldp_sync: enable if %s", circuit->interface->name);
 
-		/* config on interface, overrides global config. */
-		if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG))
-			if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED)
-			    return;
+	if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+		return;
 
-		ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED;
-		ils_debug("ldp_sync: enable if %s", circuit->interface->name);
+	/* config on interface, overrides global config. */
+	if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG))
+		if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED)
+			return;
 
-		/* send message to LDP if ptop link */
-		if (circuit->circ_type == CIRCUIT_T_P2P ||
-		    if_is_pointopoint(circuit->interface)) {
-			ldp_sync_info->state =
-				LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP;
-			isis_ldp_sync_state_req_msg(circuit);
-		} else {
-			ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED;
-			zlog_debug("ldp_sync: Sync only runs on P2P links %s",
-				   circuit->interface->name);
-		}
-	} else
-		/* delete LDP sync even if configured on an interface */
-		isis_ldp_sync_if_remove(circuit, false);
+	if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN))
+		ldp_sync_info->holddown = area->ldp_sync_cmd.holddown;
+
+	if (circuit->circ_type == CIRCUIT_T_P2P
+	    || if_is_pointopoint(circuit->interface)) {
+		ldp_sync_info->state = LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP;
+		isis_ldp_sync_state_req_msg(circuit);
+	} else {
+		ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED;
+		ils_debug("ldp_sync: Sync only runs on P2P links %s",
+			  circuit->interface->name);
+	}
+}
+
+void isis_if_ldp_sync_disable(struct isis_circuit *circuit)
+{
+	struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info;
+	struct isis_area *area = circuit->area;
+
+	/* Stop LDP-SYNC on this interface:
+	 *  if holddown timer is running stop it
+	 *  delete ldp instance on interface
+	 *  restore metric
+	 */
+	if (if_is_loopback(circuit->interface))
+		return;
+
+	ils_debug("ldp_sync: remove if %s", circuit->interface->name);
+
+	if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE))
+		return;
+
+	THREAD_OFF(ldp_sync_info->t_holddown);
+	ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED;
+	isis_ldp_sync_set_if_metric(circuit, true);
 }
 
 void isis_if_set_ldp_sync_holddown(struct isis_circuit *circuit)
 {
-	struct ldp_sync_info *ldp_sync_info;
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
+	struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info;
+	struct isis_area *area = circuit->area;
 
 	/* called when setting LDP-SYNC at the global level:
 	 *  specifed on interface overrides global config.
 	 */
-	if (!isis || if_is_loopback(circuit->interface))
+	if (if_is_loopback(circuit->interface))
 		return;
 
-	if (circuit->ldp_sync_info == NULL)
-		isis_ldp_sync_if_init(circuit, isis);
-	ldp_sync_info = circuit->ldp_sync_info;
-
 	/* config on interface, overrides global config. */
 	if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN))
 		return;
-	if (CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN))
-		ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown;
+	if (CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN))
+		ldp_sync_info->holddown = area->ldp_sync_cmd.holddown;
 	else
 		ldp_sync_info->holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT;
 }
 
-void isis_ldp_sync_gbl_exit(bool remove)
-{
-	struct isis_area *area;
-	struct listnode *node;
-	struct isis_circuit *circuit;
-	struct interface *ifp;
-	struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT);
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
-
-	/* if you delete LDP-SYNC at a gobal level is clears all LDP-SYNC
-	 * configuration, even interface configuration
-	 */
-	if (isis &&
-	    CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) {
-		/* register with opaque client to recv LDP-IGP Sync msgs */
-		zclient_unregister_opaque(zclient,
-					  LDP_IGP_SYNC_IF_STATE_UPDATE);
-		zclient_unregister_opaque(zclient,
-					  LDP_IGP_SYNC_ANNOUNCE_UPDATE);
-
-		/* disable LDP-SYNC globally */
-		UNSET_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE);
-		UNSET_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN);
-		isis->ldp_sync_cmd.holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT;
-
-		/* remove LDP-SYNC on all ISIS interfaces */
-		FOR_ALL_INTERFACES (vrf, ifp) {
-			for (ALL_LIST_ELEMENTS_RO(isis->area_list, node,
-						  area)) {
-				circuit = circuit_lookup_by_ifp(ifp,
-					area->circuit_list);
-				if (circuit == NULL)
-					continue;
-				isis_ldp_sync_if_remove(circuit, remove);
-			}
-		}
-	}
-}
-
 /*
  * LDP-SYNC routines used by show commands.
  */
@@ -693,11 +635,6 @@ DEFUN (show_isis_mpls_ldp_interface,
 		return CMD_SUCCESS;
 	}
 
-	if (!CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) {
-		vty_out(vty, "LDP-sync is disabled\n");
-		return CMD_SUCCESS;
-	}
-
 	if (argv_find(argv, argc, "INTERFACE", &idx_intf))
 		ifname = argv[idx_intf]->arg;
 
diff --git a/isisd/isis_ldp_sync.h b/isisd/isis_ldp_sync.h
index 977c5ba0d..69a180000 100644
--- a/isisd/isis_ldp_sync.h
+++ b/isisd/isis_ldp_sync.h
@@ -29,13 +29,15 @@
 			zlog_debug(__VA_ARGS__);                               \
 	} while (0)
 
-extern void isis_if_set_ldp_sync_enable(struct isis_circuit *circuit);
+extern void isis_area_ldp_sync_enable(struct isis_area *area);
+extern void isis_area_ldp_sync_disable(struct isis_area *area);
+extern void isis_area_ldp_sync_set_holddown(struct isis_area *area,
+					    uint16_t holddown);
+extern void isis_if_ldp_sync_enable(struct isis_circuit *circuit);
+extern void isis_if_ldp_sync_disable(struct isis_circuit *circuit);
 extern void isis_if_set_ldp_sync_holddown(struct  isis_circuit *circuit);
-extern void isis_ldp_sync_if_init(struct isis_circuit *circuit,
-				  struct isis *isis);
 extern void isis_ldp_sync_if_start(struct isis_circuit *circuit,
 				   bool send_state_req);
-extern void isis_ldp_sync_if_remove(struct isis_circuit *circuit, bool remove);
 extern void isis_ldp_sync_if_complete(struct isis_circuit *circuit);
 extern void isis_ldp_sync_holddown_timer_add(struct isis_circuit *circuit);
 extern void
@@ -49,5 +51,4 @@ extern void isis_ldp_sync_set_if_metric(struct isis_circuit *circuit,
 extern bool isis_ldp_sync_if_metric_config(struct isis_circuit *circuit,
 					   int level, int metric);
 extern void isis_ldp_sync_init(void);
-extern void isis_ldp_sync_gbl_exit(bool remove);
 #endif /* _ZEBRA_ISIS_LDP_SYNC_H */
diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c
index 4b68cd3be..5cad9fcfc 100644
--- a/isisd/isis_nb_config.c
+++ b/isisd/isis_nb_config.c
@@ -32,7 +32,6 @@
 #include "spf_backoff.h"
 #include "lib_errors.h"
 #include "vrf.h"
-#include "zclient.h"
 #include "ldp_sync.h"
 
 #include "isisd/isisd.h"
@@ -56,8 +55,6 @@
 DEFINE_MTYPE_STATIC(ISISD, ISIS_MPLS_TE,    "ISIS MPLS_TE parameters");
 DEFINE_MTYPE_STATIC(ISISD, ISIS_PLIST_NAME, "ISIS prefix-list name");
 
-extern struct zclient *zclient;
-
 /*
  * XPath: /frr-isisd:isis/instance
  */
@@ -87,16 +84,10 @@ int isis_instance_create(struct nb_cb_create_args *args)
 int isis_instance_destroy(struct nb_cb_destroy_args *args)
 {
 	struct isis_area *area;
-	vrf_id_t vrf_id;
 
 	if (args->event != NB_EV_APPLY)
 		return NB_OK;
 	area = nb_running_unset_entry(args->dnode);
-	vrf_id = area->isis->vrf_id;
-
-	/* remove ldp-sync config */
-	if (vrf_id == VRF_DEFAULT)
-		isis_ldp_sync_gbl_exit(true);
 
 	isis_area_destroy(area);
 	return NB_OK;
@@ -2369,11 +2360,6 @@ int isis_instance_segment_routing_prefix_sid_map_prefix_sid_n_flag_clear_modify(
 int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args)
 {
 	struct isis_area *area;
-	struct listnode *node;
-	struct isis_circuit *circuit;
-	struct interface *ifp;
-	struct vrf *vrf;
-	struct isis *isis;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
@@ -2392,30 +2378,7 @@ int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args)
 		break;
 	case NB_EV_APPLY:
 		area = nb_running_get_entry(args->dnode, NULL, true);
-		isis = area->isis;
-		vrf = vrf_lookup_by_id(isis->vrf_id);
-
-		/* register with opaque client to recv LDP-IGP Sync msgs */
-		zclient_register_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE);
-		zclient_register_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE);
-
-		if (!CHECK_FLAG(isis->ldp_sync_cmd.flags,
-				LDP_SYNC_FLAG_ENABLE)) {
-			SET_FLAG(isis->ldp_sync_cmd.flags,
-				 LDP_SYNC_FLAG_ENABLE);
-
-			/* turn on LDP-IGP Sync on all ptop ISIS interfaces */
-			FOR_ALL_INTERFACES (vrf, ifp) {
-				for (ALL_LIST_ELEMENTS_RO(isis->area_list, node,
-							  area)) {
-					circuit = circuit_lookup_by_ifp(
-						ifp, area->circuit_list);
-					if (circuit == NULL)
-						continue;
-					isis_if_set_ldp_sync_enable(circuit);
-				}
-			}
-		}
+		isis_area_ldp_sync_enable(area);
 		break;
 	}
 	return NB_OK;
@@ -2423,11 +2386,13 @@ int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args)
 
 int isis_instance_mpls_ldp_sync_destroy(struct nb_cb_destroy_args *args)
 {
+	struct isis_area *area;
+
 	if (args->event != NB_EV_APPLY)
 		return NB_OK;
 
-	/* remove ldp-sync config */
-	isis_ldp_sync_gbl_exit(false);
+	area = nb_running_get_entry(args->dnode, NULL, true);
+	isis_area_ldp_sync_disable(area);
 
 	return NB_OK;
 }
@@ -2438,12 +2403,7 @@ int isis_instance_mpls_ldp_sync_destroy(struct nb_cb_destroy_args *args)
 int isis_instance_mpls_ldp_sync_holddown_modify(struct nb_cb_modify_args *args)
 {
 	struct isis_area *area;
-	struct listnode *node;
-	struct isis_circuit *circuit;
-	struct interface *ifp;
-	struct vrf *vrf;
-	uint16_t holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT;
-	struct isis *isis;
+	uint16_t holddown;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
@@ -2462,29 +2422,8 @@ int isis_instance_mpls_ldp_sync_holddown_modify(struct nb_cb_modify_args *args)
 		break;
 	case NB_EV_APPLY:
 		area = nb_running_get_entry(args->dnode, NULL, true);
-		isis = area->isis;
-		vrf = vrf_lookup_by_id(isis->vrf_id);
 		holddown = yang_dnode_get_uint16(args->dnode, NULL);
-
-		if (holddown == LDP_IGP_SYNC_HOLDDOWN_DEFAULT)
-			UNSET_FLAG(isis->ldp_sync_cmd.flags,
-				   LDP_SYNC_FLAG_HOLDDOWN);
-		else
-			SET_FLAG(isis->ldp_sync_cmd.flags,
-				 LDP_SYNC_FLAG_HOLDDOWN);
-		isis->ldp_sync_cmd.holddown = holddown;
-
-		/* set holddown time on all ISIS interfaces */
-		FOR_ALL_INTERFACES (vrf, ifp) {
-			for (ALL_LIST_ELEMENTS_RO(isis->area_list, node,
-						  area)) {
-				circuit = circuit_lookup_by_ifp(ifp,
-							area->circuit_list);
-				if (circuit == NULL)
-					continue;
-				isis_if_set_ldp_sync_holddown(circuit);
-			}
-		}
+		isis_area_ldp_sync_set_holddown(area, holddown);
 		break;
 	}
 	return NB_OK;
@@ -2562,9 +2501,6 @@ int lib_interface_isis_destroy(struct nb_cb_destroy_args *args)
 
 	circuit = nb_running_unset_entry(args->dnode);
 
-	/* remove ldp-sync config */
-	isis_ldp_sync_if_remove(circuit, true);
-
 	isis_circuit_del(circuit);
 
 	return NB_OK;
@@ -3283,13 +3219,12 @@ int lib_interface_isis_mpls_ldp_sync_modify(struct nb_cb_modify_args *args)
 	struct isis_circuit *circuit;
 	struct ldp_sync_info *ldp_sync_info;
 	bool ldp_sync_enable;
-	struct isis *isis;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
 		circuit = nb_running_get_entry(args->dnode, NULL, false);
 		if (circuit == NULL || circuit->area == NULL)
-			return NB_ERR_VALIDATION;
+			break;
 
 		if (circuit->isis->vrf_id != VRF_DEFAULT) {
 			snprintf(args->errmsg, args->errmsg_len,
@@ -3303,39 +3238,17 @@ int lib_interface_isis_mpls_ldp_sync_modify(struct nb_cb_modify_args *args)
 	case NB_EV_APPLY:
 		circuit = nb_running_get_entry(args->dnode, NULL, true);
 		ldp_sync_enable = yang_dnode_get_bool(args->dnode, NULL);
-		isis = circuit->isis;
 
-		if (circuit->ldp_sync_info == NULL)
-			isis_ldp_sync_if_init(circuit, isis);
-		assert(circuit->ldp_sync_info != NULL);
 		ldp_sync_info = circuit->ldp_sync_info;
 
-		if (ldp_sync_enable) {
-			/* enable LDP-SYNC on an interface
-			 *  if ptop interface send message to LDP to get state
-			 */
-			SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG);
-			ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED;
-			if (circuit->circ_type == CIRCUIT_T_P2P) {
-				ldp_sync_info->state =
-					LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP;
-				isis_ldp_sync_state_req_msg(circuit);
-			} else {
-				zlog_debug("ldp_sync: only runs on P2P links %s",
-					   circuit->interface->name);
-				ldp_sync_info->state =
-					LDP_IGP_SYNC_STATE_NOT_REQUIRED;
-			}
-		} else {
-			/* disable LDP-SYNC on an interface
-			 *  stop holddown timer if running
-			 *  restore isis metric
-			 */
-			SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG);
-			ldp_sync_info->enabled = LDP_IGP_SYNC_DEFAULT;
-			ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED;
-			THREAD_OFF(ldp_sync_info->t_holddown);
-			isis_ldp_sync_set_if_metric(circuit, true);
+		SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG);
+		ldp_sync_info->enabled = ldp_sync_enable;
+
+		if (circuit->area) {
+			if (ldp_sync_enable)
+				isis_if_ldp_sync_enable(circuit);
+			else
+				isis_if_ldp_sync_disable(circuit);
 		}
 		break;
 	}
@@ -3350,13 +3263,12 @@ int lib_interface_isis_mpls_holddown_modify(struct nb_cb_modify_args *args)
 	struct isis_circuit *circuit;
 	struct ldp_sync_info *ldp_sync_info;
 	uint16_t holddown;
-	struct isis *isis;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
 		circuit = nb_running_get_entry(args->dnode, NULL, false);
 		if (circuit == NULL || circuit->area == NULL)
-			return NB_ERR_VALIDATION;
+			break;
 
 		if (circuit->isis->vrf_id != VRF_DEFAULT) {
 			snprintf(args->errmsg, args->errmsg_len,
@@ -3370,11 +3282,7 @@ int lib_interface_isis_mpls_holddown_modify(struct nb_cb_modify_args *args)
 	case NB_EV_APPLY:
 		circuit = nb_running_get_entry(args->dnode, NULL, true);
 		holddown = yang_dnode_get_uint16(args->dnode, NULL);
-		isis = circuit->isis;
 
-		if (circuit->ldp_sync_info == NULL)
-			isis_ldp_sync_if_init(circuit, isis);
-		assert(circuit->ldp_sync_info != NULL);
 		ldp_sync_info = circuit->ldp_sync_info;
 
 		SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN);
@@ -3388,14 +3296,12 @@ int lib_interface_isis_mpls_holddown_destroy(struct nb_cb_destroy_args *args)
 {
 	struct isis_circuit *circuit;
 	struct ldp_sync_info *ldp_sync_info;
-	struct isis *isis;
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
 		circuit = nb_running_get_entry(args->dnode, NULL, false);
-		if (circuit == NULL || circuit->ldp_sync_info == NULL
-		    || circuit->area == NULL)
-			return NB_ERR_VALIDATION;
+		if (circuit == NULL || circuit->area == NULL)
+			break;
 
 		if (circuit->isis->vrf_id != VRF_DEFAULT) {
 			snprintf(args->errmsg, args->errmsg_len,
@@ -3408,15 +3314,12 @@ int lib_interface_isis_mpls_holddown_destroy(struct nb_cb_destroy_args *args)
 		break;
 	case NB_EV_APPLY:
 		circuit = nb_running_get_entry(args->dnode, NULL, true);
-		isis = circuit->isis;
 		ldp_sync_info = circuit->ldp_sync_info;
+
 		UNSET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN);
 
-		if (CHECK_FLAG(isis->ldp_sync_cmd.flags,
-			       LDP_SYNC_FLAG_HOLDDOWN))
-			ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown;
-		else
-			ldp_sync_info->holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT;
+		if (circuit->area)
+			isis_if_set_ldp_sync_holddown(circuit);
 		break;
 	}
 
diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c
index cb4dd2569..90959eb98 100644
--- a/isisd/isis_zebra.c
+++ b/isisd/isis_zebra.c
@@ -728,6 +728,8 @@ static void isis_zebra_connected(struct zclient *zclient)
 {
 	zclient_send_reg_requests(zclient, VRF_DEFAULT);
 	zclient_register_opaque(zclient, LDP_RLFA_LABELS);
+	zclient_register_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE);
+	zclient_register_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE);
 }
 
 /*
@@ -818,6 +820,8 @@ void isis_zebra_init(struct thread_master *master, int instance)
 void isis_zebra_stop(void)
 {
 	zclient_unregister_opaque(zclient, LDP_RLFA_LABELS);
+	zclient_unregister_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE);
+	zclient_unregister_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE);
 	zclient_stop(zclient_sync);
 	zclient_free(zclient_sync);
 	zclient_stop(zclient);
diff --git a/isisd/isisd.h b/isisd/isisd.h
index 79717b0cb..9d0b57e9f 100644
--- a/isisd/isisd.h
+++ b/isisd/isisd.h
@@ -97,7 +97,6 @@ struct isis {
 	int snmp_notifications;
 
 	struct route_table *ext_info[REDIST_PROTOCOL_COUNT];
-	struct ldp_sync_info_cmd ldp_sync_cmd; 	/* MPLS LDP-IGP Sync */
 };
 
 extern struct isis_master *im;
@@ -209,6 +208,8 @@ struct isis_area {
 	struct prefix_list *rlfa_plist[ISIS_LEVELS];
 	size_t rlfa_protected_links[ISIS_LEVELS];
 	size_t tilfa_protected_links[ISIS_LEVELS];
+	/* MPLS LDP-IGP Sync */
+	struct ldp_sync_info_cmd ldp_sync_cmd;
 	/* Counters */
 	uint32_t circuit_state_changes;
 	struct isis_redist redist_settings[REDIST_PROTOCOL_COUNT]
diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref
index 9cb70a475..7180f84d1 100644
--- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref
+++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref
@@ -1,7 +1,7 @@
 {
   "r3-eth1":{
     "ldpIgpSyncEnabled":false,
-    "holdDownTimeInSec":50,
+    "holdDownTimeInSec":0,
     "ldpIgpSyncState":"Sync not required"
   },
   "r3-eth2":{
diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref
index 9cb70a475..7180f84d1 100644
--- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref
+++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref
@@ -1,7 +1,7 @@
 {
   "r3-eth1":{
     "ldpIgpSyncEnabled":false,
-    "holdDownTimeInSec":50,
+    "holdDownTimeInSec":0,
     "ldpIgpSyncState":"Sync not required"
   },
   "r3-eth2":{
diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref
index 9cb70a475..7180f84d1 100644
--- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref
+++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref
@@ -1,7 +1,7 @@
 {
   "r3-eth1":{
     "ldpIgpSyncEnabled":false,
-    "holdDownTimeInSec":50,
+    "holdDownTimeInSec":0,
     "ldpIgpSyncState":"Sync not required"
   },
   "r3-eth2":{
diff --git a/yang/frr-isisd.yang b/yang/frr-isisd.yang
index 7c820c961..be7426957 100644
--- a/yang/frr-isisd.yang
+++ b/yang/frr-isisd.yang
@@ -791,10 +791,10 @@ module frr-isisd {
       leaf holddown {
         type uint16 {
           range "0..10000";
-      }
-      units "seconds";
-      description
-        "Time to wait for LDP-Sync to occur before restoring interface metric.";
+        }
+        units "seconds";
+        description
+          "Time to wait for LDP-Sync to occur before restoring interface metric.";
       }
     }