* [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings
@ 2021-02-25 21:27 Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 2/7] ap: Fix an inet_aton error check Andrew Zaborowski
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]
L_AUTO_FREE_VAR only causes l_free to be called on the variable that is
freed and may leak the rest of the l_settings object's memory.
---
src/ap.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/ap.c b/src/ap.c
index fd1a9a6b..f7b1cf9f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2544,8 +2544,8 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
{
uint32_t ifindex = netdev_get_ifindex(ap->netdev);
char *passphrase;
- L_AUTO_FREE_VAR(struct l_settings *, settings) = NULL;
- int err;
+ struct l_settings *settings = NULL;
+ int err = -EINVAL;
/* No profile or DHCP settings */
if (!ap->config->profile && !pool.used)
@@ -2555,7 +2555,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
settings = l_settings_new();
if (!l_settings_load_from_file(settings, ap->config->profile))
- return -EINVAL;
+ goto cleanup;
passphrase = l_settings_get_string(settings, "Security",
"Passphrase");
@@ -2564,7 +2564,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
l_error("[Security].Passphrase must not exceed "
"63 characters");
l_free(passphrase);
- return -EINVAL;
+ goto cleanup;
}
strcpy(ap->config->passphrase, passphrase);
@@ -2573,7 +2573,8 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
if (!l_settings_has_group(settings, "IPv4")) {
*wait_dhcp = false;
- return 0;
+ err = 0;
+ goto cleanup;
}
}
@@ -2587,21 +2588,22 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
if (!ap->rtnl_add_cmd) {
l_error("Failed to add IPv4 address");
- return -EIO;
+ err = -EIO;
+ goto cleanup;
}
ap->cleanup_ip = true;
*wait_dhcp = true;
-
- return 0;
+ err = 0;
/* Selected address already set, continue normally */
} else if (err == -EALREADY) {
*wait_dhcp = false;
-
- return 0;
+ err = 0;
}
+cleanup:
+ l_settings_free(settings);
return err;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/7] ap: Fix an inet_aton error check
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 3/7] ap: Print error messages in dhcp_load_settings Andrew Zaborowski
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 467 bytes --]
inet_aton return 0 on error, no a negative number.
---
src/ap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/ap.c b/src/ap.c
index f7b1cf9f..d9443097 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -213,7 +213,7 @@ static const char *broadcast_from_ip(const char *ip)
struct in_addr ia;
uint32_t bcast;
- if (inet_aton(ip, &ia) < 0)
+ if (inet_aton(ip, &ia) != 1)
return NULL;
bcast = ntohl(ia.s_addr);
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/7] ap: Print error messages in dhcp_load_settings
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 2/7] ap: Fix an inet_aton error check Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 4/7] utils: Add util_netmask_from_prefix Andrew Zaborowski
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]
---
src/ap.c | 48 ++++++++++++++++++++++++++++--------------------
1 file changed, 28 insertions(+), 20 deletions(-)
diff --git a/src/ap.c b/src/ap.c
index d9443097..78df3355 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2397,7 +2397,6 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
{
struct l_dhcp_server *server = ap->server;
- struct in_addr ia;
L_AUTO_FREE_VAR(char *, netmask) = l_settings_get_string(settings,
"IPv4", "Netmask");
@@ -2413,33 +2412,42 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
if (!l_settings_get_uint(settings, "IPv4", "LeaseTime", &lease_time))
lease_time = 0;
- if (ip_range && l_strv_length(ip_range) != 2)
- goto parse_error;
-
- if (netmask && !l_dhcp_server_set_netmask(server, netmask))
- goto parse_error;
+ if (gateway && !l_dhcp_server_set_gateway(server, gateway)) {
+ l_error("[IPv4].Gateway value error");
+ goto error;
+ }
- if (gateway && !l_dhcp_server_set_gateway(server, gateway))
- goto parse_error;
+ if (dns && !l_dhcp_server_set_dns(server, dns)) {
+ l_error("[IPv4].DNSList value error");
+ goto error;
+ }
- if (dns && !l_dhcp_server_set_dns(server, dns))
- goto parse_error;
+ if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
+ l_error("[IPv4].Netmask value error");
+ goto error;
+ }
- if (ip_range && !l_dhcp_server_set_ip_range(server, ip_range[0],
- ip_range[1]))
- goto parse_error;
+ if (ip_range) {
+ if (l_strv_length(ip_range) != 2) {
+ l_error("Two addresses expected in [IPv4].IPRange");
+ goto error;
+ }
- if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time))
- goto parse_error;
+ if (!l_dhcp_server_set_ip_range(server, ip_range[0],
+ ip_range[1])) {
+ l_error("Error setting IP range from [IPv4].IPRange");
+ goto error;
+ }
+ }
- if (netmask && inet_pton(AF_INET, netmask, &ia) > 0)
- ap->ip_prefix = __builtin_popcountl(ia.s_addr);
- else
- ap->ip_prefix = 24;
+ if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time)) {
+ l_error("[IPv4].LeaseTime value error");
+ goto error;
+ }
ret = true;
-parse_error:
+error:
l_strv_free(dns);
l_strv_free(ip_range);
return ret;
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/7] utils: Add util_netmask_from_prefix
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 2/7] ap: Fix an inet_aton error check Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 3/7] ap: Print error messages in dhcp_load_settings Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 5/7] ap: Refactor IP address selection Andrew Zaborowski
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
---
src/util.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/util.h b/src/util.h
index e6b4747f..d2fe0d75 100644
--- a/src/util.h
+++ b/src/util.h
@@ -110,4 +110,9 @@ static inline uint32_t util_secure_fill_with_msb(uint32_t val)
bool util_ip_prefix_tohl(const char *ip, uint8_t *prefix, uint32_t *start_out,
uint32_t *end_out, uint32_t *mask_out);
+static inline uint32_t util_netmask_from_prefix(uint8_t prefix_len)
+{
+ return ~((1ull << (32 - prefix_len)) - 1);
+}
+
#endif /* __UTIL_H */
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/7] ap: Refactor IP address selection
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
` (2 preceding siblings ...)
2021-02-25 21:27 ` [PATCH 4/7] utils: Add util_netmask_from_prefix Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 6/7] autotests: Update APRanges usage in testAP Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 7/7] doc: Update AP settings in iwd.ap and iwd.config(5) Andrew Zaborowski
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 21536 bytes --]
Refactor how the subnet IP address is selected and change related
settings syntax in the following way:
* [IPv4].Address in an AP profile can now be given as just an IP address
or a list of prefix notation address spaces. In the latter case one
subnet address of the size implied by [IPv4].Netmask is selected
randomly from that space, defaults to a 28-bit netmask. In the former
case [IPv4].Address is now also checked for conflicts with addresses
already in use.
* main.conf's [IPv4].APAddressPool is now the fallback setting that uses
the same syntax as profile-local [IPv4].Address and has a default
value of 192.168.0.0/16.
* main.conf's [General].APRanges is deprecated in favour of
[IPv4].APAddressPool but this change should be backwards compatible.
ap_setup_dhcp now always prints error messages, it seems it was assumed
that the caller would do this before but it didn't. It also now always
reads [IPv4].Netmask or the value already set on the interface (when
existing IP is used) and uses that value. Previously the configured
netmask was only being announced through DHCP but not used locally.
---
src/ap.c | 557 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 350 insertions(+), 207 deletions(-)
diff --git a/src/ap.c b/src/ap.c
index 78df3355..98c89f7f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -28,6 +28,7 @@
#include <linux/if_ether.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <ifaddrs.h>
#include <ell/ell.h>
@@ -81,12 +82,11 @@ struct ap_state {
struct l_dhcp_server *server;
uint32_t rtnl_add_cmd;
char *own_ip;
- unsigned int ip_prefix;
+ uint8_t ip_prefix;
bool started : 1;
bool gtk_set : 1;
bool cleanup_ip : 1;
- bool use_ip_pool : 1;
};
struct sta_state {
@@ -115,99 +115,10 @@ struct ap_wsc_pbc_probe_record {
uint64_t timestamp;
};
-struct ap_ip_pool {
- uint32_t start;
- uint32_t end;
- uint8_t prefix;
-
- /* Fist/last valid subnet */
- uint8_t sub_start;
- uint8_t sub_end;
-
- struct l_uintset *used;
-};
-
-struct ap_ip_pool pool;
+struct l_uintset *used_ips;
static uint32_t netdev_watch;
struct l_netlink *rtnl;
-/*
- * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
- * will advance the subnet +1 so there are no IP conflicts between AP
- * interfaces
- */
-static bool ip_pool_create(const char *ip_prefix)
-{
- if (!util_ip_prefix_tohl(ip_prefix, &pool.prefix, &pool.start,
- &pool.end, NULL))
- return false;
-
- if (pool.prefix > 24) {
- l_error("APRanges prefix must 24 or less (%u used)",
- pool.prefix);
- memset(&pool, 0, sizeof(pool));
- return false;
- }
-
- /*
- * Find the number of subnets we can use, this will dictate the number
- * of AP interfaces that can be created (when using DHCP)
- */
- pool.sub_start = (pool.start & 0x0000ff00) >> 8;
- pool.sub_end = (pool.end & 0x0000ff00) >> 8;
-
- pool.used = l_uintset_new_from_range(pool.sub_start, pool.sub_end);
-
- return true;
-}
-
-static char *ip_pool_get()
-{
- uint32_t ip;
- struct in_addr ia;
- uint8_t next_subnet = (uint8_t)l_uintset_find_unused_min(pool.used);
-
- /* This shouldn't happen */
- if (next_subnet < pool.sub_start || next_subnet > pool.sub_end)
- return NULL;
-
- l_uintset_put(pool.used, next_subnet);
-
- ip = pool.start;
- ip &= 0xffff00ff;
- ip |= (next_subnet << 8);
-
- ia.s_addr = htonl(ip);
- return l_strdup(inet_ntoa(ia));
-}
-
-static bool ip_pool_put(const char *address)
-{
- struct in_addr ia;
- uint32_t ip;
- uint8_t subnet;
-
- if (inet_aton(address, &ia) < 0)
- return false;
-
- ip = ntohl(ia.s_addr);
-
- subnet = (ip & 0x0000ff00) >> 8;
-
- if (subnet < pool.sub_start || subnet > pool.sub_end)
- return false;
-
- return l_uintset_take(pool.used, subnet);
-}
-
-static void ip_pool_destroy()
-{
- if (pool.used)
- l_uintset_free(pool.used);
-
- memset(&pool, 0, sizeof(pool));
-}
-
static const char *broadcast_from_ip(const char *ip)
{
struct in_addr ia;
@@ -326,13 +237,8 @@ static void ap_reset(struct ap_state *ap)
broadcast_from_ip(ap->own_ip),
NULL, NULL, NULL);
- if (ap->own_ip) {
- /* Release IP from pool if used */
- if (ap->use_ip_pool)
- ip_pool_put(ap->own_ip);
-
+ if (ap->own_ip)
l_free(ap->own_ip);
- }
if (ap->server)
l_dhcp_server_stop(ap->server);
@@ -2394,12 +2300,12 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
}
}
-static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
+static bool dhcp_load_settings(struct ap_state *ap,
+ const struct l_settings *settings,
+ uint32_t own_ip, uint8_t prefix)
{
struct l_dhcp_server *server = ap->server;
- L_AUTO_FREE_VAR(char *, netmask) = l_settings_get_string(settings,
- "IPv4", "Netmask");
L_AUTO_FREE_VAR(char *, gateway) = l_settings_get_string(settings,
"IPv4", "Gateway");
char **dns = l_settings_get_string_list(settings, "IPv4",
@@ -2422,17 +2328,34 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
goto error;
}
- if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
- l_error("[IPv4].Netmask value error");
- goto error;
- }
-
if (ip_range) {
+ int i;
+
if (l_strv_length(ip_range) != 2) {
l_error("Two addresses expected in [IPv4].IPRange");
goto error;
}
+ for (i = 0; i < 2; i++) {
+ struct in_addr range_addr;
+
+ if (inet_aton(ip_range[i], &range_addr) != 1) {
+ l_error("Can't parse address in "
+ "[IPv4].IPRange[%i]", i + 1);
+ goto error;
+ }
+
+ if ((own_ip ^ ntohl(range_addr.s_addr)) &
+ util_netmask_from_prefix(prefix)) {
+ struct in_addr addr = { htonl(own_ip) };
+
+ l_error("[IPv4].IPRange[%i] is not in the "
+ "%s/%i subnet", i + 1,
+ inet_ntoa(addr), prefix);
+ goto error;
+ }
+ }
+
if (!l_dhcp_server_set_ip_range(server, ip_range[0],
ip_range[1])) {
l_error("Error setting IP range from [IPv4].IPRange");
@@ -2453,31 +2376,234 @@ error:
return ret;
}
+struct ap_ip_range {
+ uint32_t start;
+ uint32_t end;
+};
+
+/*
+ * Append any address ranges within an input start/end range, which contain
+ *@least one full subnet and don't intersect with any subnets already in
+ * use. This may result in the input range being split into multiple ranges
+ * of different sizes or being skipped altogether.
+ * All inputs must be rounded to the subnet boundary and the @used queue
+ * sorted by the subnet start address.
+ */
+static void ap_append_range(struct l_queue *to, const struct ap_ip_range *range,
+ const struct l_queue *used)
+{
+ const struct l_queue_entry *entry =
+ l_queue_get_entries((struct l_queue *) used);
+ const struct ap_ip_range *used_range = entry ? entry->data : NULL;
+ uint32_t start = range->start;
+
+ while (range->end > start) {
+ while (used_range && used_range->end <= start) {
+ entry = entry->next;
+ used_range = entry ? entry->data : NULL;
+ }
+
+ /* No more used ranges that intersect with @start/@range->end */
+ if (!used_range || range->end <= used_range->start) {
+ struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+ sub->start = start;
+ sub->end = range->end;
+ l_queue_push_tail(to, sub);
+ return;
+ }
+
+ /* Now we know @used_range is non-NULL and intersects */
+ if (start < used_range->start) {
+ struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+ sub->start = start;
+ sub->end = used_range->start;
+ l_queue_push_tail(to, sub);
+ }
+
+ /* Skip to the start of the next subnet */
+ start = used_range->end;
+ }
+}
+
+static int ap_ip_range_compare(const void *a, const void *b, void *user_data)
+{
+ const struct ap_ip_range *range_a = a;
+ const struct ap_ip_range *range_b = b;
+
+ return range_a->start > range_b->start;
+}
+
+/* Select a subnet and a host address from a defined space */
+static int ap_select_addr(const char **addr_str_list, uint8_t prefix,
+ const char *ifname, uint32_t *out_addr)
+{
+ uint32_t total = 0;
+ uint32_t selected;
+ unsigned int i;
+ uint32_t addr;
+ struct ifaddrs *ifaddr;
+ const struct ifaddrs *ifa;
+ uint32_t subnet_size = 1 << (32 - prefix);
+ uint32_t host_mask = subnet_size - 1;
+ uint32_t subnet_mask = ~host_mask;
+ uint32_t host_addr = 0;
+ struct l_queue *ranges = l_queue_new();
+ struct l_queue *used = l_queue_new();
+ struct in_addr ia;
+ const struct l_queue_entry *entry;
+ int err = -EINVAL;
+
+ /* Build a sorted list of used/unavailable subnets */
+ if (getifaddrs(&ifaddr) != 0)
+ ifaddr = NULL;
+
+ for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
+ const struct sockaddr_in *addr_sa;
+ const struct sockaddr_in *netmask_sa;
+ uint32_t used_addr;
+ uint32_t used_prefix;
+ struct ap_ip_range *range;
+
+ if (ifa->ifa_addr->sa_family != AF_INET)
+ continue;
+
+ /*
+ * Ignore the subnet set on the target interface, we'll be
+ * overwriting it if everything goes well.
+ */
+ if (!strcmp(ifa->ifa_name, ifname))
+ continue;
+
+ addr_sa = (struct sockaddr_in *) ifa->ifa_addr;
+ netmask_sa = (struct sockaddr_in *) ifa->ifa_netmask;
+ if (!addr_sa || !netmask_sa)
+ continue;
+
+ used_addr = ntohl(addr_sa->sin_addr.s_addr);
+ used_prefix = __builtin_popcountl(netmask_sa->sin_addr.s_addr);
+
+ range = l_new(struct ap_ip_range, 1);
+ range->start = used_addr & subnet_mask;
+ range->end = (range->start + (1 << (32 - used_prefix)) +
+ subnet_size - 1) & subnet_mask;
+ l_queue_insert(used, range, ap_ip_range_compare, NULL);
+ }
+
+ freeifaddrs(ifaddr);
+
+ /* Build the list of available subnets */
+
+ /* Check for the static IP syntax: Address=<IP> */
+ if (l_strv_length((char **) addr_str_list) == 1 &&
+ inet_pton(AF_INET, *addr_str_list, &ia) == 1) {
+ struct ap_ip_range range;
+
+ host_addr = ntohl(ia.s_addr);
+ range.start = host_addr & subnet_mask;
+ range.end = range.start + subnet_size;
+ ap_append_range(ranges, &range, used);
+ goto select_addr;
+ }
+
+ for (i = 0; addr_str_list[i]; i++) {
+ struct ap_ip_range range;
+ uint8_t addr_prefix;
+
+ if (!util_ip_prefix_tohl(addr_str_list[i], &addr_prefix, &addr,
+ NULL, NULL)) {
+ l_error("Can't parse %s as a subnet address",
+ addr_str_list[i]);
+ goto done;
+ }
+
+ if (addr_prefix > prefix)
+ continue;
+
+ range.start = addr & subnet_mask;
+ range.end = range.start + (1 << (32 - addr_prefix));
+ ap_append_range(ranges, &range, used);
+ }
+
+select_addr:
+ if (l_queue_isempty(ranges)) {
+ l_error("No IP subnets available for new Access Point after "
+ "eliminating those already in use on the system");
+ err = -EEXIST;
+ goto done;
+ }
+
+ /* Count available @prefix-sized subnets */
+ for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
+ struct ap_ip_range *range = entry->data;
+
+ total += (range->end - range->start) >> (32 - prefix);
+ }
+
+ selected = l_getrandom_uint32() % total;
+
+ /* Find the @selected'th @prefix-sized subnet */
+ for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
+ struct ap_ip_range *range = entry->data;
+ uint32_t count = (range->end - range->start) >> (32 - prefix);
+
+ if (selected < count) {
+ addr = range->start + (selected << (32 - prefix));
+ break;
+ }
+
+ selected -= count;
+ }
+
+ if (!host_addr)
+ host_addr = addr + 1;
+
+ err = 0;
+ *out_addr = host_addr;
+
+done:
+ l_queue_destroy(ranges, l_free);
+ l_queue_destroy(used, l_free);
+ return err;
+}
+
+#define AP_DEFAULT_IPV4_PREFIX_LEN 28
+
/*
- * This will determine the IP being used for DHCP. The IP will be automatically
- * set to ap->own_ip.
+ * This will determine the IP to be used for DHCP. ap->server,
+ * ap->own_ip and ap->ip_prefix will be set.
*
* The address to set (or keep) is determined in this order:
- * 1. Address defined in provisioning file
- * 2. Address already set on interface
- * 3. Address in IP pool.
+ * 1. Select from address pool defined in the provisioning file
+ * 2. Keep address already set on the interface
+ * 3. Select from the global address pool
+ * 4. Select from 192.168.0.0/16
*
* Returns: 0 if an IP was successfully selected and needs to be set
* -EALREADY if an IP was already set on the interface
- * -EEXIST if the IP pool ran out of IP's
+ * -EEXIST if all available subnet addresses are in use
* -EINVAL if there was an error.
*/
-static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
+static int ap_setup_dhcp(struct ap_state *ap,
+ const struct l_settings *profile,
+ const struct l_settings *global)
{
uint32_t ifindex = netdev_get_ifindex(ap->netdev);
- struct in_addr ia;
+ struct in_addr if_addr;
+ struct in_addr if_netmask;
uint32_t address = 0;
+ uint8_t prefix;
+ bool update = false;
int ret = -EINVAL;
+ char **addr_str_list;
+ L_AUTO_FREE_VAR(char *, netmask_str) =
+ l_settings_get_string(profile, "IPv4", "Netmask");
ap->server = l_dhcp_server_new(ifindex);
if (!ap->server) {
l_error("Failed to create DHCP server on %u", ifindex);
- return -EINVAL;;
+ return -EINVAL;
}
if (getenv("IWD_DHCP_DEBUG"))
@@ -2485,87 +2611,137 @@ static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
"[DHCPv4 SERV] ", NULL);
/* get the current address if there is one */
- if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
- address = ia.s_addr;
+ if (l_net_get_address(ifindex, &if_addr) && if_addr.s_addr != 0 &&
+ l_net_get_netmask(ifindex, &if_netmask)) {
+ address = ntohl(if_addr.s_addr);
+ prefix = __builtin_popcountl(if_netmask.s_addr);
+ }
+
+ if (profile && l_settings_get_value(profile, "IPv4", "Address")) {
+ addr_str_list = l_settings_get_string_list(profile, "IPv4",
+ "Address", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the profile [IPv4].Address "
+ "setting as a string list");
+ goto done;
+ }
+ } else if (address) {
+ addr_str_list = NULL;
+ } else if (l_settings_get_value(global, "IPv4", "APAddressPool")) {
+ addr_str_list = l_settings_get_string_list(global, "IPv4",
+ "APAddressPool", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the global [IPv4].APAddressPool "
+ "setting as a string list");
+ goto done;
+ }
+ } else if (l_settings_get_value(global, "General", "APRanges")) {
+ addr_str_list = l_settings_get_string_list(global, "General",
+ "APRanges", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the global [General].APRanges "
+ "setting as a string list");
+ goto done;
+ }
+ } else
+ addr_str_list = l_strv_append(NULL, "192.168.0.0/16");
- if (ap->config->profile) {
- char *addr;
-
- addr = l_settings_get_string(settings, "IPv4", "Address");
- if (addr) {
- if (inet_pton(AF_INET, addr, &ia) < 0)
- goto free_addr;
-
- /* Is a matching address already set on interface? */
- if (ia.s_addr == address)
- ret = -EALREADY;
- else
- ret = 0;
- } else if (address) {
- /* No address in config, but interface has one set */
- addr = l_strdup(inet_ntoa(ia));
- ret = -EALREADY;
- } else
- goto free_addr;
+ if (addr_str_list) {
+ uint32_t new_addr;
+ int r;
- /* Set the remaining DHCP options in config file */
- if (!dhcp_load_settings(ap, settings)) {
- ret = -EINVAL;
- goto free_addr;
- }
+ if (netmask_str) {
+ uint32_t netmask;
- if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
- ret = -EINVAL;
- goto free_addr;
- }
+ if (inet_pton(AF_INET, netmask_str, &if_netmask) < 0) {
+ l_error("Can't parse the profile "
+ "[IPv4].Netmask setting");
+ goto done;
+ }
- ap->own_ip = l_strdup(addr);
+ netmask = if_netmask.s_addr;
+ prefix = __builtin_popcountl(netmask);
+ if ((ntohl(netmask) !=
+ util_netmask_from_prefix(prefix))) {
+ l_error("Invalid netmask %s", netmask_str);
+ goto done;
+ }
-free_addr:
- l_free(addr);
+ if (prefix > 30 || prefix < 8) {
+ l_error("Netmasks between 8 and 30 bits "
+ "are allowed");
+ goto done;
+ }
+ } else
+ prefix = AP_DEFAULT_IPV4_PREFIX_LEN;
- return ret;
- } else if (address) {
- /* No config file and address is already set */
- ap->own_ip = l_strdup(inet_ntoa(ia));
-
- return -EALREADY;
- } else if (pool.used) {
- /* No config file, no address set. Use IP pool */
- ap->own_ip = ip_pool_get();
- if (!ap->own_ip) {
- l_error("No more IP's in pool, cannot start AP on %u",
- ifindex);
- return -EEXIST;
+ r = ap_select_addr((const char **) addr_str_list, prefix,
+ netdev_get_name(ap->netdev), &new_addr);
+ if (r < 0) {
+ ret = r;
+ goto done;
}
- ap->use_ip_pool = true;
- ap->ip_prefix = pool.prefix;
+ if (new_addr != address) {
+ address = new_addr;
+ update = true;
+ }
+ }
- return 0;
+ if_addr.s_addr = htonl(address);
+ if (!l_dhcp_server_set_ip_address(ap->server, inet_ntoa(if_addr))) {
+ l_error("l_dhcp_server_set_ip_address failed");
+ goto done;
+ }
+
+ if_netmask.s_addr = htonl(util_netmask_from_prefix(prefix));
+ if (!l_dhcp_server_set_netmask(ap->server, inet_ntoa(if_netmask))) {
+ l_error("l_dhcp_server_set_netmask failed");
+ goto done;
}
- return -EINVAL;
+ /* Set the remaining DHCP options in config file */
+ if (profile && !dhcp_load_settings(ap, profile, address, prefix))
+ goto done;
+
+ ap->own_ip = l_strdup(inet_ntoa(if_addr));
+ ap->ip_prefix = prefix;
+ ret = update ? 0 : -EALREADY;
+
+done:
+ l_strv_free(addr_str_list);
+ return ret;
}
static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
{
uint32_t ifindex = netdev_get_ifindex(ap->netdev);
char *passphrase;
- struct l_settings *settings = NULL;
+ struct l_settings *profile = NULL;
int err = -EINVAL;
+ const struct l_settings *settings = iwd_get_config();
+ bool dhcp_enable;
- /* No profile or DHCP settings */
- if (!ap->config->profile && !pool.used)
+ /*
+ * Enable DHCP server only if [General].EnableNetworkConfiguration is
+ * true. With StartProfile, individual profiles can additionally
+ * disable DHCP by not having the [IPv4] group.
+ */
+ if (!l_settings_get_bool(settings, "General",
+ "EnableNetworkConfiguration",
+ &dhcp_enable) ||
+ !dhcp_enable) {
+ *wait_dhcp = false;
return 0;
+ }
if (ap->config->profile) {
- settings = l_settings_new();
+ profile = l_settings_new();
- if (!l_settings_load_from_file(settings, ap->config->profile))
+ if (!l_settings_load_from_file(profile, ap->config->profile))
goto cleanup;
- passphrase = l_settings_get_string(settings, "Security",
+ passphrase = l_settings_get_string(profile, "Security",
"Passphrase");
if (passphrase) {
if (strlen(passphrase) > 63) {
@@ -2579,14 +2755,14 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
l_free(passphrase);
}
- if (!l_settings_has_group(settings, "IPv4")) {
+ if (!l_settings_has_group(profile, "IPv4")) {
*wait_dhcp = false;
err = 0;
goto cleanup;
}
}
- err = ap_setup_dhcp(ap, settings);
+ err = ap_setup_dhcp(ap, profile, settings);
if (err == 0) {
/* Address change required */
ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
@@ -2611,7 +2787,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
}
cleanup:
- l_settings_free(settings);
+ l_settings_free(profile);
return err;
}
@@ -3309,9 +3485,6 @@ static void ap_netdev_watch(struct netdev *netdev,
static int ap_init(void)
{
- const struct l_settings *settings = iwd_get_config();
- bool dhcp_enable;
-
netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
l_dbus_register_interface(dbus_get_bus(), IWD_AP_INTERFACE,
@@ -3320,34 +3493,6 @@ static int ap_init(void)
ap_setup_diagnostic_interface,
ap_diagnostic_interface_destroy, false);
- /*
- * Reusing [General].EnableNetworkConfiguration as a switch to enable
- * DHCP server. If no value is found or it is false do not create a
- * DHCP server.
- */
- if (!l_settings_get_bool(settings, "General",
- "EnableNetworkConfiguration", &dhcp_enable))
- dhcp_enable = false;
-
- if (dhcp_enable) {
- L_AUTO_FREE_VAR(char *, ip_prefix);
-
- ip_prefix = l_settings_get_string(settings, "General",
- "APRanges");
- /*
- * In this case its assumed the user only cares about station
- * netconfig so we let ap_init pass but DHCP will not be
- * enabled.
- */
- if (!ip_prefix) {
- l_warn("[General].APRanges must be set for DHCP");
- return 0;
- }
-
- if (!ip_pool_create(ip_prefix))
- return -EINVAL;
- }
-
rtnl = iwd_get_rtnl();
return 0;
@@ -3357,8 +3502,6 @@ static void ap_exit(void)
{
netdev_watch_remove(netdev_watch);
l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
-
- ip_pool_destroy();
}
IWD_MODULE(ap, ap_init, ap_exit)
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/7] autotests: Update APRanges usage in testAP
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
` (3 preceding siblings ...)
2021-02-25 21:27 ` [PATCH 5/7] ap: Refactor IP address selection Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 7/7] doc: Update AP settings in iwd.ap and iwd.config(5) Andrew Zaborowski
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
[General].APRange is now [IPv4].APAddressPool and the netmask is changed
from 23 to 27 bits to make the test correctly assert that only two
default-sized subnets are allowed by IWD simultaneously (default has
changed from 24 to 28 bits)
---
autotests/testAP/dhcp/main.conf | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/autotests/testAP/dhcp/main.conf b/autotests/testAP/dhcp/main.conf
index 667de23c..62ae782d 100644
--- a/autotests/testAP/dhcp/main.conf
+++ b/autotests/testAP/dhcp/main.conf
@@ -3,4 +3,6 @@ DisableMacAddressRandomization=true
[General]
EnableNetworkConfiguration=true
-APRanges=192.168.80.0/23
+
+[IPv4]
+APAddressPool=192.168.80.0/27
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 7/7] doc: Update AP settings in iwd.ap and iwd.config(5)
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
` (4 preceding siblings ...)
2021-02-25 21:27 ` [PATCH 6/7] autotests: Update APRanges usage in testAP Andrew Zaborowski
@ 2021-02-25 21:27 ` Andrew Zaborowski
5 siblings, 0 replies; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-25 21:27 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 8135 bytes --]
Update the AP-mode address logic documentation, change some of the
wording and add some references.
---
src/iwd.ap.rst | 70 +++++++++++++++++++++++++++++-----------------
src/iwd.config.rst | 30 ++++++++++++--------
2 files changed, 63 insertions(+), 37 deletions(-)
diff --git a/src/iwd.ap.rst b/src/iwd.ap.rst
index 52312855..57081702 100644
--- a/src/iwd.ap.rst
+++ b/src/iwd.ap.rst
@@ -3,7 +3,7 @@
============
--------------------------------------
-Configuration of IWD access point
+Configuration of IWD access points
--------------------------------------
:Author: James Prestwood <prestwoj@gmail.com>
@@ -25,19 +25,21 @@ Description of access point provisioning files.
DESCRIPTION
===========
-An access point provisioning files define the configuration of an IWD access
+An access point provisioning file defines the configuration of an IWD access
point. These files live in *$STATE_DIRECTORY*/ap (/var/lib/iwd/ap by default).
+They are read when the `net.connman.iwd.AccessPoint.StartProfile(ssid)` DBus
+method is used.
FILE FORMAT
===========
-See *iwd.network* for details on the file format.
+See *iwd.network* for details on the settings file syntax.
SETTINGS
========
The settings are split into several categories. Each category has a group
-associated with it and described in separate tables below.
+associated with it and is described in the corresponding table below.
Network Authentication Settings
-------------------------------
@@ -54,13 +56,15 @@ configuration.
* - Passphrase
- 8..63 character string
- Passphrase to be used with this access point.
+ PSK Passphrase to be used with this access point.
-DHCP Server Settings
---------------------
+IPv4 Network Configuration
+--------------------------
-The group ``[IPv4]`` contains settings for IWD's built in DHCP server. All
-settings are optional.
+The group ``[IPv4]`` contains settings for IWD's built-in DHCP server. All
+settings are optional. They're used if network configuration was enabled as
+described in ``iwd.config(5)``. Omitting the ``[IPv4]`` group disables
+network configuration and the DHCP server for this access point.
.. list-table::
:header-rows: 0
@@ -68,41 +72,57 @@ settings are optional.
:widths: 20 80
* - Address
- - IP Address of AP
-
- Optional address for the DHCP server/access point. If provided this
- address will be set on the AP interface and any other DHCP server options
- will be derived from this address, unless they are overriden inside the
- AP profile. If [IPv4].Address is not provided and no IP address is set
- on the interface prior to calling StartProfile the IP pool will be used.
+ - Local IP address of the AP or a comma-separated list of prefix-notation
+ addresses
+
+ Optional local address for the access point and the DHCP server. If a
+ single address is provided this address will be set on the AP interface
+ and any other DHCP server options will be derived from it, unless they
+ are overriden by other settings. If a list of addresses and prefix
+ lengths is given (in the `<IP>/<prefix-len>` format), a single subnet
+ address will be selected from the available space each time this profile
+ is started. The subnet size will be based on the ``[IPv4].Netmask``
+ setting. If ``[IPv4].Address`` is not provided and no IP address is set
+ on the interface prior to calling `StartProfile` the IP pool defined by
+ the global ``[IPv4].APAddressPool`` setting will be used, which in turn
+ defaults to 192.168.0.0/16.
+ For example, if ``[IPv4].Netmask`` is set the 255.255.255.0 and this
+ setting, or the global ``APAddressPool`` fallback, is set to
+ ``192.168.0.0/16, 10.0.0.0/22``, IWD will select one of the 256 subnets
+ with addresses in the 192.168.<0-255>.0/24 range or one of the 4 subnets
+ with addresses in the 10.0.<0-3>.0/24 range, allowing 270 possible
+ subnets. Defining an address pool larger than the subnet may be
+ desired if other interfaces on the system use dynamically assigned
+ addresses giving IWD a chance to avoid conflicts.
* - Gateway
- IP Address of gateway
- IP address of gateway. This will inherit from [IPv4].Address if not
- provided.
+ IP address of the gateway to be advertised by DHCP. This will fall back
+ to the local IP address if not provided.
* - Netmask
- - Netmask of DHCP server
+ - Local netmask of the AP
- This will be generated from [IPv4].Address if not provided.
+ Defaults to a 28-bit netmask if not provided.
* - DNSList
- - List of DNS servers
+ - List of DNS servers as a comma-separated IP address list
A list of DNS servers which will be advertised by the DHCP server. If
not provided no DNS servers will be sent by the DHCP server.
* - LeaseTime
- - Time limit for DHCP leases
+ - Time limit for DHCP leases in seconds
Override the default lease time.
* - IPRange
- - Range of IPs to use for the DHCP server
+ - Range of IPs for the DHCP server to use given as two addresses
+ separated by a comma
- If not provided a default range will be chosen which is the DHCP server
- address + 1 to 254.
+ From and to addresses of the range assigned to clients by DHCP. If not
+ provided the range from local address + 1 to .254 will be used.
SEE ALSO
========
diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 478bd616..b694bc8d 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -67,23 +67,13 @@ The group ``[General]`` contains general settings.
obtain the dynamic addresses from the network through the built-in
DHCP client.
- This also enables DHCP server when in AP mode when either
- [General].APRanges is set or an AP profile is being used.
+ This also enables network configuration and the DHCP server when in AP
+ mode and the AP profile being activated does does not override it.
The network configuration feature is disabled by default. See
``[Network]`` settings for additional settings related to network
configuration.
- * - APRanges
- - Values: <IP in prefix notation>
-
- Sets the range of IP's used for DHCP server (AP mode). The IP should be
- in prefix notation e.g. 192.168.1.0/24. AP's which are started in a
- profile-less configuration will use this pool of IP's to set the AP's
- interface address as well as default DHCP server options. Each AP will
- get a new subnet from the range and clients will be addressed in that
- subnet to avoid IP conflicts if multiple AP's are started.
-
* - UseDefaultInterface
- Values: true, **false**
@@ -312,6 +302,22 @@ No modification from defaults is normally required.
prevent **iwd** from roaming properly, but can be useful for networks
operating under extremely low rssi levels where roaming isn't possible.
+IPv4
+----
+
+The group ``[IPv4]`` contains settings related to IPv4 address configuration.
+
+ * - APAddressPool
+ - Values: comma-separated list of prefix-notation IPv4 address strings
+
+ Defines the space of IPs used for the AP-mode subnet addresses and the
+ DHCP server. Defaults to 192.168.0.0/16. The prefix length defines
+ the space from which an address is selected but the actual subnet size
+ (netmask) is based on the AP profile being activated and defaults to 28
+ bits. The AP profile's [IPv4].Address setting overrides the global
+ value here. Setting a too small address space will limit the number of
+ access points that can be running simultaneously on different interfaces.
+
SEE ALSO
========
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 21:14 ` Andrew Zaborowski
@ 2021-02-26 21:31 ` Denis Kenzior
0 siblings, 0 replies; 13+ messages in thread
From: Denis Kenzior @ 2021-02-26 21:31 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
Hi Andrew,
>>>>
>>>> Also, have you tested what happens to the old address/netmask? What should happen?
>>>
>>> We should be overwriting it (we use NLM_F_CREATE | NLM_F_REPLACE) but
>>> that doesn't seem to be happening, I confirmed this in test-runner
>>> now. This patch is not changing that part of the code though, I'll
>>> try to debug this separately.
>>
>> Existing logic fails / doesn't do dhcp if we have an address set though, but
>> fair enough.
>
> I believe existing logic takes the address from the interface and uses
> that for DHCP (this is documented to have lower priority than
> [IPv4].Address in the profile but higher than [General].APRanges) so
> I'm just keeping that logic.
Ah yes, you're correct. My bad.
>
> I've been looking at this for a moment now and it seems if we're
> setting the addresses using RTNL we have to query all existing
> addresses and remove them one by one, there's no "flush" or "replace"
In theory there is the NLM_F_REPLACE flag. Haven't played with it.
> message type, only "new" and "del". However we could use the
> SIOCSIFADDR ioctl same as we're using SIOCGIFADDR for querying the IP
> now.
>
The ioctl is a little limiting. I suppose for our purposes it would work, but
I'd rather use l_rtnl*.
Regards,
-Denis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 20:44 ` Denis Kenzior
@ 2021-02-26 21:14 ` Andrew Zaborowski
2021-02-26 21:31 ` Denis Kenzior
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-26 21:14 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 2584 bytes --]
On Fri, 26 Feb 2021 at 22:03, Denis Kenzior <denkenz@gmail.com> wrote:
> >> So would this handle duplicates or intersecting entries?
> >
> > No, duplicate subnets would have double the chance of being selected.
> > I can probably use the logic in ap_append_range to also remove the
> > duplicates if we care about it.
>
> Well, this is user-input, so you do need to be paranoid here. At least warn &
> ignore overlapping entries. Or if you can normalize the entries, that would be
> fine as well.
It could be intentional but fair enough, I'll just not add the
overlapping ranges to the list so every address has the same
probability of being selected.
>
> >>
> >> Also, have you tested what happens to the old address/netmask? What should happen?
> >
> > We should be overwriting it (we use NLM_F_CREATE | NLM_F_REPLACE) but
> > that doesn't seem to be happening, I confirmed this in test-runner
> > now. This patch is not changing that part of the code though, I'll
> > try to debug this separately.
>
> Existing logic fails / doesn't do dhcp if we have an address set though, but
> fair enough.
I believe existing logic takes the address from the interface and uses
that for DHCP (this is documented to have lower priority than
[IPv4].Address in the profile but higher than [General].APRanges) so
I'm just keeping that logic.
I've been looking at this for a moment now and it seems if we're
setting the addresses using RTNL we have to query all existing
addresses and remove them one by one, there's no "flush" or "replace"
message type, only "new" and "del". However we could use the
SIOCSIFADDR ioctl same as we're using SIOCGIFADDR for querying the IP
now.
>
> >> Does it make sense to cache this value here instead of parsing it out each time?
> >
> > I can do that, these settings can't currently change since we don't
> > watch the config file for changes and we don't have any other
> > interfaces to set them, but I thought it may be better to not assume
> > that.
> >
> > In NM it was pointed out to me that for safety the policy is to not
> > cache settings unless you subscribe to change notifications,
> > regardless of whether the specific setting can change or not.
>
> Makes sense, but this is also about separating the global settings from the
> profile ones. Profile settings are already handled as you describe. I also
> doubt that we can ever implement main.conf reloading in a sensible manner, so
> until that happens I'd rather be consistent with the rest of the codebase.
Ok.
Best regards
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 20:15 ` Andrew Zaborowski
@ 2021-02-26 20:44 ` Denis Kenzior
2021-02-26 21:14 ` Andrew Zaborowski
0 siblings, 1 reply; 13+ messages in thread
From: Denis Kenzior @ 2021-02-26 20:44 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 1722 bytes --]
Hi Andrew,
>> So would this handle duplicates or intersecting entries?
>
> No, duplicate subnets would have double the chance of being selected.
> I can probably use the logic in ap_append_range to also remove the
> duplicates if we care about it.
Well, this is user-input, so you do need to be paranoid here. At least warn &
ignore overlapping entries. Or if you can normalize the entries, that would be
fine as well.
>>
>> Also, have you tested what happens to the old address/netmask? What should happen?
>
> We should be overwriting it (we use NLM_F_CREATE | NLM_F_REPLACE) but
> that doesn't seem to be happening, I confirmed this in test-runner
> now. This patch is not changing that part of the code though, I'll
> try to debug this separately.
Existing logic fails / doesn't do dhcp if we have an address set though, but
fair enough.
>> Does it make sense to cache this value here instead of parsing it out each time?
>
> I can do that, these settings can't currently change since we don't
> watch the config file for changes and we don't have any other
> interfaces to set them, but I thought it may be better to not assume
> that.
>
> In NM it was pointed out to me that for safety the policy is to not
> cache settings unless you subscribe to change notifications,
> regardless of whether the specific setting can change or not.
Makes sense, but this is also about separating the global settings from the
profile ones. Profile settings are already handled as you describe. I also
doubt that we can ever implement main.conf reloading in a sensible manner, so
until that happens I'd rather be consistent with the rest of the codebase.
Regards,
-Denis
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 17:37 ` Denis Kenzior
@ 2021-02-26 20:15 ` Andrew Zaborowski
2021-02-26 20:44 ` Denis Kenzior
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-26 20:15 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 22516 bytes --]
On Fri, 26 Feb 2021 at 18:52, Denis Kenzior <denkenz@gmail.com> wrote:
> On 2/26/21 10:17 AM, Andrew Zaborowski wrote:
> > -struct ap_ip_pool pool;
> > +struct l_uintset *used_ips;
>
> What does this do?
Good point, I'll drop this.
>
> > static uint32_t netdev_watch;
> > struct l_netlink *rtnl;
> >
> > -/*
> > - * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
> > - * will advance the subnet +1 so there are no IP conflicts between AP
> > - * interfaces
> > - */
>
> <snip>
>
> > /*
> > - * This will determine the IP being used for DHCP. The IP will be automatically
> > - * set to ap->own_ip.
> > + * Append any address ranges within an input start/end range, which contain
> > + * at least one full subnet and don't intersect with any subnets already in
> > + * use. This may result in the input range being split into multiple ranges
> > + * of different sizes or being skipped altogether.
> > + * All inputs must be rounded to the subnet boundary and the @used queue
> > + * sorted by the subnet start address.
> > + */
> > +static void ap_append_range(struct l_queue *to, const struct ap_ip_range *range,
> > + const struct l_queue *used)
> > +{
> > + const struct l_queue_entry *entry =
> > + l_queue_get_entries((struct l_queue *) used);
> > + const struct ap_ip_range *used_range = entry ? entry->data : NULL;
> > + uint32_t start = range->start;
> > +
> > + while (range->end > start) {
> > + while (used_range && used_range->end <= start) {
> > + entry = entry->next;
> > + used_range = entry ? entry->data : NULL;
> > + }
> > +
> > + /* No more used ranges that intersect with @start/@range->end */
> > + if (!used_range || range->end <= used_range->start) {
> > + struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
> > +
> > + sub->start = start;
> > + sub->end = range->end;
> > + l_queue_push_tail(to, sub);
> > + return;
> > + }
> > +
> > + /* Now we know @used_range is non-NULL and intersects */
> > + if (start < used_range->start) {
> > + struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
> > +
> > + sub->start = start;
> > + sub->end = used_range->start;
> > + l_queue_push_tail(to, sub);
> > + }
> > +
> > + /* Skip to the start of the next subnet */
> > + start = used_range->end;
> > + }
> > +}
> > +
> > +static int ap_ip_range_compare(const void *a, const void *b, void *user_data)
> > +{
> > + const struct ap_ip_range *range_a = a;
> > + const struct ap_ip_range *range_b = b;
> > +
> > + return range_a->start > range_b->start;
> > +}
> > +
> > +/* Select a subnet and a host address from a defined space */
> > +static int ap_select_addr(const char **addr_str_list, uint8_t prefix,
> > + const char *ifname, uint32_t *out_addr)
> > +{
> > + uint32_t total = 0;
> > + uint32_t selected;
> > + unsigned int i;
> > + uint32_t addr;
> > + struct ifaddrs *ifaddr;
> > + const struct ifaddrs *ifa;
> > + uint32_t subnet_size = 1 << (32 - prefix);
> > + uint32_t host_mask = subnet_size - 1;
> > + uint32_t subnet_mask = ~host_mask;
> > + uint32_t host_addr = 0;
> > + struct l_queue *ranges = l_queue_new();
> > + struct l_queue *used = l_queue_new();
> > + struct in_addr ia;
> > + const struct l_queue_entry *entry;
> > + int err = -EINVAL;
> > +
> > + /* Build a sorted list of used/unavailable subnets */
> > + if (getifaddrs(&ifaddr) != 0)
> > + ifaddr = NULL;
> > +
> > + for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
> > + const struct sockaddr_in *addr_sa;
> > + const struct sockaddr_in *netmask_sa;
> > + uint32_t used_addr;
> > + uint32_t used_prefix;
> > + struct ap_ip_range *range;
> > +
> > + if (ifa->ifa_addr->sa_family != AF_INET)
> > + continue;
> > +
> > + /*
> > + * Ignore the subnet set on the target interface, we'll be
> > + * overwriting it if everything goes well.
> > + */
> > + if (!strcmp(ifa->ifa_name, ifname))
> > + continue;
> > +
> > + addr_sa = (struct sockaddr_in *) ifa->ifa_addr;
> > + netmask_sa = (struct sockaddr_in *) ifa->ifa_netmask;
> > + if (!addr_sa || !netmask_sa)
> > + continue;
> > +
> > + used_addr = ntohl(addr_sa->sin_addr.s_addr);
> > + used_prefix = __builtin_popcountl(netmask_sa->sin_addr.s_addr);
> > +
> > + range = l_new(struct ap_ip_range, 1);
> > + range->start = used_addr & subnet_mask;
> > + range->end = (range->start + (1 << (32 - used_prefix)) +
> > + subnet_size - 1) & subnet_mask;
> > + l_queue_insert(used, range, ap_ip_range_compare, NULL);
> > + }
> > +
> > + freeifaddrs(ifaddr);
> > +
> > + /* Build the list of available subnets */
> > +
> > + /* Check for the static IP syntax: Address=<IP> */
> > + if (l_strv_length((char **) addr_str_list) == 1 &&
> > + inet_pton(AF_INET, *addr_str_list, &ia) == 1) {
> > + struct ap_ip_range range;
> > +
> > + host_addr = ntohl(ia.s_addr);
> > + range.start = host_addr & subnet_mask;
> > + range.end = range.start + subnet_size;
> > + ap_append_range(ranges, &range, used);
> > + goto select_addr;
> > + }
> > +
> > + for (i = 0; addr_str_list[i]; i++) {
> > + struct ap_ip_range range;
> > + uint8_t addr_prefix;
> > +
> > + if (!util_ip_prefix_tohl(addr_str_list[i], &addr_prefix, &addr,
> > + NULL, NULL)) {
> > + l_error("Can't parse %s as a subnet address",
> > + addr_str_list[i]);
> > + goto done;
> > + }
> > +
> > + if (addr_prefix > prefix)
> > + continue;
> > +
> > + range.start = addr & subnet_mask;
> > + range.end = range.start + (1 << (32 - addr_prefix));
> > + ap_append_range(ranges, &range, used);
>
> So would this handle duplicates or intersecting entries?
No, duplicate subnets would have double the chance of being selected.
I can probably use the logic in ap_append_range to also remove the
duplicates if we care about it.
>
> > + }
> > +
> > +select_addr:
> > + if (l_queue_isempty(ranges)) {
> > + l_error("No IP subnets available for new Access Point after "
> > + "eliminating those already in use on the system");
> > + err = -EEXIST;
> > + goto done;
> > + }
> > +
> > + /* Count available @prefix-sized subnets */
> > + for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
> > + struct ap_ip_range *range = entry->data;
> > +
> > + total += (range->end - range->start) >> (32 - prefix);
> > + }
> > +
> > + selected = l_getrandom_uint32() % total;
> > +
> > + /* Find the @selected'th @prefix-sized subnet */
> > + for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
> > + struct ap_ip_range *range = entry->data;
> > + uint32_t count = (range->end - range->start) >> (32 - prefix);
> > +
> > + if (selected < count) {
> > + addr = range->start + (selected << (32 - prefix));
> > + break;
> > + }
> > +
> > + selected -= count;
> > + }
> > +
> > + if (!host_addr)
> > + host_addr = addr + 1;
> > +
> > + err = 0;
> > + *out_addr = host_addr;
> > +
> > +done:
> > + l_queue_destroy(ranges, l_free);
> > + l_queue_destroy(used, l_free);
> > + return err;
> > +}
> > +
> > +#define AP_DEFAULT_IPV4_PREFIX_LEN 28
> > +
> > +/*
> > + * This will determine the IP to be used for DHCP. ap->server,
> > + * ap->own_ip and ap->ip_prefix will be set.
> > *
> > * The address to set (or keep) is determined in this order:
> > - * 1. Address defined in provisioning file
> > - * 2. Address already set on interface
> > - * 3. Address in IP pool.
> > + * 1. Select from address pool defined in the provisioning file
> > + * 2. Keep address already set on the interface
> > + * 3. Select from the global address pool
> > + * 4. Select from 192.168.0.0/16
> > *
> > * Returns: 0 if an IP was successfully selected and needs to be set
> > * -EALREADY if an IP was already set on the interface
> > - * -EEXIST if the IP pool ran out of IP's
> > + * -EEXIST if all available subnet addresses are in use
> > * -EINVAL if there was an error.
> > */
> > -static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
> > +static int ap_setup_dhcp(struct ap_state *ap,
> > + const struct l_settings *profile,
> > + const struct l_settings *global)
> > {
> > uint32_t ifindex = netdev_get_ifindex(ap->netdev);
> > - struct in_addr ia;
> > + struct in_addr if_addr;
> > + struct in_addr if_netmask;
> > uint32_t address = 0;
> > + uint8_t prefix;
> > + bool update = false;
> > int ret = -EINVAL;
> > + char **addr_str_list;
> > + L_AUTO_FREE_VAR(char *, netmask_str) =
> > + l_settings_get_string(profile, "IPv4", "Netmask");
> >
> > ap->server = l_dhcp_server_new(ifindex);
> > if (!ap->server) {
> > l_error("Failed to create DHCP server on %u", ifindex);
> > - return -EINVAL;;
> > + return -EINVAL;
> > }
> >
> > if (getenv("IWD_DHCP_DEBUG"))
> > @@ -2491,87 +2611,137 @@ static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
> > "[DHCPv4 SERV] ", NULL);
> >
> > /* get the current address if there is one */
> > - if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
> > - address = ia.s_addr;
> > + if (l_net_get_address(ifindex, &if_addr) && if_addr.s_addr != 0 &&
> > + l_net_get_netmask(ifindex, &if_netmask)) {
> > + address = ntohl(if_addr.s_addr);
> > + prefix = __builtin_popcountl(if_netmask.s_addr);
> > + }
>
> l_net_get_* calls incur an ioctl, so if the existing address is not going to be
> used, should it even be queried?
I wondered the same thing but we use the existing IP at the end to
check if it's the same as the selected one, I thought I'd keep this
logic.
>
> Also, have you tested what happens to the old address/netmask? What should happen?
We should be overwriting it (we use NLM_F_CREATE | NLM_F_REPLACE) but
that doesn't seem to be happening, I confirmed this in test-runner
now. This patch is not changing that part of the code though, I'll
try to debug this separately.
>
> > +
> > + if (profile && l_settings_get_value(profile, "IPv4", "Address")) {
> > + addr_str_list = l_settings_get_string_list(profile, "IPv4",
> > + "Address", ',');
> > + if (!addr_str_list || !*addr_str_list) {
> > + l_error("Can't parse the profile [IPv4].Address "
> > + "setting as a string list");
> > + goto done;
> > + }
> > + } else if (address) {
> > + addr_str_list = NULL;
> > + } else if (l_settings_get_value(global, "IPv4", "APAddressPool")) {
> > + addr_str_list = l_settings_get_string_list(global, "IPv4",
> > + "APAddressPool", ',');
> > + if (!addr_str_list || !*addr_str_list) {
> > + l_error("Can't parse the global [IPv4].APAddressPool "
> > + "setting as a string list");
> > + goto done;
> > + }
> > + } else if (l_settings_get_value(global, "General", "APRanges")) {
> > + addr_str_list = l_settings_get_string_list(global, "General",
> > + "APRanges", ',');
> > + if (!addr_str_list || !*addr_str_list) {
> > + l_error("Can't parse the global [General].APRanges "
> > + "setting as a string list");
> > + goto done;
> > + }
> > + } else
> > + addr_str_list = l_strv_append(NULL, "192.168.0.0/16");
>
> It may be better to parse this just once in init()
I guess I can store the l_settings_get_string_list() result as a
global variable, but...
>
> >
> > - if (ap->config->profile) {
> > - char *addr;
> > -
> > - addr = l_settings_get_string(settings, "IPv4", "Address");
> > - if (addr) {
> > - if (inet_pton(AF_INET, addr, &ia) < 0)
> > - goto free_addr;
> > -
> > - /* Is a matching address already set on interface? */
> > - if (ia.s_addr == address)
> > - ret = -EALREADY;
> > - else
> > - ret = 0;
> > - } else if (address) {
> > - /* No address in config, but interface has one set */
> > - addr = l_strdup(inet_ntoa(ia));
> > - ret = -EALREADY;
> > - } else
> > - goto free_addr;
> > + if (addr_str_list) {
> > + uint32_t new_addr;
> > + int r;
> >
> > - /* Set the remaining DHCP options in config file */
> > - if (!dhcp_load_settings(ap, settings)) {
> > - ret = -EINVAL;
> > - goto free_addr;
> > - }
> > + if (netmask_str) {
> > + uint32_t netmask;
> >
> > - if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
> > - ret = -EINVAL;
> > - goto free_addr;
> > - }
> > + if (inet_pton(AF_INET, netmask_str, &if_netmask) < 0) {
> > + l_error("Can't parse the profile "
> > + "[IPv4].Netmask setting");
> > + goto done;
> > + }
> >
> > - ap->own_ip = l_strdup(addr);
> > + netmask = if_netmask.s_addr;
> > + prefix = __builtin_popcountl(netmask);
> > + if ((ntohl(netmask) !=
> > + util_netmask_from_prefix(prefix))) {
> > + l_error("Invalid netmask %s", netmask_str);
> > + goto done;
> > + }
> >
> > -free_addr:
> > - l_free(addr);
> > + if (prefix > 30 || prefix < 8) {
> > + l_error("Netmasks between 8 and 30 bits "
> > + "are allowed");
> > + goto done;
> > + }
> > + } else
> > + prefix = AP_DEFAULT_IPV4_PREFIX_LEN;
> >
> > - return ret;
> > - } else if (address) {
> > - /* No config file and address is already set */
> > - ap->own_ip = l_strdup(inet_ntoa(ia));
> > -
> > - return -EALREADY;
> > - } else if (pool.used) {
> > - /* No config file, no address set. Use IP pool */
> > - ap->own_ip = ip_pool_get();
> > - if (!ap->own_ip) {
> > - l_error("No more IP's in pool, cannot start AP on %u",
> > - ifindex);
> > - return -EEXIST;
> > + r = ap_select_addr((const char **) addr_str_list, prefix,
> > + netdev_get_name(ap->netdev), &new_addr);
> > + if (r < 0) {
> > + ret = r;
> > + goto done;
> > }
> >
> > - ap->use_ip_pool = true;
> > - ap->ip_prefix = pool.prefix;
> > + if (new_addr != address) {
> > + address = new_addr;
> > + update = true;
> > + }
> > + }
> >
> > - return 0;
> > + if_addr.s_addr = htonl(address);
> > + if (!l_dhcp_server_set_ip_address(ap->server, inet_ntoa(if_addr))) {
> > + l_error("l_dhcp_server_set_ip_address failed");
> > + goto done;
> > }
> >
> > - return -EINVAL;
> > + if_netmask.s_addr = htonl(util_netmask_from_prefix(prefix));
> > + if (!l_dhcp_server_set_netmask(ap->server, inet_ntoa(if_netmask))) {
> > + l_error("l_dhcp_server_set_netmask failed");
> > + goto done;
> > + }
> > +
> > + /* Set the remaining DHCP options in config file */
> > + if (profile && !dhcp_load_settings(ap, profile, address, prefix))
> > + goto done;
> > +
> > + ap->own_ip = l_strdup(inet_ntoa(if_addr));
> > + ap->ip_prefix = prefix;
> > + ret = update ? 0 : -EALREADY;
> > +
> > +done:
> > + l_strv_free(addr_str_list);
> > + return ret;
> > }
> >
> > static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> > {
> > uint32_t ifindex = netdev_get_ifindex(ap->netdev);
> > char *passphrase;
> > - struct l_settings *settings = NULL;
> > + struct l_settings *profile = NULL;
> > int err = -EINVAL;
> > + const struct l_settings *settings = iwd_get_config();
> > + bool dhcp_enable;
> >
> > - /* No profile or DHCP settings */
> > - if (!ap->config->profile && !pool.used)
> > + /*
> > + * Enable DHCP server only if [General].EnableNetworkConfiguration is
> > + * true. With StartProfile, individual profiles can additionally
> > + * disable DHCP by not having the [IPv4] group.
> > + */
> > + if (!l_settings_get_bool(settings, "General",
> > + "EnableNetworkConfiguration",
> > + &dhcp_enable) ||
> > + !dhcp_enable) {
> > + *wait_dhcp = false;
> > return 0;
> > + }
> >
> > if (ap->config->profile) {
> > - settings = l_settings_new();
> > + profile = l_settings_new();
> >
> > - if (!l_settings_load_from_file(settings, ap->config->profile))
> > + if (!l_settings_load_from_file(profile, ap->config->profile))
> > goto cleanup;
> >
> > - passphrase = l_settings_get_string(settings, "Security",
> > + passphrase = l_settings_get_string(profile, "Security",
> > "Passphrase");
> > if (passphrase) {
> > if (strlen(passphrase) > 63) {
> > @@ -2585,14 +2755,14 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> > l_free(passphrase);
> > }
> >
> > - if (!l_settings_has_group(settings, "IPv4")) {
> > + if (!l_settings_has_group(profile, "IPv4")) {
> > *wait_dhcp = false;
> > err = 0;
> > goto cleanup;
> > }
> > }
> >
> > - err = ap_setup_dhcp(ap, settings);
> > + err = ap_setup_dhcp(ap, profile, settings);
> > if (err == 0) {
> > /* Address change required */
> > ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
> > @@ -2617,7 +2787,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> > }
> >
> > cleanup:
> > - l_settings_free(settings);
> > + l_settings_free(profile);
> > return err;
> > }
> >
> > @@ -3315,9 +3485,6 @@ static void ap_netdev_watch(struct netdev *netdev,
> >
> > static int ap_init(void)
> > {
> > - const struct l_settings *settings = iwd_get_config();
> > - bool dhcp_enable;
> > -
> > netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
> >
> > l_dbus_register_interface(dbus_get_bus(), IWD_AP_INTERFACE,
> > @@ -3326,34 +3493,6 @@ static int ap_init(void)
> > ap_setup_diagnostic_interface,
> > ap_diagnostic_interface_destroy, false);
> >
> > - /*
> > - * Reusing [General].EnableNetworkConfiguration as a switch to enable
> > - * DHCP server. If no value is found or it is false do not create a
> > - * DHCP server.
> > - */
> > - if (!l_settings_get_bool(settings, "General",
> > - "EnableNetworkConfiguration", &dhcp_enable))
> > - dhcp_enable = false;
>
> Does it make sense to cache this value here instead of parsing it out each time?
I can do that, these settings can't currently change since we don't
watch the config file for changes and we don't have any other
interfaces to set them, but I thought it may be better to not assume
that.
In NM it was pointed out to me that for safety the policy is to not
cache settings unless you subscribe to change notifications,
regardless of whether the specific setting can change or not.
But that's not our policy so I can do it anyway.
Best regards
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 16:17 ` [PATCH 5/7] ap: Refactor IP address selection Andrew Zaborowski
@ 2021-02-26 17:37 ` Denis Kenzior
2021-02-26 20:15 ` Andrew Zaborowski
0 siblings, 1 reply; 13+ messages in thread
From: Denis Kenzior @ 2021-02-26 17:37 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 19575 bytes --]
Hi Andrew,
On 2/26/21 10:17 AM, Andrew Zaborowski wrote:
> Refactor how the subnet IP address is selected and change related
> settings syntax in the following way:
>
> * [IPv4].Address in an AP profile can now be given as just an IP address
> or a list of prefix notation address spaces. In the latter case one
> subnet address of the size implied by [IPv4].Netmask is selected
> randomly from that space, defaults to a 28-bit netmask. In the former
> case [IPv4].Address is now also checked for conflicts with addresses
> already in use.
>
> * main.conf's [IPv4].APAddressPool is now the fallback setting that uses
> the same syntax as profile-local [IPv4].Address and has a default
> value of 192.168.0.0/16.
>
> * main.conf's [General].APRanges is deprecated in favour of
> [IPv4].APAddressPool but this change should be backwards compatible.
>
> ap_setup_dhcp now always prints error messages, it seems it was assumed
> that the caller would do this before but it didn't. It also now always
> reads [IPv4].Netmask or the value already set on the interface (when
> existing IP is used) and uses that value. Previously the configured
> netmask was only being announced through DHCP but not used locally.
> ---
> src/ap.c | 563 ++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 350 insertions(+), 213 deletions(-)
>
> diff --git a/src/ap.c b/src/ap.c
> index 2042cde6..98c89f7f 100644
> --- a/src/ap.c
> +++ b/src/ap.c
> @@ -28,6 +28,7 @@
> #include <linux/if_ether.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> +#include <ifaddrs.h>
>
> #include <ell/ell.h>
>
> @@ -81,12 +82,11 @@ struct ap_state {
> struct l_dhcp_server *server;
> uint32_t rtnl_add_cmd;
> char *own_ip;
> - unsigned int ip_prefix;
> + uint8_t ip_prefix;
>
> bool started : 1;
> bool gtk_set : 1;
> bool cleanup_ip : 1;
> - bool use_ip_pool : 1;
> };
>
> struct sta_state {
> @@ -115,99 +115,10 @@ struct ap_wsc_pbc_probe_record {
> uint64_t timestamp;
> };
>
> -struct ap_ip_pool {
> - uint32_t start;
> - uint32_t end;
> - uint8_t prefix;
> -
> - /* Fist/last valid subnet */
> - uint8_t sub_start;
> - uint8_t sub_end;
> -
> - struct l_uintset *used;
> -};
> -
> -struct ap_ip_pool pool;
> +struct l_uintset *used_ips;
What does this do?
> static uint32_t netdev_watch;
> struct l_netlink *rtnl;
>
> -/*
> - * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
> - * will advance the subnet +1 so there are no IP conflicts between AP
> - * interfaces
> - */
<snip>
> /*
> - * This will determine the IP being used for DHCP. The IP will be automatically
> - * set to ap->own_ip.
> + * Append any address ranges within an input start/end range, which contain
> + * at least one full subnet and don't intersect with any subnets already in
> + * use. This may result in the input range being split into multiple ranges
> + * of different sizes or being skipped altogether.
> + * All inputs must be rounded to the subnet boundary and the @used queue
> + * sorted by the subnet start address.
> + */
> +static void ap_append_range(struct l_queue *to, const struct ap_ip_range *range,
> + const struct l_queue *used)
> +{
> + const struct l_queue_entry *entry =
> + l_queue_get_entries((struct l_queue *) used);
> + const struct ap_ip_range *used_range = entry ? entry->data : NULL;
> + uint32_t start = range->start;
> +
> + while (range->end > start) {
> + while (used_range && used_range->end <= start) {
> + entry = entry->next;
> + used_range = entry ? entry->data : NULL;
> + }
> +
> + /* No more used ranges that intersect with @start/@range->end */
> + if (!used_range || range->end <= used_range->start) {
> + struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
> +
> + sub->start = start;
> + sub->end = range->end;
> + l_queue_push_tail(to, sub);
> + return;
> + }
> +
> + /* Now we know @used_range is non-NULL and intersects */
> + if (start < used_range->start) {
> + struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
> +
> + sub->start = start;
> + sub->end = used_range->start;
> + l_queue_push_tail(to, sub);
> + }
> +
> + /* Skip to the start of the next subnet */
> + start = used_range->end;
> + }
> +}
> +
> +static int ap_ip_range_compare(const void *a, const void *b, void *user_data)
> +{
> + const struct ap_ip_range *range_a = a;
> + const struct ap_ip_range *range_b = b;
> +
> + return range_a->start > range_b->start;
> +}
> +
> +/* Select a subnet and a host address from a defined space */
> +static int ap_select_addr(const char **addr_str_list, uint8_t prefix,
> + const char *ifname, uint32_t *out_addr)
> +{
> + uint32_t total = 0;
> + uint32_t selected;
> + unsigned int i;
> + uint32_t addr;
> + struct ifaddrs *ifaddr;
> + const struct ifaddrs *ifa;
> + uint32_t subnet_size = 1 << (32 - prefix);
> + uint32_t host_mask = subnet_size - 1;
> + uint32_t subnet_mask = ~host_mask;
> + uint32_t host_addr = 0;
> + struct l_queue *ranges = l_queue_new();
> + struct l_queue *used = l_queue_new();
> + struct in_addr ia;
> + const struct l_queue_entry *entry;
> + int err = -EINVAL;
> +
> + /* Build a sorted list of used/unavailable subnets */
> + if (getifaddrs(&ifaddr) != 0)
> + ifaddr = NULL;
> +
> + for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
> + const struct sockaddr_in *addr_sa;
> + const struct sockaddr_in *netmask_sa;
> + uint32_t used_addr;
> + uint32_t used_prefix;
> + struct ap_ip_range *range;
> +
> + if (ifa->ifa_addr->sa_family != AF_INET)
> + continue;
> +
> + /*
> + * Ignore the subnet set on the target interface, we'll be
> + * overwriting it if everything goes well.
> + */
> + if (!strcmp(ifa->ifa_name, ifname))
> + continue;
> +
> + addr_sa = (struct sockaddr_in *) ifa->ifa_addr;
> + netmask_sa = (struct sockaddr_in *) ifa->ifa_netmask;
> + if (!addr_sa || !netmask_sa)
> + continue;
> +
> + used_addr = ntohl(addr_sa->sin_addr.s_addr);
> + used_prefix = __builtin_popcountl(netmask_sa->sin_addr.s_addr);
> +
> + range = l_new(struct ap_ip_range, 1);
> + range->start = used_addr & subnet_mask;
> + range->end = (range->start + (1 << (32 - used_prefix)) +
> + subnet_size - 1) & subnet_mask;
> + l_queue_insert(used, range, ap_ip_range_compare, NULL);
> + }
> +
> + freeifaddrs(ifaddr);
> +
> + /* Build the list of available subnets */
> +
> + /* Check for the static IP syntax: Address=<IP> */
> + if (l_strv_length((char **) addr_str_list) == 1 &&
> + inet_pton(AF_INET, *addr_str_list, &ia) == 1) {
> + struct ap_ip_range range;
> +
> + host_addr = ntohl(ia.s_addr);
> + range.start = host_addr & subnet_mask;
> + range.end = range.start + subnet_size;
> + ap_append_range(ranges, &range, used);
> + goto select_addr;
> + }
> +
> + for (i = 0; addr_str_list[i]; i++) {
> + struct ap_ip_range range;
> + uint8_t addr_prefix;
> +
> + if (!util_ip_prefix_tohl(addr_str_list[i], &addr_prefix, &addr,
> + NULL, NULL)) {
> + l_error("Can't parse %s as a subnet address",
> + addr_str_list[i]);
> + goto done;
> + }
> +
> + if (addr_prefix > prefix)
> + continue;
> +
> + range.start = addr & subnet_mask;
> + range.end = range.start + (1 << (32 - addr_prefix));
> + ap_append_range(ranges, &range, used);
So would this handle duplicates or intersecting entries?
> + }
> +
> +select_addr:
> + if (l_queue_isempty(ranges)) {
> + l_error("No IP subnets available for new Access Point after "
> + "eliminating those already in use on the system");
> + err = -EEXIST;
> + goto done;
> + }
> +
> + /* Count available @prefix-sized subnets */
> + for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
> + struct ap_ip_range *range = entry->data;
> +
> + total += (range->end - range->start) >> (32 - prefix);
> + }
> +
> + selected = l_getrandom_uint32() % total;
> +
> + /* Find the @selected'th @prefix-sized subnet */
> + for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
> + struct ap_ip_range *range = entry->data;
> + uint32_t count = (range->end - range->start) >> (32 - prefix);
> +
> + if (selected < count) {
> + addr = range->start + (selected << (32 - prefix));
> + break;
> + }
> +
> + selected -= count;
> + }
> +
> + if (!host_addr)
> + host_addr = addr + 1;
> +
> + err = 0;
> + *out_addr = host_addr;
> +
> +done:
> + l_queue_destroy(ranges, l_free);
> + l_queue_destroy(used, l_free);
> + return err;
> +}
> +
> +#define AP_DEFAULT_IPV4_PREFIX_LEN 28
> +
> +/*
> + * This will determine the IP to be used for DHCP. ap->server,
> + * ap->own_ip and ap->ip_prefix will be set.
> *
> * The address to set (or keep) is determined in this order:
> - * 1. Address defined in provisioning file
> - * 2. Address already set on interface
> - * 3. Address in IP pool.
> + * 1. Select from address pool defined in the provisioning file
> + * 2. Keep address already set on the interface
> + * 3. Select from the global address pool
> + * 4. Select from 192.168.0.0/16
> *
> * Returns: 0 if an IP was successfully selected and needs to be set
> * -EALREADY if an IP was already set on the interface
> - * -EEXIST if the IP pool ran out of IP's
> + * -EEXIST if all available subnet addresses are in use
> * -EINVAL if there was an error.
> */
> -static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
> +static int ap_setup_dhcp(struct ap_state *ap,
> + const struct l_settings *profile,
> + const struct l_settings *global)
> {
> uint32_t ifindex = netdev_get_ifindex(ap->netdev);
> - struct in_addr ia;
> + struct in_addr if_addr;
> + struct in_addr if_netmask;
> uint32_t address = 0;
> + uint8_t prefix;
> + bool update = false;
> int ret = -EINVAL;
> + char **addr_str_list;
> + L_AUTO_FREE_VAR(char *, netmask_str) =
> + l_settings_get_string(profile, "IPv4", "Netmask");
>
> ap->server = l_dhcp_server_new(ifindex);
> if (!ap->server) {
> l_error("Failed to create DHCP server on %u", ifindex);
> - return -EINVAL;;
> + return -EINVAL;
> }
>
> if (getenv("IWD_DHCP_DEBUG"))
> @@ -2491,87 +2611,137 @@ static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
> "[DHCPv4 SERV] ", NULL);
>
> /* get the current address if there is one */
> - if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
> - address = ia.s_addr;
> + if (l_net_get_address(ifindex, &if_addr) && if_addr.s_addr != 0 &&
> + l_net_get_netmask(ifindex, &if_netmask)) {
> + address = ntohl(if_addr.s_addr);
> + prefix = __builtin_popcountl(if_netmask.s_addr);
> + }
l_net_get_* calls incur an ioctl, so if the existing address is not going to be
used, should it even be queried?
Also, have you tested what happens to the old address/netmask? What should happen?
> +
> + if (profile && l_settings_get_value(profile, "IPv4", "Address")) {
> + addr_str_list = l_settings_get_string_list(profile, "IPv4",
> + "Address", ',');
> + if (!addr_str_list || !*addr_str_list) {
> + l_error("Can't parse the profile [IPv4].Address "
> + "setting as a string list");
> + goto done;
> + }
> + } else if (address) {
> + addr_str_list = NULL;
> + } else if (l_settings_get_value(global, "IPv4", "APAddressPool")) {
> + addr_str_list = l_settings_get_string_list(global, "IPv4",
> + "APAddressPool", ',');
> + if (!addr_str_list || !*addr_str_list) {
> + l_error("Can't parse the global [IPv4].APAddressPool "
> + "setting as a string list");
> + goto done;
> + }
> + } else if (l_settings_get_value(global, "General", "APRanges")) {
> + addr_str_list = l_settings_get_string_list(global, "General",
> + "APRanges", ',');
> + if (!addr_str_list || !*addr_str_list) {
> + l_error("Can't parse the global [General].APRanges "
> + "setting as a string list");
> + goto done;
> + }
> + } else
> + addr_str_list = l_strv_append(NULL, "192.168.0.0/16");
It may be better to parse this just once in init()
>
> - if (ap->config->profile) {
> - char *addr;
> -
> - addr = l_settings_get_string(settings, "IPv4", "Address");
> - if (addr) {
> - if (inet_pton(AF_INET, addr, &ia) < 0)
> - goto free_addr;
> -
> - /* Is a matching address already set on interface? */
> - if (ia.s_addr == address)
> - ret = -EALREADY;
> - else
> - ret = 0;
> - } else if (address) {
> - /* No address in config, but interface has one set */
> - addr = l_strdup(inet_ntoa(ia));
> - ret = -EALREADY;
> - } else
> - goto free_addr;
> + if (addr_str_list) {
> + uint32_t new_addr;
> + int r;
>
> - /* Set the remaining DHCP options in config file */
> - if (!dhcp_load_settings(ap, settings)) {
> - ret = -EINVAL;
> - goto free_addr;
> - }
> + if (netmask_str) {
> + uint32_t netmask;
>
> - if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
> - ret = -EINVAL;
> - goto free_addr;
> - }
> + if (inet_pton(AF_INET, netmask_str, &if_netmask) < 0) {
> + l_error("Can't parse the profile "
> + "[IPv4].Netmask setting");
> + goto done;
> + }
>
> - ap->own_ip = l_strdup(addr);
> + netmask = if_netmask.s_addr;
> + prefix = __builtin_popcountl(netmask);
> + if ((ntohl(netmask) !=
> + util_netmask_from_prefix(prefix))) {
> + l_error("Invalid netmask %s", netmask_str);
> + goto done;
> + }
>
> -free_addr:
> - l_free(addr);
> + if (prefix > 30 || prefix < 8) {
> + l_error("Netmasks between 8 and 30 bits "
> + "are allowed");
> + goto done;
> + }
> + } else
> + prefix = AP_DEFAULT_IPV4_PREFIX_LEN;
>
> - return ret;
> - } else if (address) {
> - /* No config file and address is already set */
> - ap->own_ip = l_strdup(inet_ntoa(ia));
> -
> - return -EALREADY;
> - } else if (pool.used) {
> - /* No config file, no address set. Use IP pool */
> - ap->own_ip = ip_pool_get();
> - if (!ap->own_ip) {
> - l_error("No more IP's in pool, cannot start AP on %u",
> - ifindex);
> - return -EEXIST;
> + r = ap_select_addr((const char **) addr_str_list, prefix,
> + netdev_get_name(ap->netdev), &new_addr);
> + if (r < 0) {
> + ret = r;
> + goto done;
> }
>
> - ap->use_ip_pool = true;
> - ap->ip_prefix = pool.prefix;
> + if (new_addr != address) {
> + address = new_addr;
> + update = true;
> + }
> + }
>
> - return 0;
> + if_addr.s_addr = htonl(address);
> + if (!l_dhcp_server_set_ip_address(ap->server, inet_ntoa(if_addr))) {
> + l_error("l_dhcp_server_set_ip_address failed");
> + goto done;
> }
>
> - return -EINVAL;
> + if_netmask.s_addr = htonl(util_netmask_from_prefix(prefix));
> + if (!l_dhcp_server_set_netmask(ap->server, inet_ntoa(if_netmask))) {
> + l_error("l_dhcp_server_set_netmask failed");
> + goto done;
> + }
> +
> + /* Set the remaining DHCP options in config file */
> + if (profile && !dhcp_load_settings(ap, profile, address, prefix))
> + goto done;
> +
> + ap->own_ip = l_strdup(inet_ntoa(if_addr));
> + ap->ip_prefix = prefix;
> + ret = update ? 0 : -EALREADY;
> +
> +done:
> + l_strv_free(addr_str_list);
> + return ret;
> }
>
> static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> {
> uint32_t ifindex = netdev_get_ifindex(ap->netdev);
> char *passphrase;
> - struct l_settings *settings = NULL;
> + struct l_settings *profile = NULL;
> int err = -EINVAL;
> + const struct l_settings *settings = iwd_get_config();
> + bool dhcp_enable;
>
> - /* No profile or DHCP settings */
> - if (!ap->config->profile && !pool.used)
> + /*
> + * Enable DHCP server only if [General].EnableNetworkConfiguration is
> + * true. With StartProfile, individual profiles can additionally
> + * disable DHCP by not having the [IPv4] group.
> + */
> + if (!l_settings_get_bool(settings, "General",
> + "EnableNetworkConfiguration",
> + &dhcp_enable) ||
> + !dhcp_enable) {
> + *wait_dhcp = false;
> return 0;
> + }
>
> if (ap->config->profile) {
> - settings = l_settings_new();
> + profile = l_settings_new();
>
> - if (!l_settings_load_from_file(settings, ap->config->profile))
> + if (!l_settings_load_from_file(profile, ap->config->profile))
> goto cleanup;
>
> - passphrase = l_settings_get_string(settings, "Security",
> + passphrase = l_settings_get_string(profile, "Security",
> "Passphrase");
> if (passphrase) {
> if (strlen(passphrase) > 63) {
> @@ -2585,14 +2755,14 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> l_free(passphrase);
> }
>
> - if (!l_settings_has_group(settings, "IPv4")) {
> + if (!l_settings_has_group(profile, "IPv4")) {
> *wait_dhcp = false;
> err = 0;
> goto cleanup;
> }
> }
>
> - err = ap_setup_dhcp(ap, settings);
> + err = ap_setup_dhcp(ap, profile, settings);
> if (err == 0) {
> /* Address change required */
> ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
> @@ -2617,7 +2787,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
> }
>
> cleanup:
> - l_settings_free(settings);
> + l_settings_free(profile);
> return err;
> }
>
> @@ -3315,9 +3485,6 @@ static void ap_netdev_watch(struct netdev *netdev,
>
> static int ap_init(void)
> {
> - const struct l_settings *settings = iwd_get_config();
> - bool dhcp_enable;
> -
> netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
>
> l_dbus_register_interface(dbus_get_bus(), IWD_AP_INTERFACE,
> @@ -3326,34 +3493,6 @@ static int ap_init(void)
> ap_setup_diagnostic_interface,
> ap_diagnostic_interface_destroy, false);
>
> - /*
> - * Reusing [General].EnableNetworkConfiguration as a switch to enable
> - * DHCP server. If no value is found or it is false do not create a
> - * DHCP server.
> - */
> - if (!l_settings_get_bool(settings, "General",
> - "EnableNetworkConfiguration", &dhcp_enable))
> - dhcp_enable = false;
Does it make sense to cache this value here instead of parsing it out each time?
> -
> - if (dhcp_enable) {
> - L_AUTO_FREE_VAR(char *, ip_prefix);
> -
> - ip_prefix = l_settings_get_string(settings, "General",
> - "APRanges");
> - /*
> - * In this case its assumed the user only cares about station
> - * netconfig so we let ap_init pass but DHCP will not be
> - * enabled.
> - */
> - if (!ip_prefix) {
> - l_warn("[General].APRanges must be set for DHCP");
> - return 0;
> - }
Same with this part. Handling backwards compatibility between APRanges &
APAddrressPool might look nicer here
> -
> - if (!ip_pool_create(ip_prefix))
> - return -EINVAL;
> - }
> -
> rtnl = iwd_get_rtnl();
>
> return 0;
> @@ -3363,8 +3502,6 @@ static void ap_exit(void)
> {
> netdev_watch_remove(netdev_watch);
> l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
> -
> - ip_pool_destroy();
> }
>
> IWD_MODULE(ap, ap_init, ap_exit)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/7] ap: Refactor IP address selection
2021-02-26 16:17 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
@ 2021-02-26 16:17 ` Andrew Zaborowski
2021-02-26 17:37 ` Denis Kenzior
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Zaborowski @ 2021-02-26 16:17 UTC (permalink / raw)
To: iwd
[-- Attachment #1: Type: text/plain, Size: 21860 bytes --]
Refactor how the subnet IP address is selected and change related
settings syntax in the following way:
* [IPv4].Address in an AP profile can now be given as just an IP address
or a list of prefix notation address spaces. In the latter case one
subnet address of the size implied by [IPv4].Netmask is selected
randomly from that space, defaults to a 28-bit netmask. In the former
case [IPv4].Address is now also checked for conflicts with addresses
already in use.
* main.conf's [IPv4].APAddressPool is now the fallback setting that uses
the same syntax as profile-local [IPv4].Address and has a default
value of 192.168.0.0/16.
* main.conf's [General].APRanges is deprecated in favour of
[IPv4].APAddressPool but this change should be backwards compatible.
ap_setup_dhcp now always prints error messages, it seems it was assumed
that the caller would do this before but it didn't. It also now always
reads [IPv4].Netmask or the value already set on the interface (when
existing IP is used) and uses that value. Previously the configured
netmask was only being announced through DHCP but not used locally.
---
src/ap.c | 563 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 350 insertions(+), 213 deletions(-)
diff --git a/src/ap.c b/src/ap.c
index 2042cde6..98c89f7f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -28,6 +28,7 @@
#include <linux/if_ether.h>
#include <netinet/in.h>
#include <arpa/inet.h>
+#include <ifaddrs.h>
#include <ell/ell.h>
@@ -81,12 +82,11 @@ struct ap_state {
struct l_dhcp_server *server;
uint32_t rtnl_add_cmd;
char *own_ip;
- unsigned int ip_prefix;
+ uint8_t ip_prefix;
bool started : 1;
bool gtk_set : 1;
bool cleanup_ip : 1;
- bool use_ip_pool : 1;
};
struct sta_state {
@@ -115,99 +115,10 @@ struct ap_wsc_pbc_probe_record {
uint64_t timestamp;
};
-struct ap_ip_pool {
- uint32_t start;
- uint32_t end;
- uint8_t prefix;
-
- /* Fist/last valid subnet */
- uint8_t sub_start;
- uint8_t sub_end;
-
- struct l_uintset *used;
-};
-
-struct ap_ip_pool pool;
+struct l_uintset *used_ips;
static uint32_t netdev_watch;
struct l_netlink *rtnl;
-/*
- * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
- * will advance the subnet +1 so there are no IP conflicts between AP
- * interfaces
- */
-static bool ip_pool_create(const char *ip_prefix)
-{
- if (!util_ip_prefix_tohl(ip_prefix, &pool.prefix, &pool.start,
- &pool.end, NULL))
- return false;
-
- if (pool.prefix > 24) {
- l_error("APRanges prefix must 24 or less (%u used)",
- pool.prefix);
- memset(&pool, 0, sizeof(pool));
- return false;
- }
-
- /*
- * Find the number of subnets we can use, this will dictate the number
- * of AP interfaces that can be created (when using DHCP)
- */
- pool.sub_start = (pool.start & 0x0000ff00) >> 8;
- pool.sub_end = (pool.end & 0x0000ff00) >> 8;
-
- pool.used = l_uintset_new_from_range(pool.sub_start, pool.sub_end);
-
- return true;
-}
-
-static char *ip_pool_get()
-{
- uint32_t ip;
- struct in_addr ia;
- uint8_t next_subnet = (uint8_t)l_uintset_find_unused_min(pool.used);
-
- /* This shouldn't happen */
- if (next_subnet < pool.sub_start || next_subnet > pool.sub_end)
- return NULL;
-
- l_uintset_put(pool.used, next_subnet);
-
- ip = pool.start;
- ip &= 0xffff00ff;
- ip |= (next_subnet << 8);
-
- ia.s_addr = htonl(ip);
- return l_strdup(inet_ntoa(ia));
-}
-
-static bool ip_pool_put(const char *address)
-{
- struct in_addr ia;
- uint32_t ip;
- uint8_t subnet;
-
- if (inet_aton(address, &ia) < 0)
- return false;
-
- ip = ntohl(ia.s_addr);
-
- subnet = (ip & 0x0000ff00) >> 8;
-
- if (subnet < pool.sub_start || subnet > pool.sub_end)
- return false;
-
- return l_uintset_take(pool.used, subnet);
-}
-
-static void ip_pool_destroy()
-{
- if (pool.used)
- l_uintset_free(pool.used);
-
- memset(&pool, 0, sizeof(pool));
-}
-
static const char *broadcast_from_ip(const char *ip)
{
struct in_addr ia;
@@ -326,13 +237,8 @@ static void ap_reset(struct ap_state *ap)
broadcast_from_ip(ap->own_ip),
NULL, NULL, NULL);
- if (ap->own_ip) {
- /* Release IP from pool if used */
- if (ap->use_ip_pool)
- ip_pool_put(ap->own_ip);
-
+ if (ap->own_ip)
l_free(ap->own_ip);
- }
if (ap->server)
l_dhcp_server_stop(ap->server);
@@ -2394,13 +2300,12 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
}
}
-static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
+static bool dhcp_load_settings(struct ap_state *ap,
+ const struct l_settings *settings,
+ uint32_t own_ip, uint8_t prefix)
{
struct l_dhcp_server *server = ap->server;
- struct in_addr ia;
- L_AUTO_FREE_VAR(char *, netmask) = l_settings_get_string(settings,
- "IPv4", "Netmask");
L_AUTO_FREE_VAR(char *, gateway) = l_settings_get_string(settings,
"IPv4", "Gateway");
char **dns = l_settings_get_string_list(settings, "IPv4",
@@ -2423,17 +2328,34 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
goto error;
}
- if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
- l_error("[IPv4].Netmask value error");
- goto error;
- }
-
if (ip_range) {
+ int i;
+
if (l_strv_length(ip_range) != 2) {
l_error("Two addresses expected in [IPv4].IPRange");
goto error;
}
+ for (i = 0; i < 2; i++) {
+ struct in_addr range_addr;
+
+ if (inet_aton(ip_range[i], &range_addr) != 1) {
+ l_error("Can't parse address in "
+ "[IPv4].IPRange[%i]", i + 1);
+ goto error;
+ }
+
+ if ((own_ip ^ ntohl(range_addr.s_addr)) &
+ util_netmask_from_prefix(prefix)) {
+ struct in_addr addr = { htonl(own_ip) };
+
+ l_error("[IPv4].IPRange[%i] is not in the "
+ "%s/%i subnet", i + 1,
+ inet_ntoa(addr), prefix);
+ goto error;
+ }
+ }
+
if (!l_dhcp_server_set_ip_range(server, ip_range[0],
ip_range[1])) {
l_error("Error setting IP range from [IPv4].IPRange");
@@ -2446,11 +2368,6 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
goto error;
}
- if (netmask && inet_pton(AF_INET, netmask, &ia) > 0)
- ap->ip_prefix = __builtin_popcountl(ia.s_addr);
- else
- ap->ip_prefix = 24;
-
ret = true;
error:
@@ -2459,31 +2376,234 @@ error:
return ret;
}
+struct ap_ip_range {
+ uint32_t start;
+ uint32_t end;
+};
+
/*
- * This will determine the IP being used for DHCP. The IP will be automatically
- * set to ap->own_ip.
+ * Append any address ranges within an input start/end range, which contain
+ * at least one full subnet and don't intersect with any subnets already in
+ * use. This may result in the input range being split into multiple ranges
+ * of different sizes or being skipped altogether.
+ * All inputs must be rounded to the subnet boundary and the @used queue
+ * sorted by the subnet start address.
+ */
+static void ap_append_range(struct l_queue *to, const struct ap_ip_range *range,
+ const struct l_queue *used)
+{
+ const struct l_queue_entry *entry =
+ l_queue_get_entries((struct l_queue *) used);
+ const struct ap_ip_range *used_range = entry ? entry->data : NULL;
+ uint32_t start = range->start;
+
+ while (range->end > start) {
+ while (used_range && used_range->end <= start) {
+ entry = entry->next;
+ used_range = entry ? entry->data : NULL;
+ }
+
+ /* No more used ranges that intersect with @start/@range->end */
+ if (!used_range || range->end <= used_range->start) {
+ struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+ sub->start = start;
+ sub->end = range->end;
+ l_queue_push_tail(to, sub);
+ return;
+ }
+
+ /* Now we know @used_range is non-NULL and intersects */
+ if (start < used_range->start) {
+ struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+ sub->start = start;
+ sub->end = used_range->start;
+ l_queue_push_tail(to, sub);
+ }
+
+ /* Skip to the start of the next subnet */
+ start = used_range->end;
+ }
+}
+
+static int ap_ip_range_compare(const void *a, const void *b, void *user_data)
+{
+ const struct ap_ip_range *range_a = a;
+ const struct ap_ip_range *range_b = b;
+
+ return range_a->start > range_b->start;
+}
+
+/* Select a subnet and a host address from a defined space */
+static int ap_select_addr(const char **addr_str_list, uint8_t prefix,
+ const char *ifname, uint32_t *out_addr)
+{
+ uint32_t total = 0;
+ uint32_t selected;
+ unsigned int i;
+ uint32_t addr;
+ struct ifaddrs *ifaddr;
+ const struct ifaddrs *ifa;
+ uint32_t subnet_size = 1 << (32 - prefix);
+ uint32_t host_mask = subnet_size - 1;
+ uint32_t subnet_mask = ~host_mask;
+ uint32_t host_addr = 0;
+ struct l_queue *ranges = l_queue_new();
+ struct l_queue *used = l_queue_new();
+ struct in_addr ia;
+ const struct l_queue_entry *entry;
+ int err = -EINVAL;
+
+ /* Build a sorted list of used/unavailable subnets */
+ if (getifaddrs(&ifaddr) != 0)
+ ifaddr = NULL;
+
+ for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
+ const struct sockaddr_in *addr_sa;
+ const struct sockaddr_in *netmask_sa;
+ uint32_t used_addr;
+ uint32_t used_prefix;
+ struct ap_ip_range *range;
+
+ if (ifa->ifa_addr->sa_family != AF_INET)
+ continue;
+
+ /*
+ * Ignore the subnet set on the target interface, we'll be
+ * overwriting it if everything goes well.
+ */
+ if (!strcmp(ifa->ifa_name, ifname))
+ continue;
+
+ addr_sa = (struct sockaddr_in *) ifa->ifa_addr;
+ netmask_sa = (struct sockaddr_in *) ifa->ifa_netmask;
+ if (!addr_sa || !netmask_sa)
+ continue;
+
+ used_addr = ntohl(addr_sa->sin_addr.s_addr);
+ used_prefix = __builtin_popcountl(netmask_sa->sin_addr.s_addr);
+
+ range = l_new(struct ap_ip_range, 1);
+ range->start = used_addr & subnet_mask;
+ range->end = (range->start + (1 << (32 - used_prefix)) +
+ subnet_size - 1) & subnet_mask;
+ l_queue_insert(used, range, ap_ip_range_compare, NULL);
+ }
+
+ freeifaddrs(ifaddr);
+
+ /* Build the list of available subnets */
+
+ /* Check for the static IP syntax: Address=<IP> */
+ if (l_strv_length((char **) addr_str_list) == 1 &&
+ inet_pton(AF_INET, *addr_str_list, &ia) == 1) {
+ struct ap_ip_range range;
+
+ host_addr = ntohl(ia.s_addr);
+ range.start = host_addr & subnet_mask;
+ range.end = range.start + subnet_size;
+ ap_append_range(ranges, &range, used);
+ goto select_addr;
+ }
+
+ for (i = 0; addr_str_list[i]; i++) {
+ struct ap_ip_range range;
+ uint8_t addr_prefix;
+
+ if (!util_ip_prefix_tohl(addr_str_list[i], &addr_prefix, &addr,
+ NULL, NULL)) {
+ l_error("Can't parse %s as a subnet address",
+ addr_str_list[i]);
+ goto done;
+ }
+
+ if (addr_prefix > prefix)
+ continue;
+
+ range.start = addr & subnet_mask;
+ range.end = range.start + (1 << (32 - addr_prefix));
+ ap_append_range(ranges, &range, used);
+ }
+
+select_addr:
+ if (l_queue_isempty(ranges)) {
+ l_error("No IP subnets available for new Access Point after "
+ "eliminating those already in use on the system");
+ err = -EEXIST;
+ goto done;
+ }
+
+ /* Count available @prefix-sized subnets */
+ for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
+ struct ap_ip_range *range = entry->data;
+
+ total += (range->end - range->start) >> (32 - prefix);
+ }
+
+ selected = l_getrandom_uint32() % total;
+
+ /* Find the @selected'th @prefix-sized subnet */
+ for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
+ struct ap_ip_range *range = entry->data;
+ uint32_t count = (range->end - range->start) >> (32 - prefix);
+
+ if (selected < count) {
+ addr = range->start + (selected << (32 - prefix));
+ break;
+ }
+
+ selected -= count;
+ }
+
+ if (!host_addr)
+ host_addr = addr + 1;
+
+ err = 0;
+ *out_addr = host_addr;
+
+done:
+ l_queue_destroy(ranges, l_free);
+ l_queue_destroy(used, l_free);
+ return err;
+}
+
+#define AP_DEFAULT_IPV4_PREFIX_LEN 28
+
+/*
+ * This will determine the IP to be used for DHCP. ap->server,
+ * ap->own_ip and ap->ip_prefix will be set.
*
* The address to set (or keep) is determined in this order:
- * 1. Address defined in provisioning file
- * 2. Address already set on interface
- * 3. Address in IP pool.
+ * 1. Select from address pool defined in the provisioning file
+ * 2. Keep address already set on the interface
+ * 3. Select from the global address pool
+ * 4. Select from 192.168.0.0/16
*
* Returns: 0 if an IP was successfully selected and needs to be set
* -EALREADY if an IP was already set on the interface
- * -EEXIST if the IP pool ran out of IP's
+ * -EEXIST if all available subnet addresses are in use
* -EINVAL if there was an error.
*/
-static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
+static int ap_setup_dhcp(struct ap_state *ap,
+ const struct l_settings *profile,
+ const struct l_settings *global)
{
uint32_t ifindex = netdev_get_ifindex(ap->netdev);
- struct in_addr ia;
+ struct in_addr if_addr;
+ struct in_addr if_netmask;
uint32_t address = 0;
+ uint8_t prefix;
+ bool update = false;
int ret = -EINVAL;
+ char **addr_str_list;
+ L_AUTO_FREE_VAR(char *, netmask_str) =
+ l_settings_get_string(profile, "IPv4", "Netmask");
ap->server = l_dhcp_server_new(ifindex);
if (!ap->server) {
l_error("Failed to create DHCP server on %u", ifindex);
- return -EINVAL;;
+ return -EINVAL;
}
if (getenv("IWD_DHCP_DEBUG"))
@@ -2491,87 +2611,137 @@ static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
"[DHCPv4 SERV] ", NULL);
/* get the current address if there is one */
- if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
- address = ia.s_addr;
+ if (l_net_get_address(ifindex, &if_addr) && if_addr.s_addr != 0 &&
+ l_net_get_netmask(ifindex, &if_netmask)) {
+ address = ntohl(if_addr.s_addr);
+ prefix = __builtin_popcountl(if_netmask.s_addr);
+ }
+
+ if (profile && l_settings_get_value(profile, "IPv4", "Address")) {
+ addr_str_list = l_settings_get_string_list(profile, "IPv4",
+ "Address", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the profile [IPv4].Address "
+ "setting as a string list");
+ goto done;
+ }
+ } else if (address) {
+ addr_str_list = NULL;
+ } else if (l_settings_get_value(global, "IPv4", "APAddressPool")) {
+ addr_str_list = l_settings_get_string_list(global, "IPv4",
+ "APAddressPool", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the global [IPv4].APAddressPool "
+ "setting as a string list");
+ goto done;
+ }
+ } else if (l_settings_get_value(global, "General", "APRanges")) {
+ addr_str_list = l_settings_get_string_list(global, "General",
+ "APRanges", ',');
+ if (!addr_str_list || !*addr_str_list) {
+ l_error("Can't parse the global [General].APRanges "
+ "setting as a string list");
+ goto done;
+ }
+ } else
+ addr_str_list = l_strv_append(NULL, "192.168.0.0/16");
- if (ap->config->profile) {
- char *addr;
-
- addr = l_settings_get_string(settings, "IPv4", "Address");
- if (addr) {
- if (inet_pton(AF_INET, addr, &ia) < 0)
- goto free_addr;
-
- /* Is a matching address already set on interface? */
- if (ia.s_addr == address)
- ret = -EALREADY;
- else
- ret = 0;
- } else if (address) {
- /* No address in config, but interface has one set */
- addr = l_strdup(inet_ntoa(ia));
- ret = -EALREADY;
- } else
- goto free_addr;
+ if (addr_str_list) {
+ uint32_t new_addr;
+ int r;
- /* Set the remaining DHCP options in config file */
- if (!dhcp_load_settings(ap, settings)) {
- ret = -EINVAL;
- goto free_addr;
- }
+ if (netmask_str) {
+ uint32_t netmask;
- if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
- ret = -EINVAL;
- goto free_addr;
- }
+ if (inet_pton(AF_INET, netmask_str, &if_netmask) < 0) {
+ l_error("Can't parse the profile "
+ "[IPv4].Netmask setting");
+ goto done;
+ }
- ap->own_ip = l_strdup(addr);
+ netmask = if_netmask.s_addr;
+ prefix = __builtin_popcountl(netmask);
+ if ((ntohl(netmask) !=
+ util_netmask_from_prefix(prefix))) {
+ l_error("Invalid netmask %s", netmask_str);
+ goto done;
+ }
-free_addr:
- l_free(addr);
+ if (prefix > 30 || prefix < 8) {
+ l_error("Netmasks between 8 and 30 bits "
+ "are allowed");
+ goto done;
+ }
+ } else
+ prefix = AP_DEFAULT_IPV4_PREFIX_LEN;
- return ret;
- } else if (address) {
- /* No config file and address is already set */
- ap->own_ip = l_strdup(inet_ntoa(ia));
-
- return -EALREADY;
- } else if (pool.used) {
- /* No config file, no address set. Use IP pool */
- ap->own_ip = ip_pool_get();
- if (!ap->own_ip) {
- l_error("No more IP's in pool, cannot start AP on %u",
- ifindex);
- return -EEXIST;
+ r = ap_select_addr((const char **) addr_str_list, prefix,
+ netdev_get_name(ap->netdev), &new_addr);
+ if (r < 0) {
+ ret = r;
+ goto done;
}
- ap->use_ip_pool = true;
- ap->ip_prefix = pool.prefix;
+ if (new_addr != address) {
+ address = new_addr;
+ update = true;
+ }
+ }
- return 0;
+ if_addr.s_addr = htonl(address);
+ if (!l_dhcp_server_set_ip_address(ap->server, inet_ntoa(if_addr))) {
+ l_error("l_dhcp_server_set_ip_address failed");
+ goto done;
}
- return -EINVAL;
+ if_netmask.s_addr = htonl(util_netmask_from_prefix(prefix));
+ if (!l_dhcp_server_set_netmask(ap->server, inet_ntoa(if_netmask))) {
+ l_error("l_dhcp_server_set_netmask failed");
+ goto done;
+ }
+
+ /* Set the remaining DHCP options in config file */
+ if (profile && !dhcp_load_settings(ap, profile, address, prefix))
+ goto done;
+
+ ap->own_ip = l_strdup(inet_ntoa(if_addr));
+ ap->ip_prefix = prefix;
+ ret = update ? 0 : -EALREADY;
+
+done:
+ l_strv_free(addr_str_list);
+ return ret;
}
static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
{
uint32_t ifindex = netdev_get_ifindex(ap->netdev);
char *passphrase;
- struct l_settings *settings = NULL;
+ struct l_settings *profile = NULL;
int err = -EINVAL;
+ const struct l_settings *settings = iwd_get_config();
+ bool dhcp_enable;
- /* No profile or DHCP settings */
- if (!ap->config->profile && !pool.used)
+ /*
+ * Enable DHCP server only if [General].EnableNetworkConfiguration is
+ * true. With StartProfile, individual profiles can additionally
+ * disable DHCP by not having the [IPv4] group.
+ */
+ if (!l_settings_get_bool(settings, "General",
+ "EnableNetworkConfiguration",
+ &dhcp_enable) ||
+ !dhcp_enable) {
+ *wait_dhcp = false;
return 0;
+ }
if (ap->config->profile) {
- settings = l_settings_new();
+ profile = l_settings_new();
- if (!l_settings_load_from_file(settings, ap->config->profile))
+ if (!l_settings_load_from_file(profile, ap->config->profile))
goto cleanup;
- passphrase = l_settings_get_string(settings, "Security",
+ passphrase = l_settings_get_string(profile, "Security",
"Passphrase");
if (passphrase) {
if (strlen(passphrase) > 63) {
@@ -2585,14 +2755,14 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
l_free(passphrase);
}
- if (!l_settings_has_group(settings, "IPv4")) {
+ if (!l_settings_has_group(profile, "IPv4")) {
*wait_dhcp = false;
err = 0;
goto cleanup;
}
}
- err = ap_setup_dhcp(ap, settings);
+ err = ap_setup_dhcp(ap, profile, settings);
if (err == 0) {
/* Address change required */
ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
@@ -2617,7 +2787,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
}
cleanup:
- l_settings_free(settings);
+ l_settings_free(profile);
return err;
}
@@ -3315,9 +3485,6 @@ static void ap_netdev_watch(struct netdev *netdev,
static int ap_init(void)
{
- const struct l_settings *settings = iwd_get_config();
- bool dhcp_enable;
-
netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
l_dbus_register_interface(dbus_get_bus(), IWD_AP_INTERFACE,
@@ -3326,34 +3493,6 @@ static int ap_init(void)
ap_setup_diagnostic_interface,
ap_diagnostic_interface_destroy, false);
- /*
- * Reusing [General].EnableNetworkConfiguration as a switch to enable
- * DHCP server. If no value is found or it is false do not create a
- * DHCP server.
- */
- if (!l_settings_get_bool(settings, "General",
- "EnableNetworkConfiguration", &dhcp_enable))
- dhcp_enable = false;
-
- if (dhcp_enable) {
- L_AUTO_FREE_VAR(char *, ip_prefix);
-
- ip_prefix = l_settings_get_string(settings, "General",
- "APRanges");
- /*
- * In this case its assumed the user only cares about station
- * netconfig so we let ap_init pass but DHCP will not be
- * enabled.
- */
- if (!ip_prefix) {
- l_warn("[General].APRanges must be set for DHCP");
- return 0;
- }
-
- if (!ip_pool_create(ip_prefix))
- return -EINVAL;
- }
-
rtnl = iwd_get_rtnl();
return 0;
@@ -3363,8 +3502,6 @@ static void ap_exit(void)
{
netdev_watch_remove(netdev_watch);
l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
-
- ip_pool_destroy();
}
IWD_MODULE(ap, ap_init, ap_exit)
--
2.27.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-02-26 21:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 21:27 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 2/7] ap: Fix an inet_aton error check Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 3/7] ap: Print error messages in dhcp_load_settings Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 4/7] utils: Add util_netmask_from_prefix Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 5/7] ap: Refactor IP address selection Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 6/7] autotests: Update APRanges usage in testAP Andrew Zaborowski
2021-02-25 21:27 ` [PATCH 7/7] doc: Update AP settings in iwd.ap and iwd.config(5) Andrew Zaborowski
2021-02-26 16:17 [PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings Andrew Zaborowski
2021-02-26 16:17 ` [PATCH 5/7] ap: Refactor IP address selection Andrew Zaborowski
2021-02-26 17:37 ` Denis Kenzior
2021-02-26 20:15 ` Andrew Zaborowski
2021-02-26 20:44 ` Denis Kenzior
2021-02-26 21:14 ` Andrew Zaborowski
2021-02-26 21:31 ` Denis Kenzior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.