[PATCH 1/8] ip-pool: Track IPv4 addresses in use

[PATCH 1/8] ip-pool: Track IPv4 addresses in use

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 6921 bytes --]

Add the ip-pool submodule that tracks IPv4 addresses in use on the
system for use when selecting the address for a new AP.  l_rtnl_address
is used internally because if we're going to return l_rtnl_address
objects it would be misleading if we didn't fill in all of their
properties like flags etc.
---
 Makefile.am   |   1 +
 src/ip-pool.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ip-pool.h |  23 +++++++
 3 files changed, 199 insertions(+)
 create mode 100644 src/ip-pool.c
 create mode 100644 src/ip-pool.h

diff --git a/Makefile.am b/Makefile.am
index 68035e46..083bc95a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -243,6 +243,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
 					src/eap-wsc.c src/eap-wsc.h \
 					src/wscutil.h src/wscutil.c \
 					src/diagnostic.h src/diagnostic.c \
+					src/ip-pool.h src/ip-pool.c \
 					$(eap_sources) \
 					$(builtin_sources)
 
diff --git a/src/ip-pool.c b/src/ip-pool.c
new file mode 100644
index 00000000..51b3ef40
--- /dev/null
+++ b/src/ip-pool.c
@@ -0,0 +1,175 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2021  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+
+#include <ell/ell.h>
+
+#include "src/util.h"
+#include "src/iwd.h"
+#include "src/module.h"
+#include "src/netdev.h"
+#include "src/ip-pool.h"
+
+struct ip_pool_addr4_record {
+	uint32_t ifindex;
+	struct l_rtnl_address *addr;
+};
+
+static struct l_queue *used_addr4_list;
+static struct l_netlink *rtnl;
+
+static void ip_pool_addr4_record_free(void *data)
+{
+	struct ip_pool_addr4_record *rec = data;
+
+	l_rtnl_address_free(rec->addr);
+	l_free(rec);
+}
+
+static bool ip_pool_addr4_match_ifindex(const void *a, const void *b)
+{
+	const struct ip_pool_addr4_record *addr = a;
+
+	return addr->ifindex == L_PTR_TO_UINT(b);
+}
+
+struct l_rtnl_address *ip_pool_get_addr4(uint32_t ifindex)
+{
+	const struct ip_pool_addr4_record *rec =
+		l_queue_find(used_addr4_list, ip_pool_addr4_match_ifindex,
+				L_UINT_TO_PTR(ifindex));
+
+	return rec ? l_rtnl_address_dup(rec->addr) : 0;
+}
+
+static bool ip_pool_addr4_match_free(void *data, void *user_data)
+{
+	const struct ip_pool_addr4_record *a = data;
+	const struct ip_pool_addr4_record *b = user_data;
+	char a_addr_str[INET_ADDRSTRLEN];
+	char b_addr_str[INET_ADDRSTRLEN];
+
+	if (a->ifindex != b->ifindex ||
+			l_rtnl_address_get_prefix_length(a->addr) !=
+			l_rtnl_address_get_prefix_length(b->addr))
+		return false;
+
+	if (!l_rtnl_address_get_address(a->addr, a_addr_str) ||
+			!l_rtnl_address_get_address(b->addr, b_addr_str) ||
+			strcmp(a_addr_str, b_addr_str))
+		return false;
+
+	ip_pool_addr4_record_free(data);
+	return true;
+}
+
+static void ip_pool_addr_notify(uint16_t type, const void *data, uint32_t len,
+				void *user_data)
+{
+	const struct ifaddrmsg *ifa = data;
+	struct l_rtnl_address *addr;
+
+	if (ifa->ifa_family != AF_INET || ifa->ifa_prefixlen < 1)
+		return;
+
+	len -= NLMSG_ALIGN(sizeof(struct ifaddrmsg));
+
+	addr = l_rtnl_ifaddr_extract(ifa, len);
+	if (!addr)
+		return;
+
+	if (type == RTM_NEWADDR) {
+		struct ip_pool_addr4_record *rec;
+
+		rec = l_new(struct ip_pool_addr4_record, 1);
+		rec->ifindex = ifa->ifa_index;
+		rec->addr = l_steal_ptr(addr);
+		l_queue_push_tail(used_addr4_list, rec);
+	} else if (type == RTM_DELADDR) {
+		struct ip_pool_addr4_record rec;
+
+		rec.ifindex = ifa->ifa_index;
+		rec.addr = addr;
+
+		l_queue_foreach_remove(used_addr4_list,
+					ip_pool_addr4_match_free, &rec);
+	}
+
+	l_rtnl_address_free(addr);
+}
+
+static void ip_pool_addr4_dump_cb(int error,
+					uint16_t type, const void *data,
+					uint32_t len, void *user_data)
+{
+	if (error) {
+		l_error("addr4_dump_cb: %s (%i)", strerror(-error), -error);
+		return;
+	}
+
+	ip_pool_addr_notify(type, data, len, user_data);
+}
+
+static int ip_pool_init(void)
+{
+	const struct l_settings *settings = iwd_get_config();
+	bool netconfig_enabled;
+
+	if (!l_settings_get_bool(settings, "General",
+				"EnableNetworkConfiguration",
+				&netconfig_enabled))
+		netconfig_enabled = false;
+
+	if (!netconfig_enabled)
+		return 0;
+
+	rtnl = iwd_get_rtnl();
+
+	if (!l_netlink_register(rtnl, RTNLGRP_IPV4_IFADDR,
+				ip_pool_addr_notify, NULL, NULL)) {
+		l_error("Failed to register for RTNL link notifications");
+		return -EIO;
+	}
+
+	if (!l_rtnl_ifaddr4_dump(rtnl, ip_pool_addr4_dump_cb, NULL, NULL)) {
+		l_error("Sending the IPv4 addr dump req failed");
+		return -EIO;
+	}
+
+	used_addr4_list = l_queue_new();
+	return 0;
+}
+
+static void ip_pool_exit(void)
+{
+	l_queue_destroy(used_addr4_list, ip_pool_addr4_record_free);
+	used_addr4_list = NULL;
+}
+
+IWD_MODULE(ip_pool, ip_pool_init, ip_pool_exit)
diff --git a/src/ip-pool.h b/src/ip-pool.h
new file mode 100644
index 00000000..f3c3011d
--- /dev/null
+++ b/src/ip-pool.h
@@ -0,0 +1,23 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2021  Intel Corporation. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct l_rtnl_address *ip_pool_get_addr4(uint32_t ifindex);
-- 
2.27.0

[PATCH 2/8] ip-pool: Add subnet address selection logic

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 7743 bytes --]

Add the ip_pool_select_addr4 function to select a random subnet of requested
size from an address space defined by a string list (for use with the
AP profile [IPv4].Address and the global [IPv4].APAddressPool settings),
avoiding those subnets that conflict with subnets in use.  We take care
to give a similar weight to all subnets contained in the specified
ranges regardless of how many ranges contain each, basically so that
overlapping ranges don't affect the probabilities (debatable.)
---
 src/ip-pool.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ip-pool.h |   2 +
 2 files changed, 208 insertions(+)

diff --git a/src/ip-pool.c b/src/ip-pool.c
index 51b3ef40..c8a70628 100644
--- a/src/ip-pool.c
+++ b/src/ip-pool.c
@@ -41,9 +41,215 @@ struct ip_pool_addr4_record {
 	struct l_rtnl_address *addr;
 };
 
+struct ip_pool_addr4_range {
+	uint32_t start;
+	uint32_t end;
+};
+
 static struct l_queue *used_addr4_list;
 static struct l_netlink *rtnl;
 
