Merge pull request #20423 from yuwata/dhcp-server-static-lease-outside-pool

sd-dhcp-server: support static lease outside of pool

src/libsystemd-network/dhcp-server-internal.h

@@ -30,6 +30,8 @@ typedef struct DHCPClientId {
 } DHCPClientId;
 
 typedef struct DHCPLease {
+        sd_dhcp_server *server;
+
         DHCPClientId client_id;
 
         be32_t address;
@@ -67,10 +69,10 @@ struct sd_dhcp_server {
 
         bool emit_router;
 
-        Hashmap *leases_by_client_id;
+        Hashmap *bound_leases_by_client_id;
+        Hashmap *bound_leases_by_address;
         Hashmap *static_leases_by_client_id;
-        DHCPLease **bound_leases;
+        Hashmap *static_leases_by_address;
-        DHCPLease invalid_lease;
 
         uint32_t max_lease_time, default_lease_time;
 
@@ -96,6 +98,8 @@ typedef struct DHCPRequest {
         const uint8_t *agent_info_option;
 } DHCPRequest;
 
+extern const struct hash_ops dhcp_lease_hash_ops;
+
 int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message,
                                size_t length);
 int dhcp_server_send_packet(sd_dhcp_server *server,

src/libsystemd-network/fuzz-dhcp-server.c

@@ -23,7 +23,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         static const uint8_t chaddr[] = {3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3};
         uint8_t *client_id;
         DHCPLease *lease;
-        int pool_offset;
 
         if (size < sizeof(DHCPMessage))
                 return 0;
@@ -46,9 +45,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         lease->gateway = htobe32(UINT32_C(10) << 24 | UINT32_C(1));
         lease->expiration = UINT64_MAX;
         memcpy(lease->chaddr, chaddr, 16);
-        pool_offset = get_pool_offset(server, lease->address);
+        assert_se(hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease) >= 0);
-        server->bound_leases[pool_offset] = lease;
+        assert_se(hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease) >= 0);
-        assert_se(hashmap_put(server->leases_by_client_id, &lease->client_id, lease) >= 0);
+        lease->server = server;
 
         (void) dhcp_server_handle_message(server, (DHCPMessage*)data, size);
 

src/libsystemd-network/sd-dhcp-server.c

@@ -28,6 +28,22 @@ static DHCPLease *dhcp_lease_free(DHCPLease *lease) {
         if (!lease)
                 return NULL;
 
+        if (lease->server) {
+                DHCPLease *e;
+
+                e = hashmap_get(lease->server->bound_leases_by_client_id, &lease->client_id);
+                if (e == lease) {
+                        hashmap_remove(lease->server->bound_leases_by_address, UINT32_TO_PTR(lease->address));
+                        hashmap_remove(lease->server->bound_leases_by_client_id, &lease->client_id);
+                }
+
+                e = hashmap_get(lease->server->static_leases_by_client_id, &lease->client_id);
+                if (e == lease) {
+                        hashmap_remove(lease->server->static_leases_by_address, UINT32_TO_PTR(lease->address));
+                        hashmap_remove(lease->server->static_leases_by_client_id, &lease->client_id);
+                }
+        }
+
         free(lease->client_id.data);
         return mfree(lease);
 }
