Skip to content

Commit

Permalink
detect: Inline default pkt inspect engines
Browse files Browse the repository at this point in the history
Scenarios with a small number of rules, no MPM-based rules,
experienced a 6%-14% performance degradation from the commit
0965afd detect: pkt inspect engines
inwhich the default pkt inspect engines were converted to callbacks to
simplify adding extra pkt inspect engines.

Avoid adding the default pkt inspect engines to the callback chain and
instead call them directly in an inlined function within
DetectRulePacketRules().

Bug: OISF#6291
  • Loading branch information
coledishington committed Jan 19, 2024
1 parent 6896a93 commit bf3d9f8
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 52 deletions.
35 changes: 19 additions & 16 deletions src/detect-engine-analyzer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1053,25 +1053,28 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s)
}
jb_close(ctx.js);

const DetectEnginePktInspectionEngine *pkt_mpm = NULL;
SigMatchData *pkt_mpm_smd = NULL;
const DetectEngineAppInspectionEngine *app_mpm = NULL;

jb_open_array(ctx.js, "pkt_engines");
if (s->sm_arrays[DETECT_SM_LIST_PMATCH]) {
pkt_mpm_smd = s->sm_arrays[DETECT_SM_LIST_PMATCH];
jb_start_object(ctx.js);
jb_set_string(ctx.js, "name", "payload");
jb_set_bool(ctx.js, "is_mpm", s->init_data->mpm_sm_list == DETECT_SM_LIST_PMATCH);
jb_close(ctx.js);
}
if (s->sm_arrays[DETECT_SM_LIST_MATCH]) {
jb_start_object(ctx.js);
jb_set_string(ctx.js, "name", "packet");
jb_set_bool(ctx.js, "is_mpm", s->init_data->mpm_sm_list == DETECT_SM_LIST_MATCH);
jb_close(ctx.js);
}
const DetectEnginePktInspectionEngine *pkt = s->pkt_inspect;
for ( ; pkt != NULL; pkt = pkt->next) {
const char *name = DetectEngineBufferTypeGetNameById(de_ctx, pkt->sm_list);
if (name == NULL) {
switch (pkt->sm_list) {
case DETECT_SM_LIST_PMATCH:
name = "payload";
break;
case DETECT_SM_LIST_MATCH:
name = "packet";
break;
default:
name = "unknown";
break;
}
name = "unknown";
}
jb_start_object(ctx.js);
jb_set_string(ctx.js, "name", name);
Expand All @@ -1089,7 +1092,7 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s)
DumpMatches(&ctx, ctx.js, pkt->smd);
jb_close(ctx.js);
if (pkt->mpm) {
pkt_mpm = pkt;
pkt_mpm_smd = pkt->smd;
}
}
jb_close(ctx.js);
Expand Down Expand Up @@ -1185,18 +1188,18 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s)
}
jb_close(ctx.js);

if (pkt_mpm || app_mpm) {
if (pkt_mpm_smd || app_mpm) {
jb_open_object(ctx.js, "mpm");

int mpm_list = pkt_mpm ? DETECT_SM_LIST_PMATCH : app_mpm->sm_list;
int mpm_list = pkt_mpm_smd ? DETECT_SM_LIST_PMATCH : app_mpm->sm_list;
const char *name;
if (mpm_list < DETECT_SM_LIST_DYNAMIC_START)
name = DetectListToHumanString(mpm_list);
else
name = DetectEngineBufferTypeGetNameById(de_ctx, mpm_list);
jb_set_string(ctx.js, "buffer", name);

SigMatchData *smd = pkt_mpm ? pkt_mpm->smd : app_mpm->smd;
SigMatchData *smd = pkt_mpm_smd ? pkt_mpm_smd : app_mpm->smd;
if (smd == NULL && mpm_list == DETECT_SM_LIST_PMATCH) {
smd = s->sm_arrays[mpm_list];
}
Expand Down
2 changes: 0 additions & 2 deletions src/detect-engine-build.c
Original file line number Diff line number Diff line change
Expand Up @@ -1898,8 +1898,6 @@ static int SigMatchPrepare(DetectEngineCtx *de_ctx)
SigMatch *sm = s->init_data->smlists[type];
s->sm_arrays[type] = SigMatchList2DataArray(sm);
}
/* set up the pkt inspection engines */
DetectEnginePktInspectionSetup(s);

if (rule_engine_analysis_set) {
EngineAnalysisAddAllRulePatterns(de_ctx, s);
Expand Down
53 changes: 20 additions & 33 deletions src/detect-engine.c
Original file line number Diff line number Diff line change
Expand Up @@ -1905,11 +1905,9 @@ int DetectEngineBufferTypeGetByIdTransforms(
}

/* returns false if no match, true if match */
static int DetectEngineInspectRulePacketMatches(
DetectEngineThreadCtx *det_ctx,
const DetectEnginePktInspectionEngine *engine,
const Signature *s,
Packet *p, uint8_t *_alert_flags)
static inline int DetectEngineInspectRulePacketMatches(DetectEngineThreadCtx *det_ctx,
const DetectEnginePktInspectionEngine *engine, const Signature *s, Packet *p,
uint8_t *_alert_flags)
{
SCEnter();

Expand All @@ -1935,10 +1933,9 @@ static int DetectEngineInspectRulePacketMatches(
return DETECT_ENGINE_INSPECT_SIG_MATCH;
}

static int DetectEngineInspectRulePayloadMatches(
DetectEngineThreadCtx *det_ctx,
const DetectEnginePktInspectionEngine *engine,
const Signature *s, Packet *p, uint8_t *alert_flags)
static inline int DetectEngineInspectRulePayloadMatches(DetectEngineThreadCtx *det_ctx,
const DetectEnginePktInspectionEngine *engine, const Signature *s, Packet *p,
uint8_t *alert_flags)
{
SCEnter();

Expand Down Expand Up @@ -1981,13 +1978,23 @@ static int DetectEngineInspectRulePayloadMatches(
return DETECT_ENGINE_INSPECT_SIG_MATCH;
}

bool DetectEnginePktInspectionRun(ThreadVars *tv,
DetectEngineThreadCtx *det_ctx, const Signature *s,
Flow *f, Packet *p,
uint8_t *alert_flags)
inline bool DetectEnginePktInspectionRun(ThreadVars *tv, DetectEngineThreadCtx *det_ctx,
const Signature *s, Flow *f, Packet *p, uint8_t *alert_flags)
{
SCEnter();

if (s->sm_arrays[DETECT_SM_LIST_PMATCH]) {
if (DetectEngineInspectRulePayloadMatches(det_ctx, NULL, s, p, alert_flags) == false) {
return false;
}
}

if (s->sm_arrays[DETECT_SM_LIST_MATCH]) {
if (DetectEngineInspectRulePacketMatches(det_ctx, NULL, s, p, alert_flags) == false) {
return false;
}
}

for (DetectEnginePktInspectionEngine *e = s->pkt_inspect; e != NULL; e = e->next) {
if (e->v1.Callback(det_ctx, e, s, p, alert_flags) != DETECT_ENGINE_INSPECT_SIG_MATCH) {
SCLogDebug("sid %u: e %p Callback returned no match", s->id, e);
Expand Down Expand Up @@ -2029,26 +2036,6 @@ static int DetectEnginePktInspectionAppend(Signature *s, InspectionBufferPktInsp
return 0;
}

int DetectEnginePktInspectionSetup(Signature *s)
{
/* only handle PMATCH here if we're not an app inspect rule */
if (s->sm_arrays[DETECT_SM_LIST_PMATCH] && (s->init_data->init_flags & SIG_FLAG_INIT_STATE_MATCH) == 0) {
if (DetectEnginePktInspectionAppend(
s, DetectEngineInspectRulePayloadMatches, NULL, DETECT_SM_LIST_PMATCH) < 0)
return -1;
SCLogDebug("sid %u: DetectEngineInspectRulePayloadMatches appended", s->id);
}

if (s->sm_arrays[DETECT_SM_LIST_MATCH]) {
if (DetectEnginePktInspectionAppend(
s, DetectEngineInspectRulePacketMatches, NULL, DETECT_SM_LIST_MATCH) < 0)
return -1;
SCLogDebug("sid %u: DetectEngineInspectRulePacketMatches appended", s->id);
}

return 0;
}

/* code to control the main thread to do a reload */

enum DetectEngineSyncState {
Expand Down
1 change: 0 additions & 1 deletion src/detect-engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ bool DetectEnginePktInspectionRun(ThreadVars *tv,
DetectEngineThreadCtx *det_ctx, const Signature *s,
Flow *f, Packet *p,
uint8_t *alert_flags);
int DetectEnginePktInspectionSetup(Signature *s);

void DetectEngineSetParseMetadata(void);
void DetectEngineUnsetParseMetadata(void);
Expand Down

0 comments on commit bf3d9f8

Please sign in to comment.