From 8cdc3e4374d125ec77e600d7ad7735ac28531f2b Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Sat, 29 Oct 2022 19:44:08 +0200 Subject: [PATCH 1/2] Add btest for expiration of all pending timers. --- .../core.expire-all-timers/conn-all.log | 12 ++++++++ .../core.expire-all-timers/conn-limited.log | 11 ++++++++ .../btest/Traces/tcp/retransmit-timeout.pcap | Bin 0 -> 452 bytes testing/btest/core/expire-all-timers.zeek | 26 ++++++++++++++++++ 4 files changed, 49 insertions(+) create mode 100644 testing/btest/Baseline/core.expire-all-timers/conn-all.log create mode 100644 testing/btest/Baseline/core.expire-all-timers/conn-limited.log create mode 100644 testing/btest/Traces/tcp/retransmit-timeout.pcap create mode 100644 testing/btest/core/expire-all-timers.zeek diff --git a/testing/btest/Baseline/core.expire-all-timers/conn-all.log b/testing/btest/Baseline/core.expire-all-timers/conn-all.log new file mode 100644 index 0000000000..194375388d --- /dev/null +++ b/testing/btest/Baseline/core.expire-all-timers/conn-all.log @@ -0,0 +1,12 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path conn +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents +#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.0.0.1 51889 192.168.0.1 80 tcp - 0.000010 18 0 OTH - - 0 Da 1 58 1 40 - +XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 10.0.0.1 51889 192.168.0.1 80 tcp - - - - OTH - - 0 D 1 58 0 0 - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline/core.expire-all-timers/conn-limited.log b/testing/btest/Baseline/core.expire-all-timers/conn-limited.log new file mode 100644 index 0000000000..c349ee4a03 --- /dev/null +++ b/testing/btest/Baseline/core.expire-all-timers/conn-limited.log @@ -0,0 +1,11 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path conn +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents +#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.0.0.1 51889 192.168.0.1 80 tcp - 300.000010 18 0 OTH - - 0 DaT 2 116 1 40 - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/tcp/retransmit-timeout.pcap b/testing/btest/Traces/tcp/retransmit-timeout.pcap new file mode 100644 index 0000000000000000000000000000000000000000..2529d4d214bcade50accac87aee587c45dccc74d GIT binary patch literal 452 zcmd<$<>l&NU|{gI(UxKa(*L1=nL(2wH!;~iSi#Up&s5J)MSJLrX7EidN>5ErEKtx0&n!wU&PXiE)>JUjGte_o&?p1bx(2%G z2FXdO2A0Vw=9XqjAe{^hAp1dfi$LlB{~>HPAU^|$L7?4+i9zniyhR>BHV8BA28lAQ z0uqcI46Y0eRv;rB*ft1r0Ywh1U|>A8ks*MAf%zo^1N+4Q0R;yBJq!%)t|1Eg3LYUL z0s4k|hP+(7Tp;s7_Cx?N2!QO-0NP^)WP>oo28eB7do-Z-sDkVO*~Jh5v%mJB)?tYEg4O?j8egKTeR#E@} literal 0 HcmV?d00001 diff --git a/testing/btest/core/expire-all-timers.zeek b/testing/btest/core/expire-all-timers.zeek new file mode 100644 index 0000000000..a6cff297f2 --- /dev/null +++ b/testing/btest/core/expire-all-timers.zeek @@ -0,0 +1,26 @@ +# @TEST-EXEC: zeek -b -C -r $TRACES/tcp/retransmit-timeout.pcap %INPUT +# @TEST-EXEC: mv conn.log conn-limited.log + +# @TEST-EXEC: zeek -b -C -r $TRACES/tcp/retransmit-timeout.pcap %INPUT max_timer_expires=0 +# @TEST-EXEC: mv conn.log conn-all.log + + +# @TEST-EXEC: btest-diff conn-limited.log +# @TEST-EXEC: btest-diff conn-all.log + +@load base/protocols/conn + +const max_timer_expires_default = max_timer_expires; + +event dummy() + { + } + +event network_time_init() + { + # Suppress connection timeout by scheduling more timers than + # can be handled in the context of a single packet, by default. + local i = 0; + while ( ++i <= max_timer_expires_default ) + schedule 4 min { dummy() }; + } From 2becb1337f8d97ba145ad4930d2b268ca1ee7522 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 24 Nov 2022 11:29:04 +0100 Subject: [PATCH 2/2] TimerMgr: Add back max_timer_expires=0 special case Commit 58fae227082dd09107f5389f2f054c4b6a6e1a20 removed the max_expire==0 handling from DoAdvance() due to not being obvious what use it is. Jan later reported that it broke the `redef max_timer_expires=0` (#2514). This commit adds back the special case re-introducing the `max_timer_expires=0` , trying to make it fairly explicit that it exists. This is an adaption of #2516 not adding a new option and trying a bit to avoid global variable accesses down in DoAdvance(), though that just moved to InitPostScript(). Fixes #2514. --- scripts/base/init-bare.zeek | 4 ++-- src/Timer.cc | 5 ++++- src/Timer.h | 6 +++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 8377be801a..cd27134c73 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -2116,8 +2116,8 @@ global discarder_check_icmp: function(p: pkt_hdr): bool; ## Zeek's watchdog interval. const watchdog_interval = 10 sec &redef; -## The maximum number of timers to expire after processing each new -## packet. The value trades off spreading out the timer expiration load +## The maximum number of expired timers to process after processing each new +## packet. The value trades off spreading out the timer expiration load ## with possibly having to hold state longer. A value of 0 means ## "process all expired timers with each new packet". const max_timer_expires = 300 &redef; diff --git a/src/Timer.cc b/src/Timer.cc index 8b3ec37478..6908e81f95 100644 --- a/src/Timer.cc +++ b/src/Timer.cc @@ -111,6 +111,8 @@ void TimerMgr::InitPostScript() { if ( iosource_mgr ) iosource_mgr->Register(this, true); + + dispatch_all_expired = zeek::detail::max_timer_expires == 0; } void TimerMgr::Add(Timer* timer) @@ -142,7 +144,8 @@ void TimerMgr::Expire() int TimerMgr::DoAdvance(double new_t, int max_expire) { Timer* timer = Top(); - for ( num_expired = 0; (num_expired < max_expire) && timer && timer->Time() <= new_t; + for ( num_expired = 0; + (num_expired < max_expire || dispatch_all_expired) && timer && timer->Time() <= new_t; ++num_expired ) { last_timestamp = timer->Time(); diff --git a/src/Timer.h b/src/Timer.h index 883903512a..4592b4d953 100644 --- a/src/Timer.h +++ b/src/Timer.h @@ -84,7 +84,8 @@ public: void Add(Timer* timer); /** - * Advance the clock to time t, expiring at most max_expire timers. + * Advance the clock to time t, dispatching at most max_expire expired + * timers, or all expired timers if dispatch_all_expired is set. * * @param t the new time. * @param max_expire the maximum number of timers to expire. @@ -152,6 +153,9 @@ private: double last_advance; int num_expired; + // Flag to indicate if Advance() should dispatch all expired timers + // for the max_timer_expires=0 case. + bool dispatch_all_expired = false; size_t peak_size = 0; size_t cumulative_num = 0;