Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. [PATCH] service: Block wifi autoconnection when hidden
connection is in progress (Saurav Babu)
2. [PATCH] service: Disconnect old service only when connecting
new service (Saurav Babu)
3. Re: [PATCH] service: Block wifi autoconnection when hidden
connection is in progress (Daniel Wagner)
4. [PATCHv2] service: Block auto connect when hidden connection
is in progress (Saurav Babu)
5. [PATCH] service: Block wifi autoconnection when hidden
connection is in progress (Saurav Babu)
----------------------------------------------------------------------
Message: 1
Date: Tue, 08 Nov 2016 11:27:47 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected], Saurav Babu <[email protected]>
Subject: [PATCH] service: Block wifi autoconnection when hidden
connection is in progress
Message-ID:
<[email protected]>
In the following scenario:
1. Device is connected to AP in non hidden mode.
2. Connection is initated to AP in hidden mode.
connman disconnects old service and tries to connect to new service. But
for connecting hidden service it first scans the hidden AP and when
network is added for hidden AP then connection is tried. In the meantime
normal AP got disconnected and was tried to autoconnect even before
hidden AP was scanned successfully.
Ideally non hidden AP should not be autoconnected when hidden AP connection
is in progress.
This patch blocks wifi autoconnection when hidden connection is in
progress and unblocks autoconnection after hidden connection is started
or hidden wifi scan is completed and hidden AP is not found in the scan list.
---
src/connman.h | 2 ++
src/network.c | 4 ++++
src/service.c | 13 ++++++++++++-
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/connman.h b/src/connman.h
index ce3ef8d..24461b1 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -796,6 +796,8 @@ void __connman_service_notify(struct connman_service
*service,
int __connman_service_counter_register(const char *counter);
void __connman_service_counter_unregister(const char *counter);
+void __connman_set_wifi_autoconnect(bool autoconnect);
+
#include <connman/peer.h>
int __connman_peer_init(void);
diff --git a/src/network.c b/src/network.c
index 2e423bc..eccb2ef 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1370,6 +1370,8 @@ void connman_network_clear_hidden(void *user_data)
DBG("user_data %p", user_data);
+ __connman_set_wifi_autoconnect(true);
+
/*
* Hidden service does not have a connect timeout so
* we do not need to remove it. We can just return
@@ -1389,6 +1391,8 @@ int connman_network_connect_hidden(struct connman_network
*network,
DBG("network %p service %p user_data %p", network, service, user_data);
+ __connman_set_wifi_autoconnect(true);
+
if (!service)
return -EINVAL;
diff --git a/src/service.c b/src/service.c
index f6a76f6..5fdff4b 100644
--- a/src/service.c
+++ b/src/service.c
@@ -48,6 +48,7 @@ static unsigned int autoconnect_timeout = 0;
static unsigned int vpn_autoconnect_timeout = 0;
static struct connman_service *current_default = NULL;
static bool services_dirty = false;
+static bool wifi_autoconnect = true;
struct connman_stats {
bool valid;
@@ -3764,6 +3765,9 @@ static bool auto_connect_service(GList *services,
ignore[CONNMAN_SERVICE_TYPE_VPN] = true;
+ if (!wifi_autoconnect)
+ ignore[CONNMAN_SERVICE_TYPE_WIFI] = true;
+
for (list = services; list; list = list->next) {
service = list->data;
@@ -5921,12 +5925,19 @@ static void prepare_8021x(struct connman_service
*service)
service->phase2);
}
+void __connman_set_wifi_autoconnect(bool autoconnect)
+{
+ wifi_autoconnect = autoconnect;
+}
+
static int service_connect(struct connman_service *service)
{
int err;
- if (service->hidden)
+ if (service->hidden) {
+ wifi_autoconnect = false;
return -EPERM;
+ }
switch (service->type) {
case CONNMAN_SERVICE_TYPE_UNKNOWN:
--
1.9.1
------------------------------
Message: 2
Date: Tue, 08 Nov 2016 11:37:28 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected], Saurav Babu <[email protected]>
Subject: [PATCH] service: Disconnect old service only when connecting
new service
Message-ID:
<[email protected]>
Hi Daniel,
Sorry for the delayed reply
>>> So the problem is that autoconnect is faster than scan,connect operation?
>> Yes, autoconnect is scheduled just after old service is disconnected,
>> scanning
>> new service is taking around 3secs at my setup, so autoconnect is faster
>> than scan,connect operation.
> Okay, good we are the same page :)
>>> I am very reluctant to move the for() loop down from connect_service()
>>> to service_connect() because it is not clear to me what the impact of it
>>> is. If I read it correctly D-Bus initiates connection attempts wouldn't
>>> change because the filter is just deeper down in the call change. Is it
>>> really only the calls from autoconnect which will be blocked?
>> Yes, with this patch only calls from autoconnect will be blocked because old
>> service will never be disconnected until connection is initiated for new
>> hidden
>> AP.
> The thing is service_connect() is called from a bunch of places inside
> ConnMan. Currently it will just do what the names says, it connects the
> service. With your change it will suddenly start to filter commands.
> That is what I do not like about it. For example we have the Session
> plugin API which allows to write your own autoconnect if need to. With
> the filter at service_connect() we change the behavior of the API. I
> would like to understand what this change means.
>>> Maybe you should try to teach autoconnect not to call
>>> __connman_service_connect() when we try to connect to a hidden AP? In my
>>> eyes it makes more sense to teach autoconnect not to do stupid stuff
>>> instead of blocking it somewhere else.
>> This patch gives me flexibility that old service is not disconnected until
>> new
>> hidden service is found in scan list. There is a scenario when user enters
>> wrong
>> AP name for hidden AP or WPA Supplicant/Driver fails to scan hidden AP then
>> also
>> currently old service will be disconnected before initiating scan for hidden
>> AP.
> I am not saying your use case is not correct or invalid.
>> This patch will allow old service to remain in connected state for more
>> time(time
>> when we are only scanning hidden AP to find if it is available) and would
>> only
>> disconnect when required(hidden AP is found in scan).
> But we still disconnect before we connect the hidden service which can
> fail too. So you just keep the current connection open a bit longer
> before trying to connect to the hidden AP.
> I would say we have two different things here:
> 1) try to stay connected as long as possible
> 2) autoconnect faster than hidden AP scan/connect operation
> If we leave out 1) for a minute, I still think autoconnect should not
> attempt to connect a service when we have a connect request from the
> user. This is not special to wifi come to think of it. I would really
> like to see if you can come up with something which makes autoconnect a
> bit smarter about this scenario?
I've now blocked autoconnect for wifi when we have hidden connect request
from user. Updated patch can be found at
https://lists.01.org/pipermail/connman/2016-November/021116.html
Thanks,
Saurav
------------------------------
Message: 3
Date: Tue, 8 Nov 2016 08:55:37 +0100
From: Daniel Wagner <[email protected]>
To: Saurav Babu <[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] service: Block wifi autoconnection when hidden
connection is in progress
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Saurav,
On 11/08/2016 06:57 AM, Saurav Babu wrote:
> In the following scenario:
> 1. Device is connected to AP in non hidden mode.
> 2. Connection is initated to AP in hidden mode.
> connman disconnects old service and tries to connect to new service. But
> for connecting hidden service it first scans the hidden AP and when
> network is added for hidden AP then connection is tried. In the meantime
> normal AP got disconnected and was tried to autoconnect even before
> hidden AP was scanned successfully.
> Ideally non hidden AP should not be autoconnected when hidden AP connection
> is in progress.
> This patch blocks wifi autoconnection when hidden connection is in
> progress and unblocks autoconnection after hidden connection is started
> or hidden wifi scan is completed and hidden AP is not found in the scan list.
> ---
> src/connman.h | 2 ++
> src/network.c | 4 ++++
> src/service.c | 13 ++++++++++++-
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index ce3ef8d..24461b1 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -796,6 +796,8 @@ void __connman_service_notify(struct connman_service
> *service,
> int __connman_service_counter_register(const char *counter);
> void __connman_service_counter_unregister(const char *counter);
>
> +void __connman_set_wifi_autoconnect(bool autoconnect);
> +
Since this function belongs to the service internal API it should
have the prefix __connman_service. Furthermore, I rather have a
textual hint what it does. Usually a plain bool is easy to get
wrong. So what about
__connman_service_{enable|disable}_autoconnect()
?
Something sligthly more hacky would be to overload
__connman_service_auto_connect() with a reason code like
CONNMAN_SERVICE_CONNECT_REASON_BLOCK. But that is also a bit
ugly. Maybe the above is better.
> #include <connman/peer.h>
>
> int __connman_peer_init(void);
> diff --git a/src/network.c b/src/network.c
> index 2e423bc..eccb2ef 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1370,6 +1370,8 @@ void connman_network_clear_hidden(void *user_data)
>
> DBG("user_data %p", user_data);
>
> + __connman_set_wifi_autoconnect(true);
> +
Shouldn't that be a false?
> /*
> * Hidden service does not have a connect timeout so
> * we do not need to remove it. We can just return
> @@ -1389,6 +1391,8 @@ int connman_network_connect_hidden(struct
> connman_network *network,
>
> DBG("network %p service %p user_data %p", network, service, user_data);
>
> + __connman_set_wifi_autoconnect(true);
> +
> if (!service)
> return -EINVAL;
>
> diff --git a/src/service.c b/src/service.c
> index f6a76f6..5fdff4b 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -48,6 +48,7 @@ static unsigned int autoconnect_timeout = 0;
> static unsigned int vpn_autoconnect_timeout = 0;
> static struct connman_service *current_default = NULL;
> static bool services_dirty = false;
> +static bool wifi_autoconnect = true;
>
> struct connman_stats {
> bool valid;
> @@ -3764,6 +3765,9 @@ static bool auto_connect_service(GList *services,
>
> ignore[CONNMAN_SERVICE_TYPE_VPN] = true;
>
> + if (!wifi_autoconnect)
> + ignore[CONNMAN_SERVICE_TYPE_WIFI] = true;
What about the other service types. With your patch we would still try
to auto connect anything besides wifi. I think you need to be very
careful here. That is pretty tricky code and this looks like it is going
to do funny things.
Maybe you should just block the auto connect requests and unblock it
later. So don't through them away. Why? Because it might be that in the
meantime when you try to connect a hidden wifi network we have a
different source for calling auto connect. If you just block it and
unblock you don't regress anything.
Yeah, better would be to this not in auto_connect_service() instead
in __connman_service_auto_connect().
> +
> for (list = services; list; list = list->next) {
> service = list->data;
>
> @@ -5921,12 +5925,19 @@ static void prepare_8021x(struct connman_service
> *service)
> service->phase2);
> }
>
> +void __connman_set_wifi_autoconnect(bool autoconnect)
> +{
> + wifi_autoconnect = autoconnect;
> +}
> +
> static int service_connect(struct connman_service *service)
> {
> int err;
>
> - if (service->hidden)
> + if (service->hidden) {
> + wifi_autoconnect = false;
> return -EPERM;
> + }
>
> switch (service->type) {
> case CONNMAN_SERVICE_TYPE_UNKNOWN:
>
cheers,
daniel
------------------------------
Message: 4
Date: Tue, 08 Nov 2016 19:16:34 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected], Saurav Babu <[email protected]>
Subject: [PATCHv2] service: Block auto connect when hidden connection
is in progress
Message-ID: <[email protected]>
In the following scenario:
1. Device is connected to AP in non hidden mode.
2. Connection is initated to AP in hidden mode.
connman disconnects old service and tries to connect to new service. But
for connecting hidden service it first scans the hidden AP and when
network is added for hidden AP then connection is tried. In the meantime
normal AP got disconnected and was tried to autoconnect even before
hidden AP was scanned successfully.
This patch blocks autoconnection when hidden connection is in
progress and unblocks autoconnection after hidden connection is started
or hidden wifi scan is completed and hidden AP is not found in the
scan list.
---
v2: Renamed service internal API to have prefix __connman_service
Block auto connect for all service types instead of only WiFi
Allow auto connect when hidden connection failed with error
src/connman.h | 2 ++
src/network.c | 4 ++++
src/service.c | 17 +++++++++++++++--
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index ce3ef8d..945518b 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -796,6 +796,8 @@ void __connman_service_notify(struct connman_service
*service,
int __connman_service_counter_register(const char *counter);
void __connman_service_counter_unregister(const char *counter);
+void __connman_service_enable_autoconnect(bool autoconnect);
+
#include <connman/peer.h>
int __connman_peer_init(void);
diff --git a/src/network.c b/src/network.c
index 2e423bc..e24970d 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1370,6 +1370,8 @@ void connman_network_clear_hidden(void *user_data)
DBG("user_data %p", user_data);
+ __connman_service_enable_autoconnect(true);
+
/*
* Hidden service does not have a connect timeout so
* we do not need to remove it. We can just return
@@ -1389,6 +1391,8 @@ int connman_network_connect_hidden(struct connman_network
*network,
DBG("network %p service %p user_data %p", network, service, user_data);
+ __connman_service_enable_autoconnect(true);
+
if (!service)
return -EINVAL;
diff --git a/src/service.c b/src/service.c
index f6a76f6..a3b6d90 100644
--- a/src/service.c
+++ b/src/service.c
@@ -48,6 +48,7 @@ static unsigned int autoconnect_timeout = 0;
static unsigned int vpn_autoconnect_timeout = 0;
static struct connman_service *current_default = NULL;
static bool services_dirty = false;
+static bool allow_autoconnect = true;
struct connman_stats {
bool valid;
@@ -3829,6 +3830,9 @@ static gboolean run_auto_connect(gpointer data)
DBG("");
+ if (!allow_autoconnect)
+ return FALSE;
+
preferred_tech = preferred_tech_list_get();
if (preferred_tech) {
autoconnecting = auto_connect_service(preferred_tech, reason,
@@ -4060,8 +4064,10 @@ static DBusMessage *connect_service(DBusConnection *conn,
service->pending = NULL;
}
- if (err < 0)
+ if (err < 0) {
+ allow_autoconnect = true;
return __connman_error_failed(msg, -err);
+ }
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
@@ -5921,12 +5927,19 @@ static void prepare_8021x(struct connman_service
*service)
service->phase2);
}
+void __connman_service_enable_autoconnect(bool autoconnect)
+{
+ allow_autoconnect = autoconnect;
+}
+
static int service_connect(struct connman_service *service)
{
int err;
- if (service->hidden)
+ if (service->hidden) {
+ allow_autoconnect = false;
return -EPERM;
+ }
switch (service->type) {
case CONNMAN_SERVICE_TYPE_UNKNOWN:
--
1.9.1
------------------------------
Message: 5
Date: Tue, 08 Nov 2016 19:31:35 +0530
From: Saurav Babu <[email protected]>
To: [email protected]
Cc: [email protected], Saurav Babu <[email protected]>
Subject: [PATCH] service: Block wifi autoconnection when hidden
connection is in progress
Message-ID: <[email protected]>
Hi Daniel,
>> In the following scenario:
>> 1. Device is connected to AP in non hidden mode.
>> 2. Connection is initated to AP in hidden mode.
>> connman disconnects old service and tries to connect to new service. But
>> for connecting hidden service it first scans the hidden AP and when
>> network is added for hidden AP then connection is tried. In the meantime
>> normal AP got disconnected and was tried to autoconnect even before
>> hidden AP was scanned successfully.
>> Ideally non hidden AP should not be autoconnected when hidden AP connection
>> is in progress.
>> This patch blocks wifi autoconnection when hidden connection is in
>> progress and unblocks autoconnection after hidden connection is started
>> or hidden wifi scan is completed and hidden AP is not found in the scan list.
>> ---
>> src/connman.h | 2 ++
>> src/network.c | 4 ++++
>> src/service.c | 13 ++++++++++++-
>> 3 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/connman.h b/src/connman.h
>> index ce3ef8d..24461b1 100644
>> --- a/src/connman.h
>> +++ b/src/connman.h
>> @@ -796,6 +796,8 @@ void __connman_service_notify(struct connman_service
>> *service,
>> int __connman_service_counter_register(const char *counter);
>> void __connman_service_counter_unregister(const char *counter);
>>
>> +void __connman_set_wifi_autoconnect(bool autoconnect);
>> +
> Since this function belongs to the service internal API it should
> have the prefix __connman_service. Furthermore, I rather have a
> textual hint what it does. Usually a plain bool is easy to get
> wrong. So what about
> __connman_service_{enable|disable}_autoconnect()
> ?
> Something sligthly more hacky would be to overload
> __connman_service_auto_connect() with a reason code like
> CONNMAN_SERVICE_CONNECT_REASON_BLOCK. But that is also a bit
> ugly. Maybe the above is better.
I've updated function name as __connman_service_enable_autoconnect().
>> #include <connman/peer.h>
>>
>> int __connman_peer_init(void);
>> diff --git a/src/network.c b/src/network.c
>> index 2e423bc..eccb2ef 100644
>> --- a/src/network.c
>> +++ b/src/network.c
>> @@ -1370,6 +1370,8 @@ void connman_network_clear_hidden(void *user_data)
>>
>> DBG("user_data %p", user_data);
>>
>> + __connman_set_wifi_autoconnect(true);
>> +
> Shouldn't that be a false?
It should be true because I've to enable auto connect after hidden connection
process is finished.
>> /*
>> * Hidden service does not have a connect timeout so
>> * we do not need to remove it. We can just return
>> @@ -1389,6 +1391,8 @@ int connman_network_connect_hidden(struct
>> connman_network *network,
>>
>> DBG("network %p service %p user_data %p", network, service, user_data);
>>
>> + __connman_set_wifi_autoconnect(true);
>> +
>> if (!service)
>> return -EINVAL;
>>
>> diff --git a/src/service.c b/src/service.c
>> index f6a76f6..5fdff4b 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -48,6 +48,7 @@ static unsigned int autoconnect_timeout = 0;
>> static unsigned int vpn_autoconnect_timeout = 0;
>> static struct connman_service *current_default = NULL;
>> static bool services_dirty = false;
>> +static bool wifi_autoconnect = true;
>>
>> struct connman_stats {
>> bool valid;
>> @@ -3764,6 +3765,9 @@ static bool auto_connect_service(GList *services,
>>
>> ignore[CONNMAN_SERVICE_TYPE_VPN] = true;
>>
>> + if (!wifi_autoconnect)
>> + ignore[CONNMAN_SERVICE_TYPE_WIFI] = true;
> What about the other service types. With your patch we would still try
> to auto connect anything besides wifi. I think you need to be very
> careful here. That is pretty tricky code and this looks like it is going
> to do funny things.
> Maybe you should just block the auto connect requests and unblock it
> later. So don't through them away. Why? Because it might be that in the
> meantime when you try to connect a hidden wifi network we have a
> different source for calling auto connect. If you just block it and
> unblock you don't regress anything.
I've updated patch to block auto connect for all service types instead of
just CONNMAN_SERVICE_TYPE_WIFI.
> Yeah, better would be to this not in auto_connect_service() instead
> in __connman_service_auto_connect().
__connman_service_auto_connect() gets called just after initiating disconnection
before hidden/normal wifi connection is started from set_disconnected() function
in __connman_network_disconnect(). Due to this reason I've added this change in
run_auto_connect().
>> +
>> for (list = services; list; list = list->next) {
>> service = list->data;
>>
>> @@ -5921,12 +5925,19 @@ static void prepare_8021x(struct connman_service
>> *service)
>> service->phase2);
>> }
>>
>> +void __connman_set_wifi_autoconnect(bool autoconnect)
>> +{
>> + wifi_autoconnect = autoconnect;
>> +}
>> +
>> static int service_connect(struct connman_service *service)
>> {
>> int err;
>>
>> - if (service->hidden)
>> + if (service->hidden) {
>> + wifi_autoconnect = false;
>> return -EPERM;
>> + }
>>
>> switch (service->type) {
>> case CONNMAN_SERVICE_TYPE_UNKNOWN:
>>
Thanks,
Saurav
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 13, Issue 6
**************************************
