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
This commit is contained in:
Martin Stein 2022-09-07 14:44:39 +02:00 committed by Norman Feske
parent a573d3a332
commit aff1db1543
5 changed files with 70 additions and 77 deletions

View File

@ -103,13 +103,14 @@ Configuration::_init_icmp_type_3_code_on_fragm_ipv4(Xml_node const &node) const
} }
Configuration::Configuration(Env &env, Configuration::Configuration(Env &env,
Xml_node const node, Xml_node const node,
Allocator &alloc, Allocator &alloc,
Cached_timer &timer, Signal_context_capability const &report_signal_cap,
Configuration &old_config, Cached_timer &timer,
Quota const &shared_quota, Configuration &old_config,
Interface_list &interfaces) Quota const &shared_quota,
Interface_list &interfaces)
: :
_alloc { alloc }, _alloc { alloc },
_max_packets_per_signal { node.attribute_value("max_packets_per_signal", (unsigned long)150) }, _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 */ /* create report generator */
_report = *new (_alloc) _report = *new (_alloc)
Report(_verbose, report_node, timer, _domains, shared_quota, Report {
env.pd(), _reporter()); _verbose, report_node, timer, _domains, shared_quota, env.pd(),
_reporter(), report_signal_cap };
} }
catch (Genode::Xml_node::Nonexistent_sub_node) { } 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() Configuration::~Configuration()
{ {
/* destroy NIC clients */ /* destroy NIC clients */

View File

@ -66,23 +66,20 @@ class Net::Configuration
public: public:
Configuration(Genode::Xml_node const node, Configuration(Genode::Xml_node const node,
Genode::Allocator &alloc); Genode::Allocator &alloc);
Configuration(Genode::Env &env, Configuration(Genode::Env &env,
Genode::Xml_node const node, Genode::Xml_node const node,
Genode::Allocator &alloc, Genode::Allocator &alloc,
Cached_timer &timer, Genode::Signal_context_capability const &report_signal_cap,
Configuration &old_config, Cached_timer &timer,
Quota const &shared_quota, Configuration &old_config,
Interface_list &interfaces); Quota const &shared_quota,
Interface_list &interfaces);
~Configuration(); ~Configuration();
void stop_reporting();
void start_reporting();
/*************** /***************
** Accessors ** ** Accessors **

View File

@ -38,12 +38,15 @@ class Net::Main
Interface_list _interfaces { }; Interface_list _interfaces { };
Cached_timer _timer { _env }; Cached_timer _timer { _env };
Genode::Heap _heap { &_env.ram(), &_env.rm() }; Genode::Heap _heap { &_env.ram(), &_env.rm() };
Signal_handler<Main> _report_handler { _env.ep(), *this, &Main::_handle_report };
Genode::Attached_rom_dataspace _config_rom { _env, "config" }; Genode::Attached_rom_dataspace _config_rom { _env, "config" };
Reference<Configuration> _config { *new (_heap) Configuration { _config_rom.xml(), _heap } }; Reference<Configuration> _config { *new (_heap) Configuration { _config_rom.xml(), _heap } };
Signal_handler<Main> _config_handler { _env.ep(), *this, &Main::_handle_config }; Signal_handler<Main> _config_handler { _env.ep(), *this, &Main::_handle_config };
Nic_session_root _nic_session_root { _env, _timer, _heap, _config(), _shared_quota, _interfaces }; 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 }; Uplink_session_root _uplink_session_root { _env, _timer, _heap, _config(), _shared_quota, _interfaces };
void _handle_report();
void _handle_config(); void _handle_config();
template <typename FUNC> template <typename FUNC>
@ -65,6 +68,14 @@ class Net::Main
}; };
void Main::_handle_report()
{
try { _config().report().generate(); }
catch (Pointer<Report>::Invalid) { }
}
Net::Main::Main(Env &env) : _env(env) Net::Main::Main(Env &env) : _env(env)
{ {
_config_rom.sigh(_config_handler); _config_rom.sigh(_config_handler);
@ -76,13 +87,12 @@ Net::Main::Main(Env &env) : _env(env)
void Net::Main::_handle_config() void Net::Main::_handle_config()
{ {
_config().stop_reporting();
_config_rom.update(); _config_rom.update();
Configuration &old_config = _config(); Configuration &old_config = _config();
Configuration &new_config = *new (_heap) Configuration &new_config = *new (_heap)
Configuration(_env, _config_rom.xml(), _heap, _timer, old_config, Configuration {
_shared_quota, _interfaces); _env, _config_rom.xml(), _heap, _report_handler, _timer,
old_config, _shared_quota, _interfaces };
_nic_session_root.handle_config(new_config); _nic_session_root.handle_config(new_config);
_uplink_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(); }); _for_each_interface([&] (Interface &intf) { intf.handle_config_3(); });
destroy(_heap, &old_config); destroy(_heap, &old_config);
_config().start_reporting();
} }

View File

@ -20,13 +20,14 @@ using namespace Net;
using namespace Genode; using namespace Genode;
Net::Report::Report(bool const &verbose, Net::Report::Report(bool const &verbose,
Xml_node const node, Xml_node const node,
Cached_timer &timer, Cached_timer &timer,
Domain_tree &domains, Domain_tree &domains,
Quota const &shared_quota, Quota const &shared_quota,
Pd_session &pd, Pd_session &pd,
Reporter &reporter) Reporter &reporter,
Signal_context_capability const &signal_cap)
: :
_verbose { verbose }, _verbose { verbose },
_config { node.attribute_value("config", true) }, _config { node.attribute_value("config", true) },
@ -42,15 +43,15 @@ Net::Report::Report(bool const &verbose,
_reporter { reporter }, _reporter { reporter },
_domains { domains }, _domains { domains },
_timeout { timer, *this, &Report::_handle_report_timeout, _timeout { timer, *this, &Report::_handle_report_timeout,
read_sec_attr(node, "interval_sec", 5) } read_sec_attr(node, "interval_sec", 5) },
{ } _signal_transmitter { signal_cap }
{
_reporter.enabled(true);
void Net::Report::_report() }
void Net::Report::generate()
{ {
if (!_reporter.enabled()) {
return;
}
try { try {
Reporter::Xml_generator xml(_reporter, [&] () { Reporter::Xml_generator xml(_reporter, [&] () {
if (_quota) { if (_quota) {
@ -79,22 +80,23 @@ void Net::Report::_report()
void Net::Report::_handle_report_timeout(Duration) void Net::Report::_handle_report_timeout(Duration)
{ {
_report(); generate();
} }
void Net::Report::handle_config() void Net::Report::handle_config()
{ {
if (!_config_triggers) { if (!_config_triggers) {
return; } return;
}
_report(); _signal_transmitter.submit();
} }
void Net::Report::handle_interface_link_state() void Net::Report::handle_interface_link_state()
{ {
if (!_link_state_triggers) { if (!_link_state_triggers) {
return; } return;
}
_report(); _signal_transmitter.submit();
} }

View File

@ -59,27 +59,29 @@ class Net::Report
Genode::Reporter &_reporter; Genode::Reporter &_reporter;
Domain_tree &_domains; Domain_tree &_domains;
Timer::Periodic_timeout<Report> _timeout; Timer::Periodic_timeout<Report> _timeout;
Genode::Signal_transmitter _signal_transmitter;
void _handle_report_timeout(Genode::Duration); void _handle_report_timeout(Genode::Duration);
void _report();
public: public:
struct Empty : Genode::Exception { }; struct Empty : Genode::Exception { };
Report(bool const &verbose, Report(bool const &verbose,
Genode::Xml_node const node, Genode::Xml_node const node,
Cached_timer &timer, Cached_timer &timer,
Domain_tree &domains, Domain_tree &domains,
Quota const &shared_quota, Quota const &shared_quota,
Genode::Pd_session &pd, Genode::Pd_session &pd,
Genode::Reporter &reporter); Genode::Reporter &reporter,
Genode::Signal_context_capability const &signal_cap);
void handle_config(); void handle_config();
void handle_interface_link_state(); void handle_interface_link_state();
void generate();
/*************** /***************
** Accessors ** ** Accessors **