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: [PATCH 3/5] service: Remove existing VPN auto connect
from main loop. (Jussi Laakkonen)
2. Re: [PATCH 4/5] service: Implement a do_auto_connect() to
include VPNs in autoconnect. (Jussi Laakkonen)
3. Re: [PATCH 0/3] Improve VPN state transitions when
re-connecting (Daniel Wagner)
4. [PATCH v2 0/5] Include VPNs in service.c autoconnect process.
(Jussi Laakkonen)
5. [PATCH v2 3/5] service: Remove existing VPN auto connect from
main loop. (Jussi Laakkonen)
6. [PATCH v2 4/5] service: Implement a do_auto_connect() to
include VPNs in autoconnect. (Jussi Laakkonen)
----------------------------------------------------------------------
Message: 1
Date: Wed, 24 Apr 2019 11:26:20 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 3/5] service: Remove existing VPN auto connect
from main loop.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 4/24/19 10:34 AM, Daniel Wagner wrote:
> Hi Jussi,
>
> On 4/23/19 5:22 PM, Jussi Laakkonen wrote:
>> To ensure better autoconnect for VPNs in case there is a network change
>> remove the existing autoconnect from main loop. Otherwise it may be that
>> VPN autoconnect would be still running with 10s delays blocking the
>> re-connection after new network has become online.
>> ---
>> ? src/service.c | 12 ++++++++++--
>> ? 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/service.c b/src/service.c
>> index 4697b055..388f6134 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -4321,8 +4321,16 @@ out:
>> ? static void vpn_auto_connect(void)
>> ? {
>> -??? if (vpn_autoconnect_id)
>> -??????? return;
>> +??? /*
>> +???? * Remove existing autoconnect from main loop to reset the attempt
>> +???? * counter in order to get VPN connected when there is a network
>> change.
>> +???? */
>> +??? if (vpn_autoconnect_id) {
>> +??????? if (!g_source_remove(vpn_autoconnect_id)) {
>> +??????????? DBG("cannot remove VPN autoconnect from main loop");
>
> I don't know if we should log this. It is not an error, it's just that
> the other code path was faster. What do you think?
>
Point taken. I'll remove this and send an updated patch later today.
>> +??????????? return;
>> +??????? }
>> +??? }
>> ????? vpn_autoconnect_id =
>> ????????? g_idle_add(run_vpn_auto_connect, NULL);
>>
>
> Thanks,
> Daniel
Cheers,
Jussi
------------------------------
Message: 2
Date: Wed, 24 Apr 2019 11:29:00 +0300
From: Jussi Laakkonen <[email protected]>
To: Daniel Wagner <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 4/5] service: Implement a do_auto_connect() to
include VPNs in autoconnect.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daniel,
On 4/24/19 10:42 AM, Daniel Wagner wrote:
> Hi Jussi,
>
> On 4/23/19 5:22 PM, Jussi Laakkonen wrote:
>> This commit adds a function to include VPNs in service.c internal
>> autoconnect processes. For other than VPN services it calls
>> __connman_service_auto_connect(), use of which is replaced with use of
>> do_auto_connect() in service.c, and with VPNs vpn_auto_connect() is
>> called. Main reason is that VPN autoconnection is not working with
>> run_auto_connect() and must be triggered with this kind of approach.
>
> Hmm, I re-read this several times but still don't understand what you
> trying to say. Would you mind to try to rewrite this a bit? Thanks!
>
Will do. Tired eyes do not spot the confusion.
>> For VPNs connection reason has to be set as user/auto. VPNs should not
>> be kept reconnecting if the VPN is in failure state, unless the user has
>> requested a connection. In case the network conditions cause the failure
>> other autoconnection mechanisms do trigger re-connection of VPN, which
>> rely on correct state reporting from connman-vpnd.
>> ---
>> ? src/service.c | 47 +++++++++++++++++++++++++++++++++++++++++------
>> ? 1 file changed, 41 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/service.c b/src/service.c
>> index 388f6134..ea6246d5 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -148,6 +148,7 @@ static struct connman_ipconfig
>> *create_ip4config(struct connman_service *service
>> ? static struct connman_ipconfig *create_ip6config(struct
>> connman_service *service,
>> ????????? int index);
>> ? static void dns_changed(struct connman_service *service);
>> +static void vpn_auto_connect(void);
>> ? struct find_data {
>> ????? const char *path;
>> @@ -3413,6 +3414,38 @@ error:
>> ????? return -EINVAL;
>> ? }
>> +static void do_auto_connect(struct connman_service *service,
>> +??? enum connman_service_connect_reason reason)
>> +{
>> +??? /*
>> +???? * CONNMAN_SERVICE_CONNECT_REASON_NONE must be ignored for VPNs.
>> VPNs
>> +???? * always have reason CONNMAN_SERVICE_CONNECT_REASON_USER/AUTO.
>> +???? */
>> +??? if (!service || (service->type == CONNMAN_SERVICE_TYPE_VPN &&
>> +??????? reason == CONNMAN_SERVICE_CONNECT_REASON_NONE))
>
> add an extra tab here, so it doesn't allign with the return.
>
Oops, will fix. Thanks for spotting this.
>> +??????? return;
>> +
>> +??? /*
>> +???? * Run service auto connect when a service is changed. This is not
>> +???? * needed to be run when VPN service changes, then vpn auto
>> connect is
>> +???? * sufficient.
>> +???? *
>> +???? * Do not run vpn_auto_connect() when VPN entering failure state
>> +???? * triggers this via service_complete() that is called for each
>> service
>> +???? * that is set to failure state in service_indicate_state().
>> Otherwise
>> +???? * the timeout of vpn_auto_connect() will be reset and the delays
>> for
>> +???? * VPN connections are reset likewise. User request should reset the
>> +???? * timer.
>> +???? */
>
> I think it would be better to move this info into the commit message,
> because it refernces a lot of other code places. This tends to go stale
> after a while because no one is updating comments.
>
Yeah, now I've re-read that as well the comment above would be best
placed in the commit message and have shorter notes in the code. I'll
modify this and send updated patch later today.
>> +??? if (service->type != CONNMAN_SERVICE_TYPE_VPN)
>> +??????? __connman_service_auto_connect(reason);
>> +??? else if (service->state == CONNMAN_SERVICE_STATE_FAILURE &&
>> +??????? reason != CONNMAN_SERVICE_CONNECT_REASON_USER)
>> +??????? return;
>> +
>> +??? vpn_auto_connect();
>> +}
>> +
>> ? int __connman_service_reset_ipconfig(struct connman_service *service,
>> ????????? enum connman_ipconfig_type type, DBusMessageIter *array,
>> ????????? enum connman_service_state *new_state)
>> @@ -3477,7 +3510,7 @@ int __connman_service_reset_ipconfig(struct
>> connman_service *service,
>> ????????? settings_changed(service, new_ipconfig);
>> ????????? address_updated(service, type);
>> -
>> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> +??????? do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> ????? }
>> ????? DBG("err %d ipconfig %p type %d method %d state %s", err,
>> @@ -3555,7 +3588,8 @@ static DBusMessage *set_property(DBusConnection
>> *conn,
>> ????????? autoconnect_changed(service);
>> ????????? if (autoconnect)
>> -
>> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> +??????????? do_auto_connect(service,
>> +??????????????????? CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> ????????? service_save(service);
>> ????? } else if (g_str_equal(name, "Nameservers.Configuration")) {
>> @@ -3881,7 +3915,7 @@ static void service_complete(struct
>> connman_service *service)
>> ????? reply_pending(service, EIO);
>> ????? if (service->connect_reason != CONNMAN_SERVICE_CONNECT_REASON_USER)
>> -??????? __connman_service_auto_connect(service->connect_reason);
>> +??????? do_auto_connect(service, service->connect_reason);
>> ????? g_get_current_time(&service->modified);
>> ????? service_save(service);
>> @@ -4433,7 +4467,7 @@ static gboolean connect_timeout(gpointer user_data)
>> ????? if (autoconnect &&
>> ????????????? service->connect_reason !=
>> ????????????????? CONNMAN_SERVICE_CONNECT_REASON_USER)
>> -
>> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> +??????? do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> ????? return FALSE;
>> ? }
>> @@ -5937,7 +5971,7 @@ static int service_indicate_state(struct
>> connman_service *service)
>> ?????????? */
>> ????????? downgrade_connected_services();
>> -
>> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> +??????? do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> ????????? break;
>> ????? case CONNMAN_SERVICE_STATE_FAILURE:
>> @@ -7288,7 +7322,8 @@ struct connman_service *
>> __connman_service_create_from_network(struct connman_ne
>> ????????????? case CONNMAN_SERVICE_TYPE_VPN:
>> ????????????? case CONNMAN_SERVICE_TYPE_WIFI:
>> ????????????? case CONNMAN_SERVICE_TYPE_CELLULAR:
>> -
>> __connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> +??????????????? do_auto_connect(service,
>> +??????????????????? CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> ????????????????? break;
>> ????????????? }
>> ????????? }
>>
>
> Thanks,
> Daniel
Cheers,
Jussi
------------------------------
Message: 3
Date: Wed, 24 Apr 2019 10:46:18 +0200
From: Daniel Wagner <[email protected]>
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH 0/3] Improve VPN state transitions when
re-connecting
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8
> All three patches applied.
Forgot to push. will do this later (when back at my computer)... Sorry
------------------------------
Message: 4
Date: Wed, 24 Apr 2019 12:46:12 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH v2 0/5] Include VPNs in service.c autoconnect process.
Message-ID: <[email protected]>
Changes since v2:
* Patch 0/5: Update "Fourth" description.
* Patch 3/5: Remove unnecessary debug logging.
* Patch 4/5:
* Conform to formatting rules.
* Modify comments to be more simple and put more info to commit msg.
* Update commit message to be more coherent and understandable.
This set of patches includes VPN connections into service.c autoconnect
procedure. The general autoconnection of VPNs is improved and is
triggered also when the default service changes.
First, run_vpn_auto_connect() is enhanced to re-add itself to mainloop
if there are VPNs with autoconnect set. Delay increases every 30s by 1s
up to 10s if the connection does not succeed, starting with 1s delay
after initial.
Second, the service state is indicated when automatically triggered
connection succeeds in order to get the state processed also by
plugins/vpn.c.
Third, the enhanced run_vpn_auto_connect() is required to be removed
from mainloop if running and there is a change in network conditions.
It is needed because with increasing delays the VPN connection may
have delay set in 10s and with changing network conditions the
connection should happen more quickly.
Fourth, service auto connection in service.c is going through new
do_auto_connect() function. It starts service auto connection
( __connman_service_auto_connect()) for other than VPN type services
and VPN auto connection (vpn_auto_connect()) is started for all types
of services. Main reason for adding this new function is that
auto_connect_service() ignores VPN type services.
Fifth, when default service changes and the new default is not VPN it
is set to trigger vpn_auto_connect() to ensure attempt of VPN
autoconnection in case networking conditions change.
Jussi Laakkonen (5):
service: Re-add run_vpn_auto_connect() with increasing delays to main
loop
service: Indicate service state when autoconnect succeeds.
service: Remove existing VPN auto connect from main loop.
service: Implement a do_auto_connect() to include VPNs in autoconnect.
service: Call vpn_auto_connect() when default service changes.
src/service.c | 143 +++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 130 insertions(+), 13 deletions(-)
--
2.20.1
------------------------------
Message: 5
Date: Wed, 24 Apr 2019 12:46:13 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH v2 3/5] service: Remove existing VPN auto connect from
main loop.
Message-ID: <[email protected]>
To ensure better autoconnect for VPNs in case there is a network change
remove the existing autoconnect from main loop. Otherwise it may be that
VPN autoconnect would be still running with 5s delays blocking the
re-connection after new network has become online.
---
Changes since v2:
* Remove unnecessary debug logging.
src/service.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/service.c b/src/service.c
index 4697b055..e25284d7 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4321,8 +4321,14 @@ out:
static void vpn_auto_connect(void)
{
- if (vpn_autoconnect_id)
- return;
+ /*
+ * Remove existing autoconnect from main loop to reset the attempt
+ * counter in order to get VPN connected when there is a network change.
+ */
+ if (vpn_autoconnect_id) {
+ if (!g_source_remove(vpn_autoconnect_id))
+ return;
+ }
vpn_autoconnect_id =
g_idle_add(run_vpn_auto_connect, NULL);
--
2.20.1
------------------------------
Message: 6
Date: Wed, 24 Apr 2019 12:46:14 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH v2 4/5] service: Implement a do_auto_connect() to
include VPNs in autoconnect.
Message-ID: <[email protected]>
This commit adds a function to include VPNs in service.c internal
autoconnect processes. Use of do_auto_connect() should be preferred
within service.c instead of __connman_service_auto_connect(). Reason is
that auto_connect_service(), which is called by run_auto_connect(),
ignores VPN services when connecting a service using preferred tech
list.
Function do_auto_connect() starts both service and VPN auto connection
in case the service is not VPN type service. For a VPN type service
running vpn_auto_connect() is sufficient and connection reason has to
be set as user/auto.
VPN services should not be kept reconnecting if the VPN is in failure
state unless the user has requested a connection. This is because
do_auto_connect() can be triggered via service_complete() by an VPN that
is entering failure state in service_indicate_state(). Otherwise the
timeout of vpn_auto_connect() will be reset and the delays for VPN
connections are being reset likewise. It must be user's request that
resets the timer. In case the network conditions cause the failure other
autoconnection mechanisms do trigger re-connection of VPN, which rely on
correct state reporting from connman-vpnd.
---
Changes since v2:
* Conform to formatting rules.
* Modify comments to be more simple and put more info to commit message.
* Update commit message to be more coherent and understandable.
src/service.c | 41 +++++++++++++++++++++++++++++++++++------
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/src/service.c b/src/service.c
index e25284d7..a1b0d5b9 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3413,6 +3413,33 @@ error:
return -EINVAL;
}
+static void vpn_auto_connect(void);
+
+static void do_auto_connect(struct connman_service *service,
+ enum connman_service_connect_reason reason)
+{
+ /*
+ * CONNMAN_SERVICE_CONNECT_REASON_NONE must be ignored for VPNs. VPNs
+ * always have reason CONNMAN_SERVICE_CONNECT_REASON_USER/AUTO.
+ */
+ if (!service || (service->type == CONNMAN_SERVICE_TYPE_VPN &&
+ reason == CONNMAN_SERVICE_CONNECT_REASON_NONE))
+ return;
+
+ /*
+ * Run service auto connect for other than VPN services. Afterwards
+ * start also VPN auto connect process.
+ */
+ if (service->type != CONNMAN_SERVICE_TYPE_VPN)
+ __connman_service_auto_connect(reason);
+ /* Only user interaction should get VPN connected in failure state. */
+ else if (service->state == CONNMAN_SERVICE_STATE_FAILURE &&
+ reason != CONNMAN_SERVICE_CONNECT_REASON_USER)
+ return;
+
+ vpn_auto_connect();
+}
+
int __connman_service_reset_ipconfig(struct connman_service *service,
enum connman_ipconfig_type type, DBusMessageIter *array,
enum connman_service_state *new_state)
@@ -3477,7 +3504,7 @@ int __connman_service_reset_ipconfig(struct
connman_service *service,
settings_changed(service, new_ipconfig);
address_updated(service, type);
-
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
+ do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
}
DBG("err %d ipconfig %p type %d method %d state %s", err,
@@ -3555,7 +3582,8 @@ static DBusMessage *set_property(DBusConnection *conn,
autoconnect_changed(service);
if (autoconnect)
-
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
+ do_auto_connect(service,
+ CONNMAN_SERVICE_CONNECT_REASON_AUTO);
service_save(service);
} else if (g_str_equal(name, "Nameservers.Configuration")) {
@@ -3881,7 +3909,7 @@ static void service_complete(struct connman_service
*service)
reply_pending(service, EIO);
if (service->connect_reason != CONNMAN_SERVICE_CONNECT_REASON_USER)
- __connman_service_auto_connect(service->connect_reason);
+ do_auto_connect(service, service->connect_reason);
g_get_current_time(&service->modified);
service_save(service);
@@ -4431,7 +4459,7 @@ static gboolean connect_timeout(gpointer user_data)
if (autoconnect &&
service->connect_reason !=
CONNMAN_SERVICE_CONNECT_REASON_USER)
-
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
+ do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
return FALSE;
}
@@ -5935,7 +5963,7 @@ static int service_indicate_state(struct connman_service
*service)
*/
downgrade_connected_services();
-
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
+ do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
break;
case CONNMAN_SERVICE_STATE_FAILURE:
@@ -7286,7 +7314,8 @@ struct connman_service *
__connman_service_create_from_network(struct connman_ne
case CONNMAN_SERVICE_TYPE_VPN:
case CONNMAN_SERVICE_TYPE_WIFI:
case CONNMAN_SERVICE_TYPE_CELLULAR:
-
__connman_service_auto_connect(CONNMAN_SERVICE_CONNECT_REASON_AUTO);
+ do_auto_connect(service,
+ CONNMAN_SERVICE_CONNECT_REASON_AUTO);
break;
}
}
--
2.20.1
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 42, Issue 19
***************************************
