-
-
Notifications
You must be signed in to change notification settings - Fork 380
Features/dropqueuedpackets #882
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
Features/dropqueuedpackets #882
Conversation
Remove config parser/lexer code as it's rebuilded every time but can break adding new config options. Also clean up the code base to avoid mixing actual code changes and lint issues. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
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.
Thank you for the nice code and some code cleanup, that implements the timestamping feature! The code looks nice, I have but a few smaller suggestions.
Is it okay if I put the banner over the new files util/timeval_func.c and util/timeval_func.h, that contain the moved timeval functions from other places in the source code? I mean to put the license text on the file, like we have for other files. And the Copyright statement, for NLnet Labs above the file as well, like for other files. That makes it similar to the other code we have. NLnet Labs wants to have the copyright and also the license on the files to maintain the project. I think it is nice to clean up the code with them.
Yep, sure, you can put statements, there is no new code in these files, it's just copy-paste and cleanup. |
There are several definitions of the same functions manipulating timeval structures. Let's move them to separate file and arrange the code preperly. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Add special field in comm_point to store the software receive timestamp for every particular UDP packet. Aux data parser is updated to read values and the whole callback is switched to use recvmsg form. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Add sock_queue_timeout config option to have queue timeout configurable. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Add counter `num_queries_timed_out` meaning queries that were sitting in the socket queue and waiting to being processed too long. There is no reason to process such queries, so let's drop it in the very beginning of the pipeline. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Add new statistic value to know the size of the queue in microseconds. Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
73cd457
to
263096d
Compare
With the adjustments, I'll approve and merge this. Thank you for the additional feature and cleanup! After that, I will likely make a couple of smaller changes; spelling error, documentation mentions, and Changelog entry, to adjust the code a little. Thanks for the header license change, that is what I wanted. |
util/timeval_func.c and util/timeval_func.h. Man page entries and example entry.
* nlnet/master: (39 commits) - Fix unbound-dnstap-socket time fraction conversion for printout. - Fix unbound-dnstap-socket printout when no query is present. - Fix to remove unused variables from RPZ clientip data structure. - Fix RPZ removal of client-ip, nsip, nsdname triggers from IXFR. - Fix to print debug log for ancillary data with correct IP address. - Fix NLnetLabs#888: [FR] Use kernel timestamps for dnstap. - Fix warning in windows compile, in set_recvtimestamp. - Fix doxygen in addr_to_nat64 header definition. - Fix to remove unused whitespace from acx_nlnetlabs.m4 and config.h. - Fix NLnetLabs#885: Error: util/configlexer.c: No such file or directory, adds error messages explaining to install flex and bison. Changelog entry for NLnetLabs#722: - Merge NLnetLabs#722 from David 'eqvinox' Lamparter: NAT64 support. - For NLnetLabs#722: minor fixes, formatting, refactoring. - For NLnetLabs#722: Minor fixes, formatting and refactoring. - Fix RPZ IP responses with trigger rpz-drop on cache entries, that they are dropped. Changelog for NLnetLabs#860 Remove msg_del_for_0ttl, call msg_cache_remove directly - Fix for NLnetLabs#882: document variable to stop doxygen warning. - Fix for NLnetLabs#882: small changes, date updated in Copyright for util/timeval_func.c and util/timeval_func.h. Man page entries and example entry. stats: add query max wait time metric stats: add counter for timed out queries config: add sock_queue_timeout configuration ...
This pull request adds
sock_queue_timeout
configuration options which stores the amount of seconds spend in the socket queue after which the packet is considered old and should be dropped. The reason to drop is because client won't be waiting for the reply anymore and there is no reason to process the query.There are also 2 metrics added:
num.queries_timed_out
to store the amount of dropped packetsquery.queue_time_us.max
to collect the max time that was found for packets to wait in the socket queue (this can be easily converted to histogram if needed)