Hi Daniel,
>>> In the following scenario:
>>> 1. Device gets connected to an AP.
>>> 2. Goto AP's settings page and add device's MAC address in block
list
>>> 3. Device gets disconnected with AP.
>>> 4. Auto Connection is again triggered with AP.
>>> In above scenario steps 3 and 4 are repeated infintely even though
>>> device would never connect to AP because it's MAC address is added in
>>> block list of AP.
>>> On the first disconnection connect-failed error is set. This patch only
>>> blocks auto connection whenever an error is set for the service.
>
>
>> service_indicate_state() {
>> [...]
>
>> switch (new_state) {
>> [...]
>
>> case CONNMAN_SERVICE_STATE_DISCONNECT:
>> set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
>
>> [...]
>> }
>
>> [...]
>> }
>
>> Is that the source of setting the error variable to
CONNMAN_SERVICE_ERROR_UNKNOWN?
> Yes.
Thanks for confirming it. So I decoded it correctly.
>> From a quick look at the pluging tells me that we don't
propagate
>> or use the disconnect reason code at all. I suspect we would get
>> some hint why the disconnect happened. For example if we see reason
>> code 4 "Disassociated due to inactivity" would be a non-error reason
and
>> we would be allowed to reconnect. See table 8-36 in the IEEE 802.11-2012 spec.
>
>> Do you see a reason code in the above scenario?
> On most occasions I got reason code as 6 "Class 2 frame received from
> nonauthenticated STA" and some occasions reason code 1.
So at leas reason code 6 seems to be correct. 1 is not really
telling
anything. Ideally we start logging things like reason code 6 or and
let the user know why stuff doesn't work.
>>> ---
>>> src/service.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/service.c b/src/service.c
>>> index 737a39f..e6fdef8 100644
>>> --- a/src/service.c
>>> +++ b/src/service.c
>>> @@ -3733,6 +3733,9 @@ static bool is_ignore(struct connman_service *service)
>>> if (service->state == CONNMAN_SERVICE_STATE_FAILURE)
>>> return true;
>>>
>>> + if (service->error != CONNMAN_SERVICE_ERROR_UNKNOWN)
>>> + return true;
>>> +
>>> if (!is_ipconfig_usable(service))
>>> return true;
>
>> Looking at the auto_connect_service() code we have following:
>
>> if (is_ignore(service) || service->state !=
>> CONNMAN_SERVICE_STATE_IDLE)
>> continue;
>
>> The service is in CONNMAN_SERVICE_STATE_IDLE state and not in
>> CONNMAN_SERVICE_STATE_FAILURE. If so is_ignored() doesn't need to be updated
>> (which I would prefer because we are looking only on the service state
>> there and not the newtork state)
>
>> So from this, should we set the service state to FAILURE if we have
>> a network error code?
> Are you trying to say that we should decide to remain to FAILURE state or change to
> IDLE state depending on the reason code?
> If yes, then what would be the reason codes on which we should change to IDLE state
> from FAILURE?
It's been a while since I last hacked on the service state
machine. IIRC the
rule is the service state is ending in FAILURE ConnMan will stop trying to
autoconnect. So my reasoning is if we get an disconnect from a plugin we check
why it was disconnected. If the disconnect just happened because the signal
got so weak (soft fail), the service should end in IDLE state. Though if we
get a disconnect because of reason code 6 we should set the state to FAILURE.
That should stop ConnMan from auto connect.
Does the attached patch work for you? I tested it on my network and
as
soon I blocked my client, the service entered into FAILURE state and
stayed there.
Thanks,
Daniel
>From 9b49a97a0730e03232e128202c1ca3744adfec1d Mon Sep 17 00:00:00
2001
From: Daniel Wagner <wagi at monom.org>
Date: Sun, 1 Jan 2017 15:17:24 +0100
Subject: [PATCH] wifi: Propagete disconnect reason code to service
Currently, if the WiFi plugin gets disconnected we ignore the reason
why that happened. Instead we just cleanup and let the corresponding
Service go to the IDLE state. If autoconnect than decides to reconnect
to that service it might just fail again, because the AP decided to
block us.
To prevent this busy loop we introduce a new error code blocked on
the
network and service layer. So whenever the AP decided to kick this
client from the network we set the error code.
BTW, wpa_supplicant sents first the reason code before it tells
ConnMan the interface got disconnected.
---
include/network.h | 1 +
include/service.h | 1 +
plugins/wifi.c | 13 +++++++++++++
src/network.c | 13 +++++++++++++
src/service.c | 2 ++
5 files changed, 30 insertions(+)
diff --git a/include/network.h b/include/network.h
index bb9647f23aaa..5d44669599c8 100644
--- a/include/network.h
+++ b/include/network.h
@@ -55,6 +55,7 @@ enum connman_network_error {
CONNMAN_NETWORK_ERROR_CONFIGURE_FAIL = 2,
CONNMAN_NETWORK_ERROR_INVALID_KEY = 3,
CONNMAN_NETWORK_ERROR_CONNECT_FAIL = 4,
+ CONNMAN_NETWORK_ERROR_BLOCKED = 5,
};
#define CONNMAN_NETWORK_PRIORITY_LOW -100
diff --git a/include/service.h b/include/service.h
index 31dfce7e217c..185f008f585b 100644
--- a/include/service.h
+++ b/include/service.h
@@ -79,6 +79,7 @@ enum connman_service_error {
CONNMAN_SERVICE_ERROR_LOGIN_FAILED = 5,
CONNMAN_SERVICE_ERROR_AUTH_FAILED = 6,
CONNMAN_SERVICE_ERROR_INVALID_KEY = 7,
+ CONNMAN_SERVICE_ERROR_BLOCKED = 8,
};
enum connman_service_proxy_method {
diff --git a/plugins/wifi.c b/plugins/wifi.c
index 70cec7772e74..f8d88fa3ada0 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -2461,6 +2461,19 @@ static void interface_state(GSupplicantInterface *interface)
network, wifi))
break;
+ /* See table 8-36 Reason codes in IEEE Std 802.11 */
+ switch (wifi->disconnect_code) {
+ case 1: /* Unspecified reason */
+ /* Let's assume it's because we got blocked */
+
+ case 6: /* Class 2 frame received from nonauthenticated STA */
+ connman_network_set_error(network,
+ CONNMAN_NETWORK_ERROR_BLOCKED);
+ break;
+
+ default:
+ break;
+ }
connman_network_set_connected(network, false);
connman_network_set_associating(network, false);
wifi->disconnecting = false;
diff --git a/src/network.c b/src/network.c
index aa82b741f62e..5b7ef5569f7a 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1256,6 +1256,16 @@ static void set_connect_error(struct connman_network *network)
CONNMAN_SERVICE_ERROR_CONNECT_FAILED);
}
+static void set_blocked_error(struct connman_network *network)
+{
+ struct connman_service *service;
+
+ service = connman_service_lookup_from_network(network);
+
+ __connman_service_indicate_error(service,
+ CONNMAN_SERVICE_ERROR_BLOCKED);
+}
+
void connman_network_set_ipv4_method(struct connman_network *network,
enum connman_ipconfig_method method)
{
@@ -1310,6 +1320,9 @@ void connman_network_set_error(struct connman_network *network,
case CONNMAN_NETWORK_ERROR_CONNECT_FAIL:
set_connect_error(network);
break;
+ case CONNMAN_NETWORK_ERROR_BLOCKED:
+ set_blocked_error(network);
+ break;
}
__connman_network_disconnect(network);
diff --git a/src/service.c b/src/service.c
index 737a39f4c3b0..2a7147639c99 100644
--- a/src/service.c
+++ b/src/service.c
@@ -320,6 +320,8 @@ static const char *error2string(enum connman_service_error error)
return "auth-failed";
case CONNMAN_SERVICE_ERROR_INVALID_KEY:
return "invalid-key";
+ case CONNMAN_SERVICE_ERROR_BLOCKED:
+ return "blocked";
}
return NULL;
--
Above patch works on my setup
Thanks,
Saurav