From 21cc5f91327746da966031371a719b13fbdd4234 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 8 Dec 2022 12:55:47 +0100 Subject: [PATCH] EventRegistry/Func: Disable events when all bodies are disabled This is just a small optimization on top of what is there. Add state to Func for tracking if any enabled bodies exist which allows us to propagate it up to the EventHandler::bool() operator. In turn, when all bodies of an event's Func have been runtime disabled, the event itself will not be invoked anymore. Experiments have shown that this allows runtime toggling of new_event() without performance impact when disabled. This could enable use-cases where new_packet() handlers are enabled for a split second once in a while to either dump or sample raw packet data at runtime. --- src/EventHandler.cc | 3 +- src/EventRegistry.cc | 22 +++- src/Func.h | 14 ++- .../recv.recv.out | 7 ++ .../send.send.out | 7 ++ .../btest/broker/event-group-interaction.zeek | 117 ++++++++++++++++++ 6 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 testing/btest/Baseline/broker.event-group-interaction/recv.recv.out create mode 100644 testing/btest/Baseline/broker.event-group-interaction/send.send.out create mode 100644 testing/btest/broker/event-group-interaction.zeek diff --git a/src/EventHandler.cc b/src/EventHandler.cc index 62a616a619..154bc7615f 100644 --- a/src/EventHandler.cc +++ b/src/EventHandler.cc @@ -24,7 +24,8 @@ EventHandler::EventHandler(std::string arg_name) EventHandler::operator bool() const { - return enabled && ((local && local->HasBodies()) || generate_always || ! auto_publish.empty()); + return enabled && + ((local && local->HasEnabledBodies()) || generate_always || ! auto_publish.empty()); } const FuncTypePtr& EventHandler::GetType(bool check_export) diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index d83368510c..7a3d1c5081 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -1,5 +1,7 @@ #include "zeek/EventRegistry.h" +#include + #include "zeek/EventHandler.h" #include "zeek/Func.h" #include "zeek/RE.h" @@ -171,19 +173,31 @@ EventGroup::~EventGroup() noexcept { } // Run through all ScriptFunc instances associated with this group and // update their bodies after a group's enable/disable state has changed. +// Once that has completed, also update the Func's has_enabled_bodies +// setting based on the new state of its bodies. // // EventGroup is private friend with Func, so fiddling with the bodies -// directly works and keeps the logic away from Func for now. +// and private members works and keeps the logic out of Func and away +// from the public zeek:: namespace. void EventGroup::UpdateFuncBodies() { - static auto is_disabled = [](const auto& g) + static auto is_group_disabled = [](const auto& g) { return g->IsDisabled(); }; - for ( const auto& func : funcs ) + for ( auto& func : funcs ) + { for ( auto& b : func->bodies ) - b.disabled = std::any_of(b.groups.cbegin(), b.groups.cend(), is_disabled); + b.disabled = std::any_of(b.groups.cbegin(), b.groups.cend(), is_group_disabled); + + static auto is_body_enabled = [](const auto& b) + { + return ! b.disabled; + }; + func->has_enabled_bodies = std::any_of(func->bodies.cbegin(), func->bodies.cend(), + is_body_enabled); + } } void EventGroup::Enable() diff --git a/src/Func.h b/src/Func.h index 05b2b6dc9a..ef6151829b 100644 --- a/src/Func.h +++ b/src/Func.h @@ -85,7 +85,14 @@ public: }; const std::vector& GetBodies() const { return bodies; } - bool HasBodies() const { return bodies.size(); } + bool HasBodies() const { return ! bodies.empty(); } + + /** + * Are there bodies and is any one of them enabled? + * + * @return true if bodies exist and at least one is enabled. + */ + bool HasEnabledBodies() const { return ! bodies.empty() && has_enabled_bodies; }; /** * Calls a Zeek function. @@ -149,8 +156,11 @@ protected: std::string name; private: - // EventGroup updates Func::Body.disabled + // EventGroup updates Func::Body.disabled and has_enabled_bodies. + // This is friend/private with EventGroup here so that we do not + // expose accessors in the zeek:: public interface. friend class EventGroup; + bool has_enabled_bodies = true; }; namespace detail diff --git a/testing/btest/Baseline/broker.event-group-interaction/recv.recv.out b/testing/btest/Baseline/broker.event-group-interaction/recv.recv.out new file mode 100644 index 0000000000..24ee373fe9 --- /dev/null +++ b/testing/btest/Baseline/broker.event-group-interaction/recv.recv.out @@ -0,0 +1,7 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +receiver added peer: endpoint=127.0.0.1 msg=handshake successful +receiver got ping: my-message, 1 +receiver got ping: my-message, 2 +receiver got ping: my-message, 3 +receiver got ping: my-message, 4 +receiver got ping: my-message, 5 diff --git a/testing/btest/Baseline/broker.event-group-interaction/send.send.out b/testing/btest/Baseline/broker.event-group-interaction/send.send.out new file mode 100644 index 0000000000..bd2182597e --- /dev/null +++ b/testing/btest/Baseline/broker.event-group-interaction/send.send.out @@ -0,0 +1,7 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +sender added peer: endpoint=127.0.0.1 msg=handshake successful +sender got pong: my-message, 1 +sender got pong: my-message, 2 +sender got pong: my-message, 3 +sender got pong: my-message, 4 +sender lost peer: endpoint=127.0.0.1 msg=lost connection to remote peer diff --git a/testing/btest/broker/event-group-interaction.zeek b/testing/btest/broker/event-group-interaction.zeek new file mode 100644 index 0000000000..e62ca9f1fa --- /dev/null +++ b/testing/btest/broker/event-group-interaction.zeek @@ -0,0 +1,117 @@ +# @TEST-DOC: Disabling an unrelated event group caused auto-publish to break because the remote event had no bodies and got disabled. This is a regression test it's not being done again. +# +# @TEST-GROUP: broker +# +# @TEST-PORT: BROKER_PORT +# +# @TEST-EXEC: btest-bg-run recv "zeek -b ../recv.zeek >recv.out" +# @TEST-EXEC: btest-bg-run send "zeek -b ../send.zeek >send.out" +# +# @TEST-EXEC: btest-bg-wait 10 +# @TEST-EXEC: btest-diff recv/recv.out +# @TEST-EXEC: btest-diff send/send.out + +@TEST-START-FILE send.zeek + +global event_count = 0; + +global ping: event(msg: string, c: count); + +event zeek_init() + { + Broker::subscribe("zeek/event/my_topic"); + Broker::auto_publish("zeek/event/my_topic", ping); + Broker::peer("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +function send_event() + { + event ping("my-message", ++event_count); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print fmt("sender added peer: endpoint=%s msg=%s", endpoint$network$address, msg); + send_event(); + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + print fmt("sender lost peer: endpoint=%s msg=%s", endpoint$network$address, msg); + terminate(); + } + +event pong(msg: string, n: count) &is_used + { + print fmt("sender got pong: %s, %s", msg, n); + send_event(); + } + +module TestDumpEvents; + +event pong(msg: string, n: count) &is_used + { + print fmt("ERROR: This should not be visible: %s, %s", msg, n); + } + +event zeek_init() + { + disable_module_events("TestDumpEvents"); + } + +@TEST-END-FILE + + +@TEST-START-FILE recv.zeek + +redef exit_only_after_terminate = T; + +const events_to_recv = 5; + +global pong: event(msg: string, c: count); + +event zeek_init() + { + Broker::subscribe("zeek/event/my_topic"); + Broker::auto_publish("zeek/event/my_topic", pong); + Broker::listen("127.0.0.1", to_port(getenv("BROKER_PORT"))); + } + +event Broker::peer_added(endpoint: Broker::EndpointInfo, msg: string) + { + print fmt("receiver added peer: endpoint=%s msg=%s", + endpoint$network$address, msg); + } + +event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) + { + print fmt("receiver lost peer: endpoint=%s msg=%s", + endpoint$network$address, msg); + } + +event ping(msg: string, n: count) &is_used + { + print fmt("receiver got ping: %s, %s", msg, n); + + if ( n == events_to_recv ) + { + terminate(); + return; + } + + event pong(msg, n); + } + +module TestDumpEvents; + +event ping(msg: string, n: count) &is_used + { + print fmt("ERROR: This should not be visible: %s, %s", msg, n); + } + +event zeek_init() + { + disable_module_events("TestDumpEvents"); + } + +@TEST-END-FILE