sd-dhcp-server: simplify pool creation

Merge sd_dhcp_server_set_address() and sd_dhcp_server_set_lease_pool() into
sd_dhcp_server_configure_pool() as the behavior of the two former depends
on the order they are called in. The flexibility is not needed, so let's
just do this in one call.

5 files changed, 77 insertions, 60 deletions

diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h
index 268210fc50..5dc3c7aa26 100644
--- a/src/libsystemd-network/dhcp-server-internal.h
+++ b/src/libsystemd-network/dhcp-server-internal.h
@@ -57,8 +57,9 @@ struct sd_dhcp_server {
         int ifindex;
         be32_t address;
         be32_t netmask;
-        be32_t pool_start;
-        size_t pool_size;
+        be32_t subnet;
+        uint32_t pool_offset;
+        uint32_t pool_size;
 
         char *timezone;
 
@@ -67,6 +68,7 @@ struct sd_dhcp_server {
 
         Hashmap *leases_by_client_id;
         DHCPLease **bound_leases;
+        DHCPLease invalid_lease;
 
         uint32_t max_lease_time, default_lease_time;
 };
diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c
index 89b9a4c6be..7a8b298b51 100644
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -22,6 +22,7 @@
 
 #include <sys/ioctl.h>
 
+#include "in-addr-util.h"
 #include "siphash24.h"
 
 #include "sd-dhcp-server.h"
@@ -31,38 +32,63 @@
 #define DHCP_DEFAULT_LEASE_TIME_USEC USEC_PER_HOUR
 #define DHCP_MAX_LEASE_TIME_USEC (USEC_PER_HOUR*12)
 
-int sd_dhcp_server_set_lease_pool(sd_dhcp_server *server,
-                                  struct in_addr *address,
-                                  size_t size) {
+/* configures the server's address and subnet, and optionally the pool's size and offset into the subnet
+ * the whole pool must fit into the subnet, and may not contain the first (any) nor last (broadcast) address
+ * moreover, the server's own address may be in the pool, and is in that case reserved in order not to
+ * accidentally hand it out */
+int sd_dhcp_server_configure_pool(sd_dhcp_server *server, struct in_addr *address, unsigned char prefixlen, uint32_t offset, uint32_t size) {
+        struct in_addr netmask_addr;
+        be32_t netmask;
+        uint32_t server_off, broadcast_off, size_max;
+
         assert_return(server, -EINVAL);
         assert_return(address, -EINVAL);
-        assert_return(address->s_addr, -EINVAL);
-        assert_return(size, -EINVAL);
-        assert_return(server->pool_start == htobe32(INADDR_ANY), -EBUSY);
-        assert_return(!server->pool_size, -EBUSY);
-        assert_return(!server->bound_leases, -EBUSY);
+        assert_return(address->s_addr != INADDR_ANY, -EINVAL);
+        assert_return(prefixlen <= 32, -ERANGE);
+        assert_return(server->address == INADDR_ANY, -EBUSY);
+
+        assert_se(in_addr_prefixlen_to_netmask(&netmask_addr, prefixlen));
+        netmask = netmask_addr.s_addr;
+
+        server_off = be32toh(address->s_addr & ~netmask);
+        broadcast_off = be32toh(~netmask);
+
+        /* the server address cannot be the subnet address */
+        assert_return(server_off != 0, -ERANGE);
+
+        /* nor the broadcast address */
+        assert_return(server_off != broadcast_off, -ERANGE);
+
+        /* 0 offset means we should set a default, we skip the first (subnet) address
+           and take the next one */
+        if (offset == 0)
+                offset = 1;
+
+        size_max = (broadcast_off + 1) /* the number of addresses in the subnet */
+                   - offset /* exclude the addresses before the offset */
+                   - 1; /* exclude the last (broadcast) address */
+
+        /* The pool must contain at least one address */
+        assert_return(size_max >= 1, -ERANGE);
+
+        if (size != 0)
+                assert_return(size <= size_max, -ERANGE);
+        else
+                size = size_max;
 
         server->bound_leases = new0(DHCPLease*, size);
         if (!server->bound_leases)
                 return -ENOMEM;
 
-        server->pool_start = address->s_addr;
+        server->pool_offset = offset;
         server->pool_size = size;
 
-        return 0;
-}
-
-int sd_dhcp_server_set_address(sd_dhcp_server *server, struct in_addr *address,
-                               unsigned char prefixlen) {
-        assert_return(server, -EINVAL);
-        assert_return(address, -EINVAL);
-        assert_return(address->s_addr, -EINVAL);
-        assert_return(prefixlen <= 32, -ERANGE);
-        assert_return(server->address == htobe32(INADDR_ANY), -EBUSY);
-        assert_return(server->netmask == htobe32(INADDR_ANY), -EBUSY);
-
         server->address = address->s_addr;
-        server->netmask = htobe32(0xfffffffflu << (32 - prefixlen));
+        server->netmask = netmask;
+        server->subnet = address->s_addr & netmask;
+
+        if (server_off >= offset && server_off - offset < size)
+                server->bound_leases[server_off - offset] = &server->invalid_lease;
 
         return 0;
 }
@@ -661,12 +687,11 @@ static int get_pool_offset(sd_dhcp_server *server, be32_t requested_ip) {
         if (!server->pool_size)
                 return -EINVAL;
 
-        if (be32toh(requested_ip) < be32toh(server->pool_start) ||
-            be32toh(requested_ip) >= be32toh(server->pool_start) +
-                                             + server->pool_size)
-                return -EINVAL;
+        if (be32toh(requested_ip) < (be32toh(server->subnet) | server->pool_offset) ||
+            be32toh(requested_ip) >= (be32toh(server->subnet) | (server->pool_offset + server->pool_size)))
+                return -ERANGE;
 
-        return be32toh(requested_ip) - be32toh(server->pool_start);
+        return be32toh(requested_ip & ~server->netmask) - server->pool_offset;
 }
 
 #define HASH_KEY SD_ID128_MAKE(0d,1d,fe,bd,f1,24,bd,b3,47,f1,dd,6e,73,21,93,30)
@@ -718,7 +743,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message,
                 if (existing_lease)
                         address = existing_lease->address;
                 else {
-                        size_t next_offer;
+                        uint32_t next_offer;
 
                         /* even with no persistence of leases, we try to offer the same client
                            the same IP address. we do this by using the hash of the client id
@@ -728,7 +753,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message,
 
                         for (i = 0; i < server->pool_size; i++) {
                                 if (!server->bound_leases[next_offer]) {
-                                        address = htobe32(be32toh(server->pool_start) + next_offer);
+                                        address = server->subnet | htobe32(server->pool_offset + next_offer);
                                         break;
                                 } else
                                         next_offer = (next_offer + 1) % server->pool_size;
@@ -1022,7 +1047,7 @@ int sd_dhcp_server_forcerenew(sd_dhcp_server *server) {
         for (i = 0; i < server->pool_size; i++) {
                 DHCPLease *lease = server->bound_leases[i];
 
-                if (!lease)
+                if (!lease || lease == &server->invalid_lease)
                         continue;
 
                 r = server_send_forcerenew(server, lease->address,
diff --git a/src/libsystemd-network/test-dhcp-server.c b/src/libsystemd-network/test-dhcp-server.c
index 9f60ab761e..7d8a1f6bd9 100644
--- a/src/libsystemd-network/test-dhcp-server.c
+++ b/src/libsystemd-network/test-dhcp-server.c
@@ -28,6 +28,14 @@
 #include "sd-dhcp-server.h"
 #include "dhcp-server-internal.h"
 
+static void test_pool(struct in_addr *address, unsigned size, int ret) {
+        _cleanup_dhcp_server_unref_ sd_dhcp_server *server = NULL;
+
+        assert_se(sd_dhcp_server_new(&server, 1) >= 0);
+
+        assert_se(sd_dhcp_server_configure_pool(server, address, 8, 0, size) == ret);
+}
+
 static int test_basic(sd_event *event) {
         _cleanup_dhcp_server_unref_ sd_dhcp_server *server = NULL;
         struct in_addr address_lo = {
@@ -54,15 +62,14 @@ static int test_basic(sd_event *event) {
         assert_se(!sd_dhcp_server_unref(server));
 
         assert_se(sd_dhcp_server_start(server) == -EUNATCH);
-        assert_se(sd_dhcp_server_set_address(server, &address_any, 28) == -EINVAL);
-        assert_se(sd_dhcp_server_set_address(server, &address_lo, 38) == -ERANGE);
-        assert_se(sd_dhcp_server_set_address(server, &address_lo, 8) >= 0);
-        assert_se(sd_dhcp_server_set_address(server, &address_lo, 8) == -EBUSY);
 
-        assert_se(sd_dhcp_server_set_lease_pool(server, &address_any, 1) == -EINVAL);
-        assert_se(sd_dhcp_server_set_lease_pool(server, &address_lo, 0) == -EINVAL);
-        assert_se(sd_dhcp_server_set_lease_pool(server, &address_lo, 1) >= 0);
-        assert_se(sd_dhcp_server_set_lease_pool(server, &address_lo, 1) == -EBUSY);
+        assert_se(sd_dhcp_server_configure_pool(server, &address_any, 28, 0, 0) == -EINVAL);
+        assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 38, 0, 0) == -ERANGE);
+        assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0) >= 0);
+        assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0) == -EBUSY);
+
+        test_pool(&address_any, 1, -EINVAL);
+        test_pool(&address_lo, 1, 0);
 
         r = sd_dhcp_server_start(server);
 
@@ -119,12 +126,10 @@ static void test_message_handler(void) {
         };
 
         assert_se(sd_dhcp_server_new(&server, 1) >= 0);
-        assert_se(sd_dhcp_server_set_address(server, &address_lo, 8) >= 0);
+        assert_se(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0) >= 0);
         assert_se(sd_dhcp_server_attach_event(server, NULL, 0) >= 0);
         assert_se(sd_dhcp_server_start(server) >= 0);
 
-        assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test)) == 0);
-        assert_se(sd_dhcp_server_set_lease_pool(server, &address_lo, 10) >= 0);
         assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test)) == DHCP_OFFER);
 
         test.end = 0;
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index d797a8ded8..74dccfccaf 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -730,7 +730,6 @@ static int link_enter_set_addresses(Link *link) {
         /* now that we can figure out a default address for the dhcp server,
            start it */
         if (link_dhcp4_server_enabled(link)) {
-                struct in_addr pool_start;
                 Address *address;
                 Link *uplink = NULL;
                 bool acquired_uplink = false;
@@ -742,16 +741,8 @@ static int link_enter_set_addresses(Link *link) {
                         return 0;
                 }
 
-                r = sd_dhcp_server_set_address(link->dhcp_server,
-                                               &address->in_addr.in,
-                                               address->prefixlen);
-                if (r < 0)
-                        return r;
-
                 /* offer 32 addresses starting from the address following the server address */
-                pool_start.s_addr = htobe32(be32toh(address->in_addr.in.s_addr) + 1);
-                r = sd_dhcp_server_set_lease_pool(link->dhcp_server,
-                                                  &pool_start, 32);
+                r = sd_dhcp_server_configure_pool(link->dhcp_server, &address->in_addr.in, address->prefixlen, 0, 32);
                 if (r < 0)
                         return r;
 
@@ -760,11 +751,6 @@ static int link_enter_set_addresses(Link *link) {
                                               &main_address->in_addr.in);
                 if (r < 0)
                         return r;
-
-                r = sd_dhcp_server_set_prefixlen(link->dhcp_server,
-                                                 main_address->prefixlen);
-                if (r < 0)
-                        return r;
                 */
 
                 if (link->network->dhcp_server_max_lease_time_usec > 0) {
diff --git a/src/systemd/sd-dhcp-server.h b/src/systemd/sd-dhcp-server.h
index 7e4f2ffb30..4b0c7a1852 100644
--- a/src/systemd/sd-dhcp-server.h
+++ b/src/systemd/sd-dhcp-server.h
@@ -44,8 +44,7 @@ bool sd_dhcp_server_is_running(sd_dhcp_server *server);
 int sd_dhcp_server_start(sd_dhcp_server *server);
 int sd_dhcp_server_stop(sd_dhcp_server *server);
 
-int sd_dhcp_server_set_address(sd_dhcp_server *server, struct in_addr *address, unsigned char prefixlen);
-int sd_dhcp_server_set_lease_pool(sd_dhcp_server *server, struct in_addr *start, size_t size);
+int sd_dhcp_server_configure_pool(sd_dhcp_server *server, struct in_addr *address, unsigned char prefixlen, uint32_t offset, uint32_t size);
 
 int sd_dhcp_server_set_timezone(sd_dhcp_server *server, const char *timezone);
 int sd_dhcp_server_set_dns(sd_dhcp_server *server, const struct in_addr ntp[], unsigned n);