From 5edc2cf3eadccb4e592fa3288136165ec9246ed9 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 10 Feb 2019 18:19:35 -0500 Subject: [PATCH 1/4] eigrpd: Remove unnecessary test for pointer If we are inside the for loop then eigrp *must* be valid. Signed-off-by: Donald Sharp --- eigrpd/eigrp_network.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eigrpd/eigrp_network.c b/eigrpd/eigrp_network.c index 6bb619f0e1..944bf6f55c 100644 --- a/eigrpd/eigrp_network.c +++ b/eigrpd/eigrp_network.c @@ -293,7 +293,7 @@ void eigrp_if_update(struct interface *ifp) */ for (ALL_LIST_ELEMENTS(eigrp_om->eigrp, node, nnode, eigrp)) { /* EIGRP must be on and Router-ID must be configured. */ - if (!eigrp || eigrp->router_id.s_addr == 0) + if (eigrp->router_id.s_addr == 0) continue; /* Run each network for this interface. */ From b245781a6bc53a28da5a0fc3c2a7b2d8954f0b04 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 11 Feb 2019 07:16:35 -0500 Subject: [PATCH 2/4] eigrp: Make the eigrp_interface have a prefix instead of a *prefix The prefix data structure was being freed yet still needed in the future and it's a fundamental part of the eigrp_interface data structure let's keep it there instead of having it be deleted and then not. Signed-off-by: Donald Sharp --- eigrpd/eigrp_dump.c | 2 +- eigrpd/eigrp_hello.c | 2 +- eigrpd/eigrp_interface.c | 12 ++++++------ eigrpd/eigrp_network.c | 2 +- eigrpd/eigrp_packet.c | 14 +++++++------- eigrpd/eigrp_structs.h | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/eigrpd/eigrp_dump.c b/eigrpd/eigrp_dump.c index 6033290914..583db6622d 100644 --- a/eigrpd/eigrp_dump.c +++ b/eigrpd/eigrp_dump.c @@ -174,7 +174,7 @@ const char *eigrp_if_ip_string(struct eigrp_interface *ei) if (!ei) return "inactive"; - ifaddr = ntohl(ei->address->u.prefix4.s_addr); + ifaddr = ntohl(ei->address.u.prefix4.s_addr); snprintf(buf, EIGRP_IF_STRING_MAXLEN, "%u.%u.%u.%u", (ifaddr >> 24) & 0xff, (ifaddr >> 16) & 0xff, (ifaddr >> 8) & 0xff, ifaddr & 0xff); diff --git a/eigrpd/eigrp_hello.c b/eigrpd/eigrp_hello.c index 413a35f2fa..b4d850be08 100644 --- a/eigrpd/eigrp_hello.c +++ b/eigrpd/eigrp_hello.c @@ -248,7 +248,7 @@ static void eigrp_peer_termination_decode(struct eigrp_neighbor *nbr, struct TLV_Peer_Termination_type *param = (struct TLV_Peer_Termination_type *)tlv; - uint32_t my_ip = nbr->ei->address->u.prefix4.s_addr; + uint32_t my_ip = nbr->ei->address.u.prefix4.s_addr; uint32_t received_ip = param->neighbor_ip; if (my_ip == received_ip) { diff --git a/eigrpd/eigrp_interface.c b/eigrpd/eigrp_interface.c index 4ad1005f2f..c52a98ee25 100644 --- a/eigrpd/eigrp_interface.c +++ b/eigrpd/eigrp_interface.c @@ -68,7 +68,7 @@ struct eigrp_interface *eigrp_if_new(struct eigrp *eigrp, struct interface *ifp, /* Set zebra interface pointer. */ ei->ifp = ifp; - ei->address = p; + prefix_copy(&ei->address, p); ifp->info = ei; listnode_add(eigrp->eiflist, ei); @@ -185,7 +185,7 @@ int eigrp_if_up(struct eigrp_interface *ei) struct prefix dest_addr; - dest_addr = *ei->address; + dest_addr = ei->address; apply_mask(&dest_addr); pe = eigrp_topology_table_lookup_ipv4(eigrp->topology_table, &dest_addr); @@ -292,7 +292,7 @@ void eigrp_if_set_multicast(struct eigrp_interface *ei) /* The interface should belong to the EIGRP-all-routers group. */ if (!ei->member_allrouters - && (eigrp_if_add_allspfrouters(ei->eigrp, ei->address, + && (eigrp_if_add_allspfrouters(ei->eigrp, &ei->address, ei->ifp->ifindex) >= 0)) /* Set the flag only if the system call to join @@ -303,7 +303,7 @@ void eigrp_if_set_multicast(struct eigrp_interface *ei) * group. */ if (ei->member_allrouters) { /* Only actually drop if this is the last reference */ - eigrp_if_drop_allspfrouters(ei->eigrp, ei->address, + eigrp_if_drop_allspfrouters(ei->eigrp, &ei->address, ei->ifp->ifindex); /* Unset the flag regardless of whether the system call to leave @@ -339,7 +339,7 @@ void eigrp_if_free(struct eigrp_interface *ei, int source) eigrp_hello_send(ei, EIGRP_HELLO_GRACEFUL_SHUTDOWN, NULL); } - dest_addr = *ei->address; + dest_addr = ei->address; apply_mask(&dest_addr); pe = eigrp_topology_table_lookup_ipv4(eigrp->topology_table, &dest_addr); @@ -375,7 +375,7 @@ struct eigrp_interface *eigrp_if_lookup_by_local_addr(struct eigrp *eigrp, if (ifp && ei->ifp != ifp) continue; - if (IPV4_ADDR_SAME(&address, &ei->address->u.prefix4)) + if (IPV4_ADDR_SAME(&address, &ei->address.u.prefix4)) return ei; } diff --git a/eigrpd/eigrp_network.c b/eigrpd/eigrp_network.c index 944bf6f55c..76f8cfc93b 100644 --- a/eigrpd/eigrp_network.c +++ b/eigrpd/eigrp_network.c @@ -333,7 +333,7 @@ int eigrp_network_unset(struct eigrp *eigrp, struct prefix *p) if (rn->info == NULL) continue; - if (eigrp_network_match_iface(ei->address, &rn->p)) { + if (eigrp_network_match_iface(&ei->address, &rn->p)) { found = true; route_unlock_node(rn); break; diff --git a/eigrpd/eigrp_packet.c b/eigrpd/eigrp_packet.c index ee0476b28d..bedaf15c47 100644 --- a/eigrpd/eigrp_packet.c +++ b/eigrpd/eigrp_packet.c @@ -281,7 +281,7 @@ int eigrp_make_sha256_digest(struct eigrp_interface *ei, struct stream *s, return 0; } - inet_ntop(AF_INET, &ei->address->u.prefix4, source_ip, PREFIX_STRLEN); + inet_ntop(AF_INET, &ei->address.u.prefix4, source_ip, PREFIX_STRLEN); memset(&ctx, 0, sizeof(ctx)); buffer[0] = '\n'; @@ -362,7 +362,7 @@ int eigrp_write(struct thread *thread) } if (ep->dst.s_addr == htonl(EIGRP_MULTICAST_ADDRESS)) - eigrp_if_ipmulticast(eigrp, ei->address, ei->ifp->ifindex); + eigrp_if_ipmulticast(eigrp, &ei->address, ei->ifp->ifindex); memset(&iph, 0, sizeof(struct ip)); memset(&sa_dst, 0, sizeof(sa_dst)); @@ -418,7 +418,7 @@ int eigrp_write(struct thread *thread) iph.ip_ttl = EIGRP_IP_TTL; iph.ip_p = IPPROTO_EIGRPIGP; iph.ip_sum = 0; - iph.ip_src.s_addr = ei->address->u.prefix4.s_addr; + iph.ip_src.s_addr = ei->address.u.prefix4.s_addr; iph.ip_dst.s_addr = ep->dst.s_addr; memset(&msg, 0, sizeof(msg)); @@ -547,7 +547,7 @@ int eigrp_read(struct thread *thread) /* Self-originated packet should be discarded silently. */ if (eigrp_if_lookup_by_local_addr(eigrp, NULL, iph->ip_src) - || (IPV4_ADDR_SAME(&iph->ip_src, &ei->address->u.prefix4))) { + || (IPV4_ADDR_SAME(&iph->ip_src, &ei->address.u.prefix4))) { if (IS_DEBUG_EIGRP_TRANSMIT(0, RECV)) zlog_debug( "eigrp_read[%s]: Dropping self-originated packet", @@ -581,7 +581,7 @@ int eigrp_read(struct thread *thread) sizeof(buf[0])), inet_ntop(AF_INET, &iph->ip_dst, buf[1], sizeof(buf[1])), - inet_ntop(AF_INET, &ei->address->u.prefix4, + inet_ntop(AF_INET, &ei->address.u.prefix4, buf[2], sizeof(buf[2]))); if (iph->ip_dst.s_addr == htonl(EIGRP_MULTICAST_ADDRESS)) { @@ -981,9 +981,9 @@ static int eigrp_check_network_mask(struct eigrp_interface *ei, if (ei->type == EIGRP_IFTYPE_POINTOPOINT) return 1; - masklen2ip(ei->address->prefixlen, &mask); + masklen2ip(ei->address.prefixlen, &mask); - me.s_addr = ei->address->u.prefix4.s_addr & mask.s_addr; + me.s_addr = ei->address.u.prefix4.s_addr & mask.s_addr; him.s_addr = ip_src.s_addr & mask.s_addr; if (IPV4_ADDR_SAME(&me, &him)) diff --git a/eigrpd/eigrp_structs.h b/eigrpd/eigrp_structs.h index 644ab0829f..a78e5a53cf 100644 --- a/eigrpd/eigrp_structs.h +++ b/eigrpd/eigrp_structs.h @@ -180,7 +180,7 @@ struct eigrp_interface { /* EIGRP Network Type. */ uint8_t type; - struct prefix *address; /* Interface prefix */ + struct prefix address; /* Interface prefix */ /* Neighbor information. */ struct list *nbrs; /* EIGRP Neighbor List */ From 051da24eef4f877140f9905b0416e424589484e8 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 11 Feb 2019 07:19:14 -0500 Subject: [PATCH 3/4] eigrpd: Correctly handle the ref-count in a couple of spots The ref count for the eigrp topology table was incorrect in a couple of spots. Let's clean it up. Signed-off-by: Donald Sharp --- eigrpd/eigrp_topology.c | 2 +- eigrpd/eigrp_vty.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/eigrpd/eigrp_topology.c b/eigrpd/eigrp_topology.c index 23f5a705eb..e861cdb333 100644 --- a/eigrpd/eigrp_topology.c +++ b/eigrpd/eigrp_topology.c @@ -141,10 +141,10 @@ void eigrp_prefix_entry_add(struct route_table *topology, __PRETTY_FUNCTION__, prefix2str(pe->destination, buf, sizeof(buf))); } + route_unlock_node(rn); } rn->info = pe; - route_lock_node(rn); } /* diff --git a/eigrpd/eigrp_vty.c b/eigrpd/eigrp_vty.c index 104f35244e..a9b103de47 100644 --- a/eigrpd/eigrp_vty.c +++ b/eigrpd/eigrp_vty.c @@ -559,6 +559,7 @@ DEFPY (show_ip_eigrp_topology, tn = rn->info; eigrp_vty_display_prefix_entry(vty, eigrp, tn, argc == 5); + route_unlock_node(rn); return CMD_SUCCESS; } From 84b5e534e6bbf4ca69d4692dc89f472eb18adc68 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Sun, 10 Feb 2019 21:19:48 -0500 Subject: [PATCH 4/4] eigrpd: Do not redelete the eigrp interface data structure On interface down do not delete the eigrp interface data structure. Ensure that the address that we have setup the eigrp data structure ontop of is what we are deleting. Additionally add a test to show that this is no-longer crashing eigrp. Future commits will further modify this test to actually ensure that the eigrp topo is updated correctly and the rib has the correct data. Signed-off-by: Donald Sharp --- eigrpd/eigrp_zebra.c | 3 ++- tests/topotests/eigrp-topo1/r1/zebra.conf | 1 + tests/topotests/eigrp-topo1/test_eigrp_topo1.py | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/eigrpd/eigrp_zebra.c b/eigrpd/eigrp_zebra.c index 09d876afaa..dc1ae675b0 100644 --- a/eigrpd/eigrp_zebra.c +++ b/eigrpd/eigrp_zebra.c @@ -258,7 +258,8 @@ static int eigrp_interface_address_delete(int command, struct zclient *zclient, return 0; /* Call interface hook functions to clean up */ - eigrp_if_free(ei, INTERFACE_DOWN_BY_ZEBRA); + if (prefix_cmp(&ei->address, c->address) == 0) + eigrp_if_free(ei, INTERFACE_DOWN_BY_ZEBRA); connected_free(c); diff --git a/tests/topotests/eigrp-topo1/r1/zebra.conf b/tests/topotests/eigrp-topo1/r1/zebra.conf index 8537f6dd80..56ae4a66f4 100644 --- a/tests/topotests/eigrp-topo1/r1/zebra.conf +++ b/tests/topotests/eigrp-topo1/r1/zebra.conf @@ -1,4 +1,5 @@ log file zebra.log +debug zebra rib detail ! hostname r1 ! diff --git a/tests/topotests/eigrp-topo1/test_eigrp_topo1.py b/tests/topotests/eigrp-topo1/test_eigrp_topo1.py index 8ea2f0b506..1c00face43 100755 --- a/tests/topotests/eigrp-topo1/test_eigrp_topo1.py +++ b/tests/topotests/eigrp-topo1/test_eigrp_topo1.py @@ -171,6 +171,16 @@ def test_zebra_ipv4_routingTable(): assertmsg = 'Zebra IPv4 Routing Table verification failed for router {}'.format(router.name) assert topotest.json_cmp(output, expected) is None, assertmsg +def test_shut_interface_and_recover(): + "Test shutdown of an interface and recovery of the interface" + + tgen = get_topogen() + router = tgen.gears['r1'] + router.run('ip link set r1-eth1 down') + topotest.sleep(5, 'Waiting for EIGRP convergence') + router.run('ip link set r1-eth1 up') + + def test_shutdown_check_stderr(): if os.environ.get('TOPOTESTS_CHECK_STDERR') is None: