Skip to content

Commit c2a8376

Browse files
zhou-runzice312963205
authored andcommitted
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 () \FRRouting#1 0xb79f62f7 in ?? () \FRRouting#2 0xbf981dd0 in ?? () \FRRouting#3 <signal handler called> \FRRouting#4 0xb79f7722 in ?? () \FRRouting#5 0xb7ed8634 in _DYNAMIC () from /home/z15467/isis_core/usr/lib/i386-linux-gnu/frr/libfrr.so.0.0.0 \FRRouting#6 0x0001003c in ?? () \FRRouting#7 0x00010000 in ?? () \FRRouting#8 0xb7df3322 in _frr_mtx_lock (mutex=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/frr_pthread.h:255 \FRRouting#9 event_timer_remain_msec (thread=0x10000) at ../lib/event.c:734 \FRRouting#10 event_timer_remain_msec (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:727 \FRRouting#11 0x004fb4aa in _send_hello_sched (circuit=<optimized out>, threadp=0x2189de0, level=1, delay=<optimized out>) at ../isisd/isis_pdu.c:2116 \FRRouting#12 0x004e8dbc in isis_circuit_up (circuit=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../isisd/isis_circuit.c:734 \FRRouting#13 0x004ea8f7 in isis_csm_state_change (event=<optimized out>, circuit=<optimized out>, arg=<optimized out>) at ../isisd/isis_csm.c:98 \FRRouting#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 \FRRouting#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 <[email protected]>
1 parent 5b88727 commit c2a8376

File tree

2 files changed

+163
-1
lines changed

2 files changed

+163
-1
lines changed

