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. Re: openvpn plugin does not set a valid netmask and so a
valid routing table (Daniel Wagner)
2. [PATCH] Remove unnecessary continue in set_tethering API
(Milind Murhekar)
3. [PATCH] service: Disconnect old service only when connecting
new service (Saurav Babu)
4. Re: [PATCH] service: Disconnect old service only when
connecting new service (Patrik Flykt)
5. Re: [PATCH] Remove unnecessary continue in set_tethering API
(Patrik Flykt)
6. Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove() (Patrik Flykt)
7. Re: [PATCH] service: Disconnect old service only when
connecting new service (Daniel Wagner)
8. Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove() (Daniel Wagner)
9. Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove() (Patrik Flykt)
----------------------------------------------------------------------
Message: 1
Date: Fri, 28 Oct 2016 08:16:28 +0200
From: Daniel Wagner <[email protected]>
To: Michele Dionisio <[email protected]>,
[email protected]
Subject: Re: openvpn plugin does not set a valid netmask and so a
valid routing table
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Michele,
On 10/27/2016 08:54 PM, Michele Dionisio wrote:
> Hi,
> The openvpn plugin does not honor the netmask received from the server
> and so connman does not set a valid routing table. The patch attached
> solve the issue.
Thanks for your patch. I had to fix it manually up though. Please use
git send-email so that there are no whitespace damages (indention and
line endings were broken). And while you are at it, also write a nice
commit message, this makes my life much easier as maintainer.
Nevertheless thanks for taking the time to track the bug down and
provide a fix.
The fix is applied and pushed.
Thanks,
Daniel
------------------------------
Message: 2
Date: Fri, 28 Oct 2016 12:11:56 +0530
From: Milind Murhekar <[email protected]>
To: [email protected]
Cc: [email protected], Milind Murhekar <[email protected]>
Subject: [PATCH] Remove unnecessary continue in set_tethering API
Message-ID: <[email protected]>
---
src/technology.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/technology.c b/src/technology.c
index 17ed456..4574f1e 100644
--- a/src/technology.c
+++ b/src/technology.c
@@ -270,10 +270,8 @@ static int set_tethering(struct connman_technology
*technology,
if (result == -EINPROGRESS)
continue;
- if (err == -EINPROGRESS || err == 0) {
+ if (err == -EINPROGRESS || err == 0)
result = err;
- continue;
- }
}
return result;
--
1.9.1
------------------------------
Message: 3
Date: Fri, 28 Oct 2016 12:20:18 +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,
>> 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?
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.
> 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.
> 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.
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).
Thanks,
Saurav
------------------------------
Message: 4
Date: Fri, 28 Oct 2016 10:26:24 +0300
From: Patrik Flykt <[email protected]>
To: Saurav Babu <[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] service: Disconnect old service only when
connecting new service
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
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
> [email protected]
> https://lists.01.org/mailman/listinfo/connman
------------------------------
Message: 5
Date: Fri, 28 Oct 2016 10:27:28 +0300
From: Patrik Flykt <[email protected]>
To: Milind Murhekar <[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Remove unnecessary continue in set_tethering API
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Fri, 2016-10-28 at 12:11 +0530, Milind Murhekar wrote:
> ---
> ?src/technology.c | 4 +---
> ?1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/src/technology.c b/src/technology.c
> index 17ed456..4574f1e 100644
> --- a/src/technology.c
> +++ b/src/technology.c
> @@ -270,10 +270,8 @@ static int set_tethering(struct
> connman_technology *technology,
> ? if (result == -EINPROGRESS)
> ? continue;
> ?
> - if (err == -EINPROGRESS || err == 0) {
> + if (err == -EINPROGRESS || err == 0)
> ? result = err;
> - continue;
> - }
> ? }
Why is the test unnecessary and what is the use case enabled by its
removal?
Cheers,
Patrik
------------------------------
Message: 6
Date: Fri, 28 Oct 2016 10:28:50 +0300
From: Patrik Flykt <[email protected]>
To: Nishant Chaprana <[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove()
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Wed, 2016-10-26 at 18:36 +0530, Nishant Chaprana wrote:
> Signed-off-by: Nishant Chaprana <[email protected]>
> ---
> ?src/service.c | 2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/service.c b/src/service.c
> index ee10e6c..f6a76f6 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -1164,7 +1164,7 @@ int __connman_service_nameserver_remove(struct
> connman_service *service,
> ? if (!nameservers)
> ? return 0;
> ?
> - for (i = 0; nameservers && nameservers[i]; i++)
> + for (i = 0; nameservers[i]; i++)
> ? if (g_strcmp0(nameservers[i], nameserver) == 0) {
> ? found = true;
> ? break;
NACK. nameservers can be NULL as well. Touching nameservers[i] will
cause a segfault.
Patrik
------------------------------
Message: 7
Date: Fri, 28 Oct 2016 09:32:22 +0200
From: Daniel Wagner <[email protected]>
To: Saurav Babu <[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] service: Disconnect old service only when
connecting new service
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
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
------------------------------
Message: 8
Date: Fri, 28 Oct 2016 09:33:42 +0200
From: Daniel Wagner <[email protected]>
To: Patrik Flykt <[email protected]>, Nishant Chaprana
<[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove()
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 10/28/2016 09:28 AM, Patrik Flykt wrote:
> On Wed, 2016-10-26 at 18:36 +0530, Nishant Chaprana wrote:
>> Signed-off-by: Nishant Chaprana <[email protected]>
>> ---
>> src/service.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/service.c b/src/service.c
>> index ee10e6c..f6a76f6 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -1164,7 +1164,7 @@ int __connman_service_nameserver_remove(struct
>> connman_service *service,
>> if (!nameservers)
>> return 0;
>>
>> - for (i = 0; nameservers && nameservers[i]; i++)
>> + for (i = 0; nameservers[i]; i++)
>> if (g_strcmp0(nameservers[i], nameserver) == 0) {
>> found = true;
>> break;
>
> NACK. nameservers can be NULL as well. Touching nameservers[i] will
> cause a segfault.
Isn't the if above the for loop testing this already?
cheers,
daniel
------------------------------
Message: 9
Date: Fri, 28 Oct 2016 10:37:54 +0300
From: Patrik Flykt <[email protected]>
To: Daniel Wagner <[email protected]>, Nishant Chaprana
<[email protected]>, [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Removed redundant NULL check for nameservers in
__connman_service_nameserver_remove()
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
On Fri, 2016-10-28 at 09:33 +0200, Daniel Wagner wrote:
> >>??????if (!nameservers)
> >>??????????????return 0;
Yes, I'm corrected! Now when did this slip in...
Cheers,
Patrik
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 12, Issue 32
***************************************
