Merge pull request systemd#29119 from yuwata/network-dhcp-server-allo…

…w-null-server-address

network/dhcp-server: allow null server address

man/systemd.network.xml

@@ -3277,12 +3277,43 @@ Token=prefixstable:2002:da8:1::</programlisting></para>
 
       <varlistentry>
         <term><varname>ServerAddress=</varname></term>
-        <listitem><para>Specifies server address for the DHCP server. Takes an IPv4 address with prefix
-        length, for example 192.168.0.1/24. This setting may be useful when the link on
-        which the DHCP server is running has multiple static addresses. When unset, one of static addresses
-        in the link will be automatically selected. Defaults to unset.</para>
+        <listitem>
+          <para>Specifies the server address for the DHCP server. Takes an IPv4 address with prefix length
+          separated with a slash, e.g. <literal>192.168.0.1/24</literal>. Defaults to unset, and one of
+          static IPv4 addresses configured in [Network] or [Address] section will be automatically selected.
+          This setting may be useful when the interface on which the DHCP server is running has multiple
+          static IPv4 addresses.</para>
+          <para>This implies <varname>Address=</varname> in [Network] or [Address] section with the same
+          address and prefix length. That is,
+          <programlisting>[Network]
+DHCPServer=yes
+Address=192.168.0.1/24
+Address=192.168.0.2/24
+[DHCPServer]
+ServerAddress=192.168.0.1/24</programlisting>
+          or
+          <programlisting>[Network]
+DHCPServer=yes
+[Address]
+Address=192.168.0.1/24
+[Address]
+Address=192.168.0.2/24
+[DHCPServer]
+ServerAddress=192.168.0.1/24</programlisting>
+          are equivalent to the following.
+          <programlisting>[Network]
+DHCPServer=yes
+Address=192.168.0.2/24
+[DHCPServer]
+ServerAddress=192.168.0.1/24</programlisting>
+          </para>
+          <para>Since version 255, like the <varname>Address=</varname> setting in [Network] or [Address]
+          section, this also supports a null address, e.g. <literal>0.0.0.0/24</literal>, and an unused
+          address will be automatically selected. For more details about the automatic address selection,
+          see <varname>Address=</varname> setting in [Network] section in the above.</para>
 
-        <xi:include href="version-info.xml" xpointer="v249"/></listitem>
+          <xi:include href="version-info.xml" xpointer="v249"/>
+        </listitem>
       </varlistentry>
 
       <varlistentry>

src/network/networkd-address.c

@@ -110,7 +110,7 @@ int address_new(Address **ret) {
         return 0;
 }
 
-static int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret) {
+int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret) {
         _cleanup_(config_section_freep) ConfigSection *n = NULL;
         _cleanup_(address_freep) Address *address = NULL;
         int r;
@@ -401,7 +401,7 @@ static int address_compare_func(const Address *a1, const Address *a2) {
         }
 }
 
-DEFINE_PRIVATE_HASH_OPS(
+DEFINE_HASH_OPS(
         address_hash_ops,
         Address,
         address_hash_func,
@@ -1561,10 +1561,6 @@ int link_request_static_addresses(Link *link) {
         if (r < 0)
                 return r;
 
-        r = link_request_dhcp_server_address(link);
-        if (r < 0)
-                return r;
-
         if (link->static_address_messages == 0) {
                 link->static_addresses_configured = true;
                 link_check_ready(link);
@@ -2334,7 +2330,7 @@ static void address_section_adjust_broadcast(Address *address) {
         address->broadcast.s_addr = 0;
 }
 
-static int address_section_verify(Address *address) {
+int address_section_verify(Address *address) {
         if (section_is_invalid(address->section))
                 return -EINVAL;
 
@@ -2444,6 +2440,10 @@ int network_drop_invalid_addresses(Network *network) {
                 assert(r > 0);
         }
 
+        r = network_adjust_dhcp_server(network, &addresses);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 

src/network/networkd-address.h

@@ -7,6 +7,7 @@
 
 #include "conf-parser.h"
 #include "firewall-util.h"
+#include "hash-funcs.h"
 #include "in-addr-util.h"
 #include "networkd-link.h"
 #include "networkd-util.h"
@@ -73,7 +74,10 @@ const char* format_lifetime(char *buf, size_t l, usec_t lifetime_usec) _warn_unu
 
 int address_flags_to_string_alloc(uint32_t flags, int family, char **ret);
 
+extern const struct hash_ops address_hash_ops;
+
 int address_new(Address **ret);
+int address_new_static(Network *network, const char *filename, unsigned section_line, Address **ret);
 Address* address_free(Address *address);
 int address_get(Link *link, const Address *in, Address **ret);
 int address_get_harder(Link *link, const Address *in, Address **ret);
@@ -115,6 +119,7 @@ int link_request_static_addresses(Link *link);
 
 int manager_rtnl_process_address(sd_netlink *nl, sd_netlink_message *message, Manager *m);
 
+int address_section_verify(Address *address);
 int network_drop_invalid_addresses(Network *network);
 
 DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(Address, address);

src/network/networkd-dhcp-server.c

@@ -38,26 +38,31 @@ static bool link_dhcp4_server_enabled(Link *link) {
         return link->network->dhcp_server;
 }
 
-void network_adjust_dhcp_server(Network *network) {
+int network_adjust_dhcp_server(Network *network, Set **addresses) {
+        int r;
+
         assert(network);
+        assert(addresses);
 
         if (!network->dhcp_server)
-                return;
+                return 0;
 
         if (network->bond) {
                 log_warning("%s: DHCPServer= is enabled for bond slave. Disabling DHCP server.",
                             network->filename);
                 network->dhcp_server = false;
-                return;
+                return 0;
         }
 
-        if (!in4_addr_is_set(&network->dhcp_server_address)) {
+        assert(network->dhcp_server_address_prefixlen <= 32);
+
+        if (network->dhcp_server_address_prefixlen == 0) {
                 Address *address;
-                bool have = false;
+
+                /* If the server address is not specified, then find suitable static address. */
 
                 ORDERED_HASHMAP_FOREACH(address, network->addresses_by_section) {
-                        if (section_is_invalid(address->section))
-                                continue;
+                        assert(!section_is_invalid(address->section));
 
                         if (address->family != AF_INET)
                                 continue;
@@ -71,83 +76,69 @@ void network_adjust_dhcp_server(Network *network) {
                         if (in4_addr_is_set(&address->in_addr_peer.in))
                                 continue;
 
-                        have = true;
+                        /* TODO: check if the prefix length is small enough for the pool. */
+
+                        network->dhcp_server_address = address;
                         break;
                 }
-                if (!have) {
-                        log_warning("%s: DHCPServer= is enabled, but no static address configured. "
+                if (!network->dhcp_server_address) {
+                        log_warning("%s: DHCPServer= is enabled, but no suitable static address configured. "
                                     "Disabling DHCP server.",
                                     network->filename);
                         network->dhcp_server = false;
-                        return;
+                        return 0;
                 }
-        }
-}
-
-int link_request_dhcp_server_address(Link *link) {
-        _cleanup_(address_freep) Address *address = NULL;
-        Address *existing;
-        int r;
-
-        assert(link);
-        assert(link->network);
-
-        if (!link_dhcp4_server_enabled(link))
-                return 0;
-
-        if (!in4_addr_is_set(&link->network->dhcp_server_address))
-                return 0;
-
-        r = address_new(&address);
-        if (r < 0)
-                return r;
 
-        address->source = NETWORK_CONFIG_SOURCE_STATIC;
-        address->family = AF_INET;
-        address->in_addr.in = link->network->dhcp_server_address;
-        address->prefixlen = link->network->dhcp_server_address_prefixlen;
-
-        if (address_get_harder(link, address, &existing) >= 0 &&
-            (address_exists(existing) || address_is_requesting(existing)) &&
-            existing->source == NETWORK_CONFIG_SOURCE_STATIC)
-                /* The same address seems explicitly configured in [Address] or [Network] section.
-                 * Configure the DHCP server address only when it is not. */
-                return 0;
+        } else {
+                _cleanup_(address_freep) Address *a = NULL;
+                Address *existing;
+                unsigned line;
+
+                /* TODO: check if the prefix length is small enough for the pool. */
+
+                /* If an address is explicitly specified, then check if the corresponding [Address] section
+                 * is configured, and add one if not. */
+
+                existing = set_get(*addresses,
+                                   &(Address) {
+                                           .family = AF_INET,
+                                           .in_addr.in = network->dhcp_server_address_in_addr,
+                                           .prefixlen = network->dhcp_server_address_prefixlen,
+                                   });
+                if (existing) {
+                        /* Corresponding [Address] section already exists. */
+                        network->dhcp_server_address = existing;
+                        return 0;
+                }
 
-        return link_request_static_address(link, address);
-}
+                r = ordered_hashmap_by_section_find_unused_line(network->addresses_by_section, network->filename, &line);
+                if (r < 0)
+                        return log_warning_errno(r, "%s: Failed to find unused line number for DHCP server address: %m",
+                                                 network->filename);
 
-static int link_find_dhcp_server_address(Link *link, Address **ret) {
-        Address *address;
+                r = address_new_static(network, network->filename, line, &a);
+                if (r < 0)
+                        return log_warning_errno(r, "%s: Failed to add new static address object for DHCP server: %m",
+                                                 network->filename);
 
-        assert(link);
-        assert(link->network);
+                a->family = AF_INET;
+                a->prefixlen = network->dhcp_server_address_prefixlen;
+                a->in_addr.in = network->dhcp_server_address_in_addr;
+                a->requested_as_null = !in4_addr_is_set(&network->dhcp_server_address_in_addr);
 
-        /* If ServerAddress= is specified, then use the address. */
-        if (in4_addr_is_set(&link->network->dhcp_server_address))
-                return link_get_ipv4_address(link, &link->network->dhcp_server_address,
-                                             link->network->dhcp_server_address_prefixlen, ret);
+                r = address_section_verify(a);
+                if (r < 0)
+                        return r;
 
-        /* If not, then select one from static addresses. */
-        SET_FOREACH(address, link->addresses) {
-                if (address->source != NETWORK_CONFIG_SOURCE_STATIC)
-                        continue;
-                if (!address_exists(address))
-                        continue;
-                if (address->family != AF_INET)
-                        continue;
-                if (in4_addr_is_localhost(&address->in_addr.in))
-                        continue;
-                if (in4_addr_is_link_local(&address->in_addr.in))
-                        continue;
-                if (in4_addr_is_set(&address->in_addr_peer.in))
-                        continue;
+                r = set_ensure_put(addresses, &address_hash_ops, a);
+                if (r < 0)
+                        return log_oom();
+                assert(r > 0);
 
-                *ret = address;
-                return 0;
+                network->dhcp_server_address = TAKE_PTR(a);
         }
 
-        return -ENOENT;
+        return 0;
 }
 
 static int dhcp_server_find_uplink(Link *link, Link **ret) {
@@ -369,6 +360,8 @@ static int dhcp4_server_configure(Link *link) {
         int r;
 
         assert(link);
+        assert(link->network);
+        assert(link->network->dhcp_server_address);
 
         log_link_debug(link, "Configuring DHCP Server.");
 
@@ -387,7 +380,7 @@ static int dhcp4_server_configure(Link *link) {
         if (r < 0)
                 return log_link_warning_errno(link, r, "Failed to set callback for DHCPv4 server instance: %m");
 
-        r = link_find_dhcp_server_address(link, &address);
+        r = address_get(link, link->network->dhcp_server_address, &address);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to find suitable address for DHCPv4 server instance: %m");
 
@@ -535,6 +528,8 @@ static bool dhcp_server_is_ready_to_configure(Link *link) {
         Address *a;
 
         assert(link);
+        assert(link->network);
+        assert(link->network->dhcp_server_address);
 
         if (!link_is_ready_to_configure(link, /* allow_unmanaged = */ false))
                 return false;
@@ -545,7 +540,7 @@ static bool dhcp_server_is_ready_to_configure(Link *link) {
         if (!link->static_addresses_configured)
                 return false;
 
-        if (link_find_dhcp_server_address(link, &a) < 0)
+        if (address_get(link, link->network->dhcp_server_address, &a) < 0)
                 return false;
 
         if (!address_is_ready(a))
@@ -711,7 +706,7 @@ int config_parse_dhcp_server_address(
         assert(rvalue);
 
         if (isempty(rvalue)) {
-                network->dhcp_server_address = (struct in_addr) {};
+                network->dhcp_server_address_in_addr = (struct in_addr) {};
                 network->dhcp_server_address_prefixlen = 0;
                 return 0;
         }
@@ -722,14 +717,14 @@ int config_parse_dhcp_server_address(
                            "Failed to parse %s=, ignoring assignment: %s", lvalue, rvalue);
                 return 0;
         }
-        if (in4_addr_is_null(&a.in) || in4_addr_is_localhost(&a.in)) {
+        if (in4_addr_is_localhost(&a.in) || in4_addr_is_link_local(&a.in)) {
                 log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "DHCP server address cannot be the ANY address or a localhost address, "
+                           "DHCP server address cannot be a localhost or link-local address, "
                            "ignoring assignment: %s", rvalue);
                 return 0;
         }
 
-        network->dhcp_server_address = a.in;
+        network->dhcp_server_address_in_addr = a.in;
         network->dhcp_server_address_prefixlen = prefixlen;
         return 0;
 }

src/network/networkd-dhcp-server.h

@@ -2,13 +2,13 @@
 #pragma once
 
 #include "conf-parser.h"
+#include "set.h"
 
 typedef struct Link Link;
 typedef struct Network Network;
 
-void network_adjust_dhcp_server(Network *network);
+int network_adjust_dhcp_server(Network *network, Set **addresses);
 
-int link_request_dhcp_server_address(Link *link);
 int link_request_dhcp_server(Link *link);
 
 CONFIG_PARSER_PROTOTYPE(config_parse_dhcp_server_relay_agent_suboption);

src/network/networkd-network.c

@@ -20,7 +20,6 @@
 #include "networkd-bridge-mdb.h"
 #include "networkd-dhcp-common.h"
 #include "networkd-dhcp-server-static-lease.h"
-#include "networkd-dhcp-server.h"
 #include "networkd-ipv6-proxy-ndp.h"
 #include "networkd-manager.h"
 #include "networkd-ndisc.h"
@@ -326,8 +325,6 @@ int network_verify(Network *network) {
                 return r; /* sr_iov_drop_invalid_sections() logs internally. */
         network_drop_invalid_static_leases(network);
 
-        network_adjust_dhcp_server(network);
-
         return 0;
 }