From 3c95d1d695ed80d7796b680fd31fa8f3afaf8ad3 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 13 Feb 2014 14:55:45 -0600 Subject: [PATCH] Refactor DNS script's state management to improve performance. The amount of timers involved in DNS::PendingMessage tables' expiration attributes have a significant performance hit. Instead the script now relies solely on maximum thresholds for pending message quantities to limit amount of accumulated state. There's a new option, "DNS::max_pending_query_ids", to limit the number outstanding messages across all DNS query IDs ("DNS::max_pending_msgs" still limits number of outstanding messages for a *given* query ID). --- scripts/base/protocols/dns/main.bro | 64 +++++++++---------- .../weird.log | 5 +- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/scripts/base/protocols/dns/main.bro b/scripts/base/protocols/dns/main.bro index 21a0711159..294220d1f2 100644 --- a/scripts/base/protocols/dns/main.bro +++ b/scripts/base/protocols/dns/main.bro @@ -109,16 +109,6 @@ export { ## DNS message query/transaction ID. type PendingMessages: table[count] of Queue::Queue; - ## Called when a pending DNS query has not been matched with a reply (or - ## vice versa) in a sufficent amount of time. - ## - ## pending: table of pending messages, indexed by transaction ID. - ## - ## id: the index of he element being expired. - ## - ## Returns: amount of time to delay expiration of the element. - global expire_pending_msg: function(pending: PendingMessages, id: count): interval; - ## The amount of time that DNS queries or replies for a given ## query/transaction ID are allowed to be queued while waiting for ## a matching reply or query. @@ -131,16 +121,21 @@ export { ## response is ongoing). const max_pending_msgs = 50 &redef; + ## Give up trying to match pending DNS queries or replies across all + ## query/transaction IDs once there is at least one unmatched query or + ## reply across this number of different query IDs. + const max_pending_query_ids = 50 &redef; + ## A record type which tracks the status of DNS queries for a given ## :bro:type:`connection`. type State: record { ## Indexed by query id, returns Info record corresponding to ## queries that haven't been matched with a response yet. - pending_queries: PendingMessages &read_expire=pending_msg_expiry_interval &expire_func=expire_pending_msg; + pending_queries: PendingMessages; ## Indexed by query id, returns Info record corresponding to ## replies that haven't been matched with a query yet. - pending_replies: PendingMessages &read_expire=pending_msg_expiry_interval &expire_func=expire_pending_msg; + pending_replies: PendingMessages; }; } @@ -176,7 +171,11 @@ function log_unmatched_msgs_queue(q: Queue::Queue) Queue::get_vector(q, infos); for ( i in infos ) + { + event flow_weird("dns_unmatched_msg", + infos[i]$id$orig_h, infos[i]$id$resp_h); Log::write(DNS::LOG, infos[i]); + } } function log_unmatched_msgs(msgs: PendingMessages) @@ -191,16 +190,28 @@ function log_unmatched_msgs(msgs: PendingMessages) function enqueue_new_msg(msgs: PendingMessages, id: count, msg: Info) { if ( id !in msgs ) - msgs[id] = Queue::init(); - else if ( Queue::len(msgs[id]) > max_pending_msgs ) { - local info: Info = Queue::peek(msgs[id]); - event flow_weird("dns_unmatched_msg_quantity", info$id$orig_h, - info$id$resp_h); - log_unmatched_msgs_queue(msgs[id]); - # Throw away all unmatched on assumption they'll never be matched. + if ( |msgs| > max_pending_query_ids ) + { + event flow_weird("dns_unmatched_query_id_quantity", + msg$id$orig_h, msg$id$resp_h); + # Throw away all unmatched on assumption they'll never be matched. + log_unmatched_msgs(msgs); + } + msgs[id] = Queue::init(); } + else + { + if ( Queue::len(msgs[id]) > max_pending_msgs ) + { + event flow_weird("dns_unmatched_msg_quantity", + msg$id$orig_h, msg$id$resp_h); + log_unmatched_msgs_queue(msgs[id]); + # Throw away all unmatched on assumption they'll never be matched. + msgs[id] = Queue::init(); + } + } Queue::put(msgs[id], msg); } @@ -447,18 +458,3 @@ event connection_state_remove(c: connection) &priority=-5 log_unmatched_msgs(c$dns_state$pending_queries); log_unmatched_msgs(c$dns_state$pending_replies); } - -function expire_pending_msg(pending: PendingMessages, id: count): interval - { - local infos: vector of Info; - Queue::get_vector(pending[id], infos); - - for ( i in infos ) - { - Log::write(DNS::LOG, infos[i]); - event flow_weird("dns_unmatched_msg", infos[i]$id$orig_h, - infos[i]$id$resp_h); - } - - return 0sec; - } diff --git a/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/weird.log b/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/weird.log index 175a474425..295de4ec2c 100644 --- a/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/weird.log +++ b/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/weird.log @@ -3,9 +3,10 @@ #empty_field (empty) #unset_field - #path weird -#open 2013-08-26-19-36-33 +#open 2014-02-13-20-36-35 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p name addl notice peer #types time string addr port addr port string string bool string 1363716396.798286 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 DNS_RR_unknown_type - F bro 1363716396.798374 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 dns_unmatched_reply - F bro -#close 2013-08-26-19-36-33 +1363716396.798374 - - - - - dns_unmatched_msg - F bro +#close 2014-02-13-20-36-35