From c73116b2e212598d7fd52a8a8595ed383d8b2ff5 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Wed, 26 May 2021 07:26:07 +0200 Subject: [PATCH] DHCPClient: Resend DHCP packets when we don't receive an answer Previously we'd only only send one DHCP request for network interfaces which were up when DHCPClient started. If that packet was lost we'd never send another request for those interfaces. Also, if an interface were to appear after DHCPClient started (not that that is possible at the moment) we wouldn't send requests for that interface either. --- Userland/Services/DHCPClient/DHCPv4Client.cpp | 53 +++++++++---------- Userland/Services/DHCPClient/DHCPv4Client.h | 10 ++-- Userland/Services/DHCPClient/main.cpp | 9 +--- 3 files changed, 30 insertions(+), 42 deletions(-) diff --git a/Userland/Services/DHCPClient/DHCPv4Client.cpp b/Userland/Services/DHCPClient/DHCPv4Client.cpp index 78039b2f695..f3edc2d3f2a 100644 --- a/Userland/Services/DHCPClient/DHCPv4Client.cpp +++ b/Userland/Services/DHCPClient/DHCPv4Client.cpp @@ -112,9 +112,7 @@ static void set_params(const InterfaceDescriptor& iface, const IPv4Address& ipv4 } } -DHCPv4Client::DHCPv4Client(Vector ifnames_for_immediate_discover, Vector ifnames_for_later) - : m_ifnames_for_immediate_discover(move(ifnames_for_immediate_discover)) - , m_ifnames_for_later(move(ifnames_for_later)) +DHCPv4Client::DHCPv4Client() { m_server = Core::UDPServer::construct(this); m_server->on_ready_to_receive = [this] { @@ -133,36 +131,37 @@ DHCPv4Client::DHCPv4Client(Vector ifnames_for_immediate_dis VERIFY_NOT_REACHED(); } - for (auto& iface : m_ifnames_for_immediate_discover) - dhcp_discover(iface); + m_check_timer = Core::Timer::create_repeating( + 1000, [this] { try_discover_ifs(); }, this); - m_fail_check_timer = Core::Timer::create_repeating( - 1000, [this] { try_discover_deferred_ifs(); }, this); + m_check_timer->start(); - m_fail_check_timer->start(); + try_discover_ifs(); } -void DHCPv4Client::try_discover_deferred_ifs() +void DHCPv4Client::try_discover_ifs() { - auto current_interval = m_fail_check_timer->interval(); - if (current_interval < m_max_timer_backoff_interval) - current_interval *= 1.9f; - m_fail_check_timer->set_interval(current_interval); - - if (m_ifnames_for_later.is_empty()) - return; - - // See if any of them are now up. auto ifs_result = get_discoverable_interfaces(); if (ifs_result.is_error()) return; + bool sent_discover_request = false; Interfaces& ifs = ifs_result.value(); for (auto& iface : ifs.ready) { - if (!m_ifnames_for_later.remove_first_matching([&](auto& if_) { return if_.m_ifname == iface.m_ifname; })) + if (iface.m_current_ip_address != IPv4Address { 0, 0, 0, 0 }) continue; - deferred_invoke([this, iface](auto&) { dhcp_discover(iface); }); + dhcp_discover(iface); + sent_discover_request = true; + } + + if (sent_discover_request) { + auto current_interval = m_check_timer->interval(); + if (current_interval < m_max_timer_backoff_interval) + current_interval *= 1.9f; + m_check_timer->set_interval(current_interval); + } else { + m_check_timer->set_interval(1000); } } @@ -192,12 +191,14 @@ Result DHCPv4Client::get_discoverable_interfac auto name = if_object.get("name").to_string(); auto mac = if_object.get("mac_address").to_string(); auto is_up = if_object.get("link_up").to_bool(); + auto ipv4_addr_maybe = IPv4Address::from_string(if_object.get("ipv4_address").to_string()); + auto ipv4_addr = ipv4_addr_maybe.has_value() ? ipv4_addr_maybe.value() : IPv4Address { 0, 0, 0, 0 }; if (is_up) { dbgln_if(DHCPV4_DEBUG, "Found adapter '{}' with mac {}, and it was up!", name, mac); - ifnames_to_immediately_discover.empend(name, mac_from_string(mac)); + ifnames_to_immediately_discover.empend(name, mac_from_string(mac), ipv4_addr); } else { dbgln_if(DHCPV4_DEBUG, "Found adapter '{}' with mac {}, but it was down", name, mac); - ifnames_to_attempt_later.empend(name, mac_from_string(mac)); + ifnames_to_attempt_later.empend(name, mac_from_string(mac), ipv4_addr); } }); @@ -344,10 +345,8 @@ void DHCPv4Client::dhcp_discover(const InterfaceDescriptor& iface) auto& dhcp_packet = builder.build(); // broadcast the discover request - if (!send(iface, dhcp_packet, this)) { - m_ifnames_for_later.append(iface); + if (!send(iface, dhcp_packet, this)) return; - } m_ongoing_transactions.set(transaction_id, make(iface)); } @@ -373,9 +372,7 @@ void DHCPv4Client::dhcp_request(DHCPv4Transaction& transaction, const DHCPv4Pack auto& dhcp_packet = builder.build(); // broadcast the "request" request - if (!send(iface, dhcp_packet, this)) { - m_ifnames_for_later.append(iface); + if (!send(iface, dhcp_packet, this)) return; - } transaction.accepted_offer = true; } diff --git a/Userland/Services/DHCPClient/DHCPv4Client.h b/Userland/Services/DHCPClient/DHCPv4Client.h index f7fcf1ea65d..72af9d2d331 100644 --- a/Userland/Services/DHCPClient/DHCPv4Client.h +++ b/Userland/Services/DHCPClient/DHCPv4Client.h @@ -21,7 +21,7 @@ struct InterfaceDescriptor { String m_ifname; MACAddress m_mac_address; - IPv4Address m_current_ip_address { 0, 0, 0, 0 }; + IPv4Address m_current_ip_address; }; struct DHCPv4Transaction { @@ -40,7 +40,7 @@ class DHCPv4Client final : public Core::Object { C_OBJECT(DHCPv4Client) public: - explicit DHCPv4Client(Vector ifnames_for_immediate_discover, Vector ifnames_for_later); + explicit DHCPv4Client(); virtual ~DHCPv4Client() override; void dhcp_discover(const InterfaceDescriptor& ifname); @@ -57,13 +57,11 @@ public: static Result get_discoverable_interfaces(); private: - void try_discover_deferred_ifs(); + void try_discover_ifs(); HashMap> m_ongoing_transactions; - Vector m_ifnames_for_immediate_discover; - Vector m_ifnames_for_later; RefPtr m_server; - RefPtr m_fail_check_timer; + RefPtr m_check_timer; int m_max_timer_backoff_interval { 600000 }; // 10 minutes void handle_offer(const DHCPv4Packet&, const ParsedDHCPv4Options&); diff --git a/Userland/Services/DHCPClient/main.cpp b/Userland/Services/DHCPClient/main.cpp index a150b07e4a7..6d55e675b17 100644 --- a/Userland/Services/DHCPClient/main.cpp +++ b/Userland/Services/DHCPClient/main.cpp @@ -34,14 +34,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) unveil(nullptr, nullptr); - auto ifs_result = DHCPv4Client::get_discoverable_interfaces(); - if (ifs_result.is_error()) { - warnln("Error: {}", ifs_result.error()); - return 1; - } - - auto ifs = ifs_result.release_value(); - auto client = DHCPv4Client::construct(move(ifs.ready), move(ifs.not_ready)); + auto client = DHCPv4Client::construct(); if (pledge("stdio inet cpath rpath", nullptr) < 0) { perror("pledge");