From aff1db15433dd107ff6ab9fb3be094f9e9d9ab42 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 7 Sep 2022 14:44:39 +0200 Subject: [PATCH] nic_router: generate reports asynchronously The NIC router used to generate reports triggered by IP config changes or link state changes synchonously, i.e., inline with the activation context that caused the change. This has two disadvantages. First, it can lead to an excessive number of report updates in situations with quick bursts of triggering changes. In such situations it is preferable to collect the changes and reflect them with only one final report update. Second, synchronous reporting may happen while the router is in a state that leads to an incorrect report (e.g. during reconfiguration). To prevent this from happening, the router so far explicitely switched off reporting when entering incoherent states and back on when leaving them. However, this solution is error-prone as the exclusion windows must be maintained manually. Both issues can be solved by not directly generating a report when necessary but instead submitting a signal and letting the signal handler do the work in a dedicated activation context. Ref #4462 --- .../os/src/server/nic_router/configuration.cc | 38 +++++---------- .../os/src/server/nic_router/configuration.h | 23 ++++------ repos/os/src/server/nic_router/main.cc | 20 +++++--- repos/os/src/server/nic_router/report.cc | 46 ++++++++++--------- repos/os/src/server/nic_router/report.h | 20 ++++---- 5 files changed, 70 insertions(+), 77 deletions(-) diff --git a/repos/os/src/server/nic_router/configuration.cc b/repos/os/src/server/nic_router/configuration.cc index 388930f922..020782a2cc 100644 --- a/repos/os/src/server/nic_router/configuration.cc +++ b/repos/os/src/server/nic_router/configuration.cc @@ -103,13 +103,14 @@ Configuration::_init_icmp_type_3_code_on_fragm_ipv4(Xml_node const &node) const } -Configuration::Configuration(Env &env, - Xml_node const node, - Allocator &alloc, - Cached_timer &timer, - Configuration &old_config, - Quota const &shared_quota, - Interface_list &interfaces) +Configuration::Configuration(Env &env, + Xml_node const node, + Allocator &alloc, + Signal_context_capability const &report_signal_cap, + Cached_timer &timer, + Configuration &old_config, + Quota const &shared_quota, + Interface_list &interfaces) : _alloc { alloc }, _max_packets_per_signal { node.attribute_value("max_packets_per_signal", (unsigned long)150) }, @@ -193,8 +194,9 @@ Configuration::Configuration(Env &env, } /* create report generator */ _report = *new (_alloc) - Report(_verbose, report_node, timer, _domains, shared_quota, - env.pd(), _reporter()); + Report { + _verbose, report_node, timer, _domains, shared_quota, env.pd(), + _reporter(), report_signal_cap }; } catch (Genode::Xml_node::Nonexistent_sub_node) { } @@ -224,24 +226,6 @@ Configuration::Configuration(Env &env, } -void Configuration::stop_reporting() -{ - if (!_reporter.valid()) { - return; - } - _reporter().enabled(false); -} - - -void Configuration::start_reporting() -{ - if (!_reporter.valid()) { - return; - } - _reporter().enabled(true); -} - - Configuration::~Configuration() { /* destroy NIC clients */ diff --git a/repos/os/src/server/nic_router/configuration.h b/repos/os/src/server/nic_router/configuration.h index 2f157d8bbe..2f80214cbf 100644 --- a/repos/os/src/server/nic_router/configuration.h +++ b/repos/os/src/server/nic_router/configuration.h @@ -66,23 +66,20 @@ class Net::Configuration public: - Configuration(Genode::Xml_node const node, - Genode::Allocator &alloc); + Configuration(Genode::Xml_node const node, + Genode::Allocator &alloc); - Configuration(Genode::Env &env, - Genode::Xml_node const node, - Genode::Allocator &alloc, - Cached_timer &timer, - Configuration &old_config, - Quota const &shared_quota, - Interface_list &interfaces); + Configuration(Genode::Env &env, + Genode::Xml_node const node, + Genode::Allocator &alloc, + Genode::Signal_context_capability const &report_signal_cap, + Cached_timer &timer, + Configuration &old_config, + Quota const &shared_quota, + Interface_list &interfaces); ~Configuration(); - void stop_reporting(); - - void start_reporting(); - /*************** ** Accessors ** diff --git a/repos/os/src/server/nic_router/main.cc b/repos/os/src/server/nic_router/main.cc index c2fdfa637e..179fd75567 100644 --- a/repos/os/src/server/nic_router/main.cc +++ b/repos/os/src/server/nic_router/main.cc @@ -38,12 +38,15 @@ class Net::Main Interface_list _interfaces { }; Cached_timer _timer { _env }; Genode::Heap _heap { &_env.ram(), &_env.rm() }; + Signal_handler
_report_handler { _env.ep(), *this, &Main::_handle_report }; Genode::Attached_rom_dataspace _config_rom { _env, "config" }; Reference _config { *new (_heap) Configuration { _config_rom.xml(), _heap } }; Signal_handler
_config_handler { _env.ep(), *this, &Main::_handle_config }; Nic_session_root _nic_session_root { _env, _timer, _heap, _config(), _shared_quota, _interfaces }; Uplink_session_root _uplink_session_root { _env, _timer, _heap, _config(), _shared_quota, _interfaces }; + void _handle_report(); + void _handle_config(); template @@ -65,6 +68,14 @@ class Net::Main }; +void Main::_handle_report() +{ + try { _config().report().generate(); } + catch (Pointer::Invalid) { } + +} + + Net::Main::Main(Env &env) : _env(env) { _config_rom.sigh(_config_handler); @@ -76,13 +87,12 @@ Net::Main::Main(Env &env) : _env(env) void Net::Main::_handle_config() { - _config().stop_reporting(); - _config_rom.update(); Configuration &old_config = _config(); Configuration &new_config = *new (_heap) - Configuration(_env, _config_rom.xml(), _heap, _timer, old_config, - _shared_quota, _interfaces); + Configuration { + _env, _config_rom.xml(), _heap, _report_handler, _timer, + old_config, _shared_quota, _interfaces }; _nic_session_root.handle_config(new_config); _uplink_session_root.handle_config(new_config); @@ -92,8 +102,6 @@ void Net::Main::_handle_config() _for_each_interface([&] (Interface &intf) { intf.handle_config_3(); }); destroy(_heap, &old_config); - - _config().start_reporting(); } diff --git a/repos/os/src/server/nic_router/report.cc b/repos/os/src/server/nic_router/report.cc index 22a9490e77..0e51330f5d 100644 --- a/repos/os/src/server/nic_router/report.cc +++ b/repos/os/src/server/nic_router/report.cc @@ -20,13 +20,14 @@ using namespace Net; using namespace Genode; -Net::Report::Report(bool const &verbose, - Xml_node const node, - Cached_timer &timer, - Domain_tree &domains, - Quota const &shared_quota, - Pd_session &pd, - Reporter &reporter) +Net::Report::Report(bool const &verbose, + Xml_node const node, + Cached_timer &timer, + Domain_tree &domains, + Quota const &shared_quota, + Pd_session &pd, + Reporter &reporter, + Signal_context_capability const &signal_cap) : _verbose { verbose }, _config { node.attribute_value("config", true) }, @@ -42,15 +43,15 @@ Net::Report::Report(bool const &verbose, _reporter { reporter }, _domains { domains }, _timeout { timer, *this, &Report::_handle_report_timeout, - read_sec_attr(node, "interval_sec", 5) } -{ } - - -void Net::Report::_report() + read_sec_attr(node, "interval_sec", 5) }, + _signal_transmitter { signal_cap } +{ + _reporter.enabled(true); +} + + +void Net::Report::generate() { - if (!_reporter.enabled()) { - return; - } try { Reporter::Xml_generator xml(_reporter, [&] () { if (_quota) { @@ -79,22 +80,23 @@ void Net::Report::_report() void Net::Report::_handle_report_timeout(Duration) { - _report(); + generate(); } void Net::Report::handle_config() { if (!_config_triggers) { - return; } - - _report(); + return; + } + _signal_transmitter.submit(); } + void Net::Report::handle_interface_link_state() { if (!_link_state_triggers) { - return; } - - _report(); + return; + } + _signal_transmitter.submit(); } diff --git a/repos/os/src/server/nic_router/report.h b/repos/os/src/server/nic_router/report.h index b205a57408..16bb3eba8b 100644 --- a/repos/os/src/server/nic_router/report.h +++ b/repos/os/src/server/nic_router/report.h @@ -59,27 +59,29 @@ class Net::Report Genode::Reporter &_reporter; Domain_tree &_domains; Timer::Periodic_timeout _timeout; + Genode::Signal_transmitter _signal_transmitter; void _handle_report_timeout(Genode::Duration); - void _report(); - public: struct Empty : Genode::Exception { }; - Report(bool const &verbose, - Genode::Xml_node const node, - Cached_timer &timer, - Domain_tree &domains, - Quota const &shared_quota, - Genode::Pd_session &pd, - Genode::Reporter &reporter); + Report(bool const &verbose, + Genode::Xml_node const node, + Cached_timer &timer, + Domain_tree &domains, + Quota const &shared_quota, + Genode::Pd_session &pd, + Genode::Reporter &reporter, + Genode::Signal_context_capability const &signal_cap); void handle_config(); void handle_interface_link_state(); + void generate(); + /*************** ** Accessors **