From 74576417f86a23ec8fb67479ee3b46adad7fd5a0 Mon Sep 17 00:00:00 2001 From: Olivier Dugeon Date: Thu, 14 Dec 2023 18:22:32 +0100 Subject: [PATCH 1/2] ospfd: Correct LSA parser which fulfill the TED Traffic Engineering Database (TED) is fulfill from the various LSA advertised and received by the router. To remove information on the TED, 2 mechanisms are used: i) parse TE Opaque LSA when there are flushed and ii) compare the list of prefixes advertised in the Router LSA with the list of corresponding edges and subnets contained in the TED. However, this second mechanism assumes that the Router LSA is unique and contains all prefixes of the advertised router. But, this is wrong. Prefixes could be advertised with several Router LSA. This conduct to remove edge and subnet in the TED while it should be maintained. The result is a faulty test with ospf_sr_te_topo1 topotest when server is heavy loaded. This simple patch removed deletion of edges and subnets when parsing the Router LSA and only removed them when the corresponding TE Opaque LSA is flushed. In addition, TE Opaque LSA are not flushed when OSPF ajacency goes down. This patch also correct this second problem. Signed-off-by: Olivier Dugeon (cherry picked from commit a3230323f74204f88e44e579c3ec7e8b9142bad1) --- ospfd/ospf_te.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c index 3cf39e5fb5..d203b5ef4d 100644 --- a/ospfd/ospf_te.c +++ b/ospfd/ospf_te.c @@ -980,8 +980,8 @@ static void ospf_mpls_te_nsm_change(struct ospf_neighbor *nbr, int old_state) struct ospf_interface *oi = nbr->oi; struct mpls_te_link *lp; - /* Process Neighbor only when its state is NSM Full */ - if (nbr->state != NSM_Full) + /* Process Link only when neighbor old or new state is NSM Full */ + if (nbr->state != NSM_Full && old_state != NSM_Full) return; /* Get interface information for Traffic Engineering */ @@ -1839,6 +1839,7 @@ static void ospf_te_delete_subnet(struct ls_ted *ted, struct in_addr addr) p.family = AF_INET; p.prefixlen = IPV4_MAX_BITLEN; p.u.prefix4 = addr; + ote_debug(" |- Delete Subnet info. for Prefix %pFX", &p); subnet = ls_find_subnet(ted, &p); /* Remove subnet if found */ @@ -1851,8 +1852,7 @@ static void ospf_te_delete_subnet(struct ls_ted *ted, struct in_addr addr) /** * Parse Router LSA. This function will create or update corresponding Vertex, - * Edge and Subnet. It also remove Edge and Subnet if they are marked as Orphan - * once Router LSA is parsed. + * Edge and Subnet. * * @param ted Link State Traffic Engineering Database * @param lsa OSPF Link State Advertisement @@ -1864,9 +1864,6 @@ static int ospf_te_parse_router_lsa(struct ls_ted *ted, struct ospf_lsa *lsa) struct router_lsa *rl; enum ls_node_type type; struct ls_vertex *vertex; - struct ls_edge *edge; - struct ls_subnet *subnet; - struct listnode *node; int len, links; /* Sanity Check */ @@ -1909,13 +1906,6 @@ static int ospf_te_parse_router_lsa(struct ls_ted *ted, struct ospf_lsa *lsa) vertex->status = SYNC; } - /* Mark outgoing Edge and Subnet as ORPHAN to detect deletion */ - for (ALL_LIST_ELEMENTS_RO(vertex->outgoing_edges, node, edge)) - edge->status = ORPHAN; - - for (ALL_LIST_ELEMENTS_RO(vertex->prefixes, node, subnet)) - subnet->status = ORPHAN; - /* Then, process Link Information */ len = lsa->size - OSPF_LSA_HEADER_SIZE - OSPF_ROUTER_LSA_MIN_SIZE; links = ntohs(rl->links); @@ -1948,11 +1938,6 @@ static int ospf_te_parse_router_lsa(struct ls_ted *ted, struct ospf_lsa *lsa) break; } } - /* Clean remaining Orphan Edges or Subnets */ - if (OspfMplsTE.export) - ls_vertex_clean(ted, vertex, zclient); - else - ls_vertex_clean(ted, vertex, NULL); return 0; } @@ -2405,7 +2390,10 @@ static int ospf_te_delete_te(struct ls_ted *ted, struct ospf_lsa *lsa) ote_debug(" |- Delete TE info. for Edge %pI4", &edge->attributes->standard.local); - /* Remove Link State Attributes TE information */ + /* First remove the associated Subnet */ + ospf_te_delete_subnet(ted, attr->standard.local); + + /* Then ,remove Link State Attributes TE information */ memset(&attr->standard, 0, sizeof(struct ls_standard)); attr->flags &= 0x0FFFF; memset(&attr->extended, 0, sizeof(struct ls_extended)); @@ -2420,7 +2408,6 @@ static int ospf_te_delete_te(struct ls_ted *ted, struct ospf_lsa *lsa) edge->status = SYNC; } else { /* Remove completely the Edge if Segment Routing is not set */ - ospf_te_delete_subnet(ted, attr->standard.local); edge->status = DELETE; ospf_te_export(LS_MSG_TYPE_ATTRIBUTES, edge); ls_edge_del_all(ted, edge); From 4104f3c4ce8d1134436957f6171280d3e135a152 Mon Sep 17 00:00:00 2001 From: Olivier Dugeon Date: Thu, 14 Dec 2023 18:34:38 +0100 Subject: [PATCH 2/2] tests: Update OSPF TE topotests The OSPF TE topotest is using switches to interconnect router. During the test, interfaces are shutdown on some routers to simulate link failure and check that the TED is correctly updated. However, the switche between router avoid the detection by the neighbor router that the interface is down i.e. the interface line remains up as it is conneted to the switch and not to the router. This patch update the tested topology by removing the switch and connect directly the router excepted the inter AS link on R3. Interface are also renamed accordingly. Signed-off-by: Olivier Dugeon (cherry picked from commit 55ad78575351fa90a2485b99b3665adb74712c65) --- tests/topotests/ospf_te_topo1/r1/ospfd.conf | 4 +- tests/topotests/ospf_te_topo1/r1/zebra.conf | 4 +- tests/topotests/ospf_te_topo1/r2/ospfd.conf | 8 +-- tests/topotests/ospf_te_topo1/r2/zebra.conf | 8 +-- tests/topotests/ospf_te_topo1/r3/ospfd.conf | 4 +- tests/topotests/ospf_te_topo1/r3/zebra.conf | 4 +- tests/topotests/ospf_te_topo1/r4/ospfd.conf | 2 +- tests/topotests/ospf_te_topo1/r4/zebra.conf | 2 +- .../ospf_te_topo1/test_ospf_te_topo1.py | 52 ++++++++----------- 9 files changed, 40 insertions(+), 48 deletions(-) diff --git a/tests/topotests/ospf_te_topo1/r1/ospfd.conf b/tests/topotests/ospf_te_topo1/r1/ospfd.conf index 312dd2697e..1541f3f2fc 100644 --- a/tests/topotests/ospf_te_topo1/r1/ospfd.conf +++ b/tests/topotests/ospf_te_topo1/r1/ospfd.conf @@ -2,13 +2,13 @@ interface lo ip ospf area 0.0.0.0 ! -interface r1-eth0 +interface eth0 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 ip ospf area 0.0.0.0 ! -interface r1-eth1 +interface eth1 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 diff --git a/tests/topotests/ospf_te_topo1/r1/zebra.conf b/tests/topotests/ospf_te_topo1/r1/zebra.conf index c47789a166..dc50e3dd6d 100644 --- a/tests/topotests/ospf_te_topo1/r1/zebra.conf +++ b/tests/topotests/ospf_te_topo1/r1/zebra.conf @@ -2,7 +2,7 @@ interface lo ip address 10.0.255.1/32 ! -interface r1-eth0 +interface eth0 ip address 10.0.0.1/24 link-params metric 20 @@ -12,7 +12,7 @@ interface r1-eth0 enable exit-link-params ! -interface r1-eth1 +interface eth1 ip address 10.0.1.1/24 link-params enable diff --git a/tests/topotests/ospf_te_topo1/r2/ospfd.conf b/tests/topotests/ospf_te_topo1/r2/ospfd.conf index e9c3f65bc2..acc2e6b234 100644 --- a/tests/topotests/ospf_te_topo1/r2/ospfd.conf +++ b/tests/topotests/ospf_te_topo1/r2/ospfd.conf @@ -2,25 +2,25 @@ interface lo ip ospf area 0.0.0.0 ! -interface r2-eth0 +interface eth0 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 ip ospf area 0.0.0.0 ! -interface r2-eth1 +interface eth1 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 ip ospf area 0.0.0.0 ! -interface r2-eth2 +interface eth2 ip ospf network point-to-point ip ospf area 0.0.0.0 ip ospf hello-interval 2 ip ospf dead-interval 10 ! -interface r2-eth3 +interface eth3 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 diff --git a/tests/topotests/ospf_te_topo1/r2/zebra.conf b/tests/topotests/ospf_te_topo1/r2/zebra.conf index a9771f70ca..6fe4dd0a5a 100644 --- a/tests/topotests/ospf_te_topo1/r2/zebra.conf +++ b/tests/topotests/ospf_te_topo1/r2/zebra.conf @@ -2,25 +2,25 @@ interface lo ip address 10.0.255.2/32 ! -interface r2-eth0 +interface eth0 ip address 10.0.0.2/24 link-params enable exit-link-params ! -interface r2-eth1 +interface eth1 ip address 10.0.1.2/24 link-params enable exit-link-params ! -interface r2-eth2 +interface eth2 ip address 10.0.3.2/24 link-params enable exit-link-params ! -interface r2-eth3 +interface eth3 ip address 10.0.4.2/24 link-params metric 30 diff --git a/tests/topotests/ospf_te_topo1/r3/ospfd.conf b/tests/topotests/ospf_te_topo1/r3/ospfd.conf index caa5f1e1eb..fc94437b64 100644 --- a/tests/topotests/ospf_te_topo1/r3/ospfd.conf +++ b/tests/topotests/ospf_te_topo1/r3/ospfd.conf @@ -2,13 +2,13 @@ interface lo ip ospf area 0.0.0.0 ! -interface r3-eth0 +interface eth0 ip ospf network point-to-point ip ospf area 0.0.0.0 ip ospf hello-interval 2 ip ospf dead-interval 10 ! -interface r3-eth1 +interface eth1 ip ospf network point-to-point ip ospf area 0.0.0.0 ip ospf hello-interval 2 diff --git a/tests/topotests/ospf_te_topo1/r3/zebra.conf b/tests/topotests/ospf_te_topo1/r3/zebra.conf index 4cf9077085..262b5adc92 100644 --- a/tests/topotests/ospf_te_topo1/r3/zebra.conf +++ b/tests/topotests/ospf_te_topo1/r3/zebra.conf @@ -2,14 +2,14 @@ interface lo ip address 10.0.255.3/32 ! -interface r3-eth0 +interface eth0 ip address 10.0.3.1/24 link-params enable admin-grp 0x20 exit-link-params ! -interface r3-eth1 +interface eth1 ip address 10.0.5.1/24 link-params enable diff --git a/tests/topotests/ospf_te_topo1/r4/ospfd.conf b/tests/topotests/ospf_te_topo1/r4/ospfd.conf index cd508017d3..5ec7918aff 100644 --- a/tests/topotests/ospf_te_topo1/r4/ospfd.conf +++ b/tests/topotests/ospf_te_topo1/r4/ospfd.conf @@ -2,7 +2,7 @@ interface lo ip ospf area 0.0.0.0 ! -interface r4-eth0 +interface eth0 ip ospf network point-to-point ip ospf hello-interval 2 ip ospf dead-interval 10 diff --git a/tests/topotests/ospf_te_topo1/r4/zebra.conf b/tests/topotests/ospf_te_topo1/r4/zebra.conf index 18c003b230..c482685094 100644 --- a/tests/topotests/ospf_te_topo1/r4/zebra.conf +++ b/tests/topotests/ospf_te_topo1/r4/zebra.conf @@ -2,7 +2,7 @@ interface lo ip address 10.0.255.4/32 ! -interface r4-eth0 +interface eth0 ip address 10.0.4.1/24 link-params enable diff --git a/tests/topotests/ospf_te_topo1/test_ospf_te_topo1.py b/tests/topotests/ospf_te_topo1/test_ospf_te_topo1.py index c8533cfbba..11fecc3211 100644 --- a/tests/topotests/ospf_te_topo1/test_ospf_te_topo1.py +++ b/tests/topotests/ospf_te_topo1/test_ospf_te_topo1.py @@ -18,22 +18,22 @@ test_ospf_te_topo1.py: Test the FRR OSPF with Traffic Engineering. | 10.0.225.1 | | | +------------+ - r1-eth0| |r1-eth1 + eth0| |eth1 | | 10.0.0.0/24| |10.0.1.0/24 | | - r2-eth0| |r2-eth1 + eth0| |eth1 +------------+ +------------+ | | | | - | R2 |r2-eth2 r3-eth0| R3 | + | R2 |eth2 eth0| R3 | | 10.0.255.2 +------------------+ 10.0.255.3 | | | 10.0.3.0/24 | | +------------+ +------+-----+ - r2-eth3| r3-eth1| + eth3| eth1| | | 10.0.4.0/24| 10.0.5.0/24| | | - r4-eth0| V + eth0| V +------------+ ASBR 10.0.255.5 | | | R4 | @@ -70,30 +70,24 @@ def build_topo(tgen): "Build function" # Create 4 routers - for routern in range(1, 5): - tgen.add_router("r{}".format(routern)) + r1 = tgen.add_router("r1") + r2 = tgen.add_router("r2") + r3 = tgen.add_router("r3") + r4 = tgen.add_router("r4") # Interconect router 1 and 2 with 2 links - switch = tgen.add_switch("s1") - switch.add_link(tgen.gears["r1"]) - switch.add_link(tgen.gears["r2"]) - switch = tgen.add_switch("s2") - switch.add_link(tgen.gears["r1"]) - switch.add_link(tgen.gears["r2"]) + tgen.add_link(r1, r2, ifname1="eth0", ifname2="eth0") + tgen.add_link(r1, r2, ifname1="eth1", ifname2="eth1") # Interconect router 3 and 2 - switch = tgen.add_switch("s3") - switch.add_link(tgen.gears["r3"]) - switch.add_link(tgen.gears["r2"]) + tgen.add_link(r2, r3, ifname1="eth2", ifname2="eth0") # Interconect router 4 and 2 - switch = tgen.add_switch("s4") - switch.add_link(tgen.gears["r4"]) - switch.add_link(tgen.gears["r2"]) + tgen.add_link(r2, r4, ifname1="eth3", ifname2="eth0") # Interconnect router 3 with next AS - switch = tgen.add_switch("s5") - switch.add_link(tgen.gears["r3"]) + s1 = tgen.add_switch("s1") + tgen.add_link(r3, s1, ifname1="eth1", ifname2="eth0") def setup_module(mod): @@ -174,8 +168,7 @@ def test_step2(): tgen = setup_testcase("Step2: Shutdown interface between r1 & r2") - tgen.net["r1"].cmd('vtysh -c "conf t" -c "interface r1-eth1" -c "shutdown"') - tgen.net["r2"].cmd('vtysh -c "conf t" -c "interface r2-eth1" -c "shutdown"') + tgen.net["r1"].cmd('vtysh -c "conf t" -c "interface eth1" -c "shutdown"') for rname in ["r1", "r2", "r3", "r4"]: compare_ted_json_output(tgen, rname, "ted_step2.json") @@ -227,28 +220,27 @@ def test_step5(): tgen = setup_testcase("Step5: Re-enable interface between r1 & r2") - tgen.net["r1"].cmd('vtysh -c "conf t" -c "interface r1-eth1" -c "no shutdown"') - tgen.net["r2"].cmd('vtysh -c "conf t" -c "interface r2-eth1" -c "no shutdown"') + tgen.net["r1"].cmd('vtysh -c "conf t" -c "interface eth1" -c "no shutdown"') for rname in ["r1", "r2", "r3", "r4"]: compare_ted_json_output(tgen, rname, "ted_step5.json") def test_step6(): - "Step6: Set delay and jitter for interface r4-eth0 on r4, remove use-bw \ - for interface r2-eth3 on r2 and verify that corresponding Edges are \ + "Step6: Set delay and jitter for interface eth0 on r4, remove use-bw \ + for interface eth3 on r2 and verify that corresponding Edges are \ updated in the TED on all routers" tgen = setup_testcase("Step6: Modify link parameters on r2 & r4") tgen.net["r2"].cmd( - 'vtysh -c "conf t" -c "interface r2-eth3" -c "link-params" -c "no use-bw"' + 'vtysh -c "conf t" -c "interface eth3" -c "link-params" -c "no use-bw"' ) tgen.net["r4"].cmd( - 'vtysh -c "conf t" -c "interface r4-eth0" -c "link-params" -c "delay 20000"' + 'vtysh -c "conf t" -c "interface eth0" -c "link-params" -c "delay 20000"' ) tgen.net["r4"].cmd( - 'vtysh -c "conf t" -c "interface r4-eth0" -c "link-params" -c "delay-variation 10000"' + 'vtysh -c "conf t" -c "interface eth0" -c "link-params" -c "delay-variation 10000"' ) for rname in ["r1", "r2", "r3", "r4"]: