[PATCH 1/5] storage: add storage_network_filename_from_path

[PATCH 1/5] storage: add storage_network_filename_from_path

[James Prestwood]

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

This is to allow this API to be used simply to get the file name
without extension. AP will be storing provisioning files similarly
to the network format but the extension will not map to existing
security types. Some of the parsing code from
storage_network_ssid_from_path was reused for parsing the file name
and verification.
---
 src/storage.c | 40 ++++++++++++++++++++++++++++++++++------
 src/storage.h |  1 +
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/src/storage.c b/src/storage.c
index 00d93933..60fb1bc3 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -287,13 +287,12 @@ char *storage_get_network_file_path(enum security type, const char *ssid)
 	return path;
 }
 
-const char *storage_network_ssid_from_path(const char *path,
-							enum security *type)
+static bool parse_file_path(const char *path, char *buf,
+				const char **ext_offset)
 {
 	const char *filename = strrchr(path, '/');
 	const char *c, *end;
 	char *decoded;
-	static char buf[67];
 
 	if (filename)
 		filename++;	/* Skip the / */
@@ -302,8 +301,8 @@ const char *storage_network_ssid_from_path(const char *path,
 
 	end = strchr(filename, '.');
 
-	if (!end || !security_from_str(end + 1, type))
-		return NULL;
+	if (!end)
+		return false;
 
 	if (filename[0] != '=') {
 		if (end == filename || end - filename > 32)
@@ -319,7 +318,7 @@ const char *storage_network_ssid_from_path(const char *path,
 		memcpy(buf, filename, end - filename);
 		buf[end - filename] = '\0';
 
-		return buf;
+		goto done;
 	}
 
 	if (end - filename <= 1 || end - filename > 65)
@@ -342,6 +341,35 @@ const char *storage_network_ssid_from_path(const char *path,
 	strcpy(buf, decoded);
 	l_free(decoded);
 
+done:
+	if (ext_offset)
+		*ext_offset = end;
+
+	return true;
+}
+
+const char *storage_network_filename_from_path(const char *path)
+{
+	static char buf[67];
+
+	if (!parse_file_path(path, buf, NULL))
+		return NULL;
+
+	return buf;
+}
+
+const char *storage_network_ssid_from_path(const char *path,
+							enum security *type)
+{
+	const char *end;
+	static char buf[67];
+
+	if (!parse_file_path(path, buf, &end))
+		return NULL;
+
+	if (!security_from_str(end + 1, type))
+		return NULL;
+
 	return buf;
 }
 
diff --git a/src/storage.h b/src/storage.h
index 80b63c53..d9b17c7f 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -37,6 +37,7 @@ void storage_cleanup_dirs(void);
 char *storage_get_path(const char *format, ...);
 char *storage_get_hotspot_path(const char *format, ...);
 
+const char *storage_network_filename_from_path(const char *path);
 const char *storage_network_ssid_from_path(const char *path,
 							enum security *type);
 char *storage_get_network_file_path(enum security type, const char *ssid);
-- 
2.26.2

[PATCH 2/5] storage: add storage_get_ap_path

[James Prestwood]

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

---
 src/storage.c | 30 ++++++++++++++++++++++++++++++
 src/storage.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/src/storage.c b/src/storage.c
index 60fb1bc3..e7ac3938 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -50,6 +50,7 @@
 
 static char *storage_path = NULL;
 static char *storage_hotspot_path = NULL;
+static char *storage_ap_path = NULL;
 
 static int create_dirs(const char *filename)
 {
@@ -221,6 +222,16 @@ bool storage_create_dirs(void)
 		return false;
 	}
 
+	storage_ap_path = l_strdup_printf("%s/ap/", storage_path);
+
+	if (create_dirs(storage_ap_path)) {
+		l_error("Failed to create %s", storage_ap_path);
+
+		l_free(storage_path);
+		l_free(storage_hotspot_path);
+		l_free(storage_ap_path);
+	}
+
 	return true;
 }
 
@@ -228,6 +239,7 @@ void storage_cleanup_dirs(void)
 {
 	l_free(storage_path);
 	l_free(storage_hotspot_path);
+	l_free(storage_ap_path);
 }
 
 char *storage_get_path(const char *format, ...)
@@ -266,6 +278,24 @@ char *storage_get_hotspot_path(const char *format, ...)
 	return str;
 }
 
+char *storage_get_ap_path(const char *format, ...)
+{
+	va_list args;
+	char *fmt, *str;
+
+	if (!format)
+		return l_strdup(storage_ap_path);
+
+	fmt = l_strdup_printf("%s/%s", storage_ap_path, format);
+
+	va_start(args, format);
+	str = l_strdup_vprintf(fmt, args);
+	va_end(args);
+
+	l_free(fmt);
+	return str;
+}
+
 char *storage_get_network_file_path(enum security type, const char *ssid)
 {
 	char *path;
diff --git a/src/storage.h b/src/storage.h
index d9b17c7f..a6076d63 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -36,6 +36,7 @@ bool storage_create_dirs(void);
 void storage_cleanup_dirs(void);
 char *storage_get_path(const char *format, ...);
 char *storage_get_hotspot_path(const char *format, ...);
+char *storage_get_ap_path(const char *format, ...);
 
 const char *storage_network_filename_from_path(const char *path);
 const char *storage_network_ssid_from_path(const char *path,
-- 
2.26.2

[PATCH 3/5] ap: add provisioning file parsing

[James Prestwood]

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

Currently AP's are quite limited in configuration options. The
Start() method is great for very basic cases but if the user
requires specific options (security, DHCP, etc.) there is not
any way of configuring them.

For these cases we can allow additional options to be added using
AP provisioning files similar to how we define station networks.
The provisioning files for AP will reside in $STATE_DIRECTORY/ap
(/var/lib/iwd/ap by default). The name of the file indicates the
SSID and, for now, the only value will be [Security].Passphrase
until more options are added. The file extension does not matter at
this point, but in tests and examples *.ap will be used.

AP provisioning files will be loaded in at startup and remain as
static configs in a queue.
---
 src/ap.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/ap.h |  3 ++
 2 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index f08835f7..956b7be9 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 <dirent.h>
 
 #include <ell/ell.h>
 
@@ -51,6 +52,7 @@
 #include "src/wscutil.h"
 #include "src/eap-wsc.h"
 #include "src/ap.h"
+#include "src/storage.h"
 
 struct ap_state {
 	struct netdev *netdev;
@@ -125,6 +127,7 @@ struct ap_ip_pool {
 struct ap_ip_pool pool;
 static uint32_t netdev_watch;
 struct l_netlink *rtnl;
+static struct l_queue *ap_configs;
 
 /*
  * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
@@ -231,6 +234,10 @@ void ap_config_free(struct ap_config *config)
 	explicit_bzero(config->psk, sizeof(config->psk));
 	l_free(config->authorized_macs);
 	l_free(config->wsc_name);
+
+	if (config->config_file)
+		l_free(config->config_file);
+
 	l_free(config);
 }
 
@@ -303,7 +310,13 @@ static void ap_reset(struct ap_state *ap)
 	if (ap->rates)
 		l_uintset_free(ap->rates);
 
-	ap_config_free(ap->config);
+	/*
+	 * Only free a non file based config. Configs which are created from
+	 * provisioning files should be kept around until IWD exits
+	 */
+	if (ap->config->config_file)
+		ap_config_free(ap->config);
+
 	ap->config = NULL;
 
 	l_queue_destroy(ap->wsc_pbc_probes, l_free);
@@ -2780,10 +2793,41 @@ static void ap_netdev_watch(struct netdev *netdev,
 	}
 }
 
+static struct ap_config *ap_config_from_settings(struct l_settings *settings,
+							const char *ssid)
+{
+	struct ap_config *config = l_new(struct ap_config, 1);
+	char *passphrase;
+
+	config->ssid = l_strdup(ssid);
+
+	passphrase = l_settings_get_string(settings, "Security", "Passphrase");
+	if (passphrase) {
+		if (strlen(passphrase) > 63) {
+			l_error("[Security].Passphrase must not exceed "
+					"63 characters");
+			goto failed;
+		}
+
+		strcpy(config->passphrase, passphrase);
+		l_free(passphrase);
+	}
+
+	l_info("Loaded AP configuration %s", config->ssid);
+	return config;
+
+failed:
+	ap_config_free(config);
+	return NULL;
+}
+
 static int ap_init(void)
 {
 	const struct l_settings *settings = iwd_get_config();
 	bool dhcp_enable = false;
+	DIR *dir;
+	struct dirent *dirent;
+	L_AUTO_FREE_VAR(char *, ap_dir) = storage_get_ap_path(NULL);
 
 	netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
 
@@ -2814,6 +2858,44 @@ static int ap_init(void)
 		rtnl = iwd_get_rtnl();
 	}
 
+	ap_configs = l_queue_new();
+
+	dir = opendir(ap_dir);
+	if (!dir)
+		return -ENOENT;
+
+	while ((dirent = readdir(dir))) {
+		struct ap_config *config;
+		struct l_settings *s;
+		char *filename;
+		const char *ssid;
+
+		if (dirent->d_type != DT_REG && dirent->d_type != DT_LNK)
+			continue;
+
+		filename = l_strdup_printf("%s/%s", ap_dir, dirent->d_name);
+		s = l_settings_new();
+
+		if (!l_settings_load_from_file(s, filename))
+			goto next;
+
+		ssid = storage_network_filename_from_path(filename);
+
+		config = ap_config_from_settings(s, ssid);
+		if (!config)
+			goto next;
+
+		config->config_file = l_strdup(filename);
+
+		l_queue_push_head(ap_configs, config);
+
+next:
+		l_free(filename);
+		l_settings_free(s);
+	}
+
+	closedir(dir);
+
 	return 0;
 }
 
@@ -2823,6 +2905,8 @@ static void ap_exit(void)
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
 
 	ip_pool_destroy();
+
+	l_queue_destroy(ap_configs, (l_queue_destroy_func_t) ap_config_free);
 }
 
 IWD_MODULE(ap, ap_init, ap_exit)
diff --git a/src/ap.h b/src/ap.h
index 1f2d438a..b3fcb882 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -63,6 +63,9 @@ struct ap_config {
 	unsigned int authorized_macs_num;
 	char *wsc_name;
 	struct wsc_primary_device_type wsc_primary_device_type;
+
+	char *config_file;
+
 	bool no_cck_rates : 1;
 };
 
-- 
2.26.2

[PATCH 4/5] ap: add StartWithConfig DBus method

[James Prestwood]

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

Users can now start an AP from settings based off a config file
on disk. The only argument is the SSID which will be used to
lookup the ap_config loaded during ap_init.
---
 src/ap.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index 956b7be9..92e8ce62 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2711,6 +2711,44 @@ static struct l_dbus_message *ap_dbus_stop(struct l_dbus *dbus,
 	return NULL;
 }
 
+static bool match_ssid(const void *a, const void *data)
+{
+	const struct ap_config *config = a;
+	const char *ssid = data;
+
+	return !strcmp(config->ssid, ssid);
+}
+
+static struct l_dbus_message *ap_dbus_start_with_config(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct ap_if_data *ap_if = user_data;
+	const char *ssid;
+	struct ap_config *config;
+	int err;
+
+	if (ap_if->ap && ap_if->ap->started)
+		return dbus_error_already_exists(message);
+
+	if (ap_if->ap || ap_if->pending)
+		return dbus_error_busy(message);
+
+	if (!l_dbus_message_get_arguments(message, "s", &ssid))
+		return dbus_error_invalid_args(message);
+
+	config = l_queue_find(ap_configs, match_ssid, ssid);
+	if (!config)
+		return dbus_error_not_found(message);
+
+	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, &err, ap_if);
+	if (!ap_if->ap)
+		return dbus_error_from_errno(err, message);
+
+	ap_if->pending = l_dbus_message_ref(message);
+	return NULL;
+}
+
 static bool ap_dbus_property_get_started(struct l_dbus *dbus,
 					struct l_dbus_message *message,
 					struct l_dbus_message_builder *builder,
@@ -2729,6 +2767,9 @@ static void ap_setup_interface(struct l_dbus_interface *interface)
 	l_dbus_interface_method(interface, "Start", 0, ap_dbus_start, "",
 			"ss", "ssid", "wpa2_passphrase");
 	l_dbus_interface_method(interface, "Stop", 0, ap_dbus_stop, "", "");
+	l_dbus_interface_method(interface, "StartWithConfig", 0,
+					ap_dbus_start_with_config, "", "s",
+					"ssid");
 
 	l_dbus_interface_property(interface, "Started", 0, "b",
 					ap_dbus_property_get_started, NULL);
-- 
2.26.2

[PATCH 5/5] ap: allow DHCP settings in provisioning files

[James Prestwood]

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

Users can now supply an AP provisioning file containing an [IPv4]
section and define various DHCP settings:

[IPv4]
Address=<address>
Netmask=<netmask>
Gateway=<gateway>
IPRange=<start_address>,<end_address>
DNSList=<dns1>,<dns2>,...<dnsN>
LeaseTime=<lease_time>

There are a few notes/requirements to keep in mind when using a
provisioning file:

 - All settings are optional but [IPv4].Address is required if the
   interface does not already have an address set.
 - If no [IPv4].Address is defined in the provisioning file and the AP
   interface does not already have an address set, StartWithConfig()
   will fail with -EINVAL.
 - If a provisioning file is provided it will take precedence, and the
   AP will not pull from the IP pool.
 - A provisioning file containing an IPv4 section assumes DHCP is being
   enabled and will override [General].EnableNetworkConfiguration.
 - Any address that AP sets on the interface will be deleted when the AP
   is stopped.
---
 src/ap.c | 225 +++++++++++++++++++++++++++++++++++++++++++++----------
 src/ap.h |   1 +
 2 files changed, 188 insertions(+), 38 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 92e8ce62..dab6ef37 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -81,9 +81,12 @@ struct ap_state {
 	struct l_dhcp_server *server;
 	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;
 };
 
 struct sta_state {
@@ -314,7 +317,7 @@ static void ap_reset(struct ap_state *ap)
 	 * Only free a non file based config. Configs which are created from
 	 * provisioning files should be kept around until IWD exits
 	 */
-	if (ap->config->config_file)
+	if (ap->config && !ap->config->config_file)
 		ap_config_free(ap->config);
 
 	ap->config = NULL;
@@ -323,13 +326,18 @@ static void ap_reset(struct ap_state *ap)
 
 	ap->started = false;
 
-	if (ap->own_ip) {
+	/* Delete IP if one was set by IWD */
+	if (ap->cleanup_ip)
 		l_rtnl_ifaddr4_delete(rtnl, netdev_get_ifindex(netdev),
-					pool.prefix, ap->own_ip,
+					ap->ip_prefix, ap->own_ip,
 					broadcast_from_ip(ap->own_ip),
 					NULL, NULL, NULL);
 
-		ip_pool_put(ap->own_ip);
+	if (ap->own_ip) {
+		/* Release IP from pool if used */
+		if (ap->use_ip_pool)
+			ip_pool_put(ap->own_ip);
+
 		l_free(ap->own_ip);
 	}
 
@@ -2220,6 +2228,162 @@ 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");
+	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 (!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))
+		goto parse_error;
+
+	if (dns && !l_dhcp_server_set_dns(server, dns))
+		goto parse_error;
+
+	if (ip_range && !l_dhcp_server_set_ip_range(server, ip_range[0],
+							ip_range[1]))
+		goto parse_error;
+
+	if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time))
+		goto parse_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;
+
+parse_error:
+	l_strv_free(dns);
+	l_strv_free(ip_range);
+	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
+ *          -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)
+{
+	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
+	struct l_settings *settings;
+	struct in_addr ia;
+	uint32_t address = 0;
+	int ret = -EINVAL;
+
+	ap->server = l_dhcp_server_new(ifindex);
+	if (!ap->server) {
+		l_error("Failed to create DHCP server on %u", ifindex);
+		return -EINVAL;;
+	}
+
+	if (getenv("IWD_DHCP_DEBUG"))
+		l_dhcp_server_set_debug(ap->server, do_debug,
+							"[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 (ap->config->config_file) {
+		char *addr;
+
+		settings = l_settings_new();
+
+		if (!l_settings_load_from_file(settings,
+						ap->config->config_file)) {
+			l_settings_free(settings);
+			return ret;
+		}
+
+		addr = l_settings_get_string(settings, "IPv4", "Address");
+		if (addr) {
+			if (inet_pton(AF_INET, addr, &ia) < 0)
+				goto free_settings;
+
+			/* 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_settings;
+
+		/* Set the remaining DHCP options in config file */
+		if (!dhcp_load_settings(ap, settings)) {
+			ret = -EINVAL;
+			goto free_settings;
+		}
+
+		if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
+			ret = -EINVAL;
+			goto free_settings;
+		}
+
+		ap->own_ip = l_strdup(addr);
+
+free_settings:
+		l_free(addr);
+		l_settings_free(settings);
+
+		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;
+		}
+
+		ap->use_ip_pool = true;
+		ap->ip_prefix = pool.prefix;
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 /*
  * Start a simple independent WPA2 AP on given netdev.
  *
@@ -2241,7 +2405,6 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 	struct l_genl_msg *cmd;
 	uint64_t wdev_id = netdev_get_wdev_id(netdev);
 	uint32_t ifindex = netdev_get_ifindex(netdev);
-	struct in_addr ia;
 	int err = -EINVAL;
 
 	if (err_out)
@@ -2342,52 +2505,35 @@ struct ap_state *ap_start(struct netdev *netdev, struct ap_config *config,
 	if (!ap->mlme_watch)
 		l_error("Registering for MLME notification failed");
 
-	/* No IP pool initialized, DHCP is not being used */
-	if (!pool.used)
+	/* No IP pool initialized or enabled in config, DHCP not being used */
+	if (!pool.used && !config->dhcp_enabled)
 		goto done;
 
-	ap->server = l_dhcp_server_new(ifindex);
-	if (!ap->server) {
-		l_error("Failed to create DHCP server on %u", ifindex);
-		goto error;
-	}
-
-	if (getenv("IWD_DHCP_DEBUG"))
-		l_dhcp_server_set_debug(ap->server, do_debug,
-							"[DHCPv4 SERV] ", NULL);
-
-	/*
-	 * If there is no IP set on the interface use one from the IP pool
-	 * defined in main.conf.
-	 */
-	if (!l_net_get_address(ifindex, &ia) || ia.s_addr == 0) {
-		ap->own_ip = ip_pool_get();
-
-		if (!ap->own_ip) {
-			l_error("No more IP's in pool, cannot start AP on %u",
-					ifindex);
-			err = -EEXIST;
-			goto error;
-		}
-
+	err = ap_setup_dhcp(ap);
+	if (err == 0) {
+		/* Address change required */
 		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
-					pool.prefix, ap->own_ip,
+					ap->ip_prefix, ap->own_ip,
 					broadcast_from_ip(ap->own_ip),
 					ap_ifaddr4_added_cb, ap, NULL);
 
 		if (!ap->rtnl_add_cmd) {
 			l_error("Failed to add IPv4 address");
+			err = -EIO;
 			goto error;
 		}
 
-
 		if (err_out)
 			*err_out = 0;
 
-		/* Finish starting AP in added callback */
+		ap->cleanup_ip = true;
+
 		return ap;
-	}
-	/* Else honor the IP set on the interface prior to calling Start() */
+	/* Selected address already set, continue normally */
+	} else if (err == -EALREADY)
+		err = 0;
+	else
+		goto error;
 
 done:
 	cmd = ap_build_cmd_start_ap(ap);
@@ -2854,6 +3000,9 @@ static struct ap_config *ap_config_from_settings(struct l_settings *settings,
 		l_free(passphrase);
 	}
 
+	if (l_settings_has_group(settings, "IPv4"))
+		config->dhcp_enabled = true;
+
 	l_info("Loaded AP configuration %s", config->ssid);
 	return config;
 
@@ -2895,10 +3044,10 @@ static int ap_init(void)
 
 		if (!ip_pool_create(ip_prefix))
 			return -EINVAL;
-
-		rtnl = iwd_get_rtnl();
 	}
 
+	rtnl = iwd_get_rtnl();
+
 	ap_configs = l_queue_new();
 
 	dir = opendir(ap_dir);
diff --git a/src/ap.h b/src/ap.h
index b3fcb882..c1bdd39a 100644
--- a/src/ap.h
+++ b/src/ap.h
@@ -66,6 +66,7 @@ struct ap_config {
 
 	char *config_file;
 
+	bool dhcp_enabled : 1;
 	bool no_cck_rates : 1;
 };
 
-- 
2.26.2

Re: [PATCH 4/5] ap: add StartWithConfig DBus method

[Denis Kenzior]

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

Hi James,

On 10/30/20 11:34 AM, James Prestwood wrote:
> Users can now start an AP from settings based off a config file
> on disk. The only argument is the SSID which will be used to
> lookup the ap_config loaded during ap_init.
> ---
>   src/ap.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/src/ap.c b/src/ap.c
> index 956b7be9..92e8ce62 100644
> --- a/src/ap.c
> +++ b/src/ap.c
> @@ -2711,6 +2711,44 @@ static struct l_dbus_message *ap_dbus_stop(struct l_dbus *dbus,
>   	return NULL;
>   }
>   
> +static bool match_ssid(const void *a, const void *data)
> +{
> +	const struct ap_config *config = a;
> +	const char *ssid = data;
> +
> +	return !strcmp(config->ssid, ssid);
> +}
> +
> +static struct l_dbus_message *ap_dbus_start_with_config(struct l_dbus *dbus,
> +						struct l_dbus_message *message,
> +						void *user_data)
> +{
> +	struct ap_if_data *ap_if = user_data;
> +	const char *ssid;
> +	struct ap_config *config;
> +	int err;
> +
> +	if (ap_if->ap && ap_if->ap->started)
> +		return dbus_error_already_exists(message);
> +
> +	if (ap_if->ap || ap_if->pending)
> +		return dbus_error_busy(message);
> +
> +	if (!l_dbus_message_get_arguments(message, "s", &ssid))
> +		return dbus_error_invalid_args(message);
> +

You may want to check that the SSID is valid if the profile is SSID based.

> +	config = l_queue_find(ap_configs, match_ssid, ssid);
> +	if (!config)
> +		return dbus_error_not_found(message);

Question, why store the profiles in a list when you could simply try and load 
the profile here?  Then you wouldn't really need to distinguish between 
filesystem and non-filesystem based ap_configs.

> +
> +	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, &err, ap_if);
> +	if (!ap_if->ap)
> +		return dbus_error_from_errno(err, message);
> +
> +	ap_if->pending = l_dbus_message_ref(message);
> +	return NULL;
> +}
> +
>   static bool ap_dbus_property_get_started(struct l_dbus *dbus,
>   					struct l_dbus_message *message,
>   					struct l_dbus_message_builder *builder,
> @@ -2729,6 +2767,9 @@ static void ap_setup_interface(struct l_dbus_interface *interface)
>   	l_dbus_interface_method(interface, "Start", 0, ap_dbus_start, "",
>   			"ss", "ssid", "wpa2_passphrase");
>   	l_dbus_interface_method(interface, "Stop", 0, ap_dbus_stop, "", "");
> +	l_dbus_interface_method(interface, "StartWithConfig", 0,

Lets try to avoid shortened words in the API.  StartWithConfiguration or 
StartProfile would be better.

> +					ap_dbus_start_with_config, "", "s",
> +					"ssid");
>   
>   	l_dbus_interface_property(interface, "Started", 0, "b",
>   					ap_dbus_property_get_started, NULL);
> 

Regards,
-Denis

Re: [PATCH 2/5] storage: add storage_get_ap_path

[Denis Kenzior]

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

On 10/30/20 11:34 AM, James Prestwood wrote:
> ---
>   src/storage.c | 30 ++++++++++++++++++++++++++++++
>   src/storage.h |  1 +
>   2 files changed, 31 insertions(+)
> 
> diff --git a/src/storage.c b/src/storage.c
> index 60fb1bc3..e7ac3938 100644
> --- a/src/storage.c
> +++ b/src/storage.c
> @@ -50,6 +50,7 @@
>   
>   static char *storage_path = NULL;
>   static char *storage_hotspot_path = NULL;
> +static char *storage_ap_path = NULL;
>   
>   static int create_dirs(const char *filename)
>   {
> @@ -221,6 +222,16 @@ bool storage_create_dirs(void)
>   		return false;
>   	}
>   
> +	storage_ap_path = l_strdup_printf("%s/ap/", storage_path);
> +
> +	if (create_dirs(storage_ap_path)) {
> +		l_error("Failed to create %s", storage_ap_path);
> +
> +		l_free(storage_path);
> +		l_free(storage_hotspot_path);
> +		l_free(storage_ap_path);

This might look better using a goto instead.

> +	}
> +
>   	return true;
>   }
>   
> @@ -228,6 +239,7 @@ void storage_cleanup_dirs(void)
>   {
>   	l_free(storage_path);
>   	l_free(storage_hotspot_path);
> +	l_free(storage_ap_path);
>   }
>   
>   char *storage_get_path(const char *format, ...)
> @@ -266,6 +278,24 @@ char *storage_get_hotspot_path(const char *format, ...)
>   	return str;
>   }
>   
> +char *storage_get_ap_path(const char *format, ...)
> +{
> +	va_list args;
> +	char *fmt, *str;
> +
> +	if (!format)
> +		return l_strdup(storage_ap_path);
> +
> +	fmt = l_strdup_printf("%s/%s", storage_ap_path, format);
> +
> +	va_start(args, format);
> +	str = l_strdup_vprintf(fmt, args);
> +	va_end(args);
> +
> +	l_free(fmt);
> +	return str;
> +}
> +

