Feature 2694 thresholds v6 #4760
Conversation
Also add tests for parsing them.
Make it easy to compare 'struct timeval's and get their difference.
…d by_both. The only difference between threshold calculations for by_src/by_dst, by_rule or by_both is which table stores the DetectThresholdEntry. Refactor the ThresholdHandlePacket* functions to do table lookup and storage individually, but calculate thresholds in a common function.
…hreshold table. Ensure that the by_rule threshold table is initialized if a rule is thresholded by_rule. Replace manual table reallocaton with calls to the common function.
|
|
||
| void *ptmp = SCRealloc(de_ctx->ths_ctx.th_entry, num * sizeof(DetectThresholdEntry *)); | ||
| if (ptmp == NULL) { | ||
| SCLogWarning(SC_ERR_MEM_ALLOC, "Error allocating memory for rule thresholds" |
There was a problem hiding this comment.
Please use a value prefixed with SC_WARN_ .... These are in util-error.[hc]
There was a problem hiding this comment.
I actually went looking for an appropriate SC_WARN to use here, but none of them seem to apply to something like a memory allocation failure. The only ones I can find that apply to memory allocation problems are SC_ERR, and there are examples of SCLogWarning(SC_ERR_MEM_*) in conf.c, log-pcap.c, log-tlsstore.c, and util-buffer.c, so I thought this was fine.
Would you like a new SC_WARN_MEM_ALLOC in util-error.h?
There was a problem hiding this comment.
No need for that. We're considering switching to a much simpler scheme soon too.
There was a problem hiding this comment.
No ... your pr looks good to me as is.
We'll address the discrepancy in a different issue.
|
|
||
| void *ptmp = SCRealloc(de_ctx->ths_ctx.th_entry, num * sizeof(DetectThresholdEntry *)); | ||
| if (ptmp == NULL) { | ||
| SCLogWarning(SC_ERR_MEM_ALLOC, "Error allocating memory for rule thresholds" |
There was a problem hiding this comment.
No ... your pr looks good to me as is.
We'll address the discrepancy in a different issue.
|
Running through QA. |
|
@mordak are there associated Suricata-Verify tests? |
Sorry, no. I didn't know that was a thing until just now. |
|
Merged in #4785, thanks Todd! |
If you're interested in doing a few tests, it would be great. The existing tests can serve as examples. |
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/2694
Describe changes:
Continuation of #4747