+static int ip_pool_addr4_range_compare(const void *a, const void *b,
+					void *user_data)
+{
+	const struct ip_pool_addr4_range *range_a = a;
+	const struct ip_pool_addr4_range *range_b = b;
+
+	return range_a->start > range_b->start ? 1 : -1;
+}
+
+/*
+ * 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 ip_pool_append_range(struct l_queue *to,
+					const struct ip_pool_addr4_range *range,
+					struct l_queue *used, const char *str)
+{
+	const struct l_queue_entry *entry = l_queue_get_entries(used);
+	const struct ip_pool_addr4_range *used_range =
+		entry ? entry->data : NULL;
+	uint32_t start = range->start;
+	bool print = true;
+
+	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 ip_pool_addr4_range *sub =
+				l_new(struct ip_pool_addr4_range, 1);
+
+			sub->start = start;
+			sub->end = range->end;
+			l_queue_push_tail(to, sub);
+			l_queue_insert(used, l_memdup(sub, sizeof(*sub)),
+					ip_pool_addr4_range_compare, NULL);
+			return;
+		}
+
+		if (print) {
+			l_debug("Address spec %s intersects with at least one "
+				"subnet already in use on the system or "
+				"specified in the settings", str);
+			print = false;
+		}
+
+		/* Now we know @used_range is non-NULL and intersects */
+		if (start < used_range->start) {
+			struct ip_pool_addr4_range *sub =
+				l_new(struct ip_pool_addr4_range, 1);
+
+			sub->start = start;
+			sub->end = used_range->start;
+			l_queue_push_tail(to, sub);
+			l_queue_insert(used, l_memdup(sub, sizeof(*sub)),
+					ip_pool_addr4_range_compare, NULL);
+		}
+
+		/* Skip to the start of the next subnet */
+		start = used_range->end;
+	}
+}
+
+/*
+ * Select a subnet and a host address from a defined space.
+ *
+ * Returns:  0 when an address was selected and written to *out_addr,
+ *          -EEXIST if all available subnet addresses are in use,
+ *          -EINVAL if there was a different error.
+ */
+int ip_pool_select_addr4(const char **addr_str_list, uint8_t subnet_prefix_len,
+				struct l_rtnl_address **out_addr)
+{
+	uint32_t total = 0;
+	uint32_t selected;
+	unsigned int i;
+	uint32_t subnet_size = 1 << (32 - subnet_prefix_len);
+	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 */
+	for (entry = l_queue_get_entries((struct l_queue *) used_addr4_list);
+			entry; entry = entry->next) {
+		const struct ip_pool_addr4_record *rec = entry->data;
+		struct ip_pool_addr4_range *range;
+		char addr_str[INET_ADDRSTRLEN];
+		uint32_t used_subnet_size = 1 <<
+			(32 - l_rtnl_address_get_prefix_length(rec->addr));
+
+		if (!l_rtnl_address_get_address(rec->addr, addr_str) ||
+				inet_aton(addr_str, &ia) < 0)
+			continue;
+
+		range = l_new(struct ip_pool_addr4_range, 1);
+		range->start = ntohl(ia.s_addr) & subnet_mask;
+		range->end = (range->start + used_subnet_size + subnet_size -
+				1) & subnet_mask;
+		l_queue_insert(used, range, ip_pool_addr4_range_compare, NULL);
+	}
+
+	/* 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 ip_pool_addr4_range range;
+
+		host_addr = ntohl(ia.s_addr);
+		range.start = host_addr & subnet_mask;
+		range.end = range.start + subnet_size;
+		ip_pool_append_range(ranges, &range, used, *addr_str_list);
+		goto check_avail;
+	}
+
+	for (i = 0; addr_str_list[i]; i++) {
+		struct ip_pool_addr4_range range;
+		uint32_t addr;
+		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 cleanup;
+		}
+
+		if (addr_prefix > subnet_prefix_len) {
+			l_debug("Address spec %s smaller than requested "
+				"subnet (prefix len %i)", addr_str_list[i],
+				subnet_prefix_len);
+			continue;
+		}
+
+		range.start = addr & subnet_mask;
+		range.end = range.start + (1 << (32 - addr_prefix));
+		ip_pool_append_range(ranges, &range, used, addr_str_list[i]);
+	}
+
+check_avail:
+	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 cleanup;
+	}
+
+	if (host_addr)
+		goto done;
+
+	/* Count available @subnet_prefix_len-sized subnets */
+	for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
+		struct ip_pool_addr4_range *range = entry->data;
+
+	total += (range->end - range->start) >>
+			(32 - subnet_prefix_len);
+	}
+
+	selected = l_getrandom_uint32() % total;
+
+	/* Find the @selected'th @subnet_prefix_len-sized subnet */
+	for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
+		struct ip_pool_addr4_range *range = entry->data;
+		uint32_t count = (range->end - range->start) >>
+			(32 - subnet_prefix_len);
+
+		if (selected < count) {
+			host_addr = range->start +
+				(selected << (32 - subnet_prefix_len));
+			break;
+		}
+
+		selected -= count;
+	}
+
+	if ((host_addr & 0xff) == 0)
+		host_addr += 1;
+
+done:
+	err = 0;
+	ia.s_addr = htonl(host_addr);
+	*out_addr = l_rtnl_address_new(inet_ntoa(ia), subnet_prefix_len);
+
+cleanup:
+	l_queue_destroy(ranges, l_free);
+	l_queue_destroy(used, l_free);
+	return err;
+}
+
 static void ip_pool_addr4_record_free(void *data)
 {
 	struct ip_pool_addr4_record *rec = data;
diff --git a/src/ip-pool.h b/src/ip-pool.h
index f3c3011d..2c832610 100644
--- a/src/ip-pool.h
+++ b/src/ip-pool.h
@@ -20,4 +20,6 @@
  *
  */
 
+int ip_pool_select_addr4(const char **addr_str_list, uint8_t subnet_prefix_len,
+				struct l_rtnl_address **out_addr);
 struct l_rtnl_address *ip_pool_get_addr4(uint32_t ifindex);
-- 
2.27.0

[PATCH 3/8] ap: Refactor DHCP settings loading

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 18696 bytes --]

Extend the [IPv4].Address setting's syntax to allow a new format: a list
of <IP>/<prefix_len> -form strings that define the address space from
which a subnet is selected.  Rewrite the DHCP settings loading with
other notable changes:

 * validate some of the settings more thoroughly,
 * name all netconfig-related ap_state members with the netconfig_
   prefix,
 * make sure we always call l_dhcp_server_set_netmask(),
 * allow netmasks other than 24-bit and change the default to 28 bits,
 * as requested avoid using the l_net_ ioctl-based functions although
   l_dhcp still uses them internally,
 * as requested avoid touching the ap_state members until the end of
   some functions so that on error they're basically a no-op (for
   readabaility).
---
 src/ap.c | 451 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 288 insertions(+), 163 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index fff7c664..33130f19 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -52,6 +52,7 @@
 #include "src/frame-xchg.h"
 #include "src/wscutil.h"
 #include "src/eap-wsc.h"
+#include "src/ip-pool.h"
 #include "src/ap.h"
 #include "src/storage.h"
 #include "src/diagnostic.h"
@@ -88,15 +89,15 @@ struct ap_state {
 	uint16_t last_aid;
 	struct l_queue *sta_states;
 
-	struct l_dhcp_server *server;
+	struct l_dhcp_server *netconfig_dhcp;
+	char *netconfig_addr4_str;
+	uint8_t netconfig_prefix_len4;
 	uint32_t rtnl_add_cmd;
-	char *own_ip;
-	unsigned int ip_prefix;
 
 	bool started : 1;
 	bool gtk_set : 1;
-	bool cleanup_ip : 1;
-	bool use_ip_pool : 1;
+	bool netconfig_set_addr4 : 1;
+	bool netconfig_use_ip_pool : 1;
 };
 
 struct sta_state {
@@ -181,7 +182,7 @@ static char *ip_pool_get()
 
 	/* This shouldn't happen */
 	if (next_subnet < pool.sub_start || next_subnet > pool.sub_end)
-		return NULL;
+		return 0;
 
 	l_uintset_put(pool.used, next_subnet);
 
@@ -321,23 +322,27 @@ static void ap_reset(struct ap_state *ap)
 	ap->started = false;
 
 	/* Delete IP if one was set by IWD */
-	if (ap->cleanup_ip)
+	if (ap->netconfig_set_addr4) {
 		l_rtnl_ifaddr4_delete(rtnl, netdev_get_ifindex(netdev),
-					ap->ip_prefix, ap->own_ip,
-					broadcast_from_ip(ap->own_ip),
-					NULL, NULL, NULL);
+				ap->netconfig_prefix_len4,
+				ap->netconfig_addr4_str,
+				broadcast_from_ip(ap->netconfig_addr4_str),
+				NULL, NULL, NULL);
+		ap->netconfig_set_addr4 = false;
+	}
 
