-
Notifications
You must be signed in to change notification settings - Fork 33
Use the latest zmq_msg_hdr_v3 from ntop_typedefs.h #111
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 7 7
Lines 826 842 +16
=====================================
- Misses 826 842 +16
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
@synfinatic : No hurry on this. I want to test it more, but wanted to get eyes on it too. One more thing we may want to do is make TLV the default instead of JSON. When testing this, I had to comment out this code in ntopng:
That code was added in commit |
80160c7
to
f92ba71
Compare
I've rebased and will test this soon, then turn this back in to a real PR. |
Not sure if this is related to this PR, but seeing an issue where traffic is no longer being processed. ie:
the first discarding messages are expected at startup, but the latter ones seem to be a bug? |
Definitely feels like a bug, unless the router has diff templates for ipv4 and ipv6? |
I can't repro this, so I'm going to try and put better logging around it, my theory is the router sends multiple templates and is getting packets for ipv4 or ipv6 before the respective template has been received. I don't have ipv6 working right now, so I haven't proved this yet. |
@kellybyrd do you have some improved logging code you want to share? happy to try on my end where i run both v4 & v6 |
This will log the error message from the template system, which shows the template number. It also removes some other logs from Trace level to make it easier to see the "TemplateNotFound" errors. If the issue is just ipv6 packets arriving before ipv6 template, I expect that after the `Sending first ZMQ message." you will see TemplateNotFound for only a single template id.
|
So....I can't repro this. I did this:
I did see that the check for first send happens outside the mutex, so maybe that's the cause? I'll push an update |
Update the ZMQ header to the latest version in as of ntop 6.4. The goal here is to just use the latest thing so we have longer before we have to worry about being deprecated. The work here was: * New header with new version constant * New header requires knowing compress/uncompressed sizes so move the compress code from Format to Transport. We're still only compressing JSON. TESTING DONE: Tested TLV, JSON, and compressed JSON with ntopng 6.4 built from source. Saw flows with all three. In order to get 6.4 to work with compressed JSON, I had to fix a bug in ntopng as well as disable the pro license checks. Without this fix, I cannot see how ntopng decompresses anything that has zmq_message_header_v3. I confirmed with them this is fixed in their dev branch, but broken in v6.4
f92ba71
to
ef23664
Compare
The latest code in the branch has the "Sending first ZMQ message" inside the mutex and also has some TRACE logs for the missing template case that should contain the template id that is missing. |
NOTE: This doesn't fix anything that is broken. It is just an attempt to get ahead of ntopng deprecating old probe formats so hopefully we don't to react to new versions of ntopng as quickly. Let me know if you'd rather not merge this
Update the ZMQ header to the latest version in as of ntop 6.4. The goal here is to just use the latest thing so we have longer before we have to worry about being deprecated.
The work here was:
New header with new version constant
New header requires knowing compress/uncompressed sizes so move the compress code from Format to Transport. We're still only compressing JSON.
TESTING DONE:
Tested TLV, JSON, and compressed JSON with ntopng 6.4 built from source. Saw flows with all three. In order to get 6.4 to work with compressed JSON, I had to fix a bug in ntopng as well as disable the pro license checks. Without my ntopng fix, I cannot see how ntopng decompresses anything that has zmq_message_header_v3.
I have opened an issue on the ntopng repo they confirmed it is fixed in their dev branch.