@@ -85,11 +101,6 @@ int sd_dhcp_server_configure_pool(
 
         if (server->address != address->s_addr || server->netmask != netmask || server->pool_size != size || server->pool_offset != offset) {
 
-                free(server->bound_leases);
-                server->bound_leases = new0(DHCPLease*, size);
-                if (!server->bound_leases)
-                        return -ENOMEM;
-
                 server->pool_offset = offset;
                 server->pool_size = size;
 
@@ -97,11 +108,9 @@ int sd_dhcp_server_configure_pool(
                 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;
-
                 /* Drop any leases associated with the old address range */
-                hashmap_clear(server->leases_by_client_id);
+                hashmap_clear(server->bound_leases_by_address);
+                hashmap_clear(server->bound_leases_by_client_id);
 
                 if (server->callback)
                         server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata);
@@ -144,8 +153,13 @@ int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b) {
         return memcmp(a->data, b->data, a->length);
 }
 
-DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(dhcp_lease_hash_ops, DHCPClientId, client_id_hash_func, client_id_compare_func,
+DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(
-                                              DHCPLease, dhcp_lease_free);
+        dhcp_lease_hash_ops,
+        DHCPClientId,
+        client_id_hash_func,
+        client_id_compare_func,
+        DHCPLease,
+        dhcp_lease_free);
 
 static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) {
         assert(server);
@@ -161,8 +175,10 @@ static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) {
         for (sd_dhcp_lease_server_type_t i = 0; i < _SD_DHCP_LEASE_SERVER_TYPE_MAX; i++)
                 free(server->servers[i].addr);
 
-        hashmap_free(server->leases_by_client_id);
+        server->bound_leases_by_address = hashmap_free(server->bound_leases_by_address);
-        hashmap_free(server->static_leases_by_client_id);
+        server->bound_leases_by_client_id = hashmap_free(server->bound_leases_by_client_id);
+        server->static_leases_by_address = hashmap_free(server->static_leases_by_address);
+        server->static_leases_by_client_id = hashmap_free(server->static_leases_by_client_id);
 
         ordered_set_free(server->extra_options);
         ordered_set_free(server->vendor_options);
@@ -170,8 +186,6 @@ static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) {
         free(server->agent_circuit_id);
         free(server->agent_remote_id);
 
-        free(server->bound_leases);
-
         free(server->ifname);
         return mfree(server);
 }
@@ -201,13 +215,6 @@ int sd_dhcp_server_new(sd_dhcp_server **ret, int ifindex) {
                 .max_lease_time = DIV_ROUND_UP(DHCP_MAX_LEASE_TIME_USEC, USEC_PER_SEC),
         };
 
-        server->leases_by_client_id = hashmap_new(&dhcp_lease_hash_ops);
-        if (!server->leases_by_client_id)
-                return -ENOMEM;
-        server->static_leases_by_client_id = hashmap_new(&dhcp_lease_hash_ops);
-        if (!server->static_leases_by_client_id)
-                return -ENOMEM;
-
         *ret = TAKE_PTR(server);
 
         return 0;
@@ -839,18 +846,6 @@ static int prepare_new_lease(
         return 0;
 }
 
