Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thresholds/v20 #11358

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
966dd63
util/var: remove printf; add assert
victorjulien Mar 1, 2024
feda2d7
util/var: add comments explaining types
victorjulien Mar 4, 2024
6e82c82
detect/threshold: implement tracking 'by_flow'
victorjulien Feb 27, 2024
f15cf52
threshold: add by_flow support for global thresholds
victorjulien Mar 2, 2024
deb16f9
detect: group types used in traffic variables
victorjulien Mar 4, 2024
b0944c6
detect: group content inspect keyword id's
victorjulien Mar 5, 2024
fc7f1e9
detect/content: fix wrong value for depth check
victorjulien Mar 5, 2024
3567f9e
doc: add thresholding by_flow
victorjulien Mar 8, 2024
06ff741
detect/detection_filter: add support for track by_flow
victorjulien Mar 13, 2024
783d735
detect: add ticket id to var related todos
victorjulien Mar 13, 2024
6bc0795
detect/threshold: implement per thread cache
victorjulien Sep 11, 2023
75ea5fc
detect/threshold: minor cleanup
victorjulien Jan 9, 2024
da3439f
detect/threshold: minor code cleanup
victorjulien Jan 9, 2024
6d17006
detect/threshold: minor rate filter cleanup
victorjulien Jan 9, 2024
21a4cfc
detect/address: constify ipv6 cmp funcs
victorjulien Jan 9, 2024
58a116c
thash: add expiration logic
victorjulien Jan 10, 2024
f83901c
range: use thash expiry API for timeout
victorjulien Jan 9, 2024
f3210fc
thresholds: use dedicated storage
victorjulien Jan 9, 2024
1432add
detect/threshold: improve hash function
victorjulien Apr 19, 2024
cccf858
detect/threshold: include rev in threshold tracking
victorjulien Apr 19, 2024
44dcc92
detect/threshold: consider tenant id in tracking
victorjulien Apr 19, 2024
11cf1a9
detect/threshold: expand cache support for rule tracking
victorjulien Apr 19, 2024
470a37c
detect/threshold: includes cleanup
victorjulien Apr 20, 2024
1fb11d0
detect/threshold: make hash size and memcap configurable
victorjulien May 15, 2024
dac619d
doc/userguide: document new threshold config options
victorjulien May 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 226 additions & 0 deletions src/detect-engine-threshold.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,212 @@ int ThresholdIPPairHasThreshold(IPPair *pair)
return IPPairGetStorageById(pair, ippair_threshold_id) ? 1 : 0;
}

#include "util-hash.h"

typedef struct ThresholdCacheItem {
int8_t track; // by_src/by_dst
int8_t ipv;
int8_t retval;
uint32_t addr;
uint32_t sid;
SCTime_t expires_at;
RB_ENTRY(ThresholdCacheItem) rb;
} ThresholdCacheItem;

static thread_local HashTable *threshold_cache_ht = NULL;

thread_local uint64_t cache_lookup_cnt = 0;
thread_local uint64_t cache_lookup_notinit = 0;
thread_local uint64_t cache_lookup_nosupport = 0;
thread_local uint64_t cache_lookup_miss_expired = 0;
thread_local uint64_t cache_lookup_miss = 0;
thread_local uint64_t cache_lookup_hit = 0;
thread_local uint64_t cache_housekeeping_check = 0;
thread_local uint64_t cache_housekeeping_expired = 0;

static void DumpCacheStats(void)
{
SCLogPerf("threshold thread cache stats: cnt:%" PRIu64 " notinit:%" PRIu64 " nosupport:%" PRIu64
" miss_expired:%" PRIu64 " miss:%" PRIu64 " hit:%" PRIu64
", housekeeping: checks:%" PRIu64 ", expired:%" PRIu64,
cache_lookup_cnt, cache_lookup_notinit, cache_lookup_nosupport,
cache_lookup_miss_expired, cache_lookup_miss, cache_lookup_hit,
cache_housekeeping_check, cache_housekeeping_expired);
}

/* rbtree for expiry handling */

static int ThresholdCacheTreeCompareFunc(ThresholdCacheItem *a, ThresholdCacheItem *b)
{
if (SCTIME_CMP_GTE(a->expires_at, b->expires_at)) {
return 1;
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unneeded else

return -1;
}
}

