From: Alan T. DeKok Date: Mon, 19 Oct 2015 17:22:35 +0000 (-0400) Subject: Add request->state_ctx for session state. X-Git-Tag: release_3_0_11~239 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=49c6146a7c81e5553012e232d8f17fdeea662413;p=thirdparty%2Ffreeradius-server.git Add request->state_ctx for session state. So that we can just move the pointers instead of copying data. This means less mutex contention in state.c --- diff --git a/src/include/radiusd.h b/src/include/radiusd.h index 1123b777a23..bdbf0c6fb41 100644 --- a/src/include/radiusd.h +++ b/src/include/radiusd.h @@ -222,6 +222,8 @@ struct rad_request { VALUE_PAIR *config; //!< #VALUE_PAIR (s) used to set per request parameters //!< for modules and the server core at runtime. + + TALLOC_CTX *state_ctx; //!< for request->state VALUE_PAIR *state; //!< #VALUE_PAIR (s) available over the lifetime of the authentication //!< attempt. Useful where the attempt involves a sequence of //!< many request/challenge packets, like OTP, and EAP. diff --git a/src/main/state.c b/src/main/state.c index e292e046d37..69b11172068 100644 --- a/src/main/state.c +++ b/src/main/state.c @@ -39,7 +39,8 @@ typedef struct state_entry_t { int tries; - VALUE_PAIR *vps; + TALLOC_CTX *ctx; + VALUE_PAIR *vps; void *opaque; void (*free_opaque)(void *opaque); @@ -125,6 +126,9 @@ static void state_entry_free(fr_state_t *state, state_entry_t *entry) (void) talloc_get_type_abort(entry, state_entry_t); #endif rbtree_deletebydata(state->tree, entry); + + if (entry->ctx) talloc_free(entry->ctx); + talloc_free(entry); } @@ -208,7 +212,7 @@ static state_entry_t *fr_state_create(fr_state_t *state, RADIUS_PACKET *packet, * Unused. We can delete it, even if now isn't * the time to clean it up. */ - if (!entry->vps && !entry->opaque) { + if (!entry->ctx && !entry->opaque) { state_entry_free(state, entry); continue; } @@ -252,8 +256,6 @@ static state_entry_t *fr_state_create(fr_state_t *state, RADIUS_PACKET *packet, if (old) { entry->tries = old->tries + 1; - rad_assert(old->vps == NULL); - /* * Track State */ @@ -380,6 +382,7 @@ void fr_state_get_vps(REQUEST *request, RADIUS_PACKET *packet) { state_entry_t *entry; fr_state_t *state = &global_state; + TALLOC_CTX *old_ctx = NULL; rad_assert(request->state == NULL); @@ -399,9 +402,17 @@ void fr_state_get_vps(REQUEST *request, RADIUS_PACKET *packet) * isn't thread-safe. */ if (entry) { - fr_pair_list_mcopy_by_num(request, &request->state, &entry->vps, 0, 0, TAG_ANY); - RDEBUG2("session-state: Found cached attributes"); - rdebug_pair_list(L_DBG_LVL_1, request, request->state, NULL); + RDEBUG2("Restoring &session-state"); + + if (request->state_ctx) old_ctx = request->state_ctx; + + request->state_ctx = entry->ctx; + request->state = entry->vps; + + entry->ctx = NULL; + entry->vps = NULL; + + rdebug_pair_list(L_DBG_LVL_2, request, request->state, "&session-state:"); } else { RDEBUG2("session-state: No cached attributes"); @@ -409,6 +420,11 @@ void fr_state_get_vps(REQUEST *request, RADIUS_PACKET *packet) PTHREAD_MUTEX_UNLOCK(&state->mutex); + /* + * Free this outside of the mutex for less contention. + */ + if (old_ctx) talloc_free(old_ctx); + VERIFY_REQUEST(request); return; } @@ -446,11 +462,13 @@ bool fr_state_put_vps(REQUEST *request, RADIUS_PACKET *original, RADIUS_PACKET * return false; } - /* - * This has to be done in a mutex lock, because talloc - * isn't thread-safe. - */ - fr_pair_list_mcopy_by_num(entry, &entry->vps, &request->state, 0, 0, TAG_ANY); + rad_assert(entry->ctx == NULL); + entry->ctx = request->state_ctx; + entry->vps = request->state; + + request->state_ctx = NULL; + request->state = NULL; + PTHREAD_MUTEX_UNLOCK(&state->mutex); rad_assert(request->state == NULL); diff --git a/src/main/tmpl.c b/src/main/tmpl.c index ae89b33b899..ab29689f5e1 100644 --- a/src/main/tmpl.c +++ b/src/main/tmpl.c @@ -339,7 +339,7 @@ TALLOC_CTX *radius_list_ctx(REQUEST *request, pair_lists_t list) return request; case PAIR_LIST_STATE: - return request; + return request->state_ctx; #ifdef WITH_PROXY case PAIR_LIST_PROXY_REQUEST: diff --git a/src/main/util.c b/src/main/util.c index 7b8c202f594..ca2a0e5d0ce 100644 --- a/src/main/util.c +++ b/src/main/util.c @@ -580,6 +580,13 @@ static int _request_free(REQUEST *request) request->home_server = NULL; #endif + /* + * This is parented separately. + */ + if (request->state_ctx) { + talloc_free(request->state_ctx); + } + return 0; } @@ -613,6 +620,8 @@ REQUEST *request_alloc(TALLOC_CTX *ctx) request->component = ""; request->log.func = vradlog_request; + request->state_ctx = talloc_init("session-state"); + return request; } @@ -1102,7 +1111,7 @@ void verify_request(char const *file, int line, REQUEST *request) #ifdef WITH_VERIFY_PTR fr_pair_list_verify(file, line, request, request->config); - fr_pair_list_verify(file, line, request, request->state); + fr_pair_list_verify(file, line, request->state_ctx, request->state); #endif if (request->packet) verify_packet(file, line, request, request->packet, "request"); diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c index e9572bc4fca..76ba6a442b8 100644 --- a/src/modules/rlm_cache/rlm_cache.c +++ b/src/modules/rlm_cache/rlm_cache.c @@ -143,7 +143,8 @@ static void CC_HINT(nonnull) cache_merge(rlm_cache_t *inst, REQUEST *request, rl if (c->state) { rdebug_pair_list(L_DBG_LVL_2, request, c->state, "&session-state:"); - radius_pairmove(request, &request->state, fr_pair_list_copy(request->state, c->state), false); + + fr_pair_list_mcopy_by_num(request->state_ctx, &request->state, &c->state, 0, 0, TAG_ANY); } if (inst->stats) { diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index 2f9eb23e990..0c3de2f43e6 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -810,7 +810,7 @@ static int do_perl(void *instance, REQUEST *request, char const *function_name) perl_store_vps(request->reply, request, &request->reply->vps, rad_reply_hv, "RAD_REPLY", "reply"); perl_store_vps(request, request, &request->config, rad_check_hv, "RAD_CHECK", "control"); perl_store_vps(request, request, &request->config, rad_config_hv, "RAD_CONFIG", "control"); - perl_store_vps(request, request, &request->state, rad_state_hv, "RAD_STATE", "session-state"); + perl_store_vps(request->state_ctx, request, &request->state, rad_state_hv, "RAD_STATE", "session-state"); #ifdef WITH_PROXY rad_request_proxy_hv = get_hv("RAD_REQUEST_PROXY",1); @@ -889,7 +889,7 @@ static int do_perl(void *instance, REQUEST *request, char const *function_name) vp = NULL; } - if ((get_hv_content(request, request, rad_state_hv, &vp, "RAD_STATE", "session-state")) == 0) { + if ((get_hv_content(request->state_ctx, request, rad_state_hv, &vp, "RAD_STATE", "session-state")) == 0) { fr_pair_list_free(&request->state); request->state = vp; vp = NULL;