diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index e9c89d2029..9d6ec66689 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -202,7 +202,7 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) return CMD_SUCCESS; implicit_commit = vty_needs_implicit_commit(vty); - ret = vty_mgmt_send_config_data(vty); + ret = vty_mgmt_send_config_data(vty, implicit_commit); if (ret >= 0 && !implicit_commit) vty->mgmt_num_pending_setcfg++; return ret; @@ -229,9 +229,16 @@ int nb_cli_apply_changes_clear_pending(struct vty *vty, if (vty_mgmt_should_process_cli_apply_changes(vty)) { VTY_CHECK_XPATH; - + /* + * The legacy user wanted to clear pending (i.e., perform a + * commit immediately) due to some non-yang compatible + * functionality. This new mgmtd code however, continues to send + * changes putting off the commit until XFRR_end is received + * (i.e., end-of-config-file). This should be fine b/c all + * conversions to mgmtd require full proper implementations. + */ implicit_commit = vty_needs_implicit_commit(vty); - ret = vty_mgmt_send_config_data(vty); + ret = vty_mgmt_send_config_data(vty, implicit_commit); if (ret >= 0 && !implicit_commit) vty->mgmt_num_pending_setcfg++; return ret; diff --git a/lib/vty.c b/lib/vty.c index 972d004cfb..fd00e11c5f 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -3652,7 +3652,7 @@ int vty_mgmt_send_lockds_req(struct vty *vty, Mgmtd__DatastoreId ds_id, return 0; } -int vty_mgmt_send_config_data(struct vty *vty) +int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit) { Mgmtd__YangDataValue value[VTY_MAXCFGCHANGES]; Mgmtd__YangData cfg_data[VTY_MAXCFGCHANGES]; @@ -3660,7 +3660,6 @@ int vty_mgmt_send_config_data(struct vty *vty) Mgmtd__YangCfgDataReq *cfgreq[VTY_MAXCFGCHANGES] = {0}; size_t indx; int cnt; - bool implicit_commit = false; if (vty->type == VTY_FILE) { /* @@ -3734,7 +3733,6 @@ int vty_mgmt_send_config_data(struct vty *vty) } vty->mgmt_req_id++; - implicit_commit = vty_needs_implicit_commit(vty); if (cnt && mgmt_fe_send_setcfg_req( mgmt_fe_client, vty->mgmt_session_id, vty->mgmt_req_id, MGMTD_DS_CANDIDATE, cfgreq, diff --git a/lib/vty.h b/lib/vty.h index 2b9ee4838e..3b651d20a2 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -147,7 +147,6 @@ struct vty { /* Dynamic transaction information. */ bool pending_allowed; bool pending_commit; - bool no_implicit_commit; char *pending_cmds_buf; size_t pending_cmds_buflen; size_t pending_cmds_bufpos; @@ -408,7 +407,7 @@ extern bool vty_mgmt_fe_enabled(void); extern bool vty_mgmt_should_process_cli_apply_changes(struct vty *vty); extern bool mgmt_vty_read_configs(void); -extern int vty_mgmt_send_config_data(struct vty *vty); +extern int vty_mgmt_send_config_data(struct vty *vty, bool implicit_commit); extern int vty_mgmt_send_commit_config(struct vty *vty, bool validate_only, bool abort); extern int vty_mgmt_send_get_config(struct vty *vty, @@ -422,11 +421,7 @@ extern void vty_mgmt_resume_response(struct vty *vty, bool success); static inline bool vty_needs_implicit_commit(struct vty *vty) { - return (frr_get_cli_mode() == FRR_CLI_CLASSIC - ? ((vty->pending_allowed || vty->no_implicit_commit) - ? false - : true) - : false); + return frr_get_cli_mode() == FRR_CLI_CLASSIC && !vty->pending_allowed; } #ifdef __cplusplus diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c index 18aeab711f..e64cbe1425 100644 --- a/mgmtd/mgmt_txn.c +++ b/mgmtd/mgmt_txn.c @@ -494,6 +494,8 @@ static void mgmt_txn_req_free(struct mgmt_txn_req **txn_req) struct mgmt_txn_reqs_head *pending_list = NULL; enum mgmt_be_client_id id; struct mgmt_be_client_adapter *adapter; + struct mgmt_commit_cfg_req *ccreq; + bool cleanup; switch ((*txn_req)->req_event) { case MGMTD_TXN_PROC_SETCFG: @@ -526,32 +528,38 @@ static void mgmt_txn_req_free(struct mgmt_txn_req **txn_req) MGMTD_TXN_DBG("Deleting COMMITCFG req-id: %" PRIu64 " txn-id: %" PRIu64, (*txn_req)->req_id, (*txn_req)->txn->txn_id); + + ccreq = &(*txn_req)->req.commit_cfg; + cleanup = (ccreq->curr_phase >= MGMTD_COMMIT_PHASE_TXN_CREATE && + ccreq->curr_phase < MGMTD_COMMIT_PHASE_TXN_DELETE); + FOREACH_MGMTD_BE_CLIENT_ID (id) { /* * Send TXN_DELETE to cleanup state for this * transaction on backend */ - if ((*txn_req)->req.commit_cfg.curr_phase >= - MGMTD_COMMIT_PHASE_TXN_CREATE && - (*txn_req)->req.commit_cfg.curr_phase < - MGMTD_COMMIT_PHASE_TXN_DELETE && - (*txn_req) - ->req.commit_cfg.subscr_info - .xpath_subscr[id]) { - adapter = mgmt_be_get_adapter_by_id(id); - if (adapter) - mgmt_txn_send_be_txn_delete( - (*txn_req)->txn, adapter); + + /* + * Get rid of the batches first so we don't end up doing + * anything more with them + */ + mgmt_txn_cleanup_be_cfg_batches((*txn_req)->txn, id); + if (ccreq->batches) { + hash_clean(ccreq->batches, + mgmt_txn_cfgbatch_hash_free); + hash_free(ccreq->batches); + ccreq->batches = NULL; } - mgmt_txn_cleanup_be_cfg_batches((*txn_req)->txn, - id); - if ((*txn_req)->req.commit_cfg.batches) { - hash_clean((*txn_req)->req.commit_cfg.batches, - mgmt_txn_cfgbatch_hash_free); - hash_free((*txn_req)->req.commit_cfg.batches); - (*txn_req)->req.commit_cfg.batches = NULL; - } + /* + * If we were in the middle of the state machine then + * send a txn delete message + */ + adapter = mgmt_be_get_adapter_by_id(id); + if (adapter && cleanup && + ccreq->subscr_info.xpath_subscr[id]) + mgmt_txn_send_be_txn_delete((*txn_req)->txn, + adapter); } break; case MGMTD_TXN_PROC_GETCFG: @@ -1424,24 +1432,16 @@ static int mgmt_txn_send_be_txn_delete(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; + struct mgmt_commit_cfg_req *cmtcfg_req = + &txn->commit_cfg_req->req.commit_cfg; - assert(txn->type == MGMTD_TXN_TYPE_CONFIG && txn->commit_cfg_req); + assert(txn->type == MGMTD_TXN_TYPE_CONFIG); + assert(!mgmt_txn_batches_count(&cmtcfg_req->curr_batches[adapter->id])); - 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_send_txn_req(adapter, txn->txn_id, false); + if (!cmtcfg_req->subscr_info.xpath_subscr[adapter->id]) + return 0; - FOREACH_TXN_CFG_BATCH_IN_LIST ( - &txn->commit_cfg_req->req.commit_cfg - .curr_batches[adapter->id], - cfg_btch) - cfg_btch->comm_phase = MGMTD_COMMIT_PHASE_TXN_DELETE; - } - - return 0; + return mgmt_be_send_txn_req(adapter, txn->txn_id, false); } static void mgmt_txn_cfg_commit_timedout(struct event *thread) diff --git a/mgmtd/mgmt_vty.c b/mgmtd/mgmt_vty.c index 93c5145d71..6a6f32353d 100644 --- a/mgmtd/mgmt_vty.c +++ b/mgmtd/mgmt_vty.c @@ -157,9 +157,7 @@ DEFPY(mgmt_set_config_data, mgmt_set_config_data_cmd, vty->cfg_changes[0].operation = NB_OP_CREATE; vty->num_cfg_changes = 1; - vty->no_implicit_commit = true; - vty_mgmt_send_config_data(vty); - vty->no_implicit_commit = false; + vty_mgmt_send_config_data(vty, false); return CMD_SUCCESS; } @@ -176,9 +174,7 @@ DEFPY(mgmt_delete_config_data, mgmt_delete_config_data_cmd, vty->cfg_changes[0].operation = NB_OP_DESTROY; vty->num_cfg_changes = 1; - vty->no_implicit_commit = true; - vty_mgmt_send_config_data(vty); - vty->no_implicit_commit = false; + vty_mgmt_send_config_data(vty, false); return CMD_SUCCESS; }