3:09 a.m.
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 only be disconnected when hidden AP is
found in the scan list and its connection is intiated.
---
src/service.c | 44 ++++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/service.c b/src/service.c
index f6a76f6..7d3847e 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4015,33 +4015,13 @@ static DBusMessage *connect_service(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
struct connman_service *service = user_data;
- int index, err = 0;
- GList *list;
+ int err = 0;
DBG("service %p", service);
if (service->pending)
return __connman_error_in_progress(msg);
- index = __connman_service_get_index(service);
-
- for (list = service_list; list; list = list->next) {
- struct connman_service *temp = list->data;
-
- if (!is_connecting(temp) && !is_connected(temp))
- break;
-
- if (service == temp)
- continue;
-
- if (service->type != temp->type)
- continue;
-
- if (__connman_service_get_index(temp) == index &&
- __connman_service_disconnect(temp) == -EINPROGRESS)
- err = -EINPROGRESS;
-
- }
if (err == -EINPROGRESS)
return __connman_error_operation_timeout(msg);
@@ -5923,11 +5903,31 @@ static void prepare_8021x(struct connman_service *service)
static int service_connect(struct connman_service *service)
{
- int err;
+ int err, index;
+ GList *list;
if (service->hidden)
return -EPERM;
+ index = __connman_service_get_index(service);
+
+ for (list = service_list; list; list = list->next) {
+ struct connman_service *temp = list->data;
+
+ if (!is_connecting(temp) && !is_connected(temp))
+ break;
+
+ if (service == temp)
+ continue;
+
+ if (service->type != temp->type)
+ continue;
+
+ if (__connman_service_get_index(temp) == index &&
+ __connman_service_disconnect(temp) == -EINPROGRESS)
+ return -ETIMEDOUT;
+ }
+
switch (service->type) {
case CONNMAN_SERVICE_TYPE_UNKNOWN:
case CONNMAN_SERVICE_TYPE_SYSTEM:
--
1.9.1
9:46 a.m.
Hi Saurav,
On 10/27/2016 09:09 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 only be disconnected when hidden AP is
found in the scan list and its connection is intiated.
So the problem is that autoconnect is faster than scan,connect operation?
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?
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.
cheers,
daniel
3:26 a.m.
On Fri, 2016-10-28 at 12:20 +0530, Saurav Babu wrote:
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.
> 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 needs very thorough testing as all the other corner cases covered
by the current code can break. There most likely was a reason that the
test was done only when receiving the D-Bus method call.
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. 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).
So that's the reason behind the change. This should definitely be in
the commit message.
Thanks,
Saurav
_______________________________________________
connman mailing list
connman(a)lists.01.org
https://lists.01.org/mailman/listinfo/connman
3:32 a.m.
Hi Saurav,
On 10/28/2016 08:50 AM, Saurav Babu wrote:
> 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?
cheers,
daniel
1:07 a.m.
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
1452
days inactive
1464
days old
4 comments
3 participants
participants (3)
-
Daniel Wagner -
Patrik Flykt -
Saurav Babu