diff mbox series

wpa_supplicant: dBus: Flush Sate Property Change

Message ID 20240208010301.2630340-1-arowa@google.com
State Changes Requested
Headers show
Series wpa_supplicant: dBus: Flush Sate Property Change | expand

Commit Message

Arowa Suliman Feb. 8, 2024, 1:02 a.m. UTC
The "State" property in wpa_supplicant helps track important WiFi
connection events (like when the security key is being updated).
Currently, updates to this property aren't always sent right away. This
delay can make it difficult to accurately track the different stages of
the connection process.

This change ensures that any updates to the "State" property are sent
immediately. This will provide a more accurate and real-time view of the
WiFi connection process.

Signed-off-by: Arowa Suliman <arowa@chromium.org>
---
 wpa_supplicant/dbus/dbus_new.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Jouni Malinen March 3, 2024, 7:38 p.m. UTC | #1
On Wed, Feb 07, 2024 at 05:02:40PM -0800, Arowa Suliman wrote:
> The "State" property in wpa_supplicant helps track important WiFi
> connection events (like when the security key is being updated).
> Currently, updates to this property aren't always sent right away. This
> delay can make it difficult to accurately track the different stages of
> the connection process.
> 
> This change ensures that any updates to the "State" property are sent
> immediately. This will provide a more accurate and real-time view of the
> WiFi connection process.

This change breaks a couple of automated test cases: dbus_roam and
dbus_remove_connected. It looks like at least the dbus_roam case is
something that could result in breaking some deployed use cases due to
modified behavior. The PropertiesChanged event that indicates
State=completed could now go out without CurrentBSS being set. That
seems undesired or at least would require more careful consideration of
existing users of the interface and clear documentation of the potential
risks in the commit message.
Arowa Suliman March 11, 2024, 8:42 p.m. UTC | #2
On Sun, Mar 3, 2024 at 11:38 AM Jouni Malinen <j@w1.fi> wrote:
>
> On Wed, Feb 07, 2024 at 05:02:40PM -0800, Arowa Suliman wrote:
> > The "State" property in wpa_supplicant helps track important WiFi
> > connection events (like when the security key is being updated).
> > Currently, updates to this property aren't always sent right away. This
> > delay can make it difficult to accurately track the different stages of
> > the connection process.
> >
> > This change ensures that any updates to the "State" property are sent
> > immediately. This will provide a more accurate and real-time view of the
> > WiFi connection process.
>
> This change breaks a couple of automated test cases: dbus_roam and
> dbus_remove_connected. It looks like at least the dbus_roam case is
> something that could result in breaking some deployed use cases due to
> modified behavior. The PropertiesChanged event that indicates
> State=completed could now go out without CurrentBSS being set. That
> seems undesired or at least would require more careful consideration of
> existing users of the interface and clear documentation of the potential
> risks in the commit message.

I checked the failures in both tests and the reason for these failures
is the assumption that the State and CurrentBSS properties are always
sent together. After modifying the two tests to wait for these
properties separately the tests are passing. Even without my change
there is still a possibility that the properties could be sent
separately. For example the state property changes right before
WPA_DBUS_SEND_PROP_CHANGED_TIMEOUT and current BSS changes right after
that.
diff mbox series

Patch

diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
index 00b38edf5..8db3407e9 100644
--- a/wpa_supplicant/dbus/dbus_new.c
+++ b/wpa_supplicant/dbus/dbus_new.c
@@ -2327,6 +2327,7 @@  void wpas_dbus_signal_prop_changed(struct wpa_supplicant *wpa_s,
 		break;
 	case WPAS_DBUS_PROP_STATE:
 		prop = "State";
+                flush = TRUE;
 		break;
 	case WPAS_DBUS_PROP_CURRENT_BSS:
 		prop = "CurrentBSS";