-	if (ap->own_ip) {
+	if (ap->netconfig_use_ip_pool) {
 		/* Release IP from pool if used */
-		if (ap->use_ip_pool)
-			ip_pool_put(ap->own_ip);
-
-		l_free(ap->own_ip);
+		ip_pool_put(ap->netconfig_addr4_str);
+		ap->netconfig_use_ip_pool = false;
 	}
 
-	if (ap->server) {
-		l_dhcp_server_destroy(ap->server);
-		ap->server = NULL;
+	l_free(ap->netconfig_addr4_str);
+	ap->netconfig_addr4_str = NULL;
+
+	if (ap->netconfig_dhcp) {
+		l_dhcp_server_destroy(ap->netconfig_dhcp);
+		ap->netconfig_dhcp = NULL;
 	}
 }
 
@@ -2165,7 +2170,7 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 		goto failed;
 	}
 
-	if (ap->server && !l_dhcp_server_start(ap->server)) {
+	if (ap->netconfig_dhcp && !l_dhcp_server_start(ap->netconfig_dhcp)) {
 		l_error("DHCP server failed to start");
 		goto failed;
 	}
@@ -2492,182 +2497,290 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-static bool dhcp_load_settings(struct ap_state *ap,
-				const struct l_settings *settings)
+#define AP_DEFAULT_IPV4_PREFIX_LEN 28
+
+static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
+				uint8_t prefix_len, const char *gateway_str,
+				const char **ip_range,
+				const char **dns_str_list,
+				unsigned int lease_time)
 {
-	struct l_dhcp_server *server = ap->server;
+	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
+	struct l_rtnl_address *existing_addr = ip_pool_get_addr4(ifindex);
+	struct l_rtnl_address *new_addr = NULL;
+	int ret;
 	struct in_addr ia;
+	struct l_dhcp_server *dhcp = NULL;
+	bool use_ip_pool = false;
+	bool r;
+	char addr_str_buf[INET_ADDRSTRLEN];
+
+	dhcp = l_dhcp_server_new(ifindex);
+	if (!dhcp) {
+		l_error("Failed to create DHCP server on ifindex %u", ifindex);
+		ret = -EIO;
+		goto cleanup;
+	}
 
-	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",
-							"DNSList", ',');
-	char **ip_range = l_settings_get_string_list(settings, "IPv4",
-							"IPRange", ',');
-	unsigned int lease_time;
-	bool ret = false;
+	if (getenv("IWD_DHCP_DEBUG"))
+		l_dhcp_server_set_debug(dhcp, do_debug,
+					"[DHCPv4 SERV] ", NULL);
+
+	/*
+	 * The address pool specified for this AP (if any) has the priority,
+	 * next is the address currently set on the interface (if any) and
+	 * last is the global AP address pool (APRanges setting).
+	 */
+	if (addr_str_list) {
+		if (!prefix_len)
+			prefix_len = AP_DEFAULT_IPV4_PREFIX_LEN;
+
+		ret = ip_pool_select_addr4(addr_str_list, prefix_len,
+						&new_addr);
+	} else if (existing_addr &&
+			l_rtnl_address_get_prefix_length(existing_addr) <
+			31) {
+		if (!prefix_len)
+			prefix_len = l_rtnl_address_get_prefix_length(
+								existing_addr);
+
+		if (!l_rtnl_address_get_address(existing_addr, addr_str_buf)) {
+			ret = -EIO;
+			goto cleanup;
+		}
 
-	if (!l_settings_get_uint(settings, "IPv4", "LeaseTime", &lease_time))
-		lease_time = 0;
+		new_addr = l_rtnl_address_new(addr_str_buf, prefix_len);
+		ret = 0;
+	} else {
+		L_AUTO_FREE_VAR(char *, addr_str) = NULL;
 
-	if (gateway && !l_dhcp_server_set_gateway(server, gateway)) {
-		l_error("[IPv4].Gateway value error");
-		goto error;
+		if (prefix_len) {
+			l_error("[IPv4].Netmask can't be used without an "
+				"address");
+			ret = -EINVAL;
+			goto cleanup;
+		}
+
+		/* No config, no address set. Use IP pool */
+		addr_str = ip_pool_get();
+		if (addr_str) {
+			use_ip_pool = true;
+			prefix_len = pool.prefix;
+			new_addr = l_rtnl_address_new(addr_str, prefix_len);
+			ret = 0;
+		} else {
+			l_error("No more IP's in pool, cannot start AP on %u",
+					ifindex);
+			ret = -EEXIST;
+		}
 	}
 
-	if (dns && !l_dhcp_server_set_dns(server, dns)) {
-		l_error("[IPv4].DNSList value error");
-		goto error;
+	if (ret)
+		goto cleanup;
+
+	ret = -EIO;
+
+	/*
+	 * l_dhcp_server_start() would retrieve the current IPv4 from
+	 * the interface but set it anyway in case there are multiple
+	 * addresses, saves one ioctl too.
+	 */
+	if (!l_rtnl_address_get_address(new_addr, addr_str_buf)) {
+		l_error("l_rtnl_address_get_address failed");
+		goto cleanup;
 	}
 
-	if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
-		l_error("[IPv4].Netmask value error");
-		goto error;
+	if (!l_dhcp_server_set_ip_address(dhcp, addr_str_buf)) {
+		l_error("l_dhcp_server_set_ip_address failed");
+		goto cleanup;
+	}
+
+	ia.s_addr = htonl(util_netmask_from_prefix(prefix_len));
+
+	if (!l_dhcp_server_set_netmask(dhcp, inet_ntoa(ia))) {
+		l_error("l_dhcp_server_set_netmask failed");
+		goto cleanup;
+	}
+
+	if (gateway_str && !l_dhcp_server_set_gateway(dhcp, gateway_str)) {
+		l_error("l_dhcp_server_set_gateway failed");
+		goto cleanup;
 	}
 
 	if (ip_range) {
-		if (l_strv_length(ip_range) != 2) {
-			l_error("Two addresses expected in [IPv4].IPRange");
-			goto error;
+		r = l_dhcp_server_set_ip_range(dhcp, ip_range[0], ip_range[1]);
+		if (!r) {
+			l_error("l_dhcp_server_set_ip_range failed");
+			goto cleanup;
 		}
+	}
 
-		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 (dns_str_list) {
+		r = l_dhcp_server_set_dns(dhcp, (char **) dns_str_list);
+		if (!r) {
+			l_error("l_dhcp_server_set_dns failed");
+			goto cleanup;
 		}
 	}
 
-	if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time)) {
-		l_error("[IPv4].LeaseTime value error");
-		goto error;
+	if (lease_time && !l_dhcp_server_set_lease_time(dhcp, lease_time)) {
+		l_error("l_dhcp_server_set_lease_time failed");
+		goto cleanup;
 	}
 
-	if (netmask && inet_pton(AF_INET, netmask, &ia) > 0)
-		ap->ip_prefix = __builtin_popcountl(ia.s_addr);
-	else
-		ap->ip_prefix = 24;
+	ap->netconfig_addr4_str = l_strdup(addr_str_buf);
+	ap->netconfig_prefix_len4 = prefix_len;
+	ap->netconfig_set_addr4 = true;
+	ap->netconfig_use_ip_pool = use_ip_pool;
+	ap->netconfig_dhcp = l_steal_ptr(dhcp);
+	ret = 0;
 
-	ret = true;
+	if (existing_addr && l_rtnl_address_get_prefix_length(existing_addr) >
+			prefix_len) {
+		char addr_str_buf2[INET_ADDRSTRLEN];
 
-error:
-	l_strv_free(dns);
-	l_strv_free(ip_range);
+		if (l_rtnl_address_get_address(existing_addr, addr_str_buf2) &&
+				!strcmp(addr_str_buf, addr_str_buf2))
+			ap->netconfig_set_addr4 = false;
+	}
+
+cleanup:
+	l_dhcp_server_destroy(dhcp);
+	l_rtnl_address_free(new_addr);
+	l_rtnl_address_free(existing_addr);
 	return ret;
 }
 
-/*
- * This will determine the IP being used for DHCP. The IP will be automatically
- * set to ap->own_ip.
- *
- * 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.
- *
- * Returns:  0 if an IP was successfully selected and needs to be set
- *          -EALREADY if an IP was already set on the interface or
- *            IP configuration was not enabled,
- *          -EEXIST if the IP pool ran out of IP's
- *          -EINVAL if there was an error.
- */
-static int ap_setup_dhcp(struct ap_state *ap, const struct l_settings *settings)
+static int ap_load_ipv4(struct ap_state *ap, const struct l_settings *config)
 {
-	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
-	struct in_addr ia;
-	uint32_t address = 0;
-	L_AUTO_FREE_VAR(char *, addr) = NULL;
 	int ret = -EINVAL;
+	char **addr_str_list = NULL;
+	uint32_t static_addr = 0;
+	uint8_t prefix_len = 0;
+	char *gateway_str = NULL;
+	char **ip_range = NULL;
+	char **dns_str_list = NULL;
+	unsigned int lease_time = 0;
+	struct in_addr ia;
 
-	/* 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_settings_has_group(config, "IPv4") || !pool.used)
+		return 0;
 
-	addr = l_settings_get_string(settings, "IPv4", "Address");
-	if (addr) {
-		if (inet_pton(AF_INET, addr, &ia) < 0)
-			return -EINVAL;
+	if (l_settings_has_key(config, "IPv4", "Address")) {
+		addr_str_list = l_settings_get_string_list(config, "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;
+		}
 
-		/* 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 if (pool.used) {
-		/* No config file, no address set. Use IP pool */
-		addr = ip_pool_get();
-		if (!addr) {
-			l_error("No more IP's in pool, cannot start AP on %u",
-					ifindex);
-			return -EEXIST;
+		/* Check for the static IP syntax: Address=<IP> */
+		if (l_strv_length(addr_str_list) == 1 &&
+				inet_pton(AF_INET, *addr_str_list, &ia) == 1)
+			static_addr = ntohl(ia.s_addr);
+	}
+
+	if (l_settings_has_key(config, "IPv4", "Netmask")) {
+		L_AUTO_FREE_VAR(char *, netmask_str) =
+			l_settings_get_string(config, "IPv4", "Netmask");
+
+		if (inet_pton(AF_INET, netmask_str, &ia) != 1) {
+			l_error("Can't parse the profile [IPv4].Netmask "
+				"setting");
+			goto done;
 		}
 
-		ap->use_ip_pool = true;
-		ap->ip_prefix = pool.prefix;
-		ret = 0;
-	} else
-		return -EALREADY;
+		prefix_len = __builtin_popcount(ia.s_addr);
 
-	ap->server = l_dhcp_server_new(ifindex);
-	if (!ap->server) {
-		l_error("Failed to create DHCP server on %u", ifindex);
-		return -EINVAL;
+		if (ntohl(ia.s_addr) != util_netmask_from_prefix(prefix_len)) {
+			l_error("Invalid profile [IPv4].Netmask value");
+			goto done;
+		}
 	}
 
-	if (getenv("IWD_DHCP_DEBUG"))
-		l_dhcp_server_set_debug(ap->server, do_debug,
-							"[DHCPv4 SERV] ", NULL);
+	if (l_settings_has_key(config, "IPv4", "Gateway")) {
+		gateway_str = l_settings_get_string(config, "IPv4", "Gateway");
+		if (!gateway_str) {
+			l_error("Invalid profile [IPv4].Gateway value");
+			goto done;
+		}
+	}
 
-	/* Set the remaining DHCP options in config file */
-	if (!dhcp_load_settings(ap, settings))
-		return -EINVAL;
+	if (l_settings_get_value(config, "IPv4", "IPRange")) {
+		int i;
+		uint32_t netmask;
+		uint8_t tmp_len = prefix_len ?: AP_DEFAULT_IPV4_PREFIX_LEN;
 
-	if (!l_dhcp_server_set_ip_address(ap->server, addr))
-		return -EINVAL;
+		ip_range = l_settings_get_string_list(config, "IPv4",
+							"IPRange", ',');
 
-	ap->own_ip = l_steal_ptr(addr);
-	return ret;
-}
+		if (!static_addr) {
+			l_error("[IPv4].IPRange only makes sense in an AP "
+				"profile if a static local address has also "
+				"been specified");
+			goto done;
+		}
 
-static int ap_load_dhcp(struct ap_state *ap, const struct l_settings *config,
-			bool *wait_dhcp)
-{
-	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
-	int err = -EINVAL;
+		if (!ip_range || l_strv_length(ip_range) != 2) {
+			l_error("Can't parse the profile [IPv4].IPRange "
+				"setting as two address strings");
+			goto done;
+		}
 
-	if (!l_settings_has_group(config, "IPv4"))
-		return 0;
+		netmask = util_netmask_from_prefix(tmp_len);
 
-	err = ap_setup_dhcp(ap, config);
-	if (err == 0) {
-		/* Address change required */
-		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
-					ap->ip_prefix, ap->own_ip,
-					broadcast_from_ip(ap->own_ip),
-					ap_ifaddr4_added_cb, ap, NULL);
+		for (i = 0; i < 2; i++) {
+			struct in_addr range_addr;
 
-		if (!ap->rtnl_add_cmd) {
-			l_error("Failed to add IPv4 address");
-			return -EIO;
+			if (inet_aton(ip_range[i], &range_addr) != 1) {
+				l_error("Can't parse address in "
+					"[IPv4].IPRange[%i]", i + 1);
+				goto done;
+			}
+
+			if ((static_addr ^ ntohl(range_addr.s_addr)) &
+					netmask) {
+				ia.s_addr = htonl(static_addr);
+				l_error("[IPv4].IPRange[%i] is not in the "
+					"%s/%i subnet", i + 1, inet_ntoa(ia),
+					tmp_len);
+				goto done;
+			}
 		}
+	}
 
-		ap->cleanup_ip = true;
+	if (l_settings_has_key(config, "IPv4", "DNSList")) {
+		dns_str_list = l_settings_get_string_list(config, "IPv4",
+								"DNSList", ',');
+		if (!dns_str_list || !*dns_str_list) {
+			l_error("Can't parse the profile [IPv4].DNSList "
+				"setting as a string list");
+			goto done;
+		}
+	}
 
-		*wait_dhcp = true;
-		err = 0;
-	/* Selected address already set, continue normally */
-	} else if (err == -EALREADY) {
-		*wait_dhcp = false;
-		err = 0;
+	if (l_settings_has_key(config, "IPv4", "LeaseTime")) {
+		if (!l_settings_get_uint(config, "IPv4", "LeaseTime",
+						&lease_time) ||
+				lease_time < 1) {
+			l_error("Error parsing [IPv4].LeaseTime as a positive "
+				"integer");
+			goto done;
+		}
 	}
 
-	return err;
+	ret = ap_setup_netconfig4(ap, (const char **) addr_str_list, prefix_len,
+					gateway_str, (const char **) ip_range,
+					(const char **) dns_str_list,
+					lease_time);
+
+done:
+	l_strv_free(addr_str_list);
+	l_free(gateway_str);
+	l_strv_free(ip_range);
+	l_strv_free(dns_str_list);
+	return ret;
 }
 
 static bool ap_load_psk(struct ap_state *ap, const struct l_settings *config)
@@ -2722,7 +2835,7 @@ static bool ap_load_psk(struct ap_state *ap, const struct l_settings *config)
 }
 
 static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
-				bool *out_wait_dhcp, bool *out_cck_rates)
+				bool *out_cck_rates)
 {
 	size_t len;
 	L_AUTO_FREE_VAR(char *, strval) = NULL;
@@ -2748,11 +2861,12 @@ static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
 		return -EINVAL;
 
 	/*
-	 * This loads DHCP settings either from the l_settings object or uses
-	 * the defaults. wait_on_address will be set true if an address change
-	 * is required.
+	 * This looks@the network configuration settings in @config and
+	 * relevant global settings and if it determines that netconfig is to
+	 * be enabled for the AP, it both creates the DHCP server object and
+	 * processes IP settings, applying the defaults where needed.
 	 */
-	err = ap_load_dhcp(ap, config, out_wait_dhcp);
+	err = ap_load_ipv4(ap, config);
 	if (err)
 		return err;
 
@@ -2868,15 +2982,15 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	struct ap_state *ap;
 	struct wiphy *wiphy = netdev_get_wiphy(netdev);
 	uint64_t wdev_id = netdev_get_wdev_id(netdev);
-	int err = -EINVAL;
-	bool wait_on_address = false;
+	int err;
 	bool cck_rates = true;
 
-	if (err_out)
-		*err_out = err;
+	if (L_WARN_ON(!config)) {
+		if (err_out)
+			*err_out = -EINVAL;
 
-	if (L_WARN_ON(!config))
 		return NULL;
+	}
 
 	ap = l_new(struct ap_state, 1);
 	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
@@ -2884,7 +2998,7 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	ap->ops = ops;
 	ap->user_data = user_data;
 
-	err = ap_load_config(ap, config, &wait_on_address, &cck_rates);
+	err = ap_load_config(ap, config, &cck_rates);
 	if (err)
 		goto error;
 
@@ -2950,9 +3064,20 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	if (!ap->mlme_watch)
 		l_error("Registering for MLME notification failed");
 
-	if (wait_on_address) {
-		if (err_out)
-			*err_out = 0;
+	if (ap->netconfig_set_addr4) {
+		const char *broadcast_str =
+			broadcast_from_ip(ap->netconfig_addr4_str);
+
+		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl,
+						netdev_get_ifindex(netdev),
+						ap->netconfig_prefix_len4,
+						ap->netconfig_addr4_str,
+						broadcast_str,
+						ap_ifaddr4_added_cb, ap, NULL);
+		if (!ap->rtnl_add_cmd) {
+			l_error("Failed to add the IPv4 address");
+			goto error;
+		}
 
 		return ap;
 	}
-- 
2.27.0

[PATCH 4/8] ap: Refactor global address pool loading

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 7983 bytes --]

