[PATCH 1/6] frame-xchg: Add no-cck-rate flag only for P2P interfaces

[PATCH 1/6] frame-xchg: Add no-cck-rate flag only for P2P interfaces

[Andrew Zaborowski]

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

We want to use this flag only on the interfaces with one of the three
P2P iftypes so set the flag automatically depending on the iftype from
the last 'config' notification.
---
 src/frame-xchg.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/src/frame-xchg.c b/src/frame-xchg.c
index 2ea8504a..d26e0c54 100644
--- a/src/frame-xchg.c
+++ b/src/frame-xchg.c
@@ -115,6 +115,7 @@ struct frame_xchg_data {
 	unsigned int resp_timeout;
 	unsigned int retries_on_ack;
 	struct wiphy_radio_work_item work;
+	bool no_cck_rates;
 };
 
 struct frame_xchg_watch_data {
@@ -951,10 +952,13 @@ static bool frame_xchg_tx_retry(struct wiphy_radio_work_item *item)
 	l_genl_msg_append_attr(msg, NL80211_ATTR_WDEV, 8, &fx->wdev_id);
 	l_genl_msg_append_attr(msg, NL80211_ATTR_WIPHY_FREQ, 4, &fx->freq);
 	l_genl_msg_append_attr(msg, NL80211_ATTR_OFFCHANNEL_TX_OK, 0, NULL);
-	l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0, NULL);
 	l_genl_msg_append_attr(msg, NL80211_ATTR_FRAME,
 				fx->tx_mpdu_len, fx->tx_mpdu);
 
+	if (fx->no_cck_rates)
+		l_genl_msg_append_attr(msg, NL80211_ATTR_TX_NO_CCK_RATE, 0,
+					NULL);
+
 	if (duration)
 		l_genl_msg_append_attr(msg, NL80211_ATTR_DURATION, 4,
 					&duration);
@@ -1102,6 +1106,14 @@ static const struct wiphy_radio_work_item_ops work_ops = {
 	.destroy = frame_xchg_destroy,
 };
 
+static bool frame_xchg_wdev_match(const void *a, const void *b)
+{
+	const struct wdev_info *wdev = a;
+	const uint64_t *id = b;
+
+	return wdev->id == *id;
+}
+
 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,
@@ -1112,6 +1124,7 @@ uint32_t frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 	size_t frame_len;
 	struct iovec *iov;
 	uint8_t *ptr;
+	struct wdev_info *wdev;
 
 	for (frame_len = 0, iov = frame; iov->iov_base; iov++)
 		frame_len += iov->iov_len;