RB_HEAD(THRESHOLD_CACHE, ThresholdCacheItem);
RB_PROTOTYPE(THRESHOLD_CACHE, ThresholdCacheItem, rb, ThresholdCacheTreeCompareFunc);
RB_GENERATE(THRESHOLD_CACHE, ThresholdCacheItem, rb, ThresholdCacheTreeCompareFunc);
thread_local struct THRESHOLD_CACHE threshold_cache_tree;
thread_local uint64_t threshold_cache_housekeeping_ts = 0;

static void ThresholdCacheExpire(SCTime_t now)
{
ThresholdCacheItem *iter, *safe = NULL;
int cnt = 0;
threshold_cache_housekeeping_ts = SCTIME_SECS(now);

RB_FOREACH_SAFE (iter, THRESHOLD_CACHE, &threshold_cache_tree, safe) {
cache_housekeeping_check++;

if (SCTIME_CMP_LT(iter->expires_at, now)) {
THRESHOLD_CACHE_RB_REMOVE(&threshold_cache_tree, iter);
HashTableRemove(threshold_cache_ht, iter, 0);
SCLogDebug("iter %p expired", iter);
cache_housekeeping_expired++;
}

if (++cnt > 1)
break;
}
}

/* hash table for threshold look ups */

static uint32_t ThresholdCacheHashFunc(HashTable *ht, void *data, uint16_t datalen)
{
ThresholdCacheItem *tci = data;
int hash = tci->ipv * tci->track + tci->addr + tci->sid;
hash = hash % ht->array_size;
return hash;
}

static char ThresholdCacheHashCompareFunc(
void *data1, uint16_t datalen1, void *data2, uint16_t datalen2)
{
ThresholdCacheItem *tci1 = data1;
ThresholdCacheItem *tci2 = data2;
return tci1->ipv == tci2->ipv && tci1->track == tci2->track && tci1->addr == tci2->addr &&
tci1->sid == tci2->sid;
}

static void ThresholdCacheHashFreeFunc(void *data)
{
SCFree(data);
}

/// \brief Thread local cache
static int SetupCache(const Packet *p, const int8_t track, const int8_t retval, const uint32_t sid,
SCTime_t expires)
{
if (!threshold_cache_ht) {
threshold_cache_ht = HashTableInit(256, ThresholdCacheHashFunc,
ThresholdCacheHashCompareFunc, ThresholdCacheHashFreeFunc);
}

uint32_t addr;
if (track == TRACK_SRC) {
addr = p->src.addr_data32[0];
} else if (track == TRACK_DST) {
addr = p->dst.addr_data32[0];
} else {
return -1;
}

Comment on lines +339 to +346
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPv6?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, implementation is IPv4 only right now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ticket required? Or implement before merge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? It's an optimization, nothing is required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moreso for completeness. It is not discussed in any of the commits, or the tickets referenced by this PR that this is an IPv4 only optimization, so the lack of IPv6 looks like an oversight.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call sites all use PacketIsIPv4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least to me that means where is the IPv6 :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can create a ticket if you want

ThresholdCacheItem lookup = {
.track = track,
.ipv = 4,
.retval = retval,
.addr = addr,
.sid = sid,
.expires_at = expires,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Do we need expires_at and retval here as they're not used for lookup?

};
ThresholdCacheItem *found = HashTableLookup(threshold_cache_ht, &lookup, 0);
if (!found) {
ThresholdCacheItem *n = SCCalloc(1, sizeof(*n));
if (n) {
n->track = track;
n->ipv = 4;
n->retval = retval;
n->addr = addr;
n->sid = sid;
n->expires_at = expires;

if (HashTableAdd(threshold_cache_ht, n, 0) == 0) {
(void)THRESHOLD_CACHE_RB_INSERT(&threshold_cache_tree, n);
return 1;
}
SCFree(n);
}
return -1;
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unneeded else

found->expires_at = expires;
found->retval = retval;

THRESHOLD_CACHE_RB_REMOVE(&threshold_cache_tree, found);
THRESHOLD_CACHE_RB_INSERT(&threshold_cache_tree, found);
return 1;
}
}

