Merge pull request #13835 from FRRouting/mergify/bp/dev/9.0/pr-13764

lib: mgmtd: make error handling more robust (backport #13764)
This commit is contained in:
Jafar Al-Gharaibeh 2023-06-23 14:03:14 -05:00 committed by GitHub
commit 5b14f854df
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -217,14 +217,16 @@ static void mgmt_be_cleanup_all_batches(struct mgmt_be_txn_ctx *txn)
}
static struct mgmt_be_txn_ctx *
mgmt_be_find_txn_by_id(struct mgmt_be_client *client_ctx, uint64_t txn_id)
mgmt_be_find_txn_by_id(struct mgmt_be_client *client_ctx, uint64_t txn_id,
bool warn)
{
struct mgmt_be_txn_ctx *txn = NULL;
FOREACH_BE_TXN_IN_LIST (client_ctx, txn) {
FOREACH_BE_TXN_IN_LIST (client_ctx, txn)
if (txn->txn_id == txn_id)
return txn;
}
if (warn)
MGMTD_BE_CLIENT_ERR("Unknown txn-id: %" PRIu64, txn_id);
return NULL;
}
@ -234,20 +236,21 @@ mgmt_be_txn_create(struct mgmt_be_client *client_ctx, uint64_t txn_id)
{
struct mgmt_be_txn_ctx *txn = NULL;
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id);
if (!txn) {
txn = XCALLOC(MTYPE_MGMTD_BE_TXN,
sizeof(struct mgmt_be_txn_ctx));
assert(txn);
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id, false);
if (txn) {
MGMTD_BE_CLIENT_ERR("Can't create existing txn-id: %" PRIu64,
txn_id);
return NULL;
}
txn = XCALLOC(MTYPE_MGMTD_BE_TXN, sizeof(struct mgmt_be_txn_ctx));
txn->txn_id = txn_id;
txn->client = client_ctx;
mgmt_be_batches_init(&txn->cfg_batches);
mgmt_be_batches_init(&txn->apply_cfgs);
mgmt_be_txns_add_tail(&client_ctx->txn_head, txn);
MGMTD_BE_CLIENT_DBG("Added new txn-id: %" PRIu64, txn_id);
}
MGMTD_BE_CLIENT_DBG("Created new txn-id: %" PRIu64, txn_id);
return txn;
}
@ -297,7 +300,7 @@ static void mgmt_be_cleanup_all_txns(struct mgmt_be_client *client_ctx)
}
static int mgmt_be_send_txn_reply(struct mgmt_be_client *client_ctx,
uint64_t txn_id, bool create, bool success)
uint64_t txn_id, bool create)
{
Mgmtd__BeMessage be_msg;
Mgmtd__BeTxnReply txn_reply;
@ -305,7 +308,7 @@ static int mgmt_be_send_txn_reply(struct mgmt_be_client *client_ctx,
mgmtd__be_txn_reply__init(&txn_reply);
txn_reply.create = create;
txn_reply.txn_id = txn_id;
txn_reply.success = success;
txn_reply.success = true;
mgmtd__be_message__init(&be_msg);
be_msg.message_case = MGMTD__BE_MESSAGE__MESSAGE_TXN_REPLY;
@ -321,44 +324,29 @@ static int mgmt_be_process_txn_req(struct mgmt_be_client *client_ctx,
{
struct mgmt_be_txn_ctx *txn;
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id);
if (create) {
if (txn) {
/*
* Transaction with same txn-id already exists.
* Should not happen under any circumstances.
*/
MGMTD_BE_CLIENT_ERR(
"txn-id: %" PRIu64 " already exists", txn_id);
mgmt_be_send_txn_reply(client_ctx, txn_id, create,
false);
}
MGMTD_BE_CLIENT_DBG("Creating new txn-id %" PRIu64, txn_id);
MGMTD_BE_CLIENT_DBG("Created new txn-id %" PRIu64, txn_id);
txn = mgmt_be_txn_create(client_ctx, txn_id);
if (!txn)
goto failed;
if (client_ctx->cbs.txn_notify)
(void)(*client_ctx->cbs.txn_notify)(
client_ctx, client_ctx->user_data,
(*client_ctx->cbs.txn_notify)(client_ctx,
client_ctx->user_data,
&txn->client_data, false);
} else {
if (!txn) {
/*
* Transaction with same txn-id does not exists.
* Return sucess anyways.
*/
MGMTD_BE_CLIENT_DBG("txn-id: %" PRIu64
" for delete does NOT exists",
txn_id);
} else {
MGMTD_BE_CLIENT_DBG("Delete txn-id: %" PRIu64, txn_id);
MGMTD_BE_CLIENT_DBG("Deleting txn-id: %" PRIu64, txn_id);
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id, false);
if (txn)
mgmt_be_txn_delete(client_ctx, &txn);
}
}
mgmt_be_send_txn_reply(client_ctx, txn_id, create, true);
return mgmt_be_send_txn_reply(client_ctx, txn_id, create);
return 0;
failed:
msg_conn_disconnect(&client_ctx->client.conn, true);
return -1;
}
static int mgmt_be_send_cfgdata_create_reply(struct mgmt_be_client *client_ctx,
@ -610,25 +598,23 @@ static int mgmt_be_process_cfgdata_req(struct mgmt_be_client *client_ctx,
{
struct mgmt_be_txn_ctx *txn;
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id);
if (!txn) {
MGMTD_BE_CLIENT_ERR("Invalid txn-id: %" PRIu64
" from MGMTD server",
txn_id);
mgmt_be_send_cfgdata_create_reply(
client_ctx, txn_id, batch_id, false,
"Transaction context not created yet");
} else {
mgmt_be_update_setcfg_in_batch(client_ctx, txn, batch_id,
cfg_req, num_req);
}
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id, true);
if (!txn)
goto failed;
mgmt_be_update_setcfg_in_batch(client_ctx, txn, batch_id, cfg_req,
num_req);
if (txn && end_of_data) {
MGMTD_BE_CLIENT_DBG("Triggering CFG_PREPARE_REQ processing");
mgmt_be_txn_cfg_prepare(txn);
MGMTD_BE_CLIENT_DBG("End of data; CFG_PREPARE_REQ processing");
if (mgmt_be_txn_cfg_prepare(txn))
goto failed;
}
return 0;
failed:
msg_conn_disconnect(&client_ctx->client.conn, true);
return -1;
}
static int mgmt_be_send_apply_reply(struct mgmt_be_client *client_ctx,
@ -731,23 +717,28 @@ static int mgmt_be_process_cfg_apply(struct mgmt_be_client *client_ctx,
{
struct mgmt_be_txn_ctx *txn;
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id);
if (!txn) {
mgmt_be_send_apply_reply(client_ctx, txn_id, NULL, 0, false,
"Transaction not created yet!");
txn = mgmt_be_find_txn_by_id(client_ctx, txn_id, true);
if (!txn)
goto failed;
MGMTD_BE_CLIENT_DBG("Trigger CFG_APPLY_REQ processing");
if (mgmt_be_txn_proc_cfgapply(txn))
goto failed;
return 0;
failed:
msg_conn_disconnect(&client_ctx->client.conn, true);
return -1;
}
MGMTD_BE_CLIENT_DBG("Trigger CFG_APPLY_REQ processing");
mgmt_be_txn_proc_cfgapply(txn);
return 0;
}
static int mgmt_be_client_handle_msg(struct mgmt_be_client *client_ctx,
Mgmtd__BeMessage *be_msg)
{
/*
* On error we may have closed the connection so don't do anything with
* the client_ctx on return.
*
* protobuf-c adds a max size enum with an internal, and changing by
* version, name; cast to an int to avoid unhandled enum warnings
*/
@ -888,6 +879,11 @@ static int _notify_conenct_disconnect(struct msg_client *msg_client,
if (client->cbs.client_connect_notify)
(void)(*client->cbs.client_connect_notify)(
client, client->user_data, connected);
/* Cleanup any in-progress TXN on disconnect */
if (!connected)
mgmt_be_cleanup_all_txns(client);
return 0;
}