Skip to content

Commit

Permalink
Merge pull request FRRouting#14609 from idryzhov/cfg-apply-remove-bat…
Browse files Browse the repository at this point in the history
…ches

mgmtd, lib: remove batch ids from cfg apply reply
  • Loading branch information
choppsv1 authored Oct 25, 2023
2 parents 91c5a47 + d2977d5 commit a709218
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 64 deletions.
5 changes: 2 additions & 3 deletions lib/mgmt.proto
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,8 @@ message BeCfgDataApplyReq {

message BeCfgDataApplyReply {
required uint64 txn_id = 1;
repeated uint64 batch_ids = 2;
required bool success = 3;
optional string error_if_any = 4;
required bool success = 2;
optional string error_if_any = 3;
}

message BeOperDataGetReq {
Expand Down
30 changes: 3 additions & 27 deletions lib/mgmt_be_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,7 @@ static int mgmt_be_process_cfgdata_req(struct mgmt_be_client *client_ctx,
}

static int mgmt_be_send_apply_reply(struct mgmt_be_client *client_ctx,
uint64_t txn_id, uint64_t batch_ids[],
size_t num_batch_ids, bool success,
uint64_t txn_id, bool success,
const char *error_if_any)
{
Mgmtd__BeMessage be_msg;
Expand All @@ -620,8 +619,6 @@ static int mgmt_be_send_apply_reply(struct mgmt_be_client *client_ctx,
mgmtd__be_cfg_data_apply_reply__init(&apply_reply);
apply_reply.success = success;
apply_reply.txn_id = txn_id;
apply_reply.batch_ids = (uint64_t *)batch_ids;
apply_reply.n_batch_ids = num_batch_ids;

if (error_if_any)
apply_reply.error_if_any = (char *)error_if_any;
Expand All @@ -630,12 +627,7 @@ static int mgmt_be_send_apply_reply(struct mgmt_be_client *client_ctx,
be_msg.message_case = MGMTD__BE_MESSAGE__MESSAGE_CFG_APPLY_REPLY;
be_msg.cfg_apply_reply = &apply_reply;

MGMTD_BE_CLIENT_DBG(
"Sending CFG_APPLY_REPLY txn-id %" PRIu64
" %zu batch ids %" PRIu64 " - %" PRIu64,
txn_id, num_batch_ids,
success && num_batch_ids ? batch_ids[0] : 0,
success && num_batch_ids ? batch_ids[num_batch_ids - 1] : 0);
MGMTD_BE_CLIENT_DBG("Sending CFG_APPLY_REPLY txn-id %" PRIu64, txn_id);

return mgmt_be_client_send_msg(client_ctx, &be_msg);
}
Expand All @@ -648,14 +640,11 @@ static int mgmt_be_txn_proc_cfgapply(struct mgmt_be_txn_ctx *txn)
unsigned long apply_nb_cfg_tm;
struct mgmt_be_batch_ctx *batch;
char err_buf[BUFSIZ];
size_t num_processed;
static uint64_t batch_ids[MGMTD_BE_MAX_BATCH_IDS_IN_REQ];

assert(txn && txn->client);
client_ctx = txn->client;

assert(txn->nb_txn);
num_processed = 0;

/*
* Now apply all the batches we have applied in one go.
Expand All @@ -673,9 +662,6 @@ static int mgmt_be_txn_proc_cfgapply(struct mgmt_be_txn_ctx *txn)
client_ctx->num_apply_nb_cfg++;
txn->nb_txn = NULL;

/*
* Send back CFG_APPLY_REPLY for all batches applied.
*/
FOREACH_BE_APPLY_BATCH_IN_LIST (txn, batch) {
/*
* No need to delete the batch yet. Will be deleted during
Expand All @@ -684,19 +670,9 @@ static int mgmt_be_txn_proc_cfgapply(struct mgmt_be_txn_ctx *txn)
SET_FLAG(batch->flags, MGMTD_BE_TXN_FLAGS_CFG_APPLIED);
mgmt_be_batches_del(&txn->apply_cfgs, batch);
mgmt_be_batches_add_tail(&txn->cfg_batches, batch);

batch_ids[num_processed] = batch->batch_id;
num_processed++;
if (num_processed == MGMTD_BE_MAX_BATCH_IDS_IN_REQ) {
mgmt_be_send_apply_reply(client_ctx, txn->txn_id,
batch_ids, num_processed,
true, NULL);
num_processed = 0;
}
}

mgmt_be_send_apply_reply(client_ctx, txn->txn_id, batch_ids,
num_processed, true, NULL);
mgmt_be_send_apply_reply(client_ctx, txn->txn_id, true, NULL);

MGMTD_BE_CLIENT_DBG("Nb-apply-duration %lu (avg: %lu) uSec",
apply_nb_cfg_tm, client_ctx->avg_apply_nb_cfg_tm);
Expand Down
8 changes: 1 addition & 7 deletions mgmtd/mgmt_be_adapter.c
Original file line number Diff line number Diff line change
Expand Up @@ -469,14 +469,10 @@ mgmt_be_adapter_handle_msg(struct mgmt_be_client_adapter *adapter,
case MGMTD__BE_MESSAGE__MESSAGE_CFG_APPLY_REPLY:
MGMTD_BE_ADAPTER_DBG(
"Got %s CFG_APPLY_REPLY from '%s' txn-id %" PRIx64
" for %zu batches id %" PRIu64 "-%" PRIu64 " err:'%s'",
" err:'%s'",
be_msg->cfg_apply_reply->success ? "successful"
: "failed",
adapter->name, be_msg->cfg_apply_reply->txn_id,
be_msg->cfg_apply_reply->n_batch_ids,
be_msg->cfg_apply_reply->batch_ids[0],
be_msg->cfg_apply_reply->batch_ids
[be_msg->cfg_apply_reply->n_batch_ids - 1],
be_msg->cfg_apply_reply->error_if_any
? be_msg->cfg_apply_reply->error_if_any
: "None");
Expand All @@ -486,8 +482,6 @@ mgmt_be_adapter_handle_msg(struct mgmt_be_client_adapter *adapter,
mgmt_txn_notify_be_cfg_apply_reply(
be_msg->cfg_apply_reply->txn_id,
be_msg->cfg_apply_reply->success,
(uint64_t *)be_msg->cfg_apply_reply->batch_ids,
be_msg->cfg_apply_reply->n_batch_ids,
be_msg->cfg_apply_reply->error_if_any, adapter);
break;
case MGMTD__BE_MESSAGE__MESSAGE_GET_REPLY:
Expand Down
38 changes: 13 additions & 25 deletions mgmtd/mgmt_txn.c
Original file line number Diff line number Diff line change
Expand Up @@ -2471,14 +2471,11 @@ int mgmt_txn_notify_be_cfgdata_reply(uint64_t txn_id, uint64_t batch_id,
}

int mgmt_txn_notify_be_cfg_apply_reply(uint64_t txn_id, bool success,
uint64_t batch_ids[],
size_t num_batch_ids, char *error_if_any,
char *error_if_any,
struct mgmt_be_client_adapter *adapter)
{
struct mgmt_txn_ctx *txn;
struct mgmt_txn_be_cfg_batch *batch;
struct mgmt_commit_cfg_req *cmtcfg_req = NULL;
size_t indx;

txn = mgmt_txn_id2ctx(txn_id);
if (!txn || txn->type != MGMTD_TXN_TYPE_CONFIG || !txn->commit_cfg_req)
Expand All @@ -2488,9 +2485,8 @@ int mgmt_txn_notify_be_cfg_apply_reply(uint64_t txn_id, bool success,

if (!success) {
MGMTD_TXN_ERR("CFGDATA_APPLY_REQ sent to '%s' failed txn-id: %" PRIu64
" batch ids %" PRIu64 " - %" PRIu64 " err: %s",
adapter->name, txn->txn_id, batch_ids[0],
batch_ids[num_batch_ids - 1],
" err: %s",
adapter->name, txn->txn_id,
error_if_any ? error_if_any : "None");
mgmt_txn_send_commit_cfg_reply(
txn, MGMTD_INTERNAL_ERROR,
Expand All @@ -2500,25 +2496,17 @@ int mgmt_txn_notify_be_cfg_apply_reply(uint64_t txn_id, bool success,
return 0;
}

for (indx = 0; indx < num_batch_ids; indx++) {
batch = mgmt_txn_cfgbatch_id2ctx(txn, batch_ids[indx]);
if (batch->txn != txn)
return -1;
mgmt_move_txn_cfg_batch_to_next(
cmtcfg_req, batch,
&cmtcfg_req->curr_batches[adapter->id],
&cmtcfg_req->next_batches[adapter->id], true,
MGMTD_COMMIT_PHASE_TXN_DELETE);
}
mgmt_move_txn_cfg_batches(txn, cmtcfg_req,
&cmtcfg_req->curr_batches[adapter->id],
&cmtcfg_req->next_batches[adapter->id],
true, MGMTD_COMMIT_PHASE_TXN_DELETE);

if (!mgmt_txn_batches_count(&cmtcfg_req->curr_batches[adapter->id])) {
/*
* All configuration for the specific backend has been applied.
* Send TXN-DELETE to wrap up the transaction for this backend.
*/
SET_FLAG(adapter->flags, MGMTD_BE_ADAPTER_FLAGS_CFG_SYNCED);
mgmt_txn_send_be_txn_delete(txn, adapter);
}
/*
* All configuration for the specific backend has been applied.
* Send TXN-DELETE to wrap up the transaction for this backend.
*/
SET_FLAG(adapter->flags, MGMTD_BE_ADAPTER_FLAGS_CFG_SYNCED);
mgmt_txn_send_be_txn_delete(txn, adapter);

mgmt_try_move_commit_to_next_phase(txn, cmtcfg_req);
if (mm->perf_stats_en)
Expand Down
3 changes: 1 addition & 2 deletions mgmtd/mgmt_txn.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,7 @@ extern int mgmt_txn_notify_be_cfg_validate_reply(
*/
extern int
mgmt_txn_notify_be_cfg_apply_reply(uint64_t txn_id, bool success,
uint64_t batch_ids[],
size_t num_batch_ids, char *error_if_any,
char *error_if_any,
struct mgmt_be_client_adapter *adapter);

/*
Expand Down

0 comments on commit a709218

Please sign in to comment.