/** \brief Check Thread local thresholding cache
* \retval negative value cache miss (-1 miss, -2 expired)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: retval values missing

*/
static int CheckCache(const Packet *p, const int8_t track, const uint32_t sid)
{
cache_lookup_cnt++;

if (!threshold_cache_ht) {
cache_lookup_notinit++;
return -3;
}

uint32_t addr;
if (track == TRACK_SRC) {
addr = p->src.addr_data32[0];
} else if (track == TRACK_DST) {
addr = p->dst.addr_data32[0];
} else {
cache_lookup_nosupport++;
return -4;
}

if (SCTIME_SECS(p->ts) > threshold_cache_housekeeping_ts) {
ThresholdCacheExpire(p->ts);
}

ThresholdCacheItem lookup = {
.track = track,
.ipv = 4,
.addr = addr,
.sid = sid,
};
Copy link
Member

@jasonish jasonish Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be vlan aware here? And elsewhere with respect to thresholding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created https://redmine.openinfosecfoundation.org/issues/7125 for this

It's an issue in existing code. I do not intend to address it in this work.

ThresholdCacheItem *found = HashTableLookup(threshold_cache_ht, &lookup, 0);
if (found) {
if (SCTIME_CMP_GT(p->ts, found->expires_at)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: what if it has just timed out? So the = condition

THRESHOLD_CACHE_RB_REMOVE(&threshold_cache_tree, found);
HashTableRemove(threshold_cache_ht, found, 0);
cache_lookup_miss_expired++;
return -2; // CACHE MISS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: also mention expired?

}
cache_lookup_hit++;
return found->retval;
}
cache_lookup_miss++;
return -1; // CACHE MISS
}

void ThresholdCacheThreadFree(void)
{
if (threshold_cache_ht) {
HashTableFree(threshold_cache_ht);
threshold_cache_ht = NULL;
}
RB_INIT(&threshold_cache_tree);
DumpCacheStats();
}

/**
* \brief Return next DetectThresholdData for signature
*
Expand Down Expand Up @@ -478,6 +684,10 @@ static int ThresholdHandlePacket(Packet *p, DetectThresholdEntry *lookup_tsh,
ret = 1;
} else {
ret = 2;

if (PacketIsIPv4(p)) {
SetupCache(p, td->track, (int8_t)ret, sid, entry);
}
}
} else {
lookup_tsh->tv1 = p->ts;
Expand Down Expand Up @@ -533,6 +743,10 @@ static int ThresholdHandlePacket(Packet *p, DetectThresholdEntry *lookup_tsh,
} else if (lookup_tsh->current_count > td->count) {
/* silent match */
ret = 2;

if (PacketIsIPv4(p)) {
SetupCache(p, td->track, (int8_t)ret, sid, entry);
}
}
} else {
/* expired, so reset */
Expand Down Expand Up @@ -701,12 +915,24 @@ int PacketAlertThreshold(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
if (td->type == TYPE_SUPPRESS) {
ret = ThresholdHandlePacketSuppress(p,td,s->id,s->gid);
} else if (td->track == TRACK_SRC) {
if (PacketIsIPv4(p) && (td->type == TYPE_LIMIT || td->type == TYPE_BOTH)) {
int cache_ret = CheckCache(p, td->track, s->id);
if (cache_ret >= 0) {
SCReturnInt(cache_ret);
}
}
Host *src = HostGetHostFromHash(&p->src);
if (src) {
ret = ThresholdHandlePacketHost(src,p,td,s->id,s->gid,pa);
HostRelease(src);
}
} else if (td->track == TRACK_DST) {
if (PacketIsIPv4(p) && (td->type == TYPE_LIMIT || td->type == TYPE_BOTH)) {
int cache_ret = CheckCache(p, td->track, s->id);
if (cache_ret >= 0) {
SCReturnInt(cache_ret);
}
}
Host *dst = HostGetHostFromHash(&p->dst);
if (dst) {
ret = ThresholdHandlePacketHost(dst,p,td,s->id,s->gid,pa);
Expand Down
1 change: 1 addition & 0 deletions src/detect-engine-threshold.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ void ThresholdContextDestroy(DetectEngineCtx *);
int ThresholdHostTimeoutCheck(Host *, SCTime_t);
int ThresholdIPPairTimeoutCheck(IPPair *, SCTime_t);
void ThresholdListFree(void *ptr);
void ThresholdCacheThreadFree(void);

void FlowThresholdVarFree(void *ptr);

Expand Down
2 changes: 2 additions & 0 deletions src/detect-engine.c
Original file line number Diff line number Diff line change
Expand Up @@ -3527,6 +3527,8 @@ static void DetectEngineThreadCtxFree(DetectEngineThreadCtx *det_ctx)
AppLayerDecoderEventsFreeEvents(&det_ctx->decoder_events);

SCFree(det_ctx);

ThresholdCacheThreadFree();
}

TmEcode DetectEngineThreadCtxDeinit(ThreadVars *tv, void *data)
Expand Down