isisd: When the ISIS types of the routers do not match on a P2P link,…

… the neighbor status remains UP Test Scenario: RouterA and RouterB are in the same routing domain and have configured a P2P link. RouterA is configured with "is-type level-1" while RouterB is configured with "is-type level-1-2". They establish a level-1 UP neighborship. In this scenario, we expect that when RouterB's configuration is switched to "is-type level-2-only", the neighborship status on both RouterA and RouterB would be non-UP. However, RouterB still shows the neighbor as UP. Upon receiving a P2P Hello packet, the function "process_p2p_hello" is invoked. According to the ISO/IEC 10589 protocol specification, section 8.2.5.2 a) and tables 5 and 7, if the "iih->circ_type" of the neighbor's hello packet does not match one's own "circuit->is_type," we may choose to take no action. When establishing a neighborship for the first time, the neighbor's status can remain in the "Initializing" state. However, if the neighborship has already been established and one's own "circuit->is_type" changes, the neighbor's UP status cannot be reset. Therefore, when processing P2P Hello packets, we should be cognizant of changes in our own link adjacency type. Topotest has identified a core issue during testing. (gdb) bt "#0 0xb7efe579 in __kernel_vsyscall () \#1 0xb79f62f7 in ?? () \#2 0xbf981dd0 in ?? () \#3 <signal handler called> \#4 0xb79f7722 in ?? () \#5 0xb7ed8634 in _DYNAMIC () from /home/z15467/isis_core/usr/lib/i386-linux-gnu/frr/libfrr.so.0.0.0 \#6 0x0001003c in ?? () \#7 0x00010000 in ?? () \#8 0xb7df3322 in _frr_mtx_lock (mutex=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/frr_pthread.h:255 \#9 event_timer_remain_msec (thread=0x10000) at ../lib/event.c:734 \#10 event_timer_remain_msec (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:727 \#11 0x004fb4aa in _send_hello_sched (circuit=<optimized out>, threadp=0x2189de0, level=1, delay=<optimized out>) at ../isisd/isis_pdu.c:2116 \#12 0x004e8dbc in isis_circuit_up (circuit=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_circuit.c:734 \#13 0x004ea8f7 in isis_csm_state_change (event=<optimized out>, circuit=<optimized out>, arg=<optimized out>) at ../isisd/isis_csm.c:98 \#14 0x004ea23f in isis_circuit_circ_type_set (circuit=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, circ_type=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_circuit.c:1578 \FRRouting#15 0x0053aefa in lib_interface_isis_network_type_modify (args=<optimized out>) at ../isisd/isis_nb_config.c:4190 \FRRouting#16 0xb7dbcc8d in nb_callback_modify (errmsg_len=8192, errmsg=0xbf982afc "", resource=0x2186220, dnode=<optimized out>, event=NB_EV_APPLY, nb_node=0x1fafe70, context=<optimized out>) at ../lib/northbound.c:1550 \FRRouting#17 nb_callback_configuration (context=<optimized out>, event=NB_EV_APPLY, change=<optimized out>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1900 \FRRouting#18 0xb7dbd646 in nb_transaction_process (errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, event=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:2028 \FRRouting#19 nb_candidate_commit_apply (transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, save_transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction_id=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1368 \FRRouting#20 0xb7dbdd68 in nb_candidate_commit (context=..., candidate=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, save_transaction=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, comment=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, transaction_id=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, errmsg_len=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/northbound.c:1401 \FRRouting#21 0xb7dc0cff in nb_cli_classic_commit (vty=vty@entry=0x21d6940) at ../lib/northbound_cli.c:57 \FRRouting#22 0xb7dc0f46 in nb_cli_apply_changes_internal (vty=vty@entry=0x21d6940, xpath_base=xpath_base@entry=0xbf986b7c "/frr-interface:lib/interface[name='r5-eth0']", clear_pending=clear_pending@entry=false) at ../lib/northbound_cli.c:184 \FRRouting#23 0xb7dc130b in nb_cli_apply_changes (vty=<optimized out>, xpath_base_fmt=<optimized out>) at ../lib/northbound_cli.c:240 \FRRouting#24 0x00542c1d in isis_network_magic (self=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argc=<optimized out>, argv=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, no=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_cli.c:3101 \FRRouting#25 isis_network (self=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argc=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, argv=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ./isisd/isis_cli_clippy.c:5499 \FRRouting#26 0xb7d6d8f1 in cmd_execute_command_real (vline=vline@entry=0x219afa0, vty=vty@entry=0x21d6940, cmd=cmd@entry=0x0, up_level=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1003 \FRRouting#27 0xb7d6d9e0 in cmd_execute_command (vline=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, cmd=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vtysh=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1061 \FRRouting#28 0xb7d6dc60 in cmd_execute (vty=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, cmd=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, matched=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>, vtysh=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/command.c:1228 \FRRouting#29 0xb7dfb58a in vty_command (vty=vty@entry=0x21d6940, buf=0x21e0ff0 ' ' <repeats 12 times>, "isis network point-to-point") at ../lib/vty.c:625 \FRRouting#30 0xb7dfc560 in vty_execute (vty=vty@entry=0x21d6940) at ../lib/vty.c:1388 \FRRouting#31 0xb7dfdc8d in vtysh_read (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/vty.c:2400 \FRRouting#32 0xb7df4d47 in event_call (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:2019 \FRRouting#33 0xb7d9a831 in frr_run (master=<optimized out>) at ../lib/libfrr.c:1232 \FRRouting#34 0x004e4758 in main (argc=7, argv=0xbf989a24, envp=0xbf989a44) at ../isisd/isis_main.c:354 (gdb) f 9 \#9 event_timer_remain_msec (thread=0x10000) at ../lib/event.c:734 734 ../lib/event.c: No such file or directory. (gdb) p pthread No symbol "pthread" in current context. (gdb) p thread $1 = (struct event *) 0x10000 When LAN links and P2P links share the` circuit->u` of a neighbor, if one link is no longer in use and the union is not cleared, the other link is unable to pass the non-empty check, resulting in accessing an invalid pointer. Unfortunately, for non-DIS devices in LAN links, `circuit->u.bc.run_dr_elect[x]` is essentially always 1, but in `isis_circuit_down()`,` circuit->u.bc.run_dr_elect[x] `will not be cleared because `circuit->u.bc.is_dr[x]` is always 0. Consequently, when switching to a P2P link, `isis_circuit_circ_type_set()` does not reset the link in a non-C_STATE_UP state, leading to subsequent accesses of `circuit->u.p2p.t_send_p2p_hello` resulting in a non-empty yet invalid address. I believe that in `isis_circuit_down()`, the LAN link should unconditionally clear `circuit->u.bc.run_dr_elect[x]`. Signed-off-by: zhou-run <zhou.run@h3c.com>
zhou-run · Nov 9, 2024 · a6cffa8 · a6cffa8
1 parent 91e157f
commit a6cffa8
Show file tree