@@ -1150,6 +1163,12 @@ uint32_t frame_xchg_startv(uint64_t wdev_id, struct iovec *frame, uint32_t freq,
 	for (iov = frame; iov->iov_base; ptr += iov->iov_len, iov++)
 		memcpy(ptr, iov->iov_base, iov->iov_len);
 
+	wdev = l_queue_find(wdevs, frame_xchg_wdev_match, &wdev_id);
+	fx->no_cck_rates = wdev &&
+		(wdev->iftype == NL80211_IFTYPE_P2P_DEVICE ||
+		 wdev->iftype == NL80211_IFTYPE_P2P_CLIENT ||
+		 wdev->iftype == NL80211_IFTYPE_P2P_GO);
+
 	/*
 	 * Subscribe to the response frames now instead of in the ACK
 	 * callback to save ourselves race condition considerations.
@@ -1265,14 +1284,6 @@ static void frame_xchg_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-static bool frame_xchg_wdev_match(const void *a, const void *b)
-{
-	const struct wdev_info *wdev = a;
-	const uint64_t *id = b;
-
-	return wdev->id == *id;
-}
-
 static void frame_xchg_config_notify(struct l_genl_msg *msg, void *user_data)
 {
 	uint64_t wdev_id;
-- 
2.25.1

[PATCH 2/6] frame-xchg: Cancel NL80211_CMD_FRAME commands when interrupted

[Andrew Zaborowski]

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

The callback for the FRAME command was causing a crash in
wiphy_radio_work_done when not cancelled when the wiphy was being
removed from the system.  This was likely to happen if this radio work
item was waiting for another item to finish.  When the first one was
being cancelled due to the wiphy being removed, this one would be
started and immediately stopped by the radio work queue.

Now this crash could be fixed by dropping all frame exchange instances
on an interface that is being removed which is easy to do, but properly
cancelling the commands saves us the headache of analysing whether
there's a race condition in other situations where a frame exchange is
being aborted.
---
 src/frame-xchg.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/frame-xchg.c b/src/frame-xchg.c
index d26e0c54..0e3e330d 100644
--- a/src/frame-xchg.c
+++ b/src/frame-xchg.c
@@ -116,6 +116,7 @@ struct frame_xchg_data {
 	unsigned int retries_on_ack;
 	struct wiphy_radio_work_item work;
 	bool no_cck_rates;
+	unsigned int tx_cmd_id;
 };
 
 struct frame_xchg_watch_data {
@@ -772,6 +773,9 @@ static void frame_xchg_reset(struct frame_xchg_data *fx)
 	if (fx->timeout)
 		l_timeout_remove(fx->timeout);
 
+	if (fx->tx_cmd_id)
+		l_genl_family_cancel(nl80211, fx->tx_cmd_id);
+
 	l_free(fx->early_frame.mpdu);
 	fx->early_frame.mpdu = NULL;
 	l_queue_destroy(fx->rx_watches, l_free);
@@ -930,12 +934,18 @@ error:
 	frame_xchg_done(fx, error);
 }
 
+static void frame_xchg_tx_destroy(void *user_data)
+{
+	struct frame_xchg_data *fx = user_data;
+
+	fx->tx_cmd_id = 0;
+}
+
 static bool frame_xchg_tx_retry(struct wiphy_radio_work_item *item)
 {
 	struct frame_xchg_data *fx = l_container_of(item,
 						struct frame_xchg_data, work);
 	struct l_genl_msg *msg;
-	uint32_t cmd_id;
 	uint32_t duration = fx->resp_timeout;
 
 	/*
@@ -963,8 +973,9 @@ static bool frame_xchg_tx_retry(struct wiphy_radio_work_item *item)
 		l_genl_msg_append_attr(msg, NL80211_ATTR_DURATION, 4,
 					&duration);
 
-	cmd_id = l_genl_family_send(nl80211, msg, frame_xchg_tx_cb, fx, NULL);
-	if (!cmd_id) {
+	fx->tx_cmd_id = l_genl_family_send(nl80211, msg, frame_xchg_tx_cb, fx,
+						frame_xchg_tx_destroy);
+	if (!fx->tx_cmd_id) {
 		l_error("Error sending frame");
 		l_genl_msg_unref(msg);
 		frame_xchg_done(fx, -EIO);
-- 
2.25.1

[PATCH 3/6] wscutil: Fix subcategory string lookup

[Andrew Zaborowski]

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

The device type category array is indexed by the category ID so if we're
skipping i == 0 in the iteration, we should also skip the 0'th element
in device_type_categories.
---
 src/wscutil.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wscutil.c b/src/wscutil.c
index f5ff5d75..81b175d8 100644
--- a/src/wscutil.c
+++ b/src/wscutil.c
@@ -3031,7 +3031,7 @@ bool wsc_device_type_to_dbus_str(const struct wsc_primary_device_type *val,
 bool wsc_device_type_from_subcategory_str(struct wsc_primary_device_type *out,
 						const char *subcategory_str)
 {
-	struct device_type_category_info *cat = device_type_categories;
+	struct device_type_category_info *cat = device_type_categories + 1;
 	unsigned int i;
 
 	for (i = 1; i < L_ARRAY_SIZE(device_type_categories); i++, cat++) {
-- 
2.25.1

[PATCH 4/6] scan: Drop unused frequency list parsing

[Andrew Zaborowski]

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

---
I have sent this once or twice before but this code is still
unused.

 src/scan.c | 53 -----------------------------------------------------
 1 file changed, 53 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index b4135d7c..fe780f51 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -115,7 +115,6 @@ struct scan_context {
 struct scan_results {
 	struct scan_context *sc;
 	struct l_queue *bss_list;
-	struct scan_freq_set *freqs;
 	uint64_t time_stamp;
 	struct scan_request *sr;
 };
@@ -1200,28 +1199,6 @@ fail:
 	return NULL;
 }
 
-static struct scan_freq_set *scan_parse_attr_scan_frequencies(
-						struct l_genl_attr *attr)
-{
-	uint16_t type, len;
-	const void *data;
-	struct scan_freq_set *set;
-
-	set = scan_freq_set_new();
-
-	while (l_genl_attr_next(attr, &type, &len, &data)) {
-		uint32_t freq;
-
-		if (len != sizeof(uint32_t))
-			continue;
-
-		freq = *((uint32_t *) data);
-		scan_freq_set_add(set, freq);
-	}
-
-	return set;
-}
-
 static struct scan_bss *scan_parse_result(struct l_genl_msg *msg,
 						uint64_t *out_wdev)
 {
@@ -1541,9 +1518,6 @@ static void get_scan_done(void *user)
 		l_queue_destroy(results->bss_list,
 				(l_queue_destroy_func_t) scan_bss_free);
 
-	if (results->freqs)
-		scan_freq_set_free(results->freqs);
-
 	l_free(results);
 }
 
@@ -1563,31 +1537,6 @@ static bool scan_parse_flush_flag_from_msg(struct l_genl_msg *msg)
 	return false;
 }
 
-static void scan_parse_new_scan_results(struct l_genl_msg *msg,
-					struct scan_results *results)
-{
-	struct l_genl_attr attr, nested;
-	uint16_t type, len;
-	const void *data;
-
-	if (!l_genl_attr_init(&attr, msg))
-		return;
-
-	while (l_genl_attr_next(&attr, &type, &len, &data)) {
-		switch (type) {
-		case NL80211_ATTR_SCAN_FREQUENCIES:
-			if (!l_genl_attr_recurse(&attr, &nested)) {
-				l_warn("Failed to parse ATTR_SCAN_FREQUENCIES");
-				break;
-			}
-
-			results->freqs =
-				scan_parse_attr_scan_frequencies(&nested);
-			break;
-		}
-	}
-}
-
 static void scan_notify(struct l_genl_msg *msg, void *user_data)
 {
 	struct l_genl_attr attr;
@@ -1706,8 +1655,6 @@ static void scan_notify(struct l_genl_msg *msg, void *user_data)
 		results->sr = sr;
 		results->bss_list = l_queue_new();
 
-		scan_parse_new_scan_results(msg, results);
-
 		scan_msg = l_genl_msg_new_sized(NL80211_CMD_GET_SCAN, 8);
 		l_genl_msg_append_attr(scan_msg, NL80211_ATTR_WDEV, 8,
 					&sc->wdev_id);
-- 
2.25.1

[PATCH 5/6] p2p: Fix adding peers from Probe Request info

[Andrew Zaborowski]

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

Make sure dev->peer_list is non-NULL before using l_queue_push_tail()
same as we do when the peer info comes from a Probe Response (active
scan in Find Phase).  Otherwise peers discovered through Probe Requests
before any Probe Responses are received will be lost.
---
 src/p2p.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/p2p.c b/src/p2p.c
index 6182cda9..662fdb61 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -3747,6 +3747,9 @@ static void p2p_device_probe_cb(const struct mmpdu_header *mpdu,
 	 */
 	peer->device_addr = bss->addr;
 
+	if (!dev->peer_list)
+		dev->peer_list = l_queue_new();
+
 	if (!p2p_device_peer_add(dev, peer))
 		p2p_peer_free(peer);
 
-- 
2.25.1

[PATCH 6/6] p2p: Respond to Probe Requests when in discovery

[Andrew Zaborowski]

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

Since our DBus API and our use cases only support initiating connections
and not accepting incoming connections we don't really need to reply to
Probe Requests on the P2P-Device interface.  Start doing it firstly so
that we can test the scenario where we get discovered and pre-authorized
to connect in an autotest (wpa_supplicant doesn't seem to have a way to
authorize everyone, which is probably why most Wi-Fi Display dongles
don't do it and instead reply with "Fail: Information not available" and
then restart connection from their side) and secondly because the spec
wants us to do it.
---
 src/p2p.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index 662fdb61..417ce3d6 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -3554,6 +3554,20 @@ static void p2p_device_send_probe_resp(struct p2p_device *dev,
 	resp_info.capability = dev->capability;
 	resp_info.device_info = dev->device_info;
 
+	/*
+	 * Note the SSID and resp_info.group_clients are not updated with
+	 * our group information because we generally won't be in the
+	 * Listen State on the P2P Device when running a group.  Otherwise
+	 * we'd be sending two Probe Responses, one from the P2P Interface
+	 * and another from the P2P Device.  According to this part in
+	 * Wi-Fi P2P Technical Specification v1.7 section 3.2.2 it seems
+	 * only the P2P Interface is supposed to be sending Probe
+	 * Responses in that situation:
+	 * "In all Probe Responses that it sends, a P2P Group Owner shall
+	 * set the SSID to the SSID of the group, and shall set the SA and
+	 * BSSID to its P2P Interface Address."
+	 */
+
 	p2p_ie = p2p_build_probe_resp(&resp_info, &p2p_ie_size);
 	if (!p2p_ie) {
 		l_error("Can't build our Probe Response P2P IE");
@@ -3686,23 +3700,10 @@ static void p2p_device_probe_cb(const struct mmpdu_header *mpdu,
 	}
 
 	/*
-	 * We don't currently have a use case for replying to Probe Requests
-	 * except when waiting for a GO Negotiation Request from our target
-	 * peer.  Some of those peers (seemingly running ancient and/or
-	 * hw-manufacturer-provided versions of wpa_s) will only send us GO
-	 * Negotiation Requests each time they receive our Probe Response
-	 * frame, even if that frame's body is unparsable.
+	 * TODO: use ap.c code to check if we match the SSID, BSSID, DSSS
+	 * Channel etc. in the Probe Request, and to build the Response body.
 	 */
-	if (from_conn_peer) {
-		/*
-		 * TODO: use ap.c code to check if we match the SSID, BSSID,
-		 * DSSS Channel etc. in the Probe Request, and to build the
-		 * Response body.
-		 */
-		p2p_device_send_probe_resp(dev, mpdu->address_2,
-						from_conn_peer);
-		goto p2p_free;
-	}
+	p2p_device_send_probe_resp(dev, mpdu->address_2, from_conn_peer);
 
 	/*
 	 * The peer's listen frequency may be different from ours.
-- 
2.25.1

[PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[Andrew Zaborowski]

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

Add the "wpa_supplicant_radios=<radio_name_list>" option in hw.conf's
SETUP section that only reserves radios for the autotest to be able to
start wpa_supplicant on them, i.e. prevents iwd or hostapd from being
started on them.
---
 tools/test-runner | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/test-runner b/tools/test-runner
index 6443af15..f2b75bc0 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -270,10 +270,8 @@ class Radio:
 
 		intf_id += 1
 
-		self.interface = Interface(ifname, hapd.config)
-		# IWD does not use interfaces in test-runner so any created
-		# interface is assumed to be used by hostapd.
-		self.use = 'hostapd'
+		self.interface = Interface(ifname, hapd.config if hapd else None)
+		self.use = 'hostapd' if hapd else 'other'
 
 		Process(['iw', 'phy', self.name, 'interface', 'add', ifname,
 				'type', 'managed'], True)
@@ -466,6 +464,7 @@ class TestContext:
 		self.args = args
 		self.hw_config = None
 		self.hostapd = None
+		self.wpas_interfaces = None
 		self.cur_radio_id = 0
 		self.cur_iface_id = 0
 		self.radios = []
@@ -609,6 +608,15 @@ class TestContext:
 
 		self.hostapd = Hostapd(self, hapd_radios, hapd_configs, radius_config)
 
+	def start_wpas_interfaces(self):
+		if 'wpa_supplicant_radios' not in self.hw_config['SETUP']:
+			return
+
+		setting = self.hw_config['SETUP']['wpa_supplicant_radios']
+		wpas_radio_names = [name.strip() for name in setting.split(',')]
+		wpas_radios = [rad for rad in self.radios if rad.name in wpas_radio_names]
+		self.wpas_interfaces = [rad.create_interface(None) for rad in wpas_radios]
+
 	def start_ofono(self):
 		sim_keys = self.hw_config['SETUP'].get('sim_keys', None)
 		if not sim_keys:
@@ -676,6 +684,7 @@ class TestContext:
 	def stop_test_processes(self):
 		self.radios = []
 		self.hostapd = None
+		self.wpas_interfaces = None
 		self.iwd_extra_options = None
 
 		for p in [p for p in self.processes if p.multi_test is False]:
@@ -870,6 +879,7 @@ def pre_test(ctx, test):
 	ctx.start_dbus_monitor()
 	ctx.start_radios()
 	ctx.start_hostapd()
+	ctx.start_wpas_interfaces()
 	ctx.start_ofono()
 
 	if ctx.hw_config.has_option('SETUP', 'start_iwd'):
-- 
2.25.1

[PATCH 2/8] test-runner: Enable --p2p when creating interfaces

[Andrew Zaborowski]

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

Enable p2p for the hwsim radios so that we can test P2P.  This can be
made optional for the tests but it seems safe to enable always.
---
 tools/test-runner | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/test-runner b/tools/test-runner
index f2b75bc0..00a0978a 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -303,7 +303,7 @@ class VirtualRadio(Radio):
 		self.disable_cipher = None
 		self.disable_iftype = None
 
-		args = ['hwsim', '--create', '--name', self.name, '--nointerface']
+		args = ['hwsim', '--create', '--name', self.name, '--nointerface', '--p2p']
 
 		if config:
 			self.disable_iftype = config.get('iftype_disable', False)
-- 
2.25.1

[PATCH 3/8] test-runner: Add flags for DHCP and TLS verbose output

[Andrew Zaborowski]

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

Add "iwd-tls" and "iwd-dhcp" flags for -v.  These should automatically
enable "-v iwd" output because of the substring matching.
---
 tools/test-runner | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/test-runner b/tools/test-runner
index 00a0978a..dc6b1f7a 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -565,6 +565,12 @@ class TestContext:
 		env['CONFIGURATION_DIRECTORY'] = config_dir
 		env['STATE_DIRECTORY'] = '/var/lib/iwd'
 
+		if self.is_verbose('iwd-dhcp'):
+			env['IWD_DHCP_DEBUG'] = '1'
+
+		if self.is_verbose('iwd-tls'):
+			env['IWD_TLS_DEBUG'] = '1'
+
 		pid = self.start_process(args, env=env)
 		return pid
 
-- 
2.25.1

[PATCH 4/8] test-runner: Make hwsim medium optional

[Andrew Zaborowski]

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

Allow the "hwsim_medium=no" setting in hw.conf's SETUP section to
disable starting hwsim.  It looks like the packets going through
userspace add enough latency that active scans don't work, probe
responses don't arrive within the "dwell time" or probe requests are not
ACKed on time.  I've tried modifying tools/hwsim.c to respond with the
HWSIM_CMD_TX_INFO_FRAME cmd as the first thing after receiving a
HWSIM_CMD_FRAME and even skipping the queue in ell/genl.c by writing the
command synchronously, but neither helped enough to make the scans work.
This does not rule out that hwsim or the way our scans are done can be
fixed and that would obviously be better than what I did in this patch.
---
 tools/test-runner | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/test-runner b/tools/test-runner
index dc6b1f7a..ea0f7b79 100755
--- a/tools/test-runner
+++ b/tools/test-runner
@@ -516,8 +516,9 @@ class TestContext:
 			self.radios.append(VirtualRadio(name, rad_config))
 			self.cur_radio_id += 1
 
-		# register hwsim as medium
-		self.start_process(['hwsim'])
+		if self.hw_config['SETUP'].get('hwsim_medium', 'yes') in ['yes', '1', 'true']:
+			# register hwsim as medium
+			self.start_process(['hwsim'])
 
 	def discover_radios(self):
 		phys = []
-- 
2.25.1

[PATCH 5/8] autotests: Basic P2P python API

[Andrew Zaborowski]

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

Add a basic wrapper for the P2P-related DBus interfaces similar to the
existing classes in iwd.py.  WFD is not included.
---
 autotests/util/iwd.py | 121 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 3 deletions(-)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index ef843a95..7e3676d9 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -35,6 +35,10 @@ IWD_SIGNAL_AGENT_INTERFACE =    'net.connman.iwd.SignalLevelAgent'
 IWD_AP_INTERFACE =              'net.connman.iwd.AccessPoint'
 IWD_ADHOC_INTERFACE =           'net.connman.iwd.AdHoc'
 IWD_STATION_INTERFACE =         'net.connman.iwd.Station'
+IWD_P2P_INTERFACE =             'net.connman.iwd.p2p.Device'
+IWD_P2P_PEER_INTERFACE =        'net.connman.iwd.p2p.Peer'
+IWD_P2P_SERVICE_MANAGER_INTERFACE = 'net.connman.iwd.p2p.ServiceManager'
+IWD_P2P_WFD_INTERFACE =         'net.connman.iwd.p2p.Display'
 
 IWD_AGENT_MANAGER_PATH =        '/net/connman/iwd'
 IWD_TOP_LEVEL_PATH =            '/'
@@ -767,9 +771,107 @@ class PSKAgent(dbus.service.Object):
         return passwd
 
 
+class P2PDevice(IWDDBusAbstract):
+    _iface_name = IWD_P2P_INTERFACE
+
+    def __init__(self, *args, **kwargs):
+        self._discovery_request = False
+        self._peer_dict = {}
+        IWDDBusAbstract.__init__(self, *args, **kwargs)
+
+    @property
+    def name(self):
+        return str(self._properties['Name'])
+
+    @name.setter
+    def name(self, name):
+        self._prop_proxy.Set(self._iface_name, 'Name', name)
+
+    @property
+    def enabled(self):
+        return bool(self._properties['Enabled'])
+
+    @enabled.setter
+    def enabled(self, enabled):
+        self._prop_proxy.Set(self._iface_name, 'Enabled', enabled)
+
+    @property
+    def discovery_request(self):
+        return self._discovery_request
+
+    @discovery_request.setter
+    def discovery_request(self, req):
+        if self._discovery_request == bool(req):
+            return
+
+        if bool(req):
+            self._iface.RequestDiscovery()
+        else:
+            self._iface.ReleaseDiscovery()
+
+        self._discovery_request = bool(req)
+
+    def get_peers(self):
+        old_dict = self._peer_dict
+        self._peer_dict = {}
+
+        for path, rssi in self._iface.GetPeers():
+            self._peer_dict[path] = old_dict[path] if path in old_dict else P2PPeer(path)
+            self._peer_dict[path].rssi = rssi
+
+        return self._peer_dict
+
+
+class P2PPeer(IWDDBusAbstract):
+    _iface_name = IWD_P2P_PEER_INTERFACE
+
+    @property
+    def name(self):
+        return str(self._properties['Name'])
+
+    @property
+    def category(self):
+        return str(self._properties['DeviceCategory'])
+
+    @property
+    def subcategory(self):
+        return str(self._properties['DeviceSubcategory'])
+
+    @property
+    def connected(self):
+        return bool(self._properties['Connected'])
+
+    @property
+    def connected_interface(self):
+        return str(self._properties['ConnectedInterface'])
+
+    @property
+    def connected_ip(self):
+        return str(self._properties['ConnectedIP'])
+
+    def connect(self, wait=True, pin=None):
+        if pin is None:
+            self._iface.PushButton(dbus_interface=IWD_WSC_INTERFACE,
+                            reply_handler=self._success,
+                            error_handler=self._failure)
+        else:
+            self._iface.StartPin(pin,
+                            dbus_interface=IWD_WSC_INTERFACE,
+                            reply_handler=self._success,
+                            error_handler=self._failure)
+
+        if wait:
+            self._wait_for_async_op()
+            return (self.connected_interface, self.connected_ip)
+
+    def disconnect(self):
+        self._iface.Disconnect()
+
+
 class DeviceList(collections.Mapping):
     def __init__(self, iwd):
         self._dict = {}
+        self._p2p_dict = {}
 
         iwd._object_manager.connect_to_signal("InterfacesAdded",
                                                self._interfaces_added_handler)
@@ -782,6 +884,8 @@ class DeviceList(collections.Mapping):
             for interface in objects[path]:
                 if interface == IWD_DEVICE_INTERFACE:
                     self._dict[path] = Device(path, objects[path][interface])
+                elif interface == IWD_P2P_INTERFACE:
+                    self._p2p_dict[path] = P2PDevice(path, objects[path][interface])
 
     def __getitem__(self, key):
         return self._dict.__getitem__(key)
@@ -798,10 +902,18 @@ class DeviceList(collections.Mapping):
     def _interfaces_added_handler(self, path, interfaces):
         if IWD_DEVICE_INTERFACE in interfaces:
             self._dict[path] = Device(path, interfaces[IWD_DEVICE_INTERFACE])
+        elif IWD_P2P_INTERFACE in interfaces:
+            self._p2p_dict[path] = P2PDevice(path, interfaces[IWD_P2P_INTERFACE])
 
     def _interfaces_removed_handler(self, path, interfaces):
         if IWD_DEVICE_INTERFACE in interfaces:
             del self._dict[path]
+        elif IWD_P2P_INTERFACE in interfaces:
+            del self._p2p_dict[path]
+
+    @property
+    def p2p_dict(self):
+        return self._p2p_dict
 
 
 class IWD(AsyncOpAbstract):
@@ -943,9 +1055,9 @@ class IWD(AsyncOpAbstract):
     def remove_from_storage(file_name):
         os.system('rm -rf ' + IWD_STORAGE_DIR + '/\'' + file_name + '\'')
 
-    def list_devices(self, wait_to_appear = 0, max_wait = 50):
+    def list_devices(self, wait_to_appear = 0, max_wait = 50, p2p = False):
         if not wait_to_appear:
-            return list(self._devices.values())
+            return list(self._devices.values() if not p2p else self._devices.p2p_dict.values())
 
         self._wait_timed_out = False
         def wait_timeout_cb():
@@ -963,7 +1075,10 @@ class IWD(AsyncOpAbstract):
             if not self._wait_timed_out:
                 GLib.source_remove(timeout)
 
-        return list(self._devices.values())
+        return list(self._devices.values() if not p2p else self._devices.p2p_dict.values())
+
+    def list_p2p_devices(self, *args, **kwargs):
+        return self.list_devices(*args, **kwargs, p2p=True)
 
     def list_known_networks(self):
         '''Returns the list of KnownNetwork objects.'''
-- 
2.25.1

[PATCH 6/8] autotests: Add basic wpa_supplicant P2P python wrapper

[Andrew Zaborowski]

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

Add the new wpas.Wpas class roughly based on hostapd.HostapdCLI but only
adding methods for the P2P-related stuff.

Adding "wpa_supplicant" to will enable output from the wpa_supplicant
process to be printed and "wpa_supplicant-dbg" will make it more verbose
("wpa_supplicant" is not needed because it seems to be automatically
enabled because of the glob matching in ctx.is_verbose)
---
 autotests/util/wpas.py | 222 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 222 insertions(+)
 create mode 100644 autotests/util/wpas.py

diff --git a/autotests/util/wpas.py b/autotests/util/wpas.py
new file mode 100644
index 00000000..e871b1b1
--- /dev/null
+++ b/autotests/util/wpas.py
@@ -0,0 +1,222 @@
+#!/usr/bin/python3
+import os
+import socket
+from gi.repository import GLib
+from config import ctx
+
+ctrl_count = 0
+mainloop = GLib.MainLoop()
+
+class Wpas:
+    def _start_wpas(self, config_path, socket_path, main_interface, ifname=None):
+        global ctrl_count
+
+        self.interface = main_interface
+        self.ifname = main_interface.name if ifname is None else ifname
+        self.socket_path = socket_path
+        self.io_watch = None
+
+        cmd = ['wpa_supplicant', '-i', self.interface.name, '-c', config_path]
+        if ctx.is_verbose('wpa_supplicant-dbg'):
+            cmd += ['-d']
+
+        self.wpa_supplicant = ctx.start_process(cmd)
+
+        self.local_ctrl = '/tmp/wpas_' + str(os.getpid()) + '_' + str(ctrl_count)
+        ctrl_count = ctrl_count + 1
+        self.ctrl_sock = socket.socket(socket.AF_UNIX, socket.SOCK_DGRAM)
+        self.ctrl_sock.bind(self.local_ctrl)
+
+        self.remote_ctrl = self.socket_path + '/' + self.ifname
+        self.wpa_supplicant.wait_for_socket(self.remote_ctrl, 2)
+        self.ctrl_sock.connect(self.remote_ctrl)
+        self.io_watch = GLib.io_add_watch(self.ctrl_sock, GLib.IO_IN, self._handle_data_in)
+
+        self.p2p_peers = {}
+        self.p2p_go_neg_requests = {}
+        self.p2p_clients = {}
+        self.p2p_group = None
+
+        self._rx_data = []
+        self._ctrl_request('ATTACH')
+        self.wait_for_event('OK')
+
+    def __init__(self, *args, **kwargs):
+        self._start_wpas(*args, **kwargs)
+
+    def wait_for_event(self, event, timeout=10):
+        global mainloop
+        self._wait_timed_out = False
+
+        def wait_timeout_cb():
+            self._wait_timed_out = True
+            return False
+
+        timeout = GLib.timeout_add_seconds(timeout, wait_timeout_cb)
+        context = mainloop.get_context()
+
+        while True:
+            context.iteration(may_block=True)
+
+            if event in self._rx_data:
+                GLib.source_remove(timeout)
+                return self._rx_data
+
+            if self._wait_timed_out:
+                raise TimeoutError('waiting for wpas event timed out')
+
+    def _event_parse(self, line):
+        # Unescape event parameter values in '', other escaping rules not implemented
+        key = None
+        value = ''
+        count = 0
+        quoted = False
+        event = {}
+
+        def handle_eow():
+            nonlocal key, value, count, event
+            if count == 0:
+                if key is not None or not value:
+                    raise Exception('Bad event name')
+                key = 'event'
+            elif key is None:
+                if not value:
+                    return
+                key = 'arg' + str(count)
+            event[key] = value
+            key = None
+            value = ''
+            count += 1
+
+        for ch in line:
+            if ch == '\'':
+                quoted = not quoted
+            elif quoted:
+                value += ch
+            elif ch == '=' and key is None:
+                key = value
+                value = ''
+            elif ch in ' \n':
+                handle_eow()
+            else:
+                value += ch
+        handle_eow()
+        return event
+
+    def _handle_data_in(self, sock, *args):
+        newdata = sock.recv(4096)
+        if len(newdata) == 0:
+            raise Exception('Wpa_s control socket error')
+
+        decoded = newdata.decode('utf-8')
+        if len(decoded) >= 3 and decoded[0] == '<' and decoded[2] == '>':
+            decoded = decoded[3:]
+        while len(decoded) and decoded[-1] == '\n':
+            decoded = decoded[:-1]
+
+        self._rx_data.append(decoded)
+
+        event = self._event_parse(decoded)
+        if event['event'] == 'P2P-DEVICE-FOUND':
+            event.pop('event')
+            event.pop('arg1')
+            self.p2p_peers[event['p2p_dev_addr']] = event
+        elif event['event'] == 'P2P-DEVICE-LOST':
+            del self.p2p_peers[event['p2p_dev_addr']]
+        elif event['event'] == 'P2P-GO-NEG-REQUEST':
+            event.pop('event')
+            event['p2p_dev_addr'] = event.pop('arg1')
+            self.p2p_go_neg_requests[event['p2p_dev_addr']] = event
+        elif event['event'] == 'P2P-GO-NEG-SUCCESS':
+            event.pop('event')
+            addr = event.pop('peer_dev')
+            event['success'] = True
+            event['p2p_dev_addr'] = addr
+
+            if addr in self.p2p_go_neg_requests:
+                self.p2p_go_neg_requests[addr].update(event)
+            else:
+                self.p2p_go_neg_requests[addr] = event
+        elif event['event'] == 'AP-STA-CONNECTED':
+            event.pop('event')
+            addr = event.pop('arg1')
+            self.p2p_clients[addr] = event
+        elif event['event'] == 'AP-STA-DISCONNECTED':
+            addr = event.pop('arg1')
+            del self.p2p_clients[addr]
+        elif event['event'] == 'P2P-GROUP-STARTED':
+            event.pop('event')
+            event['ifname'] = event.pop('arg1')
+            self.p2p_group = event
+        elif event['event'] == 'P2P-GROUP-REMOVED':
+            self.p2p_group = None
+
+        return True
+
+    def _ctrl_request(self, command, timeout=10):
+        if type(command) is str:
+            command = str.encode(command)
+
+        self.ctrl_sock.send(bytes(command))
+
+    # Normal find phase with listen and active scan states
+    def p2p_find(self):
+        self._rx_data = []
+        self._ctrl_request('P2P_SET disc_int 2 3 300')
+        self.wait_for_event('OK')
+        self._rx_data = []
+        self._ctrl_request('P2P_FIND type=social')
+        self.wait_for_event('OK')
+
+    # Like p2p_find but uses only listen states
+    def p2p_listen(self):
+        self._rx_data = []
+        self._ctrl_request('P2P_LISTEN')
+        self.wait_for_event('OK')
+
+    # Stop a p2p_find or p2p_listen
+    def p2p_stop_find_listen(self):
+        self._rx_data = []
+        self._ctrl_request('P2P_STOP_FIND')
+        self.wait_for_event('OK')
+
+    def p2p_connect(self, peer, pin=None, go_intent=None):
+        self._rx_data = []
+        self._ctrl_request('P2P_CONNECT ' + peer['p2p_dev_addr'] + ' ' + ('pbc' if pin is None else pin) +
+                        ('' if go_intent is None else 'go_intent=' + str(go_intent)))
+        self.wait_for_event('OK')
+
+    def p2p_accept_go_neg_request(self, request, pin=None, go_intent=None):
+        self._rx_data = []
+        self._ctrl_request('P2P_CONNECT ' + request['p2p_dev_addr'] + ' ' + ('pbc' if pin is None else pin) +
+                        ('' if go_intent is None else 'go_intent=' + str(go_intent)))
+        self.wait_for_event('OK')
+
+    # Pre-accept the next GO Negotiation Request from this peer to avoid the extra Respone + Request frames
+    def p2p_authorize(self, peer, pin=None, go_intent=None):
+        self._rx_data = []
+        self._ctrl_request('P2P_CONNECT ' + peer['p2p_dev_addr'] + ' ' + ('pbc' if pin is None else pin) +
+                        ('' if go_intent is None else 'go_intent=' + str(go_intent)) + ' auth')
+        self.wait_for_event('OK')
+
+    # Probably needed: remove references to self so that the GC can call __del__ automatically
+    def clean_up(self):
+        if self.io_watch is not None:
+            GLib.source_remove(self.io_watch)
+            self.io_watch = None
+        if self.wpa_supplicant is not None:
+            ctx.stop_process(self.wpa_supplicant)
+            self.wpa_supplicant = None
+
+    def _stop_wpas(self):
+        self.clean_up()
+        if self.ctrl_sock:
+            self.ctrl_sock.close()
+            self.ctrl_sock = None
+        if os.path.exists(self.remote_ctrl):
+            os.remove(self.remote_ctrl)
+        if os.path.exists(self.local_ctrl):
+            os.remove(self.local_ctrl)
+
+    def __del__(self):
+        self._stop_wpas()
-- 
2.25.1

[PATCH 7/8] autotests: Move some variables from IWD class to instance

[Andrew Zaborowski]

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

These variables were probably meant as instance members and not class
members.
---
 autotests/util/iwd.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index 7e3676d9..94a118ce 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -231,9 +231,12 @@ class Device(IWDDBusAbstract):
         with its properties and methods
     '''
     _iface_name = IWD_DEVICE_INTERFACE
-    _wps_manager_if = None
-    _station_if = None
-    _station_props = None
+
+    def __init__(self, *args, **kwargs):
+        self._wps_manager_if = None
+        self._station_if = None
+        self._station_props = None
+        IWDDBusAbstract.__init__(self, *args, **kwargs)
 
     @property
     def _wps_manager(self):
-- 
2.25.1

[PATCH 8/8] autotests: Add testP2P

[Andrew Zaborowski]

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

Add two P2P client connection scenarios.
---
 autotests/testP2P/client_test.py | 108 +++++++++++++++++++++++++++++++
 autotests/testP2P/dhcpd.conf     |   4 ++
 autotests/testP2P/hw.conf        |   4 ++
 autotests/testP2P/main.conf      |   6 ++
 autotests/testP2P/rad1-p2p.conf  |  10 +++
 5 files changed, 132 insertions(+)
 create mode 100644 autotests/testP2P/client_test.py
 create mode 100644 autotests/testP2P/dhcpd.conf
 create mode 100644 autotests/testP2P/hw.conf
 create mode 100644 autotests/testP2P/main.conf
 create mode 100644 autotests/testP2P/rad1-p2p.conf

diff --git a/autotests/testP2P/client_test.py b/autotests/testP2P/client_test.py
new file mode 100644
index 00000000..45cde721
--- /dev/null
+++ b/autotests/testP2P/client_test.py
@@ -0,0 +1,108 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+import netifaces
+import os
+
+import iwd
+from iwd import IWD
+import testutil
+from config import ctx
+from wpas import Wpas
+
+class Test(unittest.TestCase):
+    def test_1_client_go_neg_responder(self):
+        self.p2p_client_test(False)
+
+    def test_2_client_go_neg_initiator(self):
+        self.p2p_client_test(True)
+
+    def p2p_client_test(self, preauthorize):
+        wpas_interface = ctx.wpas_interfaces[0]
+        p2p_ifname = 'p2p-dev-' + wpas_interface.name
+        wpas = Wpas('/tmp/rad1-p2p.conf', '/tmp/rad1-p2p-wpas', wpas_interface, p2p_ifname)
+
+        wd = IWD()
+        # Not strictly necessary but prevents the station interface from queuing its scans
+        # in the wiphy radio work queue and delaying P2P scans.
+        wd.list_devices(1)[0].disconnect()
+
+        devices = wd.list_p2p_devices(1)
+        p2p = devices[0]
+        p2p.enabled = True
+        p2p.name = 'testdev1'
+
+        wpas.p2p_find()
+        p2p.discovery_request = True
+        wd.wait(5)
+        wd.wait_for_object_condition(wpas, 'len(obj.p2p_peers) == 1', max_wait=20)
+        p2p.discovery_request = False
+        wpas.p2p_listen()
+
+        peers = p2p.get_peers()
+        self.assertEqual(len(peers), 1)
+        peer = next(iter(peers.values()))
+        self.assertEqual(peer.name, 'testdev2')
+        self.assertEqual(peer.category, 'display')
+        self.assertEqual(peer.subcategory, 'monitor')
+
+        wpas_peer = next(iter(wpas.p2p_peers.values()))
+        self.assertEqual(wpas_peer['name'], 'testdev1')
+        self.assertEqual(wpas_peer['pri_dev_type'], '1-0050F204-6') # 1 == Computer, 6 == Desktop
+        self.assertEqual(wpas_peer['config_methods'], '0x1080')
+
+        if preauthorize:
+            wpas.p2p_authorize(wpas_peer)
+
+        peer.connect(wait=False)
+
+        self.assertEqual(len(wpas.p2p_go_neg_requests), 0)
+        self.assertEqual(len(wpas.p2p_clients), 0)
+        wd.wait_for_object_condition(wpas, 'len(obj.p2p_go_neg_requests) == 1', max_wait=3)
+        request = wpas.p2p_go_neg_requests[wpas_peer['p2p_dev_addr']]
+
+        if not preauthorize:
+            self.assertEqual(request['dev_passwd_id'], '4')
+            self.assertEqual(request['go_intent'], '2') # Hardcoded in src/p2p.c
+
+            wpas.p2p_accept_go_neg_request(request)
+
+        wd.wait_for_object_condition(request, '\'success\' in obj', max_wait=3)
+        self.assertEqual(request['success'], True)
+        self.assertEqual(request['role'], 'GO')
+        self.assertEqual(request['wps_method'], 'PBC')
+        self.assertEqual(request['p2p_dev_addr'], wpas_peer['p2p_dev_addr'])
+
+        wd.wait_for_object_condition(wpas, 'obj.p2p_group is not None', max_wait=3)
+        go_ifname = wpas.p2p_group['ifname']
+        ctx.start_process(['ifconfig', go_ifname, '192.168.1.20', 'netmask', '255.255.255.0'], wait=True)
+        os.system('> /tmp/dhcpd.leases')
+        dhcpd = ctx.start_process(['dhcpd', '-f', '-cf', '/tmp/dhcpd.conf', '-lf', '/tmp/dhcpd.leases', go_ifname])
+
+        wd.wait_for_object_condition(wpas, 'len(obj.p2p_clients) == 1', max_wait=3)
+        client = wpas.p2p_clients[request['peer_iface']]
+        self.assertEqual(client['p2p_dev_addr'], wpas_peer['p2p_dev_addr'])
+
+        wd.wait_for_object_condition(peer, 'obj.connected', max_wait=15)
+        our_ip = netifaces.ifaddresses(peer.connected_interface)[netifaces.AF_INET][0]['addr']
+        self.assertEqual(peer.connected_ip, '192.168.1.20')
+        self.assertEqual(our_ip, '192.168.1.30')
+
+        testutil.test_iface_operstate(peer.connected_interface)
+        testutil.test_ifaces_connected(peer.connected_interface, go_ifname)
+
+        peer.disconnect()
+        wd.wait_for_object_condition(wpas, 'len(obj.p2p_clients) == 0', max_wait=3)
+        self.assertEqual(peer.connected, False)
+
+        p2p.enabled = False
+        ctx.stop_process(dhcpd)
+        wpas.clean_up()
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
diff --git a/autotests/testP2P/dhcpd.conf b/autotests/testP2P/dhcpd.conf
new file mode 100644
index 00000000..a5ef0b4a
--- /dev/null
+++ b/autotests/testP2P/dhcpd.conf
@@ -0,0 +1,4 @@
+subnet 192.168.1.0 netmask 255.255.255.0
+ {
+  range 192.168.1.30;
+ }
diff --git a/autotests/testP2P/hw.conf b/autotests/testP2P/hw.conf
new file mode 100644
index 00000000..05d436a6
--- /dev/null
+++ b/autotests/testP2P/hw.conf
@@ -0,0 +1,4 @@
+[SETUP]
+num_radios=2
+wpa_supplicant_radios=rad1
+hwsim_medium=no
diff --git a/autotests/testP2P/main.conf b/autotests/testP2P/main.conf
new file mode 100644
index 00000000..aeafc2ab
--- /dev/null
+++ b/autotests/testP2P/main.conf
@@ -0,0 +1,6 @@
+[General]
+EnableNetworkConfiguration=true
+
+[P2P]
+# Something different from the default for validation
+DeviceType=desktop
diff --git a/autotests/testP2P/rad1-p2p.conf b/autotests/testP2P/rad1-p2p.conf
new file mode 100644
index 00000000..a34f3d91
--- /dev/null
+++ b/autotests/testP2P/rad1-p2p.conf
@@ -0,0 +1,10 @@
+ctrl_interface=/tmp/rad1-p2p-wpas
+update_config=0
+device_name=testdev2
+device_type=7-0050F204-4 # 7 == Display, 4 == Monitor
+config_methods=virtual_push_button
+p2p_listen_reg_class=81
+p2p_listen_channel=11
+p2p_oper_reg_class=81
+p2p_oper_channel=11
+p2p_go_intent=10
-- 
2.25.1

Re: [PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[James Prestwood]

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

Hi Andrew,

On Tue, 2020-09-29 at 18:37 +0200, Andrew Zaborowski wrote:
> Add the "wpa_supplicant_radios=<radio_name_list>" option in hw.conf's
> SETUP section that only reserves radios for the autotest to be able
> to
> start wpa_supplicant on them, i.e. prevents iwd or hostapd from being
> started on them.

Instead could we just add a new group [WPA_S] or [WPA_SUPPLICANT] and
have radN=<config>. This would be consistent with how hostapd works,
and still allows you to reserve radios for wpa_supplicant. And if we
ever require wpa_s specific options in the future we may want a group
for it.

This would also allow you to hide a lot of those init arguments to your
wpa_s constructor since you would already know the config file,
interface, and control socket.

Thanks,
James

Re: [PATCH 1/6] frame-xchg: Add no-cck-rate flag only for P2P interfaces

[Denis Kenzior]

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

Hi Andrew,

On 9/29/20 11:37 AM, Andrew Zaborowski wrote:
> We want to use this flag only on the interfaces with one of the three
> P2P iftypes so set the flag automatically depending on the iftype from
> the last 'config' notification.
> ---
>   src/frame-xchg.c | 29 ++++++++++++++++++++---------
>   1 file changed, 20 insertions(+), 9 deletions(-)
> 

All applied, thanks.

Regards,
-Denis

Re: [PATCH 2/8] test-runner: Enable --p2p when creating interfaces

[Denis Kenzior]

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

Hi Andrew,

On 9/29/20 11:37 AM, Andrew Zaborowski wrote:
> Enable p2p for the hwsim radios so that we can test P2P.  This can be
> made optional for the tests but it seems safe to enable always.
> ---
>   tools/test-runner | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Patches 2-4 applied, thanks.

Regards,
-Denis

Re: [PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[Andrew Zaborowski]

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

Hi James,

Thanks for reviewing the changes.

On Tue, 29 Sep 2020 at 19:15, James Prestwood <prestwoj@gmail.com> wrote:
> On Tue, 2020-09-29 at 18:37 +0200, Andrew Zaborowski wrote:
> > Add the "wpa_supplicant_radios=<radio_name_list>" option in hw.conf's
> > SETUP section that only reserves radios for the autotest to be able
> > to
> > start wpa_supplicant on them, i.e. prevents iwd or hostapd from being
> > started on them.
>
> Instead could we just add a new group [WPA_S] or [WPA_SUPPLICANT] and
> have radN=<config>. This would be consistent with how hostapd works,
> and still allows you to reserve radios for wpa_supplicant. And if we
> ever require wpa_s specific options in the future we may want a group
> for it.

That's doable, but with wpa_supplicant we have to specify the control
socket path inside the config file, so the setting would have to be
something like "radN=<config>,<ctrl_socket>".  I think I'd rather
avoid creating separate Wpas and WpasCLI classes, we could have one
class handle both things whether it's created by test-runner or by the
individual test, are you ok with that?

I also thought of using something like "reserved_radios=<radN>, ..."
so that individual tests in one directory can decide which config file
and even which daemon to run in each pass.  They could also generate
the config files as needed.

Best regards

Re: [PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[James Prestwood]

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

On Wed, 2020-09-30 at 01:29 +0200, Andrew Zaborowski wrote:
> Hi James,
> 
> Thanks for reviewing the changes.
> 
> On Tue, 29 Sep 2020 at 19:15, James Prestwood <prestwoj@gmail.com>
> wrote:
> > On Tue, 2020-09-29 at 18:37 +0200, Andrew Zaborowski wrote:
> > > Add the "wpa_supplicant_radios=<radio_name_list>" option in
> > > hw.conf's
> > > SETUP section that only reserves radios for the autotest to be
> > > able
> > > to
> > > start wpa_supplicant on them, i.e. prevents iwd or hostapd from
> > > being
> > > started on them.
> > 
> > Instead could we just add a new group [WPA_S] or [WPA_SUPPLICANT]
> > and
> > have radN=<config>. This would be consistent with how hostapd
> > works,
> > and still allows you to reserve radios for wpa_supplicant. And if
> > we
> > ever require wpa_s specific options in the future we may want a
> > group
> > for it.
> 
> That's doable, but with wpa_supplicant we have to specify the control
> socket path inside the config file, so the setting would have to be
> something like "radN=<config>,<ctrl_socket>".  I think I'd rather
> avoid creating separate Wpas and WpasCLI classes, we could have one
> class handle both things whether it's created by test-runner or by
> the
> individual test, are you ok with that?

After looking at the wpa_s help yeah your right, when providing a
config file you cannot provide a control socket via the command line.
But I still dont see how doing radN=<config>,<ctrl_socket> would aid in
that since <ctrl_socket> would need to be in <config> anyways. Or are
you saying having it in both places and <ctrl_socket> is merely to tell
test-runner what value is already inside the config?

We could append the control socket to the config file like I did with
hostapd (see _rewrite_config), although I'll admit this is somewhat of
a hack. But this does allow you to generate a control socket on the fly
without the test or config file knowing about it.

> 
> I also thought of using something like "reserved_radios=<radN>, ..."
> so that individual tests in one directory can decide which config
> file
> and even which daemon to run in each pass.  They could also generate
> the config files as needed.
> 
> Best regards

Re: [PATCH 6/8] autotests: Add basic wpa_supplicant P2P python wrapper

[James Prestwood]

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

Hi Andrew,

On Tue, 2020-09-29 at 18:37 +0200, Andrew Zaborowski wrote:
> Add the new wpas.Wpas class roughly based on hostapd.HostapdCLI but
> only
> adding methods for the P2P-related stuff.
> 
> Adding "wpa_supplicant" to will enable output from the wpa_supplicant
> process to be printed and "wpa_supplicant-dbg" will make it more
> verbose
> ("wpa_supplicant" is not needed because it seems to be automatically
> enabled because of the glob matching in ctx.is_verbose)
> ---
>  autotests/util/wpas.py | 222
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 222 insertions(+)
>  create mode 100644 autotests/util/wpas.py
> 
> diff --git a/autotests/util/wpas.py b/autotests/util/wpas.py
> new file mode 100644
> index 00000000..e871b1b1
> --- /dev/null
> +++ b/autotests/util/wpas.py
> @@ -0,0 +1,222 @@
> +#!/usr/bin/python3
> +import os
> +import socket
> +from gi.repository import GLib
> +from config import ctx
> +
> +ctrl_count = 0
> +mainloop = GLib.MainLoop()

I think this is gonna end causing some weird behavior. Using
ctx.mainloop should work better. I had all kinds of strange issues due
to test-runner and iwd.py both creating a mainloop.

> +
> +class Wpas:
> +    def _start_wpas(self, config_path, socket_path, main_interface,
> ifname=None):
> +        global ctrl_count
> +
> +        self.interface = main_interface
> +        self.ifname = main_interface.name if ifname is None else
> ifname
> +        self.socket_path = socket_path
> +        self.io_watch = None
> +
> +        cmd = ['wpa_supplicant', '-i', self.interface.name, '-c',
> config_path]
> +        if ctx.is_verbose('wpa_supplicant-dbg'):
> +            cmd += ['-d']
> +
> +        self.wpa_supplicant = ctx.start_process(cmd)
> +
> +        self.local_ctrl = '/tmp/wpas_' + str(os.getpid()) + '_' +
> str(ctrl_count)
> +        ctrl_count = ctrl_count + 1
> +        self.ctrl_sock = socket.socket(socket.AF_UNIX,
> socket.SOCK_DGRAM)
> +        self.ctrl_sock.bind(self.local_ctrl)
> +
> +        self.remote_ctrl = self.socket_path + '/' + self.ifname
> +        self.wpa_supplicant.wait_for_socket(self.remote_ctrl, 2)
> +        self.ctrl_sock.connect(self.remote_ctrl)
> +        self.io_watch = GLib.io_add_watch(self.ctrl_sock,
> GLib.IO_IN, self._handle_data_in)
> +
> +        self.p2p_peers = {}
> +        self.p2p_go_neg_requests = {}
> +        self.p2p_clients = {}
> +        self.p2p_group = None
> +
> +        self._rx_data = []
> +        self._ctrl_request('ATTACH')
> +        self.wait_for_event('OK')
> +
> +    def __init__(self, *args, **kwargs):
> +        self._start_wpas(*args, **kwargs)
> +
> +    def wait_for_event(self, event, timeout=10):
> +        global mainloop
> +        self._wait_timed_out = False
> +
> +        def wait_timeout_cb():
> +            self._wait_timed_out = True
> +            return False
> +
> +        timeout = GLib.timeout_add_seconds(timeout, wait_timeout_cb)
> +        context = mainloop.get_context()
> +
> +        while True:
> +            context.iteration(may_block=True)
> +
> +            if event in self._rx_data:
> +                GLib.source_remove(timeout)
> +                return self._rx_data
> +
> +            if self._wait_timed_out:
> +                raise TimeoutError('waiting for wpas event timed
> out')
> +
> +    def _event_parse(self, line):
> +        # Unescape event parameter values in '', other escaping
> rules not implemented
> +        key = None
> +        value = ''
> +        count = 0
> +        quoted = False
> +        event = {}
> +
> +        def handle_eow():
> +            nonlocal key, value, count, event
> +            if count == 0:
> +                if key is not None or not value:
> +                    raise Exception('Bad event name')
> +                key = 'event'
> +            elif key is None:
> +                if not value:
> +                    return
> +                key = 'arg' + str(count)
> +            event[key] = value
> +            key = None
> +            value = ''
> +            count += 1
> +
> +        for ch in line:
> +            if ch == '\'':
> +                quoted = not quoted
> +            elif quoted:
> +                value += ch
> +            elif ch == '=' and key is None:
> +                key = value
> +                value = ''
> +            elif ch in ' \n':
> +                handle_eow()
> +            else:
> +                value += ch
> +        handle_eow()
> +        return event
> +
> +    def _handle_data_in(self, sock, *args):
> +        newdata = sock.recv(4096)
> +        if len(newdata) == 0:
> +            raise Exception('Wpa_s control socket error')
> +
> +        decoded = newdata.decode('utf-8')
> +        if len(decoded) >= 3 and decoded[0] == '<' and decoded[2] ==
> '>':
> +            decoded = decoded[3:]
> +        while len(decoded) and decoded[-1] == '\n':
> +            decoded = decoded[:-1]
> +
> +        self._rx_data.append(decoded)
> +
> +        event = self._event_parse(decoded)
> +        if event['event'] == 'P2P-DEVICE-FOUND':
> +            event.pop('event')
> +            event.pop('arg1')
> +            self.p2p_peers[event['p2p_dev_addr']] = event
> +        elif event['event'] == 'P2P-DEVICE-LOST':
> +            del self.p2p_peers[event['p2p_dev_addr']]
> +        elif event['event'] == 'P2P-GO-NEG-REQUEST':
> +            event.pop('event')
> +            event['p2p_dev_addr'] = event.pop('arg1')
> +            self.p2p_go_neg_requests[event['p2p_dev_addr']] = event
> +        elif event['event'] == 'P2P-GO-NEG-SUCCESS':
> +            event.pop('event')
> +            addr = event.pop('peer_dev')
> +            event['success'] = True
> +            event['p2p_dev_addr'] = addr
> +
> +            if addr in self.p2p_go_neg_requests:
> +                self.p2p_go_neg_requests[addr].update(event)
> +            else:
> +                self.p2p_go_neg_requests[addr] = event
> +        elif event['event'] == 'AP-STA-CONNECTED':
> +            event.pop('event')
> +            addr = event.pop('arg1')
> +            self.p2p_clients[addr] = event
> +        elif event['event'] == 'AP-STA-DISCONNECTED':
> +            addr = event.pop('arg1')
> +            del self.p2p_clients[addr]
> +        elif event['event'] == 'P2P-GROUP-STARTED':
> +            event.pop('event')
> +            event['ifname'] = event.pop('arg1')
> +            self.p2p_group = event
> +        elif event['event'] == 'P2P-GROUP-REMOVED':
> +            self.p2p_group = None
> +
> +        return True
> +
> +    def _ctrl_request(self, command, timeout=10):
> +        if type(command) is str:
> +            command = str.encode(command)
> +
> +        self.ctrl_sock.send(bytes(command))
> +
> +    # Normal find phase with listen and active scan states
> +    def p2p_find(self):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_SET disc_int 2 3 300')
> +        self.wait_for_event('OK')
> +        self._rx_data = []
> +        self._ctrl_request('P2P_FIND type=social')
> +        self.wait_for_event('OK')
> +
> +    # Like p2p_find but uses only listen states
> +    def p2p_listen(self):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_LISTEN')
> +        self.wait_for_event('OK')
> +
> +    # Stop a p2p_find or p2p_listen
> +    def p2p_stop_find_listen(self):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_STOP_FIND')
> +        self.wait_for_event('OK')
> +
> +    def p2p_connect(self, peer, pin=None, go_intent=None):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_CONNECT ' + peer['p2p_dev_addr'] + '
> ' + ('pbc' if pin is None else pin) +
> +                        ('' if go_intent is None else 'go_intent=' +
> str(go_intent)))
> +        self.wait_for_event('OK')
> +
> +    def p2p_accept_go_neg_request(self, request, pin=None,
> go_intent=None):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_CONNECT ' + request['p2p_dev_addr']
> + ' ' + ('pbc' if pin is None else pin) +
> +                        ('' if go_intent is None else 'go_intent=' +
> str(go_intent)))
> +        self.wait_for_event('OK')
> +
> +    # Pre-accept the next GO Negotiation Request from this peer to
> avoid the extra Respone + Request frames
> +    def p2p_authorize(self, peer, pin=None, go_intent=None):
> +        self._rx_data = []
> +        self._ctrl_request('P2P_CONNECT ' + peer['p2p_dev_addr'] + '
> ' + ('pbc' if pin is None else pin) +
> +                        ('' if go_intent is None else 'go_intent=' +
> str(go_intent)) + ' auth')
> +        self.wait_for_event('OK')
> +
> +    # Probably needed: remove references to self so that the GC can
> call __del__ automatically
> +    def clean_up(self):
> +        if self.io_watch is not None:
> +            GLib.source_remove(self.io_watch)
> +            self.io_watch = None
> +        if self.wpa_supplicant is not None:
> +            ctx.stop_process(self.wpa_supplicant)
> +            self.wpa_supplicant = None
> +
> +    def _stop_wpas(self):
> +        self.clean_up()
> +        if self.ctrl_sock:
> +            self.ctrl_sock.close()
> +            self.ctrl_sock = None
> +        if os.path.exists(self.remote_ctrl):
> +            os.remove(self.remote_ctrl)
> +        if os.path.exists(self.local_ctrl):
> +            os.remove(self.local_ctrl)
> +
> +    def __del__(self):
> +        self._stop_wpas()

Re: [PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[Andrew Zaborowski]

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

On Wed, 30 Sep 2020 at 17:56, James Prestwood <prestwoj@gmail.com> wrote:
> On Wed, 2020-09-30 at 01:29 +0200, Andrew Zaborowski wrote:
> > On Tue, 29 Sep 2020 at 19:15, James Prestwood <prestwoj@gmail.com>
> > wrote:
> > > Instead could we just add a new group [WPA_S] or [WPA_SUPPLICANT]
> > > and
> > > have radN=<config>. This would be consistent with how hostapd
> > > works,
> > > and still allows you to reserve radios for wpa_supplicant. And if
> > > we
> > > ever require wpa_s specific options in the future we may want a
> > > group
> > > for it.
> >
> > That's doable, but with wpa_supplicant we have to specify the control
> > socket path inside the config file, so the setting would have to be
> > something like "radN=<config>,<ctrl_socket>".  I think I'd rather
> > avoid creating separate Wpas and WpasCLI classes, we could have one
> > class handle both things whether it's created by test-runner or by
> > the
> > individual test, are you ok with that?
>
> After looking at the wpa_s help yeah your right, when providing a
> config file you cannot provide a control socket via the command line.
> But I still dont see how doing radN=<config>,<ctrl_socket> would aid in
> that since <ctrl_socket> would need to be in <config> anyways. Or are
> you saying having it in both places and <ctrl_socket> is merely to tell
> test-runner what value is already inside the config?

Yes, the Wpas class needs to know what value was in the config file so
that it can connect to the right socket, so I thought of providing it
through this setting.

>
> We could append the control socket to the config file like I did with
> hostapd (see _rewrite_config), although I'll admit this is somewhat of
> a hack. But this does allow you to generate a control socket on the fly
> without the test or config file knowing about it.

Oh, I didn't realize we were doing this.  It makes more sense because
we don't duplicate the socket path information.  We could use the same
mechanism to avoid duplicating other information like the device name
and type in case of P2P or the SSID in case of hostapd but I wonder if
it's easier to let the test generate the whole wpa_supplicant config
file.

Best regards

Re: [PATCH 6/8] autotests: Add basic wpa_supplicant P2P python wrapper

[Andrew Zaborowski]

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

On Wed, 30 Sep 2020 at 17:58, James Prestwood <prestwoj@gmail.com> wrote:
> On Tue, 2020-09-29 at 18:37 +0200, Andrew Zaborowski wrote:
> > +ctrl_count = 0
> > +mainloop = GLib.MainLoop()
>
> I think this is gonna end causing some weird behavior. Using
> ctx.mainloop should work better.

Ok.

> I had all kinds of strange issues due
> to test-runner and iwd.py both creating a mainloop.

It's not clear from
https://developer.gnome.org/pygobject/stable/class-glibmainloop.html
what creating multiple main loops implies, but the page says they'll
all refer to one "context" so maybe there's no issue... I copied this
part from hostapd.py assuming multiple GLib.MainLoop() calls would
return the same object.

Best regards

Re: [PATCH 1/8] test-runner: Reserve radios for wpa_supplicant

[James Prestwood]

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

On Wed, 2020-09-30 at 22:34 +0200, Andrew Zaborowski wrote:
> On Wed, 30 Sep 2020 at 17:56, James Prestwood <prestwoj@gmail.com>
> wrote:
> > On Wed, 2020-09-30 at 01:29 +0200, Andrew Zaborowski wrote:
> > > On Tue, 29 Sep 2020 at 19:15, James Prestwood <prestwoj@gmail.com
> > > >
> > > wrote:
> > > > Instead could we just add a new group [WPA_S] or
> > > > [WPA_SUPPLICANT]
> > > > and
> > > > have radN=<config>. This would be consistent with how hostapd
> > > > works,
> > > > and still allows you to reserve radios for wpa_supplicant. And
> > > > if
> > > > we
> > > > ever require wpa_s specific options in the future we may want a
> > > > group
> > > > for it.
> > > 
> > > That's doable, but with wpa_supplicant we have to specify the
> > > control
> > > socket path inside the config file, so the setting would have to
> > > be
> > > something like "radN=<config>,<ctrl_socket>".  I think I'd rather
> > > avoid creating separate Wpas and WpasCLI classes, we could have
> > > one
> > > class handle both things whether it's created by test-runner or
> > > by
> > > the
> > > individual test, are you ok with that?
> > 
> > After looking at the wpa_s help yeah your right, when providing a
> > config file you cannot provide a control socket via the command
> > line.
> > But I still dont see how doing radN=<config>,<ctrl_socket> would
> > aid in
> > that since <ctrl_socket> would need to be in <config> anyways. Or
> > are
> > you saying having it in both places and <ctrl_socket> is merely to
> > tell
> > test-runner what value is already inside the config?
> 
> Yes, the Wpas class needs to know what value was in the config file
> so
> that it can connect to the right socket, so I thought of providing it
> through this setting.
> 
> > We could append the control socket to the config file like I did
> > with
> > hostapd (see _rewrite_config), although I'll admit this is somewhat
> > of
> > a hack. But this does allow you to generate a control socket on the
> > fly
> > without the test or config file knowing about it.
> 
> Oh, I didn't realize we were doing this.  It makes more sense because
> we don't duplicate the socket path information.  We could use the
> same
> mechanism to avoid duplicating other information like the device name
> and type in case of P2P or the SSID in case of hostapd but I wonder
> if
> it's easier to let the test generate the whole wpa_supplicant config
> file.

I don't really know much about p2p but if the test itself ends up
needing to plug in a bunch of values then maybe it is better going in
the test.

No idea how many p2p tests will end up getting written, but if there
ends up being lots of common test code then maybe we could decide then
if it should go into test-runner itself.

> 
> Best regards