[PATCH 1/3] station: Remove diagnostics interface reliably

[PATCH 1/3] station: Remove diagnostics interface reliably

[Andrew Zaborowski]

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

If the idea is that the interface should only be present when connected
then don't do this in the DISCONNECTING state as there are various
possible transitions from CONNECTED or ROAMING directly to DISCONNECTED.
---
 src/station.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/station.c b/src/station.c
index 5b8b61b4..11711973 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1211,9 +1211,6 @@ static void station_enter_state(struct station *station,
 		periodic_scan_stop(station);
 		break;
 	case STATION_STATE_DISCONNECTING:
-		l_dbus_object_remove_interface(dbus_get_bus(),
-					netdev_get_path(station->netdev),
-					IWD_STATION_DIAGNOSTIC_INTERFACE);
 		break;
 	case STATION_STATE_ROAMING:
 		break;
@@ -1317,6 +1314,8 @@ static void station_reset_connection_state(struct station *station)
 				IWD_STATION_INTERFACE, "ConnectedNetwork");
 	l_dbus_property_changed(dbus, network_get_path(network),
 				IWD_NETWORK_INTERFACE, "Connected");
+	l_dbus_object_remove_interface(dbus, netdev_get_path(station->netdev),
+				IWD_STATION_DIAGNOSTIC_INTERFACE);
 }
 
 static void station_disassociated(struct station *station)
-- 
2.30.2

[PATCH 2/3] station: Check if busy in station_get_diagnostics

[Andrew Zaborowski]

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

---
 src/station.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/station.c b/src/station.c
index 11711973..0b0cc59c 100644
--- a/src/station.c
+++ b/src/station.c
@@ -3768,6 +3768,9 @@ static struct l_dbus_message *station_get_diagnostics(struct l_dbus *dbus,
 	struct station *station = user_data;
 	int ret;
 
+	if (station->get_station_pending)
+		return dbus_error_busy(message);
+
 	ret = netdev_get_current_station(station->netdev,
 				station_get_diagnostic_cb, station,
 				station_get_diagnostic_destroy);
-- 
2.30.2

Re: [PATCH 2/3] station: Check if busy in station_get_diagnostics

[Denis Kenzior]

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

Hi Andrew,

On 6/18/21 2:33 AM, Andrew Zaborowski wrote:
> ---
>   src/station.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

[PATCH 3/3] station: Stop periodic scan when netdev connected

[Andrew Zaborowski]

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

For consistency call periodic_scan_stop() right after netdev signals
connected even if station stays in the CONNECTING state because it's
waiting for netconfig to finish.
---
 src/station.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/station.c b/src/station.c
index 0b0cc59c..66ddc76e 100644
--- a/src/station.c
+++ b/src/station.c
@@ -2468,6 +2468,7 @@ static void station_connect_ok(struct station *station)
 	}
 
 	network_connected(station->connected_network);
+	periodic_scan_stop(station);
 
 	if (station->netconfig)
 		netconfig_configure(station->netconfig,
-- 
2.30.2

Re: [PATCH 3/3] station: Stop periodic scan when netdev connected

[Denis Kenzior]

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

Hi Andrew,

On 6/18/21 2:33 AM, Andrew Zaborowski wrote:
> For consistency call periodic_scan_stop() right after netdev signals
> connected even if station stays in the CONNECTING state because it's
> waiting for netconfig to finish.

Hmm, why is this needed?  We should be calling periodic_scan_stop as soon as we 
transition to CONNECTING/CONNECTING_AUTO state.  In fact, even the 
periodic_scan_stop on transition to CONNECTED seems suspicious now.

Might be helpful to include an actual log of the issue you're fixing in the 
commit description?

> ---
>   src/station.c | 1 +
>   1 file changed, 1 insertion(+)
> 

Regards,
-Denis

Re: [PATCH 3/3] station: Stop periodic scan when netdev connected

[Andrew Zaborowski]

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

On Fri, 18 Jun 2021 at 17:05, Denis Kenzior <denkenz@gmail.com> wrote:
> On 6/18/21 2:33 AM, Andrew Zaborowski wrote:
> > For consistency call periodic_scan_stop() right after netdev signals
> > connected even if station stays in the CONNECTING state because it's
> > waiting for netconfig to finish.
>
> Hmm, why is this needed?  We should be calling periodic_scan_stop as soon as we
> transition to CONNECTING/CONNECTING_AUTO state.

That's another option but I think the logic until now was that if the
connection failed we'd just return to periodic scanning without ever
stopping and restarting it.  During the netdev CMD_CONNECT it would be
inhibited by the kernel or by the radio work.  We only needed to
explicitly stop the scanning when CMD_CONNECT succeeded, so that's
what I was trying to restore in this patch, otherwise if we were
unlucky we might have been scanning during the netconfig.

Best regards

Re: [PATCH 3/3] station: Stop periodic scan when netdev connected

[Denis Kenzior]

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

Hi Andrew,

On 6/18/21 11:16 AM, Andrew Zaborowski wrote:
> On Fri, 18 Jun 2021 at 17:05, Denis Kenzior <denkenz@gmail.com> wrote:
>> On 6/18/21 2:33 AM, Andrew Zaborowski wrote:
>>> For consistency call periodic_scan_stop() right after netdev signals
>>> connected even if station stays in the CONNECTING state because it's
>>> waiting for netconfig to finish.
>>
>> Hmm, why is this needed?  We should be calling periodic_scan_stop as soon as we
>> transition to CONNECTING/CONNECTING_AUTO state.
> 
> That's another option but I think the logic until now was that if the
> connection failed we'd just return to periodic scanning without ever
> stopping and restarting it.  During the netdev CMD_CONNECT it would be

I don't think so.  See commit c3e160880f32f and commit da485179a6e92 prior to 
that.  It looks like we have stopped periodic scan when entering CONNECTING 
state for quite some time.

> inhibited by the kernel or by the radio work.  We only needed to
> explicitly stop the scanning when CMD_CONNECT succeeded, so that's
> what I was trying to restore in this patch, otherwise if we were
> unlucky we might have been scanning during the netconfig.

Fair enough, but it doesn't look like your patch does that since we stop the 
periodic scan when we reach CONNECTING state today.

Regards,
-Denis

Re: [PATCH 3/3] station: Stop periodic scan when netdev connected

[Andrew Zaborowski]

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

On Fri, 18 Jun 2021 at 19:21, Denis Kenzior <denkenz@gmail.com> wrote:
> On 6/18/21 11:16 AM, Andrew Zaborowski wrote:
> > On Fri, 18 Jun 2021 at 17:05, Denis Kenzior <denkenz@gmail.com> wrote:
> >>  We should be calling periodic_scan_stop as soon as we
> >> transition to CONNECTING/CONNECTING_AUTO state.
> >
> > That's another option but I think the logic until now was that if the
> > connection failed we'd just return to periodic scanning without ever
> > stopping and restarting it.  During the netdev CMD_CONNECT it would be
>
> I don't think so.  See commit c3e160880f32f and commit da485179a6e92 prior to
> that.  It looks like we have stopped periodic scan when entering CONNECTING
> state for quite some time.

True, I missed this.  Please ignore this patch.

Best regards

Re: [PATCH 1/3] station: Remove diagnostics interface reliably

[Denis Kenzior]

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

Hi Andrew,

On 6/18/21 2:33 AM, Andrew Zaborowski wrote:
> If the idea is that the interface should only be present when connected
> then don't do this in the DISCONNECTING state as there are various
> possible transitions from CONNECTED or ROAMING directly to DISCONNECTED.
> ---
>   src/station.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 

Applied, thanks.

Regards,
-Denis