diff options
24 files changed, 313 insertions, 376 deletions
diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c index ac5cdd6acb..f3c308afb9 100644 --- a/bgpd/bgp_clist.c +++ b/bgpd/bgp_clist.c @@ -659,9 +659,6 @@ bool community_list_match(struct community *com, struct community_list *list) struct community_entry *entry; for (entry = list->head; entry; entry = entry->next) { - if (entry->any) - return entry->direct == COMMUNITY_PERMIT; - if (entry->style == COMMUNITY_LIST_STANDARD) { if (community_include(entry->u.com, COMMUNITY_INTERNET)) return entry->direct == COMMUNITY_PERMIT; @@ -681,9 +678,6 @@ bool lcommunity_list_match(struct lcommunity *lcom, struct community_list *list) struct community_entry *entry; for (entry = list->head; entry; entry = entry->next) { - if (entry->any) - return entry->direct == COMMUNITY_PERMIT; - if (entry->style == LARGE_COMMUNITY_LIST_STANDARD) { if (lcommunity_match(lcom, entry->u.lcom)) return entry->direct == COMMUNITY_PERMIT; @@ -705,9 +699,6 @@ bool lcommunity_list_exact_match(struct lcommunity *lcom, struct community_entry *entry; for (entry = list->head; entry; entry = entry->next) { - if (entry->any) - return entry->direct == COMMUNITY_PERMIT; - if (entry->style == LARGE_COMMUNITY_LIST_STANDARD) { if (lcommunity_cmp(lcom, entry->u.lcom)) return entry->direct == COMMUNITY_PERMIT; @@ -724,9 +715,6 @@ bool ecommunity_list_match(struct ecommunity *ecom, struct community_list *list) struct community_entry *entry; for (entry = list->head; entry; entry = entry->next) { - if (entry->any) - return entry->direct == COMMUNITY_PERMIT; - if (entry->style == EXTCOMMUNITY_LIST_STANDARD) { if (ecommunity_match(ecom, entry->u.ecom)) return entry->direct == COMMUNITY_PERMIT; @@ -746,9 +734,6 @@ bool community_list_exact_match(struct community *com, struct community_entry *entry; for (entry = list->head; entry; entry = entry->next) { - if (entry->any) - return entry->direct == COMMUNITY_PERMIT; - if (entry->style == COMMUNITY_LIST_STANDARD) { if (community_include(entry->u.com, COMMUNITY_INTERNET)) return entry->direct == COMMUNITY_PERMIT; @@ -781,28 +766,18 @@ struct community *community_list_match_delete(struct community *com, val = community_val_get(com, i); for (entry = list->head; entry; entry = entry->next) { - if (entry->any) { - if (entry->direct == COMMUNITY_PERMIT) { - com_index_to_delete[delete_index] = i; - delete_index++; - } - break; - } - - else if ((entry->style == COMMUNITY_LIST_STANDARD) - && (community_include(entry->u.com, - COMMUNITY_INTERNET) - || community_include(entry->u.com, val))) { + if ((entry->style == COMMUNITY_LIST_STANDARD) && + (community_include(entry->u.com, + COMMUNITY_INTERNET) || + community_include(entry->u.com, val))) { if (entry->direct == COMMUNITY_PERMIT) { com_index_to_delete[delete_index] = i; delete_index++; } break; - } - - else if ((entry->style == COMMUNITY_LIST_EXPANDED) - && community_regexp_include(entry->reg, com, - i)) { + } else if ((entry->style == COMMUNITY_LIST_EXPANDED) && + community_regexp_include(entry->reg, com, + i)) { if (entry->direct == COMMUNITY_PERMIT) { com_index_to_delete[delete_index] = i; delete_index++; @@ -836,12 +811,6 @@ static bool community_list_dup_check(struct community_list *list, if (entry->direct != new->direct) continue; - if (entry->any != new->any) - continue; - - if (entry->any) - return true; - switch (entry->style) { case COMMUNITY_LIST_STANDARD: if (community_cmp(entry->u.com, new->u.com)) @@ -910,7 +879,6 @@ int community_list_set(struct community_list_handler *ch, const char *name, entry = community_entry_new(); entry->direct = direct; entry->style = style; - entry->any = (str ? false : true); entry->u.com = com; entry->reg = regex; entry->seq = seqnum; @@ -987,16 +955,8 @@ struct lcommunity *lcommunity_list_match_delete(struct lcommunity *lcom, for (i = 0; i < lcom->size; i++) { ptr = lcom->val + (i * LCOMMUNITY_SIZE); for (entry = list->head; entry; entry = entry->next) { - if (entry->any) { - if (entry->direct == COMMUNITY_PERMIT) { - com_index_to_delete[delete_index] = i; - delete_index++; - } - break; - } - - else if ((entry->style == LARGE_COMMUNITY_LIST_STANDARD) - && lcommunity_include(entry->u.lcom, ptr)) { + if ((entry->style == LARGE_COMMUNITY_LIST_STANDARD) && + lcommunity_include(entry->u.lcom, ptr)) { if (entry->direct == COMMUNITY_PERMIT) { com_index_to_delete[delete_index] = i; delete_index++; @@ -1004,9 +964,10 @@ struct lcommunity *lcommunity_list_match_delete(struct lcommunity *lcom, break; } - else if ((entry->style == LARGE_COMMUNITY_LIST_EXPANDED) - && lcommunity_regexp_include(entry->reg, lcom, - i)) { + else if ((entry->style == + LARGE_COMMUNITY_LIST_EXPANDED) && + lcommunity_regexp_include(entry->reg, lcom, + i)) { if (entry->direct == COMMUNITY_PERMIT) { com_index_to_delete[delete_index] = i; delete_index++; @@ -1125,7 +1086,6 @@ int lcommunity_list_set(struct community_list_handler *ch, const char *name, entry = community_entry_new(); entry->direct = direct; entry->style = style; - entry->any = (str ? false : true); entry->u.lcom = lcom; entry->reg = regex; entry->seq = seqnum; @@ -1246,7 +1206,6 @@ int extcommunity_list_set(struct community_list_handler *ch, const char *name, entry = community_entry_new(); entry->direct = direct; entry->style = style; - entry->any = false; if (ecom) entry->config = ecommunity_ecom2str( ecom, ECOMMUNITY_FORMAT_COMMUNITY_LIST, 0); diff --git a/bgpd/bgp_clist.h b/bgpd/bgp_clist.h index 7a9b28038c..8e5d637bab 100644 --- a/bgpd/bgp_clist.h +++ b/bgpd/bgp_clist.h @@ -65,9 +65,6 @@ struct community_entry { /* Standard or expanded. */ uint8_t style; - /* Any match. */ - bool any; - /* Sequence number. */ int64_t seq; diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 04bdba1345..7063bc26ab 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -20863,16 +20863,13 @@ static const char *community_list_config_str(struct community_entry *entry) { const char *str; - if (entry->any) - str = ""; - else { - if (entry->style == COMMUNITY_LIST_STANDARD) - str = community_str(entry->u.com, false, false); - else if (entry->style == LARGE_COMMUNITY_LIST_STANDARD) - str = lcommunity_str(entry->u.lcom, false, false); - else - str = entry->config; - } + if (entry->style == COMMUNITY_LIST_STANDARD) + str = community_str(entry->u.com, false, false); + else if (entry->style == LARGE_COMMUNITY_LIST_STANDARD) + str = lcommunity_str(entry->u.lcom, false, false); + else + str = entry->config; + return str; } @@ -20895,13 +20892,8 @@ static void community_list_show(struct vty *vty, struct community_list *list) : "expanded", list->name); } - if (entry->any) - vty_out(vty, " %s\n", - community_direct_str(entry->direct)); - else - vty_out(vty, " %s %s\n", - community_direct_str(entry->direct), - community_list_config_str(entry)); + vty_out(vty, " %s %s\n", community_direct_str(entry->direct), + community_list_config_str(entry)); } } @@ -21260,13 +21252,8 @@ static void lcommunity_list_show(struct vty *vty, struct community_list *list) : "expanded", list->name); } - if (entry->any) - vty_out(vty, " %s\n", - community_direct_str(entry->direct)); - else - vty_out(vty, " %s %s\n", - community_direct_str(entry->direct), - community_list_config_str(entry)); + vty_out(vty, " %s %s\n", community_direct_str(entry->direct), + community_list_config_str(entry)); } } @@ -21562,13 +21549,8 @@ static void extcommunity_list_show(struct vty *vty, struct community_list *list) : "expanded", list->name); } - if (entry->any) - vty_out(vty, " %s\n", - community_direct_str(entry->direct)); - else - vty_out(vty, " %s %s\n", - community_direct_str(entry->direct), - community_list_config_str(entry)); + vty_out(vty, " %s %s\n", community_direct_str(entry->direct), + community_list_config_str(entry)); } } diff --git a/ldpd/init.c b/ldpd/init.c index 15d653b747..f0cb98e5c0 100644 --- a/ldpd/init.c +++ b/ldpd/init.c @@ -31,13 +31,13 @@ send_init(struct nbr *nbr) if ((buf = ibuf_open(size)) == NULL) fatal(__func__); - err |= gen_ldp_hdr(buf, size); + SET_FLAG(err, gen_ldp_hdr(buf, size)); size -= LDP_HDR_SIZE; - err |= gen_msg_hdr(buf, MSG_TYPE_INIT, size); - err |= gen_init_prms_tlv(buf, nbr); - err |= gen_cap_dynamic_tlv(buf); - err |= gen_cap_twcard_tlv(buf, 1); - err |= gen_cap_unotif_tlv(buf, 1); + SET_FLAG(err, gen_msg_hdr(buf, MSG_TYPE_INIT, size)); + SET_FLAG(err, gen_init_prms_tlv(buf, nbr)); + SET_FLAG(err, gen_cap_dynamic_tlv(buf)); + SET_FLAG(err, gen_cap_twcard_tlv(buf, 1)); + SET_FLAG(err, gen_cap_unotif_tlv(buf, 1)); if (err) { ibuf_free(buf); return; @@ -121,62 +121,56 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len) return (-1); case TLV_TYPE_DYNAMIC_CAP: if (tlv_len != CAP_TLV_DYNAMIC_LEN) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } - if (caps_rcvd & F_CAP_TLV_RCVD_DYNAMIC) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, - msg.type); + if (CHECK_FLAG(caps_rcvd, F_CAP_TLV_RCVD_DYNAMIC)) { + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); return (-1); } - caps_rcvd |= F_CAP_TLV_RCVD_DYNAMIC; + SET_FLAG(caps_rcvd, F_CAP_TLV_RCVD_DYNAMIC); - nbr->flags |= F_NBR_CAP_DYNAMIC; + SET_FLAG(nbr->flags, F_NBR_CAP_DYNAMIC); log_debug("%s: lsr-id %pI4 announced the Dynamic Capability Announcement capability", __func__, &nbr->id); break; case TLV_TYPE_TWCARD_CAP: if (tlv_len != CAP_TLV_TWCARD_LEN) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } - if (caps_rcvd & F_CAP_TLV_RCVD_TWCARD) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, - msg.type); + if (CHECK_FLAG(caps_rcvd, F_CAP_TLV_RCVD_TWCARD)) { + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); return (-1); } - caps_rcvd |= F_CAP_TLV_RCVD_TWCARD; + SET_FLAG(caps_rcvd, F_CAP_TLV_RCVD_TWCARD); - nbr->flags |= F_NBR_CAP_TWCARD; + SET_FLAG(nbr->flags, F_NBR_CAP_TWCARD); log_debug("%s: lsr-id %pI4 announced the Typed Wildcard FEC capability", __func__, &nbr->id); break; case TLV_TYPE_UNOTIF_CAP: if (tlv_len != CAP_TLV_UNOTIF_LEN) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } - if (caps_rcvd & F_CAP_TLV_RCVD_UNOTIF) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, - msg.type); + if (CHECK_FLAG(caps_rcvd, F_CAP_TLV_RCVD_UNOTIF)) { + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); return (-1); } - caps_rcvd |= F_CAP_TLV_RCVD_UNOTIF; + SET_FLAG(caps_rcvd, F_CAP_TLV_RCVD_UNOTIF); - nbr->flags |= F_NBR_CAP_UNOTIF; + SET_FLAG(nbr->flags, F_NBR_CAP_UNOTIF); log_debug("%s: lsr-id %pI4 announced the Unrecognized Notification capability", __func__, &nbr->id); break; default: - if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) + if (!CHECK_FLAG(ntohs(tlv.type), UNKNOWN_FLAG)) send_notification_rtlvs(nbr, S_UNSSUPORTDCAP, msg.id, msg.type, tlv_type, tlv_len, buf); /* ignore unknown tlv */ @@ -217,16 +211,16 @@ send_capability(struct nbr *nbr, uint16_t capability, int enable) if ((buf = ibuf_open(size)) == NULL) fatal(__func__); - err |= gen_ldp_hdr(buf, size); + SET_FLAG(err, gen_ldp_hdr(buf, size)); size -= LDP_HDR_SIZE; - err |= gen_msg_hdr(buf, MSG_TYPE_CAPABILITY, size); + SET_FLAG(err, gen_msg_hdr(buf, MSG_TYPE_CAPABILITY, size)); switch (capability) { case TLV_TYPE_TWCARD_CAP: - err |= gen_cap_twcard_tlv(buf, enable); + SET_FLAG(err, gen_cap_twcard_tlv(buf, enable)); break; case TLV_TYPE_UNOTIF_CAP: - err |= gen_cap_unotif_tlv(buf, enable); + SET_FLAG(err, gen_cap_unotif_tlv(buf, enable)); break; case TLV_TYPE_DYNAMIC_CAP: /* @@ -288,52 +282,47 @@ recv_capability(struct nbr *nbr, char *buf, uint16_t len) switch (tlv_type) { case TLV_TYPE_TWCARD_CAP: if (tlv_len != CAP_TLV_TWCARD_LEN) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } - if (caps_rcvd & F_CAP_TLV_RCVD_TWCARD) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, - msg.type); + if (CHECK_FLAG(caps_rcvd, F_CAP_TLV_RCVD_TWCARD)) { + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); return (-1); } - caps_rcvd |= F_CAP_TLV_RCVD_TWCARD; + SET_FLAG(caps_rcvd, F_CAP_TLV_RCVD_TWCARD); memcpy(&reserved, buf, sizeof(reserved)); enable = reserved & STATE_BIT; if (enable) - nbr->flags |= F_NBR_CAP_TWCARD; + SET_FLAG(nbr->flags, F_NBR_CAP_TWCARD); else - nbr->flags &= ~F_NBR_CAP_TWCARD; + UNSET_FLAG(nbr->flags, F_NBR_CAP_TWCARD); log_debug("%s: lsr-id %pI4 %s the Typed Wildcard FEC capability", __func__, &nbr->id, (enable) ? "announced" : "withdrew"); break; case TLV_TYPE_UNOTIF_CAP: if (tlv_len != CAP_TLV_UNOTIF_LEN) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } - if (caps_rcvd & F_CAP_TLV_RCVD_UNOTIF) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, - msg.type); + if (CHECK_FLAG(caps_rcvd, F_CAP_TLV_RCVD_UNOTIF)) { + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); return (-1); } - caps_rcvd |= F_CAP_TLV_RCVD_UNOTIF; + SET_FLAG(caps_rcvd, F_CAP_TLV_RCVD_UNOTIF); memcpy(&reserved, buf, sizeof(reserved)); enable = reserved & STATE_BIT; if (enable) - nbr->flags |= F_NBR_CAP_UNOTIF; + SET_FLAG(nbr->flags, F_NBR_CAP_UNOTIF); else - nbr->flags &= ~F_NBR_CAP_UNOTIF; + UNSET_FLAG(nbr->flags, F_NBR_CAP_UNOTIF); log_debug("%s: lsr-id %pI4 %s the Unrecognized Notification capability", __func__, - &nbr->id, (enable) ? "announced" : - "withdrew"); + &nbr->id, (enable) ? "announced" : "withdrew"); break; case TLV_TYPE_DYNAMIC_CAP: /* @@ -346,7 +335,7 @@ recv_capability(struct nbr *nbr, char *buf, uint16_t len) */ /* FALLTHROUGH */ default: - if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) + if (!CHECK_FLAG(ntohs(tlv.type), UNKNOWN_FLAG)) send_notification_rtlvs(nbr, S_UNSSUPORTDCAP, msg.id, msg.type, tlv_type, tlv_len, buf); /* ignore unknown tlv */ diff --git a/lib/northbound.c b/lib/northbound.c index 775f6ff92f..ef2344ee11 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -792,18 +792,19 @@ static void nb_update_candidate_changes(struct nb_config *candidate, LYD_TREE_DFS_BEGIN (root, dnode) { op = nb_lyd_diff_get_op(dnode); switch (op) { - case 'c': + case 'c': /* create */ nb_config_diff_created(dnode, seq, cfg_chgs); LYD_TREE_DFS_continue = 1; break; - case 'd': + case 'd': /* delete */ nb_config_diff_deleted(dnode, seq, cfg_chgs); LYD_TREE_DFS_continue = 1; break; - case 'r': + case 'r': /* replace */ nb_config_diff_add_change(cfg_chgs, NB_OP_MODIFY, seq, dnode); break; + case 'n': /* none */ default: break; } @@ -2420,6 +2420,7 @@ void vty_close(struct vty *vty) vty->status = VTY_CLOSE; if (mgmt_fe_client && vty->mgmt_session_id) { + MGMTD_FE_CLIENT_DBG("closing vty session"); mgmt_fe_destroy_client_session(mgmt_fe_client, vty->mgmt_client_id); vty->mgmt_session_id = 0; diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c index 2d01f8ecad..e4a62951d2 100644 --- a/mgmtd/mgmt_be_adapter.c +++ b/mgmtd/mgmt_be_adapter.c @@ -564,8 +564,8 @@ mgmt_be_adapter_handle_msg(struct mgmt_be_client_adapter *adapter, return 0; } -static int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id, bool create) +int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id, bool create) { Mgmtd__BeMessage be_msg; Mgmtd__BeTxnReq txn_req; @@ -584,11 +584,10 @@ static int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter, return mgmt_be_adapter_send_msg(adapter, &be_msg); } -static int -mgmt_be_send_cfgdata_create_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id, uint64_t batch_id, - Mgmtd__YangCfgDataReq **cfgdata_reqs, - size_t num_reqs, bool end_of_data) +int mgmt_be_send_cfgdata_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id, uint64_t batch_id, + Mgmtd__YangCfgDataReq **cfgdata_reqs, + size_t num_reqs, bool end_of_data) { Mgmtd__BeMessage be_msg; Mgmtd__BeCfgDataCreateReq cfgdata_req; @@ -612,8 +611,8 @@ mgmt_be_send_cfgdata_create_req(struct mgmt_be_client_adapter *adapter, return mgmt_be_adapter_send_msg(adapter, &be_msg); } -static int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id) +int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id) { Mgmtd__BeMessage be_msg; Mgmtd__BeCfgDataApplyReq apply_req; @@ -834,35 +833,6 @@ int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, return 0; } -int mgmt_be_create_txn(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id) -{ - return mgmt_be_send_txn_req(adapter, txn_id, true); -} - -int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id) -{ - return mgmt_be_send_txn_req(adapter, txn_id, false); -} - -int mgmt_be_send_cfg_data_create_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id, uint64_t batch_id, - struct mgmt_be_cfgreq *cfg_req, - bool end_of_data) -{ - return mgmt_be_send_cfgdata_create_req( - adapter, txn_id, batch_id, cfg_req->cfgdata_reqs, - cfg_req->num_reqs, end_of_data); -} - -extern int -mgmt_be_send_cfg_apply_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id) -{ - return mgmt_be_send_cfgapply_req(adapter, txn_id); -} - void mgmt_be_get_subscr_info_for_xpath( const char *xpath, struct mgmt_be_client_subscr_info *subscr_info) { diff --git a/mgmtd/mgmt_be_adapter.h b/mgmtd/mgmt_be_adapter.h index 8f4eef5fb3..e1676e63af 100644 --- a/mgmtd/mgmt_be_adapter.h +++ b/mgmtd/mgmt_be_adapter.h @@ -115,13 +115,9 @@ mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter, struct mgmt_ds_ctx *ds_ctx, struct nb_config_cbs **cfg_chgs); -/* Create a transaction. */ -extern int mgmt_be_create_txn(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id); - -/* Destroy a transaction. */ -extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id); +/* Create/destroy a transaction. */ +extern int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id, bool create); /* * Send config data create request to backend client. @@ -135,8 +131,11 @@ extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter, * batch_id * Request batch ID. * - * cfg_req - * Config data request. + * cfgdata_reqs + * An array of pointer to Mgmtd__YangCfgDataReq. + * + * num_reqs + * Length of the cfgdata_reqs array. * * end_of_data * TRUE if the data from last batch, FALSE otherwise. @@ -144,37 +143,15 @@ extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter, * Returns: * 0 on success, -1 on failure. */ -extern int mgmt_be_send_cfg_data_create_req( - struct mgmt_be_client_adapter *adapter, uint64_t txn_id, - uint64_t batch_id, struct mgmt_be_cfgreq *cfg_req, bool end_of_data); - -/* - * Send config validate request to backend client. - * - * adaptr - * Backend adapter information. - * - * txn_id - * Unique transaction identifier. - * - * batch_ids - * List of request batch IDs. - * - * num_batch_ids - * Number of batch ids. - * - * Returns: - * 0 on success, -1 on failure. - */ -extern int -mgmt_be_send_cfg_validate_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id, uint64_t batch_ids[], - size_t num_batch_ids); +extern int mgmt_be_send_cfgdata_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id, uint64_t batch_id, + Mgmtd__YangCfgDataReq **cfgdata_reqs, + size_t num_reqs, bool end_of_data); /* * Send config apply request to backend client. * - * adaptr + * adapter * Backend adapter information. * * txn_id @@ -183,9 +160,8 @@ mgmt_be_send_cfg_validate_req(struct mgmt_be_client_adapter *adapter, * Returns: * 0 on success, -1 on failure. */ -extern int -mgmt_be_send_cfg_apply_req(struct mgmt_be_client_adapter *adapter, - uint64_t txn_id); +extern int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter, + uint64_t txn_id); /* * Dump backend adapter status to vty. diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index 3d818cb4c2..588693b7e3 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -635,7 +635,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread) txn->session_id); FOREACH_TXN_REQ_IN_LIST (&txn->set_cfg_reqs, txn_req) { - error = false; assert(txn_req->req_event == MGMTD_TXN_PROC_SETCFG); ds_ctx = txn_req->req.set_cfg->ds_ctx; if (!ds_ctx) { @@ -644,7 +643,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread) txn_req->req.set_cfg->ds_id, txn_req->req_id, MGMTD_INTERNAL_ERROR, "No such datastore!", txn_req->req.set_cfg->implicit_commit); - error = true; goto mgmt_txn_process_set_cfg_done; } @@ -656,7 +654,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread) MGMTD_INTERNAL_ERROR, "Unable to retrieve DS Config Tree!", txn_req->req.set_cfg->implicit_commit); - error = true; goto mgmt_txn_process_set_cfg_done; } @@ -713,7 +710,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread) "Failed to send SET_CONFIG_REPLY txn-id %" PRIu64 " session-id: %" PRIu64, txn->txn_id, txn->session_id); - error = true; } mgmt_txn_process_set_cfg_done: @@ -1122,7 +1118,6 @@ static int mgmt_txn_create_config_batches(struct mgmt_txn_req *txn_req, } free(xpath); - xpath = NULL; } cmtcfg_req->cmt_stats->last_batch_cnt = num_chgs; @@ -1338,8 +1333,7 @@ static int mgmt_txn_send_be_txn_create(struct mgmt_txn_ctx *txn) FOREACH_MGMTD_BE_CLIENT_ID (id) { if (cmtcfg_req->subscr_info.xpath_subscr[id]) { adapter = mgmt_be_get_adapter_by_id(id); - if (mgmt_be_create_txn(adapter, txn->txn_id) - != 0) { + if (mgmt_be_send_txn_req(adapter, txn->txn_id, true)) { (void)mgmt_txn_send_commit_cfg_reply( txn, MGMTD_INTERNAL_ERROR, "Could not send TXN_CREATE to backend adapter"); @@ -1372,9 +1366,8 @@ static int mgmt_txn_send_be_txn_create(struct mgmt_txn_ctx *txn) return 0; } -static int -mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn, - struct mgmt_be_client_adapter *adapter) +static int mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn, + struct mgmt_be_client_adapter *adapter) { struct mgmt_commit_cfg_req *cmtcfg_req; struct mgmt_txn_be_cfg_batch *cfg_btch; @@ -1396,10 +1389,10 @@ mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn, cfg_req.cfgdata_reqs = cfg_btch->cfg_datap; cfg_req.num_reqs = cfg_btch->num_cfg_data; indx++; - if (mgmt_be_send_cfg_data_create_req( - adapter, txn->txn_id, cfg_btch->batch_id, &cfg_req, - indx == num_batches ? true : false) - != 0) { + if (mgmt_be_send_cfgdata_req( + adapter, txn->txn_id, cfg_btch->batch_id, + cfg_req.cfgdata_reqs, cfg_req.num_reqs, + indx == num_batches ? true : false)) { (void)mgmt_txn_send_commit_cfg_reply( txn, MGMTD_INTERNAL_ERROR, "Internal Error! Could not send config data to backend!"); @@ -1419,7 +1412,7 @@ mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn, } /* - * This could ne the last Backend Client to send CFGDATA_CREATE_REQ to. + * This could be the last Backend Client to send CFGDATA_CREATE_REQ to. * Try moving the commit to next phase. */ mgmt_try_move_commit_to_next_phase(txn, cmtcfg_req); @@ -1439,7 +1432,7 @@ mgmt_txn_send_be_txn_delete(struct mgmt_txn_ctx *txn, cmtcfg_req = &txn->commit_cfg_req->req.commit_cfg; if (cmtcfg_req->subscr_info.xpath_subscr[adapter->id]) { adapter = mgmt_be_get_adapter_by_id(adapter->id); - (void)mgmt_be_destroy_txn(adapter, txn->txn_id); + (void)mgmt_be_send_txn_req(adapter, txn->txn_id, false); FOREACH_TXN_CFG_BATCH_IN_LIST ( &txn->commit_cfg_req->req.commit_cfg @@ -1512,8 +1505,7 @@ static int mgmt_txn_send_be_cfg_apply(struct mgmt_txn_ctx *txn) return -1; btch_list = &cmtcfg_req->curr_batches[id]; - if (mgmt_be_send_cfg_apply_req(adapter, txn->txn_id) - != 0) { + if (mgmt_be_send_cfgapply_req(adapter, txn->txn_id)) { (void)mgmt_txn_send_commit_cfg_reply( txn, MGMTD_INTERNAL_ERROR, "Could not send CFG_APPLY_REQ to backend adapter"); @@ -2261,11 +2253,6 @@ uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type) return txn ? txn->txn_id : MGMTD_TXN_ID_NONE; } -bool mgmt_txn_id_is_valid(uint64_t txn_id) -{ - return mgmt_txn_id2ctx(txn_id) ? true : false; -} - void mgmt_destroy_txn(uint64_t *txn_id) { struct mgmt_txn_ctx *txn; @@ -2278,17 +2265,6 @@ void mgmt_destroy_txn(uint64_t *txn_id) *txn_id = MGMTD_TXN_ID_NONE; } -enum mgmt_txn_type mgmt_get_txn_type(uint64_t txn_id) -{ - struct mgmt_txn_ctx *txn; - - txn = mgmt_txn_id2ctx(txn_id); - if (!txn) - return MGMTD_TXN_TYPE_NONE; - - return txn->type; -} - int mgmt_txn_send_set_config_req(uint64_t txn_id, uint64_t req_id, Mgmtd__DatastoreId ds_id, struct mgmt_ds_ctx *ds_ctx, @@ -2542,7 +2518,7 @@ int mgmt_txn_notify_be_cfgdata_reply( { struct mgmt_txn_ctx *txn; struct mgmt_txn_be_cfg_batch *cfg_btch; - struct mgmt_commit_cfg_req *cmtcfg_req = NULL; + struct mgmt_commit_cfg_req *cmtcfg_req; txn = mgmt_txn_id2ctx(txn_id); if (!txn || txn->type != MGMTD_TXN_TYPE_CONFIG) diff --git a/mgmtd/mgmt_txn.h b/mgmtd/mgmt_txn.h index be781ab954..0718397138 100644 --- a/mgmtd/mgmt_txn.h +++ b/mgmtd/mgmt_txn.h @@ -101,16 +101,6 @@ extern uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type); extern void mgmt_destroy_txn(uint64_t *txn_id); /* - * Check if transaction is valid given an ID. - */ -extern bool mgmt_txn_id_is_valid(uint64_t txn_id); - -/* - * Returns the type of transaction given an ID. - */ -extern enum mgmt_txn_type mgmt_get_txn_type(uint64_t txn_id); - -/* * Send set-config request to be processed later in transaction. * * txn_id diff --git a/tests/topotests/babel_topo1/r1/babeld.conf b/tests/topotests/babel_topo1/r1/babeld.conf index 372d2edff1..4058362cc3 100644 --- a/tests/topotests/babel_topo1/r1/babeld.conf +++ b/tests/topotests/babel_topo1/r1/babeld.conf @@ -1,4 +1,3 @@ -log file eigrpd.log interface r1-eth0 babel hello-interval 1000 diff --git a/tests/topotests/babel_topo1/r2/babeld.conf b/tests/topotests/babel_topo1/r2/babeld.conf index 8a36dda5f8..bae4e59e0b 100644 --- a/tests/topotests/babel_topo1/r2/babeld.conf +++ b/tests/topotests/babel_topo1/r2/babeld.conf @@ -1,4 +1,3 @@ -log file eigrpd.log ! interface r2-eth0 babel hello-interval 1000 diff --git a/tests/topotests/babel_topo1/r3/babeld.conf b/tests/topotests/babel_topo1/r3/babeld.conf index 1e9dc261f5..bfda3622dd 100644 --- a/tests/topotests/babel_topo1/r3/babeld.conf +++ b/tests/topotests/babel_topo1/r3/babeld.conf @@ -1,4 +1,3 @@ -log file eigrpd.log ! interface r3-eth0 babel hello-interval 1000 diff --git a/tests/topotests/bgp_gr_functionality_topo3/bgp_gr_functionality_topo3.py b/tests/topotests/bgp_gr_functionality_topo3/test_bgp_gr_functionality_topo3.py index 593a8d6417..593a8d6417 100644 --- a/tests/topotests/bgp_gr_functionality_topo3/bgp_gr_functionality_topo3.py +++ b/tests/topotests/bgp_gr_functionality_topo3/test_bgp_gr_functionality_topo3.py diff --git a/tests/topotests/conftest.py b/tests/topotests/conftest.py index b78a2f1052..cb25d63a36 100755 --- a/tests/topotests/conftest.py +++ b/tests/topotests/conftest.py @@ -4,6 +4,7 @@ Topotest conftest.py file. """ # pylint: disable=consider-using-f-string +import contextlib import glob import logging import os @@ -12,6 +13,7 @@ import resource import subprocess import sys import time +from pathlib import Path import lib.fixtures import pytest @@ -41,6 +43,30 @@ except (AttributeError, ImportError): pass +# Remove this and use munet version when we move to pytest_asyncio +@contextlib.contextmanager +def chdir(ndir, desc=""): + odir = os.getcwd() + os.chdir(ndir) + if desc: + logging.debug("%s: chdir from %s to %s", desc, odir, ndir) + try: + yield + finally: + if desc: + logging.debug("%s: chdir back from %s to %s", desc, ndir, odir) + os.chdir(odir) + + +@contextlib.contextmanager +def log_handler(basename, logpath): + topolog.logstart(basename, logpath) + try: + yield + finally: + topolog.logfinish(basename, logpath) + + def pytest_addoption(parser): """ Add topology-only option to the topology tester. This option makes pytest @@ -272,6 +298,20 @@ def check_for_memleaks(): @pytest.fixture(autouse=True, scope="module") +def module_autouse(request): + basename = get_test_logdir(request.node.nodeid, True) + logdir = Path(topotest.g_pytest_config.option.rundir) / basename + logpath = logdir / "exec.log" + + subprocess.check_call("mkdir -p -m 1777 {}".format(logdir), shell=True) + + with log_handler(basename, logpath): + sdir = os.path.dirname(os.path.realpath(request.fspath)) + with chdir(sdir, "module autouse fixture"): + yield + + +@pytest.fixture(autouse=True, scope="module") def module_check_memtest(request): yield if request.config.option.valgrind_memleaks: @@ -282,14 +322,19 @@ def module_check_memtest(request): check_for_memleaks() -def pytest_runtest_logstart(nodeid, location): - # location is (filename, lineno, testname) - topolog.logstart(nodeid, location, topotest.g_pytest_config.option.rundir) - - -def pytest_runtest_logfinish(nodeid, location): - # location is (filename, lineno, testname) - topolog.logfinish(nodeid, location) +# +# Disable per test function logging as FRR CI system can't handle it. +# +# @pytest.fixture(autouse=True, scope="function") +# def function_autouse(request): +# # For tests we actually use the logdir name as the logfile base +# logbase = get_test_logdir(nodeid=request.node.nodeid, module=False) +# logbase = os.path.join(topotest.g_pytest_config.option.rundir, logbase) +# logpath = Path(logbase) +# path = Path(f"{logpath.parent}/exec-{logpath.name}.log") +# subprocess.check_call("mkdir -p -m 1777 {}".format(logpath.parent), shell=True) +# with log_handler(request.node.nodeid, path): +# yield @pytest.hookimpl(hookwrapper=True) @@ -340,8 +385,10 @@ def pytest_configure(config): os.environ["PYTEST_TOPOTEST_WORKER"] = "" is_xdist = os.environ["PYTEST_XDIST_MODE"] != "no" is_worker = False + wname = "" else: - os.environ["PYTEST_TOPOTEST_WORKER"] = os.environ["PYTEST_XDIST_WORKER"] + wname = os.environ["PYTEST_XDIST_WORKER"] + os.environ["PYTEST_TOPOTEST_WORKER"] = wname is_xdist = True is_worker = True @@ -375,6 +422,16 @@ def pytest_configure(config): if not config.getoption("--log-file") and not config.getini("log_file"): config.option.log_file = os.path.join(rundir, "exec.log") + # Handle pytest-xdist each worker get's it's own top level log file + # `exec-worker-N.log` + if wname: + wname = wname.replace("gw", "worker-") + cpath = Path(config.option.log_file).absolute() + config.option.log_file = f"{cpath.parent}/{cpath.stem}-{wname}{cpath.suffix}" + elif is_xdist: + cpath = Path(config.option.log_file).absolute() + config.option.log_file = f"{cpath.parent}/{cpath.stem}-xdist{cpath.suffix}" + # Turn on live logging if user specified verbose and the config has a CLI level set if config.getoption("--verbose") and not is_xdist and not config.getini("log_cli"): if config.getoption("--log-cli-level", None) is None: @@ -433,6 +490,10 @@ def pytest_configure(config): @pytest.fixture(autouse=True, scope="session") def setup_session_auto(): + # Aligns logs nicely + logging.addLevelName(logging.WARNING, " WARN") + logging.addLevelName(logging.INFO, " INFO") + if "PYTEST_TOPOTEST_WORKER" not in os.environ: is_worker = False elif not os.environ["PYTEST_TOPOTEST_WORKER"]: diff --git a/tests/topotests/lib/micronet_compat.py b/tests/topotests/lib/micronet_compat.py index d648a120ab..b348c85988 100644 --- a/tests/topotests/lib/micronet_compat.py +++ b/tests/topotests/lib/micronet_compat.py @@ -121,7 +121,7 @@ class Mininet(BaseMunet): g_mnet_inst = None - def __init__(self, rundir=None, pytestconfig=None): + def __init__(self, rundir=None, pytestconfig=None, logger=None): """ Create a Micronet. """ @@ -140,7 +140,7 @@ class Mininet(BaseMunet): # os.umask(0) super(Mininet, self).__init__( - pid=False, rundir=rundir, pytestconfig=pytestconfig + pid=False, rundir=rundir, pytestconfig=pytestconfig, logger=logger ) # From munet/munet/native.py diff --git a/tests/topotests/lib/topogen.py b/tests/topotests/lib/topogen.py index 0e685a97b0..6ddd223e25 100644 --- a/tests/topotests/lib/topogen.py +++ b/tests/topotests/lib/topogen.py @@ -84,7 +84,7 @@ def get_exabgp_cmd(commander=None): """Return the command to use for ExaBGP version < 4.""" if commander is None: - commander = Commander("topogen") + commander = Commander("exabgp", logger=logging.getLogger("exabgp")) def exacmd_version_ok(exacmd): logger.debug("checking %s for exabgp < version 4", exacmd) @@ -107,7 +107,7 @@ def get_exabgp_cmd(commander=None): exacmd = py2_path + " -m exabgp" if exacmd_version_ok(exacmd): return exacmd - py2_path = commander.get_exec_path("python") + py2_path = commander.get_exec_path("python") if py2_path: exacmd = py2_path + " -m exabgp" if exacmd_version_ok(exacmd): @@ -209,7 +209,11 @@ class Topogen(object): # Mininet(Micronet) to build the actual topology. assert not inspect.isclass(topodef) - self.net = Mininet(rundir=self.logdir, pytestconfig=topotest.g_pytest_config) + self.net = Mininet( + rundir=self.logdir, + pytestconfig=topotest.g_pytest_config, + logger=topolog.get_logger("mu", log_level="debug"), + ) # Adjust the parent namespace topotest.fix_netns_limits(self.net) @@ -1090,8 +1094,9 @@ class TopoSwitch(TopoGear): # pylint: disable=too-few-public-methods def __init__(self, tgen, name, **params): + logger = topolog.get_logger(name, log_level="debug") super(TopoSwitch, self).__init__(tgen, name, **params) - tgen.net.add_switch(name) + tgen.net.add_switch(name, logger=logger) def __str__(self): gear = super(TopoSwitch, self).__str__() diff --git a/tests/topotests/lib/topolog.py b/tests/topotests/lib/topolog.py index b501670789..aceb2cb031 100644 --- a/tests/topotests/lib/topolog.py +++ b/tests/topotests/lib/topolog.py @@ -15,13 +15,6 @@ This file defines our logging abstraction. import logging import os -import subprocess -import sys - -if sys.version_info[0] > 2: - pass -else: - pass try: from xdist import is_xdist_controller @@ -31,8 +24,6 @@ except ImportError: return False -BASENAME = "topolog" - # Helper dictionary to convert Topogen logging levels to Python's logging. DEBUG_TOPO2LOGGING = { "debug": logging.DEBUG, @@ -42,13 +33,43 @@ DEBUG_TOPO2LOGGING = { "error": logging.ERROR, "critical": logging.CRITICAL, } -FORMAT = "%(asctime)s.%(msecs)03d %(levelname)s: %(name)s: %(message)s" +FORMAT = "%(asctime)s %(levelname)s: %(name)s: %(message)s" handlers = {} -logger = logging.getLogger("topolog") +logger = logging.getLogger("topo") + + +# Remove this and use munet version when we move to pytest_asyncio +def get_test_logdir(nodeid=None, module=False): + """Get log directory relative pathname.""" + xdist_worker = os.getenv("PYTEST_XDIST_WORKER", "") + mode = os.getenv("PYTEST_XDIST_MODE", "no") + + # nodeid: all_protocol_startup/test_all_protocol_startup.py::test_router_running + # may be missing "::testname" if module is True + if not nodeid: + nodeid = os.environ["PYTEST_CURRENT_TEST"].split(" ")[0] + + cur_test = nodeid.replace("[", "_").replace("]", "_") + if module: + idx = cur_test.rfind("::") + path = cur_test if idx == -1 else cur_test[:idx] + testname = "" + else: + path, testname = cur_test.split("::") + testname = testname.replace("/", ".") + path = path[:-3].replace("/", ".") + # We use different logdir paths based on how xdist is running. + if mode == "each": + if module: + return os.path.join(path, "worker-logs", xdist_worker) + return os.path.join(path, testname, xdist_worker) + assert mode in ("no", "load", "loadfile", "loadscope"), f"Unknown dist mode {mode}" + return path if module else os.path.join(path, testname) -def set_handler(l, target=None): + +def set_handler(lg, target=None): if target is None: h = logging.NullHandler() else: @@ -59,106 +80,81 @@ def set_handler(l, target=None): h.setFormatter(logging.Formatter(fmt=FORMAT)) # Don't filter anything at the handler level h.setLevel(logging.DEBUG) - l.addHandler(h) + lg.addHandler(h) return h -def set_log_level(l, level): +def set_log_level(lg, level): "Set the logging level." # Messages sent to this logger only are created if this level or above. log_level = DEBUG_TOPO2LOGGING.get(level, level) - l.setLevel(log_level) + lg.setLevel(log_level) -def get_logger(name, log_level=None, target=None): - l = logging.getLogger("{}.{}".format(BASENAME, name)) +def reset_logger(lg): + while lg.handlers: + x = lg.handlers.pop() + x.close() + lg.removeHandler(x) - if log_level is not None: - set_log_level(l, log_level) - if target is not None: - set_handler(l, target) - - return l - - -# nodeid: all_protocol_startup/test_all_protocol_startup.py::test_router_running - - -def get_test_logdir(nodeid=None): - """Get log directory relative pathname.""" - xdist_worker = os.getenv("PYTEST_XDIST_WORKER", "") - mode = os.getenv("PYTEST_XDIST_MODE", "no") +def get_logger(name, log_level=None, target=None, reset=True): + lg = logging.getLogger(name) - if not nodeid: - nodeid = os.environ["PYTEST_CURRENT_TEST"].split(" ")[0] + if reset: + reset_logger(lg) - cur_test = nodeid.replace("[", "_").replace("]", "_") - path, testname = cur_test.split("::") - path = path[:-3].replace("/", ".") + if log_level is not None: + set_log_level(lg, log_level) - # We use different logdir paths based on how xdist is running. - if mode == "each": - return os.path.join(path, testname, xdist_worker) - elif mode == "load": - return os.path.join(path, testname) - else: - assert ( - mode == "no" or mode == "loadfile" or mode == "loadscope" - ), "Unknown dist mode {}".format(mode) + if target is not None: + set_handler(lg, target) - return path + return lg -def logstart(nodeid, location, rundir): +def logstart(nodeid, logpath): """Called from pytest before module setup.""" - - mode = os.getenv("PYTEST_XDIST_MODE", "no") worker = os.getenv("PYTEST_TOPOTEST_WORKER", "") + wstr = f" on worker {worker}" if worker else "" + handler_id = nodeid + worker + logpath = logpath.absolute() - # We only per-test log in the workers (or non-dist) - if not worker and mode != "no": - return + logging.debug("logstart: adding logging for %s%s at %s", nodeid, wstr, logpath) + root_logger = logging.getLogger() + handler = logging.FileHandler(logpath, mode="w") + handler.setFormatter(logging.Formatter(FORMAT)) - handler_id = nodeid + worker - assert handler_id not in handlers - - rel_log_dir = get_test_logdir(nodeid) - exec_log_dir = os.path.join(rundir, rel_log_dir) - subprocess.check_call( - "mkdir -p {0} && chmod 1777 {0}".format(exec_log_dir), shell=True - ) - exec_log_path = os.path.join(exec_log_dir, "exec.log") - - # Add test based exec log handler - h = set_handler(logger, exec_log_path) - handlers[handler_id] = h - - if worker: - logger.info( - "Logging on worker %s for %s into %s", worker, handler_id, exec_log_path - ) - else: - logger.info("Logging for %s into %s", handler_id, exec_log_path) + root_logger.addHandler(handler) + handlers[handler_id] = handler + logging.debug("logstart: added logging for %s%s at %s", nodeid, wstr, logpath) + return handler -def logfinish(nodeid, location): - """Called from pytest after module teardown.""" - # This function may not be called if pytest is interrupted. +def logfinish(nodeid, logpath): + """Called from pytest after module teardown.""" worker = os.getenv("PYTEST_TOPOTEST_WORKER", "") - handler_id = nodeid + worker + wstr = f" on worker {worker}" if worker else "" + + root_logger = logging.getLogger() - if handler_id in handlers: - # Remove test based exec log handler - if worker: - logger.info("Closing logs for %s", handler_id) + handler_id = nodeid + worker + if handler_id not in handlers: + logging.critical("can't find log handler to remove") + else: + logging.debug( + "logfinish: removing logging for %s%s at %s", nodeid, wstr, logpath + ) h = handlers[handler_id] - logger.removeHandler(handlers[handler_id]) + root_logger.removeHandler(h) h.flush() h.close() del handlers[handler_id] + logging.debug( + "logfinish: removed logging for %s%s at %s", nodeid, wstr, logpath + ) console_handler = set_handler(logger, None) diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py index 0e96921b7f..845d3e3b53 100644 --- a/tests/topotests/lib/topotest.py +++ b/tests/topotests/lib/topotest.py @@ -24,6 +24,7 @@ import subprocess import sys import tempfile import time +import logging from collections.abc import Mapping from copy import deepcopy @@ -38,7 +39,7 @@ g_pytest_config = None def get_logs_path(rundir): - logspath = topolog.get_test_logdir() + logspath = topolog.get_test_logdir(module=True) return os.path.join(rundir, logspath) @@ -1137,7 +1138,9 @@ def _sysctl_assure(commander, variable, value): def sysctl_atleast(commander, variable, min_value, raises=False): try: if commander is None: - commander = micronet.Commander("topotest") + logger = logging.getLogger("topotest") + commander = micronet.Commander("sysctl", logger=logger) + return _sysctl_atleast(commander, variable, min_value) except subprocess.CalledProcessError as error: logger.warning( @@ -1153,7 +1156,8 @@ def sysctl_atleast(commander, variable, min_value, raises=False): def sysctl_assure(commander, variable, value, raises=False): try: if commander is None: - commander = micronet.Commander("topotest") + logger = logging.getLogger("topotest") + commander = micronet.Commander("sysctl", logger=logger) return _sysctl_assure(commander, variable, value) except subprocess.CalledProcessError as error: logger.warning( diff --git a/tests/topotests/mgmt_startup/test_bigconf.py b/tests/topotests/mgmt_startup/test_bigconf.py index 3b13229af5..4f46c8fabd 100644 --- a/tests/topotests/mgmt_startup/test_bigconf.py +++ b/tests/topotests/mgmt_startup/test_bigconf.py @@ -42,8 +42,10 @@ def tgen(request): tgen = Topogen(topodef, request.module.__name__) tgen.start_topology() + prologue = open(f"{CWD}/r1/mgmtd.conf").read() + confpath = f"{tgen.gears['r1'].gearlogdir}/r1-late-big.conf" - start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath) + start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath, prologue) ROUTE_RANGE[0] = start ROUTE_RANGE[1] = end diff --git a/tests/topotests/mgmt_startup/test_late_bigconf.py b/tests/topotests/mgmt_startup/test_late_bigconf.py index 5e594aba6c..0b5bf38d10 100644 --- a/tests/topotests/mgmt_startup/test_late_bigconf.py +++ b/tests/topotests/mgmt_startup/test_late_bigconf.py @@ -42,8 +42,10 @@ def tgen(request): tgen = Topogen(topodef, request.module.__name__) tgen.start_topology() + prologue = open(f"{CWD}/r1/mgmtd.conf").read() + confpath = f"{tgen.gears['r1'].gearlogdir}/r1-late-big.conf" - start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath) + start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath, prologue) ROUTE_RANGE[0] = start ROUTE_RANGE[1] = end @@ -74,9 +76,23 @@ def test_staticd_latestart(tgen): assert result is not None, "last route present and should not be" step("Starting staticd") + t2 = Timeout(0) r1.startDaemons(["staticd"]) result = check_kernel(r1, ROUTE_RANGE[0], retry_timeout=60) assert result is None, "first route not present and should be" - result = check_kernel(r1, ROUTE_RANGE[1], retry_timeout=60) + logging.info("r1: elapsed time for first route %ss", t2.elapsed()) + + count = 0 + ocount = 0 + while count < ROUTE_COUNT: + rc, o, e = r1.net.cmd_status("ip -o route | wc -l") + if not rc: + if count > ocount + 100: + ocount = count + logging.info("r1: elapsed time for %d routes %s", count, t2.elapsed()) + count = int(o) + + result = check_kernel(r1, ROUTE_RANGE[1], retry_timeout=1200) assert result is None, "last route not present and should be" + logging.info("r1: elapsed time for last route %ss", t2.elapsed()) diff --git a/tests/topotests/mgmt_startup/util.py b/tests/topotests/mgmt_startup/util.py index 87a2ad442e..e366351326 100644 --- a/tests/topotests/mgmt_startup/util.py +++ b/tests/topotests/mgmt_startup/util.py @@ -50,11 +50,13 @@ def get_ip_networks(super_prefix, count): return tuple(network.subnets(count_log2))[0:count] -def write_big_route_conf(super_prefix, count, confpath): +def write_big_route_conf(super_prefix, count, confpath, prologue=""): start = None end = None with open(confpath, "w+", encoding="ascii") as f: + if prologue: + f.write(prologue + "\n") for net in get_ip_networks(super_prefix, count): end = net if not start: diff --git a/zebra/rib.h b/zebra/rib.h index a56bb05d68..65cc1ffab9 100644 --- a/zebra/rib.h +++ b/zebra/rib.h @@ -465,6 +465,13 @@ extern uint8_t route_distance(int type); extern void zebra_rib_evaluate_rn_nexthops(struct route_node *rn, uint32_t seq, bool rt_delete); +/* + * rib_find_rn_from_ctx + * + * Returns a lock increased route_node for the appropriate + * table and prefix specified by the context. Developer + * should unlock the node when done. + */ extern struct route_node * rib_find_rn_from_ctx(const struct zebra_dplane_ctx *ctx); diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 4c6c336d41..4bc9f4acfa 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -801,11 +801,17 @@ int zsend_route_notify_owner(const struct route_node *rn, int zsend_route_notify_owner_ctx(const struct zebra_dplane_ctx *ctx, enum zapi_route_notify_owner note) { - return (route_notify_internal( - rib_find_rn_from_ctx(ctx), dplane_ctx_get_type(ctx), - dplane_ctx_get_instance(ctx), dplane_ctx_get_vrf(ctx), - dplane_ctx_get_table(ctx), note, dplane_ctx_get_afi(ctx), - dplane_ctx_get_safi(ctx))); + int result; + struct route_node *rn = rib_find_rn_from_ctx(ctx); + + result = route_notify_internal( + rn, dplane_ctx_get_type(ctx), dplane_ctx_get_instance(ctx), + dplane_ctx_get_vrf(ctx), dplane_ctx_get_table(ctx), note, + dplane_ctx_get_afi(ctx), dplane_ctx_get_safi(ctx)); + + route_unlock_node(rn); + + return result; } static void zread_route_notify_request(ZAPI_HANDLER_ARGS) |