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 1123b777a238b6e291aec9e182c584ca65101cd7..bdbf0c6fb41deafb1b9700e77bc6764008f9ece8 100644 (file)
--- 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 e292e046d375ac63f37ef67590dc489f22aed088..69b11172068783d11a7164b6d01169e38f57a942 100644 (file)
--- 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 ae89b33b899c39aac926a46135124f0ad812b04d..ab29689f5e1cc0223fd39e765b47376b74e1c073 100644 (file)
--- 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 7b8c202f594b14a8dc3c7a591a04ed63bf7a1728..ca2a0e5d0ceee29508f289134ffaa14b38f855b4 100644 (file)
--- 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 = "<core>";
        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 e9572bc4fca414948d7a4524d3f17034ec9eb4c0..76ba6a442b8ec344721650f2ec7eb8f1ca5a7957 100644 (file)
--- 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 2f9eb23e990b03592f80bfce3e0d9a5dae4da371..0c3de2f43e667769e295a5dd6006edaaec706c40 100644 (file)
--- 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;