From 99634696183dfabae20104e58157c69029a11594 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 28 Aug 2015 20:29:10 +0200 Subject: [PATCH 1/3] 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. --- src/libsystemd-network/dhcp-server-internal.h | 6 +- src/libsystemd-network/sd-dhcp-server.c | 85 ++++++++++++------- src/libsystemd-network/test-dhcp-server.c | 27 +++--- src/network/networkd-link.c | 16 +--- src/systemd/sd-dhcp-server.h | 3 +- 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 268210fc500..5dc3c7aa262 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 89b9a4c6be9..7a8b298b514 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -22,6 +22,7 @@ #include +#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 9f60ab761e5..7d8a1f6bd9b 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 d797a8ded8d..74dccfccaf6 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 7e4f2ffb302..4b0c7a18526 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); From 61986155d273342ffaf5be4d6a4386be96f9b46b Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 28 Aug 2015 20:37:03 +0200 Subject: [PATCH 2/3] networkd: dhcp-server - default to manage the whole subnet Don't restrict yourselves to 32 leases, simply manage the whole subnet by default. --- src/network/networkd-link.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 74dccfccaf6..c5434147edd 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -741,8 +741,8 @@ static int link_enter_set_addresses(Link *link) { return 0; } - /* offer 32 addresses starting from the address following the server address */ - r = sd_dhcp_server_configure_pool(link->dhcp_server, &address->in_addr.in, address->prefixlen, 0, 32); + /* use the server address' subnet as the pool */ + r = sd_dhcp_server_configure_pool(link->dhcp_server, &address->in_addr.in, address->prefixlen, 0, 0); if (r < 0) return r; From 9b3a67c55b7df6642a0389306c513b17c211f280 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Sat, 29 Aug 2015 00:18:20 +0200 Subject: [PATCH 3/3] networkd: dhcp-server - allow configuration of the pool The constraints we place on the pool is that it is a contiguous sequence of addresses in the same subnet as the server address, not including the subnet nor broadcast addresses, but possibly including the server address itself. If the server address is included in the pool it is (obviously) reserved and not handed out to clients. --- man/systemd.network.xml | 17 +++++++++++++++++ src/network/networkd-link.c | 3 ++- src/network/networkd-network-gperf.gperf | 2 ++ src/network/networkd-network.h | 2 ++ src/shared/conf-parser.c | 1 + src/shared/conf-parser.h | 1 + 6 files changed, 25 insertions(+), 1 deletion(-) diff --git a/man/systemd.network.xml b/man/systemd.network.xml index ded2c0ceff4..2fb47334943 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -683,6 +683,23 @@ + + PoolOffset= + PoolSize= + + Configures the pool of addresses to hand out. The pool + is a contiguous sequence of IP addresses in the subnet configured for + the server address, which does not include the subnet nor the broadcast + address. PoolOffset= takes the offset of the pool + from the start of subnet, or zero to use the default value. + PoolSize= takes the number of IP addresses in the + pool or zero to use the default value. By default the pool starts at + the first address after the subnet address and takes up the rest of + the subnet, excluding the broadcast address. If the pool includes + the server address (the default), this is reserved and not handed + out to clients. + + DefaultLeaseTimeSec= MaxLeaseTimeSec= diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index c5434147edd..979f3115f6d 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -742,7 +742,8 @@ static int link_enter_set_addresses(Link *link) { } /* use the server address' subnet as the pool */ - r = sd_dhcp_server_configure_pool(link->dhcp_server, &address->in_addr.in, address->prefixlen, 0, 0); + r = sd_dhcp_server_configure_pool(link->dhcp_server, &address->in_addr.in, address->prefixlen, + link->network->dhcp_server_pool_offset, link->network->dhcp_server_pool_size); if (r < 0) return r; diff --git a/src/network/networkd-network-gperf.gperf b/src/network/networkd-network-gperf.gperf index 108e892fb86..10ca9dae357 100644 --- a/src/network/networkd-network-gperf.gperf +++ b/src/network/networkd-network-gperf.gperf @@ -82,6 +82,8 @@ DHCPServer.EmitNTP, config_parse_bool, 0 DHCPServer.NTP, config_parse_dhcp_server_ntp, 0, 0 DHCPServer.EmitTimezone, config_parse_bool, 0, offsetof(Network, dhcp_server_emit_timezone) DHCPServer.Timezone, config_parse_timezone, 0, offsetof(Network, dhcp_server_timezone) +DHCPServer.PoolOffset, config_parse_uint32, 0, offsetof(Network, dhcp_server_pool_offset) +DHCPServer.PoolSize, config_parse_uint32, 0, offsetof(Network, dhcp_server_pool_size) Bridge.Cost, config_parse_unsigned, 0, offsetof(Network, cost) Bridge.UseBPDU, config_parse_bool, 0, offsetof(Network, use_bpdu) Bridge.HairPin, config_parse_bool, 0, offsetof(Network, hairpin) diff --git a/src/network/networkd-network.h b/src/network/networkd-network.h index d691cc3a45a..c3439a70ba2 100644 --- a/src/network/networkd-network.h +++ b/src/network/networkd-network.h @@ -102,6 +102,8 @@ struct Network { bool dhcp_server_emit_timezone; char *dhcp_server_timezone; usec_t dhcp_server_default_lease_time_usec, dhcp_server_max_lease_time_usec; + uint32_t dhcp_server_pool_offset; + uint32_t dhcp_server_pool_size; /* IPV4LL Support */ AddressFamilyBoolean link_local; diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index d99aa1d6e95..1f4aea6d6be 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -450,6 +450,7 @@ int config_parse_many(const char *conf_file, DEFINE_PARSER(int, int, safe_atoi) DEFINE_PARSER(long, long, safe_atoli) +DEFINE_PARSER(uint32, uint32_t, safe_atou32) DEFINE_PARSER(uint64, uint64_t, safe_atou64) DEFINE_PARSER(unsigned, unsigned, safe_atou) DEFINE_PARSER(double, double, safe_atod) diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 6152ee33b93..66c80890d3e 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -104,6 +104,7 @@ int config_parse_many(const char *conf_file, /* possibly NULL */ int config_parse_int(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_unsigned(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_long(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); +int config_parse_uint32(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_uint64(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_double(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata); int config_parse_iec_size(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);