From c84355a93212939e98084137aa927ab911305614 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 25 Oct 2019 09:06:26 -0400 Subject: [PATCH 1/2] ospfd: interface name and ip address can be 32 bytes We are only saving 20 bytes of string output for ospf neighbor commands. Fixed output: act-7326-05# show ip ospf vrf vrf1012 neighbor all VRF Name: vrf1012 Neighbor ID Pri State Dead Time Address Interface RXmtL RqstL DBsmL 9.9.12.11 1 Full/DROther 39.973s 200.254.2.10 swp49s0.2:200.254.2.9 4 0 0 9.9.12.12 1 Full/DROther 39.995s 200.254.2.14 swp49s1.2:200.254.2.13 9 0 0 9.9.12.13 1 Exchange/DROthe 39.981s 200.254.2.18 swp49s2.2:200.254.2.17 157 0 0 Signed-off-by: Donald Sharp --- ospfd/ospf_vty.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 4c97615ed1..14455b23fd 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -4154,7 +4154,7 @@ DEFUN (show_ip_ospf_interface_traffic, static void show_ip_ospf_neighbour_header(struct vty *vty) { - vty_out(vty, "\n%-15s %3s %-15s %9s %-15s %-20s %5s %5s %5s\n", + vty_out(vty, "\n%-15s %3s %-15s %9s %-15s %-32s %5s %5s %5s\n", "Neighbor ID", "Pri", "State", "Dead Time", "Address", "Interface", "RXmtL", "RqstL", "DBsmL"); } @@ -4260,7 +4260,7 @@ static void show_ip_ospf_neighbor_sub(struct vty *vty, timebuf, sizeof(timebuf))); vty_out(vty, "%-15s ", inet_ntoa(nbr->src)); - vty_out(vty, "%-20s %5ld %5ld %5d\n", + vty_out(vty, "%-32s %5ld %5ld %5d\n", IF_NAME(oi), ospf_ls_retransmit_count(nbr), ospf_ls_request_count(nbr), @@ -4524,7 +4524,7 @@ static int show_ip_ospf_neighbor_all_common(struct vty *vty, struct ospf *ospf, "-", nbr_nbma->priority, "Down", "-"); vty_out(vty, - "%-15s %-20s %5d %5d %5d\n", + "%-32s %-20s %5d %5d %5d\n", inet_ntoa(nbr_nbma->addr), IF_NAME(oi), 0, 0, 0); } From 85d25587bfd551e4ce39501dd54a07c4325bb7ae Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 24 Oct 2019 15:09:18 -0400 Subject: [PATCH 2/2] eigrpd, ospfd, pimd: Fix assumption that interface may not be up Commit: ddbf3e60604019d4b38d51226700e2244cc531b6 This commit modified the interface up handling code in ZAPI such that the zclient handled the decoding for you. Prior to this commit ospf assumed that it could use the old ifp pointer to know state before reading the stream. This lead to a situation where ospf would `smartly` track and do the right thing in this situation. This commit changed this assumption and in certain scenarios, say a interface was changed after it was already up would lead to situations where ospf would not properly handle the new interface up. Modify ospf to track data that is important to it in it's interface->info pointer. This code pattern was followed in both eigrp and pim. In eigrp's case it was just behaving weirdly in any event so fixing this pattern is not a big deal. In pim's case it was not properly using this so it's a no-op to fix. Signed-off-by: Donald Sharp --- eigrpd/eigrp_interface.c | 66 +++++++++++++++++++--------------------- eigrpd/eigrp_structs.h | 2 ++ ospfd/ospf_interface.c | 32 ++++++++----------- ospfd/ospf_interface.h | 2 ++ pimd/pim_iface.c | 37 ++++++++++++---------- 5 files changed, 70 insertions(+), 69 deletions(-) diff --git a/eigrpd/eigrp_interface.c b/eigrpd/eigrp_interface.c index 6294c0dd0f..ece0b4b0c4 100644 --- a/eigrpd/eigrp_interface.c +++ b/eigrpd/eigrp_interface.c @@ -99,6 +99,9 @@ struct eigrp_interface *eigrp_if_new(struct eigrp *eigrp, struct interface *ifp, ei->params.auth_type = EIGRP_AUTH_TYPE_NONE; ei->params.auth_keychain = NULL; + ei->curr_bandwidth = ifp->bandwidth; + ei->curr_mtu = ifp->mtu; + return ei; } @@ -139,45 +142,40 @@ static int eigrp_ifp_create(struct interface *ifp) static int eigrp_ifp_up(struct interface *ifp) { - /* Interface is already up. */ - if (if_is_operative(ifp)) { - /* Temporarily keep ifp values. */ - struct interface if_tmp; - memcpy(&if_tmp, ifp, sizeof(struct interface)); - - if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) - zlog_debug("Zebra: Interface[%s] state update.", - ifp->name); - - if (if_tmp.bandwidth != ifp->bandwidth) { - if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) - zlog_debug( - "Zebra: Interface[%s] bandwidth change %d -> %d.", - ifp->name, if_tmp.bandwidth, - ifp->bandwidth); - - // eigrp_if_recalculate_output_cost (ifp); - } - - if (if_tmp.mtu != ifp->mtu) { - if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) - zlog_debug( - "Zebra: Interface[%s] MTU change %u -> %u.", - ifp->name, if_tmp.mtu, ifp->mtu); - - /* Must reset the interface (simulate down/up) when MTU - * changes. */ - eigrp_if_reset(ifp); - } - return 0; - } + struct eigrp_interface *ei = ifp->info; if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) zlog_debug("Zebra: Interface[%s] state change to up.", ifp->name); - if (ifp->info) - eigrp_if_up(ifp->info); + if (!ei) + return 0; + + if (ei->curr_bandwidth != ifp->bandwidth) { + if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) + zlog_debug( + "Zebra: Interface[%s] bandwidth change %d -> %d.", + ifp->name, ei->curr_bandwidth, + ifp->bandwidth); + + ei->curr_bandwidth = ifp->bandwidth; + // eigrp_if_recalculate_output_cost (ifp); + } + + if (ei->curr_mtu != ifp->mtu) { + if (IS_DEBUG_EIGRP(zebra, ZEBRA_INTERFACE)) + zlog_debug( + "Zebra: Interface[%s] MTU change %u -> %u.", + ifp->name, ei->curr_mtu, ifp->mtu); + + ei->curr_mtu = ifp->mtu; + /* Must reset the interface (simulate down/up) when MTU + * changes. */ + eigrp_if_reset(ifp); + return 0; + } + + eigrp_if_up(ifp->info); return 0; } diff --git a/eigrpd/eigrp_structs.h b/eigrpd/eigrp_structs.h index e50858f071..82bddaaae3 100644 --- a/eigrpd/eigrp_structs.h +++ b/eigrpd/eigrp_structs.h @@ -176,6 +176,8 @@ struct eigrp_interface { /* To which multicast groups do we currently belong? */ + uint32_t curr_bandwidth; + uint32_t curr_mtu; uint8_t multicast_memberships; diff --git a/ospfd/ospf_interface.c b/ospfd/ospf_interface.c index 7ddffbcdbd..cfcffcdb3d 100644 --- a/ospfd/ospf_interface.c +++ b/ospfd/ospf_interface.c @@ -636,9 +636,13 @@ void ospf_if_update_params(struct interface *ifp, struct in_addr addr) int ospf_if_new_hook(struct interface *ifp) { int rc = 0; + struct ospf_if_info *oii; ifp->info = XCALLOC(MTYPE_OSPF_IF_INFO, sizeof(struct ospf_if_info)); + oii = ifp->info; + oii->curr_mtu = ifp->mtu; + IF_OIFS(ifp) = route_table_init(); IF_OIFS_PARAMS(ifp) = route_table_init(); @@ -1261,31 +1265,21 @@ static int ospf_ifp_up(struct interface *ifp) { struct ospf_interface *oi; struct route_node *rn; + struct ospf_if_info *oii = ifp->info; - /* Interface is already up. */ - if (if_is_operative(ifp)) { - /* Temporarily keep ifp values. */ - struct interface if_tmp; - memcpy(&if_tmp, ifp, sizeof(struct interface)); + ospf_if_recalculate_output_cost(ifp); + if (oii && oii->curr_mtu != ifp->mtu) { if (IS_DEBUG_OSPF(zebra, ZEBRA_INTERFACE)) zlog_debug( - "Zebra: Interface[%s] state update speed %u -> %u, bw %d -> %d", - ifp->name, if_tmp.speed, ifp->speed, - if_tmp.bandwidth, ifp->bandwidth); + "Zebra: Interface[%s] MTU change %u -> %u.", + ifp->name, oii->curr_mtu, ifp->mtu); - ospf_if_recalculate_output_cost(ifp); + oii->curr_mtu = ifp->mtu; + /* Must reset the interface (simulate down/up) when MTU + * changes. */ + ospf_if_reset(ifp); - if (if_tmp.mtu != ifp->mtu) { - if (IS_DEBUG_OSPF(zebra, ZEBRA_INTERFACE)) - zlog_debug( - "Zebra: Interface[%s] MTU change %u -> %u.", - ifp->name, if_tmp.mtu, ifp->mtu); - - /* Must reset the interface (simulate down/up) when MTU - * changes. */ - ospf_if_reset(ifp); - } return 0; } diff --git a/ospfd/ospf_interface.h b/ospfd/ospf_interface.h index cde52dbb9e..4b3dbcc5c2 100644 --- a/ospfd/ospf_interface.h +++ b/ospfd/ospf_interface.h @@ -117,6 +117,8 @@ struct ospf_if_info { struct route_table *oifs; unsigned int membership_counts[MEMBER_MAX]; /* multicast group refcnts */ + + uint32_t curr_mtu; }; struct ospf_interface; diff --git a/pimd/pim_iface.c b/pimd/pim_iface.c index 3ee9caebcf..c615540149 100644 --- a/pimd/pim_iface.c +++ b/pimd/pim_iface.c @@ -1481,6 +1481,13 @@ void pim_if_create_pimreg(struct pim_instance *pim) pim_if_new(pim->regiface, false, false, true, false /*vxlan_term*/); + /* + * On vrf moves we delete the interface if there + * is nothing going on with it. We cannot have + * the pimregiface deleted. + */ + pim->regiface->configured = true; + } } @@ -1581,6 +1588,7 @@ int pim_ifp_create(struct interface *ifp) int pim_ifp_up(struct interface *ifp) { + struct pim_interface *pim_ifp; struct pim_instance *pim; uint32_t table_id; @@ -1593,24 +1601,21 @@ int pim_ifp_up(struct interface *ifp) } pim = pim_get_pim_instance(ifp->vrf_id); - if (if_is_operative(ifp)) { - struct pim_interface *pim_ifp; - pim_ifp = ifp->info; - /* - * If we have a pim_ifp already and this is an if_add - * that means that we probably have a vrf move event - * If that is the case, set the proper vrfness. - */ - if (pim_ifp) - pim_ifp->pim = pim; + pim_ifp = ifp->info; + /* + * If we have a pim_ifp already and this is an if_add + * that means that we probably have a vrf move event + * If that is the case, set the proper vrfness. + */ + if (pim_ifp) + pim_ifp->pim = pim; - /* - pim_if_addr_add_all() suffices for bringing up both IGMP and - PIM - */ - pim_if_addr_add_all(ifp); - } + /* + pim_if_addr_add_all() suffices for bringing up both IGMP and + PIM + */ + pim_if_addr_add_all(ifp); /* * If we have a pimreg device callback and it's for a specific