From 4fd0745b782b768a400f86867b2f6cdaed8ce0a7 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 9 Jan 2024 14:44:32 +0100 Subject: [PATCH 01/26] detect: minor cleanup --- src/detect.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/detect.c b/src/detect.c index 6eb8698cf4a8..4cc428eb999e 100644 --- a/src/detect.c +++ b/src/detect.c @@ -729,7 +729,6 @@ static inline void DetectRulePacketRules( const DetectRunScratchpad *scratch ) { - const Signature *s = NULL; const Signature *next_s = NULL; /* inspect the sigs against the packet */ @@ -760,7 +759,7 @@ static inline void DetectRulePacketRules( #ifdef PROFILE_RULES bool smatch = false; /* signature match */ #endif - s = next_s; + const Signature *s = next_s; sflags = next_sflags; if (match_cnt) { next_s = *match_array++; From dcfa59bb6963b4f1b554cbc82b503834a499033c Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Sat, 13 Jan 2024 19:41:40 +0100 Subject: [PATCH 02/26] mpm/ac: tidy up main search loop --- src/util-mpm-ac.c | 56 ++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/src/util-mpm-ac.c b/src/util-mpm-ac.c index 05d777397c45..e7eb63cf4fc5 100644 --- a/src/util-mpm-ac.c +++ b/src/util-mpm-ac.c @@ -939,7 +939,6 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, PrefilterRuleStore *pmq, const uint8_t *buf, uint32_t buflen) { const SCACCtx *ctx = (SCACCtx *)mpm_ctx->ctx; - uint32_t i = 0; int matches = 0; /* \todo tried loop unrolling with register var, with no perf increase. Need @@ -952,30 +951,26 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (ctx->state_count < 32767) { register SC_AC_STATE_TYPE_U16 state = 0; - SC_AC_STATE_TYPE_U16 (*state_table_u16)[256] = ctx->state_table_u16; - for (i = 0; i < buflen; i++) { + const SC_AC_STATE_TYPE_U16(*state_table_u16)[256] = ctx->state_table_u16; + for (uint32_t i = 0; i < buflen; i++) { state = state_table_u16[state & 0x7FFF][u8_tolower(buf[i])]; if (state & 0x8000) { - uint32_t no_of_entries = ctx->output_table[state & 0x7FFF].no_of_entries; - uint32_t *pids = ctx->output_table[state & 0x7FFF].pids; - uint32_t k; - for (k = 0; k < no_of_entries; k++) { + const uint32_t no_of_entries = ctx->output_table[state & 0x7FFF].no_of_entries; + const uint32_t *pids = ctx->output_table[state & 0x7FFF].pids; + for (uint32_t k = 0; k < no_of_entries; k++) { if (pids[k] & AC_CASE_MASK) { - uint32_t lower_pid = pids[k] & AC_PID_MASK; + const uint32_t lower_pid = pids[k] & AC_PID_MASK; const SCACPatternList *pat = &pid_pat_list[lower_pid]; const int offset = i - pat->patlen + 1; if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; - if (SCMemcmp(pat->cs, buf + offset, pat->patlen) != 0) - { + if (SCMemcmp(pat->cs, buf + offset, pat->patlen) != 0) { /* inside loop */ continue; } - if (bitarray[(lower_pid) / 8] & (1 << ((lower_pid) % 8))) { - ; - } else { + if (!(bitarray[(lower_pid) / 8] & (1 << ((lower_pid) % 8)))) { bitarray[(lower_pid) / 8] |= (1 << ((lower_pid) % 8)); PrefilterAddSids(pmq, pat->sids, pat->sids_size); matches++; @@ -987,32 +982,26 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; - if (bitarray[pids[k] / 8] & (1 << (pids[k] % 8))) { - ; - } else { + if (!(bitarray[pids[k] / 8] & (1 << (pids[k] % 8)))) { bitarray[pids[k] / 8] |= (1 << (pids[k] % 8)); PrefilterAddSids(pmq, pat->sids, pat->sids_size); matches++; } } - //loop1: - //; } } - } /* for (i = 0; i < buflen; i++) */ - + } } else { register SC_AC_STATE_TYPE_U32 state = 0; - SC_AC_STATE_TYPE_U32 (*state_table_u32)[256] = ctx->state_table_u32; - for (i = 0; i < buflen; i++) { + const SC_AC_STATE_TYPE_U32(*state_table_u32)[256] = ctx->state_table_u32; + for (uint32_t i = 0; i < buflen; i++) { state = state_table_u32[state & 0x00FFFFFF][u8_tolower(buf[i])]; if (state & 0xFF000000) { - uint32_t no_of_entries = ctx->output_table[state & 0x00FFFFFF].no_of_entries; - uint32_t *pids = ctx->output_table[state & 0x00FFFFFF].pids; - uint32_t k; - for (k = 0; k < no_of_entries; k++) { + const uint32_t no_of_entries = ctx->output_table[state & 0x00FFFFFF].no_of_entries; + const uint32_t *pids = ctx->output_table[state & 0x00FFFFFF].pids; + for (uint32_t k = 0; k < no_of_entries; k++) { if (pids[k] & AC_CASE_MASK) { - uint32_t lower_pid = pids[k] & 0x0000FFFF; + const uint32_t lower_pid = pids[k] & 0x0000FFFF; const SCACPatternList *pat = &pid_pat_list[lower_pid]; const int offset = i - pat->patlen + 1; @@ -1024,9 +1013,7 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, /* inside loop */ continue; } - if (bitarray[(lower_pid) / 8] & (1 << ((lower_pid) % 8))) { - ; - } else { + if (!(bitarray[(lower_pid) / 8] & (1 << ((lower_pid) % 8)))) { bitarray[(lower_pid) / 8] |= (1 << ((lower_pid) % 8)); PrefilterAddSids(pmq, pat->sids, pat->sids_size); matches++; @@ -1038,21 +1025,16 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; - if (bitarray[pids[k] / 8] & (1 << (pids[k] % 8))) { - ; - } else { + if (!(bitarray[pids[k] / 8] & (1 << (pids[k] % 8)))) { bitarray[pids[k] / 8] |= (1 << (pids[k] % 8)); PrefilterAddSids(pmq, pat->sids, pat->sids_size); matches++; } } - //loop1: - //; } } - } /* for (i = 0; i < buflen; i++) */ + } } - return matches; } From b499239ef571891b4a0ec7e5f98945d5e78296ef Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 19 Dec 2023 11:42:42 +0100 Subject: [PATCH 03/26] mpm: register algo features This is so patterns can reply on mpm match meaning a full match. Not yet used. --- src/util-mpm-ac-ks.c | 1 + src/util-mpm-ac.c | 1 + src/util-mpm-hs.c | 1 + src/util-mpm.h | 4 ++++ 4 files changed, 7 insertions(+) diff --git a/src/util-mpm-ac-ks.c b/src/util-mpm-ac-ks.c index df36452be3f3..60bb1b7a9e42 100644 --- a/src/util-mpm-ac-ks.c +++ b/src/util-mpm-ac-ks.c @@ -1408,6 +1408,7 @@ void MpmACTileRegister(void) #ifdef UNITTESTS mpm_table[MPM_AC_KS].RegisterUnittests = SCACTileRegisterTests; #endif + mpm_table[MPM_AC_KS].feature_flags = MPM_FEATURE_FLAG_DEPTH | MPM_FEATURE_FLAG_OFFSET; } diff --git a/src/util-mpm-ac.c b/src/util-mpm-ac.c index e7eb63cf4fc5..e85b6e503215 100644 --- a/src/util-mpm-ac.c +++ b/src/util-mpm-ac.c @@ -1127,6 +1127,7 @@ void MpmACRegister(void) #ifdef UNITTESTS mpm_table[MPM_AC].RegisterUnittests = SCACRegisterTests; #endif + mpm_table[MPM_AC].feature_flags = MPM_FEATURE_FLAG_DEPTH | MPM_FEATURE_FLAG_OFFSET; return; } diff --git a/src/util-mpm-hs.c b/src/util-mpm-hs.c index 5c241991842e..b22b6f869db6 100644 --- a/src/util-mpm-hs.c +++ b/src/util-mpm-hs.c @@ -1055,6 +1055,7 @@ void MpmHSRegister(void) #ifdef UNITTESTS mpm_table[MPM_HS].RegisterUnittests = SCHSRegisterTests; #endif + mpm_table[MPM_HS].feature_flags = MPM_FEATURE_FLAG_DEPTH | MPM_FEATURE_FLAG_OFFSET; /* Set Hyperscan memory allocators */ SCHSSetAllocators(); } diff --git a/src/util-mpm.h b/src/util-mpm.h index c3b12d8a1afd..2df692040aba 100644 --- a/src/util-mpm.h +++ b/src/util-mpm.h @@ -138,6 +138,9 @@ typedef struct MpmCtxFactoryContainer_ { * what is passed through the API */ #define MPM_PATTERN_CTX_OWNS_ID 0x20 +#define MPM_FEATURE_FLAG_DEPTH BIT_U8(0) +#define MPM_FEATURE_FLAG_OFFSET BIT_U8(1) + typedef struct MpmTableElmt_ { const char *name; void (*InitCtx)(struct MpmCtx_ *); @@ -166,6 +169,7 @@ typedef struct MpmTableElmt_ { #ifdef UNITTESTS void (*RegisterUnittests)(void); #endif + uint8_t feature_flags; } MpmTableElmt; extern MpmTableElmt mpm_table[MPM_TABLE_SIZE]; From c312d670d4c33aeeba87da127656b045a5344c73 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 15 Jan 2024 20:42:28 +0100 Subject: [PATCH 04/26] mpm/ac: implement endswith When a pattern is using endswith, only consider it a match when it is the end of the data. Ticket: #6852. --- src/detect-engine-mpm.c | 10 ++++++++-- src/util-mpm-ac.c | 10 ++++++++++ src/util-mpm-ac.h | 2 ++ src/util-mpm.h | 6 ++++-- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/detect-engine-mpm.c b/src/detect-engine-mpm.c index d8630dd1c467..e7122e90caa9 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -1559,6 +1559,9 @@ static void MpmStoreSetup(const DetectEngineCtx *de_ctx, MpmStore *ms) MpmInitCtx(ms->mpm_ctx, de_ctx->mpm_matcher); + const bool mpm_supports_endswith = + (mpm_table[de_ctx->mpm_matcher].feature_flags & MPM_FEATURE_FLAG_ENDSWITH) != 0; + /* add the patterns */ for (sig = 0; sig < (ms->sid_array_size * 8); sig++) { if (ms->sid_array[sig / 8] & (1 << (sig % 8))) { @@ -1585,8 +1588,11 @@ static void MpmStoreSetup(const DetectEngineCtx *de_ctx, MpmStore *ms) } if (!skip) { - PopulateMpmHelperAddPattern(ms->mpm_ctx, - cd, s, 0, (cd->flags & DETECT_CONTENT_FAST_PATTERN_CHOP)); + uint8_t flags = 0; + if ((cd->flags & DETECT_CONTENT_ENDS_WITH) && mpm_supports_endswith) + flags = MPM_PATTERN_FLAG_ENDSWITH; + PopulateMpmHelperAddPattern( + ms->mpm_ctx, cd, s, flags, (cd->flags & DETECT_CONTENT_FAST_PATTERN_CHOP)); } } } diff --git a/src/util-mpm-ac.c b/src/util-mpm-ac.c index e85b6e503215..891113ee837f 100644 --- a/src/util-mpm-ac.c +++ b/src/util-mpm-ac.c @@ -781,6 +781,8 @@ int SCACPreparePatterns(MpmCtx *mpm_ctx) } ctx->pid_pat_list[ctx->parray[i]->id].offset = ctx->parray[i]->offset; ctx->pid_pat_list[ctx->parray[i]->id].depth = ctx->parray[i]->depth; + ctx->pid_pat_list[ctx->parray[i]->id].endswith = + (ctx->parray[i]->flags & MPM_PATTERN_FLAG_ENDSWITH) != 0; /* ACPatternList now owns this memory */ //SCLogInfo("ctx->parray[i]->sids_size %u", ctx->parray[i]->sids_size); @@ -965,6 +967,8 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; + if (pat->endswith && (uint32_t)offset + pat->patlen != buflen) + continue; if (SCMemcmp(pat->cs, buf + offset, pat->patlen) != 0) { /* inside loop */ @@ -981,6 +985,8 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; + if (pat->endswith && (uint32_t)offset + pat->patlen != buflen) + continue; if (!(bitarray[pids[k] / 8] & (1 << (pids[k] % 8)))) { bitarray[pids[k] / 8] |= (1 << (pids[k] % 8)); @@ -1007,6 +1013,8 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; + if (pat->endswith && (uint32_t)offset + pat->patlen != buflen) + continue; if (SCMemcmp(pat->cs, buf + offset, pat->patlen) != 0) { @@ -1024,6 +1032,8 @@ uint32_t SCACSearch(const MpmCtx *mpm_ctx, MpmThreadCtx *mpm_thread_ctx, if (offset < (int)pat->offset || (pat->depth && i > pat->depth)) continue; + if (pat->endswith && (uint32_t)offset + pat->patlen != buflen) + continue; if (!(bitarray[pids[k] / 8] & (1 << (pids[k] % 8)))) { bitarray[pids[k] / 8] |= (1 << (pids[k] % 8)); diff --git a/src/util-mpm-ac.h b/src/util-mpm-ac.h index f6cbe512b779..96b1c8e990ee 100644 --- a/src/util-mpm-ac.h +++ b/src/util-mpm-ac.h @@ -37,6 +37,8 @@ typedef struct SCACPatternList_ { uint16_t offset; uint16_t depth; + bool endswith; + /* sid(s) for this pattern */ uint32_t sids_size; SigIntId *sids; diff --git a/src/util-mpm.h b/src/util-mpm.h index 2df692040aba..0bcd87899b6c 100644 --- a/src/util-mpm.h +++ b/src/util-mpm.h @@ -137,9 +137,11 @@ typedef struct MpmCtxFactoryContainer_ { /** the ctx uses it's own internal id instead of * what is passed through the API */ #define MPM_PATTERN_CTX_OWNS_ID 0x20 +#define MPM_PATTERN_FLAG_ENDSWITH 0x40 -#define MPM_FEATURE_FLAG_DEPTH BIT_U8(0) -#define MPM_FEATURE_FLAG_OFFSET BIT_U8(1) +#define MPM_FEATURE_FLAG_DEPTH BIT_U8(0) +#define MPM_FEATURE_FLAG_OFFSET BIT_U8(1) +#define MPM_FEATURE_FLAG_ENDSWITH BIT_U8(2) typedef struct MpmTableElmt_ { const char *name; From d47bfbb14dda9b737273e30cdd2a0dfecebc0bd8 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 16 Jan 2024 16:40:25 +0100 Subject: [PATCH 05/26] mpm/ac: add endswith test --- src/util-mpm-ac.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/util-mpm-ac.c b/src/util-mpm-ac.c index 891113ee837f..270250c6fde3 100644 --- a/src/util-mpm-ac.c +++ b/src/util-mpm-ac.c @@ -2118,6 +2118,35 @@ static int SCACTest29(void) return result; } +/** \test endswith logic */ +static int SCACTest30(void) +{ + MpmCtx mpm_ctx; + MpmThreadCtx mpm_thread_ctx; + PrefilterRuleStore pmq; + + memset(&mpm_ctx, 0, sizeof(MpmCtx)); + memset(&mpm_thread_ctx, 0, sizeof(MpmThreadCtx)); + MpmInitCtx(&mpm_ctx, MPM_AC); + + /* 0 match */ + MpmAddPatternCS(&mpm_ctx, (uint8_t *)"xyz", 3, 0, 0, 0, 0, MPM_PATTERN_FLAG_ENDSWITH); + PmqSetup(&pmq); + + SCACPreparePatterns(&mpm_ctx); + + const char *buf1 = "abcdefghijklmnopqrstuvwxyz"; + uint32_t cnt = SCACSearch(&mpm_ctx, &mpm_thread_ctx, &pmq, (uint8_t *)buf1, strlen(buf1)); + FAIL_IF_NOT(cnt == 1); + const char *buf2 = "xyzxyzxyzxyzxyzxyzxyza"; + cnt = SCACSearch(&mpm_ctx, &mpm_thread_ctx, &pmq, (uint8_t *)buf2, strlen(buf2)); + FAIL_IF_NOT(cnt == 0); + + SCACDestroyCtx(&mpm_ctx); + PmqFree(&pmq); + PASS; +} + void SCACRegisterTests(void) { UtRegisterTest("SCACTest01", SCACTest01); @@ -2149,5 +2178,6 @@ void SCACRegisterTests(void) UtRegisterTest("SCACTest27", SCACTest27); UtRegisterTest("SCACTest28", SCACTest28); UtRegisterTest("SCACTest29", SCACTest29); + UtRegisterTest("SCACTest30", SCACTest30); } #endif /* UNITTESTS */ From 1122fa24b6ee8170043f80db2395b6116239874c Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 16 Jan 2024 16:33:53 +0100 Subject: [PATCH 06/26] mpm/ac: minor test cleanups --- src/util-mpm-ac.c | 57 ++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/src/util-mpm-ac.c b/src/util-mpm-ac.c index 270250c6fde3..27d5ff064338 100644 --- a/src/util-mpm-ac.c +++ b/src/util-mpm-ac.c @@ -2035,7 +2035,6 @@ static int SCACTest27(void) static int SCACTest28(void) { - int result = 0; MpmCtx mpm_ctx; MpmThreadCtx mpm_thread_ctx; PrefilterRuleStore pmq; @@ -2053,69 +2052,51 @@ static int SCACTest28(void) const char *buf = "tONE"; uint32_t cnt = SCACSearch(&mpm_ctx, &mpm_thread_ctx, &pmq, (uint8_t *)buf, strlen(buf)); - - if (cnt == 0) - result = 1; - else - printf("0 != %" PRIu32 " ",cnt); + FAIL_IF_NOT(cnt == 0); SCACDestroyCtx(&mpm_ctx); PmqFree(&pmq); - return result; + PASS; } static int SCACTest29(void) { uint8_t buf[] = "onetwothreefourfivesixseveneightnine"; uint16_t buflen = sizeof(buf) - 1; - Packet *p = NULL; ThreadVars th_v; DetectEngineThreadCtx *det_ctx = NULL; - int result = 0; memset(&th_v, 0, sizeof(th_v)); - p = UTHBuildPacket(buf, buflen, IPPROTO_TCP); + Packet *p = UTHBuildPacket(buf, buflen, IPPROTO_TCP); + FAIL_IF_NULL(p); DetectEngineCtx *de_ctx = DetectEngineCtxInit(); - if (de_ctx == NULL) - goto end; - + FAIL_IF_NULL(de_ctx); de_ctx->flags |= DE_QUIET; - de_ctx->sig_list = SigInit(de_ctx, "alert tcp any any -> any any " - "(content:\"onetwothreefourfivesixseveneightnine\"; sid:1;)"); - if (de_ctx->sig_list == NULL) - goto end; - de_ctx->sig_list->next = SigInit(de_ctx, "alert tcp any any -> any any " - "(content:\"onetwothreefourfivesixseveneightnine\"; fast_pattern:3,3; sid:2;)"); - if (de_ctx->sig_list->next == NULL) - goto end; + Signature *s = DetectEngineAppendSig(de_ctx, + "alert tcp any any -> any any " + "(content:\"onetwothreefourfivesixseveneightnine\"; sid:1;)"); + FAIL_IF_NULL(s); + s = DetectEngineAppendSig(de_ctx, + "alert tcp any any -> any any " + "(content:\"onetwothreefourfivesixseveneightnine\"; fast_pattern:3,3; sid:2;)"); + FAIL_IF_NULL(s); SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); SigMatchSignatures(&th_v, de_ctx, det_ctx, p); - if (PacketAlertCheck(p, 1) != 1) { - printf("if (PacketAlertCheck(p, 1) != 1) failure\n"); - goto end; - } - if (PacketAlertCheck(p, 2) != 1) { - printf("if (PacketAlertCheck(p, 1) != 2) failure\n"); - goto end; - } - result = 1; -end: - if (de_ctx != NULL) { - SigGroupCleanup(de_ctx); - SigCleanSignatures(de_ctx); + FAIL_IF(PacketAlertCheck(p, 1) != 1); + FAIL_IF(PacketAlertCheck(p, 2) != 1); - DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); - DetectEngineCtxFree(de_ctx); - } + DetectEngineThreadCtxDeinit(&th_v, (void *)det_ctx); + DetectEngineCtxFree(de_ctx); + StatsThreadCleanup(&th_v); UTHFreePackets(&p, 1); - return result; + PASS; } /** \test endswith logic */ From 85d321a689753ff7c1647159089d9c2134bbbe1c Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 16 Oct 2023 10:43:27 -0400 Subject: [PATCH 07/26] output/plugin: Use Suri thread-id for plugins Issue: 6408 Use the Suricata thread id for plugin thread initialization to give the plugin a better correlating factor to the actual Suricata threads. --- src/output-eve-null.c | 2 +- src/output-json-common.c | 4 ++-- src/output-json-stats.c | 6 +++--- src/suricata-plugin.h | 6 ++++-- src/util-logopenfile.c | 35 +++++++++++++++++++++-------------- src/util-logopenfile.h | 11 +++++++---- 6 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/output-eve-null.c b/src/output-eve-null.c index 1b62b96b36cb..298c3527a74a 100644 --- a/src/output-eve-null.c +++ b/src/output-eve-null.c @@ -47,7 +47,7 @@ static int NullLogWrite(const char *buffer, int buffer_len, void *init_data, voi return 0; } -static int NullLogThreadInit(void *init_data, int thread_id, void **thread_data) +static int NullLogThreadInit(void *init_data, ThreadId thread_id, void **thread_data) { *thread_data = NULL; return 0; diff --git a/src/output-json-common.c b/src/output-json-common.c index 42595c3dde25..5723e05a386a 100644 --- a/src/output-json-common.c +++ b/src/output-json-common.c @@ -38,7 +38,7 @@ OutputJsonThreadCtx *CreateEveThreadCtx(ThreadVars *t, OutputJsonCtx *ctx) goto error; } - thread->file_ctx = LogFileEnsureExists(ctx->file_ctx); + thread->file_ctx = LogFileEnsureExists(t->id, ctx->file_ctx); if (!thread->file_ctx) { goto error; } @@ -104,7 +104,7 @@ TmEcode JsonLogThreadInit(ThreadVars *t, const void *initdata, void **data) } thread->ctx = ((OutputCtx *)initdata)->data; - thread->file_ctx = LogFileEnsureExists(thread->ctx->file_ctx); + thread->file_ctx = LogFileEnsureExists(t->id, thread->ctx->file_ctx); if (!thread->file_ctx) { goto error_exit; } diff --git a/src/output-json-stats.c b/src/output-json-stats.c index 7cc880727dce..ecc7dce4b092 100644 --- a/src/output-json-stats.c +++ b/src/output-json-stats.c @@ -375,7 +375,7 @@ static TmEcode JsonStatsLogThreadInit(ThreadVars *t, const void *initdata, void /* Use the Output Context (file pointer and mutex) */ aft->statslog_ctx = ((OutputCtx *)initdata)->data; - aft->file_ctx = LogFileEnsureExists(aft->statslog_ctx->file_ctx); + aft->file_ctx = LogFileEnsureExists(t->id, aft->statslog_ctx->file_ctx); if (!aft->file_ctx) { goto error_exit; } @@ -471,8 +471,8 @@ static OutputInitResult OutputStatsLogInitSub(ConfNode *conf, OutputCtx *parent_ } SCLogDebug("Preparing file context for stats submodule logger"); - /* Share output slot with thread 1 */ - stats_ctx->file_ctx = LogFileEnsureExists(ajt->file_ctx); + /* prepared by suricata-main */ + stats_ctx->file_ctx = LogFileEnsureExists(0, ajt->file_ctx); if (!stats_ctx->file_ctx) { SCFree(stats_ctx); SCFree(output_ctx); diff --git a/src/suricata-plugin.h b/src/suricata-plugin.h index cb2c667eaedb..4ba5697691c4 100644 --- a/src/suricata-plugin.h +++ b/src/suricata-plugin.h @@ -40,6 +40,7 @@ typedef struct SCPlugin_ { } SCPlugin; typedef SCPlugin *(*SCPluginRegisterFunc)(void); +typedef uint32_t ThreadId; /** * Structure used to define an Eve output file type plugin. @@ -54,8 +55,9 @@ typedef struct SCEveFileType_ { int (*Write)(const char *buffer, int buffer_len, void *init_data, void *thread_data); /* Close - Called on final close */ void (*Deinit)(void *init_data); - /* ThreadInit - Called for each thread using file object*/ - int (*ThreadInit)(void *init_data, int thread_id, void **thread_data); + /* ThreadInit - Called for each thread using file object; non-zero thread_ids correlate + * to Suricata's worker threads; 0 correlates to the Suricata main thread */ + int (*ThreadInit)(void *init_data, ThreadId thread_id, void **thread_data); /* ThreadDeinit - Called for each thread using file object */ int (*ThreadDeinit)(void *init_data, void *thread_data); TAILQ_ENTRY(SCEveFileType_) entries; diff --git a/src/util-logopenfile.c b/src/util-logopenfile.c index 1b1986490658..24dfcc4ff784 100644 --- a/src/util-logopenfile.c +++ b/src/util-logopenfile.c @@ -50,7 +50,7 @@ static bool LogFileNewThreadedCtx(LogFileCtx *parent_ctx, const char *log_path, ThreadLogFileHashEntry *entry); // Threaded eve.json identifier -static SC_ATOMIC_DECL_AND_INIT_WITH_VAL(uint32_t, eve_file_id, 1); +static SC_ATOMIC_DECL_AND_INIT_WITH_VAL(uint16_t, eve_file_id, 1); #ifdef BUILD_WITH_UNIXSOCKET /** \brief connect to the indicated local stream socket, logging any errors @@ -677,7 +677,7 @@ LogFileCtx *LogFileNewCtx(void) * Each thread -- identified by its operating system thread-id -- has its * own file entry that includes a file pointer. */ -static ThreadLogFileHashEntry *LogFileThread2Slot(LogThreadedFileCtx *parent) +static ThreadLogFileHashEntry *LogFileThread2Slot(LogThreadedFileCtx *parent, ThreadId thread_id) { ThreadLogFileHashEntry thread_hash_entry; @@ -689,12 +689,14 @@ static ThreadLogFileHashEntry *LogFileThread2Slot(LogThreadedFileCtx *parent) if (!ent) { ent = SCCalloc(1, sizeof(*ent)); if (!ent) { - FatalError("Unable to allocate thread/entry entry"); + FatalError("Unable to allocate thread/hash-entry entry"); } ent->thread_id = thread_hash_entry.thread_id; - SCLogDebug("Trying to add thread %ld to entry %d", ent->thread_id, ent->slot_number); + ent->internal_thread_id = thread_id; + SCLogDebug( + "Trying to add thread %" PRIi64 " to entry %d", ent->thread_id, ent->slot_number); if (0 != HashTableAdd(parent->ht, ent, 0)) { - FatalError("Unable to add thread/entry mapping"); + FatalError("Unable to add thread/hash-entry mapping"); } } return ent; @@ -704,7 +706,7 @@ static ThreadLogFileHashEntry *LogFileThread2Slot(LogThreadedFileCtx *parent) * \param parent_ctx * \retval LogFileCtx * pointer if successful; NULL otherwise */ -LogFileCtx *LogFileEnsureExists(LogFileCtx *parent_ctx) +LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *parent_ctx) { /* threaded output disabled */ if (!parent_ctx->threaded) @@ -712,15 +714,16 @@ LogFileCtx *LogFileEnsureExists(LogFileCtx *parent_ctx) SCMutexLock(&parent_ctx->threads->mutex); /* Find this thread's entry */ - ThreadLogFileHashEntry *entry = LogFileThread2Slot(parent_ctx->threads); - SCLogDebug("Adding reference for thread %ld [slot %d] to file %s [ctx %p]", SCGetThreadIdLong(), - entry->slot_number, parent_ctx->filename, parent_ctx); + ThreadLogFileHashEntry *entry = LogFileThread2Slot(parent_ctx->threads, thread_id); + SCLogDebug("%s: Adding reference for thread %" PRIi64 + " (local thread id %d) to file %s [ctx %p]", + t_thread_name, SCGetThreadIdLong(), thread_id, parent_ctx->filename, parent_ctx); bool new = entry->isopen; /* has it been opened yet? */ if (!entry->isopen) { - SCLogDebug("Opening new file for thread/slot %d to file %s [ctx %p]", entry->slot_number, - parent_ctx->filename, parent_ctx); + SCLogDebug("%s: Opening new file for thread/id %d to file %s [ctx %p]", t_thread_name, + thread_id, parent_ctx->filename, parent_ctx); if (LogFileNewThreadedCtx( parent_ctx, parent_ctx->filename, parent_ctx->threads->append, entry)) { entry->isopen = true; @@ -810,11 +813,13 @@ static bool LogFileNewThreadedCtx(LogFileCtx *parent_ctx, const char *log_path, *thread = *parent_ctx; if (parent_ctx->type == LOGFILE_TYPE_FILE) { char fname[LOGFILE_NAME_MAX]; - if (!LogFileThreadedName(log_path, fname, sizeof(fname), SC_ATOMIC_ADD(eve_file_id, 1))) { + entry->slot_number = SC_ATOMIC_ADD(eve_file_id, 1); + if (!LogFileThreadedName(log_path, fname, sizeof(fname), entry->slot_number)) { SCLogError("Unable to create threaded filename for log"); goto error; } - SCLogDebug("Thread open -- using name %s [replaces %s]", fname, log_path); + SCLogDebug("%s: thread open -- using name %s [replaces %s] - thread %d [slot %d]", + t_thread_name, fname, log_path, entry->internal_thread_id, entry->slot_number); thread->fp = SCLogOpenFileFp(fname, append, thread->filemode); if (thread->fp == NULL) { goto error; @@ -830,8 +835,10 @@ static bool LogFileNewThreadedCtx(LogFileCtx *parent_ctx, const char *log_path, OutputRegisterFileRotationFlag(&thread->rotation_flag); } else if (parent_ctx->type == LOGFILE_TYPE_PLUGIN) { entry->slot_number = SC_ATOMIC_ADD(eve_file_id, 1); + SCLogDebug("%s - thread %d [slot %d]", log_path, entry->internal_thread_id, + entry->slot_number); thread->plugin.plugin->ThreadInit( - thread->plugin.init_data, entry->slot_number, &thread->plugin.thread_data); + thread->plugin.init_data, entry->internal_thread_id, &thread->plugin.thread_data); } thread->threaded = false; thread->parent = parent_ctx; diff --git a/src/util-logopenfile.h b/src/util-logopenfile.h index e225be489032..7b78c9cc459e 100644 --- a/src/util-logopenfile.h +++ b/src/util-logopenfile.h @@ -49,10 +49,13 @@ typedef struct SyslogSetup_ { } SyslogSetup; typedef struct ThreadLogFileHashEntry { - uint64_t thread_id; - int slot_number; /* slot identifier -- for plugins */ - bool isopen; struct LogFileCtx_ *ctx; + + uint64_t thread_id; /* OS thread identifier */ + ThreadId internal_thread_id; /* Suri internal thread id; to assist output plugins correlating + usage */ + uint16_t slot_number; /* Slot identifier - used when forming per-thread output names*/ + bool isopen; } ThreadLogFileHashEntry; struct LogFileCtx_; @@ -168,7 +171,7 @@ LogFileCtx *LogFileNewCtx(void); int LogFileFreeCtx(LogFileCtx *); int LogFileWrite(LogFileCtx *file_ctx, MemBuffer *buffer); -LogFileCtx *LogFileEnsureExists(LogFileCtx *lf_ctx); +LogFileCtx *LogFileEnsureExists(ThreadId thread_id, LogFileCtx *lf_ctx); int SCConfLogOpenGeneric(ConfNode *conf, LogFileCtx *, const char *, int); int SCConfLogReopen(LogFileCtx *); bool SCLogOpenThreadedFile(const char *log_path, const char *append, LogFileCtx *parent_ctx); From 3bf92bb14f5cdfef594112d716b5cdb937dcbf92 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Sun, 7 Jan 2024 09:34:45 -0500 Subject: [PATCH 08/26] example/plugin: Use ThreadId --- examples/plugins/c-json-filetype/filetype.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/plugins/c-json-filetype/filetype.c b/examples/plugins/c-json-filetype/filetype.c index 9c81d7f03267..cf8ca64d28b9 100644 --- a/examples/plugins/c-json-filetype/filetype.c +++ b/examples/plugins/c-json-filetype/filetype.c @@ -22,7 +22,7 @@ #define FILETYPE_NAME "json-filetype-plugin" -static int FiletypeThreadInit(void *ctx, int thread_id, void **thread_data); +static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data); static int FiletypeThreadDeinit(void *ctx, void *thread_data); /** @@ -30,7 +30,7 @@ static int FiletypeThreadDeinit(void *ctx, void *thread_data); */ typedef struct ThreadData_ { /** The thread ID, for demonstration purposes only. */ - int thread_id; + ThreadId thread_id; /** The number of records logged on this thread. */ uint64_t count; @@ -143,7 +143,7 @@ static void FiletypeDeinit(void *data) * Suricata, but instead this plugin chooses to use this method to create a * default (single) thread context. */ -static int FiletypeThreadInit(void *ctx, int thread_id, void **thread_data) +static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) { ThreadData *tdata = SCCalloc(1, sizeof(ThreadData)); if (tdata == NULL) { From ead09c24977e6f2402bc2f5f03592dc3a42b0764 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 7 Mar 2024 15:33:28 -0600 Subject: [PATCH 09/26] eve/filetypes: remove from plugin context Remove EVE filetypes from plugin context as they are not only used from plugins. Plugins allow user code to register filetypes, but we also have internal file types that use this api including the null output and syslog. Additionally library users can use this API to register filetypes, and they are not plugins. Ideally this code would go in "output-json.[ch]" as the "primary" eve API, however there are currently some include circular include issues there, so start new cleaned up EVE API in "output-eve.[ch]" which is "clean" with respect to includes, and as we cleanup existing EVE API for "public" use, it can be moved here. Ticket: #6838 --- examples/plugins/c-json-filetype/filetype.c | 1 + src/Makefile.am | 2 + src/output-eve-null.c | 1 + src/output-eve-syslog.c | 1 + src/output-eve.c | 82 +++++++++++++++++++++ src/output-eve.h | 63 ++++++++++++++++ src/output-json.c | 7 +- src/suricata-plugin.h | 26 +------ src/util-logopenfile.h | 1 + src/util-plugin.c | 64 ---------------- src/util-plugin.h | 1 - 11 files changed, 154 insertions(+), 95 deletions(-) create mode 100644 src/output-eve.c create mode 100644 src/output-eve.h diff --git a/examples/plugins/c-json-filetype/filetype.c b/examples/plugins/c-json-filetype/filetype.c index cf8ca64d28b9..e85273d76885 100644 --- a/examples/plugins/c-json-filetype/filetype.c +++ b/examples/plugins/c-json-filetype/filetype.c @@ -17,6 +17,7 @@ #include "suricata-common.h" #include "suricata-plugin.h" +#include "output-eve.h" #include "util-mem.h" #include "util-debug.h" diff --git a/src/Makefile.am b/src/Makefile.am index 21cb34c39b3a..df37a5e9dc08 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -393,6 +393,7 @@ noinst_HEADERS = \ log-tcp-data.h \ log-tlslog.h \ log-tlsstore.h \ + output-eve.h \ output-eve-stream.h \ output-eve-null.h \ output-filedata.h \ @@ -1058,6 +1059,7 @@ libsuricata_c_a_SOURCES = \ output-json-template.c \ output-json-tftp.c \ output-json-tls.c \ + output-eve.c \ output-eve-syslog.c \ output-eve-null.c \ output-lua.c \ diff --git a/src/output-eve-null.c b/src/output-eve-null.c index 298c3527a74a..afe11afc068f 100644 --- a/src/output-eve-null.c +++ b/src/output-eve-null.c @@ -27,6 +27,7 @@ #include "output.h" /* DEFAULT_LOG_* */ #include "output-eve-null.h" +#include "output-eve.h" #ifdef OS_WIN32 void NullLogInitialize(void) diff --git a/src/output-eve-syslog.c b/src/output-eve-syslog.c index 5d71b5d807d1..0787e4b75071 100644 --- a/src/output-eve-syslog.c +++ b/src/output-eve-syslog.c @@ -26,6 +26,7 @@ #include "suricata-common.h" /* errno.h, string.h, etc. */ #include "output.h" /* DEFAULT_LOG_* */ +#include "output-eve.h" #include "output-eve-syslog.h" #include "util-syslog.h" diff --git a/src/output-eve.c b/src/output-eve.c new file mode 100644 index 000000000000..d0d775cba7f7 --- /dev/null +++ b/src/output-eve.c @@ -0,0 +1,82 @@ +/* Copyright (C) 2024 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#include "output-eve.h" +#include "util-debug.h" + +static TAILQ_HEAD(, SCEveFileType_) output_types = TAILQ_HEAD_INITIALIZER(output_types); + +static bool IsBuiltinTypeName(const char *name) +{ + const char *builtin[] = { + "regular", + "unix_dgram", + "unix_stream", + "redis", + NULL, + }; + for (int i = 0;; i++) { + if (builtin[i] == NULL) { + break; + } + if (strcmp(builtin[i], name) == 0) { + return true; + } + } + return false; +} + +SCEveFileType *SCEveFindFileType(const char *name) +{ + SCEveFileType *plugin = NULL; + TAILQ_FOREACH (plugin, &output_types, entries) { + if (strcmp(name, plugin->name) == 0) { + return plugin; + } + } + return NULL; +} + +/** + * \brief Register an Eve file type. + * + * \retval true if registered successfully, false if the file type name + * conflicts with a built-in or previously registered + * file type. + */ +bool SCRegisterEveFileType(SCEveFileType *plugin) +{ + /* First check that the name doesn't conflict with a built-in filetype. */ + if (IsBuiltinTypeName(plugin->name)) { + SCLogError("Eve file type name conflicts with built-in type: %s", plugin->name); + return false; + } + + /* Now check against previously registered file types. */ + SCEveFileType *existing = NULL; + TAILQ_FOREACH (existing, &output_types, entries) { + if (strcmp(existing->name, plugin->name) == 0) { + SCLogError("Eve file type name conflicts with previously registered type: %s", + plugin->name); + return false; + } + } + + SCLogDebug("Registering EVE file type plugin %s", plugin->name); + TAILQ_INSERT_TAIL(&output_types, plugin, entries); + return true; +} diff --git a/src/output-eve.h b/src/output-eve.h new file mode 100644 index 000000000000..e8c41996229e --- /dev/null +++ b/src/output-eve.h @@ -0,0 +1,63 @@ +/* Copyright (C) 2024 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \brief EVE logging subsystem + * + * This file will attempt to the main module for EVE logging + * sub-system. Currently most of the API resides in output-json.[ch], + * but due to some circular dependencies between EVE, and LogFileCtx, + * it made it hard to add EVE filetype modules there until some + * include issues are figured out. + */ + +#ifndef SURICATA_OUTPUT_EVE_H +#define SURICATA_OUTPUT_EVE_H + +#include "suricata-common.h" +#include "conf.h" + +typedef uint32_t ThreadId; + +/** + * Structure used to define an Eve output file type plugin. + */ +typedef struct SCEveFileType_ { + /* The name of the output, used to specify the output in the filetype section + * of the eve-log configuration. */ + const char *name; + /* Init Called on first access */ + int (*Init)(ConfNode *conf, bool threaded, void **init_data); + /* Write - Called on each write to the object */ + int (*Write)(const char *buffer, int buffer_len, void *init_data, void *thread_data); + /* Close - Called on final close */ + void (*Deinit)(void *init_data); + /* ThreadInit - Called for each thread using file object; non-zero thread_ids correlate + * to Suricata's worker threads; 0 correlates to the Suricata main thread */ + int (*ThreadInit)(void *init_data, ThreadId thread_id, void **thread_data); + /* ThreadDeinit - Called for each thread using file object */ + int (*ThreadDeinit)(void *init_data, void *thread_data); + TAILQ_ENTRY(SCEveFileType_) entries; +} SCEveFileType; + +bool SCRegisterEveFileType(SCEveFileType *); + +SCEveFileType *SCEveFindFileType(const char *name); + +#endif diff --git a/src/output-json.c b/src/output-json.c index 1dd2f948aba3..ab46abb14bdd 100644 --- a/src/output-json.c +++ b/src/output-json.c @@ -65,7 +65,6 @@ #include "util-log-redis.h" #include "util-device.h" #include "util-validate.h" -#include "util-plugin.h" #include "flow-var.h" #include "flow-bit.h" @@ -73,8 +72,6 @@ #include "source-pcap-file-helper.h" -#include "suricata-plugin.h" - #define DEFAULT_LOG_FILENAME "eve.json" #define MODULE_NAME "OutputJSON" @@ -1088,13 +1085,11 @@ OutputInitResult OutputJsonInitCtx(ConfNode *conf) enum LogFileType log_filetype = FileTypeFromConf(output_s); if (log_filetype == LOGFILE_TYPE_NOTSET) { -#ifdef HAVE_PLUGINS - SCEveFileType *plugin = SCPluginFindFileType(output_s); + SCEveFileType *plugin = SCEveFindFileType(output_s); if (plugin != NULL) { log_filetype = LOGFILE_TYPE_PLUGIN; json_ctx->plugin = plugin; } else -#endif FatalError("Invalid JSON output option: %s", output_s); } diff --git a/src/suricata-plugin.h b/src/suricata-plugin.h index 4ba5697691c4..85a91ef663f2 100644 --- a/src/suricata-plugin.h +++ b/src/suricata-plugin.h @@ -21,6 +21,8 @@ #include #include +#include "queue.h" + #include "conf.h" /** @@ -40,30 +42,6 @@ typedef struct SCPlugin_ { } SCPlugin; typedef SCPlugin *(*SCPluginRegisterFunc)(void); -typedef uint32_t ThreadId; - -/** - * Structure used to define an Eve output file type plugin. - */ -typedef struct SCEveFileType_ { - /* The name of the output, used to specify the output in the filetype section - * of the eve-log configuration. */ - const char *name; - /* Init Called on first access */ - int (*Init)(ConfNode *conf, bool threaded, void **init_data); - /* Write - Called on each write to the object */ - int (*Write)(const char *buffer, int buffer_len, void *init_data, void *thread_data); - /* Close - Called on final close */ - void (*Deinit)(void *init_data); - /* ThreadInit - Called for each thread using file object; non-zero thread_ids correlate - * to Suricata's worker threads; 0 correlates to the Suricata main thread */ - int (*ThreadInit)(void *init_data, ThreadId thread_id, void **thread_data); - /* ThreadDeinit - Called for each thread using file object */ - int (*ThreadDeinit)(void *init_data, void *thread_data); - TAILQ_ENTRY(SCEveFileType_) entries; -} SCEveFileType; - -bool SCRegisterEveFileType(SCEveFileType *); typedef struct SCCapturePlugin_ { char *name; diff --git a/src/util-logopenfile.h b/src/util-logopenfile.h index 7b78c9cc459e..f3ab81565a3b 100644 --- a/src/util-logopenfile.h +++ b/src/util-logopenfile.h @@ -34,6 +34,7 @@ #endif /* HAVE_LIBHIREDIS */ #include "suricata-plugin.h" +#include "output-eve.h" enum LogFileType { LOGFILE_TYPE_FILE, diff --git a/src/util-plugin.c b/src/util-plugin.c index 3a08aa8876ad..472c555b89e4 100644 --- a/src/util-plugin.c +++ b/src/util-plugin.c @@ -19,7 +19,6 @@ #include "suricata-plugin.h" #include "suricata.h" #include "runmodes.h" -#include "output-eve-syslog.h" #include "util-plugin.h" #include "util-debug.h" @@ -41,8 +40,6 @@ typedef struct PluginListNode_ { */ static TAILQ_HEAD(, PluginListNode_) plugins = TAILQ_HEAD_INITIALIZER(plugins); -static TAILQ_HEAD(, SCEveFileType_) output_types = TAILQ_HEAD_INITIALIZER(output_types); - static TAILQ_HEAD(, SCCapturePlugin_) capture_plugins = TAILQ_HEAD_INITIALIZER(capture_plugins); bool RegisterPlugin(SCPlugin *plugin, void *lib) @@ -133,67 +130,6 @@ void SCPluginsLoad(const char *capture_plugin_name, const char *capture_plugin_a } } -static bool IsBuiltinTypeName(const char *name) -{ - const char *builtin[] = { - "regular", - "unix_dgram", - "unix_stream", - "redis", - NULL, - }; - for (int i = 0;; i++) { - if (builtin[i] == NULL) { - break; - } - if (strcmp(builtin[i], name) == 0) { - return true; - } - } - return false; -} - -/** - * \brief Register an Eve file type. - * - * \retval true if registered successfully, false if the file type name - * conflicts with a built-in or previously registered - * file type. - */ -bool SCRegisterEveFileType(SCEveFileType *plugin) -{ - /* First check that the name doesn't conflict with a built-in filetype. */ - if (IsBuiltinTypeName(plugin->name)) { - SCLogError("Eve file type name conflicts with built-in type: %s", plugin->name); - return false; - } - - /* Now check against previously registered file types. */ - SCEveFileType *existing = NULL; - TAILQ_FOREACH (existing, &output_types, entries) { - if (strcmp(existing->name, plugin->name) == 0) { - SCLogError("Eve file type name conflicts with previously registered type: %s", - plugin->name); - return false; - } - } - - SCLogDebug("Registering EVE file type plugin %s", plugin->name); - TAILQ_INSERT_TAIL(&output_types, plugin, entries); - return true; -} - -SCEveFileType *SCPluginFindFileType(const char *name) -{ - SCEveFileType *plugin = NULL; - TAILQ_FOREACH(plugin, &output_types, entries) { - if (strcmp(name, plugin->name) == 0) { - return plugin; - } - } - return NULL; -} - int SCPluginRegisterCapture(SCCapturePlugin *plugin) { TAILQ_INSERT_TAIL(&capture_plugins, plugin, entries); diff --git a/src/util-plugin.h b/src/util-plugin.h index 7b5531c816a2..f749e287a366 100644 --- a/src/util-plugin.h +++ b/src/util-plugin.h @@ -21,7 +21,6 @@ #include "suricata-plugin.h" void SCPluginsLoad(const char *capture_plugin_name, const char *capture_plugin_args); -SCEveFileType *SCPluginFindFileType(const char *name); SCCapturePlugin *SCPluginFindCaptureByName(const char *name); bool RegisterPlugin(SCPlugin *, void *); From 7c8c9fff3250c5bb5f9dce90f157be4030aaf04e Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 7 Mar 2024 15:40:03 -0600 Subject: [PATCH 10/26] plugins: remove conf.h from suricata-plugin.h Remove "conf.h" from suricata-plugin.h as its not needed by that header. However, some other files became transitively dependent on through other includes, so fix those up. --- src/decode-erspan.c | 1 + src/defrag-config.c | 1 + src/source-pcap-file.c | 1 + src/suricata-plugin.h | 2 -- src/util-ebpf.h | 1 + src/util-exception-policy.c | 1 + src/util-macset.c | 1 + src/util-plugin.c | 1 + 8 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/decode-erspan.c b/src/decode-erspan.c index ccdf64aeaef9..76cb9bd5af8c 100644 --- a/src/decode-erspan.c +++ b/src/decode-erspan.c @@ -39,6 +39,7 @@ #include "util-validate.h" #include "util-unittest.h" #include "util-debug.h" +#include "conf.h" /** * \brief Functions to decode ERSPAN Type I and II packets diff --git a/src/defrag-config.c b/src/defrag-config.c index 7e2ae0cde96a..adb88470499a 100644 --- a/src/defrag-config.c +++ b/src/defrag-config.c @@ -26,6 +26,7 @@ #include "defrag-config.h" #include "util-misc.h" #include "util-radix-tree.h" +#include "conf.h" static SCRadixTree *defrag_tree = NULL; diff --git a/src/source-pcap-file.c b/src/source-pcap-file.c index c4f97bc0c4c1..647904a8bdec 100644 --- a/src/source-pcap-file.c +++ b/src/source-pcap-file.c @@ -31,6 +31,7 @@ #include "util-checksum.h" #include "runmode-unix-socket.h" #include "suricata.h" +#include "conf.h" extern uint16_t max_pending_packets; PcapFileGlobalVars pcap_g; diff --git a/src/suricata-plugin.h b/src/suricata-plugin.h index 85a91ef663f2..639dd7c7313e 100644 --- a/src/suricata-plugin.h +++ b/src/suricata-plugin.h @@ -23,8 +23,6 @@ #include "queue.h" -#include "conf.h" - /** * The size of the data chunk inside each packet structure a plugin * has for private data (Packet->plugin_v). diff --git a/src/util-ebpf.h b/src/util-ebpf.h index 55c1fd608a28..c4f524c3bdd1 100644 --- a/src/util-ebpf.h +++ b/src/util-ebpf.h @@ -25,6 +25,7 @@ #define SURICATA_UTIL_EBPF_H #include "flow-bypass.h" +#include "conf.h" #ifdef HAVE_PACKET_EBPF diff --git a/src/util-exception-policy.c b/src/util-exception-policy.c index 6c8ba0fba975..05f88f0c9a3b 100644 --- a/src/util-exception-policy.c +++ b/src/util-exception-policy.c @@ -26,6 +26,7 @@ #include "util-misc.h" #include "stream-tcp-reassemble.h" #include "action-globals.h" +#include "conf.h" enum ExceptionPolicy g_eps_master_switch = EXCEPTION_POLICY_NOT_SET; /** true if exception policy was defined in config */ diff --git a/src/util-macset.c b/src/util-macset.c index 9853a3241680..a4d1eb664c34 100644 --- a/src/util-macset.c +++ b/src/util-macset.c @@ -33,6 +33,7 @@ #include "util-macset.h" #include "util-unittest.h" #include "util-unittest-helper.h" +#include "conf.h" typedef uint8_t MacAddr[6]; typedef enum { diff --git a/src/util-plugin.c b/src/util-plugin.c index 472c555b89e4..6d34ff0e66b3 100644 --- a/src/util-plugin.c +++ b/src/util-plugin.c @@ -21,6 +21,7 @@ #include "runmodes.h" #include "util-plugin.h" #include "util-debug.h" +#include "conf.h" #ifdef HAVE_PLUGINS From 3ff72d3efa3eb717e5e6794a96d04266311e1fc2 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Thu, 7 Mar 2024 16:01:48 -0600 Subject: [PATCH 11/26] eve: rename plugin to filetypes EVE filetypes are not always plugins, for example, null and syslog that are built-in filetypes. --- src/output-json.c | 16 ++++++++-------- src/output-json.h | 4 +--- src/util-logopenfile.c | 28 +++++++++++++++------------- src/util-logopenfile.h | 12 ++++++------ 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/output-json.c b/src/output-json.c index ab46abb14bdd..830027cb02c4 100644 --- a/src/output-json.c +++ b/src/output-json.c @@ -1008,7 +1008,7 @@ static int LogFileTypePrepare( } } #endif - else if (log_filetype == LOGFILE_TYPE_PLUGIN) { + else if (log_filetype == LOGFILE_TYPE_FILETYPE) { if (json_ctx->file_ctx->threaded) { /* Prepare for threaded log output. */ if (!SCLogOpenThreadedFile(NULL, NULL, json_ctx->file_ctx)) { @@ -1016,11 +1016,11 @@ static int LogFileTypePrepare( } } void *init_data = NULL; - if (json_ctx->plugin->Init(conf, json_ctx->file_ctx->threaded, &init_data) < 0) { + if (json_ctx->filetype->Init(conf, json_ctx->file_ctx->threaded, &init_data) < 0) { return -1; } - json_ctx->file_ctx->plugin.plugin = json_ctx->plugin; - json_ctx->file_ctx->plugin.init_data = init_data; + json_ctx->file_ctx->filetype.filetype = json_ctx->filetype; + json_ctx->file_ctx->filetype.init_data = init_data; } return 0; @@ -1085,10 +1085,10 @@ OutputInitResult OutputJsonInitCtx(ConfNode *conf) enum LogFileType log_filetype = FileTypeFromConf(output_s); if (log_filetype == LOGFILE_TYPE_NOTSET) { - SCEveFileType *plugin = SCEveFindFileType(output_s); - if (plugin != NULL) { - log_filetype = LOGFILE_TYPE_PLUGIN; - json_ctx->plugin = plugin; + SCEveFileType *filetype = SCEveFindFileType(output_s); + if (filetype != NULL) { + log_filetype = LOGFILE_TYPE_FILETYPE; + json_ctx->filetype = filetype; } else FatalError("Invalid JSON output option: %s", output_s); } diff --git a/src/output-json.h b/src/output-json.h index 961e1204c58b..531740b2cdfa 100644 --- a/src/output-json.h +++ b/src/output-json.h @@ -28,10 +28,8 @@ #include "util-buffer.h" #include "util-logopenfile.h" #include "output.h" -#include "rust.h" #include "app-layer-htp-xff.h" -#include "suricata-plugin.h" void OutputJsonRegister(void); @@ -83,7 +81,7 @@ typedef struct OutputJsonCtx_ { enum LogFileType json_out; OutputJsonCommonSettings cfg; HttpXFFCfg *xff_cfg; - SCEveFileType *plugin; + SCEveFileType *filetype; } OutputJsonCtx; typedef struct OutputJsonThreadCtx_ { diff --git a/src/util-logopenfile.c b/src/util-logopenfile.c index 24dfcc4ff784..5675b145be7c 100644 --- a/src/util-logopenfile.c +++ b/src/util-logopenfile.c @@ -833,12 +833,12 @@ static bool LogFileNewThreadedCtx(LogFileCtx *parent_ctx, const char *log_path, thread->Write = SCLogFileWriteNoLock; thread->Close = SCLogFileCloseNoLock; OutputRegisterFileRotationFlag(&thread->rotation_flag); - } else if (parent_ctx->type == LOGFILE_TYPE_PLUGIN) { + } else if (parent_ctx->type == LOGFILE_TYPE_FILETYPE) { entry->slot_number = SC_ATOMIC_ADD(eve_file_id, 1); SCLogDebug("%s - thread %d [slot %d]", log_path, entry->internal_thread_id, entry->slot_number); - thread->plugin.plugin->ThreadInit( - thread->plugin.init_data, entry->internal_thread_id, &thread->plugin.thread_data); + thread->filetype.filetype->ThreadInit(thread->filetype.init_data, entry->internal_thread_id, + &thread->filetype.thread_data); } thread->threaded = false; thread->parent = parent_ctx; @@ -871,8 +871,9 @@ int LogFileFreeCtx(LogFileCtx *lf_ctx) SCReturnInt(0); } - if (lf_ctx->type == LOGFILE_TYPE_PLUGIN && lf_ctx->parent != NULL) { - lf_ctx->plugin.plugin->ThreadDeinit(lf_ctx->plugin.init_data, lf_ctx->plugin.thread_data); + if (lf_ctx->type == LOGFILE_TYPE_FILETYPE && lf_ctx->parent != NULL) { + lf_ctx->filetype.filetype->ThreadDeinit( + lf_ctx->filetype.init_data, lf_ctx->filetype.thread_data); } if (lf_ctx->threaded) { @@ -885,7 +886,7 @@ int LogFileFreeCtx(LogFileCtx *lf_ctx) } SCFree(lf_ctx->threads); } else { - if (lf_ctx->type != LOGFILE_TYPE_PLUGIN) { + if (lf_ctx->type != LOGFILE_TYPE_FILETYPE) { if (lf_ctx->fp != NULL) { lf_ctx->Close(lf_ctx); } @@ -908,11 +909,11 @@ int LogFileFreeCtx(LogFileCtx *lf_ctx) OutputUnregisterFileRotationFlag(&lf_ctx->rotation_flag); } - /* Deinitialize output plugins. We only want to call this for the - * parent of threaded output, or always for non-threaded + /* Deinitialize output filetypes. We only want to call this for + * the parent of threaded output, or always for non-threaded * output. */ - if (lf_ctx->type == LOGFILE_TYPE_PLUGIN && lf_ctx->parent == NULL) { - lf_ctx->plugin.plugin->Deinit(lf_ctx->plugin.init_data); + if (lf_ctx->type == LOGFILE_TYPE_FILETYPE && lf_ctx->parent == NULL) { + lf_ctx->filetype.filetype->Deinit(lf_ctx->filetype.init_data); } memset(lf_ctx, 0, sizeof(*lf_ctx)); @@ -929,9 +930,10 @@ int LogFileWrite(LogFileCtx *file_ctx, MemBuffer *buffer) MemBufferWriteString(buffer, "\n"); file_ctx->Write((const char *)MEMBUFFER_BUFFER(buffer), MEMBUFFER_OFFSET(buffer), file_ctx); - } else if (file_ctx->type == LOGFILE_TYPE_PLUGIN) { - file_ctx->plugin.plugin->Write((const char *)MEMBUFFER_BUFFER(buffer), - MEMBUFFER_OFFSET(buffer), file_ctx->plugin.init_data, file_ctx->plugin.thread_data); + } else if (file_ctx->type == LOGFILE_TYPE_FILETYPE) { + file_ctx->filetype.filetype->Write((const char *)MEMBUFFER_BUFFER(buffer), + MEMBUFFER_OFFSET(buffer), file_ctx->filetype.init_data, + file_ctx->filetype.thread_data); } #ifdef HAVE_LIBHIREDIS else if (file_ctx->type == LOGFILE_TYPE_REDIS) { diff --git a/src/util-logopenfile.h b/src/util-logopenfile.h index f3ab81565a3b..5e2fd327d33a 100644 --- a/src/util-logopenfile.h +++ b/src/util-logopenfile.h @@ -33,7 +33,6 @@ #include "util-log-redis.h" #endif /* HAVE_LIBHIREDIS */ -#include "suricata-plugin.h" #include "output-eve.h" enum LogFileType { @@ -41,7 +40,8 @@ enum LogFileType { LOGFILE_TYPE_UNIX_DGRAM, LOGFILE_TYPE_UNIX_STREAM, LOGFILE_TYPE_REDIS, - LOGFILE_TYPE_PLUGIN, + /** New style or modular filetypes. */ + LOGFILE_TYPE_FILETYPE, LOGFILE_TYPE_NOTSET }; @@ -66,11 +66,11 @@ typedef struct LogThreadedFileCtx_ { char *append; } LogThreadedFileCtx; -typedef struct LogFilePluginCtx_ { - SCEveFileType *plugin; +typedef struct LogFileTypeCtx_ { + SCEveFileType *filetype; void *init_data; void *thread_data; -} LogFilePluginCtx; +} LogFileTypeCtx; /** Global structure for Output Context */ typedef struct LogFileCtx_ { @@ -91,7 +91,7 @@ typedef struct LogFileCtx_ { int (*Write)(const char *buffer, int buffer_len, struct LogFileCtx_ *fp); void (*Close)(struct LogFileCtx_ *fp); - LogFilePluginCtx plugin; + LogFileTypeCtx filetype; /** It will be locked if the log/alert * record cannot be written to the file in one call */ From bd55cd4c5500c7e3f00026fec9bdd057493439ff Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Fri, 8 Mar 2024 00:23:25 -0600 Subject: [PATCH 12/26] eve/filetypes: common init for threaded and non-threaded In 7.0 if EVE was non-threaded, the ThreadInit for the filetype was not called meaning that the filetype author had to handle the threaded and non-threaded cases. To simplify this, if non-threaded, still call ThreadInit (and ThreadDeinit) once with a thread_id of 0. This should simplify authoring EVE filetype plugins. --- examples/plugins/c-json-filetype/filetype.c | 38 ++++++--------------- src/output-json.c | 9 +++-- src/util-logopenfile.c | 2 +- 3 files changed, 17 insertions(+), 32 deletions(-) diff --git a/examples/plugins/c-json-filetype/filetype.c b/examples/plugins/c-json-filetype/filetype.c index e85273d76885..9b7d86e80b6d 100644 --- a/examples/plugins/c-json-filetype/filetype.c +++ b/examples/plugins/c-json-filetype/filetype.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2020-2023 Open Information Security Foundation +/* Copyright (C) 2020-2024 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -43,9 +43,6 @@ typedef struct ThreadData_ { typedef struct Context_ { /** Verbose, or print to stdout. */ int verbose; - - /** A thread context to use when not running in threaded mode. */ - ThreadData *thread; } Context; /** @@ -59,9 +56,9 @@ typedef struct Context_ { * \param data A pointer where context data can be stored relevant to this * output. * - * Eve output plugins need to be thread aware as the threading happens at lower - * level than the EVE output, so a flag is provided here to notify the plugin if - * threading is enabled or not. + * Eve output plugins need to be thread aware as the threading happens + * at a lower level than the EVE output, so a flag is provided here to + * notify the plugin if threading is enabled or not. * * If the plugin does not work with threads disabled, or enabled, this function * should return -1. @@ -92,15 +89,6 @@ static int FiletypeInit(ConfNode *conf, bool threaded, void **data) } context->verbose = verbose; - if (!threaded) { - /* We're not running in threaded mode so allocate a thread context here - * to avoid duplication of context data such as file pointers, database - * connections, etc. */ - if (FiletypeThreadInit(context, 0, (void **)&context->thread) != 0) { - SCFree(context); - return -1; - } - } *data = context; return 0; } @@ -115,12 +103,9 @@ static int FiletypeInit(ConfNode *conf, bool threaded, void **data) */ static void FiletypeDeinit(void *data) { - printf("TemplateClose\n"); + SCLogNotice("data=%p", data); Context *ctx = data; if (ctx != NULL) { - if (ctx->thread) { - FiletypeThreadDeinit(ctx, (void *)ctx->thread); - } SCFree(ctx); } } @@ -140,12 +125,12 @@ static void FiletypeDeinit(void *data) * of "eve..json". This plugin may want to do similar, or open * multiple connections to whatever the final logging location might be. * - * In the case of non-threaded EVE logging this function is NOT called by - * Suricata, but instead this plugin chooses to use this method to create a - * default (single) thread context. + * In the case of non-threaded EVE logging this function is called + * once with a thread_id of 0. */ static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) { + SCLogNotice("thread_id=%d", thread_id); ThreadData *tdata = SCCalloc(1, sizeof(ThreadData)); if (tdata == NULL) { SCLogError("Failed to allocate thread data"); @@ -166,6 +151,7 @@ static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) */ static int FiletypeThreadDeinit(void *ctx, void *thread_data) { + SCLogNotice("thread_data=%p", thread_data); if (thread_data == NULL) { // Nothing to do. return 0; @@ -194,11 +180,7 @@ static int FiletypeWrite(const char *buffer, int buffer_len, void *data, void *t Context *ctx = data; ThreadData *thread = thread_data; - /* The thread_data could be null which is valid, or it could be that we are - * in single threaded mode. */ - if (thread == NULL) { - thread = ctx->thread; - } + SCLogNotice("thread_id=%d, data=%p, thread_data=%p", thread->thread_id, data, thread_data); thread->count++; diff --git a/src/output-json.c b/src/output-json.c index 830027cb02c4..be2cc81fe2be 100644 --- a/src/output-json.c +++ b/src/output-json.c @@ -1015,12 +1015,15 @@ static int LogFileTypePrepare( return -1; } } - void *init_data = NULL; - if (json_ctx->filetype->Init(conf, json_ctx->file_ctx->threaded, &init_data) < 0) { + if (json_ctx->filetype->Init(conf, json_ctx->file_ctx->threaded, + &json_ctx->file_ctx->filetype.init_data) < 0) { + return -1; + } + if (json_ctx->filetype->ThreadInit(json_ctx->file_ctx->filetype.init_data, 0, + &json_ctx->file_ctx->filetype.thread_data) < 0) { return -1; } json_ctx->file_ctx->filetype.filetype = json_ctx->filetype; - json_ctx->file_ctx->filetype.init_data = init_data; } return 0; diff --git a/src/util-logopenfile.c b/src/util-logopenfile.c index 5675b145be7c..b58bdd43efc7 100644 --- a/src/util-logopenfile.c +++ b/src/util-logopenfile.c @@ -871,7 +871,7 @@ int LogFileFreeCtx(LogFileCtx *lf_ctx) SCReturnInt(0); } - if (lf_ctx->type == LOGFILE_TYPE_FILETYPE && lf_ctx->parent != NULL) { + if (lf_ctx->type == LOGFILE_TYPE_FILETYPE) { lf_ctx->filetype.filetype->ThreadDeinit( lf_ctx->filetype.init_data, lf_ctx->filetype.thread_data); } From 500d29f356f67792c87471f34679daa143bd348e Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 16:59:14 -0600 Subject: [PATCH 13/26] doxygen: document the examples directory --- doxygen.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doxygen.cfg b/doxygen.cfg index 5e07af09c618..22fc4543a34d 100644 --- a/doxygen.cfg +++ b/doxygen.cfg @@ -829,7 +829,7 @@ WARN_LOGFILE = # spaces. See also FILE_PATTERNS and EXTENSION_MAPPING # Note: If this tag is empty the current directory is searched. -INPUT = src/ libhtp/htp/ +INPUT = src/ libhtp/htp/ examples/ # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses From cd85d89f1462080b68f9b832490b9dac32395fc6 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 16:59:25 -0600 Subject: [PATCH 14/26] output-eve: doxygen docs for SCEveFileType Add documentation for the SCEveFileType in Doxygen format. --- src/output-eve.h | 132 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 123 insertions(+), 9 deletions(-) diff --git a/src/output-eve.h b/src/output-eve.h index e8c41996229e..385ecbf45816 100644 --- a/src/output-eve.h +++ b/src/output-eve.h @@ -36,23 +36,137 @@ typedef uint32_t ThreadId; /** - * Structure used to define an Eve output file type plugin. + * \brief Structure used to define an EVE output file type plugin. + * + * EVE filetypes implement an object with a file-like interface and + * are used to output EVE log records to files, syslog, or + * database. They can be built-in such as the syslog (see + * SyslogInitialize()) and nullsink (see NullLogInitialize()) outputs, + * registered by a library user or dynamically loaded as a plugin. + * + * The life cycle of an EVE filetype is: + * - Init: called once for each EVE instance using this filetype + * - ThreadInit: called once for each output thread + * - Write: called for each log record + * - ThreadInit: called once for each output thread on exit + * - Deinit: called once for each EVE instance using this filetype on exit + * + * Examples: + * - built-in syslog: \ref src/output-eve-syslog.c + * - built-in nullsink: \ref src/output-eve-null.c + * - example plugin: \ref examples/plugins/c-json-filetype/filetype.c + * + * ### Multi-Threaded Note: + * + * The EVE logging system can be configured by the Suricata user to + * run in threaded or non-threaded modes. In the default non-threaded + * mode, ThreadInit will only be called once and the filetype does not + * need to be concerned with threads. + * + * However, in **threaded** mode, ThreadInit will be called multiple + * times and the filetype needs to be thread aware and thread-safe. If + * utilizing a unique resource such as a file for each thread then you + * may be naturally thread safe. However, if sharing a single file + * handle across all threads then your filetype will have to take care + * of locking, etc. */ typedef struct SCEveFileType_ { - /* The name of the output, used to specify the output in the filetype section - * of the eve-log configuration. */ + /** + * \brief The name of the output, used in the configuration. + * + * This name is used by the configuration file to specify the EVE + * filetype used. + * + * For example: + * + * \code{.yaml} + * outputs: + * - eve-log: + * filetype: my-output-name + * \endcode + */ const char *name; - /* Init Called on first access */ + + /** + * \brief Function to initialize this filetype. + * + * \param conf The ConfNode of the `eve-log` configuration + * section this filetype is being initialized for + * + * \param threaded Flag to specify if the EVE sub-systems is in + * threaded mode or not + * + * \param init_data An output pointer for filetype specific data + * + * \retval 0 on success, -1 on failure + */ int (*Init)(ConfNode *conf, bool threaded, void **init_data); - /* Write - Called on each write to the object */ + + /** + * \brief Called for each EVE log record. + * + * The Write function is called for each log EVE log record. The + * provided buffer contains a fully formatted EVE record in JSON + * format. + * + * \param buffer The fully formatted JSON EVE log record + * + * \param buffer_len The length of the buffer + * + * \param init_data The data setup in the call to Init + * + * \param thread_data The data setup in the call to ThreadInit + * + * \retval 0 on success, -1 on failure + */ int (*Write)(const char *buffer, int buffer_len, void *init_data, void *thread_data); - /* Close - Called on final close */ + + /** + * \brief Final call to deinitialize this filetype. + * + * Called, usually on exit to deinitialize and free any resources + * allocated during Init. + * + * \param init_data Data setup in the call to Init. + */ void (*Deinit)(void *init_data); - /* ThreadInit - Called for each thread using file object; non-zero thread_ids correlate - * to Suricata's worker threads; 0 correlates to the Suricata main thread */ + + /** + * \brief Initialize thread specific data. + * + * Initialize any thread specific data. For example, if + * implementing a file output you might open the files here, so + * you have one output file per thread. + * + * \param init_data Data setup during Init + * + * \param thread_id A unique ID to differentiate this thread from + * others. If EVE is not in threaded mode this will be called + * one with a ThreadId of 0. In threaded mode the ThreadId of + * 0 correlates to the main Suricata thread. + * + * \param thread_data Output pointer for any data required by this + * thread. + * + * \retval 0 on success, -1 on failure + */ int (*ThreadInit)(void *init_data, ThreadId thread_id, void **thread_data); - /* ThreadDeinit - Called for each thread using file object */ + + /** + * \brief Called to deinitialize each thread. + * + * This function will be called for each thread. It is where any + * resources allocated in ThreadInit should be released. + * + * \param init_data The data setup in Init + * + * \param thread_data The data setup in ThreadInit + * + * \retval 0 on success, -1 on failure + */ int (*ThreadDeinit)(void *init_data, void *thread_data); + + /* Internal list management. */ TAILQ_ENTRY(SCEveFileType_) entries; } SCEveFileType; From eee9757dba4d7b1734ee861d5035778275b1d64e Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 17:06:50 -0600 Subject: [PATCH 15/26] eve/filetype: ThreadDeinit can return void Change ThreadDeinit to return void instead of an int, there is nothing to be done on success or failure. --- examples/plugins/c-json-filetype/filetype.c | 8 ++------ src/output-eve-null.c | 3 +-- src/output-eve.h | 4 +--- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/examples/plugins/c-json-filetype/filetype.c b/examples/plugins/c-json-filetype/filetype.c index 9b7d86e80b6d..e20697bcb4c1 100644 --- a/examples/plugins/c-json-filetype/filetype.c +++ b/examples/plugins/c-json-filetype/filetype.c @@ -23,9 +23,6 @@ #define FILETYPE_NAME "json-filetype-plugin" -static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data); -static int FiletypeThreadDeinit(void *ctx, void *thread_data); - /** * Per thread context data for each logging thread. */ @@ -149,19 +146,18 @@ static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) * This is where any cleanup per thread should be done including free'ing of the * thread_data if needed. */ -static int FiletypeThreadDeinit(void *ctx, void *thread_data) +static void FiletypeThreadDeinit(void *ctx, void *thread_data) { SCLogNotice("thread_data=%p", thread_data); if (thread_data == NULL) { // Nothing to do. - return 0; + return; } ThreadData *tdata = thread_data; SCLogNotice( "Deinitializing thread %d: records written: %" PRIu64, tdata->thread_id, tdata->count); SCFree(tdata); - return 0; } /** diff --git a/src/output-eve-null.c b/src/output-eve-null.c index afe11afc068f..59e8c452fbec 100644 --- a/src/output-eve-null.c +++ b/src/output-eve-null.c @@ -54,9 +54,8 @@ static int NullLogThreadInit(void *init_data, ThreadId thread_id, void **thread_ return 0; } -static int NullLogThreadDeInit(void *init_data, void *thread_data) +static void NullLogThreadDeInit(void *init_data, void *thread_data) { - return 0; } static void NullLogDeInit(void *init_data) diff --git a/src/output-eve.h b/src/output-eve.h index 385ecbf45816..6ee0228a07ea 100644 --- a/src/output-eve.h +++ b/src/output-eve.h @@ -161,10 +161,8 @@ typedef struct SCEveFileType_ { * \param init_data The data setup in Init * * \param thread_data The data setup in ThreadInit - * - * \retval 0 on success, -1 on failure */ - int (*ThreadDeinit)(void *init_data, void *thread_data); + void (*ThreadDeinit)(void *init_data, void *thread_data); /* Internal list management. */ TAILQ_ENTRY(SCEveFileType_) entries; From a3354e55e674213b6232df89a713f6d3c43e14a9 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 17:13:25 -0600 Subject: [PATCH 16/26] eve/filetypes: use more const --- examples/plugins/c-json-filetype/filetype.c | 11 ++++++----- src/output-eve-null.c | 9 +++++---- src/output-eve-syslog.c | 7 ++++--- src/output-eve.h | 9 +++++---- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/examples/plugins/c-json-filetype/filetype.c b/examples/plugins/c-json-filetype/filetype.c index e20697bcb4c1..a66b9779dfb3 100644 --- a/examples/plugins/c-json-filetype/filetype.c +++ b/examples/plugins/c-json-filetype/filetype.c @@ -64,7 +64,7 @@ typedef struct Context_ { * configuration for the eve instance, not just a node named after the plugin. * This allows the plugin to get more context about what it is logging. */ -static int FiletypeInit(ConfNode *conf, bool threaded, void **data) +static int FiletypeInit(const ConfNode *conf, const bool threaded, void **data) { SCLogNotice("Initializing template eve output plugin: threaded=%d", threaded); Context *context = SCCalloc(1, sizeof(Context)); @@ -125,7 +125,7 @@ static void FiletypeDeinit(void *data) * In the case of non-threaded EVE logging this function is called * once with a thread_id of 0. */ -static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) +static int FiletypeThreadInit(const void *ctx, const ThreadId thread_id, void **thread_data) { SCLogNotice("thread_id=%d", thread_id); ThreadData *tdata = SCCalloc(1, sizeof(ThreadData)); @@ -146,7 +146,7 @@ static int FiletypeThreadInit(void *ctx, ThreadId thread_id, void **thread_data) * This is where any cleanup per thread should be done including free'ing of the * thread_data if needed. */ -static void FiletypeThreadDeinit(void *ctx, void *thread_data) +static void FiletypeThreadDeinit(const void *ctx, void *thread_data) { SCLogNotice("thread_data=%p", thread_data); if (thread_data == NULL) { @@ -171,9 +171,10 @@ static void FiletypeThreadDeinit(void *ctx, void *thread_data) * to any resource that may block it might be best to enqueue the buffers for * further processing which will require copying of the provided buffer. */ -static int FiletypeWrite(const char *buffer, int buffer_len, void *data, void *thread_data) +static int FiletypeWrite( + const char *buffer, const int buffer_len, const void *data, void *thread_data) { - Context *ctx = data; + const Context *ctx = data; ThreadData *thread = thread_data; SCLogNotice("thread_id=%d, data=%p, thread_data=%p", thread->thread_id, data, thread_data); diff --git a/src/output-eve-null.c b/src/output-eve-null.c index 59e8c452fbec..368836e78301 100644 --- a/src/output-eve-null.c +++ b/src/output-eve-null.c @@ -37,24 +37,25 @@ void NullLogInitialize(void) #define OUTPUT_NAME "nullsink" -static int NullLogInit(ConfNode *conf, bool threaded, void **init_data) +static int NullLogInit(const ConfNode *conf, const bool threaded, void **init_data) { *init_data = NULL; return 0; } -static int NullLogWrite(const char *buffer, int buffer_len, void *init_data, void *thread_data) +static int NullLogWrite( + const char *buffer, const int buffer_len, const void *init_data, void *thread_data) { return 0; } -static int NullLogThreadInit(void *init_data, ThreadId thread_id, void **thread_data) +static int NullLogThreadInit(const void *init_data, const ThreadId thread_id, void **thread_data) { *thread_data = NULL; return 0; } -static void NullLogThreadDeInit(void *init_data, void *thread_data) +static void NullLogThreadDeInit(const void *init_data, void *thread_data) { } diff --git a/src/output-eve-syslog.c b/src/output-eve-syslog.c index 0787e4b75071..4560bc73e3a8 100644 --- a/src/output-eve-syslog.c +++ b/src/output-eve-syslog.c @@ -41,7 +41,7 @@ typedef struct Context_ { int alert_syslog_level; } Context; -static int SyslogInit(ConfNode *conf, bool threaded, void **init_data) +static int SyslogInit(const ConfNode *conf, const bool threaded, void **init_data) { Context *context = SCCalloc(1, sizeof(Context)); if (context == NULL) { @@ -79,9 +79,10 @@ static int SyslogInit(ConfNode *conf, bool threaded, void **init_data) return 0; } -static int SyslogWrite(const char *buffer, int buffer_len, void *init_data, void *thread_data) +static int SyslogWrite( + const char *buffer, const int buffer_len, const void *init_data, void *thread_data) { - Context *context = init_data; + const Context *context = init_data; syslog(context->alert_syslog_level, "%s", (const char *)buffer); return 0; diff --git a/src/output-eve.h b/src/output-eve.h index 6ee0228a07ea..3b2637f64ccf 100644 --- a/src/output-eve.h +++ b/src/output-eve.h @@ -100,7 +100,7 @@ typedef struct SCEveFileType_ { * * \retval 0 on success, -1 on failure */ - int (*Init)(ConfNode *conf, bool threaded, void **init_data); + int (*Init)(const ConfNode *conf, const bool threaded, void **init_data); /** * \brief Called for each EVE log record. @@ -119,7 +119,8 @@ typedef struct SCEveFileType_ { * * \retval 0 on success, -1 on failure */ - int (*Write)(const char *buffer, int buffer_len, void *init_data, void *thread_data); + int (*Write)( + const char *buffer, const int buffer_len, const void *init_data, void *thread_data); /** * \brief Final call to deinitialize this filetype. @@ -150,7 +151,7 @@ typedef struct SCEveFileType_ { * * \retval 0 on success, -1 on failure */ - int (*ThreadInit)(void *init_data, ThreadId thread_id, void **thread_data); + int (*ThreadInit)(const void *init_data, const ThreadId thread_id, void **thread_data); /** * \brief Called to deinitialize each thread. @@ -162,7 +163,7 @@ typedef struct SCEveFileType_ { * * \param thread_data The data setup in ThreadInit */ - void (*ThreadDeinit)(void *init_data, void *thread_data); + void (*ThreadDeinit)(const void *init_data, void *thread_data); /* Internal list management. */ TAILQ_ENTRY(SCEveFileType_) entries; From b7b16fb4811c5fb3a8196264fb6df1c0641825a1 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 17:14:30 -0600 Subject: [PATCH 17/26] eve/filetype: reorder fields to match lifecycle Enhances readability. --- src/output-eve.h | 60 ++++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/output-eve.h b/src/output-eve.h index 3b2637f64ccf..7046c7b98005 100644 --- a/src/output-eve.h +++ b/src/output-eve.h @@ -102,36 +102,6 @@ typedef struct SCEveFileType_ { */ int (*Init)(const ConfNode *conf, const bool threaded, void **init_data); - /** - * \brief Called for each EVE log record. - * - * The Write function is called for each log EVE log record. The - * provided buffer contains a fully formatted EVE record in JSON - * format. - * - * \param buffer The fully formatted JSON EVE log record - * - * \param buffer_len The length of the buffer - * - * \param init_data The data setup in the call to Init - * - * \param thread_data The data setup in the call to ThreadInit - * - * \retval 0 on success, -1 on failure - */ - int (*Write)( - const char *buffer, const int buffer_len, const void *init_data, void *thread_data); - - /** - * \brief Final call to deinitialize this filetype. - * - * Called, usually on exit to deinitialize and free any resources - * allocated during Init. - * - * \param init_data Data setup in the call to Init. - */ - void (*Deinit)(void *init_data); - /** * \brief Initialize thread specific data. * @@ -153,6 +123,26 @@ typedef struct SCEveFileType_ { */ int (*ThreadInit)(const void *init_data, const ThreadId thread_id, void **thread_data); + /** + * \brief Called for each EVE log record. + * + * The Write function is called for each log EVE log record. The + * provided buffer contains a fully formatted EVE record in JSON + * format. + * + * \param buffer The fully formatted JSON EVE log record + * + * \param buffer_len The length of the buffer + * + * \param init_data The data setup in the call to Init + * + * \param thread_data The data setup in the call to ThreadInit + * + * \retval 0 on success, -1 on failure + */ + int (*Write)( + const char *buffer, const int buffer_len, const void *init_data, void *thread_data); + /** * \brief Called to deinitialize each thread. * @@ -165,6 +155,16 @@ typedef struct SCEveFileType_ { */ void (*ThreadDeinit)(const void *init_data, void *thread_data); + /** + * \brief Final call to deinitialize this filetype. + * + * Called, usually on exit to deinitialize and free any resources + * allocated during Init. + * + * \param init_data Data setup in the call to Init. + */ + void (*Deinit)(void *init_data); + /* Internal list management. */ TAILQ_ENTRY(SCEveFileType_) entries; } SCEveFileType; From 8284df3ed426bc0e07899fc12c51b944b3627e33 Mon Sep 17 00:00:00 2001 From: Jason Ish Date: Mon, 11 Mar 2024 17:23:33 -0600 Subject: [PATCH 18/26] devguide: add an upgrade section Add an upgrade section to the devguide. This should cover any changes to APIs that users might be using from plugins or as a library user. --- doc/userguide/devguide/index.rst | 1 + doc/userguide/devguide/upgrading/index.rst | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 doc/userguide/devguide/upgrading/index.rst diff --git a/doc/userguide/devguide/index.rst b/doc/userguide/devguide/index.rst index 9393af8b3209..bcfac16812c8 100644 --- a/doc/userguide/devguide/index.rst +++ b/doc/userguide/devguide/index.rst @@ -9,3 +9,4 @@ Suricata Developer Guide internals/index.rst extending/index.rst libsuricata/index.rst + upgrading/index.rst diff --git a/doc/userguide/devguide/upgrading/index.rst b/doc/userguide/devguide/upgrading/index.rst new file mode 100644 index 000000000000..fb5d0c3e6ab1 --- /dev/null +++ b/doc/userguide/devguide/upgrading/index.rst @@ -0,0 +1,21 @@ +Upgrading +========= + +Upgrading 7.0 to 8.0 +-------------------- + +EVE File Types +~~~~~~~~~~~~~~ + +- The ``ThreadInit`` function will now be called when in *threaded* + and *non-threaded* modes. This simplifies the initialization for EVE + filetypes as they can use the same flow of execution for both + modes. To upgrade, either remove the call to ``ThreadInit`` from + ``Init``, or move per-thread setup code from ``Init`` to + ``ThreadInit``. +- Many of the function arguments to the callbacks have been made + ``const`` where it made sense. + +Please see the latest example EVE filetype plugin for an up to date +example. + From 45bb936187352898036ba3732318cb36ee9e3cbf Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Mon, 11 Mar 2024 11:18:34 +0100 Subject: [PATCH 19/26] http: event on request line missing protocol Ticket: 6856 --- rules/http-events.rules | 4 +++- src/app-layer-htp.c | 2 ++ src/app-layer-htp.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/rules/http-events.rules b/rules/http-events.rules index 8c7763f1b661..b5cce76bf235 100644 --- a/rules/http-events.rules +++ b/rules/http-events.rules @@ -91,4 +91,6 @@ alert http any any -> any any (msg:"SURICATA HTTP failed protocol change"; flow: #alert http any any -> any any (msg:"SURICATA HTTP request chunk extension"; flow:established; app-layer-event:http.request_chunk_extension; classtype:protocol-command-decode; sid:2221054; rev:1;) -# next sid 2221055 +alert http any any -> any any (msg:"SURICATA HTTP request missing protocol"; flow:established,to_server; app-layer-event:http.request_line_missing_protocol; classtype:protocol-command-decode; sid:2221055; rev:1;) + +# next sid 2221056 diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index f8e6e9e8de06..1b4d31c841c8 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -167,6 +167,7 @@ SCEnumCharMap http_decoder_event_table[] = { { "RANGE_INVALID", HTTP_DECODER_EVENT_RANGE_INVALID }, { "REQUEST_CHUNK_EXTENSION", HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION }, + { "REQUEST_LINE_MISSING_PROTOCOL", HTTP_DECODER_EVENT_REQUEST_LINE_MISSING_PROTOCOL }, /* suricata warnings/errors */ { "MULTIPART_GENERIC_ERROR", HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR }, @@ -642,6 +643,7 @@ struct { { "Ambiguous response C-L value", HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE }, { "Request chunk extension", HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION }, + { "Request line: missing protocol", HTTP_DECODER_EVENT_REQUEST_LINE_MISSING_PROTOCOL }, }; #define HTP_ERROR_MAX (sizeof(htp_errors) / sizeof(htp_errors[0])) diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index a61121db595c..f3a5aedaab51 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -128,6 +128,7 @@ enum { HTTP_DECODER_EVENT_RANGE_INVALID, HTTP_DECODER_EVENT_REQUEST_CHUNK_EXTENSION, + HTTP_DECODER_EVENT_REQUEST_LINE_MISSING_PROTOCOL, /* suricata errors/warnings */ HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR, From 0b5966c3474abeb17292510a3bd8804dadb60810 Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Sat, 2 Mar 2024 18:15:16 +0100 Subject: [PATCH 20/26] dpdk: only close the port when workers are synchronized When Suricata was running in IPS mode and received a signal to stop, the first worker of every interface/port stopped the port and proactively stopped the peered interface as well. This was done to be as accurate with port stats as possible. However, in a highly active scenarios (lots of packets moving around) the peered workers might still be in the process of a packet release operation. These workers would then attempt to transmit on a stopped interface - resulting in an errorneous operation. Instead, this patch proposes a worker synchronization of the given port. After these workers are synchronized, it is known that no packets will be sent of the peered interface, therefore the first worker can stop it. This however cannot be assumed about "its own" port as the peered workers can still try to send the packets. Therefore, ports are only stopped by the peered workers. Ticket: #6790 --- src/runmode-dpdk.c | 6 ++++++ src/source-dpdk.c | 20 +++++++++++++++++++- src/source-dpdk.h | 7 +++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/runmode-dpdk.c b/src/runmode-dpdk.c index 64f849d08596..33fc53a7ddec 100644 --- a/src/runmode-dpdk.c +++ b/src/runmode-dpdk.c @@ -1592,6 +1592,12 @@ static void *ParseDpdkConfigAndConfigureDevice(const char *iface) // This counter is increased by worker threads that individually pick queue IDs. SC_ATOMIC_RESET(iconf->queue_id); SC_ATOMIC_RESET(iconf->inconsitent_numa_cnt); + iconf->workers_sync = SCCalloc(1, sizeof(*iconf->workers_sync)); + if (iconf->workers_sync == NULL) { + FatalError("Failed to allocate memory for workers_sync"); + } + SC_ATOMIC_RESET(iconf->workers_sync->worker_checked_in); + iconf->workers_sync->worker_cnt = iconf->threads; // initialize LiveDev DPDK values LiveDevice *ldev_instance = LiveGetDevice(iface); diff --git a/src/source-dpdk.c b/src/source-dpdk.c index 6a7833ee4b62..21d67526972a 100644 --- a/src/source-dpdk.c +++ b/src/source-dpdk.c @@ -134,6 +134,7 @@ typedef struct DPDKThreadVars_ { int32_t port_socket_id; struct rte_mempool *pkt_mempool; struct rte_mbuf *received_mbufs[BURST_SIZE]; + DPDKWorkerSync *workers_sync; } DPDKThreadVars; static TmEcode ReceiveDPDKThreadInit(ThreadVars *, const void *, void **); @@ -427,10 +428,22 @@ static TmEcode ReceiveDPDKLoop(ThreadVars *tv, void *data, void *slot) while (1) { if (unlikely(suricata_ctl_flags != 0)) { SCLogDebug("Stopping Suricata!"); + SC_ATOMIC_ADD(ptv->workers_sync->worker_checked_in, 1); + while (SC_ATOMIC_GET(ptv->workers_sync->worker_checked_in) < + ptv->workers_sync->worker_cnt) { + rte_delay_us(10); + } if (ptv->queue_id == 0) { - rte_eth_dev_stop(ptv->port_id); + rte_delay_us(20); // wait for all threads to get out of the sync loop + SC_ATOMIC_SET(ptv->workers_sync->worker_checked_in, 0); + // If Suricata runs in peered mode, the peer threads might still want to send + // packets to our port. Instead, we know, that we are done with the peered port, so + // we stop it. The peered threads will stop our port. if (ptv->copy_mode == DPDK_COPY_MODE_TAP || ptv->copy_mode == DPDK_COPY_MODE_IPS) { rte_eth_dev_stop(ptv->out_port_id); + } else { + // in IDS we stop our port - no peer threads are running + rte_eth_dev_stop(ptv->port_id); } } DPDKDumpCounters(ptv); @@ -605,6 +618,7 @@ static TmEcode ReceiveDPDKThreadInit(ThreadVars *tv, const void *initdata, void ptv->port_socket_id, thread_numa); } + ptv->workers_sync = dpdk_config->workers_sync; uint16_t queue_id = SC_ATOMIC_ADD(dpdk_config->queue_id, 1); ptv->queue_id = queue_id; @@ -748,6 +762,10 @@ static TmEcode ReceiveDPDKThreadDeinit(ThreadVars *tv, void *data) } DevicePreClosePMDSpecificActions(ptv, dev_info.driver_name); + + if (ptv->workers_sync) { + SCFree(ptv->workers_sync); + } } ptv->pkt_mempool = NULL; // MP is released when device is closed diff --git a/src/source-dpdk.h b/src/source-dpdk.h index 3187793e2c8a..dce791c14cec 100644 --- a/src/source-dpdk.h +++ b/src/source-dpdk.h @@ -43,6 +43,12 @@ typedef enum { DPDK_COPY_MODE_NONE, DPDK_COPY_MODE_TAP, DPDK_COPY_MODE_IPS } Dpd #define DPDK_RX_CHECKSUM_OFFLOAD (1 << 4) /**< Enable chsum offload */ void DPDKSetTimevalOfMachineStart(void); + +typedef struct DPDKWorkerSync_ { + uint16_t worker_cnt; + SC_ATOMIC_DECLARE(uint16_t, worker_checked_in); +} DPDKWorkerSync; + typedef struct DPDKIfaceConfig_ { #ifdef HAVE_DPDK char iface[RTE_ETH_NAME_MAX_LEN]; @@ -71,6 +77,7 @@ typedef struct DPDKIfaceConfig_ { /* threads bind queue id one by one */ SC_ATOMIC_DECLARE(uint16_t, queue_id); SC_ATOMIC_DECLARE(uint16_t, inconsitent_numa_cnt); + DPDKWorkerSync *workers_sync; void (*DerefFunc)(void *); struct rte_flow *flow[100]; From 5592ec079d3f37c3ad41674e05e808555be26921 Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Tue, 12 Mar 2024 23:24:07 +0100 Subject: [PATCH 21/26] dpdk: refactor the main packet loop into smaller functions --- src/source-dpdk.c | 303 +++++++++++++++++++++++++++------------------- 1 file changed, 176 insertions(+), 127 deletions(-) diff --git a/src/source-dpdk.c b/src/source-dpdk.c index 21d67526972a..69e13bc1640f 100644 --- a/src/source-dpdk.c +++ b/src/source-dpdk.c @@ -185,7 +185,8 @@ static inline void InterruptsTurnOnOff(uint16_t port_id, uint16_t queue_id, bool rte_spinlock_unlock(&(intr_lock[port_id])); } -static void DPDKFreeMbufArray(struct rte_mbuf **mbuf_array, uint16_t mbuf_cnt, uint16_t offset) +static inline void DPDKFreeMbufArray( + struct rte_mbuf **mbuf_array, uint16_t mbuf_cnt, uint16_t offset) { for (int i = offset; i < mbuf_cnt; i++) { rte_pktmbuf_free(mbuf_array[i]); @@ -394,151 +395,204 @@ static void DPDKReleasePacket(Packet *p) PacketFreeOrRelease(p); } -/** - * \brief Main DPDK reading Loop function - */ -static TmEcode ReceiveDPDKLoop(ThreadVars *tv, void *data, void *slot) +static TmEcode ReceiveDPDKLoopInit(ThreadVars *tv, DPDKThreadVars *ptv) { SCEnter(); - Packet *p; - uint16_t nb_rx; - time_t last_dump = 0; - time_t current_time; - bool segmented_mbufs_warned = 0; - SCTime_t t = DPDKSetTimevalReal(&machine_start_time); - uint64_t last_timeout_msec = SCTIME_MSECS(t); - - DPDKThreadVars *ptv = (DPDKThreadVars *)data; - TmSlot *s = (TmSlot *)slot; - - ptv->slot = s->slot_next; - - // Indicate that the thread is actually running its application level code (i.e., it can poll - // packets) + // Indicate that the thread is actually running its application level + // code (i.e., it can poll packets) TmThreadsSetFlag(tv, THV_RUNNING); PacketPoolWait(); rte_eth_stats_reset(ptv->port_id); rte_eth_xstats_reset(ptv->port_id); - uint32_t pwd_zero_rx_packet_polls_count = 0; if (ptv->intr_enabled && !InterruptsRXEnable(ptv->port_id, ptv->queue_id)) SCReturnInt(TM_ECODE_FAILED); - while (1) { - if (unlikely(suricata_ctl_flags != 0)) { - SCLogDebug("Stopping Suricata!"); - SC_ATOMIC_ADD(ptv->workers_sync->worker_checked_in, 1); - while (SC_ATOMIC_GET(ptv->workers_sync->worker_checked_in) < - ptv->workers_sync->worker_cnt) { - rte_delay_us(10); + SCReturnInt(TM_ECODE_OK); +} + +static inline void LoopHandleTimeoutOnIdle(ThreadVars *tv) +{ + static uint64_t last_timeout_msec = 0; + SCTime_t t = DPDKSetTimevalReal(&machine_start_time); + uint64_t msecs = SCTIME_MSECS(t); + if (msecs > last_timeout_msec + 100) { + TmThreadsCaptureHandleTimeout(tv, NULL); + last_timeout_msec = msecs; + } +} + +/** + * \brief Decides if it should retry the packet poll or continue with the packet processing + * \return true if the poll should be retried, false otherwise + */ +static inline bool RXPacketCountHeuristic(ThreadVars *tv, DPDKThreadVars *ptv, uint16_t nb_rx) +{ + static uint32_t zero_pkt_polls_cnt = 0; + + if (nb_rx > 0) { + zero_pkt_polls_cnt = 0; + return false; + } + + LoopHandleTimeoutOnIdle(tv); + if (!ptv->intr_enabled) + return true; + + zero_pkt_polls_cnt++; + if (zero_pkt_polls_cnt <= MIN_ZERO_POLL_COUNT) + return true; + + uint32_t pwd_idle_hint = InterruptsSleepHeuristic(zero_pkt_polls_cnt); + if (pwd_idle_hint < STANDARD_SLEEP_TIME_US) { + rte_delay_us(pwd_idle_hint); + } else { + InterruptsTurnOnOff(ptv->port_id, ptv->queue_id, true); + struct rte_epoll_event event; + rte_epoll_wait(RTE_EPOLL_PER_THREAD, &event, 1, MAX_EPOLL_TIMEOUT_MS); + InterruptsTurnOnOff(ptv->port_id, ptv->queue_id, false); + return true; + } + + return false; +} + +/** + * \brief Initializes a packet from an mbuf + * \return true if the packet was initialized successfully, false otherwise + */ +static inline Packet *PacketInitFromMbuf(DPDKThreadVars *ptv, struct rte_mbuf *mbuf) +{ + Packet *p = PacketGetFromQueueOrAlloc(); + if (unlikely(p == NULL)) { + return NULL; + } + PKT_SET_SRC(p, PKT_SRC_WIRE); + p->datalink = LINKTYPE_ETHERNET; + if (ptv->checksum_mode == CHECKSUM_VALIDATION_DISABLE) { + p->flags |= PKT_IGNORE_CHECKSUM; + } + + p->ts = DPDKSetTimevalReal(&machine_start_time); + p->dpdk_v.mbuf = mbuf; + p->ReleasePacket = DPDKReleasePacket; + p->dpdk_v.copy_mode = ptv->copy_mode; + p->dpdk_v.out_port_id = ptv->out_port_id; + p->dpdk_v.out_queue_id = ptv->queue_id; + p->livedev = ptv->livedev; + + if (ptv->checksum_mode == CHECKSUM_VALIDATION_DISABLE) { + p->flags |= PKT_IGNORE_CHECKSUM; + } else if (ptv->checksum_mode == CHECKSUM_VALIDATION_OFFLOAD) { + uint64_t ol_flags = p->dpdk_v.mbuf->ol_flags; + if ((ol_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) == RTE_MBUF_F_RX_IP_CKSUM_GOOD && + (ol_flags & RTE_MBUF_F_RX_L4_CKSUM_MASK) == RTE_MBUF_F_RX_L4_CKSUM_GOOD) { + SCLogDebug("HW detected GOOD IP and L4 chsum, ignoring validation"); + p->flags |= PKT_IGNORE_CHECKSUM; + } else { + if ((ol_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) == RTE_MBUF_F_RX_IP_CKSUM_BAD) { + SCLogDebug("HW detected BAD IP checksum"); + // chsum recalc will not be triggered but rule keyword check will be + p->level3_comp_csum = 0; } - if (ptv->queue_id == 0) { - rte_delay_us(20); // wait for all threads to get out of the sync loop - SC_ATOMIC_SET(ptv->workers_sync->worker_checked_in, 0); - // If Suricata runs in peered mode, the peer threads might still want to send - // packets to our port. Instead, we know, that we are done with the peered port, so - // we stop it. The peered threads will stop our port. - if (ptv->copy_mode == DPDK_COPY_MODE_TAP || ptv->copy_mode == DPDK_COPY_MODE_IPS) { - rte_eth_dev_stop(ptv->out_port_id); - } else { - // in IDS we stop our port - no peer threads are running - rte_eth_dev_stop(ptv->port_id); - } + if ((ol_flags & RTE_MBUF_F_RX_L4_CKSUM_MASK) == RTE_MBUF_F_RX_L4_CKSUM_BAD) { + SCLogDebug("HW detected BAD L4 chsum"); + p->level4_comp_csum = 0; } - DPDKDumpCounters(ptv); - break; + } + } + + return p; +} + +static inline void DPDKSegmentedMbufWarning(struct rte_mbuf *mbuf) +{ + static bool segmented_mbufs_warned = false; + if (!segmented_mbufs_warned && !rte_pktmbuf_is_contiguous(mbuf)) { + char warn_s[] = "Segmented mbufs detected! Redmine Ticket #6012 " + "Check your configuration or report the issue"; + enum rte_proc_type_t eal_t = rte_eal_process_type(); + if (eal_t == RTE_PROC_SECONDARY) { + SCLogWarning("%s. To avoid segmented mbufs, " + "try to increase mbuf size in your primary application", + warn_s); + } else if (eal_t == RTE_PROC_PRIMARY) { + SCLogWarning("%s. To avoid segmented mbufs, " + "try to increase MTU in your suricata.yaml", + warn_s); } - nb_rx = rte_eth_rx_burst(ptv->port_id, ptv->queue_id, ptv->received_mbufs, BURST_SIZE); - if (unlikely(nb_rx == 0)) { - t = DPDKSetTimevalReal(&machine_start_time); - uint64_t msecs = SCTIME_MSECS(t); - if (msecs > last_timeout_msec + 100) { - TmThreadsCaptureHandleTimeout(tv, NULL); - last_timeout_msec = msecs; - } + segmented_mbufs_warned = true; + } +} - if (!ptv->intr_enabled) - continue; +static void HandleShutdown(DPDKThreadVars *ptv) +{ + SCLogDebug("Stopping Suricata!"); + SC_ATOMIC_ADD(ptv->workers_sync->worker_checked_in, 1); + while (SC_ATOMIC_GET(ptv->workers_sync->worker_checked_in) < ptv->workers_sync->worker_cnt) { + rte_delay_us(10); + } + if (ptv->queue_id == 0) { + rte_delay_us(20); // wait for all threads to get out of the sync loop + SC_ATOMIC_SET(ptv->workers_sync->worker_checked_in, 0); + // If Suricata runs in peered mode, the peer threads might still want to send + // packets to our port. Instead, we know, that we are done with the peered port, so + // we stop it. The peered threads will stop our port. + if (ptv->copy_mode == DPDK_COPY_MODE_TAP || ptv->copy_mode == DPDK_COPY_MODE_IPS) { + rte_eth_dev_stop(ptv->out_port_id); + } else { + // in IDS we stop our port - no peer threads are running + rte_eth_dev_stop(ptv->port_id); + } + } + DPDKDumpCounters(ptv); +} - pwd_zero_rx_packet_polls_count++; - if (pwd_zero_rx_packet_polls_count <= MIN_ZERO_POLL_COUNT) - continue; +static void PeriodicDPDKDumpCounters(DPDKThreadVars *ptv) +{ + static time_t last_dump = 0; + time_t current_time = DPDKGetSeconds(); + /* Trigger one dump of stats every second */ + if (current_time != last_dump) { + DPDKDumpCounters(ptv); + last_dump = current_time; + } +} - uint32_t pwd_idle_hint = InterruptsSleepHeuristic(pwd_zero_rx_packet_polls_count); +/** + * \brief Main DPDK reading Loop function + */ +static TmEcode ReceiveDPDKLoop(ThreadVars *tv, void *data, void *slot) +{ + SCEnter(); + DPDKThreadVars *ptv = (DPDKThreadVars *)data; + ptv->slot = (TmSlot *)slot; + TmEcode ret = ReceiveDPDKLoopInit(tv, ptv); + if (ret != TM_ECODE_OK) { + SCReturnInt(ret); + } + while (true) { + if (unlikely(suricata_ctl_flags != 0)) { + HandleShutdown(ptv); + break; + } - if (pwd_idle_hint < STANDARD_SLEEP_TIME_US) { - rte_delay_us(pwd_idle_hint); - } else { - InterruptsTurnOnOff(ptv->port_id, ptv->queue_id, true); - struct rte_epoll_event event; - rte_epoll_wait(RTE_EPOLL_PER_THREAD, &event, 1, MAX_EPOLL_TIMEOUT_MS); - InterruptsTurnOnOff(ptv->port_id, ptv->queue_id, false); - continue; - } - } else if (ptv->intr_enabled && pwd_zero_rx_packet_polls_count) { - pwd_zero_rx_packet_polls_count = 0; + uint16_t nb_rx = + rte_eth_rx_burst(ptv->port_id, ptv->queue_id, ptv->received_mbufs, BURST_SIZE); + if (RXPacketCountHeuristic(tv, ptv, nb_rx)) { + continue; } ptv->pkts += (uint64_t)nb_rx; for (uint16_t i = 0; i < nb_rx; i++) { - p = PacketGetFromQueueOrAlloc(); - if (unlikely(p == NULL)) { + Packet *p = PacketInitFromMbuf(ptv, ptv->received_mbufs[i]); + if (p == NULL) { + rte_pktmbuf_free(ptv->received_mbufs[i]); continue; } - PKT_SET_SRC(p, PKT_SRC_WIRE); - p->datalink = LINKTYPE_ETHERNET; - if (ptv->checksum_mode == CHECKSUM_VALIDATION_DISABLE) { - p->flags |= PKT_IGNORE_CHECKSUM; - } - - p->ts = DPDKSetTimevalReal(&machine_start_time); - p->dpdk_v.mbuf = ptv->received_mbufs[i]; - p->ReleasePacket = DPDKReleasePacket; - p->dpdk_v.copy_mode = ptv->copy_mode; - p->dpdk_v.out_port_id = ptv->out_port_id; - p->dpdk_v.out_queue_id = ptv->queue_id; - p->livedev = ptv->livedev; - - if (ptv->checksum_mode == CHECKSUM_VALIDATION_DISABLE) { - p->flags |= PKT_IGNORE_CHECKSUM; - } else if (ptv->checksum_mode == CHECKSUM_VALIDATION_OFFLOAD) { - uint64_t ol_flags = ptv->received_mbufs[i]->ol_flags; - if ((ol_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) == RTE_MBUF_F_RX_IP_CKSUM_GOOD && - (ol_flags & RTE_MBUF_F_RX_L4_CKSUM_MASK) == RTE_MBUF_F_RX_L4_CKSUM_GOOD) { - SCLogDebug("HW detected GOOD IP and L4 chsum, ignoring validation"); - p->flags |= PKT_IGNORE_CHECKSUM; - } else { - if ((ol_flags & RTE_MBUF_F_RX_IP_CKSUM_MASK) == RTE_MBUF_F_RX_IP_CKSUM_BAD) { - SCLogDebug("HW detected BAD IP checksum"); - // chsum recalc will not be triggered but rule keyword check will be - p->level3_comp_csum = 0; - } - if ((ol_flags & RTE_MBUF_F_RX_L4_CKSUM_MASK) == RTE_MBUF_F_RX_L4_CKSUM_BAD) { - SCLogDebug("HW detected BAD L4 chsum"); - p->level4_comp_csum = 0; - } - } - } - - if (!rte_pktmbuf_is_contiguous(p->dpdk_v.mbuf) && !segmented_mbufs_warned) { - char warn_s[] = "Segmented mbufs detected! Redmine Ticket #6012 " - "Check your configuration or report the issue"; - enum rte_proc_type_t eal_t = rte_eal_process_type(); - if (eal_t == RTE_PROC_SECONDARY) { - SCLogWarning("%s. To avoid segmented mbufs, " - "try to increase mbuf size in your primary application", - warn_s); - } else if (eal_t == RTE_PROC_PRIMARY) { - SCLogWarning("%s. To avoid segmented mbufs, " - "try to increase MTU in your suricata.yaml", - warn_s); - } - - segmented_mbufs_warned = 1; - } - + DPDKSegmentedMbufWarning(ptv->received_mbufs[i]); PacketSetData(p, rte_pktmbuf_mtod(p->dpdk_v.mbuf, uint8_t *), rte_pktmbuf_pkt_len(p->dpdk_v.mbuf)); if (TmThreadsSlotProcessPkt(ptv->tv, ptv->slot, p) != TM_ECODE_OK) { @@ -548,12 +602,7 @@ static TmEcode ReceiveDPDKLoop(ThreadVars *tv, void *data, void *slot) } } - /* Trigger one dump of stats every second */ - current_time = DPDKGetSeconds(); - if (current_time != last_dump) { - DPDKDumpCounters(ptv); - last_dump = current_time; - } + PeriodicDPDKDumpCounters(ptv); StatsSyncCountersIfSignalled(tv); } From 16c88f2db74d43fa9e3eb82a70fe551dc4870a5c Mon Sep 17 00:00:00 2001 From: Lukas Sismis Date: Thu, 14 Mar 2024 12:49:14 +0100 Subject: [PATCH 22/26] dpdk: fix typo in the struct member name --- src/runmode-dpdk.c | 2 +- src/source-dpdk.c | 4 ++-- src/source-dpdk.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/runmode-dpdk.c b/src/runmode-dpdk.c index 33fc53a7ddec..a06bb08a03cc 100644 --- a/src/runmode-dpdk.c +++ b/src/runmode-dpdk.c @@ -1591,7 +1591,7 @@ static void *ParseDpdkConfigAndConfigureDevice(const char *iface) (void)SC_ATOMIC_ADD(iconf->ref, iconf->threads); // This counter is increased by worker threads that individually pick queue IDs. SC_ATOMIC_RESET(iconf->queue_id); - SC_ATOMIC_RESET(iconf->inconsitent_numa_cnt); + SC_ATOMIC_RESET(iconf->inconsistent_numa_cnt); iconf->workers_sync = SCCalloc(1, sizeof(*iconf->workers_sync)); if (iconf->workers_sync == NULL) { FatalError("Failed to allocate memory for workers_sync"); diff --git a/src/source-dpdk.c b/src/source-dpdk.c index 69e13bc1640f..84a860f3fdc7 100644 --- a/src/source-dpdk.c +++ b/src/source-dpdk.c @@ -662,7 +662,7 @@ static TmEcode ReceiveDPDKThreadInit(ThreadVars *tv, const void *initdata, void thread_numa = GetNumaNode(); if (thread_numa >= 0 && ptv->port_socket_id != SOCKET_ID_ANY && thread_numa != ptv->port_socket_id) { - SC_ATOMIC_ADD(dpdk_config->inconsitent_numa_cnt, 1); + SC_ATOMIC_ADD(dpdk_config->inconsistent_numa_cnt, 1); SCLogPerf("%s: NIC is on NUMA %d, thread on NUMA %d", dpdk_config->iface, ptv->port_socket_id, thread_numa); } @@ -691,7 +691,7 @@ static TmEcode ReceiveDPDKThreadInit(ThreadVars *tv, const void *initdata, void // some PMDs requires additional actions only after the device has started DevicePostStartPMDSpecificActions(ptv, dev_info.driver_name); - uint16_t inconsistent_numa_cnt = SC_ATOMIC_GET(dpdk_config->inconsitent_numa_cnt); + uint16_t inconsistent_numa_cnt = SC_ATOMIC_GET(dpdk_config->inconsistent_numa_cnt); if (inconsistent_numa_cnt > 0 && ptv->port_socket_id != SOCKET_ID_ANY) { SCLogWarning("%s: NIC is on NUMA %d, %u threads on different NUMA node(s)", dpdk_config->iface, ptv->port_socket_id, inconsistent_numa_cnt); diff --git a/src/source-dpdk.h b/src/source-dpdk.h index dce791c14cec..36617134d6fd 100644 --- a/src/source-dpdk.h +++ b/src/source-dpdk.h @@ -76,7 +76,7 @@ typedef struct DPDKIfaceConfig_ { SC_ATOMIC_DECLARE(unsigned int, ref); /* threads bind queue id one by one */ SC_ATOMIC_DECLARE(uint16_t, queue_id); - SC_ATOMIC_DECLARE(uint16_t, inconsitent_numa_cnt); + SC_ATOMIC_DECLARE(uint16_t, inconsistent_numa_cnt); DPDKWorkerSync *workers_sync; void (*DerefFunc)(void *); From 632e52ca2b72f8a3462b22896cb5d25d9d724f9c Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 14 Mar 2024 09:00:15 +0100 Subject: [PATCH 23/26] ci: update ubuntu22.04 builds with clang14+asan using a workround about ASLR --- .github/workflows/builds.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 21d3531f79cf..3f3fd1241ee7 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -1471,7 +1471,9 @@ jobs: ubuntu-22-04-cov-fuzz: name: Ubuntu 22.04 (fuzz corpus coverage) runs-on: ubuntu-latest - container: ubuntu:22.04 + container: + image: ubuntu:22.04 + options: --privileged needs: [prepare-deps, prepare-cbindgen] steps: - name: Cache ~/.cargo @@ -1519,6 +1521,7 @@ jobs: parallel \ python3-yaml \ software-properties-common \ + sudo \ zlib1g \ zlib1g-dev \ exuberant-ctags \ @@ -1546,6 +1549,11 @@ jobs: cp prep/cbindgen $HOME/.cargo/bin chmod 755 $HOME/.cargo/bin/cbindgen echo "$HOME/.cargo/bin" >> $GITHUB_PATH + - name: Fix kernel mmap rnd bits + # Asan in llvm 14 provided in ubuntu 22.04 is incompatible with + # high-entropy ASLR in much newer kernels that GitHub runners are + # using leading to random crashes: https://github.com/actions/runner-images/issues/9491 + run: sudo sysctl vm.mmap_rnd_bits=28 - run: ./autogen.sh - run: ./configure --with-gnu-ld --enable-fuzztargets --disable-shared --enable-gccprotect env: @@ -1729,7 +1737,9 @@ jobs: ubuntu-22-04-debug-validation: name: Ubuntu 22.04 (Debug Validation) runs-on: ubuntu-22.04 - container: ubuntu:22.04 + container: + image: ubuntu:22.04 + options: --privileged needs: [prepare-deps, prepare-cbindgen] steps: @@ -1776,6 +1786,7 @@ jobs: python3-yaml \ rustc \ software-properties-common \ + sudo \ zlib1g \ zlib1g-dev \ exuberant-ctags @@ -1795,6 +1806,11 @@ jobs: cp prep/cbindgen $HOME/.cargo/bin chmod 755 $HOME/.cargo/bin/cbindgen echo "$HOME/.cargo/bin" >> $GITHUB_PATH + - name: Fix kernel mmap rnd bits + # Asan in llvm 14 provided in ubuntu 22.04 is incompatible with + # high-entropy ASLR in much newer kernels that GitHub runners are + # using leading to random crashes: https://github.com/actions/runner-images/issues/9491 + run: sudo sysctl vm.mmap_rnd_bits=28 - run: ./autogen.sh - run: ./configure --enable-debug-validation env: From 9ad73faa0a52428e47412474514b125fda6aa03d Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 11 Mar 2024 14:57:16 -0400 Subject: [PATCH 24/26] flow/inject: Ensure initialized thread value used Issue: 6835 When injecting a flow, ensure that the selected thread_id has been initialized. When a flow is picked up midstream, the initialized thread can be the second thread element. --- src/flow-timeout.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/flow-timeout.c b/src/flow-timeout.c index 546fc35fe93d..92632e68f619 100644 --- a/src/flow-timeout.c +++ b/src/flow-timeout.c @@ -334,14 +334,21 @@ int FlowForceReassemblyNeedReassembly(Flow *f) * * The function requires flow to be locked beforehand. * + * Normally, the first thread_id value should be used. This is when the flow is + * created on seeing the first packet to the server; sometimes, if the first + * packet is determined to be to the client, the second thread_id value should + * be used. + * * \param f Pointer to the flow. * * \retval 0 This flow doesn't need any reassembly processing; 1 otherwise. */ void FlowForceReassemblyForFlow(Flow *f) { - const int thread_id = (int)f->thread_id[0]; - TmThreadsInjectFlowById(f, thread_id); + // Have packets traveled to the server? If not, + // use the reverse direction + int idx = f->todstpktcnt > 0 ? 0 : 1; + TmThreadsInjectFlowById(f, (const int)f->thread_id[idx]); } /** From 3c5745978f85f4bf049e2892c8bda167f9e53033 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 11 Mar 2024 14:58:07 -0400 Subject: [PATCH 25/26] flow: Swap thread_ids Issue: 6835 When swapping the flow's direction, also swap the thread_ids. This should help with the issues identified in https://redmine.openinfosecfoundation.org/issues/2725 --- src/flow.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/flow.c b/src/flow.c index a7f4788150ed..b326a0d0fadb 100644 --- a/src/flow.c +++ b/src/flow.c @@ -291,6 +291,8 @@ void FlowSwap(Flow *f) FlowSwapFlags(f); FlowSwapFileFlags(f); + SWAP_VARS(FlowThreadId, f->thread_id[0], f->thread_id[1]); + if (f->proto == IPPROTO_TCP) { TcpStreamFlowSwap(f); } From e41c2f15c29ceb96dc2545d3acc703e217628fbe Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 11 Mar 2024 14:59:38 -0400 Subject: [PATCH 26/26] gen/typo: Correct comment typo --- src/flow-timeout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/flow-timeout.c b/src/flow-timeout.c index 92632e68f619..0b75228ca375 100644 --- a/src/flow-timeout.c +++ b/src/flow-timeout.c @@ -362,7 +362,7 @@ void FlowForceReassemblyForFlow(Flow *f) * - silence complaining profilers * - allow us to aggressively check using debug validation assertions * - be robust in case of future changes - * - locking overhead if neglectable when no other thread fights us + * - locking overhead is negligible when no other thread fights us * * \param q The queue to process flows from. */