matthieu/frr
1
0
Fork 0
forked from Mirror/frr
Code Pull requests Activity

isisd: When the ISIS types of the routers do not match on a P2P link, the neighbor status remains UP

Browse source 
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
\#15 0x0053aefa in lib_interface_isis_network_type_modify (args=<optimized out>) at ../isisd/isis_nb_config.c:4190
\#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
\#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
\#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
\#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
\#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
\#21 0xb7dc0cff in nb_cli_classic_commit (vty=vty@entry=0x21d6940) at ../lib/northbound_cli.c:57
\#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
\#23 0xb7dc130b in nb_cli_apply_changes (vty=<optimized out>, xpath_base_fmt=<optimized out>) at ../lib/northbound_cli.c:240
\#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
\#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
\#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
\#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
\#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
\#29 0xb7dfb58a in vty_command (vty=vty@entry=0x21d6940, buf=0x21e0ff0 ' ' <repeats 12 times>, "isis network point-to-point") at ../lib/vty.c:625
\#30 0xb7dfc560 in vty_execute (vty=vty@entry=0x21d6940) at ../lib/vty.c:1388
\#31 0xb7dfdc8d in vtysh_read (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/vty.c:2400
\#32 0xb7df4d47 in event_call (thread=<error reading variable: dwarf2_find_location_expression: Corrupted DWARF expression.>) at ../lib/event.c:2019
\#33 0xb7d9a831 in frr_run (master=<optimized out>) at ../lib/libfrr.c:1232
\#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>
This commit is contained in:
zhou-run 2024-10-24 11:09:37 +08:00 committed by zhourun (RD)
parent 91e157f3ae
commit a6cffa842b
3 changed files with 165 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
isisd/isis_circuit.c
View file

@ -851,11 +851,13 @@ void isis_circuit_down(struct isis_circuit *circuit)
isis_dr_resign(circuit, 1); isis_dr_resign(circuit, 1);
circuit->u.bc.is_dr[0] = 0; 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); memset(circuit->u.bc.l1_desig_is, 0, ISIS_SYS_ID_LEN + 1);
if (circuit->u.bc.is_dr[1]) { if (circuit->u.bc.is_dr[1]) {
isis_dr_resign(circuit, 2); isis_dr_resign(circuit, 2);
circuit->u.bc.is_dr[1] = 0; 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.l2_desig_is, 0, ISIS_SYS_ID_LEN + 1);
memset(circuit->u.bc.snpa, 0, ETH_ALEN); memset(circuit->u.bc.snpa, 0, ETH_ALEN);

3
isisd/isis_pdu.c
View file

@ -231,7 +231,8 @@ static int process_p2p_hello(struct iih_info *iih)
return ISIS_OK; 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) { if (!adj) {
adj = isis_new_adj(iih->sys_id, NULL, adj = isis_new_adj(iih->sys_id, NULL,
iih->calculated_type, iih->circuit); iih->calculated_type, iih->circuit);

161
tests/topotests/isis_topo1/test_isis_topo1.py
View file

@ -12,6 +12,7 @@
""" """
test_isis_topo1.py: Test ISIS topology. test_isis_topo1.py: Test ISIS topology.
""" """
import time
import datetime import datetime
import functools import functools
import json import json
@ -314,6 +315,107 @@ def test_isis_neighbor_json():
), assertmsg ), 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(): def test_isis_database_json():
"Check json struct in show 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 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) @retry(retry_timeout=10)
def check_last_iih_packet_for_padding(router, expect_padding): def check_last_iih_packet_for_padding(router, expect_padding):
logfilename = "{}/{}".format(router.gearlogdir, "isisd.log") logfilename = "{}/{}".format(router.gearlogdir, "isisd.log")