From e5b9a6cc8b92934cc110d58885cc3d023a3fadeb Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Mon, 25 Sep 2017 16:41:33 +0200 Subject: [PATCH] nic_router: rework round-trip-time handling Do not use two times the RTT for the lifetime of links but use it as it is configured to simplify the usage of the router. Internally, use Microseconds/Duration type instead of plain integers. Ref #2490 --- repos/libports/run/nic_dump.run | 2 +- repos/libports/run/nic_router.run | 2 +- repos/os/src/server/nic_router/README | 16 ++++++------ .../os/src/server/nic_router/configuration.cc | 24 +++++++----------- .../os/src/server/nic_router/configuration.h | 25 ++++++++++++------- repos/os/src/server/nic_router/link.cc | 2 +- repos/os/src/server/nic_router/link.h | 2 -- 7 files changed, 36 insertions(+), 37 deletions(-) diff --git a/repos/libports/run/nic_dump.run b/repos/libports/run/nic_dump.run index 1f4af10305..d4eef61f8b 100644 --- a/repos/libports/run/nic_dump.run +++ b/repos/libports/run/nic_dump.run @@ -178,7 +178,7 @@ append config { - + diff --git a/repos/libports/run/nic_router.run b/repos/libports/run/nic_router.run index ef24eb445b..e0f86e6ac6 100644 --- a/repos/libports/run/nic_router.run +++ b/repos/libports/run/nic_router.run @@ -197,7 +197,7 @@ append config { - + diff --git a/repos/os/src/server/nic_router/README b/repos/os/src/server/nic_router/README index b275aec20d..a9f2ab942f 100644 --- a/repos/os/src/server/nic_router/README +++ b/repos/os/src/server/nic_router/README @@ -220,18 +220,18 @@ link states so you should choose it with care. If it is too low, replies that normally need no routing rule may get lost. If it is too high, link states are held longer than necessary. -In general, each link state is discarded after a duration of two times the -round-trip time without a matching packet. For UDP link states, this is the -only rule and better known as hole punching. It allows peers to keep alive a -UDP pseudo-connection through the router by frequently sending empty packets. -The need for such a pseudo-connection arises from the router's demand to -support NAT for UDP transfers and the consequence of keeping the corresponding -mapping information. +In general, each link state is discarded after a duration of the round-trip +time without a matching packet. For UDP link states, this is the only rule and +better known as hole punching. It allows peers to keep alive a UDP +pseudo-connection through the router by frequently sending empty packets. The +need for such a pseudo-connection arises from the router's demand to support +NAT for UDP transfers and the consequence of keeping the corresponding mapping +information. The lifetime management of TCP link states, in contrast, is more complex. In addition to the common timeout, they may be discarded even if they still receive packets. This is the case when the router observed the four-way -termination handshake of TCP and two times the round-trip time has passed. +termination handshake of TCP and the round-trip time has passed. Configuring NAT diff --git a/repos/os/src/server/nic_router/configuration.cc b/repos/os/src/server/nic_router/configuration.cc index 5e7b2c8a7e..d3ecf9e2a1 100644 --- a/repos/os/src/server/nic_router/configuration.cc +++ b/repos/os/src/server/nic_router/configuration.cc @@ -23,29 +23,23 @@ using namespace Net; using namespace Genode; -/*************** - ** Utilities ** - ***************/ - -static unsigned read_rtt_sec(Xml_node const node) +Microseconds Configuration::_init_rtt(Xml_node const node) { - unsigned const rtt_sec = node.attribute_value("rtt_sec", 0UL); + unsigned rtt_sec = node.attribute_value("rtt_sec", 0UL); if (!rtt_sec) { - warning("fall back to default rtt_sec=\"3\""); - return 3; + warning("fall back to default rtt_sec=\"", + (unsigned)DEFAULT_RTT_SEC, "\""); + rtt_sec = DEFAULT_RTT_SEC; } - return rtt_sec; + return Microseconds(rtt_sec * 1000 * 1000); } -/******************* - ** Configuration ** - *******************/ - -Configuration::Configuration(Xml_node const node, Allocator &alloc) +Configuration::Configuration(Xml_node const node, + Allocator &alloc) : _alloc(alloc), _verbose(node.attribute_value("verbose", false)), - _rtt_sec(read_rtt_sec(node)), _node(node) + _rtt(_init_rtt(node)), _node(node) { /* read domains */ node.for_each_sub_node("domain", [&] (Xml_node const node) { diff --git a/repos/os/src/server/nic_router/configuration.h b/repos/os/src/server/nic_router/configuration.h index 4ff290e003..b25d3af2db 100644 --- a/repos/os/src/server/nic_router/configuration.h +++ b/repos/os/src/server/nic_router/configuration.h @@ -17,6 +17,9 @@ /* local includes */ #include +/* Genode includes */ +#include + namespace Genode { class Allocator; } namespace Net { class Configuration; } @@ -26,14 +29,18 @@ class Net::Configuration { private: - Genode::Allocator &_alloc; - bool const _verbose; - unsigned const _rtt_sec; - Domain_tree _domains; - Genode::Xml_node const _node; + Genode::Allocator &_alloc; + bool const _verbose; + Genode::Microseconds const _rtt; + Domain_tree _domains; + Genode::Xml_node const _node; + + Genode::Microseconds _init_rtt(Genode::Xml_node const node); public: + enum { DEFAULT_RTT_SEC = 6 }; + Configuration(Genode::Xml_node const node, Genode::Allocator &alloc); @@ -41,10 +48,10 @@ class Net::Configuration ** Accessors ** ***************/ - bool verbose() const { return _verbose; } - unsigned rtt_sec() const { return _rtt_sec; } - Domain_tree &domains() { return _domains; } - Genode::Xml_node node() const { return _node; } + bool verbose() const { return _verbose; } + Genode::Microseconds rtt() const { return _rtt; } + Domain_tree &domains() { return _domains; } + Genode::Xml_node node() const { return _node; } }; #endif /* _CONFIGURATION_H_ */ diff --git a/repos/os/src/server/nic_router/link.cc b/repos/os/src/server/nic_router/link.cc index b3a74a80f0..5c0b85c2f2 100644 --- a/repos/os/src/server/nic_router/link.cc +++ b/repos/os/src/server/nic_router/link.cc @@ -124,7 +124,7 @@ Link::Link(Interface &cln_interface, _server_port_alloc(srv_port_alloc), _server(srv_interface, srv_id, *this), _close_timeout(timer, *this, &Link::_handle_close_timeout), - _close_timeout_us(_config.rtt_sec() * 2 * 1000 * 1000), + _close_timeout_us(_config.rtt()), _protocol(protocol) { _close_timeout.schedule(_close_timeout_us); diff --git a/repos/os/src/server/nic_router/link.h b/repos/os/src/server/nic_router/link.h index 31123a5361..8cd4cbc90b 100644 --- a/repos/os/src/server/nic_router/link.h +++ b/repos/os/src/server/nic_router/link.h @@ -124,8 +124,6 @@ class Net::Link : public Link_list::Element { protected: - using Signal_handler = Genode::Signal_handler; - Configuration &_config; Link_side _client; Pointer const _server_port_alloc;