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 0/4] VPN tweaks (Slava Monich)
2. [PATCH 1/4] vpn: More informative debug trace (Slava Monich)
3. [PATCH 2/4] vpn: Do not call Disconnect too many times
(Slava Monich)
4. [PATCH 3/4] vpn: Refuse to connect if there's no default
service (Slava Monich)
5. [PATCH 4/4] service: service_indicate_state() doesn't need to
disconnect VPN (Slava Monich)
6. Re: [PATCH v2] doc: fixed description of ServicesChanged and
PeersChanged signal (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Wed, 19 Sep 2018 01:03:14 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: [PATCH 0/4] VPN tweaks
Message-ID: <[email protected]>
These are basically follow-ups to the recent VPN auto-disconnect
changes, on top of the memory leak fix that was sent separately.
Slava Monich (4):
vpn: More informative debug trace
vpn: Do not call Disconnect too many times
vpn: Refuse to connect if there's no default service
service: service_indicate_state() doesn't need to disconnect VPN
plugins/vpn.c | 113 ++++++++++++++++++++++++++++++++++------------------------
src/service.c | 7 ----
2 files changed, 67 insertions(+), 53 deletions(-)
--
1.9.1
------------------------------
Message: 2
Date: Wed, 19 Sep 2018 01:03:15 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: [PATCH 1/4] vpn: More informative debug trace
Message-ID: <[email protected]>
Helps to analyze why VPN gets disconnected.
---
plugins/vpn.c | 30 +++++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index e55c4b8..58e3b94 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -1871,15 +1871,40 @@ static struct connman_service
*vpn_find_online_transport()
static bool vpn_is_valid_transport(struct connman_service *transport)
{
if (transport) {
+ struct connman_service *online;
+
switch (connman_service_get_state(transport)) {
case CONNMAN_SERVICE_STATE_READY:
- return vpn_find_online_transport() == NULL;
+ online = vpn_find_online_transport();
+
+ /* Stay connected if there are no online services */
+ if (!online)
+ return true;
+
+ DBG("%s is ready, %s is online, disconnecting",
+ connman_service_get_identifier(transport),
+ connman_service_get_identifier(online));
+ break;
+
case CONNMAN_SERVICE_STATE_ONLINE:
- return vpn_find_online_transport() == transport;
+ online = vpn_find_online_transport();
+
+ /* Check if our transport is still the default */
+ if (online == transport)
+ return true;
+
+ DBG("%s is replaced by %s as default, disconnecting",
+ connman_service_get_identifier(transport),
+ connman_service_get_identifier(online));
+ break;
+
default:
break;
}
+ } else {
+ DBG("transport gone");
}
+
return false;
}
@@ -1891,7 +1916,6 @@ static void vpn_disconnect_check_provider(struct
connection_data *data)
(data->service_ident);
if (!vpn_is_valid_transport(service)) {
- DBG("transport gone");
disconnect_provider(data);
}
}
--
1.9.1
------------------------------
Message: 3
Date: Wed, 19 Sep 2018 01:03:16 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: [PATCH 2/4] vpn: Do not call Disconnect too many times
Message-ID: <[email protected]>
disconnect_provider() ends up calling iself like this:
disconnect_provider () at plugins/vpn.c
provider_disconnect () at plugins/vpn.c
connman_provider_disconnect () at src/provider.c
provider_service_changed () at src/provider.c
__connman_notifier_service_state_changed () at src/notifier.c
state_changed () at src/service.c
service_indicate_state () at src/service.c
__connman_service_ipconfig_indicate_state () at src/service.c
provider_indicate_state () at src/provider.c
connman_provider_set_state () at src/provider.c
disconnect_provider () at plugins/vpn.c
which resulted in two Disconnect D-Bus calls which is one too many.
connman should at least wait until the previous call has completed.
---
plugins/vpn.c | 56 +++++++++++++++++++++++++++-----------------------------
1 file changed, 27 insertions(+), 29 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 58e3b94..e2824d0 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -72,6 +72,7 @@ struct connection_data {
struct connman_provider *provider;
int index;
DBusPendingCall *call;
+ DBusPendingCall *disconnect_call;
bool connect_pending;
struct config_create_data *cb_data;
char *service_ident;
@@ -919,12 +920,10 @@ static int provider_connect(struct connman_provider
*provider,
static void disconnect_reply(DBusPendingCall *call, void *user_data)
{
+ struct connection_data *data = user_data;
DBusMessage *reply;
DBusError error;
- if (!dbus_pending_call_get_completed(call))
- return;
-
DBG("user %p", user_data);
reply = dbus_pending_call_steal_reply(call);
@@ -939,53 +938,47 @@ static void disconnect_reply(DBusPendingCall *call, void
*user_data)
done:
dbus_message_unref(reply);
-
dbus_pending_call_unref(call);
+ data->disconnect_call = NULL;
}
static int disconnect_provider(struct connection_data *data)
{
- DBusPendingCall *call;
+ bool sent;
DBusMessage *message;
DBG("data %p path %s", data, data->path);
+ if (data->disconnect_call) {
+ DBG("already disconnecting");
+ return -EINVAL;
+ }
+
message = dbus_message_new_method_call(VPN_SERVICE, data->path,
VPN_CONNECTION_INTERFACE,
VPN_DISCONNECT);
if (!message)
return -ENOMEM;
- if (!dbus_connection_send_with_reply(connection, message,
- &call, DBUS_TIMEOUT)) {
+ sent = dbus_connection_send_with_reply(connection, message,
+ &data->disconnect_call, DBUS_TIMEOUT);
+ dbus_message_unref(message);
+
+ if (!sent || !data->disconnect_call) {
connman_error("Unable to call %s.%s()",
VPN_CONNECTION_INTERFACE, VPN_DISCONNECT);
- dbus_message_unref(message);
- return -EINVAL;
- }
-
- if (!call) {
- dbus_message_unref(message);
return -EINVAL;
}
- dbus_pending_call_set_notify(call, disconnect_reply, NULL, NULL);
-
- dbus_message_unref(message);
+ dbus_pending_call_set_notify(data->disconnect_call, disconnect_reply,
+ data, NULL);
g_free(data->service_ident);
data->service_ident = NULL;
connman_provider_set_state(data->provider,
CONNMAN_PROVIDER_STATE_DISCONNECT);
- /*
- * We return 0 here instead of -EINPROGRESS because
- * __connman_service_disconnect() needs to return something
- * to gdbus so that gdbus will not call Disconnect() more
- * than once. This way we do not need to pass the dbus reply
- * message around the code.
- */
- return 0;
+ return -EINPROGRESS;
}
static int provider_disconnect(struct connman_provider *provider)
@@ -1509,13 +1502,8 @@ static void destroy_provider(struct connection_data
*data)
if (provider_is_connected(data))
connman_provider_disconnect(data->provider);
- if (data->call)
- dbus_pending_call_cancel(data->call);
-
connman_provider_set_data(data->provider, NULL);
-
connman_provider_remove(data->provider);
-
data->provider = NULL;
}
@@ -1528,6 +1516,16 @@ static void connection_destroy(gpointer hash_data)
if (data->provider)
destroy_provider(data);
+ if (data->call) {
+ dbus_pending_call_cancel(data->call);
+ dbus_pending_call_unref(data->call);
+ }
+
+ if (data->disconnect_call) {
+ dbus_pending_call_cancel(data->disconnect_call);
+ dbus_pending_call_unref(data->disconnect_call);
+ }
+
g_free(data->service_ident);
g_free(data->path);
g_free(data->ident);
--
1.9.1
------------------------------
Message: 4
Date: Wed, 19 Sep 2018 01:03:17 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: [PATCH 3/4] vpn: Refuse to connect if there's no default
service
Message-ID: <[email protected]>
There's still room for a race condition - the default route can change
by the time when VPN binary actually gets connected, but his code should
be able to recover from that (by reconnecting VPN)
---
plugins/vpn.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index e2824d0..3d067a4 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -536,6 +536,11 @@ static int connect_provider(struct connection_data *data,
void *user_data,
DBG("data %p user %p path %s sender %s", data, cb_data, data->path,
dbus_sender);
+ if (!transport) {
+ DBG("no default service, refusing to connect");
+ return -EINVAL;
+ }
+
data->connect_pending = false;
#define VPN_CONNECT2 "Connect2"
@@ -573,20 +578,14 @@ static int connect_provider(struct connection_data *data,
void *user_data,
cb_data->path = g_strdup(data->path);
}
- if (transport) {
- /*
- * This is the service which (most likely) will be used
- * as a transport for VPN connection.
- */
- g_free(data->service_ident);
- data->service_ident =
- g_strdup(connman_service_get_identifier(transport));
- DBG("transport %s", data->service_ident);
- } else {
- DBG("no transport????");
- g_free(data->service_ident);
- data->service_ident = NULL;
- }
+ /*
+ * This is the service which (most likely) will be used
+ * as a transport for VPN connection.
+ */
+ g_free(data->service_ident);
+ data->service_ident =
+ g_strdup(connman_service_get_identifier(transport));
+ DBG("transport %s", data->service_ident);
dbus_pending_call_set_notify(call, connect_reply, data, NULL);
--
1.9.1
------------------------------
Message: 5
Date: Wed, 19 Sep 2018 01:03:18 +0300
From: Slava Monich <[email protected]>
To: [email protected]
Subject: [PATCH 4/4] service: service_indicate_state() doesn't need to
disconnect VPN
Message-ID: <[email protected]>
VPN plugin now disconnects the provider when it finds that appropriate.
---
src/service.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/src/service.c b/src/service.c
index f0234a6..eea8235 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5770,13 +5770,6 @@ static int service_indicate_state(struct connman_service
*service)
reply_pending(service, ECONNABORTED);
- def_service = connman_service_get_default();
-
- if (!__connman_notifier_is_connected() &&
- def_service &&
- def_service->provider)
- connman_provider_disconnect(def_service->provider);
-
default_changed();
__connman_wispr_stop(service);
--
1.9.1
------------------------------
Message: 6
Date: Wed, 19 Sep 2018 08:05:26 +0200
From: Daniel Wagner <[email protected]>
To: Vasyl Vavrychuk <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2] doc: fixed description of ServicesChanged and
PeersChanged signal
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Vasyl,
On 09/10/2018 11:37 PM, Vasyl Vavrychuk wrote:
> Previous version of documentation makes a feel that first argument of
> ServicesChanged and PeersChanged signals contains only changed or added
> services/peers while in fact it contains all services/peers.
I reformatted it slightly so it fits int the 80 chars limit.
Patch applied.
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 35, Issue 9
**************************************