Hide file tree

Showing 3 changed files with 165 additions and 1 deletion.
diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c
@@ -851,11 +851,13 @@ void isis_circuit_down(struct isis_circuit *circuit)
 			isis_dr_resign(circuit, 1);
 			circuit->u.bc.is_dr[0] = 0;
 		}
+		circuit->u.bc.run_dr_elect[0] = 0;
 		memset(circuit->u.bc.l1_desig_is, 0, ISIS_SYS_ID_LEN + 1);
 		if (circuit->u.bc.is_dr[1]) {
 			isis_dr_resign(circuit, 2);
 			circuit->u.bc.is_dr[1] = 0;
 		}
+		circuit->u.bc.run_dr_elect[1] = 0;
 		memset(circuit->u.bc.l2_desig_is, 0, ISIS_SYS_ID_LEN + 1);
 		memset(circuit->u.bc.snpa, 0, ETH_ALEN);
 

diff --git a/isisd/isis_pdu.c b/isisd/isis_pdu.c
@@ -231,7 +231,8 @@ static int process_p2p_hello(struct iih_info *iih)
 			return ISIS_OK;
 		}
 	}
-	if (!adj || adj->level != iih->calculated_type) {
+	if (!adj || adj->level != iih->calculated_type ||
+	    !(iih->circuit->is_type & iih->circ_type)) {
 		if (!adj) {
 			adj = isis_new_adj(iih->sys_id, NULL,
 					   iih->calculated_type, iih->circuit);

diff --git a/tests/topotests/isis_topo1/test_isis_topo1.py b/tests/topotests/isis_topo1/test_isis_topo1.py
@@ -12,6 +12,7 @@
 """
 test_isis_topo1.py: Test ISIS topology.
 """
+import time
 import datetime
 import functools
 import json
@@ -314,6 +315,107 @@ def test_isis_neighbor_json():
         ), assertmsg
 
 
+def test_isis_neighbor_state():
+    "Check that the neighbor states remain normal when the ISIS type is switched."
+
+    tgen = get_topogen()
+    # Don't run this test if we have any failure.
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    logger.info("Checking 'show isis neighbor state on a p2p link'")
+
+    # Establish a P2P link
+    # When the IS-IS type of r3 is set to level-1-2 and the IS-IS type of r5 is set to level-1,
+    # it is expected that all neighbors exist and are in the Up state
+    r3 = tgen.gears["r3"]
+    r3.vtysh_cmd(
+        """
+        configure
+        router isis 1
+            no redistribute ipv4 connected level-1
+            no redistribute ipv4 connected level-2
+            no redistribute ipv6 connected level-1
+            no redistribute ipv6 connected level-2
+        interface r3-eth1
+            no isis circuit-type
+            isis network point-to-point
+        end
+        """
+    )
+    r5 = tgen.gears["r5"]
+    r5.vtysh_cmd(
+        """
+        configure
+        router isis 1
+            no redistribute ipv4 connected level-1
+            no redistribute ipv6 connected level-1
+            no redistribute ipv4 table 20 level-1
+        interface r5-eth0
+            no isis circuit-type
+            isis network point-to-point
+        end
+        """
+    )
+    result = _check_isis_neighbor_json("r3", "r5", True, "Up")
+    assert result is True, result
+    result = _check_isis_neighbor_json("r5", "r3", True, "Up")
+    assert result is True, result
+
+    # Remove the configuration that affects the switch of IS-IS type.
+    # Configure the IS-IS type of r3 to transition from level-1-2 to level-2-only,
+    # while maintaining the IS-IS type of r5 as level-1.
+    # In this scenario,
+    # the expectation is that some neighbors do not exist or are in the Initializing state
+    r3.vtysh_cmd(
+        """
+        configure
+        router isis 1
+            is-type level-2-only
+        end
+        """
+    )
+    result = _check_isis_neighbor_json("r3", "r5", False, "Initializing")
+    assert result is True, result
+    result = _check_isis_neighbor_json("r5", "r3", False, "Initializing")
+    assert result is True, result
+
+    # Restore to initial configuration
+    logger.info("Checking 'restore to initial configuration'")
+    r3.vtysh_cmd(
+        """
+        configure
+        interface r3-eth1
+            isis circuit-type level-1
+            no isis network point-to-point
+        router isis 1
+            no is-type
+            redistribute ipv4 connected level-1
+            redistribute ipv4 connected level-2
+            redistribute ipv6 connected level-1
+            redistribute ipv6 connected level-2
+        end
+        """
+    )
+    r5.vtysh_cmd(
+        """
+        configure
+        interface r5-eth0
+            isis circuit-type level-1
+            no isis network point-to-point
+        router isis 1
+            redistribute ipv4 connected level-1
+            redistribute ipv6 connected level-1
+            redistribute ipv4 table 20 level-1
+        end
+        """
+    )
+    result = _check_isis_neighbor_json("r3", "r5", True, "Up")
+    assert result is True, result
+    result = _check_isis_neighbor_json("r5", "r3", True, "Up")
+    assert result is True, result
+
+
 def test_isis_database_json():
     "Check json struct in show isis database json"
 
@@ -623,6 +725,65 @@ def test_isis_hello_padding_during_adjacency_formation():
     assert result is True, result
 
 
+def _check_isis_neighbor_json(
+    self, neighbor, neighbor_expected, neighbor_state_expected
+):
+    tgen = get_topogen()
+    router = tgen.gears[self]
+    logger.info(
+        f"check_isis_neighbor_json {router} {neighbor} {neighbor_expected} {neighbor_state_expected}"
+    )
+
+    result = _check_isis_neighbor_exist(self, neighbor)
+    if result == True:
+        return _check_isis_neighbor_state(self, neighbor, neighbor_state_expected)
+    elif neighbor_expected == True:
+        return "{} with expected neighbor {} got none ".format(router.name, neighbor)
+    else:
+        return True
+
+
+@retry(retry_timeout=60)
+def _check_isis_neighbor_exist(self, neighbor):
+    tgen = get_topogen()
+    router = tgen.gears[self]
+    logger.info(f"check_isis_neighbor_exist {router} {neighbor}")
+    neighbor_json = router.vtysh_cmd("show isis neighbor json", isjson=True)
+
+    circuits = neighbor_json.get("areas", [])[0].get("circuits", [])
+    for circuit in circuits:
+        if "adj" in circuit and circuit["adj"] == neighbor:
+            return True
+
+    return "The neighbor {} of router {} has not been learned yet ".format(
+        neighbor, router.name
+    )
+
+
+@retry(retry_timeout=5)
+def _check_isis_neighbor_state(self, neighbor, neighbor_state_expected):
+    tgen = get_topogen()
+    router = tgen.gears[self]
+    logger.info(
+        f"check_isis_neighbor_state {router} {neighbor} {neighbor_state_expected}"
+    )
+    neighbor_json = router.vtysh_cmd(
+        "show isis neighbor {} json".format(neighbor), isjson=True
+    )
+
+    circuits = neighbor_json.get("areas", [])[0].get("circuits", [])
+    for circuit in circuits:
+        interface = circuit.get("interface", {})
+        if "state" in interface:
+            neighbor_state = interface["state"]
+            if neighbor_state == neighbor_state_expected:
+                return True
+
+    return "{} peer with expected neighbor_state {} got {} ".format(
+        router.name, neighbor_state_expected, neighbor_state
+    )
+
+
 @retry(retry_timeout=10)
 def check_last_iih_packet_for_padding(router, expect_padding):
     logfilename = "{}/{}".format(router.gearlogdir, "isisd.log")