Deprecate the global [General].APRanges setting in favour of
[IPv4].APAddressPool with an extended (but backwards-compatible) syntax.
Drop the existing address pool creation code.

The new APAddressPool setting has the same syntax as the profile-local
[IPv4].Address setting and the subnet selection code will fall back
to the global setting if it's missing, this way we use common code to
handle both settings.
---
 src/ap.c | 185 +++++++++++++------------------------------------------
 1 file changed, 42 insertions(+), 143 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 33130f19..9ffccd04 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -97,7 +97,6 @@ struct ap_state {
 	bool started : 1;
 	bool gtk_set : 1;
 	bool netconfig_set_addr4 : 1;
-	bool netconfig_use_ip_pool : 1;
 };
 
 struct sta_state {
@@ -128,99 +127,11 @@ 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;
+static bool netconfig_enabled;
+static char **global_addr4_strs;
 static uint32_t netdev_watch;
 static 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 0;
-
-	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;
@@ -331,12 +242,6 @@ static void ap_reset(struct ap_state *ap)
 		ap->netconfig_set_addr4 = false;
 	}
 
-	if (ap->netconfig_use_ip_pool) {
-		/* Release IP from pool if used */
-		ip_pool_put(ap->netconfig_addr4_str);
-		ap->netconfig_use_ip_pool = false;
-	}
-
 	l_free(ap->netconfig_addr4_str);
 	ap->netconfig_addr4_str = NULL;
 