-static bool static_leases_have_address(sd_dhcp_server *server, be32_t address) {
-        DHCPLease *s;
-
-        assert(server);
-
-        HASHMAP_FOREACH(s, server->static_leases_by_client_id)
-                if (s->address == address)
-                        return true;
-
-        return false;
-}
-
 #define HASH_KEY SD_ID128_MAKE(0d,1d,fe,bd,f1,24,bd,b3,47,f1,dd,6e,73,21,93,30)
 
 int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, size_t length) {
@@ -880,14 +875,13 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                 /* this only fails on critical errors */
                 return r;
 
-        existing_lease = hashmap_get(server->leases_by_client_id, &req->client_id);
+        existing_lease = hashmap_get(server->bound_leases_by_client_id, &req->client_id);
         static_lease = hashmap_get(server->static_leases_by_client_id, &req->client_id);
 
         switch(type) {
 
         case DHCP_DISCOVER: {
                 be32_t address = INADDR_ANY;
-                unsigned i;
 
                 log_dhcp_server(server, "DISCOVER (0x%x)", be32toh(req->message->xid));
 
@@ -903,7 +897,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                 else {
                         struct siphash state;
                         uint64_t hash;
-                        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
@@ -912,18 +905,16 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                         siphash24_init(&state, HASH_KEY.bytes);
                         client_id_hash_func(&req->client_id, &state);
                         hash = htole64(siphash24_finalize(&state));
-                        next_offer = hash % server->pool_size;
 
-                        for (i = 0; i < server->pool_size; i++) {
+                        for (unsigned i = 0; i < server->pool_size; i++) {
-                                if (!server->bound_leases[next_offer]) {
+                                be32_t tmp_address;
-                                        be32_t tmp = server->subnet | htobe32(server->pool_offset + next_offer);
+
-                                        if (!static_leases_have_address(server, tmp)) {
+                                tmp_address = server->subnet | htobe32(server->pool_offset + (hash + i) % server->pool_size);
-                                                address = tmp;
+                                if (!hashmap_contains(server->bound_leases_by_address, &tmp_address) &&
-                                                break;
+                                    !hashmap_contains(server->static_leases_by_address, &tmp_address)) {
-                                        }
+                                        address = tmp_address;
+                                        break;
                                 }
-
-                                next_offer = (next_offer + 1) % server->pool_size;
                         }
                 }
 
@@ -947,6 +938,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                 return 1;
 
         case DHCP_REQUEST: {
+                DHCPLease *existing_lease_by_address;
                 be32_t address;
                 bool init_reboot = false;
                 int pool_offset;
@@ -997,11 +989,12 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                 }
 
                 pool_offset = get_pool_offset(server, address);
+                existing_lease_by_address = hashmap_get(server->bound_leases_by_address, UINT32_TO_PTR(address));
 
                 /* verify that the requested address is from the pool, and either
                    owned by the current client or free */
-                if (pool_offset >= 0 && static_lease) {
+                if (static_lease && static_lease->address == address) {
-                        _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL, *old_lease = NULL;
+                        _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL;
                         usec_t time_now, expiration;
 
                         r = sd_event_now(server->event, clock_boottime_or_monotonic(), &time_now);
@@ -1022,12 +1015,16 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
 
                         log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid));
 
-                        server->bound_leases[pool_offset] = lease;
+                        dhcp_lease_free(hashmap_remove(server->bound_leases_by_client_id, &lease->client_id));
-
+                        r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
-                        old_lease = hashmap_remove(server->leases_by_client_id, &lease->client_id);
-                        r = hashmap_put(server->leases_by_client_id, &lease->client_id, lease);
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
+
+                        r = hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease);
+                        if (r < 0)
+                                return log_dhcp_server_errno(server, r, "Could not save lease: %m");
+
+                        lease->server = server;
                         TAKE_PTR(lease);
 
                         if (server->callback)
@@ -1035,11 +1032,13 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
 
                         return DHCP_ACK;
 
-                } else if (pool_offset >= 0 && server->bound_leases[pool_offset] == existing_lease) {
+                } else if (pool_offset >= 0 && existing_lease_by_address == existing_lease) {
                         _cleanup_(dhcp_lease_freep) DHCPLease *new_lease = NULL;
                         usec_t time_now, expiration;
                         DHCPLease *lease;
 
+                        /* Note that in the above condition we accept the case that both leases are NULL. */
+
                         r = sd_event_now(server->event, clock_boottime_or_monotonic(), &time_now);
                         if (r < 0)
                                 return r;
@@ -1065,10 +1064,14 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
 
                         log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid));
 
-                        server->bound_leases[pool_offset] = lease;
+                        r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
-                        r = hashmap_put(server->leases_by_client_id, &lease->client_id, lease);
                         if (r < 0)
                                 return log_dhcp_server_errno(server, r, "Could not save lease: %m");
+                        r = hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease);
+                        if (r < 0)
+                                return log_dhcp_server_errno(server, r, "Could not save lease: %m");
+
+                        lease->server = server;
                         TAKE_PTR(new_lease);
 
                         if (server->callback)
@@ -1090,8 +1093,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
         }
 
         case DHCP_RELEASE: {
-                int pool_offset;
-
                 log_dhcp_server(server, "RELEASE (0x%x)",
                                 be32toh(req->message->xid));
 
@@ -1101,18 +1102,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz
                 if (existing_lease->address != req->message->ciaddr)
                         return 0;
 
-                pool_offset = get_pool_offset(server, req->message->ciaddr);
+                dhcp_lease_free(existing_lease);
-                if (pool_offset < 0)
-                        return 0;
 
-                if (server->bound_leases[pool_offset] == existing_lease) {
+                if (server->callback)
-                        server->bound_leases[pool_offset] = NULL;
+                        server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata);
-                        hashmap_remove(server->leases_by_client_id, existing_lease);
-                        dhcp_lease_free(existing_lease);
-
-                        if (server->callback)
-                                server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata);
-                }
 
                 return 0;
         }}