I think this really isn't necessary, you could use storage_get_path for this 
just as well.  Or just return storage_ap_path which is all you're using this for.

The more useful thing might be to actually obtain the AP path given an SSID...

>   char *storage_get_network_file_path(enum security type, const char *ssid)
>   {
>   	char *path;
> diff --git a/src/storage.h b/src/storage.h
> index d9b17c7f..a6076d63 100644
> --- a/src/storage.h
> +++ b/src/storage.h
> @@ -36,6 +36,7 @@ bool storage_create_dirs(void);
>   void storage_cleanup_dirs(void);
>   char *storage_get_path(const char *format, ...);
>   char *storage_get_hotspot_path(const char *format, ...);
> +char *storage_get_ap_path(const char *format, ...);
>   
>   const char *storage_network_filename_from_path(const char *path);
>   const char *storage_network_ssid_from_path(const char *path,
> 

Regards,
-Denis

Re: [PATCH 4/5] ap: add StartWithConfig DBus method

[James Prestwood]

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

Hi Denis,

> 
> > +	config = l_queue_find(ap_configs, match_ssid, ssid);
> > +	if (!config)
> > +		return dbus_error_not_found(message);
> 
> Question, why store the profiles in a list when you could simply try
> and load 
> the profile here?  Then you wouldn't really need to distinguish
> between 
> filesystem and non-filesystem based ap_configs.

Yes I like this much better. This would also allow the configs to be
changed while IWD was running as long as the AP is stopped.

> 
> > +
> > +	ap_if->ap = ap_start(ap_if->netdev, config, &ap_dbus_ops, &err,
> > ap_if);
> > +	if (!ap_if->ap)
> > +		return dbus_error_from_errno(err, message);
> > +
> > +	ap_if->pending = l_dbus_message_ref(message);
> > +	return NULL;
> > +}
> > +
> >   static bool ap_dbus_property_get_started(struct l_dbus *dbus,
> >   					struct l_dbus_message *message,
> >   					struct l_dbus_message_builder
> > *builder,
> > @@ -2729,6 +2767,9 @@ static void ap_setup_interface(struct
> > l_dbus_interface *interface)
> >   	l_dbus_interface_method(interface, "Start", 0, ap_dbus_start,
> > "",
> >   			"ss", "ssid", "wpa2_passphrase");
> >   	l_dbus_interface_method(interface, "Stop", 0, ap_dbus_stop, "",
> > "");
> > +	l_dbus_interface_method(interface, "StartWithConfig", 0,
> 
> Lets try to avoid shortened words in the API.  StartWithConfiguration
> or 
> StartProfile would be better.

Sure, StartProfile it is.

> 
> > +					ap_dbus_start_with_config, "",
> > "s",
> > +					"ssid");
> >   
> >   	l_dbus_interface_property(interface, "Started", 0, "b",
> >   					ap_dbus_property_get_started,
> > NULL);
> > 
> 
> Regards,
> -Denis

Re: [PATCH 2/5] storage: add storage_get_ap_path

[James Prestwood]

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

Hi Denis,

> >   
> > +char *storage_get_ap_path(const char *format, ...)
> > +{
> > +	va_list args;
> > +	char *fmt, *str;
> > +
> > +	if (!format)
> > +		return l_strdup(storage_ap_path);
> > +
> > +	fmt = l_strdup_printf("%s/%s", storage_ap_path, format);
> > +
> > +	va_start(args, format);
> > +	str = l_strdup_vprintf(fmt, args);
> > +	va_end(args);
> > +
> > +	l_free(fmt);
> > +	return str;
> > +}
> > +
> 
> I think this really isn't necessary, you could use storage_get_path
> for this 
> just as well.  Or just return storage_ap_path which is all you're
> using this for.
> 
> The more useful thing might be to actually obtain the AP path given
> an SSID...

Yeah I was just copying the way network/hotspot did it. But yes your
right, its rather complex for the one or two places it would be used.
And even getting the AP path + SSID should be easy enough:

path = storage_get_path("ap/%s", ssid);

I could also refactor hotspot to do this as well at some point.

> 
> >   char *storage_get_network_file_path(enum security type, const
> > char *ssid)
> >   {
> >   	char *path;
> > diff --git a/src/storage.h b/src/storage.h
> > index d9b17c7f..a6076d63 100644
> > --- a/src/storage.h
> > +++ b/src/storage.h
> > @@ -36,6 +36,7 @@ bool storage_create_dirs(void);
> >   void storage_cleanup_dirs(void);
> >   char *storage_get_path(const char *format, ...);
> >   char *storage_get_hotspot_path(const char *format, ...);
> > +char *storage_get_ap_path(const char *format, ...);
> >   
> >   const char *storage_network_filename_from_path(const char *path);
> >   const char *storage_network_ssid_from_path(const char *path,
> > 
> 
> Regards,
> -Denis