@@ -2511,7 +2416,6 @@ static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
 	int ret;
 	struct in_addr ia;
 	struct l_dhcp_server *dhcp = NULL;
-	bool use_ip_pool = false;
 	bool r;
 	char addr_str_buf[INET_ADDRSTRLEN];
 
@@ -2552,27 +2456,11 @@ static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
 		new_addr = l_rtnl_address_new(addr_str_buf, prefix_len);
 		ret = 0;
 	} else {
-		L_AUTO_FREE_VAR(char *, addr_str) = NULL;
-
-		if (prefix_len) {
-			l_error("[IPv4].Netmask can't be used without an "
-				"address");
-			ret = -EINVAL;
-			goto cleanup;
-		}
+		if (!prefix_len)
+			prefix_len = AP_DEFAULT_IPV4_PREFIX_LEN;
 
-		/* No config, no address set. Use IP pool */
-		addr_str = ip_pool_get();
-		if (addr_str) {
-			use_ip_pool = true;
-			prefix_len = pool.prefix;
-			new_addr = l_rtnl_address_new(addr_str, prefix_len);
-			ret = 0;
-		} else {
-			l_error("No more IP's in pool, cannot start AP on %u",
-					ifindex);
-			ret = -EEXIST;
-		}
+		ret = ip_pool_select_addr4((const char **) global_addr4_strs,
+						prefix_len, &new_addr);
 	}
 
 	if (ret)
@@ -2631,7 +2519,6 @@ static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
 	ap->netconfig_addr4_str = l_strdup(addr_str_buf);
 	ap->netconfig_prefix_len4 = prefix_len;
 	ap->netconfig_set_addr4 = true;
-	ap->netconfig_use_ip_pool = use_ip_pool;
 	ap->netconfig_dhcp = l_steal_ptr(dhcp);
 	ret = 0;
 
@@ -2663,7 +2550,7 @@ static int ap_load_ipv4(struct ap_state *ap, const struct l_settings *config)
 	unsigned int lease_time = 0;
 	struct in_addr ia;
 
-	if (!l_settings_has_group(config, "IPv4") || !pool.used)
+	if (!l_settings_has_group(config, "IPv4") || !netconfig_enabled)
 		return 0;
 
 	if (l_settings_has_key(config, "IPv4", "Address")) {
@@ -3652,7 +3539,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);
 
@@ -3663,31 +3549,44 @@ static int ap_init(void)
 			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.
+	 * Enable network configuration and DHCP only if
+	 * [General].EnableNetworkConfiguration is true.
 	 */
 	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;
+					"EnableNetworkConfiguration",
+					&netconfig_enabled))
+		netconfig_enabled = false;
+
+	if (netconfig_enabled) {
+		if (l_settings_get_value(settings, "IPv4", "APAddressPool")) {
+			global_addr4_strs = l_settings_get_string_list(settings,
+								"IPv4",
+								"APAddressPool",
+								',');
+			if (!global_addr4_strs || !*global_addr4_strs) {
+				l_error("Can't parse the [IPv4].APAddressPool "
+					"setting as a string list");
+				l_strv_free(global_addr4_strs);
+				global_addr4_strs = NULL;
+			}
+		} else if (l_settings_get_value(settings,
+						"General", "APRanges")) {
+			global_addr4_strs = l_settings_get_string_list(settings,
+								"General",
+								"APRanges",
+								',');
+			if (!global_addr4_strs || !*global_addr4_strs) {
+				l_error("Can't parse the [General].APRanges "
+					"setting as a string list");
+				l_strv_free(global_addr4_strs);
+				global_addr4_strs = NULL;
+			}
 		}
 
-		if (!ip_pool_create(ip_prefix))
-			return -EINVAL;
+		/* Fall back to 192.168.0.0/16 */
+		if (!global_addr4_strs)
+			global_addr4_strs =
+				l_strv_append(NULL, "192.168.0.0/16");
 	}
 
 	rtnl = iwd_get_rtnl();
