From 9bd91e34aaf7c759617d4763853e55f419c06ffe Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Oct 2023 18:13:55 +0900 Subject: [PATCH 1/5] network: restart dhcp4 client when renewing lease is requested but the client is stopped Follow-up for fc35a9f8d1632c4e7a279228f869bfc77d8f5b9c. Fixes the issue https://github.com/systemd/systemd/pull/29472#issuecomment-1759092138. --- src/network/networkd-link-bus.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index 06749307832..a42eb8dd63d 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -626,11 +626,15 @@ int bus_link_method_renew(sd_bus_message *message, void *userdata, sd_bus_error if (r == 0) return 1; /* Polkit will call us back */ - if (l->dhcp_client) { + if (sd_dhcp_client_is_running(l->dhcp_client)) r = sd_dhcp_client_send_renew(l->dhcp_client); - if (r < 0) - return r; - } + else + /* The DHCPv4 client may have been stopped by the IPv6 only mode. Let's unconditionally + * restart the client here. Note, if the DHCPv4 client is disabled, then dhcp4_start() does + * nothing and returns 0. */ + r = dhcp4_start(l); + if (r < 0) + return r; return sd_bus_reply_method_return(message, NULL); } From aa7336f1d382d06d72e586f71f2d6403719d3f28 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Oct 2023 18:33:52 +0900 Subject: [PATCH 2/5] test-network: add test case for renewing DHCP lease --- test/test-network/systemd-networkd-tests.py | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index f0b248917ce..7d1691c0940 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5271,6 +5271,31 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): print(f"DHCPv4 client state = {state}") self.assertEqual(state, 'stopped') + # restart dnsmasq to clear log + stop_dnsmasq() + start_dnsmasq('--dhcp-option=108,00:00:02:00') + + # Test renew command + # See https://github.com/systemd/systemd/pull/29472#issuecomment-1759092138 + check_output(*networkctl_cmd, 'renew', 'veth99', env=env) + + for _ in range(100): + state = get_dhcp4_client_state('veth99') + if state == 'stopped': + break + time.sleep(.2) + + print(f"DHCPv4 client state = {state}") + self.assertEqual(state, 'stopped') + + print('## dnsmasq log') + output = read_dnsmasq_log_file() + print(output) + self.assertIn('DHCPDISCOVER(veth-peer) 12:34:56:78:9a:bc', output) + self.assertIn('DHCPOFFER(veth-peer)', output) + self.assertNotIn('DHCPREQUEST(veth-peer)', output) + self.assertNotIn('DHCPACK(veth-peer)', output) + def test_dhcp_client_ipv6_only_with_custom_client_identifier(self): copy_network_unit('25-veth.netdev', '25-dhcp-server-veth-peer.network', '25-dhcp-client-ipv6-only-custom-client-identifier.network') From cb0e97e7de972b738bb415ff5ea7b0448c3d172c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Oct 2023 18:34:20 +0900 Subject: [PATCH 3/5] test-network: drop unnecessary explicit stop of dnsmasq --- test/test-network/systemd-networkd-tests.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 7d1691c0940..122f9e875dd 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -5319,8 +5319,6 @@ class NetworkdDHCPClientTests(unittest.TestCase, Utilities): self.assertIn('DHCPREPLY(veth-peer)', output) self.assertIn('sent size: 0 option: 14 rapid-commit', output) - stop_dnsmasq() - def test_dhcp_client_ipv4_only(self): copy_network_unit('25-veth.netdev', '25-dhcp-server-veth-peer.network', '25-dhcp-client-ipv4-only.network') From 39ba10f19e7d384ad48aaad9ff6c0b3c3e6bbef1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Oct 2023 18:35:55 +0900 Subject: [PATCH 4/5] sd-dhcp-server: make sd_dhcp_server_is_running() silently work with NULL We already do in the same way for sd-dhcp-client and friends. --- src/libsystemd-network/sd-dhcp-server.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index c69572d5b08..a2385ccb893 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -113,7 +113,8 @@ int sd_dhcp_server_configure_pool( } int sd_dhcp_server_is_running(sd_dhcp_server *server) { - assert_return(server, false); + if (!server) + return false; return !!server->receive_message; } From d311f5e277ae3609e661415b6c429fe3cd25e40b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Oct 2023 18:38:01 +0900 Subject: [PATCH 5/5] network: do not trigger assertion by forcerenew command When DHCP server is not running, sending force-renew command triggers assertion. --- src/network/networkd-link-bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index a42eb8dd63d..5fd5734ce06 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -599,7 +599,7 @@ int bus_link_method_force_renew(sd_bus_message *message, void *userdata, sd_bus_ if (r == 0) return 1; /* Polkit will call us back */ - if (l->dhcp_server) { + if (sd_dhcp_server_is_running(l->dhcp_server)) { r = sd_dhcp_server_forcerenew(l->dhcp_server); if (r < 0) return r;