-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[draft] flow callbacks for ndpi - v1 #11935
Conversation
This plugin stub shows how a plugin like nDPI might be use the flow init and flow update callbacks to do its work. Also shows usage of FlowStorage to avoid modifying the Flow struct directly.
5520c15
to
fe62c1c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11935 +/- ##
==========================================
- Coverage 82.74% 82.73% -0.01%
==========================================
Files 912 913 +1
Lines 249102 249136 +34
==========================================
+ Hits 206117 206130 +13
- Misses 42985 43006 +21
Flags with carried forward coverage won't be shown. Click here to find out more. |
SCFree(ptr); | ||
} | ||
|
||
static void OnFlowInit(Flow *f, const Packet *p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'd expect a OnFlowDestruct too, although perhaps we don't need it directly here, as the FlowStorage API knowns how to destroy the user data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, for completeness should probably add it. Not needed for storage cleanup using the storage API, and someone else uses the flow event for their private cleanup, but it does feel missing.
FlowSetStorageById(f, flow_storage_id, dummy_storage); | ||
} | ||
|
||
static void OnFlowUpdate(Flow *f, Packet *p, ThreadVars *tv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should there be user data here as well? general user data? Perhaps not needed for ndpi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess more for the library, but if someone wanted to create a simple flow logger, they'd want to pass a file pointer around these callbacks to log a new flow creation, or a flow destruction, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should have user data, was being short sighted and only looking at the case at hand which doesn't need it.
Replaced by #11942, all callbacks accept user data now. |
In order for nDPI to become a plugin it needs to hook into flow initialization and flow updates. This PR provides user registerable flow callbacks for these events.
It also includes a "dummy" ndpi plugin using these callbacks, and also shows how to use the flow storage API to avoid modifying the Flow structure directly.
Original nDPI PR: #11671
Tickets: