Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe 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: simpler reproduction of connman error (Daniel Wagner)
2. Re: [PATCH 0/5] Track VPN connection requests and reply only once to one
(Jussi Laakkonen)
3. Re: [PATCH 0/5] Track VPN connection requests and reply only once to one
(Daniel Wagner)
4. Re: [PATCH 0/5] Track VPN connection requests and reply only once to one
(Jussi Laakkonen)
5. [PATCH v2 5/5] service: Clear VPN error and state if AutoConnect is set on
(Jussi Laakkonen)
6. Re: [PATCH v2 5/5] service: Clear VPN error and state if AutoConnect is
set on
(Daniel Wagner)
7. Re: [PATCH v2 5/5] service: Clear VPN error and state if AutoConnect is
set on
(Daniel Wagner)
8. Re: [PATCH 00/10] Rewrite of OpenVPN plugin, VPN agent additions and VPN
provider fixes
(Daniel Wagner)
----------------------------------------------------------------------
Date: Thu, 14 Nov 2019 10:21:56 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: simpler reproduction of connman error
To: Thomas Green <[email protected]>, [email protected]
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Please don't drop the mailing list. This is the last time I'll say
this.
On Wed, Nov 13, 2019 at 05:01:10PM +0000, Thomas Green wrote:
> Here are two logs that capture the problem. One is much longer (we have a
> lot of wireless APs) and one that is a little more focused.
Here is when the config is reloaded triggered by the last connect. As
you can see the gateway is not set.
connmand[2761]: ../connman-1.37/src/ipconfig.c:__connman_ipconfig_load()
ipconfig 0x715e50 identifier ethernet_0008720978d7_cable
connmand[2761]: ../connman-1.37/src/inet.c:connman_inet_set_address() index 3
address 10.20.187.10 prefix_len 24
connmand[2761]: ../connman-1.37/src/inet.c:__connman_inet_modify_address() cmd
0x14 flags 0x104 index 3 family 2 address 10.20.187.10 peer (null) prefixlen 24
broadcast (null)
connmand[2761]: ../connman-1.37/src/ipconfig.c:__connman_ipconfig_gateway_add()
connmand[2761]: ../connman-1.37/src/ipconfig.c:__connman_ipconfig_gateway_add()
type 1 gw (null) peer (null)
connmand[2761]:
../connman-1.37/src/connection.c:__connman_connection_gateway_add() service
0x705850 index 3 gateway 0.0.0.0 vpn ip (null) type 1
connmand[2761]: ../connman-1.37/src/service.c:connman_service_ref_debug()
0x705850 ref 2 by ../connman-1.37/src/connection.c:403:add_gateway()
So no suprise the gateway is not set. The question is why ipconfig is
updated.
I starred for a while at the log but I couldn't really trace what's
really the source of the gateway update which set it's to NULL. I
suggest you instrument the code at __connman_ipconfig_gateway_add(),
__connman_ipconfig_load(), __connman_ipconfig_save() and
__connman_ipconfig_set_gateway(). Also you might want add a breakpoint
with gdb and post the stack trace.
Thanks,
Daniel
------------------------------
Date: Thu, 14 Nov 2019 09:42:34 -0000
From: "Jussi Laakkonen" <[email protected]>
Subject: Re: [PATCH 0/5] Track VPN connection requests and reply only
once to one
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
Hi Daniel,
>
> I've applied patches 1-4. I reply on 5.
>
Thanks.
Waiting for the comment on 5. But the main idea behind 5th patch is that one
could just simply toggle the autoconnect value via D-Bus for a VPN to get it
connecting without even calling Connect() over D-Bus. And when that VPN is in
failure state connection is not attempted if only the autoconnect is toggled.
Furthermore, correct me if I'm wrong, but there can be only one VPN per
physical device in ConnMan, based on the physical -> vpn index relation in
connection.c? That is why autoconnect is with that 5th patch limited to one VPN
connection only.
Cheers,
Jussi
------------------------------
Date: Thu, 14 Nov 2019 11:15:26 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 0/5] Track VPN connection requests and reply only
once to one
To: Jussi Laakkonen <[email protected]>, [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jussi,
On 14.11.19 10:42, Jussi Laakkonen wrote:
> Waiting for the comment on 5.
Grrr, the mailing list did again not send you the response?
> But the main idea behind 5th patch is that one could just simply toggle the
> autoconnect value via D-Bus for a VPN to get it connecting without even
> calling Connect() over D-Bus. And when that VPN is in failure state
> connection is not attempted if only the autoconnect is toggled.
That is okay.
> Furthermore, correct me if I'm wrong, but there can be only one VPN per
> physical device in ConnMan, based on the physical -> vpn index relation in
> connection.c? That is why autoconnect is with that 5th patch limited to one
> VPN connection only.
My question was around this. So something which currently works (IIRC, I
played with it and I remember it worked) was setting up a full VPN
tunnel and having a splitted on top of it. My point is, with splitted
tunnels we could stack them and it makes sense at least for me :)
Thanks,
Daniel
------------------------------
Date: Thu, 14 Nov 2019 10:29:58 -0000
From: "Jussi Laakkonen" <[email protected]>
Subject: Re: [PATCH 0/5] Track VPN connection requests and reply only
once to one
To: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset="utf-8"
>
> Grrr, the mailing list did again not send you the response?
>
Apparently it was delayed a long time as this UI did not show that
message until I posted a reply to the previous one. Now it shows this:
> This makes sense if only a full VPN tunnel is configured. But what
> about the case you have two splittet tunnels? Is this a realistic
> scenario?
>
> My question was around this. So something which currently works (IIRC, I
> played with it and I remember it worked) was setting up a full VPN
> tunnel and having a splitted on top of it. My point is, with splitted
> tunnels we could stack them and it makes sense at least for me :)
>
Ah, we haven't used split tunnels at all. So I have no experience with them.
I think should look at them first about how they work and rethink it a bit. I'm
guessing we have gone into a bit different direction with the fork, as there
is a feature to enable/disable the VPN as default route. It may be that we've
implemented a feature that already existed?
I'll remove that disabling of autoconnect from the 5th patch and send a v2.
Sorry about that.
Cheers,
Jussi
------------------------------
Date: Thu, 14 Nov 2019 13:10:00 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH v2 5/5] service: Clear VPN error and state if
AutoConnect is set on
To: [email protected]
Message-ID: <[email protected]>
Changing AutoConnect value to "true" must have the same effect as user
calling "Connect" method via D-Bus. The failure state is ignored in
do_auto_connect() if user called the VPN to connect.
This fixes the inconsistency with AutoConnect value setting, if user
sets AutoConnect to "true" clear error and set the service state to
idle, if VPN is in failure state. This makes it possible to get a
VPN connecting immediately after toggling the AutoConnect value.
---
Changes since v2:
* Removed function disable_autoconnect_for_services() and its use.
* Small changes to commit message.
src/service.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7e1446b7..cc87ed5c 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3588,6 +3588,9 @@ void __connman_service_wispr_start(struct connman_service
*service,
__connman_wispr_start(service, type);
}
+static void set_error(struct connman_service *service,
+ enum connman_service_error error);
+
static DBusMessage *set_property(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -3625,18 +3628,26 @@ static DBusMessage *set_property(DBusConnection *conn,
dbus_message_iter_get_basic(&value, &autoconnect);
- if (service->autoconnect == autoconnect)
- return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
-
- service->autoconnect = autoconnect;
+ if (autoconnect && service->type == CONNMAN_SERVICE_TYPE_VPN) {
+ /*
+ * Changing the autoconnect flag on VPN to "on" should
+ * have the same effect as user connecting the VPN =
+ * clear previous error and change state to idle.
+ */
+ set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
- autoconnect_changed(service);
+ if (service->state == CONNMAN_SERVICE_STATE_FAILURE) {
+ service->state = CONNMAN_SERVICE_STATE_IDLE;
+ state_changed(service);
+ }
+ }
- if (autoconnect)
- do_auto_connect(service,
+ if (connman_service_set_autoconnect(service, autoconnect)) {
+ service_save(service);
+ if (autoconnect)
+ do_auto_connect(service,
CONNMAN_SERVICE_CONNECT_REASON_AUTO);
-
- service_save(service);
+ }
} else if (g_str_equal(name, "Nameservers.Configuration")) {
DBusMessageIter entry;
GString *str;
--
2.20.1
------------------------------
Date: Thu, 14 Nov 2019 17:26:00 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH v2 5/5] service: Clear VPN error and state if
AutoConnect is set on
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Thu, Nov 14, 2019 at 01:10:00PM +0200, Jussi Laakkonen wrote:
> Changing AutoConnect value to "true" must have the same effect as user
> calling "Connect" method via D-Bus. The failure state is ignored in
> do_auto_connect() if user called the VPN to connect.
>
> This fixes the inconsistency with AutoConnect value setting, if user
> sets AutoConnect to "true" clear error and set the service state to
> idle, if VPN is in failure state. This makes it possible to get a
> VPN connecting immediately after toggling the AutoConnect value.
>
> ---
> Changes since v2:
> * Removed function disable_autoconnect_for_services() and its use.
> * Small changes to commit message.
Thanks,
Daniel
------------------------------
Date: Thu, 14 Nov 2019 17:31:28 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH v2 5/5] service: Clear VPN error and state if
AutoConnect is set on
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
On 14.11.19 17:26, Daniel Wagner wrote:
>> Changes since v2:
>> * Removed function disable_autoconnect_for_services() and its use.
>> * Small changes to commit message.
Patch applied :)
------------------------------
Date: Thu, 14 Nov 2019 18:13:01 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 00/10] Rewrite of OpenVPN plugin, VPN agent
additions and VPN provider fixes
To: Jussi Laakkonen <[email protected]>
Cc: [email protected]
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Jussi,
On Mon, Nov 11, 2019 at 04:01:44PM +0200, Jussi Laakkonen wrote:
> This contains changes to three different components necessary for the OpenVPN
> plugin rewrite. This work is a combined effort of 4 authors.
>
> First, VPN agent is amended with three boolean control values in order to
> allow controlling of credential storing, retrieval and whether to keep the old
> credentials or not. The last one is meant to be used in situations where the
> second credential request is to be done and the second credentials are not to
> be stored, but the setting of AllowStoreCredentials to false should not affect
> to storing of the main credentials. Also, the parametrization in the generic
> requests of vpn-agent.c has been improved to support hiding of the password.
>
> Second, vpn-provider.c is amended to handle ENOENT when connection callback is
> called and to have a function to record an error without creating a signal out
> of it and changing state. This is the case when the authentication credentials
> are handled within the VPN process lifetime and it does not shutdown after the
> credentials are invalid but requests them via other means from plugin. In such
> case it is imperative just to record the error as otherwise signaling about
> the error while the VPN process is still running would have undesired effects.
>
> And last, VPN agent support was implemented to OpenVPN plugin. OpenVPN
> management interface is used to get credential as well as encrypted private
> key password requests. These requests are forwarded to VPN agent. Credential
> request is same as with other VPN plugins but is handled within the OpenVPN
> process lifetime, during which the credentials can be requested multiple times
> without restart. For this reason each authentication error is not need to be
> signaled but only recorded to inform VPN agent. The private key password
> request is also a different case as the private key password is not to be
> stored by the VPN agent. In this case VPN agent is requested not to store or
> retrieve the private key password but is instructed to keep the other (main)
> credentials.
I just tested this and nothing breaks. So I went ahaed and applied all
patches. If there are issues we fix it up in the tree.
Again thanks a lot for upstreaming these gems!
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 49, Issue 19
***************************************
