From ba348b73e24ee00969e531e0346d4ce96b4386e9 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 16 May 2018 13:55:13 +0200 Subject: [PATCH] nic_router: re-use dynamic IPv4 config if possible When re-configuring the NIC router, determine for each domain if at least one interface stays with the domain. If a domain fullfills this and has a dynamic IP config (received via a DHCP client), keep the IP config. To achieve this, the following changes have been made to the existing NIC router code: * Split-up Interface::handle_config into three steps: 1) Determine for each interface if its domain can keep its IP config or or if it has to mark it invalid. This must be done before (re-)attaching any interface because during "attach" several decisions are made based on the validity of the IP config of corresponding the domain. (E.g. whether to participate in sending DHCP DISCOVERs {IP config invalid} or whether to participate in sending pending ARP REQUESTs {IP config valid} ). 2) Detach, attach, or re-attach each interface according to the configuration. This must be done before re-considering the temporary state objects of each interface because the latter might have effects on the interfaces of remote domains which must then be in place already. 3) Re-consider temporary state objects of each interface. (E.g. transport layer connection states) * Re-work IP-config setter in a way that it works as follows: 1) If the old IP config is valid, let all local interfaces as well as remote interfaces that depend on the IP config of the domain detach from the old IP config. 2) Overwrite with new IP config 3) If the new IP config is valid, let all local interfaces as well as remote interfaces that depend on the IP config of the domain attach to the new IP config. Issue #2815 --- repos/os/src/server/nic_router/domain.cc | 108 ++++++++++------ repos/os/src/server/nic_router/domain.h | 7 +- repos/os/src/server/nic_router/interface.cc | 130 ++++++++++++++------ repos/os/src/server/nic_router/interface.h | 28 +++-- repos/os/src/server/nic_router/main.cc | 18 +-- 5 files changed, 195 insertions(+), 96 deletions(-) diff --git a/repos/os/src/server/nic_router/domain.cc b/repos/os/src/server/nic_router/domain.cc index 78b48c3791..3e88fd863f 100644 --- a/repos/os/src/server/nic_router/domain.cc +++ b/repos/os/src/server/nic_router/domain.cc @@ -42,28 +42,82 @@ Domain_avl_member::Domain_avl_member(Domain_name const &name, *****************/ Domain_base::Domain_base(Xml_node const node) -: _name(node.attribute_value("name", Domain_name())) { } +: + _name(node.attribute_value("name", Domain_name())) +{ } /************ ** Domain ** ************/ -void Domain::ip_config(Ipv4_address ip, - Ipv4_address subnet_mask, - Ipv4_address gateway, - Ipv4_address dns_server) +void Domain::ip_config(Ipv4_config const &new_ip_config) { - _ip_config.construct(Ipv4_address_prefix(ip, subnet_mask), gateway, - dns_server); - _ip_config_changed(); + /* discard old IP config if any */ + if (ip_config().valid) { + + /* mark IP config invalid */ + _ip_config.construct(); + + /* detach all dependent interfaces from old IP config */ + _interfaces.for_each([&] (Interface &interface) { + interface.detach_from_ip_config(); + }); + _ip_config_dependents.for_each([&] (Domain &domain) { + domain._interfaces.for_each([&] (Interface &interface) { + interface.detach_from_remote_ip_config(); + }); + }); + } + /* overwrite old with new IP config */ + _ip_config.construct(new_ip_config); + + /* log the event */ + if (_config.verbose_domain_state()) { + log("[", *this, "] IP config: ", ip_config()); } + + /* attach all dependent interfaces to new IP config if it is valid */ + if (ip_config().valid) { + _interfaces.for_each([&] (Interface &interface) { + interface.attach_to_ip_config(*this, ip_config()); + }); + _ip_config_dependents.for_each([&] (Domain &domain) { + domain._interfaces.for_each([&] (Interface &interface) { + interface.attach_to_remote_ip_config(); + }); + }); + } } void Domain::discard_ip_config() { - _ip_config.construct(); - _ip_config_changed(); + /* install invalid IP config */ + ip_config(Ipv4_address(), Ipv4_address(), Ipv4_address(), Ipv4_address()); +} + + +void Domain::ip_config(Ipv4_address ip, + Ipv4_address subnet_mask, + Ipv4_address gateway, + Ipv4_address dns_server) +{ + Ipv4_config const new_ip_config(Ipv4_address_prefix(ip, subnet_mask), + gateway, dns_server); + ip_config(new_ip_config); +} + + +void Domain::try_reuse_ip_config(Domain const &domain) +{ + if (ip_config().valid || + !_ip_config_dynamic || + !domain.ip_config().valid || + !domain._ip_config_dynamic) + { + return; + } + ip_config(domain.ip_config()); } @@ -122,10 +176,6 @@ Domain::Domain(Configuration &config, Xml_node const node, Allocator &alloc) log("[?] Missing name attribute in domain node"); throw Invalid(); } - if (!ip_config().valid && _node.has_sub_node("dhcp-server")) { - log("[", *this, "] cannot act as DHCP client and server at once"); - throw Invalid(); - } if (_config.verbose_domain_state()) { log("[", *this, "] NIC sessions: ", _interface_cnt); } @@ -142,31 +192,6 @@ Link_side_tree &Domain::links(L3_protocol const protocol) } -void Domain::_ip_config_changed() -{ - /* detach all dependent interfaces from old IP config */ - _interfaces.for_each([&] (Interface &interface) { - interface.detach_from_ip_config(); - }); - _ip_config_dependents.for_each([&] (Domain &domain) { - domain._interfaces.for_each([&] (Interface &interface) { - interface.detach_from_remote_ip_config(); - }); - }); - /* log the change */ - if (_config.verbose_domain_state()) { - if (!ip_config().valid) { - log("[", *this, "] IP config: none"); - } else { - log("[", *this, "] IP config:" - " interface ", ip_config().interface, - ", gateway ", ip_config().gateway, - ", DNS server ", ip_config().dns_server); - } - } -} - - Domain::~Domain() { /* destroy rules */ @@ -210,7 +235,7 @@ void Domain::init(Domain_tree &domains) /* read DHCP server configuration */ try { Xml_node const dhcp_server_node = _node.sub_node("dhcp-server"); - if (!ip_config().valid) { + if (_ip_config_dynamic) { log("[", *this, "] cannot be DHCP server and client at the same time"); throw Invalid(); } @@ -282,6 +307,9 @@ void Domain::detach_interface(Interface &interface) { _interfaces.remove(&interface); _interface_cnt--; + if (!_interface_cnt && _ip_config_dynamic) { + discard_ip_config(); + } if (_config.verbose_domain_state()) { log("[", *this, "] NIC sessions: ", _interface_cnt); } diff --git a/repos/os/src/server/nic_router/domain.h b/repos/os/src/server/nic_router/domain.h index 005fcf5ef3..2b1b6f5a53 100644 --- a/repos/os/src/server/nic_router/domain.h +++ b/repos/os/src/server/nic_router/domain.h @@ -101,6 +101,7 @@ class Net::Domain : public Domain_base, unsigned long _interface_cnt { 0 }; Pointer _dhcp_server { }; Genode::Reconstructible _ip_config; + bool const _ip_config_dynamic { !ip_config().valid }; List _ip_config_dependents { }; Arp_cache _arp_cache { *this }; Arp_waiter_list _foreign_arp_waiters { }; @@ -124,8 +125,6 @@ class Net::Domain : public Domain_base, char const *type, Transport_rule_list &rules); - void _ip_config_changed(); - void __FIXME__dissolve_foreign_arp_waiters(); public: @@ -143,6 +142,8 @@ class Net::Domain : public Domain_base, Ipv4_address const &next_hop(Ipv4_address const &ip) const; + void ip_config(Ipv4_config const &ip_config); + void ip_config(Ipv4_address ip, Ipv4_address subnet_mask, Ipv4_address gateway, @@ -150,6 +151,8 @@ class Net::Domain : public Domain_base, void discard_ip_config(); + void try_reuse_ip_config(Domain const &domain); + Link_side_tree &links(L3_protocol const protocol); void attach_interface(Interface &interface); diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index 6f1427dad9..4bde1ecc8b 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -253,27 +253,32 @@ void Interface::_detach_from_domain_raw() } -void Interface::_attach_to_domain(Domain_name const &domain_name, - bool apply_foreign_arp) +void Interface::_attach_to_domain(Domain_name const &domain_name) { _attach_to_domain_raw(domain_name); - - /* ensure that DHCP requests are sent on each interface of the domain */ - if (!domain().ip_config().valid) { - _dhcp_client.discover(); - } - /* ensure that DHCP requests are sent on each interface of the domain */ - if ( apply_foreign_arp) { _apply_foreign_arp(); } - else { _apply_foreign_arp_pending = true; } + _attach_to_domain_finish(); } -void Interface::_apply_foreign_arp() +void Interface::_attach_to_domain_finish() { - /* sent ARP request for each foreign ARP waiter */ - Domain &domain_ = domain(); - domain_.foreign_arp_waiters().for_each([&] (Arp_waiter_list_element &le) { - _broadcast_arp_request(domain_.ip_config().interface.address, + /* if domain has yet no IP config, participate in requesting one */ + Domain &domain = _domain(); + Ipv4_config const &ip_config = domain.ip_config(); + if (!ip_config.valid) { + _dhcp_client.discover(); + return; + } + attach_to_ip_config(domain, ip_config); +} + + +void Interface::attach_to_ip_config(Domain &domain, + Ipv4_config const &ip_config) +{ + /* if others wait for ARP at the domain, participate in requesting it */ + domain.foreign_arp_waiters().for_each([&] (Arp_waiter_list_element &le) { + _broadcast_arp_request(ip_config.interface.address, le.object()->ip()); }); } @@ -296,16 +301,6 @@ void Interface::link_state_sigh(Signal_context_capability sigh) } -void Interface::handle_config_aftermath() -{ - /* ensure that ARP requests are sent on each interface of the domain */ - if (_apply_foreign_arp_pending) { - _apply_foreign_arp(); - _apply_foreign_arp_pending = false; - } -} - - void Interface::detach_from_ip_config() { /* destroy our own ARP waiters */ @@ -336,12 +331,18 @@ void Interface::detach_from_remote_ip_config() } +void Interface::attach_to_remote_ip_config() +{ + /* only the DNS server address of the local DHCP server can be remote */ + Signal_transmitter(_link_state_sigh).submit(); +} + + void Interface::_detach_from_domain() { try { detach_from_ip_config(); _detach_from_domain_raw(); - _apply_foreign_arp_pending = false; } catch (Pointer::Invalid) { } } @@ -1349,7 +1350,7 @@ Interface::Interface(Genode::Entrypoint &ep, void Interface::init() { - try { _attach_to_domain(_policy.determine_domain_name(), true); } + try { _attach_to_domain(_policy.determine_domain_name()); } catch (Domain_tree::No_match) { } } @@ -1541,9 +1542,9 @@ void Interface::_update_own_arp_waiters(Domain &domain) } -void Interface::handle_config(Configuration &config) +void Interface::handle_config_1(Configuration &config) { - /* deteremine the name of the new domain */ + /* update config and policy */ _config = config; _policy.handle_config(config); Domain_name const &new_domain_name = _policy.determine_domain_name(); @@ -1555,10 +1556,29 @@ void Interface::handle_config(Configuration &config) _destroy_dissolved_links (_dissolved_tcp_links, _alloc); _destroy_released_dhcp_allocations(old_domain); + /* do not consider to reuse IP config if the domains differ */ + if (old_domain.name() != new_domain_name) { + return; } + + /* interface stays with its domain, so, try to reuse IP config */ + Domain &new_domain = config.domains().find_by_name(new_domain_name); + new_domain.try_reuse_ip_config(old_domain); + return; + } + catch (Domain_tree::No_match) { } + catch (Pointer::Invalid) { } +} + + +void Interface::handle_config_2() +{ + Domain_name const &new_domain_name = _policy.determine_domain_name(); + try { /* if the domains differ, detach completely from the domain */ + Domain &old_domain = domain(); if (old_domain.name() != new_domain_name) { _detach_from_domain(); - _attach_to_domain(new_domain_name, false); + _attach_to_domain_raw(new_domain_name); return; } /* move to new domain object without considering any state objects */ @@ -1566,6 +1586,43 @@ void Interface::handle_config(Configuration &config) _attach_to_domain_raw(new_domain_name); Domain &new_domain = domain(); + /* remember that the interface stays attached to the same domain */ + _update_domain = *new (_alloc) + Update_domain { old_domain, new_domain }; + + return; + } + catch (Domain_tree::No_match) { + + /* the interface no longer has a domain */ + _detach_from_domain(); + } + catch (Pointer::Invalid) { + + /* the interface had no domain but now it may get one */ + try { + _attach_to_domain_raw(new_domain_name); + } + catch (Domain_tree::No_match) { } + } +} + + +void Interface::handle_config_3() +{ + try { + /* + * Update the domain object only if handle_config_2 determined that + * the interface stays attached to the same domain. Otherwise the + * interface already got detached from its old domain and there is + * nothing to update. + */ + Update_domain &update_domain = _update_domain(); + Domain &old_domain = update_domain.old_domain; + Domain &new_domain = update_domain.new_domain; + destroy(_alloc, &update_domain); + _update_domain = Pointer(); + /* if the IP configs differ, detach completely from the IP config */ if (old_domain.ip_config() != new_domain.ip_config()) { detach_from_ip_config(); @@ -1582,16 +1639,11 @@ void Interface::handle_config(Configuration &config) _update_dhcp_allocations(old_domain, new_domain); _update_own_arp_waiters(new_domain); } - catch (Domain_tree::No_match) { + catch (Pointer::Invalid) { - /* the interface no longer has a domain */ - _detach_from_domain(); - } - catch (Pointer::Invalid) { - - /* the interface had no domain but now it may get one */ - try { _attach_to_domain(new_domain_name, false); } - catch (Domain_tree::No_match) { } + /* if the interface moved to another domain, finish the operation */ + try { _attach_to_domain_finish(); } + catch (Pointer::Invalid) { } } } diff --git a/repos/os/src/server/nic_router/interface.h b/repos/os/src/server/nic_router/interface.h index 0aa91543de..76d627a960 100644 --- a/repos/os/src/server/nic_router/interface.h +++ b/repos/os/src/server/nic_router/interface.h @@ -81,6 +81,12 @@ class Net::Interface : private Interface_list::Element struct Dismiss_link : Genode::Exception { }; struct Dismiss_arp_waiter : Genode::Exception { }; + struct Update_domain + { + Domain &old_domain; + Domain &new_domain; + }; + Reference _config; Interface_policy &_policy; Timer::Connection &_timer; @@ -97,8 +103,8 @@ class Net::Interface : private Interface_list::Element Dhcp_allocation_list _released_dhcp_allocations { }; Dhcp_client _dhcp_client { _alloc, _timer, *this }; Interface_list &_interfaces; - bool _apply_foreign_arp_pending { false }; Genode::Signal_context_capability _link_state_sigh { }; + Pointer _update_domain { }; void _new_link(L3_protocol const protocol, Link_side_id const &local_id, @@ -263,12 +269,13 @@ class Net::Interface : private Interface_list::Element void _detach_from_domain_raw(); - void _attach_to_domain_raw(Domain_name const &domain_name); - void _detach_from_domain(); - void _attach_to_domain(Domain_name const &domain_name, - bool apply_foreign_arp); + void _attach_to_domain(Domain_name const &domain_name); + + void _attach_to_domain_raw(Domain_name const &domain_name); + + void _attach_to_domain_finish(); void _apply_foreign_arp(); @@ -351,14 +358,21 @@ class Net::Interface : private Interface_list::Element void cancel_arp_waiting(Arp_waiter &waiter); - void handle_config(Configuration &new_config); + void handle_config_1(Configuration &config); - void handle_config_aftermath(); + void handle_config_2(); + + void handle_config_3(); void detach_from_ip_config(); + void attach_to_ip_config(Domain &domain, + Ipv4_config const &ip_config); + void detach_from_remote_ip_config(); + void attach_to_remote_ip_config(); + bool link_state(); void link_state_sigh(Genode::Signal_context_capability sigh); diff --git a/repos/os/src/server/nic_router/main.cc b/repos/os/src/server/nic_router/main.cc index a0b1840e72..2677c24679 100644 --- a/repos/os/src/server/nic_router/main.cc +++ b/repos/os/src/server/nic_router/main.cc @@ -143,16 +143,18 @@ void Net::Main::_uplink_handle_config(Configuration &config) void Net::Main::_handle_config() { _config_rom.update(); - Configuration &config = *new (_heap) - Configuration(_env, _config_rom.xml(), _heap, _timer, _config()); + Configuration &old_config = _config(); + Configuration &new_config = *new (_heap) + Configuration(_env, _config_rom.xml(), _heap, _timer, old_config); - _uplink_handle_config(config); - _root.handle_config(config); - _for_each_interface([&] (Interface &intf) { intf.handle_config(config); }); - _for_each_interface([&] (Interface &intf) { intf.handle_config_aftermath(); }); + _uplink_handle_config(new_config); + _root.handle_config(new_config); + _for_each_interface([&] (Interface &intf) { intf.handle_config_1(new_config); }); + _for_each_interface([&] (Interface &intf) { intf.handle_config_2(); }); + _config = Reference(new_config); + _for_each_interface([&] (Interface &intf) { intf.handle_config_3(); }); - destroy(_heap, &_config()); - _config = Reference(config); + destroy(_heap, &old_config); }