[PATCH v2 0/7] Radio Management Module

[PATCH v2 0/7] Radio Management Module

[James Prestwood]

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

Version 2:
 - Renamed from 'offchannel'
 - Fixed potential problems in initial p2p patch. Added work ID tracking
   as well as setting ID to zero once completed since the ID is reused
   for multiple requests. If Andrew could verify everything looks ok that
   would be great.
 - Changed priority from enum to integer. This makes things much more
   flexible.
 - Removed the concept of suspended completely. This was to support scanning
   but with the addition of priorities we can actually insert ANQP entries
   before any pending scans, which avoids needing to suspend scanning.
 - Connection attempts are now treated as radio management work items. This
   in addition to the above changes allow us again to avoid suspending scans
   as well as waiting for ANQP to complete before connecting. Connections
   can be inserted after ANQP, but before scans which removes the need for
   the ANQP watch in station.


James Prestwood (7):
  radio_mgmt: introduce new radio management module
  wiphy: integrate radio management module
  frame-xchg: refactor to use radio management module
  anqp: refactor to use frame-xchg
  scan: refactor to use radio management module
  network: use radio management for connections
  station: remove ANQP watch

 Makefile.am      |   1 +
 src/anqp.c       | 443 ++++++++++-------------------------------------
 src/anqp.h       |   3 +-
 src/frame-xchg.c |  99 +++++------
 src/frame-xchg.h |   6 +-
 src/network.c    | 148 ++++++++--------
 src/p2p.c        |  22 ++-
 src/radio_mgmt.c | 240 +++++++++++++++++++++++++
 src/radio_mgmt.h |  48 +++++
 src/scan.c       | 104 ++++-------
 src/scan.h       |   3 -
 src/station.c    |  75 ++------
 src/station.h    |   5 -
 src/wiphy.c      |   5 +
 14 files changed, 575 insertions(+), 627 deletions(-)
 create mode 100644 src/radio_mgmt.c
 create mode 100644 src/radio_mgmt.h

-- 
2.21.1

[PATCH v2 1/7] radio_mgmt: introduce new radio management module

[James Prestwood]

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

This module will handle fairness and order in any operations which
radios can only do sequentially (offchannel, scanning, connection etc.).

Both scan and frame-xchg are complex modules (especially scanning)
which is why this module was implemented generic enough where the
changes to both modules will be minimal. Any module that requires
this kind of work can push a work item into the radio management work
queue (radio_mgmt_push) and when the work is ready to be started
radio management will call back into the module. Once the work is
completed (and this may be some time later e.g. in scan results or a
frame watch) the module can signal back to radio management that the
work is finished (radio_mgmt_done). Radio management will then pop
the queue and continue with the next work item.

A concept of priority was added in order to allow important offchannel
operations (e.g. ANQP) to take priority over other work items. The
priority is an integer, where lower values are of a higher priority.
The concept of priority cleanly solves a lot of the complexity that
was added in order to support ANQP queries (suspending scanning and
waiting for ANQP to finish before connecting).

Instead ANQP queries can be queued at a higher priority than scanning
which removes the need for suspending scans. In addition we can treat
connections as radio management work and insert them at a lower
priority than ANQP, but higher than scanning. This forces the
connection to wait for ANQP without having to track any state.

The radio management work queue's are created on a per-phy basis, so
each queue is tied to the creation and destruction of wiphys. Because
of this wiphy.c takes care of creating and destroying offchannel queues
using:

radio_mgmt_create_from_wiphy()
radio_mgmt_destroy()
---
 Makefile.am      |   1 +
 src/radio_mgmt.c | 240 +++++++++++++++++++++++++++++++++++++++++++++++
 src/radio_mgmt.h |  48 ++++++++++
 3 files changed, 289 insertions(+)
 create mode 100644 src/radio_mgmt.c
 create mode 100644 src/radio_mgmt.h

diff --git a/Makefile.am b/Makefile.am
index 57c694df..b0455e71 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -229,6 +229,7 @@ src_iwd_SOURCES = src/main.c linux/nl80211.h src/iwd.h src/missing.h \
 					src/frame-xchg.h src/frame-xchg.c \
 					src/eap-wsc.c src/eap-wsc.h \
 					src/wscutil.h src/wscutil.c \
+					src/radio_mgmt.h src/radio_mgmt.c \
 					$(eap_sources) \
 					$(builtin_sources)
 
diff --git a/src/radio_mgmt.c b/src/radio_mgmt.c
new file mode 100644
index 00000000..ff7f944c
--- /dev/null
+++ b/src/radio_mgmt.c
@@ -0,0 +1,240 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2020  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
+ *
+ */
+
+#include <stdbool.h>
+
+#include <ell/ell.h>
+
+#include "src/iwd.h"
+#include "src/module.h"
+#include "src/wiphy.h"
+#include "src/radio_mgmt.h"
+
+struct radio_mgmt_work {
+	uint32_t id;
+	radio_mgmt_work_func_t do_work;
+	void *user_data;
+	radio_mgmt_work_destroy_func_t destroy;
+	int priority;
+};
+
+/*
+ * Per-phy list of work requests
+ */
+struct radio_mgmt_phy {
+	struct wiphy *wiphy;
+	struct l_queue *work;
+	bool idle : 1;
+};
+
+static struct l_queue *phys;
+static uint32_t work_ids;
+
+static void destroy_work(void *user_data)
+{
+	struct radio_mgmt_work *work = user_data;
+
+	if (work->destroy)
+		work->destroy(work->user_data);
+
+	l_free(work);
+}
+
+static void radio_mgmt_work_next(struct radio_mgmt_phy *phy)
+{
+	struct radio_mgmt_work *work;
+	bool done;
+
+	if (!phy->idle)
+		return;
+
+	/* find first item that isn't suspended */
+	work = l_queue_peek_head(phy->work);
+	if (!work) {
+		phy->idle = true;
+		return;
+	}
+
+	l_debug("Starting radio work %u", work->id);
+
+	/* Set idle as false in case the callback starts new work items */
+	phy->idle = false;
+
+	done = work->do_work(work->user_data);
+
+	if (done) {
+		l_debug("Radio work %u done", work->id);
+		l_queue_remove(phy->work, work);
+
+		destroy_work(work);
+
+		phy->idle = true;
+
+		radio_mgmt_work_next(phy);
+	}
+}
+
+static bool match_wiphy(const void *a, const void *b)
+{
+	const struct radio_mgmt_phy *phy = a;
+
+	return phy->wiphy == b;
+}
+
+static struct radio_mgmt_phy *find_phy(uint64_t wdev_id)
+{
+	struct wiphy *wiphy = wiphy_find(wdev_id >> 32);
+
+	if (!wiphy)
+		return NULL;
+
+	return l_queue_find(phys, match_wiphy, wiphy);
+}
+
+static int insert_by_priority(const void *a, const void *b, void *user_data)
+{
+	const struct radio_mgmt_work *new = a;
+	const struct radio_mgmt_work *work = b;
+	uint8_t *count = user_data;
+
+	/* This ensures we are not pushing before head, or any priority items */
+	if (*count == 0 || work->priority <= new->priority) {
+		l_put_u8(*count + 1, count);
+		return 1;
+	}
+
+	l_debug("Inserting priority work item at index %u", *count);
+
+	return -1;
+}
+
+/*
+ * Push a new work item into the queue for this phy. The wdev_id is used to
+ * get the actual phy its under.
+ */
+uint32_t radio_mgmt_push(uint64_t wdev_id, int priority,
+				radio_mgmt_work_func_t func, void *user_data,
+				radio_mgmt_work_destroy_func_t destroy)
+{
+	struct radio_mgmt_work *work;
+	struct radio_mgmt_phy *phy = find_phy(wdev_id);
+	uint8_t count = 0;
+
+	if (!phy)
+		return 0;
+
+	work = l_new(struct radio_mgmt_work, 1);
+	work->do_work = func;
+	work->destroy = destroy;
+	work->user_data = user_data;
+	work->id = ++work_ids;
+	work->priority = priority;
+
+	l_debug("Queuing radio work %u", work->id);
+
+	/*
+	 * The head of the queue will always contain the current work item, so
+	 * we pass a count in to ensure we always insert after the head.
+	 */
+	l_queue_insert(phy->work, work, insert_by_priority, &count);
+
+	/* Start work if we are sitting idle */
+	radio_mgmt_work_next(phy);
+
+	return work->id;
+}
+
+static bool match_id(const void *a, const void *b)
+{
+	const struct radio_mgmt_work *work = a;
+
+	return work->id == L_PTR_TO_UINT(b);
+}
+
+/*
+ * Signals that the work item is done. This can also be used to cancel any
+ * pending work item in the queue.
+ */
+void radio_mgmt_done(uint64_t wdev_id, uint32_t id)
+{
+	struct radio_mgmt_work *work;
+	struct radio_mgmt_phy *phy = find_phy(wdev_id);
+
+	if (!phy)
+		return;
+
+	work = l_queue_remove_if(phy->work, match_id, L_UINT_TO_PTR(id));
+	if (!work)
+		return;
+
+	l_debug("Radio work done %u", id);
+
+	destroy_work(work);
+
+	phy->idle = true;
+
+	radio_mgmt_work_next(phy);
+}
+
+void radio_mgmt_create_from_wiphy(struct wiphy *wiphy)
+{
+	struct radio_mgmt_phy *phy = l_new(struct radio_mgmt_phy, 1);
+
+	phy->wiphy = wiphy;
+	phy->work = l_queue_new();
+	phy->idle = true;
+
+	l_queue_push_head(phys, phy);
+}
+
+static void destroy_radio_mgmt_phy(void *user_data)
+{
+	struct radio_mgmt_phy *phy = user_data;
+
+	l_queue_destroy(phy->work, destroy_work);
+
+	l_free(phy);
+}
+
+void radio_mgmt_destroy(struct wiphy *wiphy)
+{
+	struct radio_mgmt_phy *phy = l_queue_remove_if(phys, match_wiphy, wiphy);
+
+	if (!phy)
+		return;
+
+	destroy_radio_mgmt_phy(phy);
+}
+
+static int radio_mgmt_init(void)
+{
+	phys = l_queue_new();
+
+	return 0;
+}
+
+static void radio_mgmt_exit(void)
+{
+	l_queue_destroy(phys, destroy_radio_mgmt_phy);
+}
+
+IWD_MODULE(radio_mgmt, radio_mgmt_init, radio_mgmt_exit);
diff --git a/src/radio_mgmt.h b/src/radio_mgmt.h
new file mode 100644
index 00000000..4429ea8c
--- /dev/null
+++ b/src/radio_mgmt.h
@@ -0,0 +1,48 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2020  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
+ *
+ */
+
+/*
+ * Module specific function that does the actual offchannel work, whether this
+ * is scanning, sending offchannel frames, or listening for a message. This
+ * function should return a boolean indicating if the work is done (e.g.
+ * fire-and-forget frame). Most cases the work is not done by the time the
+ * function returns, and generally we need to wait for a frame to be received
+ * or for scan results to come in. In this case radio_mgmt_work_done() should be
+ * called asynchronously once the work has completed.
+ */
+#include <stdbool.h>
+#include <stdint.h>
+
+#include <ell/ell.h>
+
+struct wiphy;
+
+typedef bool (*radio_mgmt_work_func_t)(void *user_data);
+typedef void (*radio_mgmt_work_destroy_func_t)(void *user_data);
+
+uint32_t radio_mgmt_push(uint64_t wdev_id, int priority,
+				radio_mgmt_work_func_t func, void *user_data,
+				radio_mgmt_work_destroy_func_t destroy);
+void radio_mgmt_done(uint64_t wdev_id, uint32_t id);
+
+void radio_mgmt_create_from_wiphy(struct wiphy *wiphy);
+void radio_mgmt_destroy(struct wiphy *wiphy);
-- 
2.21.1