@@ -3700,7 +3599,7 @@ static void ap_exit(void)
 	netdev_watch_remove(netdev_watch);
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
 
-	ip_pool_destroy();
+	l_strv_free(global_addr4_strs);
 }
 
 IWD_MODULE(ap, ap_init, ap_exit)
-- 
2.27.0

[PATCH 5/8] ap: Send a specific error message on async AP start failure

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3543 bytes --]

We generate the DBus error reply type from the errno only when
ap_start() was failing synchronously, now also send the errno through
the callbacks so that we can also return a specific DBus reply when
failing asynchronously.  Thea AP autotest relies on receiving the
AlreadyExists DBus error.
---
 src/ap.c | 32 ++++++++++++++++++--------------
 src/ap.h |  4 ++++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 9ffccd04..430fdfa3 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2054,9 +2054,11 @@ static void do_debug(const char *str, void *user_data)
 	l_info("%s%s", prefix, str);
 }
 
-static void ap_start_failed(struct ap_state *ap)
+static void ap_start_failed(struct ap_state *ap, int err)
 {
-	ap->ops->handle_event(AP_EVENT_START_FAILED, NULL, ap->user_data);
+	struct ap_event_start_failed_data data = { err };
+
+	ap->ops->handle_event(AP_EVENT_START_FAILED, &data, ap->user_data);
 	ap_reset(ap);
 	l_genl_family_free(ap->nl80211);
 
@@ -2071,22 +2073,18 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 
 	if (l_genl_msg_get_error(msg) < 0) {
 		l_error("START_AP failed: %i", l_genl_msg_get_error(msg));
-
-		goto failed;
+		ap_start_failed(ap, l_genl_msg_get_error(msg));
+		return;
 	}
 
 	if (ap->netconfig_dhcp && !l_dhcp_server_start(ap->netconfig_dhcp)) {
 		l_error("DHCP server failed to start");
-		goto failed;
+		ap_start_failed(ap, -EINVAL);
+		return;
 	}
 
 	ap->started = true;
 	ap->ops->handle_event(AP_EVENT_STARTED, NULL, ap->user_data);
-
-	return;
-
-failed:
-	ap_start_failed(ap);
 }
 
 static struct l_genl_msg *ap_build_cmd_start_ap(struct ap_state *ap)
