From f69e5eb2c34220af42b456ce9322401d6041d3ce Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 1 Jun 2018 14:36:55 -0400 Subject: [PATCH] Return separate counts of TID reqs that succeed and result in error * Pass result codes back from req callbacks for tr_gss connections * Separately count TID responses and TID error responses * Add monitoring handlers for the error response * Rename monitoring option #defines to better match the string names * Add more TR_GSS_RC codes * Update trmon documentation string --- common/tr_gss.c | 4 +++- include/mon_internal.h | 7 +++--- include/tid_internal.h | 5 +++-- include/tr_gss.h | 5 ++++- mon/mon_common.c | 15 ++++++++----- mon/mons.c | 17 +++++++------- mon/tests/test_mon_req_decode.c | 4 ++-- mon/tests/test_mon_req_encode.c | 4 ++-- mon/tests/test_mon_resp_encode.c | 4 ++-- tid/tids.c | 48 ++++++++++++++++++++++++++-------------- tr/tr_tid_mons.c | 28 ++++++++++++++++++----- tr/trmon_main.c | 43 +++++++++++++++++------------------ 12 files changed, 113 insertions(+), 71 deletions(-) diff --git a/common/tr_gss.c b/common/tr_gss.c index d576449..251c29e 100644 --- a/common/tr_gss.c +++ b/common/tr_gss.c @@ -274,7 +274,9 @@ TR_GSS_RC tr_gss_handle_connection(int conn, } /* Hand off the request for processing and get the response */ - resp_msg = req_cb(tmp_ctx, req_msg, req_cookie); + rc = req_cb(tmp_ctx, req_msg, &resp_msg, req_cookie); + if (rc != TR_GSS_SUCCESS) + goto cleanup; if (resp_msg == NULL) { // no response, clean up diff --git a/include/mon_internal.h b/include/mon_internal.h index 6d94a60..281e05b 100644 --- a/include/mon_internal.h +++ b/include/mon_internal.h @@ -97,9 +97,10 @@ enum mon_opt_type { // System statistics OPT_TYPE_SHOW_UPTIME, - OPT_TYPE_SHOW_TID_REQ_COUNT, - OPT_TYPE_SHOW_TID_REQ_ERR_COUNT, - OPT_TYPE_SHOW_TID_REQ_PENDING, + OPT_TYPE_SHOW_TID_REQS_PROCESSED, + OPT_TYPE_SHOW_TID_REQS_FAILED, + OPT_TYPE_SHOW_TID_ERROR_COUNT, + OPT_TYPE_SHOW_TID_REQS_PENDING, // Dynamic trust router state OPT_TYPE_SHOW_ROUTES, diff --git a/include/tid_internal.h b/include/tid_internal.h index 220775b..e742c4f 100644 --- a/include/tid_internal.h +++ b/include/tid_internal.h @@ -96,8 +96,9 @@ struct tid_process { }; struct tids_instance { - int req_count; - int error_count; + int req_count; /* successful requests */ + int req_error_count; /* unsuccessful requests */ + int error_count; /* invalid requests or internal errors */ char *priv_key; char *ipaddr; const char *hostname; diff --git a/include/tr_gss.h b/include/tr_gss.h index e0cf770..f7fcd41 100644 --- a/include/tr_gss.h +++ b/include/tr_gss.h @@ -38,10 +38,13 @@ #include typedef int (TR_GSS_AUTH_FN)(gss_name_t, TR_NAME *, void *); -typedef TR_MSG *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, TR_MSG *, void *); +typedef enum tr_gss_rc (TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, TR_MSG *, TR_MSG **, void *); typedef enum tr_gss_rc { TR_GSS_SUCCESS = 0, /* success */ + TR_GSS_AUTH_FAILED, /* authorization failed */ + TR_GSS_REQUEST_FAILED, /* request failed */ + TR_GSS_INTERNAL_ERROR, /* internal error (memory allocation, etc) */ TR_GSS_ERROR, /* unspecified error */ } TR_GSS_RC; diff --git a/mon/mon_common.c b/mon/mon_common.c index 3ca8d16..c302a44 100644 --- a/mon/mon_common.c +++ b/mon/mon_common.c @@ -89,13 +89,16 @@ const char *mon_opt_type_to_string(MON_OPT_TYPE opt_type) case OPT_TYPE_SHOW_UPTIME: return "uptime"; - case OPT_TYPE_SHOW_TID_REQ_COUNT: + case OPT_TYPE_SHOW_TID_REQS_PROCESSED: return "tid_reqs_processed"; - case OPT_TYPE_SHOW_TID_REQ_ERR_COUNT: + case OPT_TYPE_SHOW_TID_REQS_FAILED: + return "tid_reqs_failed"; + + case OPT_TYPE_SHOW_TID_ERROR_COUNT: return "tid_error_count"; - case OPT_TYPE_SHOW_TID_REQ_PENDING: + case OPT_TYPE_SHOW_TID_REQS_PENDING: return "tid_reqs_pending"; case OPT_TYPE_SHOW_ROUTES: @@ -128,9 +131,9 @@ MON_OPT_TYPE mon_opt_type_from_string(const char *s) return_if_matches(s, OPT_TYPE_SHOW_VERSION); return_if_matches(s, OPT_TYPE_SHOW_CONFIG_FILES); return_if_matches(s, OPT_TYPE_SHOW_UPTIME); - return_if_matches(s, OPT_TYPE_SHOW_TID_REQ_COUNT); - return_if_matches(s, OPT_TYPE_SHOW_TID_REQ_ERR_COUNT); - return_if_matches(s, OPT_TYPE_SHOW_TID_REQ_PENDING); + return_if_matches(s, OPT_TYPE_SHOW_TID_REQS_PROCESSED); + return_if_matches(s, OPT_TYPE_SHOW_TID_ERROR_COUNT); + return_if_matches(s, OPT_TYPE_SHOW_TID_REQS_PENDING); return_if_matches(s, OPT_TYPE_SHOW_ROUTES); return_if_matches(s, OPT_TYPE_SHOW_PEERS); return_if_matches(s, OPT_TYPE_SHOW_COMMUNITIES); diff --git a/mon/mons.c b/mon/mons.c index 4aff0d1..b47f30a 100644 --- a/mon/mons.c +++ b/mon/mons.c @@ -111,13 +111,13 @@ MONS_INSTANCE *mons_new(TALLOC_CTX *mem_ctx) * @param data pointer to a MONS_INSTANCE * @return pointer to the response string or null to send no response */ -static TR_MSG *mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, void *data) +static TR_GSS_RC mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, TR_MSG **resp_msg, void *data) { TALLOC_CTX *tmp_ctx = talloc_new(NULL); MONS_INSTANCE *mons = talloc_get_type_abort(data, MONS_INSTANCE); MON_REQ *req = NULL; MON_RESP *resp = NULL; - TR_MSG *resp_msg = NULL; /* This is the response value */ + TR_GSS_RC rc = TR_GSS_ERROR; /* Validate inputs */ if (req_msg == NULL) @@ -134,15 +134,15 @@ static TR_MSG *mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, void *data) } /* Allocate a response message */ - resp_msg = talloc(tmp_ctx, TR_MSG); - if (resp_msg == NULL) { + *resp_msg = talloc(tmp_ctx, TR_MSG); + if (*resp_msg == NULL) { /* can't return a message, just emit an error */ tr_crit("mons_req_cb: Error allocating response message."); goto cleanup; } /* Handle the request */ - resp = mons_handle_request(resp_msg, mons, req); + resp = mons_handle_request(*resp_msg, mons, req); if (resp == NULL) { /* error processing the request */ /* TODO send back an error */ @@ -150,14 +150,15 @@ static TR_MSG *mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, void *data) } /* Set the response message payload */ - tr_msg_set_mon_resp(resp_msg, resp); + tr_msg_set_mon_resp(*resp_msg, resp); /* Put the response message in the caller's context so it does not get freed when we exit */ - talloc_steal(mem_ctx, resp_msg); + talloc_steal(mem_ctx, *resp_msg); + rc = TR_GSS_SUCCESS; cleanup: talloc_free(tmp_ctx); - return resp_msg; + return rc; } /** diff --git a/mon/tests/test_mon_req_decode.c b/mon/tests/test_mon_req_decode.c index 7c76b18..360be1f 100644 --- a/mon/tests/test_mon_req_decode.c +++ b/mon/tests/test_mon_req_decode.c @@ -45,8 +45,8 @@ static MON_REQ *show_all_options() OPT_TYPE_SHOW_CONFIG_FILES, OPT_TYPE_SHOW_VERSION, OPT_TYPE_SHOW_UPTIME, - OPT_TYPE_SHOW_TID_REQ_COUNT, - OPT_TYPE_SHOW_TID_REQ_PENDING, + OPT_TYPE_SHOW_TID_REQS_PROCESSED, + OPT_TYPE_SHOW_TID_REQS_PENDING, OPT_TYPE_SHOW_ROUTES, OPT_TYPE_SHOW_COMMUNITIES, OPT_TYPE_UNKNOWN // terminator diff --git a/mon/tests/test_mon_req_encode.c b/mon/tests/test_mon_req_encode.c index eb48ce9..9fe9b1b 100644 --- a/mon/tests/test_mon_req_encode.c +++ b/mon/tests/test_mon_req_encode.c @@ -89,8 +89,8 @@ int main(void) opts[0] = OPT_TYPE_SHOW_CONFIG_FILES; opts[1] = OPT_TYPE_SHOW_VERSION; opts[2] = OPT_TYPE_SHOW_UPTIME; - opts[3] = OPT_TYPE_SHOW_TID_REQ_COUNT; - opts[4] = OPT_TYPE_SHOW_TID_REQ_PENDING; + opts[3] = OPT_TYPE_SHOW_TID_REQS_PROCESSED; + opts[4] = OPT_TYPE_SHOW_TID_REQS_PENDING; opts[5] = OPT_TYPE_SHOW_ROUTES; opts[6] = OPT_TYPE_SHOW_COMMUNITIES; opts[7] = OPT_TYPE_UNKNOWN; diff --git a/mon/tests/test_mon_resp_encode.c b/mon/tests/test_mon_resp_encode.c index c9d6d2c..3c1dd83 100644 --- a/mon/tests/test_mon_resp_encode.c +++ b/mon/tests/test_mon_resp_encode.c @@ -35,10 +35,10 @@ static char *show_success() mon_opt_type_to_string(OPT_TYPE_SHOW_CONFIG_FILES), json_integer(86400))); assert(! json_object_set_new(payload, - mon_opt_type_to_string(OPT_TYPE_SHOW_TID_REQ_PENDING), + mon_opt_type_to_string(OPT_TYPE_SHOW_TID_REQS_PENDING), json_integer(13))); assert(! json_object_set_new(payload, - mon_opt_type_to_string(OPT_TYPE_SHOW_TID_REQ_COUNT), + mon_opt_type_to_string(OPT_TYPE_SHOW_TID_REQS_PROCESSED), json_integer(1432))); resp = mon_resp_new(NULL, MON_RESP_SUCCESS, "success", payload); diff --git a/tid/tids.c b/tid/tids.c index a984b8d..915e358 100644 --- a/tid/tids.c +++ b/tid/tids.c @@ -262,18 +262,18 @@ int tids_send_response (TIDS_INSTANCE *tids, TID_REQ *req, TID_RESP *resp) * @param data pointer to a TIDS_INSTANCE * @return pointer to the response string or null to send no response */ -static TR_MSG *tids_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *mreq, void *data) +static TR_GSS_RC tids_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *mreq, TR_MSG **mresp, void *data) { TALLOC_CTX *tmp_ctx = talloc_new(NULL); TIDS_INSTANCE *tids = talloc_get_type_abort(data, TIDS_INSTANCE); TID_REQ *req = NULL; TID_RESP *resp = NULL; - TR_MSG *resp_msg = NULL; /* this is the return value */ - int rc = 0; + TR_GSS_RC rc = TR_GSS_ERROR; /* If this isn't a TID Request, just drop it. */ if (mreq->msg_type != TID_REQUEST) { tr_debug("tids_req_cb: Not a TID request, dropped."); + rc = TR_GSS_INTERNAL_ERROR; goto cleanup; } @@ -281,40 +281,45 @@ static TR_MSG *tids_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *mreq, void *data) req = tr_msg_get_req(mreq); /* Allocate a response message */ - resp_msg = talloc(tmp_ctx, TR_MSG); - if (resp_msg == NULL) { + *mresp = talloc(tmp_ctx, TR_MSG); + if (*mresp == NULL) { /* We cannot create a response message, so all we can really do is emit * an error message and return. */ tr_crit("tids_req_cb: Error allocating response message."); + rc = TR_GSS_INTERNAL_ERROR; goto cleanup; } /* Allocate a response structure and populate common fields. Put it in the * response message's talloc context. */ - resp = tids_create_response(resp_msg, req); + resp = tids_create_response(mresp, req); if (resp == NULL) { /* If we were unable to create a response, we cannot reply. Log an * error if we can, then drop the request. */ tr_crit("tids_req_cb: Error creating response structure."); - resp_msg = NULL; /* the contents are in tmp_ctx, so they will still be cleaned up */ + *mresp = NULL; /* the contents are in tmp_ctx, so they will still be cleaned up */ + rc = TR_GSS_INTERNAL_ERROR; goto cleanup; } /* Now officially assign the response to the message. */ - tr_msg_set_resp(resp_msg, resp); + tr_msg_set_resp(*mresp, resp); /* Handle the request and fill in resp */ - rc = tids_handle_request(tids, req, resp); - if (rc < 0) { - tr_debug("tids_req_cb: Error from tids_handle_request(), rc = %d.", rc); + if (tids_handle_request(tids, req, resp) >= 0) + rc = TR_GSS_SUCCESS; + else { + /* The TID request was an error response */ + tr_debug("tids_req_cb: Error from tids_handle_request"); + rc = TR_GSS_REQUEST_FAILED; /* Fall through, to send the response, either way */ } /* put the response message in the caller's context */ - talloc_steal(mem_ctx, resp_msg); + talloc_steal(mem_ctx, *mresp); cleanup: talloc_free(tmp_ctx); - return resp_msg; + return rc; } static int tids_destructor(void *object) @@ -402,8 +407,9 @@ nfds_t tids_get_listener(TIDS_INSTANCE *tids, * TIDS_MAX_MESSAGE_LEN must be longer than the longest message, including * null termination (i.e., strlen() + 1) */ #define TIDS_MAX_MESSAGE_LEN (10) -#define TIDS_SUCCESS_MESSAGE "OK" -#define TIDS_ERROR_MESSAGE "ERR" +#define TIDS_SUCCESS_MESSAGE "OK" /* a success message was sent */ +#define TIDS_ERROR_MESSAGE "ERR" /* an error message was sent */ +#define TIDS_REQ_FAIL_MESSAGE "FAIL" /* sending failed */ /** * Process to handle an incoming TIDS request @@ -430,9 +436,14 @@ static void tids_handle_proc(TIDS_INSTANCE *tids, int conn_fd, int result_fd) response_message = TIDS_SUCCESS_MESSAGE; break; + case TR_GSS_REQUEST_FAILED: + response_message = TIDS_ERROR_MESSAGE; + break; + + case TR_GSS_INTERNAL_ERROR: case TR_GSS_ERROR: default: - response_message = TIDS_ERROR_MESSAGE; + response_message = TIDS_REQ_FAIL_MESSAGE; break; } @@ -565,7 +576,10 @@ void tids_sweep_procs(TIDS_INSTANCE *tids) if ((result_len > 0) && (strcmp(result, TIDS_SUCCESS_MESSAGE) == 0)) { tids->req_count++; - tr_info("tids_sweep_procs: TID process %d exited successfully.", tp.pid); + tr_info("tids_sweep_procs: TID process %d exited after successful request.", tp.pid); + } else if ((result_len > 0) && (strcmp(result, TIDS_ERROR_MESSAGE) == 0)) { + tids->req_error_count++; + tr_info("tids_sweep_procs: TID process %d exited after unsuccessful request.", tp.pid); } else { tids->error_count++; tr_info("tids_sweep_procs: TID process %d exited with an error.", tp.pid); diff --git a/tr/tr_tid_mons.c b/tr/tr_tid_mons.c index 0a18d2d..e9a802b 100644 --- a/tr/tr_tid_mons.c +++ b/tr/tr_tid_mons.c @@ -41,7 +41,7 @@ #include /** - * Get the count of completed TID requests + * Get the count of successfully completed TID requests */ static MON_RC handle_show_req_count(void *cookie, json_t **response_ptr) { @@ -50,7 +50,20 @@ static MON_RC handle_show_req_count(void *cookie, json_t **response_ptr) return (*response_ptr == NULL) ? MON_NOMEM : MON_SUCCESS; } -static MON_RC handle_show_req_err_count(void *cookie, json_t **response_ptr) +/** + * Get the count of completed TID requests that resulted in error responses + */ +static MON_RC handle_show_req_error_count(void *cookie, json_t **response_ptr) +{ + TIDS_INSTANCE *tids = talloc_get_type_abort(cookie, TIDS_INSTANCE); + *response_ptr = json_integer(tids->req_error_count); + return (*response_ptr == NULL) ? MON_NOMEM : MON_SUCCESS; +} + +/** + * Get the count of TID requests that could not be completed + */ +static MON_RC handle_show_error_count(void *cookie, json_t **response_ptr) { TIDS_INSTANCE *tids = talloc_get_type_abort(cookie, TIDS_INSTANCE); *response_ptr = json_integer(tids->error_count); @@ -67,12 +80,15 @@ static MON_RC handle_show_req_pending(void *cookie, json_t **response_ptr) void tr_tid_register_mons_handlers(TIDS_INSTANCE *tids, MONS_INSTANCE *mons) { mons_register_handler(mons, - MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQ_COUNT, + MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQS_PROCESSED, handle_show_req_count, tids); mons_register_handler(mons, - MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQ_ERR_COUNT, - handle_show_req_err_count, tids); + MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQS_FAILED, + handle_show_req_error_count, tids); + mons_register_handler(mons, + MON_CMD_SHOW, OPT_TYPE_SHOW_TID_ERROR_COUNT, + handle_show_error_count, tids); mons_register_handler(mons, - MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQ_PENDING, + MON_CMD_SHOW, OPT_TYPE_SHOW_TID_REQS_PENDING, handle_show_req_pending, tids); } diff --git a/tr/trmon_main.c b/tr/trmon_main.c index 4da3555..fe5e660 100644 --- a/tr/trmon_main.c +++ b/tr/trmon_main.c @@ -55,27 +55,28 @@ const char *argp_program_bug_address=PACKAGE_BUGREPORT; /* bug reporting address /* doc strings */ static const char doc[] = PACKAGE_NAME " - Moonshot Trust Router Monitoring Client" - "\v" /* options list goes here */ - "Supported monitoring commands:\n" - "\n" - " show [