@@ -1266,24 +1259,17 @@ on_error:
 }
 
 int sd_dhcp_server_forcerenew(sd_dhcp_server *server) {
-        int r = 0;
+        DHCPLease *lease;
+        int k, r = 0;
 
         assert_return(server, -EINVAL);
-        assert(server->bound_leases);
 
-        for (uint32_t i = 0; i < server->pool_size; i++) {
+        log_dhcp_server(server, "FORCERENEW");
-                DHCPLease *lease = server->bound_leases[i];
 
-                if (!lease || lease == &server->invalid_lease)
+        HASHMAP_FOREACH(lease, server->bound_leases_by_client_id) {
-                        continue;
+                k = server_send_forcerenew(server, lease->address, lease->gateway, lease->chaddr);
-
+                if (k < 0)
-                r = server_send_forcerenew(server, lease->address,
+                        r = k;
-                                           lease->gateway,
-                                           lease->chaddr);
-                if (r < 0)
-                        return r;
-
-                log_dhcp_server(server, "FORCERENEW");
         }
 
         return r;
@@ -1479,8 +1465,7 @@ int sd_dhcp_server_set_static_lease(
                 uint8_t *client_id,
                 size_t client_id_size) {
 
-        _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL, *old = NULL;
+        _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL;
-        DHCPClientId c;
         int r;
 
         assert_return(server, -EINVAL);
@@ -1492,6 +1477,7 @@ int sd_dhcp_server_set_static_lease(
         * the server removes any static lease with the specified mac address. */
         if (!address || address->s_addr == 0) {
                 _cleanup_free_ void *data = NULL;
+                DHCPClientId c;
 
                 data = memdup(client_id, client_id_size);
                 if (!data)
@@ -1502,11 +1488,11 @@ int sd_dhcp_server_set_static_lease(
                         .data = data,
                 };
 
-                old = hashmap_remove(server->static_leases_by_client_id, &c);
+                dhcp_lease_free(hashmap_remove(server->static_leases_by_client_id, &c));
                 return 0;
         }
 
-        if (static_leases_have_address(server, address->s_addr))
+        if (hashmap_contains(server->static_leases_by_address, UINT32_TO_PTR(address->s_addr)))
                 return -EEXIST;
 
         lease = new(DHCPLease, 1);
@@ -1524,9 +1510,13 @@ int sd_dhcp_server_set_static_lease(
                 return -ENOMEM;
 
         r = hashmap_ensure_put(&server->static_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease);
+        if (r < 0)
+                return r;
+        r = hashmap_ensure_put(&server->static_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease);
         if (r < 0)
                 return r;
 
+        lease->server = server;
         TAKE_PTR(lease);
         return 0;
 }

src/network/networkd-dhcp-server-bus.c

@@ -38,7 +38,7 @@ static int property_get_leases(
         if (r < 0)
                 return r;
 
-        HASHMAP_FOREACH(lease, s->leases_by_client_id) {
+        HASHMAP_FOREACH(lease, s->bound_leases_by_client_id) {
                 r = sd_bus_message_open_container(reply, 'r', "uayayayayt");
                 if (r < 0)
                         return r;

test/test-network/conf/dhcp-server-static-lease.network

@@ -14,4 +14,4 @@ DNS=9.9.9.9
 
 [DHCPServerStaticLease]
 MACAddress=12:34:56:78:9a:bc
-Address=10.1.1.3
+Address=10.1.1.200

test/test-network/systemd-networkd-tests.py

@@ -3863,7 +3863,7 @@ class NetworkdDHCPServerTests(unittest.TestCase, Utilities):
 
         output = check_output(*networkctl_cmd, '-n', '0', 'status', 'veth99', env=env)
         print(output)
-        self.assertIn('10.1.1.3 (DHCP4 via 10.1.1.1)', output)
+        self.assertIn('10.1.1.200 (DHCP4 via 10.1.1.1)', output)
 
 class NetworkdDHCPServerRelayAgentTests(unittest.TestCase, Utilities):
     links = [