@@ -2201,12 +2199,12 @@ static void ap_ifaddr4_added_cb(int error, uint16_t type, const void *data,
 
 	if (error) {
 		l_error("Failed to set IP address");
-		ap_start_failed(ap);
+		ap_start_failed(ap, error);
 		return;
 	}
 
 	if (!ap_start_send(ap))
-		ap_start_failed(ap);
+		ap_start_failed(ap, -EIO);
 }
 
 static bool ap_parse_new_station_ies(const void *data, uint16_t len,
@@ -2378,10 +2376,12 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	switch (l_genl_msg_get_command(msg)) {
 	case NL80211_CMD_STOP_AP:
 		if (ap->start_stop_cmd_id) {
+			struct ap_event_start_failed_data data = { -ECANCELED };
+
 			l_genl_family_cancel(ap->nl80211,
 						ap->start_stop_cmd_id);
 			ap->start_stop_cmd_id = 0;
-			ap->ops->handle_event(AP_EVENT_START_FAILED, NULL,
+			ap->ops->handle_event(AP_EVENT_START_FAILED, &data,
 						ap->user_data);
 		} else if (ap->started) {
 			ap->started = false;
@@ -3164,13 +3164,17 @@ static void ap_if_event_func(enum ap_event_type type, const void *event_data,
 
 	switch (type) {
 	case AP_EVENT_START_FAILED:
+	{
+		const struct ap_event_start_failed_data *data = event_data;
+
 		if (L_WARN_ON(!ap_if->pending))
 			break;
 
-		reply = dbus_error_failed(ap_if->pending);
+		reply = dbus_error_from_errno(data->error, ap_if->pending);
 		dbus_pending_reply(&ap_if->pending, reply);
 		ap_if->ap = NULL;
 		break;
+	}
 
 	case AP_EVENT_STARTED:
 		if (L_WARN_ON(!ap_if->pending))
diff --git a/src/ap.h b/src/ap.h
index b0100f34..9a03d3ae 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -35,6 +35,10 @@ enum ap_event_type {
 	AP_EVENT_PBC_MODE_EXIT,
 };
 
+struct ap_event_start_failed_data {
+	int error;
+};
+
 struct ap_event_station_added_data {
 	const uint8_t *mac;
 	const uint8_t *assoc_ies;
-- 
2.27.0

[PATCH 6/8] ap: Save AP address as l_rtnl_address

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3360 bytes --]

Change the char *addr_str and uint8_t prefix_len pair to an
l_rtnl_address object and use ell/rtnl.h utilities that use that
directly.  Extend broadcast_from_ip to handle prefix_len.
---
 src/ap.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 430fdfa3..dc067352 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -90,8 +90,7 @@ struct ap_state {
 	struct l_queue *sta_states;
 
 	struct l_dhcp_server *netconfig_dhcp;
-	char *netconfig_addr4_str;
-	uint8_t netconfig_prefix_len4;
+	struct l_rtnl_address *netconfig_addr4;
 	uint32_t rtnl_add_cmd;
 
 	bool started : 1;
@@ -132,20 +131,15 @@ static char **global_addr4_strs;
 static uint32_t netdev_watch;
 static struct l_netlink *rtnl;
 
-static const char *broadcast_from_ip(const char *ip)
+static const char *broadcast_from_ip(const char *ip, uint8_t prefix_len)
 {
 	struct in_addr ia;
-	uint32_t bcast;
+	uint32_t netmask = util_netmask_from_prefix(prefix_len);
 
 	if (inet_aton(ip, &ia) != 1)
 		return NULL;
 
-	bcast = ntohl(ia.s_addr);
-	bcast &= 0xffffff00;
-	bcast |= 0x000000ff;
-
-	ia.s_addr = htonl(bcast);
-
+	ia.s_addr |= htonl(~netmask);
 	return inet_ntoa(ia);
 }
 
@@ -234,16 +228,12 @@ static void ap_reset(struct ap_state *ap)
 
 	/* Delete IP if one was set by IWD */
 	if (ap->netconfig_set_addr4) {
-		l_rtnl_ifaddr4_delete(rtnl, netdev_get_ifindex(netdev),
-				ap->netconfig_prefix_len4,
-				ap->netconfig_addr4_str,
-				broadcast_from_ip(ap->netconfig_addr4_str),
-				NULL, NULL, NULL);
+		l_rtnl_ifaddr_delete(rtnl, netdev_get_ifindex(netdev),
+					ap->netconfig_addr4, NULL, NULL, NULL);
 		ap->netconfig_set_addr4 = false;
 	}
 
-	l_free(ap->netconfig_addr4_str);
-	ap->netconfig_addr4_str = NULL;
+	l_rtnl_address_free(l_steal_ptr(ap->netconfig_addr4));
 
 	if (ap->netconfig_dhcp) {
 		l_dhcp_server_destroy(ap->netconfig_dhcp);
@@ -2454,6 +2444,13 @@ static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
 		}
 
 		new_addr = l_rtnl_address_new(addr_str_buf, prefix_len);
+
+		if (!l_rtnl_address_set_broadcast(new_addr,
+				broadcast_from_ip(addr_str_buf, prefix_len))) {
+			ret = -EIO;
+			goto cleanup;
+		}
+
 		ret = 0;
 	} else {
 		if (!prefix_len)
@@ -2516,9 +2513,8 @@ static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
 		goto cleanup;
 	}
 
-	ap->netconfig_addr4_str = l_strdup(addr_str_buf);
-	ap->netconfig_prefix_len4 = prefix_len;
 	ap->netconfig_set_addr4 = true;
+	ap->netconfig_addr4 = l_steal_ptr(new_addr);
 	ap->netconfig_dhcp = l_steal_ptr(dhcp);
 	ret = 0;
 
@@ -2952,14 +2948,9 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 		l_error("Registering for MLME notification failed");
 
 	if (ap->netconfig_set_addr4) {
-		const char *broadcast_str =
-			broadcast_from_ip(ap->netconfig_addr4_str);
-
-		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl,
+		ap->rtnl_add_cmd = l_rtnl_ifaddr_add(rtnl,
 						netdev_get_ifindex(netdev),
-						ap->netconfig_prefix_len4,
-						ap->netconfig_addr4_str,
-						broadcast_str,
+						ap->netconfig_addr4,
 						ap_ifaddr4_added_cb, ap, NULL);
 		if (!ap->rtnl_add_cmd) {
 			l_error("Failed to add the IPv4 address");
-- 
2.27.0

[PATCH 7/8] autotests: Update APRanges usage in testAP

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2087 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 +++-
 autotests/testAP/dhcp_test.py   | 12 +++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

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
diff --git a/autotests/testAP/dhcp_test.py b/autotests/testAP/dhcp_test.py
index bf52fb28..e69c4465 100644
--- a/autotests/testAP/dhcp_test.py
+++ b/autotests/testAP/dhcp_test.py
@@ -58,8 +58,14 @@ class Test(unittest.TestCase):
             testutil.test_iface_operstate(dev2.name)
             testutil.test_ifaces_connected(dev1.name, dev2.name, group=False)
 
-            testutil.test_ip_address_match(dev1.name, "192.168.80.1")
-            testutil.test_ip_address_match(dev2.name, "192.168.80.2")
+            try:
+                testutil.test_ip_address_match(dev1.name, "192.168.80.1")
+                testutil.test_ip_address_match(dev2.name, "192.168.80.2")
+                ip = "192.168.80.1"
+            except:
+                testutil.test_ip_address_match(dev1.name, "192.168.80.17")
+                testutil.test_ip_address_match(dev2.name, "192.168.80.18")
+                ip = "192.168.80.17"
 
             wd.unregister_psk_agent(psk_agent)
 
@@ -76,7 +82,7 @@ class Test(unittest.TestCase):
             # got initially.
             dev4.start_ap('TestAP4', 'Password4')
 
-            testutil.test_ip_address_match(dev4.name, "192.168.80.1")
+            testutil.test_ip_address_match(dev4.name, ip)
 
         finally:
             dev1.stop_ap()
-- 
2.27.0

[PATCH 8/8] doc: Update AP settings in iwd.ap(5) and iwd.config(5)

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 4922 bytes --]

---
 src/iwd.ap.rst     | 31 ++++++++++++++++++++++++-------
 src/iwd.config.rst | 29 ++++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/src/iwd.ap.rst b/src/iwd.ap.rst
index 51622509..5fa23179 100644
--- a/src/iwd.ap.rst
+++ b/src/iwd.ap.rst
@@ -104,14 +104,31 @@ is desired, the group header line must still be present:
    :widths: 20 80
 
    * - Address
-     - Local IP address
+     - Local IP address or a comma-separated list of prefix-notation addresses
 
        Optional local address pool for the access point and the DHCP server.
-       If provided this addresss will be set on the AP interface and any other
-       DHCP server options will be derived from it, unless they are overridden
-       by other settings below.  If *Address* is not provided and no IP
-       address is set on the interface prior to calling `StartProfile`,  the IP
-       pool defined by the global ``[General].APRanges`` setting will be used.
+       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
+       if not overridden by other settings below.
+
+       If a list of addresses and prefix lengths is specified (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 is based on the ``[IPv4].Netmask`` setting.
+
+       If *Address* is not provided and no IP address is set on the
+       interface prior to calling `StartProfile` the value of the main.conf
+       ``[IPv4].APAddressPool`` setting will be inherited, which in turn
+       defaults to 192.168.0.0/16.
+
+       For example, if ``[IPv4].Netmask`` is set to 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 desired subnet gives
+       IWD a chance to avoid conflicts if other interfaces on the system use
+       dynamically assigned addresses.
 
    * - Gateway
      - IP Address of gateway
@@ -122,7 +139,7 @@ is desired, the group header line must still be present:
    * - Netmask
      - 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 as a comma-separated IP address list
diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 60f5b5db..78ec284e 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -67,8 +67,8 @@ 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 not override it.
 
        The network configuration feature is disabled by default.  See
        ``[Network]`` settings for additional settings related to network
@@ -182,7 +182,7 @@ The group ``[General]`` contains general settings.
        then setting ``DisableANQP`` to ``false`` is recommended.
 
 Network
----------
+-------
 
 The group ``[Network]`` contains network configuration related settings.
 
@@ -319,6 +319,29 @@ 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 network configuration.
+
+.. list-table::
+   :header-rows: 0
+   :stub-columns: 0
+   :widths: 20 80
+   :align: left
+
+   * - APAddressPool
+     - Values: comma-separated list of prefix-notation IP strings
+
+       Defines the space of IPs used for the Access Point-mode subnet addresses
+       and the DHCP server.  Defaults to 192.168.0.0/16.  The prefix length
+       decides the size of the pool 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 set 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

Re: [PATCH 1/8] ip-pool: Track IPv4 addresses in use

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Add the ip-pool submodule that tracks IPv4 addresses in use on the
> system for use when selecting the address for a new AP.  l_rtnl_address
> is used internally because if we're going to return l_rtnl_address
> objects it would be misleading if we didn't fill in all of their
> properties like flags etc.
> ---
>   Makefile.am   |   1 +
>   src/ip-pool.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/ip-pool.h |  23 +++++++
>   3 files changed, 199 insertions(+)
>   create mode 100644 src/ip-pool.c
>   create mode 100644 src/ip-pool.h
> 

rc/ip-pool.c: In function ‘ip_pool_get_addr4’:
src/ip-pool.c:68:15: error: implicit declaration of function 
‘l_rtnl_address_dup’; did you mean ‘l_rtnl_address_new’? 
[-Werror=implicit-function-declaration]
    68 |  return rec ? l_rtnl_address_dup(rec->addr) : 0;
       |               ^~~~~~~~~~~~~~~~~~
       |               l_rtnl_address_new
src/ip-pool.c:68:45: error: returning ‘int’ from a function with return type 
‘struct l_rtnl_address *’ makes pointer from integer without a cast 
[-Werror=int-conversion]
    68 |  return rec ? l_rtnl_address_dup(rec->addr) : 0;
       |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~


Regards,
-Denis

Re: [PATCH 1/8] ip-pool: Track IPv4 addresses in use

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

Hi Denis,

On Fri, 28 May 2021 at 16:22, Denis Kenzior <denkenz@gmail.com> wrote:
> On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> src/ip-pool.c:68:15: error: implicit declaration of function
> ‘l_rtnl_address_dup’; did you mean ‘l_rtnl_address_new’?

Oops, sure enough I forgot to send the ell patch..

Best regards

Re: [PATCH 1/8] ip-pool: Track IPv4 addresses in use

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Add the ip-pool submodule that tracks IPv4 addresses in use on the
> system for use when selecting the address for a new AP.  l_rtnl_address
> is used internally because if we're going to return l_rtnl_address
> objects it would be misleading if we didn't fill in all of their
> properties like flags etc.
> ---
>   Makefile.am   |   1 +
>   src/ip-pool.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/ip-pool.h |  23 +++++++
>   3 files changed, 199 insertions(+)
>   create mode 100644 src/ip-pool.c
>   create mode 100644 src/ip-pool.h
> 

I s/l_rtnl_address_dup/l_rtnl_address_clone to match ell changes and..

Applied, thanks.

Regards,
-Denis

Re: [PATCH 3/8] ap: Refactor DHCP settings loading

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Extend the [IPv4].Address setting's syntax to allow a new format: a list
> of <IP>/<prefix_len> -form strings that define the address space from
> which a subnet is selected.  Rewrite the DHCP settings loading with
> other notable changes:
> 
>   * validate some of the settings more thoroughly,
>   * name all netconfig-related ap_state members with the netconfig_
>     prefix,
>   * make sure we always call l_dhcp_server_set_netmask(),
>   * allow netmasks other than 24-bit and change the default to 28 bits,
>   * as requested avoid using the l_net_ ioctl-based functions although
>     l_dhcp still uses them internally,
>   * as requested avoid touching the ap_state members until the end of
>     some functions so that on error they're basically a no-op (for
>     readabaility).

I fixed this typo above and..

> ---
>   src/ap.c | 451 +++++++++++++++++++++++++++++++++++--------------------
>   1 file changed, 288 insertions(+), 163 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 2/8] ip-pool: Add subnet address selection logic

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Add the ip_pool_select_addr4 function to select a random subnet of requested
> size from an address space defined by a string list (for use with the
> AP profile [IPv4].Address and the global [IPv4].APAddressPool settings),
> avoiding those subnets that conflict with subnets in use.  We take care
> to give a similar weight to all subnets contained in the specified
> ranges regardless of how many ranges contain each, basically so that
> overlapping ranges don't affect the probabilities (debatable.)
> ---
>   src/ip-pool.c | 206 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/ip-pool.h |   2 +
>   2 files changed, 208 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 4/8] ap: Refactor global address pool loading

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Deprecate the global [General].APRanges setting in favour of
> [IPv4].APAddressPool with an extended (but backwards-compatible) syntax.
> Drop the existing address pool creation code.
> 
> The new APAddressPool setting has the same syntax as the profile-local
> [IPv4].Address setting and the subnet selection code will fall back
> to the global setting if it's missing, this way we use common code to
> handle both settings.
> ---
>   src/ap.c | 185 +++++++++++++------------------------------------------
>   1 file changed, 42 insertions(+), 143 deletions(-)
> 

<snip>

> +	if (netconfig_enabled) {
> +		if (l_settings_get_value(settings, "IPv4", "APAddressPool")) {
> +			global_addr4_strs = l_settings_get_string_list(settings,
> +								"IPv4",
> +								"APAddressPool",
> +								',');
> +			if (!global_addr4_strs || !*global_addr4_strs) {
> +				l_error("Can't parse the [IPv4].APAddressPool "
> +					"setting as a string list");
> +				l_strv_free(global_addr4_strs);
> +				global_addr4_strs = NULL;
> +			}
> +		} else if (l_settings_get_value(settings,
> +						"General", "APRanges")) {
> +			global_addr4_strs = l_settings_get_string_list(settings,
> +								"General",
> +								"APRanges",
> +								',');

Do you want to print a warning that this setting is deprecated?

> +			if (!global_addr4_strs || !*global_addr4_strs) {
> +				l_error("Can't parse the [General].APRanges "
> +					"setting as a string list");
> +				l_strv_free(global_addr4_strs);
> +				global_addr4_strs = NULL;
> +			}
>   		}
>   

Applied, thanks.

Regards,
-Denis

Re: [PATCH 5/8] ap: Send a specific error message on async AP start failure

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> We generate the DBus error reply type from the errno only when
> ap_start() was failing synchronously, now also send the errno through
> the callbacks so that we can also return a specific DBus reply when
> failing asynchronously.  Thea AP autotest relies on receiving the
> AlreadyExists DBus error.
> ---
>   src/ap.c | 32 ++++++++++++++++++--------------
>   src/ap.h |  4 ++++
>   2 files changed, 22 insertions(+), 14 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 6/8] ap: Save AP address as l_rtnl_address

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> Change the char *addr_str and uint8_t prefix_len pair to an
> l_rtnl_address object and use ell/rtnl.h utilities that use that
> directly.  Extend broadcast_from_ip to handle prefix_len.
> ---
>   src/ap.c | 43 +++++++++++++++++--------------------------
>   1 file changed, 17 insertions(+), 26 deletions(-)
> 

<snip>

> @@ -132,20 +131,15 @@ static char **global_addr4_strs;
>   static uint32_t netdev_watch;
>   static struct l_netlink *rtnl;
>   
> -static const char *broadcast_from_ip(const char *ip)
> +static const char *broadcast_from_ip(const char *ip, uint8_t prefix_len)
>   {
>   	struct in_addr ia;
> -	uint32_t bcast;
> +	uint32_t netmask = util_netmask_from_prefix(prefix_len);
>   
>   	if (inet_aton(ip, &ia) != 1)
>   		return NULL;
>   
> -	bcast = ntohl(ia.s_addr);
> -	bcast &= 0xffffff00;
> -	bcast |= 0x000000ff;
> -
> -	ia.s_addr = htonl(bcast);
> -
> +	ia.s_addr |= htonl(~netmask);
>   	return inet_ntoa(ia);
>   }
>   

Okay, but don't you really want to use l_rtnl_address_set_broadcast(rtnl, NULL) 
instead?  Also, you may want to do this directly in ip-pool.c?

Applied, thanks.

Regards,
-Denis

Re: [PATCH 7/8] autotests: Update APRanges usage in testAP

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi Andrew,

On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> [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 +++-
>   autotests/testAP/dhcp_test.py   | 12 +++++++++---
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 

Patches 7 and 8 applied, thanks.

Regards,
-Denis

Re: [PATCH 4/8] ap: Refactor global address pool loading

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

Hi Denis,

On Tue, 1 Jun 2021 at 17:14, Denis Kenzior <denkenz@gmail.com> wrote:
> On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> > +             } else if (l_settings_get_value(settings,
> > +                                             "General", "APRanges")) {
> > +                     global_addr4_strs = l_settings_get_string_list(settings,
> > +                                                             "General",
> > +                                                             "APRanges",
> > +                                                             ',');
>
> Do you want to print a warning that this setting is deprecated?

Ok, will send a patch.

Best regards

Re: [PATCH 6/8] ap: Save AP address as l_rtnl_address

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 1102 bytes --]

Hi Denis,

On Tue, 1 Jun 2021 at 17:20, Denis Kenzior <denkenz@gmail.com> wrote:
> On 5/27/21 7:47 PM, Andrew Zaborowski wrote:
> > -static const char *broadcast_from_ip(const char *ip)
> > +static const char *broadcast_from_ip(const char *ip, uint8_t prefix_len)
> >   {
> >       struct in_addr ia;
> > -     uint32_t bcast;
> > +     uint32_t netmask = util_netmask_from_prefix(prefix_len);
> >
> >       if (inet_aton(ip, &ia) != 1)
> >               return NULL;
> >
> > -     bcast = ntohl(ia.s_addr);
> > -     bcast &= 0xffffff00;
> > -     bcast |= 0x000000ff;
> > -
> > -     ia.s_addr = htonl(bcast);
> > -
> > +     ia.s_addr |= htonl(~netmask);
> >       return inet_ntoa(ia);
> >   }
> >
>
> Okay, but don't you really want to use l_rtnl_address_set_broadcast(rtnl, NULL)
> instead?

Oh, I didn't know this existed, and apparently it's already done in
l_rtnl_address_new() so I can just drop this from ap.c.

>  Also, you may want to do this directly in ip-pool.c?

Considered that too but in this case we're creating a new l_rtnl_address.

Best regards