Skip to content

Commit

Permalink
datasets: fix memuse to include string len
Browse files Browse the repository at this point in the history 
So far, when the data size was passed to the THash API, it was sent as
a sizeof(Struct) which works fine for the other data types as they have
a fixed length but not for the StringType.
However, because of the sizeof construct, the length of a string type
dataset was always taken to be 16 Bytes which is only the size of the struct
itself. It did not accomodate the actual size of the string that the
StringType holds. Fix this so that the memuse that is used to determine
whether memcap was reached also takes into consideration the size of the
actual string.

Bug 3910
  • Loading branch information
@inashivb @victorjulien
inashivb authored and victorjulien committed Jul 4, 2024
1 parent 00f7038 commit f1b44ca
Showing 1 changed file with 56 additions and 15 deletions.
71 changes: 56 additions & 15 deletions src/util-thash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
#include "util-hash-lookup3.h"
#include "util-validate.h"

static THashData *THashGetUsed(THashTableContext *ctx);
static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size);
static void THashDataEnqueue (THashDataQueue *q, THashData *h);

void THashDataMoveToSpare(THashTableContext *ctx, THashData *h)
Expand Down Expand Up @@ -157,17 +157,19 @@ static uint32_t THashDataQueueLen(THashDataQueue *q)
}
#endif

static THashData *THashDataAlloc(THashTableContext *ctx)
static THashData *THashDataAlloc(THashTableContext *ctx, uint32_t data_size)
{
const size_t data_size = THASH_DATA_SIZE(ctx);
const size_t thash_data_size = THASH_DATA_SIZE(ctx);

if (!(THASH_CHECK_MEMCAP(ctx, data_size))) {
if (!(THASH_CHECK_MEMCAP(ctx, thash_data_size + data_size))) {
return NULL;
}

(void) SC_ATOMIC_ADD(ctx->memuse, data_size);
size_t total_data_size = thash_data_size + data_size;

THashData *h = SCCalloc(1, data_size);
(void)SC_ATOMIC_ADD(ctx->memuse, total_data_size);

THashData *h = SCCalloc(1, thash_data_size);
if (unlikely(h == NULL))
goto error;

Expand All @@ -181,6 +183,7 @@ static THashData *THashDataAlloc(THashTableContext *ctx)
return h;

error:
(void)SC_ATOMIC_SUB(ctx->memuse, total_data_size);
return NULL;
}

Expand All @@ -189,12 +192,16 @@ static void THashDataFree(THashTableContext *ctx, THashData *h)
if (h != NULL) {
DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) != 0);

uint32_t data_size = 0;
if (h->data != NULL) {
if (ctx->config.DataSize) {
data_size = ctx->config.DataSize(h->data);
}
ctx->config.DataFree(h->data);
}
SCMutexDestroy(&h->m);
SCFree(h);
(void) SC_ATOMIC_SUB(ctx->memuse, THASH_DATA_SIZE(ctx));
(void)SC_ATOMIC_SUB(ctx->memuse, THASH_DATA_SIZE(ctx) + (uint64_t)data_size);
}
}

Expand Down Expand Up @@ -268,7 +275,7 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix)
for (i = 0; i < ctx->config.hash_size; i++) {
HRLOCK_INIT(&ctx->array[i]);
}
(void) SC_ATOMIC_ADD(ctx->memuse, (ctx->config.hash_size * sizeof(THashHashRow)));
(void)SC_ATOMIC_ADD(ctx->memuse, (ctx->config.hash_size * sizeof(THashHashRow)));

/* pre allocate prealloc */
for (i = 0; i < ctx->config.prealloc; i++) {
Expand All @@ -281,7 +288,7 @@ static int THashInitConfig(THashTableContext *ctx, const char *cnf_prefix)
return -1;
}

THashData *h = THashDataAlloc(ctx);
THashData *h = THashDataAlloc(ctx, 0 /* as we don't have string data here */);
if (h == NULL) {
SCLogError("preallocating data failed: %s", strerror(errno));
return -1;
Expand Down Expand Up @@ -374,6 +381,7 @@ void THashShutdown(THashTableContext *ctx)
}
(void) SC_ATOMIC_SUB(ctx->memuse, ctx->config.hash_size * sizeof(THashHashRow));
THashDataQueueDestroy(&ctx->spare_q);
DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(ctx->memuse) != 0);
SCFree(ctx);
}

