From d87a235abb0d3c27e2e7f25e241d4b5d067deaaa Mon Sep 17 00:00:00 2001 From: Johannes Schlatow Date: Wed, 16 Oct 2024 13:23:02 +0200 Subject: [PATCH] nic_router: fix DHCP deallocation on domain update Commit ac42ade introduced a regression that triggered an assertion in `Dhcp_server::free_ip()` because the DHCP allocation was not properly removed during a domain update. The underlying issue was that `with_dhcp_server()` silently landed in the `no_dhcp_server_fn`. Fixes #5364 --- repos/os/src/server/nic_router/domain.h | 2 +- repos/os/src/server/nic_router/interface.cc | 34 +++++++++++++-------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/repos/os/src/server/nic_router/domain.h b/repos/os/src/server/nic_router/domain.h index 1067d6c9e9..7bc4be1baa 100644 --- a/repos/os/src/server/nic_router/domain.h +++ b/repos/os/src/server/nic_router/domain.h @@ -232,7 +232,7 @@ class Net::Domain : public List::Element, dhcp_server_fn(*_dhcp_server_ptr); } - void with_dhcp_server(auto const &fn) { with_dhcp_server(fn, []{}); } + void with_optional_dhcp_server(auto const &fn) { with_dhcp_server(fn, []{}); } /********* ** log ** diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index ce58bc84f3..7247865402 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -1734,7 +1734,7 @@ void Interface::_continue_handle_eth(Packet_descriptor const &pkt) void Interface::_destroy_dhcp_allocation(Dhcp_allocation &allocation, Domain &local_domain) { - local_domain.with_dhcp_server([&] (Dhcp_server &srv) { + local_domain.with_optional_dhcp_server([&] (Dhcp_server &srv) { srv.free_ip(allocation.ip()); }); destroy(_alloc, &allocation); } @@ -2035,6 +2035,18 @@ void Interface::_update_dhcp_allocations(Domain &old_domain, Domain &new_domain) { bool dhcp_clients_outdated { false }; + + auto dismiss_all_fn = [&] () { + dhcp_clients_outdated = true; + while (Dhcp_allocation *allocation = _dhcp_allocations.first()) { + if (_config_ptr->verbose()) + log("[", new_domain, "] dismiss DHCP allocation: ", + *allocation, " (other/no DHCP server)"); + _dhcp_allocations.remove(*allocation); + _destroy_dhcp_allocation(*allocation, old_domain); + } + }; + old_domain.with_dhcp_server([&] (Dhcp_server &old_dhcp_srv) { new_domain.with_dhcp_server([&] (Dhcp_server &new_dhcp_srv) { if (old_dhcp_srv.config_equal_to_that_of(new_dhcp_srv)) { @@ -2054,18 +2066,16 @@ void Interface::_update_dhcp_allocations(Domain &old_domain, }); } else { /* dismiss all DHCP allocations */ - dhcp_clients_outdated = true; - while (Dhcp_allocation *allocation = _dhcp_allocations.first()) { - if (_config_ptr->verbose()) - log("[", new_domain, "] dismiss DHCP allocation: ", - *allocation, " (other/no DHCP server)"); - _dhcp_allocations.remove(*allocation); - _destroy_dhcp_allocation(*allocation, old_domain); - } + dismiss_all_fn(); } - if (dhcp_clients_outdated) - _reset_and_refetch_domain_ready_state(); - });}); + }, + /* no DHCP server on new domain */ + [&] () { dismiss_all_fn(); }); }, + /* no DHCP server on old domain */ + [&] () { dismiss_all_fn(); }); + + if (dhcp_clients_outdated) + _reset_and_refetch_domain_ready_state(); }