[PATCH 1/7] handshake: Add cleanup function for handshake_state

[PATCH 1/7] handshake: Add cleanup function for handshake_state

[Denis Kenzior]

To allow _auto_(handshake_state_free) variables to be used.
---
 src/handshake.c | 7 ++++++-
 src/handshake.h | 3 +++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/handshake.c b/src/handshake.c
index 6b93774ab137..1c5ed2c9bc12 100644
--- a/src/handshake.c
+++ b/src/handshake.c
@@ -105,7 +105,12 @@ void __handshake_set_install_ext_tk_func(handshake_install_ext_tk_func_t func)
 
 void handshake_state_free(struct handshake_state *s)
 {
-	__typeof__(s->free) destroy = s->free;
+	__typeof__(s->free) destroy;
+
+	if (!s)
+		return;
+
+	destroy = s->free;
 
 	if (s->in_event) {
 		s->in_event = false;
diff --git a/src/handshake.h b/src/handshake.h
index 7200c3617b8e..815eb44ff6ba 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -24,6 +24,7 @@
 #include <stdbool.h>
 #include <asm/byteorder.h>
 #include <linux/types.h>
+#include <ell/cleanup.h>
 
 struct handshake_state;
 enum crypto_cipher;
@@ -298,3 +299,5 @@ const uint8_t *handshake_util_find_pmkid_kde(const uint8_t *data,
 					size_t data_len);
 void handshake_util_build_gtk_kde(enum crypto_cipher cipher, const uint8_t *key,
 					unsigned int key_index, uint8_t *to);
+
+DEFINE_CLEANUP_FUNC(handshake_state_free);
-- 
2.43.0

[PATCH 2/7] p2p: Simplify handshake_state cleanup

[Denis Kenzior]

---
 src/p2p.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index 5d96e6825e1a..c823b2b134dd 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1496,7 +1496,7 @@ static void p2p_handshake_event(struct handshake_state *hs,
 static void p2p_try_connect_group(struct p2p_device *dev)
 {
 	struct scan_bss *bss = dev->conn_wsc_bss;
-	struct handshake_state *hs = NULL;
+	_auto_(handshake_state_free) struct handshake_state *hs = NULL;
 	struct iovec ie_iov[16];
 	int ie_num = 0;
 	int r;
@@ -1562,6 +1562,7 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 		goto error;
 	}
 
+	l_steal_ptr(hs);
 	dev->conn_retry_count++;
 
 done:
@@ -1570,9 +1571,6 @@ done:
 
 error:
 not_supported:
-	if (hs)
-		handshake_state_free(hs);
-
 	p2p_connect_failed(dev);
 	goto done;
 }
-- 
2.43.0

[PATCH 3/7] p2p: Simplify cleanup of ies

[Denis Kenzior]

Use an _auto_ variable to cleanup IEs allocated by
p2p_build_association_req().  While here, take out unneeded L_WARN_ON
since p2p_build_association_req cannot fail.
---
 src/p2p.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index c823b2b134dd..b66a45cc0bcd 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1500,19 +1500,18 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 	struct iovec ie_iov[16];
 	int ie_num = 0;
 	int r;
-	struct p2p_association_req info = {};
+	struct p2p_association_req info = {
+		.capability = dev->capability,
+		.device_info = dev->device_info,
+	};
 	struct ie_rsn_info bss_info = {};
 	struct ie_rsn_info rsn_info = {};
 	uint8_t rsne_buf[256];
 	uint8_t wfd_ie[32];
+	_auto_(l_free) uint8_t *req_ie =
+		p2p_build_association_req(&info, &ie_iov[ie_num].iov_len);
 
-	info.capability = dev->capability;
-	info.device_info = dev->device_info;
-
-	ie_iov[0].iov_base = p2p_build_association_req(&info,
-							&ie_iov[0].iov_len);
-	L_WARN_ON(!ie_iov[0].iov_base);
-	ie_num = 1;
+	ie_iov[ie_num++].iov_base = req_ie;
 
 	if (dev->conn_own_wfd) {
 		ie_iov[ie_num].iov_base = wfd_ie;
@@ -1564,15 +1563,11 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 
 	l_steal_ptr(hs);
 	dev->conn_retry_count++;
-
-done:
-	l_free(ie_iov[0].iov_base);
 	return;
 
 error:
 not_supported:
 	p2p_connect_failed(dev);
-	goto done;
 }
 
 static void p2p_peer_provision_done(int err, struct wsc_credentials_info *creds,
-- 
2.43.0

[PATCH 4/7] netdev: iov_ie_append: Support iovecs with multiple IEs

[Denis Kenzior]

iov_ie_append assumed that a single IE was being added and thus the
length of the IE could be extracted directly from the element.  However,
iov_ie_append was used on buffers which could contain multiple IEs
concatenated together, for example in handshake_state::vendor_ies.  Most
of the time this was safe since vendor_ies was NULL or contained a
single element, but would result in incorrect behavior in the general
case.  Fix that by changing iov_ie_append signature to take an explicit
length argument and have the caller specify whether the element is a
single IE or multiple.

Fixes: 7e9971661bcb ("netdev: Append any vendor IEs from the handshake")
---
 src/netdev.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 208a15b94507..eb408447224c 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -209,7 +209,7 @@ static bool mac_per_ssid;
 
 static unsigned int iov_ie_append(struct iovec *iov,
 					unsigned int n_iov, unsigned int c,
-					const uint8_t *ie)
+					const uint8_t *ie, size_t len)
 {
 	if (L_WARN_ON(c >= n_iov))
 		return n_iov;
@@ -218,7 +218,7 @@ static unsigned int iov_ie_append(struct iovec *iov,
 		return c;
 
 	iov[c].iov_base = (void *) ie;
-	iov[c].iov_len = ie[1] + 2;
+	iov[c].iov_len = len;
 
 	return c + 1u;
 }
@@ -286,19 +286,22 @@ static unsigned int netdev_populate_common_ies(struct netdev *netdev,
 
 	extended_capabilities = wiphy_get_extended_capabilities(netdev->wiphy,
 								netdev->type);
-	c_iov = iov_ie_append(iov, n_iov, c_iov, extended_capabilities);
+	c_iov = iov_ie_append(iov, n_iov, c_iov, extended_capabilities,
+				IE_LEN(extended_capabilities));
 
 	rm_enabled_capabilities =
 		wiphy_get_rm_enabled_capabilities(netdev->wiphy);
-	c_iov = iov_ie_append(iov, n_iov, c_iov, rm_enabled_capabilities);
+	c_iov = iov_ie_append(iov, n_iov, c_iov, rm_enabled_capabilities,
+				IE_LEN(rm_enabled_capabilities));
 
 	if (rm_enabled_capabilities)
 		l_genl_msg_append_attr(msg, NL80211_ATTR_USE_RRM, 0, NULL);
 
-	c_iov = iov_ie_append(iov, n_iov, c_iov, hs->vendor_ies);
+	c_iov = iov_ie_append(iov, n_iov, c_iov,
+				hs->vendor_ies, hs->vendor_ies_len);
 
-	if (hs->fils_ip_req_ie)
-		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->fils_ip_req_ie);
+	c_iov = iov_ie_append(iov, n_iov, c_iov, hs->fils_ip_req_ie,
+				IE_LEN(hs->fils_ip_req_ie));
 
 	return c_iov;
 }
@@ -2502,7 +2505,8 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 
 	if (is_rsn) {
 		nl80211_append_rsn_attributes(msg, hs);
-		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->supplicant_ie);
+		c_iov = iov_ie_append(iov, n_iov, c_iov, hs->supplicant_ie,
+					IE_LEN(hs->supplicant_ie));
 	}
 
 	if (is_rsn || hs->settings_8021x) {
@@ -2517,10 +2521,10 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 
 	if (netdev->owe_sm) {
 		owe_build_dh_ie(netdev->owe_sm, owe_dh_ie, &dh_ie_len);
-		c_iov = iov_ie_append(iov, n_iov, c_iov, owe_dh_ie);
+		c_iov = iov_ie_append(iov, n_iov, c_iov, owe_dh_ie, dh_ie_len);
 	}
 
-	c_iov = iov_ie_append(iov, n_iov, c_iov, hs->mde);
+	c_iov = iov_ie_append(iov, n_iov, c_iov, hs->mde, IE_LEN(hs->mde));
 	c_iov = netdev_populate_common_ies(netdev, hs, msg, iov, n_iov, c_iov);
 
 	mpdu_sort_ies(subtype, iov, c_iov);
@@ -3267,9 +3271,11 @@ static void netdev_sae_tx_associate(void *user_data)
 
 	msg = netdev_build_cmd_associate_common(netdev);
 
-	n_used = iov_ie_append(iov, n_iov, n_used, hs->supplicant_ie);
-	n_used = iov_ie_append(iov, n_iov, n_used, hs->mde);
-	n_used = iov_ie_append(iov, n_iov, n_used, hs->supplicant_rsnxe);
+	n_used = iov_ie_append(iov, n_iov, n_used, hs->supplicant_ie,
+						IE_LEN(hs->supplicant_ie));
+	n_used = iov_ie_append(iov, n_iov, n_used, hs->mde, IE_LEN(hs->mde));
+	n_used = iov_ie_append(iov, n_iov, n_used, hs->supplicant_rsnxe,
+					IE_LEN(hs->supplicant_rsnxe));
 	n_used = netdev_populate_common_ies(netdev, hs, msg,
 							iov, n_iov, n_used);
 	mpdu_sort_ies(subtype, iov, n_used);
-- 
2.43.0

[PATCH 5/7] p2p: Use handshake to pass vendor ies

[Denis Kenzior]

Instead of passing them directly via netdev_connect
---
 src/p2p.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/p2p.c b/src/p2p.c
index b66a45cc0bcd..e2d752776cb8 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1550,11 +1550,12 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 	handshake_state_set_event_func(hs, p2p_handshake_event, dev);
 	handshake_state_set_ssid(hs, bss->ssid, bss->ssid_len);
 	handshake_state_set_pmk(hs, dev->conn_psk, 32);
+	handshake_state_set_vendor_ies(hs, ie_iov, ie_num);
 
 	if (dev->conn_peer_capability.group_caps & P2P_GROUP_CAP_IP_ALLOCATION)
 		hs->support_ip_allocation = true;
 
-	r = netdev_connect(dev->conn_netdev, bss, hs, ie_iov, ie_num,
+	r = netdev_connect(dev->conn_netdev, bss, hs, NULL, 0,
 				p2p_netdev_event, p2p_netdev_connect_cb, dev);
 	if (r < 0) {
 		l_error("netdev_connect error: %s (%i)", strerror(-r), -r);
-- 
2.43.0

[PATCH 6/7] wsc: Use handshake to pass vendor ies

[Denis Kenzior]

Instead of passing them directly via netdev_connect
---
 src/wsc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/wsc.c b/src/wsc.c
index cb2e2c3e2ad0..b063313ac6dc 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -382,7 +382,9 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 	if (ies_num)
 		memcpy(ie_iov + 1, ies, sizeof(struct iovec) * ies_num);
 
-	r = netdev_connect(wsce->netdev, bss, hs, ie_iov, 1 + ies_num,
+	handshake_state_set_vendor_ies(hs, ie_iov, 1 + ies_num);
+
+	r = netdev_connect(wsce->netdev, bss, hs, NULL, 0,
 				wsc_enrollee_netdev_event,
 				wsc_enrollee_connect_cb, wsce);
 	l_free(ie_iov[0].iov_base);
-- 
2.43.0

[PATCH 7/7] netdev: Remove vendor_ies from netdev_connect signature

[Denis Kenzior]

The vendor IEs are now passed in the handshake_state object instead.
---
 src/netdev.c  | 27 ++++++---------------------
 src/netdev.h  |  2 --
 src/p2p.c     |  2 +-
 src/station.c |  2 +-
 src/wsc.c     |  2 +-
 5 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index eb408447224c..f2e887b4d70b 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -2450,9 +2450,7 @@ static void netdev_driver_connected(struct netdev *netdev)
 
 static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 						struct handshake_state *hs,
-						const uint8_t *prev_bssid,
-						const struct iovec *vendor_ies,
-						size_t num_vendor_ies)
+						const uint8_t *prev_bssid)
 {
 	struct netdev_handshake_state *nhs =
 		l_container_of(hs, struct netdev_handshake_state, super);
@@ -2529,12 +2527,6 @@ static struct l_genl_msg *netdev_build_cmd_connect(struct netdev *netdev,
 
 	mpdu_sort_ies(subtype, iov, c_iov);
 
-	if (vendor_ies && !L_WARN_ON(n_iov - c_iov < num_vendor_ies)) {
-		memcpy(iov + c_iov, vendor_ies,
-					sizeof(*vendor_ies) * num_vendor_ies);
-		c_iov += num_vendor_ies;
-	}
-
 	if (c_iov)
 		l_genl_msg_append_attrv(msg, NL80211_ATTR_IE, iov, c_iov);
 
@@ -2570,8 +2562,7 @@ static bool netdev_retry_owe(struct netdev *netdev)
 	if (!owe_next_group(netdev->owe_sm))
 		return false;
 
-	connect_cmd = netdev_build_cmd_connect(netdev,
-					netdev->handshake, NULL, NULL, 0);
+	connect_cmd = netdev_build_cmd_connect(netdev, netdev->handshake, NULL);
 
 	netdev->connect_cmd_id = l_genl_family_send(nl80211, connect_cmd,
 						netdev_cmd_connect_cb, netdev,
@@ -3807,8 +3798,6 @@ static void netdev_connect_common(struct netdev *netdev,
 					const struct scan_bss *bss,
 					const struct scan_bss *prev_bss,
 					struct handshake_state *hs,
-					const struct iovec *vendor_ies,
-					size_t num_vendor_ies,
 					netdev_event_func_t event_filter,
 					netdev_connect_cb_t cb, void *user_data)
 {
@@ -3866,8 +3855,7 @@ static void netdev_connect_common(struct netdev *netdev,
 		break;
 	default:
 build_cmd_connect:
-		cmd_connect = netdev_build_cmd_connect(netdev, hs, prev_bssid,
-						vendor_ies, num_vendor_ies);
+		cmd_connect = netdev_build_cmd_connect(netdev, hs, prev_bssid);
 
 		if (!is_offload(hs) && (is_rsn || hs->settings_8021x)) {
 			sm = eapol_sm_new(hs);
@@ -3900,8 +3888,6 @@ build_cmd_connect:
 
 int netdev_connect(struct netdev *netdev, const struct scan_bss *bss,
 				struct handshake_state *hs,
-				const struct iovec *vendor_ies,
-				size_t num_vendor_ies,
 				netdev_event_func_t event_filter,
 				netdev_connect_cb_t cb, void *user_data)
 {
@@ -3918,9 +3904,8 @@ int netdev_connect(struct netdev *netdev, const struct scan_bss *bss,
 	if (netdev_handshake_state_setup_connection_type(hs) < 0)
 		return -ENOTSUP;
 
-	netdev_connect_common(netdev, bss, NULL, hs, vendor_ies,
-					num_vendor_ies, event_filter, cb,
-					user_data);
+	netdev_connect_common(netdev, bss, NULL, hs,
+				event_filter, cb, user_data);
 
 	return 0;
 }
@@ -4025,7 +4010,7 @@ int netdev_reassociate(struct netdev *netdev, const struct scan_bss *target_bss,
 	netdev->connected = false;
 	netdev->in_reassoc = true;
 
-	netdev_connect_common(netdev, target_bss, orig_bss, hs, NULL, 0,
+	netdev_connect_common(netdev, target_bss, orig_bss, hs,
 					event_filter, cb, user_data);
 
 	if (netdev->ap)
diff --git a/src/netdev.h b/src/netdev.h
index f27130f14e56..03d1b6e94dd4 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -155,8 +155,6 @@ struct handshake_state *netdev_get_handshake(struct netdev *netdev);
 
 int netdev_connect(struct netdev *netdev, const struct scan_bss *bss,
 				struct handshake_state *hs,
-				const struct iovec *vendor_ies,
-				size_t num_vendor_ies,
 				netdev_event_func_t event_filter,
 				netdev_connect_cb_t cb, void *user_data);
 int netdev_disconnect(struct netdev *netdev,
diff --git a/src/p2p.c b/src/p2p.c
index e2d752776cb8..08fe8444fbb9 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1555,7 +1555,7 @@ static void p2p_try_connect_group(struct p2p_device *dev)
 	if (dev->conn_peer_capability.group_caps & P2P_GROUP_CAP_IP_ALLOCATION)
 		hs->support_ip_allocation = true;
 
-	r = netdev_connect(dev->conn_netdev, bss, hs, NULL, 0,
+	r = netdev_connect(dev->conn_netdev, bss, hs,
 				p2p_netdev_event, p2p_netdev_connect_cb, dev);
 	if (r < 0) {
 		l_error("netdev_connect error: %s (%i)", strerror(-r), -r);
diff --git a/src/station.c b/src/station.c
index 49cad135eaf0..48a595dcd340 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3453,7 +3453,7 @@ int __station_connect_network(struct station *station, struct network *network,
 	if (!hs)
 		return -ENOTSUP;
 
-	r = netdev_connect(station->netdev, bss, hs, NULL, 0,
+	r = netdev_connect(station->netdev, bss, hs,
 				station_netdev_event,
 				station_connect_cb, station);
 	if (r < 0) {
diff --git a/src/wsc.c b/src/wsc.c
index b063313ac6dc..cda877cf7e6d 100644
--- a/src/wsc.c
+++ b/src/wsc.c
@@ -384,7 +384,7 @@ static int wsc_enrollee_connect(struct wsc_enrollee *wsce, struct scan_bss *bss,
 
 	handshake_state_set_vendor_ies(hs, ie_iov, 1 + ies_num);
 
-	r = netdev_connect(wsce->netdev, bss, hs, NULL, 0,
+	r = netdev_connect(wsce->netdev, bss, hs,
 				wsc_enrollee_netdev_event,
 				wsc_enrollee_connect_cb, wsce);
 	l_free(ie_iov[0].iov_base);
-- 
2.43.0

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[James Prestwood]

Hi Denis,

On 11/30/23 20:00, Denis Kenzior wrote:
> To allow _auto_(handshake_state_free) variables to be used.
> ---
>   src/handshake.c | 7 ++++++-
>   src/handshake.h | 3 +++
>   2 files changed, 9 insertions(+), 1 deletion(-)
All LGTM

Thanks,

James

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[Denis Kenzior]

Hi James,

On 12/1/23 06:42, James Prestwood wrote:
> Hi Denis,
> 
> On 11/30/23 20:00, Denis Kenzior wrote:
>> To allow _auto_(handshake_state_free) variables to be used.
>> ---
>>   src/handshake.c | 7 ++++++-
>>   src/handshake.h | 3 +++
>>   2 files changed, 9 insertions(+), 1 deletion(-)
> All LGTM
> 

Any ideas why testPSK-roam tests are failing?  They seem to be quite flaky on my 
machine as well, passing only 4/9?  Doesn't seem related to this series though?

|     testPSK-roam    |   4    |   5    |    0    | 256.84 |


Regards,
-Denis

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[James Prestwood]

Hi Denis,

On 12/1/23 07:08, Denis Kenzior wrote:
> Hi James,
>
> On 12/1/23 06:42, James Prestwood wrote:
>> Hi Denis,
>>
>> On 11/30/23 20:00, Denis Kenzior wrote:
>>> To allow _auto_(handshake_state_free) variables to be used.
>>> ---
>>>   src/handshake.c | 7 ++++++-
>>>   src/handshake.h | 3 +++
>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>> All LGTM
>>
>
> Any ideas why testPSK-roam tests are failing?  They seem to be quite 
> flaky on my machine as well, passing only 4/9?  Doesn't seem related 
> to this series though?
>
> |     testPSK-roam    |   4    |   5    |    0    | 256.84 |

Not seeing that on my end (with and without your patches). The 
packet-loss roam tests is always flaky on the CI, but I've never seen 
that many failures.

What UML kernel are you running so I can take that out of the equation?

>
>
> Regards,
> -Denis
>

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[James Prestwood]

On 12/1/23 07:22, James Prestwood wrote:
> Hi Denis,
>
> On 12/1/23 07:08, Denis Kenzior wrote:
>> Hi James,
>>
>> On 12/1/23 06:42, James Prestwood wrote:
>>> Hi Denis,
>>>
>>> On 11/30/23 20:00, Denis Kenzior wrote:
>>>> To allow _auto_(handshake_state_free) variables to be used.
>>>> ---
>>>>   src/handshake.c | 7 ++++++-
>>>>   src/handshake.h | 3 +++
>>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>> All LGTM
>>>
>>
>> Any ideas why testPSK-roam tests are failing?  They seem to be quite 
>> flaky on my machine as well, passing only 4/9?  Doesn't seem related 
>> to this series though?
>>
>> |     testPSK-roam    |   4    |   5    |    0    | 256.84 |
>
> Not seeing that on my end (with and without your patches). The 
> packet-loss roam tests is always flaky on the CI, but I've never seen 
> that many failures.
>
> What UML kernel are you running so I can take that out of the equation?
Nvm, I'm getting the 4 failures on a 6.6 kernel now. Something must have 
changed so I'll look into it.
>
>>
>>
>> Regards,
>> -Denis
>>

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[James Prestwood]

Hi Denis,

On 12/1/23 07:32, James Prestwood wrote:
>
> On 12/1/23 07:22, James Prestwood wrote:
>> Hi Denis,
>>
>> On 12/1/23 07:08, Denis Kenzior wrote:
>>> Hi James,
>>>
>>> On 12/1/23 06:42, James Prestwood wrote:
>>>> Hi Denis,
>>>>
>>>> On 11/30/23 20:00, Denis Kenzior wrote:
>>>>> To allow _auto_(handshake_state_free) variables to be used.
>>>>> ---
>>>>>   src/handshake.c | 7 ++++++-
>>>>>   src/handshake.h | 3 +++
>>>>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>>> All LGTM
>>>>
>>>
>>> Any ideas why testPSK-roam tests are failing?  They seem to be quite 
>>> flaky on my machine as well, passing only 4/9?  Doesn't seem related 
>>> to this series though?
>>>
>>> |     testPSK-roam    |   4    |   5    |    0    | 256.84 |
>>
>> Not seeing that on my end (with and without your patches). The 
>> packet-loss roam tests is always flaky on the CI, but I've never seen 
>> that many failures.
>>
>> What UML kernel are you running so I can take that out of the equation?
> Nvm, I'm getting the 4 failures on a 6.6 kernel now. Something must 
> have changed so I'll look into it.

6.2 - works

6.5 - works

6.6 - broken

I tried applying that patch [1] from Johannes onto 6.6 but it failed to 
apply. So anyways it appears CQM events are broken in at least 6.6.

[1] 
https://lore.kernel.org/linux-wireless/bbfd6f959e7ff4b567084ef3d962bf255aa25c85.camel@sipsolutions.net/T/#t

>>
>>>
>>>
>>> Regards,
>>> -Denis
>>>

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[Denis Kenzior]

Hi James,

>> Nvm, I'm getting the 4 failures on a 6.6 kernel now. Something must have 
>> changed so I'll look into it.
> 
> 6.2 - works
> 
> 6.5 - works
> 
> 6.6 - broken

Sigh, figures.  I doubt the patch from Johannes would fix anything for us since 
we don't query carrier state directly if I recall correctly.  We only react to 
events, at least on the connect path.

Regards,
-Denis

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[James Prestwood]

Hi Denis,

On 12/1/23 08:32, Denis Kenzior wrote:
> Hi James,
>
>>> Nvm, I'm getting the 4 failures on a 6.6 kernel now. Something must 
>>> have changed so I'll look into it.
>>
>> 6.2 - works
>>
>> 6.5 - works
>>
>> 6.6 - broken
>
> Sigh, figures.  I doubt the patch from Johannes would fix anything for 
> us since we don't query carrier state directly if I recall correctly.  
> We only react to events, at least on the connect path.

Two separate things here:

  - Confirmed that CQM events are broken at least in 6.6, this was the 
patch I linked in this thread.

  - Earlier email I sent about the carrier state was more of a suspected 
problem for flaky tests, before I even saw the roaming tests were 
failing. I haven't tried applying those kernel patches, but it would 
also require userspace changes checking that extra attribute. I just 
need to look more into this.

Anyways, yes, unfortunate. At it's also broken in wpa_supplicant so its 
going to get a lot of visibility.

Thanks,

James

>
> Regards,
> -Denis

Re: [PATCH 1/7] handshake: Add cleanup function for handshake_state

[Denis Kenzior]

On 11/30/23 22:00, Denis Kenzior wrote:
> To allow _auto_(handshake_state_free) variables to be used.
> ---
>   src/handshake.c | 7 ++++++-
>   src/handshake.h | 3 +++
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 

Applied.