Expand Down Expand Up @@ -447,6 +455,11 @@ uint32_t THashExpire(THashTableContext *ctx, const SCTime_t ts)
h->next = NULL;
h->prev = NULL;
SCLogDebug("timeout: removing data %p", h);
if (ctx->config.DataSize) {
uint32_t data_size = ctx->config.DataSize(h->data);
if (data_size > 0)
(void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)data_size);
}
ctx->config.DataFree(h->data);
THashDataUnlock(h);
THashDataMoveToSpare(ctx, h);
Expand Down Expand Up @@ -495,6 +508,11 @@ void THashCleanup(THashTableContext *ctx)
hb->tail = h->prev;
h->next = NULL;
h->prev = NULL;
if (ctx->config.DataSize) {
uint32_t data_size = ctx->config.DataSize(h->data);
if (data_size > 0)
(void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)data_size);
}
THashDataMoveToSpare(ctx, h);
h = n;
}
Expand Down Expand Up @@ -537,13 +555,17 @@ static inline int THashCompare(const THashConfig *cnf, void *a, void *b)
static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
{
THashData *h = NULL;
uint32_t data_size = 0;
if (ctx->config.DataSize) {
data_size = ctx->config.DataSize(data);
}

/* get data from the spare queue */
h = THashDataDequeue(&ctx->spare_q);
if (h == NULL) {
/* If we reached the max memcap, we get used data */
if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx)))) {
h = THashGetUsed(ctx);
if (!(THASH_CHECK_MEMCAP(ctx, THASH_DATA_SIZE(ctx) + data_size))) {
h = THashGetUsed(ctx, data_size);
if (h == NULL) {
return NULL;
}
Expand All @@ -555,7 +577,7 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
/* freed data, but it's unlocked */
} else {
/* now see if we can alloc a new data */
h = THashDataAlloc(ctx);
h = THashDataAlloc(ctx, data_size);
if (h == NULL) {
return NULL;
}
Expand All @@ -564,13 +586,24 @@ static THashData *THashDataGetNew(THashTableContext *ctx, void *data)
}
} else {
/* data has been recycled before it went into the spare queue */

/* data is initialized (recycled) but *unlocked* */
/* the recycled data was THashData and again does not include
* the size of current data to be added */
if (data_size > 0) {
/* Since it is prealloc'd data, it already has THashData in its memuse */
(void)SC_ATOMIC_ADD(ctx->memuse, data_size);
if (!(THASH_CHECK_MEMCAP(ctx, data_size))) {
if (!SC_ATOMIC_GET(ctx->memcap_reached)) {
SC_ATOMIC_SET(ctx->memcap_reached, true);
}
SCLogError("Adding data will exceed memcap: %" PRIu64 ", current memuse: %" PRIu64,
(ctx)->config.memcap, SC_ATOMIC_GET(ctx->memuse));
}
}
}

// setup the data
BUG_ON(ctx->config.DataSet(h->data, data) != 0);

(void) SC_ATOMIC_ADD(ctx->counter, 1);
SCMutexLock(&h->m);
return h;
Expand Down Expand Up @@ -765,7 +798,7 @@ THashData *THashLookupFromHash (THashTableContext *ctx, void *data)
*
* \retval h data or NULL
*/
static THashData *THashGetUsed(THashTableContext *ctx)
static THashData *THashGetUsed(THashTableContext *ctx, uint32_t data_size)
{
uint32_t idx = SC_ATOMIC_GET(ctx->prune_idx) % ctx->config.hash_size;
uint32_t cnt = ctx->config.hash_size;
Expand Down Expand Up @@ -811,11 +844,19 @@ static THashData *THashGetUsed(THashTableContext *ctx)
HRLOCK_UNLOCK(hb);

if (h->data != NULL) {
if (ctx->config.DataSize) {
uint32_t h_data_size = ctx->config.DataSize(h->data);
if (h_data_size > 0) {
(void)SC_ATOMIC_SUB(ctx->memuse, (uint64_t)h_data_size);
}
}
ctx->config.DataFree(h->data);
}
SCMutexUnlock(&h->m);

(void) SC_ATOMIC_ADD(ctx->prune_idx, (ctx->config.hash_size - cnt));
if (data_size > 0)
(void)SC_ATOMIC_ADD(ctx->memuse, data_size);
return h;
}

Expand Down

0 comments on commit f1b44ca

Please sign in to comment.