[PATCH v2 2/7] wiphy: integrate radio management module

[James Prestwood]

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

Tie wiphy creation/destruction to radio management
---
 src/wiphy.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/wiphy.c b/src/wiphy.c
index aef39549..49215fe3 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -53,6 +53,7 @@
 #include "src/watchlist.h"
 #include "src/nl80211util.h"
 #include "src/nl80211cmd.h"
+#include "src/radio_mgmt.h"
 
 #define EXT_CAP_LEN 10
 
@@ -1235,6 +1236,8 @@ void wiphy_create_complete(struct wiphy *wiphy)
 	wiphy_get_reg_domain(wiphy);
 
 	wiphy_print_basic_info(wiphy);
+
+	radio_mgmt_create_from_wiphy(wiphy);
 }
 
 bool wiphy_destroy(struct wiphy *wiphy)
@@ -1247,6 +1250,8 @@ bool wiphy_destroy(struct wiphy *wiphy)
 	if (wiphy->registered)
 		l_dbus_unregister_object(dbus_get_bus(), wiphy_get_path(wiphy));
 
+	radio_mgmt_destroy(wiphy);
+
 	wiphy_free(wiphy);
 	return true;
 }
-- 
2.21.1

[PATCH v2 3/7] frame-xchg: refactor to use radio management module

[James Prestwood]

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

In order to first integrate frame-xchg some refactoring needed to
be done. First it is useful to allow queueing frames up rather than
requiring the module (p2p, anqp etc) to wait for the last frame to
finish. This can be aided by radio management but frame-xchg needed
some refactoring as well.

First was getting rid of this fx pointer re-use. It looks like this
was done to save a bit of memory but things get pretty complex
needed to check if the pointer is stale or has been reset. Instead
of this we now just allocate a new pointer each frame-xchg. This
allows for the module to queue multiple requests as well as removes
the complexity of needed to check if the fx pointer is stale.

Next was adding the ability to track frame-xchgs by ID. If a module
can queue up multiple requests it also needs to be able to cancel
them individually vs per-wdev. This comes free with radio management
since it returns an ID which can be given directly to the caller.

Then radio management was simply piped in by adding the push/done APIs.
---
 src/frame-xchg.c | 99 +++++++++++++++++++++---------------------------
 src/frame-xchg.h |  6 +--
 src/p2p.c        | 22 +++++++++--
 3 files changed, 64 insertions(+), 63 deletions(-)

diff --git a/src/frame-xchg.c b/src/frame-xchg.c
index a9c1e4c3..32a317fd 100644
--- a/src/frame-xchg.c
+++ b/src/frame-xchg.c
@@ -41,6 +41,7 @@
 #include "src/nl80211util.h"
 #include "src/netdev.h"
 #include "src/frame-xchg.h"
+#include "src/radio_mgmt.h"
 
 #ifndef SOL_NETLINK
 #define SOL_NETLINK 270
@@ -109,12 +110,11 @@ struct frame_xchg_data {
 	frame_xchg_destroy_func_t destroy;
 	void *user_data;
 	uint32_t group_id;
+	uint32_t work_id;
 	unsigned int retry_cnt;
 	unsigned int retry_interval;
 	unsigned int resp_timeout;
 	unsigned int retries_on_ack;
-	bool in_frame_cb : 1;
-	bool stale : 1;
 };
 
 struct frame_xchg_watch_data {
@@ -726,7 +726,7 @@ static bool frame_watch_remove_by_handler(uint64_t wdev_id, uint32_t group_id,
 					&handler_info) > 0;
 }
 
-static void frame_xchg_tx_retry(struct frame_xchg_data *fx);
+static bool frame_xchg_tx_retry(void *user_data);
 static bool frame_xchg_resp_handle(const struct mmpdu_header *mpdu,
 					const void *body, size_t body_len,
 					int rssi, void *user_data);
@@ -753,9 +753,6 @@ static void frame_xchg_wait_cancel(struct frame_xchg_data *fx)
 
 static void frame_xchg_reset(struct frame_xchg_data *fx)
 {
-	fx->in_frame_cb = false;
-	fx->stale = false;
-
 	frame_xchg_wait_cancel(fx);
 
 	if (fx->timeout)
@@ -789,7 +786,7 @@ static void frame_xchg_done(struct frame_xchg_data *fx, int err)
 	if (fx->cb)
 		fx->cb(err, fx->user_data);
 
-	frame_xchg_destroy(fx);
+	radio_mgmt_done(fx->wdev_id, fx->work_id);
 }
 
 static void frame_xchg_timeout_destroy(void *user_data)
@@ -918,12 +915,16 @@ error:
 	frame_xchg_done(fx, error);
 }
 
