From: b29332 <bao.zhen@h3c.com>
Date: Thu, 15 Aug 2024 08:20:19 +0000 (+0800)
Subject: isisd: 'tiebreaker' command line funtionality is inconsistent with its implementation
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=27413dd24a431ac1a67d3e790c3d0e67000fb47b;p=mirror%2Ffrr.git

isisd: 'tiebreaker' command line funtionality is inconsistent with its implementation

The command fast-reroute lfa tiebreaker [downstream | lowest-backup-metric | node-protecting] index (1-255) [level-1 | level-2] will overwrite configurations with the same index but different types. This is because the index is set as the key in frr-isisd.yang. However, the lfa_tiebreaker_cmp function uses a tuple (index, type) as the key. Therefore, the yang file should be modified to stay in sync with the business logic.

Test Scenario:
On RouterA, first configure fast-reroute lfa tiebreaker downstream index 100 level-1, then configure fast-reroute lfa tiebreaker lowest-backup-metric index 100 level-1, and check the configuration:

!
router isis 10
 fast-reroute lfa tiebreaker lowest-backup-metric index 100 level-1
exit
!

Signed-off-by: baozhen-H3C <bao.zhen@h3c.com>
---

diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c
index 735e39a377..c86d929903 100644
--- a/isisd/isis_cli.c
+++ b/isisd/isis_cli.c
@@ -2365,31 +2365,27 @@ DEFPY_YANG (isis_frr_lfa_tiebreaker,
 
 	if (!level || strmatch(level, "level-1")) {
 		if (no) {
-			snprintf(
-				xpath, XPATH_MAXLEN,
-				"./fast-reroute/level-1/lfa/tiebreaker[index='%s']",
-				index_str);
+			snprintf(xpath, XPATH_MAXLEN,
+				 "./fast-reroute/level-1/lfa/tiebreaker[index='%s'][type='%s']",
+				 index_str, type);
 			nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL);
 		} else {
-			snprintf(
-				xpath, XPATH_MAXLEN,
-				"./fast-reroute/level-1/lfa/tiebreaker[index='%s']/type",
-				index_str);
+			snprintf(xpath, XPATH_MAXLEN,
+				 "./fast-reroute/level-1/lfa/tiebreaker[index='%s'][type='%s']/type",
+				 index_str, type);
 			nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, type);
 		}
 	}
 	if (!level || strmatch(level, "level-2")) {
 		if (no) {
-			snprintf(
-				xpath, XPATH_MAXLEN,
-				"./fast-reroute/level-2/lfa/tiebreaker[index='%s']",
-				index_str);
+			snprintf(xpath, XPATH_MAXLEN,
+				 "./fast-reroute/level-2/lfa/tiebreaker[index='%s'][type='%s']",
+				 index_str, type);
 			nb_cli_enqueue_change(vty, xpath, NB_OP_DESTROY, NULL);
 		} else {
-			snprintf(
-				xpath, XPATH_MAXLEN,
-				"./fast-reroute/level-2/lfa/tiebreaker[index='%s']/type",
-				index_str);
+			snprintf(xpath, XPATH_MAXLEN,
+				 "./fast-reroute/level-2/lfa/tiebreaker[index='%s'][type='%s']/type",
+				 index_str, type);
 			nb_cli_enqueue_change(vty, xpath, NB_OP_CREATE, type);
 		}
 	}
diff --git a/yang/frr-isisd.yang b/yang/frr-isisd.yang
index a3e073f626..228faa4f10 100644
--- a/yang/frr-isisd.yang
+++ b/yang/frr-isisd.yang
@@ -403,7 +403,7 @@ module frr-isisd {
           "Limit backup computation up to the prefix priority.";
       }
       list tiebreaker {
-        key "index";
+        key "index type";
         unique "type";
         description
           "Configure tiebreaker for multiple backups.";