isisd/isis_pdu.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,8 @@ static int process_p2p_hello(struct iih_info *iih)
231231
return ISIS_OK;
232232
}
233233
}
234-
if (!adj || adj->level != iih->calculated_type) {
234+
if (!adj || adj->level != iih->calculated_type ||
235+
!(iih->circuit->is_type & iih->circ_type)) {
235236
if (!adj) {
236237
adj = isis_new_adj(iih->sys_id, NULL,
237238
iih->calculated_type, iih->circuit);

tests/topotests/isis_topo1/test_isis_topo1.py

+161
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"""
1313
test_isis_topo1.py: Test ISIS topology.
1414
"""
15+
import time
1516
import datetime
1617
import functools
1718
import json
@@ -314,6 +315,107 @@ def test_isis_neighbor_json():
314315
), assertmsg
315316

316317

318+
def test_isis_neighbor_state():
319+
"Check that the neighbor states remain normal when the ISIS type is switched."
320+
321+
tgen = get_topogen()
322+
# Don't run this test if we have any failure.
323+
if tgen.routers_have_failure():
324+
pytest.skip(tgen.errors)
325+
326+
logger.info("Checking 'show isis neighbor state on a p2p link'")
327+
328+
# Establish a P2P link
329+
# 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,
330+
# it is expected that all neighbors exist and are in the Up state
331+
r3 = tgen.gears["r3"]
332+
r3.vtysh_cmd(
333+
"""
334+
configure
335+
router isis 1
336+
no redistribute ipv4 connected level-1
337+
no redistribute ipv4 connected level-2
338+
no redistribute ipv6 connected level-1
339+
no redistribute ipv6 connected level-2
340+
interface r3-eth1
341+
no isis circuit-type
342+
isis network point-to-point
343+
end
344+
"""
345+
)
346+
r5 = tgen.gears["r5"]
347+
r5.vtysh_cmd(
348+
"""
349+
configure
350+
router isis 1
351+
no redistribute ipv4 connected level-1
352+
no redistribute ipv6 connected level-1
353+
no redistribute ipv4 table 20 level-1
354+
interface r5-eth0
355+
no isis circuit-type
356+
isis network point-to-point
357+
end
358+
"""
359+
)
360+
result = _check_isis_neighbor_json("r3", "r5", True, "Up")
361+
assert result is True, result
362+
result = _check_isis_neighbor_json("r5", "r3", True, "Up")
363+
assert result is True, result
364+
365+
# Remove the configuration that affects the switch of IS-IS type.
366+
# Configure the IS-IS type of r3 to transition from level-1-2 to level-2-only,
367+
# while maintaining the IS-IS type of r5 as level-1.
368+
# In this scenario,
369+
# the expectation is that some neighbors do not exist or are in the Initializing state
370+
r3.vtysh_cmd(
371+
"""
372+
configure
373+
router isis 1
374+
is-type level-2-only
375+
end
376+
"""
377+
)
378+
result = _check_isis_neighbor_json("r3", "r5", False, "Initializing")
379+
assert result is True, result
380+
result = _check_isis_neighbor_json("r5", "r3", False, "Initializing")
381+
assert result is True, result
382+
383+
# Restore to initial configuration
384+
logger.info("Checking 'restore to initial configuration'")
385+
r3.vtysh_cmd(
386+
"""
387+
configure
388+
interface r3-eth1
389+
isis circuit-type level-1
390+
no isis network point-to-point
391+
router isis 1
392+
no is-type
393+
redistribute ipv4 connected level-1
394+
redistribute ipv4 connected level-2
395+
redistribute ipv6 connected level-1
396+
redistribute ipv6 connected level-2
397+
end
398+
"""
399+
)
400+
r5.vtysh_cmd(
401+
"""
402+
configure
403+
interface r5-eth0
404+
isis circuit-type level-1
405+
no isis network point-to-point
406+
router isis 1
407+
redistribute ipv4 connected level-1
408+
redistribute ipv6 connected level-1
409+
redistribute ipv4 table 20 level-1
410+
end
411+
"""
412+
)
413+
result = _check_isis_neighbor_json("r3", "r5", True, "Up")
414+
assert result is True, result
415+
result = _check_isis_neighbor_json("r5", "r3", True, "Up")
416+
assert result is True, result
417+
418+
317419
def test_isis_database_json():
318420
"Check json struct in show isis database json"
319421

@@ -623,6 +725,65 @@ def test_isis_hello_padding_during_adjacency_formation():
623725
assert result is True, result
624726

625727

728+
def _check_isis_neighbor_json(
729+
self, neighbor, neighbor_expected, neighbor_state_expected
730+
):
731+
tgen = get_topogen()
732+
router = tgen.gears[self]
733+
logger.info(
734+
f"check_isis_neighbor_json {router} {neighbor} {neighbor_expected} {neighbor_state_expected}"
735+
)
736+
737+
result = _check_isis_neighbor_exist(self, neighbor)
738+
if result == True:
739+
return _check_isis_neighbor_state(self, neighbor, neighbor_state_expected)
740+
elif neighbor_expected == True:
741+
return "{} with expected neighbor {} got none ".format(router.name, neighbor)
742+
else:
743+
return True
744+
745+
746+
@retry(retry_timeout=60)
747+
def _check_isis_neighbor_exist(self, neighbor):
748+
tgen = get_topogen()
749+
router = tgen.gears[self]
750+
logger.info(f"check_isis_neighbor_exist {router} {neighbor}")
751+
neighbor_json = router.vtysh_cmd("show isis neighbor json", isjson=True)
752+
753+
circuits = neighbor_json.get("areas", [])[0].get("circuits", [])
754+
for circuit in circuits:
755+
if "adj" in circuit and circuit["adj"] == neighbor:
756+
return True
757+
758+
return "The neighbor {} of router {} has not been learned yet ".format(
759+
neighbor, router.name
760+
)
761+
762+
763+
@retry(retry_timeout=5)
764+
def _check_isis_neighbor_state(self, neighbor, neighbor_state_expected):
765+
tgen = get_topogen()
766+
router = tgen.gears[self]
767+
logger.info(
768+
f"check_isis_neighbor_state {router} {neighbor} {neighbor_state_expected}"
769+
)
770+
neighbor_json = router.vtysh_cmd(
771+
"show isis neighbor {} json".format(neighbor), isjson=True
772+
)
773+
774+
circuits = neighbor_json.get("areas", [])[0].get("circuits", [])
775+
for circuit in circuits:
776+
interface = circuit.get("interface", {})
777+
if "state" in interface:
778+
neighbor_state = interface["state"]
779+
if neighbor_state == neighbor_state_expected:
780+
return True
781+
782+
return "{} peer with expected neighbor_state {} got {} ".format(
783+
router.name, neighbor_state_expected, neighbor_state
784+
)
785+
786+
626787
@retry(retry_timeout=10)
627788
def check_last_iih_packet_for_padding(router, expect_padding):
628789
logfilename = "{}/{}".format(router.gearlogdir, "isisd.log")

0 commit comments

Comments
 (0)