-static void frame_xchg_tx_retry(struct frame_xchg_data *fx)
+static bool frame_xchg_tx_retry(void *user_data)
 {
+	struct frame_xchg_data *fx = user_data;
 	struct l_genl_msg *msg;
 	uint32_t cmd_id;
 	uint32_t duration = fx->resp_timeout;
 
+	/* push fx now that we are ready to start radio work */
+	l_queue_push_tail(frame_xchgs, fx);
+
 	/*
 	 * TODO: in Station, AP, P2P-Client, GO or Ad-Hoc modes if we're
 	 * transmitting the frame on the BSS's operating channel we can skip
@@ -951,13 +952,15 @@ static void frame_xchg_tx_retry(struct frame_xchg_data *fx)
 		l_error("Error sending frame");
 		l_genl_msg_unref(msg);
 		frame_xchg_done(fx, -EIO);
-		return;
+		return true;
 	}
 
 	fx->tx_acked = false;
 	fx->have_cookie = false;
 	fx->early_status = false;
 	fx->retry_cnt++;
+
+	return false;
 }
 
 static bool frame_xchg_resp_handle(const struct mmpdu_header *mpdu,
@@ -999,20 +1002,10 @@ static bool frame_xchg_resp_handle(const struct mmpdu_header *mpdu,
 		if (!fx->tx_acked)
 			goto early_frame;
 
-		fx->in_frame_cb = true;
 		done = watch->cb(mpdu, body, body_len, rssi, fx->user_data);
 
-		/*
-		 * If the callback has started a new frame exchange it will
-		 * have reset and taken over the state variables and we need
-		 * to just exit without touching anything.
-		 */
-		if (!fx->in_frame_cb)
-			return true;
-
-		fx->in_frame_cb = false;
-
-		if (done || fx->stale) {
+		if (done) {
+			/* NULL callback here since the caller is done */
 			fx->cb = NULL;
 			frame_xchg_done(fx, 0);
 			return true;
@@ -1070,22 +1063,24 @@ static bool frame_xchg_match(const void *a, const void *b)
  * @resp_timeout was 0.  @frame is an iovec array terminated by an iovec
  * struct with NULL-iov_base.
  */
-void frame_xchg_start(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
+uint32_t frame_xchg_start(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 			unsigned int retry_interval, unsigned int resp_timeout,
 			unsigned int retries_on_ack, uint32_t group_id,
 			frame_xchg_cb_t cb, void *user_data,
 			frame_xchg_destroy_func_t destroy, ...)
 {
+	uint32_t id;
 	va_list args;
 
 	va_start(args, destroy);
-	frame_xchg_startv(wdev_id, frame, freq, retry_interval, resp_timeout,
+	id = frame_xchg_startv(wdev_id, frame, freq, retry_interval, resp_timeout,
 				retries_on_ack, group_id, cb, user_data,
 				destroy, args);
 	va_end(args);
+	return id;
 }
 
-void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
+uint32_t frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 			unsigned int retry_interval, unsigned int resp_timeout,
 			unsigned int retries_on_ack, uint32_t group_id,
 			frame_xchg_cb_t cb, void *user_data,
@@ -1108,31 +1103,13 @@ void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 				frame[0].iov_base)) {
 		l_error("Frame too short");
 		cb(-EMSGSIZE, user_data);
-		return;
+		return 0;
 	}
 
-	fx = l_queue_find(frame_xchgs, frame_xchg_match, &wdev_id);
+	fx = l_new(struct frame_xchg_data, 1);
 
-	if (fx) {
-		/*
-		 * If a frame callback calls us assume it's the end of
-		 * that earlier frame exchange and the start of a new one.
-		 */
-		if (fx->in_frame_cb)
-			frame_xchg_reset(fx);
-		else {
-			l_error("Frame exchange in progress");
-			cb(-EBUSY, user_data);
-			return;
-		}
-	} else {
-		fx = l_new(struct frame_xchg_data, 1);
-
-		if (!frame_xchgs)
-			frame_xchgs = l_queue_new();
-
-		l_queue_push_tail(frame_xchgs, fx);
-	}
+	if (!frame_xchgs)
+		frame_xchgs = l_queue_new();
 
 	fx->wdev_id = wdev_id;
 	fx->freq = freq;
@@ -1177,24 +1154,34 @@ void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 	}
 
 	fx->retry_cnt = 0;
-	frame_xchg_tx_retry(fx);
+
+	/*
+	 * TODO: Assume any offchannel frames are a high priority (0). This may
+	 * need to be re-examined in the future if other operations (e.g.
+	 * wait on channel) are introduced.
+	 */
+	fx->work_id = radio_mgmt_push(wdev_id, 0, frame_xchg_tx_retry,
+						fx, frame_xchg_destroy);
+	return fx->work_id;
 }
 
-void frame_xchg_stop(uint64_t wdev_id)
+static bool frame_xchg_match_id(const void *a, const void *b)
+{
+	const struct frame_xchg_data *fx = a;
+	const uint32_t *id = b;
+
+	return fx->work_id == *id;
+}
+
+void frame_xchg_cancel(uint32_t id)
 {
 	struct frame_xchg_data *fx =
-		l_queue_remove_if(frame_xchgs, frame_xchg_match, &wdev_id);
+		l_queue_remove_if(frame_xchgs, frame_xchg_match_id, &id);
 
 	if (!fx)
 		return;
 
-	if (fx->in_frame_cb) {
-		fx->stale = true;
-		return;
-	}
-
-	frame_xchg_reset(fx);
-	l_free(fx);
+	radio_mgmt_done(fx->wdev_id, id);
 }
 
 static void frame_xchg_mlme_notify(struct l_genl_msg *msg, void *user_data)
diff --git a/src/frame-xchg.h b/src/frame-xchg.h
index 55a55f73..ba4dc61a 100644
--- a/src/frame-xchg.h
+++ b/src/frame-xchg.h
@@ -43,14 +43,14 @@ bool frame_watch_add(uint64_t wdev_id, uint32_t group, uint16_t frame_type,
 bool frame_watch_group_remove(uint64_t wdev_id, uint32_t group);
 bool frame_watch_wdev_remove(uint64_t wdev_id);
 
-void frame_xchg_start(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
+uint32_t frame_xchg_start(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 			unsigned int retry_interval, unsigned int resp_timeout,
 			unsigned int retries_on_ack, uint32_t group_id,
 			frame_xchg_cb_t cb, void *user_data,
 			frame_xchg_destroy_func_t destroy, ...);
-void frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
+uint32_t frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 			unsigned int retry_interval, unsigned int resp_timeout,
 			unsigned int retries_on_ack, uint32_t group_id,
 			frame_xchg_cb_t cb, void *user_data,
 			frame_xchg_destroy_func_t destroy, va_list resp_args);
-void frame_xchg_stop(uint64_t wdev_id);
+void frame_xchg_cancel(uint32_t id);
diff --git a/src/p2p.c b/src/p2p.c
index 1c9cb081..b8c299a3 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -68,6 +68,7 @@ struct p2p_device {
 	uint32_t start_stop_cmd_id;
 	l_dbus_property_complete_cb_t pending_complete;
 	struct l_dbus_message *pending_message;
+	uint32_t xchg_id;
 
 	uint8_t listen_country[3];
 	uint8_t listen_oper_class;
@@ -383,7 +384,11 @@ static void p2p_connection_reset(struct p2p_device *dev)
 	netdev_watch_remove(dev->conn_netdev_watch_id);
 
 	frame_watch_group_remove(dev->wdev_id, FRAME_GROUP_CONNECT);
-	frame_xchg_stop(dev->wdev_id);
+
+	if (dev->xchg_id) {
+		frame_xchg_cancel(dev->xchg_id);
+		dev->xchg_id = 0;
+	}
 
 	if (!dev->enabled || (dev->enabled && dev->start_stop_cmd_id)) {
 		/*
@@ -458,9 +463,10 @@ static void p2p_peer_frame_xchg(struct p2p_peer *peer, struct iovec *tx_body,
 		peer->bss->frequency;
 
 	va_start(args, cb);
-	frame_xchg_startv(dev->wdev_id, frame, freq, retry_interval,
-				resp_timeout, retries_on_ack, group_id,
-				cb, dev, NULL, args);
+
+	dev->xchg_id = frame_xchg_startv(dev->wdev_id, frame, freq,
+				retry_interval, resp_timeout, retries_on_ack,
+				group_id, cb, dev, NULL, args);
 	va_end(args);
 
 	l_free(frame);
@@ -1175,11 +1181,17 @@ static void p2p_go_negotiation_resp_done(int error, void *user_data)
 	else
 		l_error("No GO Negotiation Confirmation frame received");
 
+	dev->xchg_id = 0;
+
 	p2p_connect_failed(dev);
 }
 
 static void p2p_go_negotiation_resp_err_done(int error, void *user_data)
 {
+	struct p2p_device *dev = user_data;
+
+	dev->xchg_id = 0;
+
 	if (error)
 		l_error("Sending the GO Negotiation Response failed: %s (%i)",
 			strerror(-error), -error);
@@ -1272,6 +1284,8 @@ static bool p2p_go_negotiation_confirm_cb(const struct mmpdu_header *mpdu,
 
 	l_debug("");
 
+	dev->xchg_id = 0;
+
 	if (body_len < 8) {
 		l_error("GO Negotiation Confirmation frame too short");
 		p2p_connect_failed(dev);
-- 
2.21.1

[PATCH v2 4/7] anqp: refactor to use frame-xchg

[James Prestwood]

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

The new frame-xchg module now handles a lot of what ANQP used to do. ANQP
now does not need to depend on nl80211/netdev for building and sending
frames. It also no longer needs any of the request lookups, frame watches
or to maintain a queue of requests because frame-xchg filters this for us.

From an API perspective:
 - anqp_request() was changed to take the wdev_id rather than ifindex.
 - anqp_cancel() was added so that station can properly clean up ANQP
   requests if the device disappears.

During testing a bug was also fixed in station on the timeout path
where the request queue would get popped twice.
---
 src/anqp.c    | 443 +++++++++++---------------------------------------
 src/anqp.h    |   3 +-
 src/station.c |  19 +--
 3 files changed, 103 insertions(+), 362 deletions(-)

diff --git a/src/anqp.c b/src/anqp.c
index 6627bd81..3b597835 100644
--- a/src/anqp.c
+++ b/src/anqp.c
@@ -31,105 +31,61 @@
 #include "src/module.h"
 #include "src/anqp.h"
 #include "src/util.h"
-#include "src/eap-private.h"
 #include "src/ie.h"
-#include "src/nl80211util.h"
-#include "src/nl80211cmd.h"
 #include "src/scan.h"
-#include "src/netdev.h"
 #include "src/iwd.h"
 #include "src/mpdu.h"
-#include "src/wiphy.h"
+#include "src/frame-xchg.h"
 
 #include "linux/nl80211.h"
 
+#define ANQP_GROUP	0
+
 struct anqp_request {
-	uint32_t ifindex;
+	uint64_t wdev_id;
 	anqp_response_func_t anqp_cb;
 	anqp_destroy_func_t anqp_destroy;
 	void *anqp_data;
-	uint64_t anqp_cookie;
 	uint8_t anqp_token;
+	uint32_t frequency;
+	uint8_t *frame;
+	size_t frame_len;
+	uint32_t id;
 };
 
-static struct l_genl_family *nl80211 = NULL;
-
-static struct l_queue *anqp_requests;
 static uint8_t anqp_token = 0;
 
-static uint32_t netdev_watch;
-static uint32_t unicast_watch;
-
 static void anqp_destroy(void *user_data)
 {
 	struct anqp_request *request = user_data;
 
 	if (request->anqp_destroy)
 		request->anqp_destroy(request->anqp_data);
-}
-
-static void netdev_gas_request_cb(struct l_genl_msg *msg, void *user_data)
-{
-	struct anqp_request *request = user_data;
-
-	if (l_genl_msg_get_error(msg) != 0)
-		goto error;
-
-	if (nl80211_parse_attrs(msg, NL80211_ATTR_COOKIE, &request->anqp_cookie,
-					NL80211_ATTR_UNSPEC) < 0)
-		goto error;
-
-	return;
-
-error:
-	l_debug("Error sending CMD_FRAME (%d)", l_genl_msg_get_error(msg));
-
-	if (request->anqp_cb)
-		request->anqp_cb(ANQP_FAILED, NULL, 0, request->anqp_data);
-
-	if (request->anqp_destroy)
-		request->anqp_destroy(request->anqp_data);
 
+	l_free(request->frame);
 	l_free(request);
 }
 
-static bool match_token(const void *a, const void *b)
-{
-	const struct anqp_request *request = a;
-	const struct token_match {
-		uint32_t ifindex;
-		uint8_t token;
-
-	} *match = b;
-
-	if (request->ifindex != match->ifindex)
-		return false;
-
-	if (request->anqp_token != match->token)
-		return false;
-
-	return true;
-}
-
-static void anqp_response_frame_event(uint32_t ifindex,
-					const struct mmpdu_header *hdr,
-					const void *body, size_t body_len)
+/*
+ * By using frame-xchg we should get called back here for any frame matching our
+ * prefix until the duration expires. If frame-xchg is never signalled 'done'
+ * (returning true) we should get a timeout in anqp_frame_timeout. This is
+ * why we drop any improperly formatted frames without cleaning up the request.
+ */
+static bool anqp_response_frame_event(const struct mmpdu_header *hdr,
+					const void *body, size_t body_len,
+					int rssi, void *user_data)
 {
-	struct anqp_request *request;
+	struct anqp_request *request = user_data;
 	const uint8_t *ptr = body;
 	uint16_t status_code;
 	uint16_t delay;
 	uint16_t qrlen;
 	uint8_t adv_proto_len;
 	uint8_t token;
-	struct token_match {
-		uint32_t ifindex;
-		uint8_t token;
-
-	} match;
 
 	if (body_len < 9)
-		return;
+		return false;
 
 	/* Skip past category/action since this frame was prefixed matched */
 	ptr += 2;
@@ -138,12 +94,8 @@ static void anqp_response_frame_event(uint32_t ifindex,
 	/* dialog token */
 	token = *ptr++;
 
-	match.ifindex = ifindex;
-	match.token = token;
-
-	request = l_queue_find(anqp_requests, match_token, &match);
-	if (!request)
-		return;
+	if (request->anqp_token != token)
+		return false;
 
 	status_code = l_get_le16(ptr);
 	ptr += 2;
@@ -151,7 +103,7 @@ static void anqp_response_frame_event(uint32_t ifindex,
 
 	if (status_code != 0) {
 		l_error("Bad status code on GAS response %u", status_code);
-		return;
+		return false;
 	}
 
 	delay = l_get_le16(ptr);
@@ -166,12 +118,12 @@ static void anqp_response_frame_event(uint32_t ifindex,
 	 */
 	if (delay != 0) {
 		l_error("GAS comeback delay was not zero");
-		return;
+		return false;
 	}
 
 	if (*ptr != IE_TYPE_ADVERTISEMENT_PROTOCOL) {
 		l_error("GAS request not advertisement protocol");
-		return;
+		return false;
 	}
 
 	ptr++;
@@ -181,339 +133,128 @@ static void anqp_response_frame_event(uint32_t ifindex,
 	body_len--;
 
 	if (body_len < adv_proto_len)
-		return;
+		return false;
 
 	ptr += adv_proto_len;
 	body_len -= adv_proto_len;
 
 	if (body_len < 2)
-		return;
+		return false;
 
 	qrlen = l_get_le16(ptr);
 	ptr += 2;
 
 	if (body_len < qrlen)
-		return;
-
-	l_queue_remove(anqp_requests, request);
+		return false;
 
 	l_debug("ANQP response received from "MAC, MAC_STR(hdr->address_2));
 
 	if (request->anqp_cb)
-		request->anqp_cb(ANQP_SUCCESS, ptr, qrlen,
-					request->anqp_data);
-
-	if (request->anqp_destroy)
-		request->anqp_destroy(request->anqp_data);
+		request->anqp_cb(ANQP_SUCCESS, ptr, qrlen, request->anqp_data);
 
-	l_free(request);
+	anqp_destroy(request);
 
-	return;
+	return true;
 }
 
-static void netdev_gas_timeout_cb(void *user_data)
+static const struct frame_xchg_prefix anqp_frame_prefix = {
+	.data = (uint8_t []) {
+		0x04, 0x0b,
+	},
+	.len = 2,
+};
+
+static void anqp_frame_timeout(int error, void *user_data)
 {
 	struct anqp_request *request = user_data;
+	enum anqp_result result = ANQP_TIMEOUT;
 
-	l_debug("GAS request timed out");
+	if (error < 0) {
+		result = ANQP_FAILED;
+		l_error("Sending ANQP request failed: %s (%i)",
+			strerror(-error), -error);
+	}
 
 	if (request->anqp_cb)
-		request->anqp_cb(ANQP_TIMEOUT, NULL, 0,
-					request->anqp_data);
+		request->anqp_cb(result, NULL, 0, request->anqp_data);
 
-	/* allows anqp_request to be re-entrant */
 	if (request->anqp_destroy)
 		request->anqp_destroy(request->anqp_data);
 
-	l_queue_remove(anqp_requests, request);
-	l_free(request);
+	anqp_destroy(request);
 }
 
-static bool match_cookie(const void *a, const void *b)
+static uint8_t *anqp_build_frame(const uint8_t *addr, struct scan_bss *bss,
+					const uint8_t *anqp, size_t len,
+					size_t *len_out)
 {
-	const struct anqp_request *request = a;
-	const struct cookie_match {
-		uint64_t cookie;
-		uint32_t ifindex;
-	} *match = b;
+	uint8_t *frame = l_malloc(len + 33);
+	uint8_t *ptr;
 
-	if (match->ifindex != request->ifindex)
-		return false;
+	memset(frame, 0, len + 33);
 
-	if (match->cookie != request->anqp_cookie)
-		return false;
+	l_put_le16(0x00d0, frame + 0);
+	memcpy(frame + 4, bss->addr, 6);
+	memcpy(frame + 10, addr, 6);
+	memcpy(frame + 16, bss->addr, 6);
 
-	return true;
-}
+	ptr = frame + 24;
 
-static void anqp_frame_wait_cancel_event(struct l_genl_msg *msg,
-						uint32_t ifindex)
-{
-	uint64_t cookie;
-	struct anqp_request *request;
-	struct cookie_match {
-		uint64_t cookie;
-		uint32_t ifindex;
-	} match;
+	*ptr++ = 0x04;			/* Category: Public */
+	*ptr++ = 0x0a;			/* Action: GAS initial Request */
+	*ptr++ = anqp_token++;		/* Dialog Token */
+	*ptr++ = IE_TYPE_ADVERTISEMENT_PROTOCOL;
+	*ptr++ = 2;
 
-	l_debug("");
-
-	if (nl80211_parse_attrs(msg, NL80211_ATTR_COOKIE, &cookie,
-					NL80211_ATTR_UNSPEC) < 0)
-		return;
-
-	match.cookie = cookie;
-	match.ifindex = ifindex;
+	*ptr++ = 0x7f;
+	*ptr++ = IE_ADVERTISEMENT_ANQP;
+	l_put_le16(len, ptr);
+	ptr += 2;
 
-	request = l_queue_find(anqp_requests, match_cookie, &match);
-	if (!request)
-		return;
+	memcpy(ptr, anqp, len);
+	ptr += len;
 
-	if (cookie != request->anqp_cookie)
-		return;
+	*len_out = ptr - frame;
 
-	netdev_gas_timeout_cb(request);
+	return frame;
 }
 
-uint32_t anqp_request(uint32_t ifindex, const uint8_t *addr,
-				struct scan_bss *bss, const uint8_t *anqp,
-				size_t len, anqp_response_func_t cb,
-				void *user_data, anqp_destroy_func_t destroy)
+uint32_t anqp_request(uint64_t wdev_id, const uint8_t *addr,
+			struct scan_bss *bss, const uint8_t *anqp,
+			size_t len, anqp_response_func_t cb,
+			void *user_data, anqp_destroy_func_t destroy)
 {
 	struct anqp_request *request;
-	uint8_t frame[512];
-	struct l_genl_msg *msg;
 	struct iovec iov[2];
-	uint32_t id;
-	uint32_t duration = 300;
-	struct netdev *netdev = netdev_find(ifindex);
-
-	if (!netdev)
-		return 0;
-
-	/*
-	 * TODO: Netdev dependencies will eventually be removed so we need
-	 * another way to figure out wiphy capabilities.
-	 */
-	if (!wiphy_can_offchannel_tx(netdev_get_wiphy(netdev))) {
-		l_error("ANQP failed, driver does not support offchannel TX");
-		return 0;
-	}
-
-	frame[0] = 0x04;		/* Category: Public */
-	frame[1] = 0x0a;		/* Action: GAS initial Request */
-	frame[2] = anqp_token;		/* Dialog Token */
-	frame[3] = IE_TYPE_ADVERTISEMENT_PROTOCOL;
-	frame[4] = 2;
-	frame[5] = 0x7f;
-	frame[6] = IE_ADVERTISEMENT_ANQP;
-	l_put_le16(len, frame + 7);
-
-	iov[0].iov_base = frame;
-	iov[0].iov_len = 9;
-	iov[1].iov_base = (void *)anqp;
-	iov[1].iov_len = len;
 
 	request = l_new(struct anqp_request, 1);
 
-	request->ifindex = ifindex;
+	request->wdev_id = wdev_id;
+	request->frequency = bss->frequency;
 	request->anqp_cb = cb;
 	request->anqp_destroy = destroy;
-	request->anqp_token = anqp_token++;
+	request->anqp_token = anqp_token;
 	request->anqp_data = user_data;
 
-	msg = nl80211_build_cmd_frame(ifindex, addr, bss->addr,
-					bss->frequency, iov, 2);
-
-	l_genl_msg_append_attr(msg, NL80211_ATTR_OFFCHANNEL_TX_OK, 0, "");
-	l_genl_msg_append_attr(msg, NL80211_ATTR_DURATION, 4, &duration);
-
-	id = l_genl_family_send(nl80211, msg, netdev_gas_request_cb,
-					request, NULL);
-
-	if (!id) {
-		l_debug("Failed to send ANQP request");
-		l_genl_msg_unref(msg);
-		l_free(request);
-		return 0;
-	}
-
-	l_debug("ANQP request sent to "MAC, MAC_STR(bss->addr));
-
-	l_queue_push_head(anqp_requests, request);
-
-	return id;
-}
-
-static void netdev_frame_cb(struct l_genl_msg *msg, void *user_data)
-{
-	if (l_genl_msg_get_error(msg) < 0)
-		l_error("Could not register frame watch type %04x: %i",
-			L_PTR_TO_UINT(user_data), l_genl_msg_get_error(msg));
-}
-
-static void anqp_register_frame(uint32_t ifindex)
-{
-	struct l_genl_msg *msg;
-	uint16_t frame_type = 0x00d0;
-	uint8_t prefix[] = { 0x04, 0x0b };
-
-	msg = l_genl_msg_new_sized(NL80211_CMD_REGISTER_FRAME, 34);
-
-	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &ifindex);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_FRAME_TYPE, 2, &frame_type);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_FRAME_MATCH,
-					sizeof(prefix), prefix);
-
-	l_genl_family_send(nl80211, msg, netdev_frame_cb,
-			L_UINT_TO_PTR(frame_type), NULL);
-}
-
-static void anqp_netdev_watch(struct netdev *netdev,
-				enum netdev_watch_event event, void *user_data)
-{
-	switch (event) {
-	case NETDEV_WATCH_EVENT_NEW:
-		if (netdev_get_iftype(netdev) == NETDEV_IFTYPE_STATION)
-			anqp_register_frame(netdev_get_ifindex(netdev));
-
-		return;
-	default:
-		break;
-	}
-}
-
-static void anqp_unicast_notify(struct l_genl_msg *msg, void *user_data)
-{
-	const struct mmpdu_header *mpdu = NULL;
-	const uint8_t *body;
-	struct l_genl_attr attr;
-	uint16_t type, len;
-	uint16_t frame_len = 0;
-	const void *data;
-	uint8_t cmd;
-	uint32_t ifindex = 0;
-
-	if (l_queue_isempty(anqp_requests))
-		return;
-
-	cmd = l_genl_msg_get_command(msg);
-	if (!cmd)
-		return;
-
-	if (!l_genl_attr_init(&attr, msg))
-		return;
-
-	while (l_genl_attr_next(&attr, &type, &len, &data)) {
-		switch (type) {
-		case NL80211_ATTR_IFINDEX:
-			if (len != sizeof(uint32_t)) {
-				l_warn("Invalid interface index attribute");
-				return;
-			}
-
-			ifindex = *((uint32_t *) data);
-
-			break;
-		case NL80211_ATTR_FRAME:
-			if (mpdu)
-				return;
-
-			mpdu = mpdu_validate(data, len);
-			if (!mpdu)
-				l_error("Frame didn't validate as MMPDU");
-
-			frame_len = len;
-			break;
-		}
-	}
-
-	if (!ifindex || !mpdu)
-		return;
-
-	body = mmpdu_body(mpdu);
-
-	anqp_response_frame_event(ifindex, mpdu, body,
-				(const uint8_t *) mpdu + frame_len - body);
-}
-
-static void anqp_mlme_notify(struct l_genl_msg *msg, void *user_data)
-{
-	struct l_genl_attr attr;
-	uint16_t type, len;
-	const void *data;
-	uint8_t cmd;
-	uint32_t ifindex = 0;
-
-	if (l_queue_isempty(anqp_requests))
-		return;
-
-	cmd = l_genl_msg_get_command(msg);
-
-	l_debug("MLME notification %s(%u)", nl80211cmd_to_string(cmd), cmd);
-
-	if (!l_genl_attr_init(&attr, msg))
-		return;
-
-	while (l_genl_attr_next(&attr, &type, &len, &data)) {
-		switch (type) {
-		case NL80211_ATTR_IFINDEX:
-			if (len != sizeof(uint32_t)) {
-				l_warn("Invalid interface index attribute");
-				return;
-			}
-
-			ifindex = *((uint32_t *) data);
-			break;
-		}
-	}
+	request->frame = anqp_build_frame(addr, bss, anqp, len,
+						&request->frame_len);
 
-	if (!ifindex) {
-		l_warn("MLME notification is missing ifindex attribute");
-		return;
-	}
-
-	switch (cmd) {
-	case NL80211_CMD_FRAME_WAIT_CANCEL:
-		anqp_frame_wait_cancel_event(msg, ifindex);
-		break;
-	}
-}
-
-static int anqp_init(void)
-{
-	struct l_genl *genl = iwd_get_genl();
-
-	nl80211 = l_genl_family_new(genl, NL80211_GENL_NAME);
-
-	anqp_requests = l_queue_new();
+	iov[0].iov_base = request->frame;
+	iov[0].iov_len = request->frame_len;
+	iov[1].iov_base = NULL;
 
-	netdev_watch =  netdev_watch_add(anqp_netdev_watch, NULL, NULL);
+	l_debug("Sending ANQP request");
 
-	unicast_watch = l_genl_add_unicast_watch(genl, NL80211_GENL_NAME,
-						anqp_unicast_notify,
-						NULL, NULL);
+	request->id = frame_xchg_start(request->wdev_id, iov,
+				request->frequency, 0, 300, 0,
+				ANQP_GROUP, anqp_frame_timeout, request, NULL,
+				&anqp_frame_prefix, anqp_response_frame_event,
+				NULL);
 
-	if (!l_genl_family_register(nl80211, "mlme", anqp_mlme_notify,
-								NULL, NULL))
-		l_error("Registering for MLME notification failed");
-
-	return 0;
+	return true;
 }
 
-static void anqp_exit(void)
+void anqp_cancel(uint32_t id)
 {
-	struct l_genl *genl = iwd_get_genl();
-
-	l_genl_family_free(nl80211);
-	nl80211 = NULL;
-
-	l_queue_destroy(anqp_requests, anqp_destroy);
-
-	netdev_watch_remove(netdev_watch);
-
-	l_genl_remove_unicast_watch(genl, unicast_watch);
+	frame_xchg_cancel(id);
 }
-
-IWD_MODULE(anqp, anqp_init, anqp_exit);
-IWD_MODULE_DEPENDS(anqp, netdev);
diff --git a/src/anqp.h b/src/anqp.h
index 998277dd..4d96c9ec 100644
--- a/src/anqp.h
+++ b/src/anqp.h
@@ -34,7 +34,8 @@ typedef void (*anqp_response_func_t)(enum anqp_result result,
 					const void *anqp, size_t len,
 					void *user_data);
 
-uint32_t anqp_request(uint32_t ifindex, const uint8_t *addr,
+uint32_t anqp_request(uint64_t wdev_id, const uint8_t *addr,
 			struct scan_bss *bss, const uint8_t *anqp, size_t len,
 			anqp_response_func_t cb, void *user_data,
 			anqp_destroy_func_t destroy);
+void anqp_cancel(uint32_t id);
diff --git a/src/station.c b/src/station.c
index 80b6f07e..b23fd1a0 100644
--- a/src/station.c
+++ b/src/station.c
@@ -446,6 +446,9 @@ static void remove_anqp(void *data)
 {
 	struct anqp_entry *entry = data;
 
+	if (entry->pending)
+		anqp_cancel(entry->pending);
+
 	l_free(entry);
 }
 
@@ -475,12 +478,9 @@ static void station_anqp_response_cb(enum anqp_result result,
 	char **realms = NULL;
 	struct nai_search search;
 
-	entry->pending = 0;
-
 	l_debug("");
 
-	if (result == ANQP_TIMEOUT) {
-		l_queue_remove(station->anqp_pending, entry);
+	if (result != ANQP_SUCCESS) {
 		/* TODO: try next BSS */
 		goto request_done;
 	}
@@ -582,11 +582,10 @@ static bool station_start_anqp(struct station *station, struct network *network,
 	 * these are checked in hs20_find_settings_file.
 	 */
 
-	entry->pending = anqp_request(netdev_get_ifindex(station->netdev),
-					netdev_get_address(station->netdev),
-					bss, anqp, ptr - anqp,
-					station_anqp_response_cb,
-					entry, l_free);
+	entry->pending = anqp_request(netdev_get_wdev_id(station->netdev),
+				netdev_get_address(station->netdev), bss, anqp,
+				ptr - anqp, station_anqp_response_cb,
+				entry, NULL);
 	if (!entry->pending) {
 		l_free(entry);
 		return false;
@@ -3215,7 +3214,7 @@ static void station_free(struct station *station)
 
 	watchlist_destroy(&station->state_watches);
 
-	l_queue_destroy(station->anqp_pending, l_free);
+	l_queue_destroy(station->anqp_pending, remove_anqp);
 
 	l_free(station);
 }
-- 
2.21.1

[PATCH v2 5/7] scan: refactor to use radio management module

[James Prestwood]

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

To use radio management scanning mostly remained the same.
start_next_scan_request was modified to be used as the work callback,
as well as not start the next scan if the current one was done
(since this is taken care of by radio management now). All calls to
start_next_scan_request were removed, and more or less replaced with
radio_mgmt_done.

scan_{suspend,resume} were both removed since radio management
priorities solve this for us. ANQP requests can be inserted ahead of
scan requests, which accomplishes the same thing.
---
 src/scan.c    | 104 +++++++++++++++-----------------------------------
 src/scan.h    |   3 --
 src/station.c |  16 +-------
 3 files changed, 31 insertions(+), 92 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index 83804e57..c3e201d4 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -49,6 +49,7 @@
 #include "src/p2putil.h"
 #include "src/mpdu.h"
 #include "src/scan.h"
+#include "src/radio_mgmt.h"
 
 #define SCAN_MAX_INTERVAL 320
 #define SCAN_INIT_INTERVAL 10
@@ -56,7 +57,8 @@
 static struct l_queue *scan_contexts;
 
 static struct l_genl_family *nl80211;
-static uint32_t next_scan_request_id;
+
+struct scan_context;
 
 struct scan_periodic {
 	struct l_timeout *timeout;
@@ -70,6 +72,7 @@ struct scan_periodic {
 };
 
 struct scan_request {
+	struct scan_context *sc;
 	uint32_t id;
 	scan_trigger_func_t trigger;
 	scan_notify_func_t callback;
@@ -106,7 +109,6 @@ struct scan_context {
 	bool triggered:1;
 	/* Whether any commands from current request's queue have started */
 	bool started:1;
-	bool suspended:1;
 	struct wiphy *wiphy;
 };
 
@@ -118,7 +120,7 @@ struct scan_results {
 	struct scan_request *sr;
 };
 
-static bool start_next_scan_request(struct scan_context *sc);
+static bool start_next_scan_request(void *user_data);
 static void scan_periodic_rearm(struct scan_context *sc);
 
 static bool scan_context_match(const void *a, const void *b)
@@ -159,7 +161,7 @@ static void scan_request_failed(struct scan_context *sc,
 	else if (sr->callback)
 		sr->callback(err, NULL, sr->userdata);
 
-	scan_request_free(sr);
+	radio_mgmt_done(sc->wdev_id, sr->id);
 }
 
 static struct scan_context *scan_context_new(uint64_t wdev_id)
@@ -180,11 +182,18 @@ static struct scan_context *scan_context_new(uint64_t wdev_id)
 	return sc;
 }
 
+static void scan_request_cancel(void *data)
+{
+	struct scan_request *sr = data;
+
+	radio_mgmt_done(sr->sc->wdev_id, sr->id);
+}
+
 static void scan_context_free(struct scan_context *sc)
 {
 	l_debug("sc: %p", sc);
 
-	l_queue_destroy(sc->requests, scan_request_free);
+	l_queue_destroy(sc->requests, scan_request_cancel);
 
 	if (sc->sp.timeout)
 		l_timeout_remove(sc->sp.timeout);
@@ -215,7 +224,6 @@ static void scan_request_triggered(struct l_genl_msg *msg, void *userdata)
 		}
 
 		l_queue_remove(sc->requests, sr);
-		start_next_scan_request(sc);
 
 		scan_request_failed(sc, sr, err);
 
@@ -532,35 +540,21 @@ static uint32_t scan_common(uint64_t wdev_id, bool passive,
 		return 0;
 
 	sr = l_new(struct scan_request, 1);
+	sr->sc = sc;
 	sr->trigger = trigger;
 	sr->callback = notify;
 	sr->userdata = userdata;
 	sr->destroy = destroy;
 	sr->passive = passive;
-	sr->id = ++next_scan_request_id;
 	sr->cmds = l_queue_new();
 
 	scan_cmds_add(sr->cmds, sc, passive, params);
 
-	/* Queue empty implies !sc->triggered && !sc->start_cmd_id */
-	if (!l_queue_isempty(sc->requests))
-		goto done;
-
-	if (sc->suspended)
-		goto done;
-
-	if (sc->state != SCAN_STATE_NOT_RUNNING)
-		goto done;
-
-	if (!scan_request_send_trigger(sc, sr))
-		goto done;
-
-	sr->destroy = NULL;	/* Don't call destroy when returning error */
-	scan_request_free(sr);
-	return 0;
-done:
 	l_queue_push_tail(sc->requests, sr);
 
+	sr->id = radio_mgmt_push(wdev_id, 2, start_next_scan_request, sr,
+					scan_request_free);
+
 	return sr->id;
 }
 
@@ -633,6 +627,8 @@ bool scan_cancel(uint64_t wdev_id, uint32_t id)
 			sr->destroy = NULL;
 		}
 
+		radio_mgmt_done(wdev_id, sr->id);
+
 		return true;
 	}
 
@@ -649,11 +645,11 @@ bool scan_cancel(uint64_t wdev_id, uint32_t id)
 		sc->start_cmd_id = 0;
 		l_queue_remove(sc->requests, sr);
 		sc->started = false;
-		start_next_scan_request(sc);
 	} else
 		l_queue_remove(sc->requests, sr);
 
-	scan_request_free(sr);
+	radio_mgmt_done(wdev_id, sr->id);
+
 	return true;
 }
 
@@ -832,34 +828,22 @@ static void scan_periodic_rearm(struct scan_context *sc)
 						scan_periodic_timeout_destroy);
 }
 
-static bool start_next_scan_request(struct scan_context *sc)
+static bool start_next_scan_request(void *user_data)
 {
-	struct scan_request *sr = l_queue_peek_head(sc->requests);
+	struct scan_request *sr = user_data;
+	struct scan_context *sc = sr->sc;
 
-	if (sc->suspended)
-		return true;
-
-	if (sc->state != SCAN_STATE_NOT_RUNNING)
-		return true;
-
-	if (sc->start_cmd_id || sc->get_scan_cmd_id)
-		return true;
-
-	while (sr) {
-		if (!scan_request_send_trigger(sc, sr))
-			return true;
-
-		scan_request_failed(sc, sr, -EIO);
+	if (!scan_request_send_trigger(sc, sr))
+		return false;
 
-		sr = l_queue_peek_head(sc->requests);
-	}
+	scan_request_failed(sc, sr, -EIO);
 
 	if (sc->sp.retry) {
 		sc->sp.retry = false;
 		scan_periodic_queue(sc);
 	}
 
-	return false;
+	return true;
 }
 
 static bool scan_parse_vendor_specific(struct scan_bss *bss, const void *data,
@@ -1511,9 +1495,7 @@ static void scan_finished(struct scan_context *sc,
 		 * taken care of sending the next command for a new or ongoing
 		 * scan, or scheduling the next periodic scan.
 		 */
-		start_next_scan_request(sc);
-
-		scan_request_free(sr);
+		radio_mgmt_done(sc->wdev_id, sr->id);
 	} else if (sc->sp.callback)
 		new_owner = sc->sp.callback(err, bss_list, sc->sp.userdata);
 
@@ -2133,32 +2115,6 @@ bool scan_wdev_remove(uint64_t wdev_id)
 	return true;
 }
 
-bool scan_suspend(uint64_t wdev_id)
-{
-	struct scan_context *sc;
-
-	sc = l_queue_find(scan_contexts, scan_context_match, &wdev_id);
-	if (!sc)
-		return false;
-
-	sc->suspended = true;
-
-	return true;
-}
-
-void scan_resume(uint64_t wdev_id)
-{
-	struct scan_context *sc;
-
-	sc = l_queue_find(scan_contexts, scan_context_match, &wdev_id);
-	if (!sc)
-		return;
-
-	sc->suspended = false;
-
-	start_next_scan_request(sc);
-}
-
 static int scan_init(void)
 {
 	const struct l_settings *config = iwd_get_config();
diff --git a/src/scan.h b/src/scan.h
index aeeddf05..df0cc17c 100644
--- a/src/scan.h
+++ b/src/scan.h
@@ -170,6 +170,3 @@ bool scan_freq_set_isempty(const struct scan_freq_set *set);
 
 bool scan_wdev_add(uint64_t wdev_id);
 bool scan_wdev_remove(uint64_t wdev_id);
-
-bool scan_suspend(uint64_t wdev_id);
-void scan_resume(uint64_t wdev_id);
diff --git a/src/station.c b/src/station.c
index b23fd1a0..dd9347a8 100644
--- a/src/station.c
+++ b/src/station.c
@@ -530,8 +530,6 @@ request_done:
 		station_network_foreach(station, network_add_foreach, station);
 		station_autoconnect_next(station);
 	}
-
-	scan_resume(netdev_get_wdev_id(station->netdev));
 }
 
 static bool station_start_anqp(struct station *station, struct network *network,
@@ -661,19 +659,7 @@ void station_set_scan_results(struct station *station,
 
 	l_hashmap_foreach_remove(station->networks, process_network, station);
 
-	/*
-	 * ANQP requests are scheduled in the same manor as scans, and cannot
-	 * be done simultaneously. To avoid long queue times (waiting for a
-	 * scan to finish) its best to stop scanning, do ANQP, then resume
-	 * scanning.
-	 *
-	 * TODO: It may be possible for some hardware to actually scan and do
-	 * ANQP at the same time. Detecting this could allow us to continue
-	 * scanning.
-	 */
-	if (wait_for_anqp)
-		scan_suspend(netdev_get_wdev_id(station->netdev));
-	else if (add_to_autoconnect) {
+	if (!wait_for_anqp && add_to_autoconnect) {
 		station_network_foreach(station, network_add_foreach, station);
 		station_autoconnect_next(station);
 	}
-- 
2.21.1

[PATCH v2 6/7] network: use radio management for connections

[James Prestwood]

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

Network was refactored to use the radio management module for
connections. This removes the need for an ANQP watch since we can
insert the connect work item at a lower priority than ANQP. This
will, by nature, force the connect attempt to wait for ANQP to
complete, ensuring that the network_info is populated.
---
 src/network.c | 148 ++++++++++++++++++++++++++------------------------
 src/station.c |   6 +-
 2 files changed, 81 insertions(+), 73 deletions(-)

diff --git a/src/network.c b/src/network.c
index 70541b59..c86d0c7b 100644
--- a/src/network.c
+++ b/src/network.c
@@ -50,9 +50,15 @@
 #include "src/network.h"
 #include "src/blacklist.h"
 #include "src/util.h"
+#include "src/radio_mgmt.h"
 
 static uint32_t known_networks_watch;
-static uint32_t anqp_watch;
+
+struct network_connect_info {
+	uint32_t work_id;
+	struct scan_bss *bss;
+	struct l_dbus_message *message;
+};
 
 struct network {
 	char ssid[33];
@@ -73,10 +79,8 @@ struct network {
 	bool update_psk:1;  /* Whether PSK should be written to storage */
 	bool ask_passphrase:1; /* Whether we should force-ask agent */
 	bool is_hs20:1;
-	bool anqp_pending:1;	/* Set if there is a pending ANQP request */
 	int rank;
-	/* Holds DBus Connect() message if it comes in before ANQP finishes */
-	struct l_dbus_message *connect_after_anqp;
+	struct network_connect_info connect;
 };
 
 static bool network_settings_load(struct network *network)
@@ -138,6 +142,7 @@ void network_connected(struct network *network)
 	enum security security = network_get_security(network);
 	const char *ssid = network_get_ssid(network);
 	int err;
+	struct netdev *netdev = station_get_netdev(network->station);
 
 	if (!network->info) {
 		/*
@@ -164,6 +169,8 @@ void network_connected(struct network *network)
 				network_secret_check_cacheable, network);
 
 	l_queue_clear(network->blacklist, NULL);
+
+	radio_mgmt_done(netdev_get_wdev_id(netdev), network->connect.work_id);
 }
 
 void network_disconnected(struct network *network)
@@ -516,17 +523,39 @@ static bool bss_is_sae(const struct scan_bss *bss)
 	return __bss_is_sae(bss, &rsn);
 }
 
+static void network_connect_info_destroy(void *user_data)
+{
+	struct network_connect_info *info = user_data;
+
+	if (info->message)
+		l_dbus_message_unref(info->message);
+
+	info->work_id = 0;
+	info->bss = NULL;
+	info->message = NULL;
+}
+
+static bool network_autoconnect_ready(void *user_data)
+{
+	struct network_connect_info *info = user_data;
+	struct network *network = l_container_of(info, struct network, connect);
+
+	return __station_connect_network(network->station, network,
+						info->bss) != 0;
+}
+
 int network_autoconnect(struct network *network, struct scan_bss *bss)
 {
 	struct station *station = network->station;
+	struct netdev *netdev = station_get_netdev(network->station);
 	struct wiphy *wiphy = station_get_wiphy(station);
 	enum security security = network_get_security(network);
 	struct ie_rsn_info rsn;
 	bool is_rsn;
 	int ret;
 
-	/* already waiting for an agent request, connect in progress */
-	if (network->agent_request)
+	/* A connect is already in progress */
+	if (network->connect.work_id)
 		return -EALREADY;
 
 	switch (security) {
@@ -588,7 +617,13 @@ int network_autoconnect(struct network *network, struct scan_bss *bss)
 	}
 
 done:
-	return __station_connect_network(station, network, bss);
+	network->connect.bss = bss;
+	network->connect.work_id = radio_mgmt_push(netdev_get_wdev_id(netdev),
+						1, network_autoconnect_ready,
+						&network->connect,
+						network_connect_info_destroy);
+
+	return 0;
 
 close_settings:
 	network_settings_close(network);
@@ -597,6 +632,8 @@ close_settings:
 
 void network_connect_failed(struct network *network, bool in_handshake)
 {
+	struct netdev *netdev = station_get_netdev(network->station);
+
 	/*
 	 * Connection failed during the handshake phase.  If PSK try asking
 	 * for the passphrase once more
@@ -610,6 +647,8 @@ void network_connect_failed(struct network *network, bool in_handshake)
 	network->secrets = NULL;
 
 	l_queue_clear(network->blacklist, NULL);
+
+	radio_mgmt_done(netdev_get_wdev_id(netdev), network->connect.work_id);
 }
 
 static bool hotspot_info_matches(struct network *network,
@@ -1130,12 +1169,37 @@ error:
 	return reply;
 }
 
+static bool network_connect_ready(void *user_data)
+{
+	struct network_connect_info *info = user_data;
+	struct network *network = l_container_of(info, struct network, connect);
+	struct scan_bss *bss = info->bss;
+	struct l_dbus_message *message = info->message;
+
+	switch (network_get_security(network)) {
+	case SECURITY_PSK:
+		return network_connect_psk(network, bss, message);
+	case SECURITY_NONE:
+		station_connect_network(network->station, network, bss,
+						message);
+		return NULL;
+	case SECURITY_8021X:
+		if (!network_settings_load(network))
+			return dbus_error_not_configured(message);
+
+		return network_connect_8021x(network, bss, message);
+	default:
+		return dbus_error_not_supported(message);
+	}
+}
+
 static struct l_dbus_message *network_connect(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
 {
 	struct network *network = user_data;
 	struct station *station = network->station;
+	struct netdev *netdev = station_get_netdev(station);
 	struct scan_bss *bss;
 
 	l_debug("");
@@ -1157,36 +1221,14 @@ static struct l_dbus_message *network_connect(struct l_dbus *dbus,
 	if (!bss)
 		return dbus_error_not_supported(message);
 
-	switch (network_get_security(network)) {
-	case SECURITY_PSK:
-		return network_connect_psk(network, bss, message);
-	case SECURITY_NONE:
-		station_connect_network(station, network, bss, message);
-		return NULL;
-	case SECURITY_8021X:
-		if (network->connect_after_anqp)
-			return dbus_error_busy(message);
-
-		/*
-		 * If there is an ongoing ANQP request we must wait for that to
-		 * finish. Save the message and wait for the ANQP watch to
-		 * fire
-		 */
-		if (network->anqp_pending) {
-			network->connect_after_anqp =
-						l_dbus_message_ref(message);
-			l_debug("Pending ANQP request, delaying connect to %s",
-						network->ssid);
-			return NULL;
-		}
+	network->connect.bss = bss;
+	network->connect.message = l_dbus_message_ref(message);
+	network->connect.work_id = radio_mgmt_push(netdev_get_wdev_id(netdev),
+						1, network_connect_ready,
+						&network->connect,
+						network_connect_info_destroy);
 
-		if (!network_settings_load(network))
-			return dbus_error_not_configured(message);
-
-		return network_connect_8021x(network, bss, message);
-	default:
-		return dbus_error_not_supported(message);
-	}
+	return NULL;
 }
 
 void network_connect_new_hidden_network(struct network *network,
@@ -1508,35 +1550,6 @@ static void known_networks_changed(enum known_networks_event event,
 	}
 }
 
-static void anqp_watch_changed(enum station_anqp_state state,
-				struct network *network, void *user_data)
-{
-	network->anqp_pending = state == STATION_ANQP_STARTED;
-
-	if (state == STATION_ANQP_FINISHED && network->connect_after_anqp) {
-		struct l_dbus_message *reply;
-
-		l_debug("ANQP complete, resuming connect to %s", network->ssid);
-
-		if (!network_settings_load(network)) {
-			reply = dbus_error_not_configured(
-						network->connect_after_anqp);
-			dbus_pending_reply(&network->connect_after_anqp, reply);
-			return;
-		}
-
-		reply = network_connect_8021x(network,
-					network_bss_select(network, true),
-					network->connect_after_anqp);
-
-		if (reply)
-			l_dbus_send(dbus_get_bus(), reply);
-
-		l_dbus_message_unref(network->connect_after_anqp);
-		network->connect_after_anqp = NULL;
-	}
-}
-
 static void setup_network_interface(struct l_dbus_interface *interface)
 {
 	l_dbus_interface_method(interface, "Connect", 0,
@@ -1570,8 +1583,6 @@ static int network_init(void)
 	known_networks_watch =
 		known_networks_watch_add(known_networks_changed, NULL, NULL);
 
-	anqp_watch = station_add_anqp_watch(anqp_watch_changed, NULL, NULL);
-
 	return 0;
 }
 
@@ -1580,9 +1591,6 @@ static void network_exit(void)
 	known_networks_watch_remove(known_networks_watch);
 	known_networks_watch = 0;
 
-	station_remove_anqp_watch(anqp_watch);
-	anqp_watch = 0;
-
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_NETWORK_INTERFACE);
 }
 
diff --git a/src/station.c b/src/station.c
index dd9347a8..69c4ae01 100644
--- a/src/station.c
+++ b/src/station.c
@@ -183,10 +183,8 @@ static void station_autoconnect_next(struct station *station)
 		r = network_autoconnect(entry->network, entry->bss);
 		l_free(entry);
 
-		if (!r) {
-			station_enter_state(station, STATION_STATE_CONNECTING);
+		if (!r)
 			return;
-		}
 	}
 }
 
@@ -2377,6 +2375,8 @@ int __station_connect_network(struct station *station, struct network *network,
 	station->connected_bss = bss;
 	station->connected_network = network;
 
+	station_enter_state(station, STATION_STATE_CONNECTING);
+
 	return 0;
 }
 
-- 
2.21.1

[PATCH v2 7/7] station: remove ANQP watch

[James Prestwood]

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

---
 src/station.c | 34 ++--------------------------------
 src/station.h |  5 -----
 2 files changed, 2 insertions(+), 37 deletions(-)

diff --git a/src/station.c b/src/station.c
index 69c4ae01..113f030b 100644
--- a/src/station.c
+++ b/src/station.c
@@ -59,7 +59,6 @@ static uint32_t netdev_watch;
 static uint32_t mfp_setting;
 static bool anqp_disabled;
 static bool netconfig_enabled;
-static struct watchlist anqp_watches;
 
 struct station {
 	enum station_state state;
@@ -450,18 +449,6 @@ static void remove_anqp(void *data)
 	l_free(entry);
 }
 
-static bool anqp_entry_foreach(void *data, void *user_data)
-{
-	struct anqp_entry *e = data;
-
-	WATCHLIST_NOTIFY(&anqp_watches, station_anqp_watch_func_t,
-				STATION_ANQP_FINISHED, e->network);
-
-	remove_anqp(e);
-
-	return true;
-}
-
 static void station_anqp_response_cb(enum anqp_result result,
 					const void *anqp, size_t anqp_len,
 					void *user_data)
@@ -518,8 +505,8 @@ request_done:
 	if (l_queue_find(station->anqp_pending, match_pending, NULL))
 		return;
 
-	/* Notify all watchers now that every ANQP request has finished */
-	l_queue_foreach_remove(station->anqp_pending, anqp_entry_foreach, NULL);
+	/* Clear queue now that every ANQP request has finished */
+	l_queue_clear(station->anqp_pending, remove_anqp);
 
 	l_queue_destroy(station->autoconnect_list, l_free);
 	station->autoconnect_list = l_queue_new();
@@ -589,8 +576,6 @@ static bool station_start_anqp(struct station *station, struct network *network,
 
 	l_queue_push_head(station->anqp_pending, entry);
 
-	WATCHLIST_NOTIFY(&anqp_watches, station_anqp_watch_func_t,
-				STATION_ANQP_STARTED, network);
 	return true;
 }
 
@@ -1232,18 +1217,6 @@ bool station_remove_state_watch(struct station *station, uint32_t id)
 	return watchlist_remove(&station->state_watches, id);
 }
 
-uint32_t station_add_anqp_watch(station_anqp_watch_func_t func,
-				void *user_data,
-				station_destroy_func_t destroy)
-{
-	return watchlist_add(&anqp_watches, func, user_data, destroy);
-}
-
-void station_remove_anqp_watch(uint32_t id)
-{
-	watchlist_remove(&anqp_watches, id);
-}
-
 bool station_set_autoconnect(struct station *station, bool autoconnect)
 {
 	if (station->autoconnect == autoconnect)
@@ -3303,8 +3276,6 @@ static int station_init(void)
 	if (!netconfig_enabled)
 		l_info("station: Network configuration is disabled.");
 
-	watchlist_init(&anqp_watches, NULL);
-
 	return 0;
 }
 
@@ -3314,7 +3285,6 @@ static void station_exit(void)
 	netdev_watch_remove(netdev_watch);
 	l_queue_destroy(station_list, NULL);
 	station_list = NULL;
-	watchlist_destroy(&anqp_watches);
 }
 
 IWD_MODULE(station, station_init, station_exit)
diff --git a/src/station.h b/src/station.h
index 17a0f8df..a9a33afe 100644
--- a/src/station.h
+++ b/src/station.h
@@ -74,11 +74,6 @@ uint32_t station_add_state_watch(struct station *station,
 					station_destroy_func_t destroy);
 bool station_remove_state_watch(struct station *station, uint32_t id);
 
-uint32_t station_add_anqp_watch(station_anqp_watch_func_t func,
-				void *user_data,
-				station_destroy_func_t destroy);
-void station_remove_anqp_watch(uint32_t id);
-
 bool station_set_autoconnect(struct station *station, bool autoconnect);
 
 void station_ap_directed_roam(struct station *station,
-- 
2.21.1