diff options
47 files changed, 627 insertions, 182 deletions
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index a46616803c..5a511eac1c 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -714,6 +714,11 @@ static void bgp_nht_ifp_table_handle(struct bgp *bgp, { struct bgp_nexthop_cache *bnc; + if (ifp->ifindex == IFINDEX_INTERNAL) { + zlog_warn("%s: The interface %s ignored", __func__, ifp->name); + return; + } + frr_each (bgp_nexthop_cache, table, bnc) { if (bnc->ifindex != ifp->ifindex) continue; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 24be160656..c559be5c52 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -13834,11 +13834,11 @@ static int bgp_table_stats_single(struct vty *vty, struct bgp *bgp, afi_t afi, json_bitlen = json_object_new_array(); for (i = 0; i <= bitlen; i++) { - struct json_object *ind_bit = json_object_new_object(); - if (!ts.prefix_len_count[i]) continue; + struct json_object *ind_bit = json_object_new_object(); + snprintf(temp_buf, sizeof(temp_buf), "%u", i); json_object_int_add(ind_bit, temp_buf, ts.prefix_len_count[i]); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 5c97f90c55..00a5e190cb 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -5506,9 +5506,14 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, if (rmap) { if (!peer->default_rmap[afi][safi].name || strcmp(rmap, peer->default_rmap[afi][safi].name) != 0) { - if (peer->default_rmap[afi][safi].name) + struct route_map *map = NULL; + + if (peer->default_rmap[afi][safi].name) { + map = route_map_lookup_by_name( + peer->default_rmap[afi][safi].name); XFREE(MTYPE_ROUTE_MAP_NAME, peer->default_rmap[afi][safi].name); + } /* * When there is a change in route-map policy, @@ -5521,16 +5526,21 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_DEFAULT_ORIGINATE); - route_map_counter_decrement(peer->default_rmap[afi][safi].map); + route_map_counter_decrement(map); peer->default_rmap[afi][safi].name = XSTRDUP(MTYPE_ROUTE_MAP_NAME, rmap); peer->default_rmap[afi][safi].map = route_map; route_map_counter_increment(route_map); } } else if (!rmap) { - if (peer->default_rmap[afi][safi].name) + struct route_map *map = NULL; + + if (peer->default_rmap[afi][safi].name) { + map = route_map_lookup_by_name( + peer->default_rmap[afi][safi].name); XFREE(MTYPE_ROUTE_MAP_NAME, peer->default_rmap[afi][safi].name); + } /* * This is triggered in case of route-map deletion. @@ -5541,7 +5551,7 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_DEFAULT_ORIGINATE); - route_map_counter_decrement(peer->default_rmap[afi][safi].map); + route_map_counter_decrement(map); peer->default_rmap[afi][safi].name = NULL; peer->default_rmap[afi][safi].map = NULL; } @@ -5573,11 +5583,16 @@ int peer_default_originate_set(struct peer *peer, afi_t afi, safi_t safi, SET_FLAG(member->af_flags[afi][safi], PEER_FLAG_DEFAULT_ORIGINATE); if (rmap) { - if (member->default_rmap[afi][safi].name) + struct route_map *map = NULL; + + if (member->default_rmap[afi][safi].name) { + map = route_map_lookup_by_name( + member->default_rmap[afi][safi].name); XFREE(MTYPE_ROUTE_MAP_NAME, member->default_rmap[afi][safi].name); - route_map_counter_decrement( - member->default_rmap[afi][safi].map); + } + + route_map_counter_decrement(map); member->default_rmap[afi][safi].name = XSTRDUP(MTYPE_ROUTE_MAP_NAME, rmap); member->default_rmap[afi][safi].map = route_map; @@ -5611,13 +5626,18 @@ int peer_default_originate_unset(struct peer *peer, afi_t afi, safi_t safi) PEER_ATTR_INHERIT(peer, peer->group, default_rmap[afi][safi].map); } else { + struct route_map *map = NULL; + /* Otherwise remove flag and configuration from peer. */ peer_af_flag_unset(peer, afi, safi, PEER_FLAG_DEFAULT_ORIGINATE); - if (peer->default_rmap[afi][safi].name) + if (peer->default_rmap[afi][safi].name) { + map = route_map_lookup_by_name( + peer->default_rmap[afi][safi].name); XFREE(MTYPE_ROUTE_MAP_NAME, peer->default_rmap[afi][safi].name); - route_map_counter_decrement(peer->default_rmap[afi][safi].map); + } + route_map_counter_decrement(map); peer->default_rmap[afi][safi].name = NULL; peer->default_rmap[afi][safi].map = NULL; } @@ -5640,6 +5660,10 @@ int peer_default_originate_unset(struct peer *peer, afi_t afi, safi_t safi) * they are explicitly overriding peer-group configuration. */ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + struct route_map *map; + + map = NULL; + /* Skip peers with overridden configuration. */ if (CHECK_FLAG(member->af_flags_override[afi][safi], PEER_FLAG_DEFAULT_ORIGINATE)) @@ -5648,10 +5672,13 @@ int peer_default_originate_unset(struct peer *peer, afi_t afi, safi_t safi) /* Remove flag and configuration on peer-group member. */ UNSET_FLAG(member->af_flags[afi][safi], PEER_FLAG_DEFAULT_ORIGINATE); - if (member->default_rmap[afi][safi].name) + if (member->default_rmap[afi][safi].name) { + map = route_map_lookup_by_name( + member->default_rmap[afi][safi].name); XFREE(MTYPE_ROUTE_MAP_NAME, member->default_rmap[afi][safi].name); - route_map_counter_decrement(member->default_rmap[afi][safi].map); + } + route_map_counter_decrement(map); member->default_rmap[afi][safi].name = NULL; member->default_rmap[afi][safi].map = NULL; @@ -7186,6 +7213,7 @@ int peer_route_map_set(struct peer *peer, afi_t afi, safi_t safi, int direct, struct peer *member; struct bgp_filter *filter; struct listnode *node, *nnode; + struct route_map *map = NULL; if (direct != RMAP_IN && direct != RMAP_OUT) return BGP_ERR_INVALID_VALUE; @@ -7199,9 +7227,10 @@ int peer_route_map_set(struct peer *peer, afi_t afi, safi_t safi, int direct, if (strcmp(filter->map[direct].name, name) == 0) return 0; + map = route_map_lookup_by_name(filter->map[direct].name); XFREE(MTYPE_BGP_FILTER_NAME, filter->map[direct].name); } - route_map_counter_decrement(filter->map[direct].map); + route_map_counter_decrement(map); filter->map[direct].name = XSTRDUP(MTYPE_BGP_FILTER_NAME, name); filter->map[direct].map = route_map; route_map_counter_increment(route_map); @@ -7223,6 +7252,7 @@ int peer_route_map_set(struct peer *peer, afi_t afi, safi_t safi, int direct, * explicitly overriding peer-group configuration. */ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + map = NULL; /* Skip peers with overridden configuration. */ if (CHECK_FLAG(member->filter_override[afi][safi][direct], PEER_FT_ROUTE_MAP)) @@ -7230,9 +7260,11 @@ int peer_route_map_set(struct peer *peer, afi_t afi, safi_t safi, int direct, /* Set configuration on peer-group member. */ filter = &member->filter[afi][safi]; - if (filter->map[direct].name) + if (filter->map[direct].name) { + map = route_map_lookup_by_name(filter->map[direct].name); XFREE(MTYPE_BGP_FILTER_NAME, filter->map[direct].name); - route_map_counter_decrement(filter->map[direct].map); + } + route_map_counter_decrement(map); filter->map[direct].name = XSTRDUP(MTYPE_BGP_FILTER_NAME, name); filter->map[direct].map = route_map; route_map_counter_increment(route_map); @@ -7265,11 +7297,16 @@ int peer_route_map_unset(struct peer *peer, afi_t afi, safi_t safi, int direct) PEER_ATTR_INHERIT(peer, peer->group, filter[afi][safi].map[direct].map); } else { + struct route_map *map = NULL; + /* Otherwise remove configuration from peer. */ filter = &peer->filter[afi][safi]; - if (filter->map[direct].name) + + if (filter->map[direct].name) { + map = route_map_lookup_by_name(filter->map[direct].name); XFREE(MTYPE_BGP_FILTER_NAME, filter->map[direct].name); - route_map_counter_decrement(filter->map[direct].map); + } + route_map_counter_decrement(map); filter->map[direct].name = NULL; filter->map[direct].map = NULL; } @@ -7289,6 +7326,10 @@ int peer_route_map_unset(struct peer *peer, afi_t afi, safi_t safi, int direct) * explicitly overriding peer-group configuration. */ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + struct route_map *map; + + map = NULL; + /* Skip peers with overridden configuration. */ if (CHECK_FLAG(member->filter_override[afi][safi][direct], PEER_FT_ROUTE_MAP)) @@ -7296,9 +7337,11 @@ int peer_route_map_unset(struct peer *peer, afi_t afi, safi_t safi, int direct) /* Remove configuration on peer-group member. */ filter = &member->filter[afi][safi]; - if (filter->map[direct].name) + if (filter->map[direct].name) { + map = route_map_lookup_by_name(filter->map[direct].name); XFREE(MTYPE_BGP_FILTER_NAME, filter->map[direct].name); - route_map_counter_decrement(filter->map[direct].map); + } + route_map_counter_decrement(map); filter->map[direct].name = NULL; filter->map[direct].map = NULL; @@ -7343,6 +7386,10 @@ int peer_unsuppress_map_set(struct peer *peer, afi_t afi, safi_t safi, * explicitly overriding peer-group configuration. */ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + struct route_map *map; + + map = NULL; + /* Skip peers with overridden configuration. */ if (CHECK_FLAG(member->filter_override[afi][safi][0], PEER_FT_UNSUPPRESS_MAP)) @@ -7350,9 +7397,11 @@ int peer_unsuppress_map_set(struct peer *peer, afi_t afi, safi_t safi, /* Set configuration on peer-group member. */ filter = &member->filter[afi][safi]; - if (filter->usmap.name) + if (filter->usmap.name) { + map = route_map_lookup_by_name(filter->usmap.name); XFREE(MTYPE_BGP_FILTER_NAME, filter->usmap.name); - route_map_counter_decrement(filter->usmap.map); + } + route_map_counter_decrement(map); filter->usmap.name = XSTRDUP(MTYPE_BGP_FILTER_NAME, name); filter->usmap.map = route_map; route_map_counter_increment(route_map); @@ -7382,11 +7431,15 @@ int peer_unsuppress_map_unset(struct peer *peer, afi_t afi, safi_t safi) PEER_ATTR_INHERIT(peer, peer->group, filter[afi][safi].usmap.map); } else { + struct route_map *map = NULL; + /* Otherwise remove configuration from peer. */ filter = &peer->filter[afi][safi]; - if (filter->usmap.name) + if (filter->usmap.name) { + map = route_map_lookup_by_name(filter->usmap.name); XFREE(MTYPE_BGP_FILTER_NAME, filter->usmap.name); - route_map_counter_decrement(filter->usmap.map); + } + route_map_counter_decrement(map); filter->usmap.name = NULL; filter->usmap.map = NULL; } @@ -7405,6 +7458,10 @@ int peer_unsuppress_map_unset(struct peer *peer, afi_t afi, safi_t safi) * explicitly overriding peer-group configuration. */ for (ALL_LIST_ELEMENTS(peer->group->peer, node, nnode, member)) { + struct route_map *map; + + map = NULL; + /* Skip peers with overridden configuration. */ if (CHECK_FLAG(member->filter_override[afi][safi][0], PEER_FT_UNSUPPRESS_MAP)) @@ -7412,9 +7469,11 @@ int peer_unsuppress_map_unset(struct peer *peer, afi_t afi, safi_t safi) /* Remove configuration on peer-group member. */ filter = &member->filter[afi][safi]; - if (filter->usmap.name) + if (filter->usmap.name) { + map = route_map_lookup_by_name(filter->usmap.name); XFREE(MTYPE_BGP_FILTER_NAME, filter->usmap.name); - route_map_counter_decrement(filter->usmap.map); + } + route_map_counter_decrement(map); filter->usmap.name = NULL; filter->usmap.map = NULL; diff --git a/doc/user/ospfd.rst b/doc/user/ospfd.rst index 232b1c3934..4e30ef2aec 100644 --- a/doc/user/ospfd.rst +++ b/doc/user/ospfd.rst @@ -884,10 +884,11 @@ Showing Information Show detailed information about the OSPF link-state database. -.. clicmd:: show ip ospf route [json] +.. clicmd:: show ip ospf route [detail] [json] Show the OSPF routing table, as determined by the most recent SPF - calculation. + calculation. When detail option is used, it shows more information + to the CLI like advertising router ID for each route, etc. .. clicmd:: show ip ospf [vrf <NAME|all>] border-routers [json] @@ -898,7 +899,7 @@ Showing Information .. clicmd:: show ip ospf graceful-restart helper [detail] [json] - Displays the Grcaeful Restart Helper details including helper + Displays the Graceful Restart Helper details including helper config changes. .. _opaque-lsa: diff --git a/doc/user/overview.rst b/doc/user/overview.rst index 33a1934628..a1cb0cc579 100644 --- a/doc/user/overview.rst +++ b/doc/user/overview.rst @@ -358,6 +358,8 @@ BGP :t:`Outbound Route Filtering Capability. E. Chen, Y. Rekhter. August 2008.` - :rfc:`5292` :t:`Address-Prefix-Based Outbound Route Filter for BGP-4. E. Chen, S. Sangli. August 2008.` +- :rfc:`5396` + :t:`Textual Representation of Autonomous System (AS) Numbers. G. Michaelson, G. Huston. December 2008.` - :rfc:`5492` :t:`Capabilities Advertisement with BGP-4. J. Scudder, R. Chandra. February 2009.` - :rfc:`5575` diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c index da19db463f..7e42e1c09e 100644 --- a/lib/mgmt_fe_client.c +++ b/lib/mgmt_fe_client.c @@ -629,6 +629,11 @@ uint mgmt_fe_client_session_count(struct mgmt_fe_client *client) return mgmt_sessions_count(&client->sessions); } +bool mgmt_fe_client_current_msg_short_circuit(struct mgmt_fe_client *client) +{ + return client->client.conn.is_short_circuit; +} + /* * Create a new Session for a Frontend Client connection. */ diff --git a/lib/mgmt_fe_client.h b/lib/mgmt_fe_client.h index 286141da44..349b7e4cf4 100644 --- a/lib/mgmt_fe_client.h +++ b/lib/mgmt_fe_client.h @@ -373,6 +373,12 @@ extern void mgmt_fe_client_destroy(struct mgmt_fe_client *client); */ extern uint mgmt_fe_client_session_count(struct mgmt_fe_client *client); +/* + * True if the current handled message is being short-circuited + */ +extern bool +mgmt_fe_client_current_msg_short_circuit(struct mgmt_fe_client *client); + #ifdef __cplusplus } #endif diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index 4659850994..5767e0aacf 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -353,6 +353,7 @@ static int lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) { struct route_map_index *rmi; + struct route_map *map; int rm_action; int policy; @@ -382,6 +383,7 @@ lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) break; case NB_EV_APPLY: rmi = nb_running_get_entry(args->dnode, NULL, true); + map = rmi->map; policy = yang_dnode_get_enum(args->dnode, NULL); switch (policy) { @@ -395,6 +397,14 @@ lib_route_map_entry_exit_policy_modify(struct nb_cb_modify_args *args) rmi->exitpolicy = RMAP_GOTO; break; } + + /* Execute event hook. */ + if (route_map_master.event_hook) { + (*route_map_master.event_hook)(map->name); + route_map_notify_dependencies(map->name, + RMAP_EVENT_CALL_ADDED); + } + break; } @@ -3524,6 +3524,7 @@ static void vty_mgmt_ds_lock_notified(struct mgmt_fe_client *client, char *errmsg_if_any) { struct vty *vty; + bool is_short_circuit = mgmt_fe_client_current_msg_short_circuit(client); vty = (struct vty *)session_ctx; @@ -3540,8 +3541,10 @@ static void vty_mgmt_ds_lock_notified(struct mgmt_fe_client *client, vty->mgmt_locked_running_ds = lock_ds; } - if (vty->mgmt_req_pending_cmd) + if (!is_short_circuit && vty->mgmt_req_pending_cmd) { + assert(!strcmp(vty->mgmt_req_pending_cmd, "MESSAGE_LOCKDS_REQ")); vty_mgmt_resume_response(vty, success); + } } static void vty_mgmt_set_config_result_notified( @@ -3734,6 +3737,7 @@ int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit) } else if (vty_mgmt_lock_running_inline(vty)) { vty_out(vty, "%% command failed, could not lock running DS\n"); + vty_mgmt_unlock_candidate_inline(vty); return -1; } } @@ -231,6 +231,10 @@ struct vty { const char *mgmt_req_pending_cmd; bool mgmt_locked_candidate_ds; bool mgmt_locked_running_ds; + /* Need to track when we file-lock in vtysh to re-lock on end/conf t + * workaround + */ + bool vtysh_file_locked; }; static inline void vty_push_context(struct vty *vty, int node, uint64_t id) diff --git a/mgmtd/mgmt_fe_adapter.c b/mgmtd/mgmt_fe_adapter.c index c12d8646b1..2b2471c901 100644 --- a/mgmtd/mgmt_fe_adapter.c +++ b/mgmtd/mgmt_fe_adapter.c @@ -593,9 +593,8 @@ mgmt_fe_session_handle_lockds_req_msg(struct mgmt_fe_session_ctx *session, } if (lockds_req->lock) { - if (mgmt_fe_session_write_lock_ds(lockds_req->ds_id, - ds_ctx, session) - != 0) { + if (mgmt_fe_session_write_lock_ds(lockds_req->ds_id, ds_ctx, + session)) { fe_adapter_send_lockds_reply( session, lockds_req->ds_id, lockds_req->req_id, lockds_req->lock, false, @@ -1160,8 +1159,7 @@ int mgmt_fe_send_set_cfg_reply(uint64_t session_id, uint64_t txn_id, } return fe_adapter_send_set_cfg_reply(session, ds_id, req_id, - result == MGMTD_SUCCESS ? true - : false, + result == MGMTD_SUCCESS, error_if_any, implicit_commit); } diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index eff3b7e34c..53b9b7df46 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -1380,8 +1380,7 @@ static int mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn, batch->batch_id, cfg_req.cfgdata_reqs, cfg_req.num_reqs, - indx == num_batches ? true - : false)) { + indx == num_batches)) { (void)mgmt_txn_send_commit_cfg_reply( txn, MGMTD_INTERNAL_ERROR, "Internal Error! Could not send config data to backend!"); diff --git a/ospf6d/ospf6_flood.c b/ospf6d/ospf6_flood.c index a81d5222d6..fd5a4a426e 100644 --- a/ospf6d/ospf6_flood.c +++ b/ospf6d/ospf6_flood.c @@ -389,7 +389,7 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa, if (req == on->last_ls_req) { /* sanity check refcount */ assert(req->lock >= 2); - req = ospf6_lsa_unlock(req); + ospf6_lsa_unlock(&req); on->last_ls_req = NULL; } if (req) @@ -406,7 +406,7 @@ void ospf6_flood_interface(struct ospf6_neighbor *from, struct ospf6_lsa *lsa, zlog_debug( "Received is newer, remove requesting"); if (req == on->last_ls_req) { - req = ospf6_lsa_unlock(req); + ospf6_lsa_unlock(&req); on->last_ls_req = NULL; } if (req) diff --git a/ospf6d/ospf6_gr.c b/ospf6d/ospf6_gr.c index f39da91415..69230e572b 100644 --- a/ospf6d/ospf6_gr.c +++ b/ospf6d/ospf6_gr.c @@ -294,7 +294,7 @@ static int ospf6_router_lsa_contains_adj(struct ospf6_area *area, continue; if (lsdesc->neighbor_router_id == neighbor_router_id) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); return RTR_LSA_ADJ_FOUND; } } @@ -514,7 +514,7 @@ static bool ospf6_gr_check_adjs(struct ospf6 *ospf6) lsa_self)) { found = true; if (!ospf6_gr_check_adjs_lsa(area, lsa_self)) { - ospf6_lsa_unlock(lsa_self); + ospf6_lsa_unlock(&lsa_self); return false; } } diff --git a/ospf6d/ospf6_gr_helper.c b/ospf6d/ospf6_gr_helper.c index 216d78c1cc..8159ac0cd3 100644 --- a/ospf6d/ospf6_gr_helper.c +++ b/ospf6d/ospf6_gr_helper.c @@ -228,9 +228,9 @@ static bool ospf6_check_chg_in_rxmt_list(struct ospf6_neighbor *nbr) lsa->header->adv_router, lsa->lsdb); if (lsa_in_db && lsa_in_db->tobe_acknowledged) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); if (lsanext) - ospf6_lsa_unlock(lsanext); + ospf6_lsa_unlock(&lsanext); return OSPF6_TRUE; } diff --git a/ospf6d/ospf6_lsa.c b/ospf6d/ospf6_lsa.c index 2b806afe06..e745c6c577 100644 --- a/ospf6d/ospf6_lsa.c +++ b/ospf6d/ospf6_lsa.c @@ -797,17 +797,17 @@ struct ospf6_lsa *ospf6_lsa_lock(struct ospf6_lsa *lsa) } /* decrement reference counter of struct ospf6_lsa */ -struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *lsa) +void ospf6_lsa_unlock(struct ospf6_lsa **lsa) { /* decrement reference counter */ - assert(lsa->lock > 0); - lsa->lock--; + assert((*lsa)->lock > 0); + (*lsa)->lock--; - if (lsa->lock != 0) - return lsa; + if ((*lsa)->lock != 0) + return; - ospf6_lsa_delete(lsa); - return NULL; + ospf6_lsa_delete(*lsa); + *lsa = NULL; } diff --git a/ospf6d/ospf6_lsa.h b/ospf6d/ospf6_lsa.h index be7b94f3d3..c9ac27df88 100644 --- a/ospf6d/ospf6_lsa.h +++ b/ospf6d/ospf6_lsa.h @@ -239,7 +239,7 @@ extern void ospf6_lsa_delete(struct ospf6_lsa *lsa); extern struct ospf6_lsa *ospf6_lsa_copy(struct ospf6_lsa *lsa); extern struct ospf6_lsa *ospf6_lsa_lock(struct ospf6_lsa *lsa); -extern struct ospf6_lsa *ospf6_lsa_unlock(struct ospf6_lsa *lsa); +extern void ospf6_lsa_unlock(struct ospf6_lsa **lsa); extern void ospf6_lsa_expire(struct event *thread); extern void ospf6_lsa_refresh(struct event *thread); diff --git a/ospf6d/ospf6_lsdb.c b/ospf6d/ospf6_lsdb.c index fa1a9a408d..355860f6b1 100644 --- a/ospf6d/ospf6_lsdb.c +++ b/ospf6d/ospf6_lsdb.c @@ -139,7 +139,7 @@ void ospf6_lsdb_add(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb) } /* to free the lookup lock in node get*/ route_unlock_node(current); - ospf6_lsa_unlock(old); + ospf6_lsa_unlock(&old); } ospf6_lsdb_count_assert(lsdb); @@ -168,7 +168,7 @@ void ospf6_lsdb_remove(struct ospf6_lsa *lsa, struct ospf6_lsdb *lsdb) route_unlock_node(node); /* to free the lookup lock */ route_unlock_node(node); /* to free the original lock */ - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); ospf6_lsdb_count_assert(lsdb); } @@ -237,7 +237,7 @@ struct ospf6_lsa *ospf6_find_inter_prefix_lsa(struct ospf6 *ospf6, ospf6_prefix_in6_addr(&prefix.u.prefix6, prefix_lsa, &prefix_lsa->prefix); if (prefix_same(p, &prefix)) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); return lsa; } } @@ -328,7 +328,7 @@ struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend, { struct route_node *node = lsa->rn; - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); do node = route_next_until(node, iterend); @@ -361,7 +361,7 @@ void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa) if (lsa != NULL) { if (lsa->rn != NULL) route_unlock_node(lsa->rn); - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); } } diff --git a/ospf6d/ospf6_lsdb.h b/ospf6d/ospf6_lsdb.h index a2444f1c14..604406d75f 100644 --- a/ospf6d/ospf6_lsdb.h +++ b/ospf6d/ospf6_lsdb.h @@ -63,11 +63,11 @@ extern struct ospf6_lsa *ospf6_lsdb_next(const struct route_node *iterend, * it really early. */ #define ALL_LSDB(lsdb, lsa, lsanext) \ - const struct route_node *iterend = \ - ospf6_lsdb_head(lsdb, 0, 0, 0, &lsa); \ - (lsa) != NULL && ospf6_lsa_lock(lsa) \ - && ((lsanext) = ospf6_lsdb_next(iterend, (lsa)), 1); \ - ospf6_lsa_unlock(lsa), (lsa) = (lsanext) + const struct route_node *iterend = ospf6_lsdb_head(lsdb, 0, 0, 0, \ + &lsa); \ + (lsa) != NULL && ospf6_lsa_lock(lsa) && \ + ((lsanext) = ospf6_lsdb_next(iterend, (lsa)), 1); \ + ospf6_lsa_unlock(&lsa), (lsa) = (lsanext) extern void ospf6_lsdb_remove_all(struct ospf6_lsdb *lsdb); extern void ospf6_lsdb_lsa_unlock(struct ospf6_lsa *lsa); diff --git a/ospf6d/ospf6_message.c b/ospf6d/ospf6_message.c index cc00604f0a..e9b19ea0a6 100644 --- a/ospf6d/ospf6_message.c +++ b/ospf6d/ospf6_message.c @@ -2335,9 +2335,9 @@ static uint16_t ospf6_make_dbdesc(struct ospf6_neighbor *on, struct stream *s) if ((length + sizeof(struct ospf6_lsa_header) + OSPF6_HEADER_SIZE) > ospf6_packet_max(on->ospf6_if)) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); if (lsanext) - ospf6_lsa_unlock(lsanext); + ospf6_lsa_unlock(&lsanext); break; } stream_put(s, lsa->header, @@ -2415,9 +2415,9 @@ void ospf6_dbdesc_send_newone(struct event *thread) if (size + sizeof(struct ospf6_lsa_header) > ospf6_packet_max(on->ospf6_if)) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); if (lsanext) - ospf6_lsa_unlock(lsanext); + ospf6_lsa_unlock(&lsanext); break; } @@ -2447,9 +2447,9 @@ static uint16_t ospf6_make_lsreq(struct ospf6_neighbor *on, struct stream *s) for (ALL_LSDB(on->request_list, lsa, lsanext)) { if ((length + OSPF6_HEADER_SIZE) > ospf6_packet_max(on->ospf6_if)) { - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); if (lsanext) - ospf6_lsa_unlock(lsanext); + ospf6_lsa_unlock(&lsanext); break; } stream_putw(s, 0); /* reserved */ @@ -2462,7 +2462,7 @@ static uint16_t ospf6_make_lsreq(struct ospf6_neighbor *on, struct stream *s) if (last_req != NULL) { if (on->last_ls_req != NULL) - on->last_ls_req = ospf6_lsa_unlock(on->last_ls_req); + ospf6_lsa_unlock(&on->last_ls_req); ospf6_lsa_lock(last_req); on->last_ls_req = last_req; @@ -2944,9 +2944,9 @@ static uint16_t ospf6_make_lsack_interface(struct ospf6_interface *oi, event_add_event(master, ospf6_lsack_send_interface, oi, 0, &oi->thread_send_lsack); - ospf6_lsa_unlock(lsa); + ospf6_lsa_unlock(&lsa); if (lsanext) - ospf6_lsa_unlock(lsanext); + ospf6_lsa_unlock(&lsanext); break; } ospf6_lsa_age_update_to_send(lsa, oi->transdelay); diff --git a/ospf6d/ospf6_neighbor.c b/ospf6d/ospf6_neighbor.c index dc7abdd84c..e1aec06f8e 100644 --- a/ospf6d/ospf6_neighbor.c +++ b/ospf6d/ospf6_neighbor.c @@ -98,9 +98,10 @@ static void ospf6_neighbor_clear_ls_lists(struct ospf6_neighbor *on) ospf6_lsdb_remove_all(on->summary_list); if (on->last_ls_req) { - ospf6_lsa_unlock(on->last_ls_req); + ospf6_lsa_unlock(&on->last_ls_req); on->last_ls_req = NULL; } + ospf6_lsdb_remove_all(on->request_list); for (ALL_LSDB(on->retrans_list, lsa, lsanext)) { ospf6_decrement_retrans_count(lsa); diff --git a/ospfd/ospf_route.c b/ospfd/ospf_route.c index cdb1eb0095..3ffa7c0bb1 100644 --- a/ospfd/ospf_route.c +++ b/ospfd/ospf_route.c @@ -48,6 +48,7 @@ struct ospf_route *ospf_route_new(void) new->paths = list_new(); new->paths->del = (void (*)(void *))ospf_path_free; + new->u.std.transit = false; return new; } @@ -500,6 +501,7 @@ void ospf_intra_add_transit(struct route_table *rt, struct vertex *v, or->cost = v->distance; or->type = OSPF_DESTINATION_NETWORK; or->u.std.origin = (struct lsa_header *)lsa; + or->u.std.transit = true; ospf_route_copy_nexthops_from_vertex(area, or, v); @@ -851,7 +853,7 @@ void ospf_route_copy_nexthops_from_vertex(struct ospf_area *area, || area->spf_dry_run) { path = ospf_path_new(); path->nexthop = nexthop->router; - path->adv_router = v->id; + path->adv_router = v->lsa->adv_router; if (oi) { path->ifindex = oi->ifp->ifindex; diff --git a/ospfd/ospf_route.h b/ospfd/ospf_route.h index 7639a0049e..44e80216d7 100644 --- a/ospfd/ospf_route.h +++ b/ospfd/ospf_route.h @@ -69,6 +69,8 @@ struct route_standard { /* */ uint8_t flags; /* From router-LSA */ + + bool transit; /* Transit network or not */ }; struct route_external { diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 54fd60af23..a23802719b 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -10732,7 +10732,7 @@ static void config_write_stub_router(struct vty *vty, struct ospf *ospf) static void show_ip_ospf_route_network(struct vty *vty, struct ospf *ospf, struct route_table *rt, - json_object *json) + json_object *json, bool detail) { struct route_node *rn; struct ospf_route * or ; @@ -10792,15 +10792,17 @@ static void show_ip_ospf_route_network(struct vty *vty, struct ospf *ospf, if (json) { json_object_string_add(json_route, "routeType", "N"); + json_object_boolean_add(json_route, "transit", + or->u.std.transit); json_object_int_add(json_route, "cost", or->cost); json_object_string_addf(json_route, "area", "%pI4", &or->u.std.area_id); } else { - vty_out(vty, "N %-18s [%d] area: %pI4\n", - buf1, or->cost, - &or->u.std.area_id); + vty_out(vty, "N %s %-18s [%d] area: %pI4\n", + or->u.std.transit && detail ? "T" : " ", + buf1, or->cost, &or->u.std.area_id); } break; default: @@ -10857,6 +10859,11 @@ static void show_ip_ospf_route_network(struct vty *vty, struct ospf *ospf, ifindex2ifname( path->ifindex, ospf->vrf_id)); + json_object_string_addf( + json_nexthop, + "advertisedRouter", + "%pI4", + &path->adv_router); } else { vty_out(vty, "%24s via %pI4, %s\n", @@ -10866,6 +10873,11 @@ static void show_ip_ospf_route_network(struct vty *vty, struct ospf *ospf, path->ifindex, ospf->vrf_id)); } + if (detail && !json) + vty_out(vty, + "%24s adv %pI4\n", + "", + &path->adv_router); } } } @@ -11020,7 +11032,7 @@ static void show_ip_ospf_route_router(struct vty *vty, struct ospf *ospf, static void show_ip_ospf_route_external(struct vty *vty, struct ospf *ospf, struct route_table *rt, - json_object *json) + json_object *json, bool detail) { struct route_node *rn; struct ospf_route *er; @@ -11124,6 +11136,11 @@ static void show_ip_ospf_route_external(struct vty *vty, struct ospf *ospf, ifindex2ifname( path->ifindex, ospf->vrf_id)); + json_object_string_addf( + json_nexthop, + "advertisedRouter", + "%pI4", + &path->adv_router); } else { vty_out(vty, "%24s via %pI4, %s\n", @@ -11133,6 +11150,10 @@ static void show_ip_ospf_route_external(struct vty *vty, struct ospf *ospf, path->ifindex, ospf->vrf_id)); } + if (detail && !json) + vty_out(vty, + "%24s adv %pI4\n", "", + &path->adv_router); } } } @@ -11419,7 +11440,8 @@ DEFUN (show_ip_ospf_instance_border_routers, } static int show_ip_ospf_route_common(struct vty *vty, struct ospf *ospf, - json_object *json, uint8_t use_vrf) + json_object *json, uint8_t use_vrf, + bool detail) { json_object *json_vrf = NULL; @@ -11446,8 +11468,15 @@ static int show_ip_ospf_route_common(struct vty *vty, struct ospf *ospf, return CMD_SUCCESS; } + if (detail && json == NULL) { + vty_out(vty, "Codes: N - network T - transitive\n"); + vty_out(vty, " IA - inter-area E - external route\n"); + vty_out(vty, " D - destination R - router\n\n"); + } + /* Show Network routes. */ - show_ip_ospf_route_network(vty, ospf, ospf->new_table, json_vrf); + show_ip_ospf_route_network(vty, ospf, ospf->new_table, json_vrf, + detail); /* Show Router routes. */ show_ip_ospf_route_router(vty, ospf, ospf->new_rtrs, json_vrf); @@ -11458,7 +11487,7 @@ static int show_ip_ospf_route_common(struct vty *vty, struct ospf *ospf, /* Show AS External routes. */ show_ip_ospf_route_external(vty, ospf, ospf->old_external_route, - json_vrf); + json_vrf, detail); if (json) { if (use_vrf) { @@ -11476,13 +11505,14 @@ static int show_ip_ospf_route_common(struct vty *vty, struct ospf *ospf, DEFUN (show_ip_ospf_route, show_ip_ospf_route_cmd, - "show ip ospf [vrf <NAME|all>] route [json]", + "show ip ospf [vrf <NAME|all>] route [detail] [json]", SHOW_STR IP_STR "OSPF information\n" VRF_CMD_HELP_STR "All VRFs\n" "OSPF routing table\n" + "Detailed information\n" JSON_STR) { struct ospf *ospf = NULL; @@ -11491,14 +11521,19 @@ DEFUN (show_ip_ospf_route, bool all_vrf = false; int ret = CMD_SUCCESS; int inst = 0; + int idx = 0; int idx_vrf = 0; uint8_t use_vrf = 0; bool uj = use_json(argc, argv); + bool detail = false; json_object *json = NULL; if (uj) json = json_object_new_object(); + if (argv_find(argv, argc, "detail", &idx)) + detail = true; + OSPF_FIND_VRF_ARGS(argv, argc, idx_vrf, vrf_name, all_vrf); /* vrf input is provided could be all or specific vrf*/ @@ -11512,8 +11547,8 @@ DEFUN (show_ip_ospf_route, if (!ospf->oi_running) continue; ospf_output = true; - ret = show_ip_ospf_route_common(vty, ospf, json, - use_vrf); + ret = show_ip_ospf_route_common( + vty, ospf, json, use_vrf, detail); } if (uj) { @@ -11550,7 +11585,8 @@ DEFUN (show_ip_ospf_route, } if (ospf) { - ret = show_ip_ospf_route_common(vty, ospf, json, use_vrf); + ret = show_ip_ospf_route_common(vty, ospf, json, use_vrf, + detail); /* Keep Non-pretty format */ if (uj) vty_out(vty, "%s\n", @@ -11566,16 +11602,22 @@ DEFUN (show_ip_ospf_route, DEFUN (show_ip_ospf_instance_route, show_ip_ospf_instance_route_cmd, - "show ip ospf (1-65535) route", + "show ip ospf (1-65535) route [detail]", SHOW_STR IP_STR "OSPF information\n" "Instance ID\n" - "OSPF routing table\n") + "OSPF routing table\n" + "Detailed information\n") { int idx_number = 3; + int idx = 0; struct ospf *ospf; unsigned short instance = 0; + bool detail = false; + + if (argv_find(argv, argc, "detail", &idx)) + detail = true; instance = strtoul(argv[idx_number]->arg, NULL, 10); if (instance != ospf_instance) @@ -11585,7 +11627,7 @@ DEFUN (show_ip_ospf_instance_route, if (!ospf || !ospf->oi_running) return CMD_SUCCESS; - return show_ip_ospf_route_common(vty, ospf, NULL, 0); + return show_ip_ospf_route_common(vty, ospf, NULL, 0, detail); } diff --git a/pimd/pim_ifchannel.c b/pimd/pim_ifchannel.c index bc6b84ff2c..da55189941 100644 --- a/pimd/pim_ifchannel.c +++ b/pimd/pim_ifchannel.c @@ -135,7 +135,7 @@ void pim_ifchannel_delete(struct pim_ifchannel *ch) * being inherited. So let's figure out what * needs to be done here */ - if (!pim_addr_is_any(ch->sg.src) && + if (!pim_addr_is_any(ch->sg.src) && ch->parent && pim_upstream_evaluate_join_desired_interface( ch->upstream, ch, ch->parent)) pim_channel_add_oif(ch->upstream->channel_oil, diff --git a/tests/lib/test_grpc.cpp b/tests/lib/test_grpc.cpp index 87530d41d8..182c1d338d 100644 --- a/tests/lib/test_grpc.cpp +++ b/tests/lib/test_grpc.cpp @@ -34,8 +34,8 @@ #include <grpcpp/security/credentials.h> #include "grpc/frr-northbound.grpc.pb.h" -DEFINE_HOOK(frr_late_init, (struct event_loop * tm), (tm)); -DEFINE_KOOH(frr_fini, (), ()); +DEFINE_HOOK(test_grpc_late_init, (struct event_loop * tm), (tm)); +DEFINE_KOOH(test_grpc_fini, (), ()); struct vty *vty; @@ -85,7 +85,7 @@ static void static_startup(void) zprivs_init(&static_privs); /* Load the server side module -- check libtool path first */ - std::string modpath = std::string(binpath) + std::string("../../../lib/.libs"); + std::string modpath = std::string(binpath) + std::string("../../lib/.libs"); grpc_module = frrmod_load("grpc:50051", modpath.c_str(), 0, 0); if (!grpc_module) { modpath = std::string(binpath) + std::string("../../lib"); @@ -127,12 +127,12 @@ static void static_startup(void) frr_pthread_init(); // frr_config_fork(); - hook_call(frr_late_init, master); + hook_call(test_grpc_late_init, master); } static void static_shutdown(void) { - hook_call(frr_fini); + hook_call(test_grpc_fini); vty_close(vty); vrf_terminate(); vty_terminate(); diff --git a/tests/ospf6d/test_lsdb.c b/tests/ospf6d/test_lsdb.c index 4bc6e869b6..f9df73538a 100644 --- a/tests/ospf6d/test_lsdb.c +++ b/tests/ospf6d/test_lsdb.c @@ -59,7 +59,7 @@ DEFPY(lsa_set, lsa_set_cmd, lsa_check_resize(idx + 1); if (lsas[idx]) - ospf6_lsa_unlock(lsas[idx]); + ospf6_lsa_unlock(&lsas[idx]); lsas[idx] = ospf6_lsa_create_headeronly(&hdr); ospf6_lsa_lock(lsas[idx]); return CMD_SUCCESS; @@ -75,7 +75,7 @@ DEFPY(lsa_drop, lsa_drop_cmd, return CMD_SUCCESS; if (lsas[idx]->lock != 1) vty_out(vty, "refcount at %u\n", lsas[idx]->lock); - ospf6_lsa_unlock(lsas[idx]); + ospf6_lsa_unlock(&lsas[idx]); lsas[idx] = NULL; return CMD_SUCCESS; } diff --git a/tests/topotests/babel_topo1/test_babel_topo1.py b/tests/topotests/babel_topo1/test_babel_topo1.py index 6a0297a7ee..decf0c2a6f 100644 --- a/tests/topotests/babel_topo1/test_babel_topo1.py +++ b/tests/topotests/babel_topo1/test_babel_topo1.py @@ -19,6 +19,7 @@ import re import sys import pytest import json +from functools import partial pytestmark = [pytest.mark.babeld] @@ -110,6 +111,17 @@ def test_converge_protocols(): topotest.sleep(10, "Waiting for BABEL convergence") +def runit(router, assertmsg, cmd, expfile): + logger.info(expfile) + + # Read expected result from file + expected = json.loads(open(expfile).read()) + + test_func = partial(topotest.router_json_cmp, router, cmd, expected) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, assertmsg + + def test_zebra_ipv4_routingTable(): "Test 'show ip route'" @@ -121,14 +133,12 @@ def test_zebra_ipv4_routingTable(): failures = 0 router_list = tgen.routers().values() for router in router_list: - output = router.vtysh_cmd("show ip route json", isjson=True) - refTableFile = "{}/{}/show_ip_route.json_ref".format(CWD, router.name) - expected = json.loads(open(refTableFile).read()) - assertmsg = "Zebra IPv4 Routing Table verification failed for router {}".format( router.name ) - assert topotest.json_cmp(output, expected) is None, assertmsg + refTableFile = "{}/{}/show_ip_route.json_ref".format(CWD, router.name) + runit(router, assertmsg, "show ip route json", refTableFile) + def test_shutdown_check_stderr(): if os.environ.get("TOPOTESTS_CHECK_STDERR") is None: diff --git a/tests/topotests/bgp_route_map_on_match_next/__init__.py b/tests/topotests/bgp_route_map_on_match_next/__init__.py new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/__init__.py diff --git a/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf b/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf new file mode 100644 index 0000000000..b858fffd81 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r1/bgpd.conf @@ -0,0 +1,10 @@ +! +router bgp 65001 + no bgp network import-check + neighbor 192.168.255.2 remote-as 65001 + neighbor 192.168.255.2 timers 3 10 + address-family ipv4 unicast + network 10.100.100.1/32 + exit-address-family + ! +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf b/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf new file mode 100644 index 0000000000..581e2e634f --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r1/zebra.conf @@ -0,0 +1,6 @@ +! +interface r1-eth0 + ip address 192.168.255.1/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf b/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf new file mode 100644 index 0000000000..518d63d692 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r2/bgpd.conf @@ -0,0 +1,17 @@ +! +router bgp 65001 + neighbor 192.168.255.1 remote-as 65001 + neighbor 192.168.255.1 timers 3 10 + address-family ipv4 unicast + neighbor 192.168.255.1 route-map RM in + exit-address-family + ! +! +route-map RM permit 10 + set weight 100 +exit +! +route-map RM permit 20 + set metric 20 +exit +! diff --git a/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf b/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf new file mode 100644 index 0000000000..fd45c48d6d --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/r2/zebra.conf @@ -0,0 +1,6 @@ +! +interface r2-eth0 + ip address 192.168.255.2/30 +! +ip forwarding +! diff --git a/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py b/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py new file mode 100644 index 0000000000..8fe45a3498 --- /dev/null +++ b/tests/topotests/bgp_route_map_on_match_next/test_bgp_route_map_on_match_next.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# test_bgp_route_map_on_match_next.py +# +# Copyright (c) 2023 Rubicon Communications, LLC. +# + +""" +Test whether `on-match next` added to an existing route-map entry takes effect. +""" + +import os +import sys +import json +import pytest +import functools + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen + +pytestmark = [pytest.mark.bgpd] + + +def build_topo(tgen): + for routern in range(1, 3): + tgen.add_router("r{}".format(routern)) + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for i, (rname, router) in enumerate(router_list.items(), 1): + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, os.path.join(CWD, "{}/bgpd.conf".format(rname)) + ) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_route_map_on_match_next(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + router1 = tgen.gears["r1"] + router2 = tgen.gears["r2"] + + def _bgp_converge(router): + output = json.loads(router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json")) + expected = {"192.168.255.1": {"bgpState": "Established"}} + return topotest.json_cmp(output, expected) + + def _bgp_has_routes(router, metric, weight): + output = json.loads( + router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 routes json") + ) + expected = { + "routes": {"10.100.100.1/32": [{"metric": metric, "weight": weight}]} + } + return topotest.json_cmp(output, expected) + + # Check thst session is established + test_func = functools.partial(_bgp_converge, router2) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "Failed bgp convergence on r2" + + # Check that metric is 0 and weight is 100 for the received prefix + test_func = functools.partial(_bgp_has_routes, router2, 0, 100) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "r2 does not receive routes with metric 0 and weight 100" + + # Update the route-map and add "on-match next" to entry 10 + cmd = """ + configure terminal + route-map RM permit 10 + on-match next + exit + """ + router2.vtysh_cmd(cmd) + + # Check that metric is 20 and weight is 100 for the received prefix + test_func = functools.partial(_bgp_has_routes, router2, 20, 100) + success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5) + assert result is None, "r2 does not receive routes with metric 20 and weight 100" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/conftest.py b/tests/topotests/conftest.py index f829ed2d12..0afebde1cf 100755 --- a/tests/topotests/conftest.py +++ b/tests/topotests/conftest.py @@ -298,49 +298,53 @@ def check_for_memleaks(): def check_for_core_dumps(): - dumps = [] tgen = get_topogen() # pylint: disable=redefined-outer-name - latest = [] + if not tgen: + return - if tgen is not None: - logdir = tgen.logdir - cores = glob.glob(os.path.join(logdir, "*/*.dmp")) + if not hasattr(tgen, "existing_core_files"): + tgen.existing_core_files = set() + existing = tgen.existing_core_files - if cores: - logger.error("Cores found:\n\t%s", "\n\t".join(cores)) - pytest.fail("Core files found") + cores = glob.glob(os.path.join(tgen.logdir, "*/*.dmp")) + latest = {x for x in cores if x not in existing} + if latest: + existing |= latest + tgen.existing_core_files = existing + + emsg = "New core[s] found: " + ", ".join(latest) + logger.error(emsg) + pytest.fail(emsg) def check_for_backtraces(): - backtraces = [] tgen = get_topogen() # pylint: disable=redefined-outer-name - latest = [] - existing = [] - if tgen is not None: - logdir = tgen.logdir - if hasattr(tgen, "backtraces_existing_files"): - existing = tgen.backtraces_existing_files - latest = glob.glob(os.path.join(logdir, "*/*.log")) + if not tgen: + return - daemons = [] + if not hasattr(tgen, "existing_backtrace_files"): + tgen.existing_backtrace_files = {} + existing = tgen.existing_backtrace_files + + latest = glob.glob(os.path.join(tgen.logdir, "*/*.log")) + backtraces = [] for vfile in latest: - if vfile in existing: - continue with open(vfile, encoding="ascii") as vf: vfcontent = vf.read() - backtrace = vfcontent.count("Backtrace:") - if backtrace: - existing.append(vfile) # have backtrace don't check again - emsg = "Backtrace found in {}, failing test".format(vfile) - backtraces.append(emsg) - - if tgen is not None: - tgen.backtrace_existing_files = existing + btcount = vfcontent.count("Backtrace:") + if not btcount: + continue + if vfile not in existing: + existing[vfile] = 0 + if btcount == existing[vfile]: + continue + existing[vfile] = btcount + backtraces.append(vfile) if backtraces: - logger.error("Backtraces found in test suite, erroring") - logger.error(backtraces) - pytest.fail("Backtraces found") + emsg = "New backtraces found in: " + ", ".join(backtraces) + logger.error(emsg) + pytest.fail(emsg) @pytest.fixture(autouse=True, scope="module") @@ -366,8 +370,6 @@ def module_check_memtest(request): if request.config.option.memleaks: if get_topogen() is not None: check_for_memleaks() - check_for_backtraces() - check_for_core_dumps() # @@ -397,9 +399,13 @@ def pytest_runtest_call(item: pytest.Item) -> None: # Let the default pytest_runtest_call execute the test function yield + check_for_backtraces() + check_for_core_dumps() + # Check for leaks if requested if item.config.option.valgrind_memleaks: check_for_valgrind_memleaks() + if item.config.option.memleaks: check_for_memleaks() diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py index 7371230057..da24b45297 100644 --- a/tests/topotests/lib/topotest.py +++ b/tests/topotests/lib/topotest.py @@ -44,33 +44,89 @@ def get_logs_path(rundir): def gdb_core(obj, daemon, corefiles): - gdbcmds = """ - info threads - bt full - disassemble - up - disassemble - up - disassemble - up - disassemble - up - disassemble - up - disassemble + gdbcmds = r""" +set print elements 1024 +echo -------\n +echo threads\n +echo -------\n +info threads +echo ---------\n +echo registers\n +echo ---------\n +info registers +echo ---------\n +echo backtrace\n +echo ---------\n +bt """ gdbcmds = [["-ex", i.strip()] for i in gdbcmds.strip().split("\n")] gdbcmds = [item for sl in gdbcmds for item in sl] daemon_path = os.path.join(obj.daemondir, daemon) - backtrace = subprocess.check_output( - ["gdb", daemon_path, corefiles[0], "--batch"] + gdbcmds + p = subprocess.run( + ["gdb", daemon_path, corefiles[0], "--batch"] + gdbcmds, + encoding="utf-8", + errors="ignore", + capture_output=True, ) + backtrace = p.stdout + + # + # Grab the disassemble of top couple frames + # + m = re.search(r"#(\d+) .*assert.*", backtrace) + if not m: + m = re.search(r"#(\d+) .*abort.*", backtrace) + frames = re.findall(r"\n#(\d+) ", backtrace) + if m: + frstart = -1 + astart = int(m.group(1)) + 1 + ocount = f"-{int(frames[-1]) - astart + 1}" + else: + astart = -1 + frstart = 0 + ocount = "" + m = re.search(r"#(\d+) .*core_handler.*", backtrace) + if m: + frstart = int(m.group(1)) + 2 + ocount = f"-{int(frames[-1]) - frstart + 1}" + sys.stderr.write( - "\n%s: %s crashed. Core file found - Backtrace follows:\n" % (obj.name, daemon) + f"\nCORE FOUND: {obj.name}: {daemon} crashed: see log for backtrace and more\n" ) - sys.stderr.write("%s" % backtrace) - return backtrace + + gdbcmds = rf""" +set print elements 1024 +echo -------------------------\n +echo backtrace with local args\n +echo -------------------------\n +bt full {ocount} +""" + if frstart >= 0: + gdbcmds += rf"""echo ---------------------------------------\n +echo disassemble of failing funciton (guess)\n +echo ---------------------------------------\n +fr {frstart} +disassemble /m +""" + + gdbcmds = [["-ex", i.strip()] for i in gdbcmds.strip().split("\n")] + gdbcmds = [item for sl in gdbcmds for item in sl] + + daemon_path = os.path.join(obj.daemondir, daemon) + p = subprocess.run( + ["gdb", daemon_path, corefiles[0], "-q", "--batch"] + gdbcmds, + encoding="utf-8", + errors="ignore", + capture_output=True, + ) + btdump = p.stdout + + # sys.stderr.write( + # "\n%s: %s crashed. Core file found - Backtrace follows:\n" % (obj.name, daemon) + # ) + + return backtrace + btdump class json_cmp_result(object): @@ -1189,8 +1245,8 @@ def rlimit_atleast(rname, min_value, raises=False): def fix_netns_limits(ns): # Maximum read and write socket buffer sizes - sysctl_atleast(ns, "net.ipv4.tcp_rmem", [10 * 1024, 87380, 16 * 2 ** 20]) - sysctl_atleast(ns, "net.ipv4.tcp_wmem", [10 * 1024, 87380, 16 * 2 ** 20]) + sysctl_atleast(ns, "net.ipv4.tcp_rmem", [10 * 1024, 87380, 16 * 2**20]) + sysctl_atleast(ns, "net.ipv4.tcp_wmem", [10 * 1024, 87380, 16 * 2**20]) sysctl_assure(ns, "net.ipv4.conf.all.rp_filter", 0) sysctl_assure(ns, "net.ipv4.conf.default.rp_filter", 0) @@ -1249,8 +1305,8 @@ def fix_host_limits(): sysctl_atleast(None, "net.core.netdev_max_backlog", 4 * 1024) # Maximum read and write socket buffer sizes - sysctl_atleast(None, "net.core.rmem_max", 16 * 2 ** 20) - sysctl_atleast(None, "net.core.wmem_max", 16 * 2 ** 20) + sysctl_atleast(None, "net.core.rmem_max", 16 * 2**20) + sysctl_atleast(None, "net.core.wmem_max", 16 * 2**20) # Garbage Collection Settings for ARP and Neighbors sysctl_atleast(None, "net.ipv4.neigh.default.gc_thresh2", 4 * 1024) @@ -2088,8 +2144,7 @@ class Router(Node): backtrace = gdb_core(self, daemon, corefiles) traces = ( traces - + "\n%s: %s crashed. Core file found - Backtrace follows:\n%s" - % (self.name, daemon, backtrace) + + f"\nCORE FOUND: {self.name}: {daemon} crashed. Backtrace follows:\n{backtrace}" ) reportMade = True elif reportLeaks: diff --git a/tests/topotests/mgmt_config/r1/frr.conf b/tests/topotests/mgmt_config/r1/frr.conf new file mode 100644 index 0000000000..076715c068 --- /dev/null +++ b/tests/topotests/mgmt_config/r1/frr.conf @@ -0,0 +1,15 @@ +debug northbound notifications +! debug northbound libyang +debug northbound events +debug northbound callbacks +debug mgmt backend datastore frontend transaction +debug mgmt client backend +debug mgmt client frontend + +log timestamp precision 6 +log file frr.log debug + +interface r1-eth0 + ip address 101.0.0.1/24 + ipv6 address 2101::1/64 +exit
\ No newline at end of file diff --git a/tests/topotests/mgmt_config/test_regression.py b/tests/topotests/mgmt_config/test_regression.py new file mode 100644 index 0000000000..00c3e01724 --- /dev/null +++ b/tests/topotests/mgmt_config/test_regression.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 eval: (blacken-mode 1) -*- +# SPDX-License-Identifier: ISC +# +# July 13 2023, Christian Hopps <chopps@labn.net> +# +# Copyright (c) 2023, LabN Consulting, L.L.C. +# +""" +Test mgmtd regressions + +""" +import pytest +from lib.topogen import Topogen + +pytestmark = [pytest.mark.staticd] + + +@pytest.fixture(scope="module") +def tgen(request): + "Setup/Teardown the environment and provide tgen argument to tests" + + topodef = {"s1": ("r1",)} + tgen = Topogen(topodef, request.module.__name__) + tgen.start_topology() + tgen.gears["r1"].load_frr_config("frr.conf") + tgen.start_router() + yield tgen + tgen.stop_topology() + + +def test_regression_issue_13920(tgen): + """Issue #13920 + + ubuntu2204# conf t + ubuntu2204(config)# ip route 3.2.4.0/24 6.5.5.11 loop3 + ubuntu2204(config)# nexthop-group nh2 + ubuntu2204(config-nh-group)# nexthop 6.5.5.12 + ubuntu2204(config-nh-group)# exi + ubuntu2204(config)# ip route 3.22.4.0/24 6.5.5.12 + crash + """ + + r1 = tgen.gears["r1"] + r1.vtysh_multicmd( + """ + conf t + nexthop-group nh2 + exit + ip route 3.22.4.0/24 6.5.5.12 + """ + ) + output = r1.net.checkRouterCores() + assert not output.strip() diff --git a/tests/topotests/pytest.ini b/tests/topotests/pytest.ini index 0062cf3de2..6c2d42ef40 100644 --- a/tests/topotests/pytest.ini +++ b/tests/topotests/pytest.ini @@ -6,7 +6,8 @@ # We always turn this on inside conftest.py, default shown # addopts = --junitxml=<rundir>/topotests.xml -log_level = DEBUG +# This affects what gets dumped to the screen on test failure +log_level = ERROR log_format = %(asctime)s,%(msecs)03d %(levelname)s: %(name)s: %(message)s log_date_format = %Y-%m-%d %H:%M:%S diff --git a/tools/etc/frr/support_bundle_commands.conf b/tools/etc/frr/support_bundle_commands.conf index 914363157a..a727f86a03 100644 --- a/tools/etc/frr/support_bundle_commands.conf +++ b/tools/etc/frr/support_bundle_commands.conf @@ -68,6 +68,8 @@ show ip nht vrf all show ipv6 nht vrf all show ip route vrf all show ipv6 route vrf all +show ip fib +show ipv6 fib show nexthop-group rib show route-map show memory diff --git a/tools/frrinit.sh.in b/tools/frrinit.sh.in index 42adefb9ea..428d57c55b 100644 --- a/tools/frrinit.sh.in +++ b/tools/frrinit.sh.in @@ -123,7 +123,7 @@ reload) NEW_CONFIG_FILE="${2:-$C_PATH/frr.conf}" [ ! -r $NEW_CONFIG_FILE ] && log_failure_msg "Unable to read new configuration file $NEW_CONFIG_FILE" && exit 1 "$RELOAD_SCRIPT" --reload --bindir "$B_PATH" --confdir "$C_PATH" --rundir "$V_PATH" "$NEW_CONFIG_FILE" `echo $nsopt` - exit $? + exit 0 ;; *) diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index ee52a98adb..8b223d1aa4 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -2356,8 +2356,14 @@ static int vtysh_exit(struct vty *vty) if (vty->node == CONFIG_NODE) { /* resync in case one of the daemons is somewhere else */ vtysh_execute("end"); - vtysh_execute("configure terminal file-lock"); + /* NOTE: a rather expensive thing to do, can we avoid it? */ + + if (vty->vtysh_file_locked) + vtysh_execute("configure terminal file-lock"); + else + vtysh_execute("configure terminal"); } + return CMD_SUCCESS; } diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c index a5f790bbc6..593de7ffbc 100644 --- a/vtysh/vtysh_config.c +++ b/vtysh/vtysh_config.c @@ -607,7 +607,8 @@ static int vtysh_read_file(FILE *confp, bool dry_run) vty->node = CONFIG_NODE; vtysh_execute_no_pager("enable"); - vtysh_execute_no_pager("configure terminal file-lock"); + vtysh_execute_no_pager("conf term file-lock"); + vty->vtysh_file_locked = true; if (!dry_run) vtysh_execute_no_pager("XFRR_start_configuration"); @@ -619,6 +620,7 @@ static int vtysh_read_file(FILE *confp, bool dry_run) vtysh_execute_no_pager("XFRR_end_configuration"); vtysh_execute_no_pager("end"); + vty->vtysh_file_locked = false; vtysh_execute_no_pager("disable"); vty_close(vty); diff --git a/zebra/interface.c b/zebra/interface.c index 989763d13c..4006f9c574 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -787,6 +787,15 @@ void if_delete_update(struct interface **pifp) /* Delete connected routes from the kernel. */ if_delete_connected(ifp); + /* if the ifp is in a vrf, move it to default so vrf can be deleted if + * desired. This operation is not done for netns implementation to avoid + * collision with interface with the same name in the default vrf (can + * occur with this implementation whereas it is not possible with + * vrf-lite). + */ + if (ifp->vrf->vrf_id && !vrf_is_backend_netns()) + if_handle_vrf_change(ifp, VRF_DEFAULT); + /* Send out notification on interface delete. */ zebra_interface_delete_update(ifp); @@ -800,15 +809,6 @@ void if_delete_update(struct interface **pifp) if_set_index(ifp, IFINDEX_INTERNAL); ifp->node = NULL; - /* if the ifp is in a vrf, move it to default so vrf can be deleted if - * desired. This operation is not done for netns implementation to avoid - * collision with interface with the same name in the default vrf (can - * occur with this implementation whereas it is not possible with - * vrf-lite). - */ - if (ifp->vrf->vrf_id && !vrf_is_backend_netns()) - if_handle_vrf_change(ifp, VRF_DEFAULT); - UNSET_FLAG(ifp->status, ZEBRA_INTERFACE_VRF_LOOPBACK); /* Reset some zebra interface params to default values. */ diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index bb507893b0..977ea425d5 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -2366,17 +2366,18 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe, if (is_default_prefix(&rn->p) && !rnh_resolve_via_default(zvrf, p.family)) { if (IS_ZEBRA_DEBUG_RIB_DETAILED) - zlog_debug( - " :%s: Resolved against default route", - __func__); + zlog_debug(" :%s: %pFX Resolved against default route", + __func__, &p); return 0; } dest = rib_dest_from_rnode(rn); - if (dest && dest->selected_fib - && !CHECK_FLAG(dest->selected_fib->status, - ROUTE_ENTRY_REMOVED) - && dest->selected_fib->type != ZEBRA_ROUTE_TABLE) + if (dest && dest->selected_fib && + (!CHECK_FLAG(dest->selected_fib->status, + ROUTE_ENTRY_REMOVED) || + CHECK_FLAG(dest->selected_fib->status, + ROUTE_ENTRY_ROUTE_REPLACING)) && + dest->selected_fib->type != ZEBRA_ROUTE_TABLE) match = dest->selected_fib; /* If there is no selected route or matched route is EGP, go up @@ -2388,7 +2389,6 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe, } while (rn && rn->info == NULL); if (rn) route_lock_node(rn); - continue; } diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 93b33c9abd..a5e036fe48 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2824,8 +2824,13 @@ static void process_subq_early_route_add(struct zebra_early_route *ere) rib_addnode(rn, re, 1); /* Free implicit route.*/ - if (same) + if (same) { + rib_dest_t *dest = rn->info; + + if (same == dest->selected_fib) + SET_FLAG(same->status, ROUTE_ENTRY_ROUTE_REPLACING); rib_delnode(rn, same); + } /* See if we can remove some RE entries that are queued for * removal, but won't be considered in rib processing. diff --git a/zebra/zebra_routemap.c b/zebra/zebra_routemap.c index 142501b149..eb94e26c30 100644 --- a/zebra/zebra_routemap.c +++ b/zebra/zebra_routemap.c @@ -367,7 +367,7 @@ static int ip_nht_rm_add(struct zebra_vrf *zvrf, const char *rmap, int rtype, route_map_counter_increment(NHT_RM_MAP(zvrf, afi, rtype)); if (NHT_RM_MAP(zvrf, afi, rtype)) - zebra_evaluate_rnh(zvrf, AFI_IP, 1, NULL, SAFI_UNICAST); + zebra_evaluate_rnh(zvrf, afi, 1, NULL, SAFI_UNICAST); return CMD_SUCCESS; } @@ -388,7 +388,7 @@ static int ip_nht_rm_del(struct zebra_vrf *zvrf, const char *rmap, int rtype, zvrf->vrf->vrf_id, rtype); NHT_RM_MAP(zvrf, afi, rtype) = NULL; - zebra_evaluate_rnh(zvrf, AFI_IP, 1, NULL, SAFI_UNICAST); + zebra_evaluate_rnh(zvrf, afi, 1, NULL, SAFI_UNICAST); } XFREE(MTYPE_ROUTE_MAP_NAME, NHT_RM_NAME(zvrf, afi, rtype)); } @@ -1703,7 +1703,7 @@ static void zebra_nht_rm_update(const char *rmap) afi_ipv6 = 1; zebra_evaluate_rnh( - zvrf, AFI_IP, 1, NULL, + zvrf, AFI_IP6, 1, NULL, SAFI_UNICAST); } } |