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. [PATCH 0/5] Track VPN connection requests and reply only once to one
(Jussi Laakkonen)
2. [PATCH 4/5] provider: Handle -EALREADY in __connman_provider_connect()
(Jussi Laakkonen)
3. [PATCH 1/5] vpn-provider: Change provider in failure state to idle when
saving
(Jussi Laakkonen)
4. [PATCH 2/5] vpn-provider: Reply to connect request only once
(Jussi Laakkonen)
5. [PATCH 3/5] vpn: Restrict connection requests to one per connection
(Jussi Laakkonen)
6. [PATCH 5/5] service: Clear VPN error and state if AutoConnect is set on
(Jussi Laakkonen)
----------------------------------------------------------------------
Date: Tue, 12 Nov 2019 16:00:50 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 0/5] Track VPN connection requests and reply only once
to one
To: [email protected]
Message-ID: <[email protected]>
This set of patches addresses the issue of replying a VPN connection
request twice. Also the VPN connection requests are limited to one per
VPN. Connecting a previously failed VPN is also improved.
The plugins/vpn.c is enhanced to keep track of connection requests. If
there is a pending call to connect the VPN report -EALREADY and handle
that in provider.c. This must not create a state change indication but
only inform that the connection is still in progress.
Each connection request must be replied only once and vpn-provider.c:
do_connect() must not reply to the call immediately if -EINPROGRESS is
returned. This is because vpn-provider.c:connect_cb() will send an
appropriate reply later using the VPN plugin return code. If the VPN
is in failure state or is disconnecting a direct reply must be sent as
VPN plugin connect() is not called, thus connect_cb() is not reached.
Also, when a VPN settings are saved the previously set failure state
must be changed to idle. This then follows same approach as it is used
with error counters (reset when settings saved).
The error and state must be also reset, when user changes autoconnect
value for the VPN to true value. This is similar to when connecting a
VPN using D-Bus method Connect(). Without changing the state autoconnect
for the VPN (service.c:do_auto_connect()) does not work as with
CONNMAN_SERVICE_CONNET_REASON_AUTO a VPN in failure state should not
be connected.
Jussi Laakkonen (5):
vpn-provider: Change provider in failure state to idle when saving
vpn-provider: Always reply to connect request D-Bus message
vpn: Restrict connection requests to one per connection
provider: Handle -EALREADY in __connman_provider_connect()
service: Clear VPN error and state if AutoConnect is set on
plugins/vpn.c | 42 ++++++++++++++++++++++++++++++----
src/provider.c | 17 +++++++++-----
src/service.c | 56 +++++++++++++++++++++++++++++++++++++++-------
vpn/vpn-provider.c | 19 +++++++++++++++-
4 files changed, 116 insertions(+), 18 deletions(-)
--
2.20.1
------------------------------
Date: Tue, 12 Nov 2019 16:00:54 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 4/5] provider: Handle -EALREADY in
__connman_provider_connect()
To: [email protected]
Message-ID: <[email protected]>
The error -EALREADY returned by vpn.c:provider_connect() in case there
is already a pending request for connecting the VPN is to be handled
separately from -EINPROGRESS. With -EALREADY the state has not changed
and should not be indicated. From service.c point of view the returned
status should still be -EINPROGRESS as the actual status of the VPN has
not changed.
---
src/provider.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/provider.c b/src/provider.c
index 8d90d5f8..7de96643 100644
--- a/src/provider.c
+++ b/src/provider.c
@@ -176,17 +176,24 @@ int __connman_provider_connect(struct connman_provider
*provider,
else
return -EOPNOTSUPP;
- if (err < 0) {
- if (err != -EINPROGRESS)
- return err;
+ switch (err) {
+ case 0:
+ return 0;
+ case -EINPROGRESS:
provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_ASSOCIATION);
-
+ /* fall through */
+ /*
+ * Return EINPROGRESS also for when there is an existing pending call.
+ * The state should not be indicated again but the real state is
+ * still in progress for the provider.
+ */
+ case -EALREADY:
return -EINPROGRESS;
}
- return 0;
+ return err;
}
int __connman_provider_remove_by_path(const char *path)
--
2.20.1
------------------------------
Date: Tue, 12 Nov 2019 16:00:51 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 1/5] vpn-provider: Change provider in failure state to
idle when saving
To: [email protected]
Message-ID: <[email protected]>
If the VPN provider is in failure state and it is saved the old state
must be cleared and changed to idle. This represents better the VPN
provider state in such scenario, the failure can be caused by connection
error or login error, both of which must be reset if the settings are
saved (most likely user changes settings to get the VPN to work).
---
vpn/vpn-provider.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 08792ecc..666bd016 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -879,6 +879,9 @@ static int vpn_provider_save(struct vpn_provider *provider)
reset_error_counters(provider);
+ if (provider->state == VPN_PROVIDER_STATE_FAILURE)
+ vpn_provider_set_state(provider, VPN_PROVIDER_STATE_IDLE);
+
if (provider->immutable) {
/*
* Do not save providers that are provisioned via .config
--
2.20.1
------------------------------
Date: Tue, 12 Nov 2019 16:00:52 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 2/5] vpn-provider: Reply to connect request only once
To: [email protected]
Message-ID: <[email protected]>
It is imperative to reply to the D-Bus method call only once. The
-EINPROGRESS returned by __vpn_provider_connect() should not be
immediately replied with an appropriate error. This is because the
connect_cb() will also send a reply based on the error returned by
the VPN plugin, or an empty one if there is no error.
In case the VPN is in failure state its state will be changed to idle
and -EINPROGRESS is reported as will be when VPN is being disconnected.
In both of these cases a reply to the method call is to be sent
directly.
Also, set VPN driver state to idle when a VPN is attempted to be
connected in failure state. This ensures that state is set for both
provider and the driver impl.
---
vpn/vpn-provider.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 666bd016..a6e2384d 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -503,7 +503,7 @@ static DBusMessage *do_connect(DBusConnection *conn,
DBusMessage *msg,
DBG("conn %p provider %p", conn, provider);
err = __vpn_provider_connect(provider, msg);
- if (err < 0)
+ if (err < 0 && err != -EINPROGRESS)
return __connman_error_failed(msg, -err);
return NULL;
@@ -1160,6 +1160,7 @@ static void connect_cb(struct vpn_provider *provider,
void *user_data,
int __vpn_provider_connect(struct vpn_provider *provider, DBusMessage *msg)
{
+ DBusMessage *reply;
int err;
DBG("provider %p state %d", provider, provider->state);
@@ -1172,6 +1173,10 @@ int __vpn_provider_connect(struct vpn_provider
*provider, DBusMessage *msg)
* attempt will continue as normal.
*/
case VPN_PROVIDER_STATE_FAILURE:
+ if (provider->driver && provider->driver->set_state)
+ provider->driver->set_state(provider,
+ VPN_PROVIDER_STATE_IDLE);
+
vpn_provider_set_state(provider, VPN_PROVIDER_STATE_IDLE);
/* fall through */
/*
@@ -1183,6 +1188,15 @@ int __vpn_provider_connect(struct vpn_provider
*provider, DBusMessage *msg)
* -EINPROGRESS to indicate that transition is in progress.
*/
case VPN_PROVIDER_STATE_DISCONNECT:
+ /*
+ * Failure transition or disconnecting does not yield a
+ * message to be sent. Send in progress message to avoid
+ * D-Bus LimitsExceeded error message.
+ */
+ reply = __connman_error_in_progress(msg);
+ if (reply)
+ g_dbus_send_message(connection, reply);
+
return -EINPROGRESS;
case VPN_PROVIDER_STATE_UNKNOWN:
case VPN_PROVIDER_STATE_IDLE:
--
2.20.1
------------------------------
Date: Tue, 12 Nov 2019 16:00:53 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 3/5] vpn: Restrict connection requests to one per
connection
To: [email protected]
Message-ID: <[email protected]>
Utilize DBusPendingCall in struct connection_data to keep track of
connection requests for VPNs. There should not be more than one pending
call per VPN at a time. If there is a pending connection request return
-EALREADY to indicate the status.
---
plugins/vpn.c | 42 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 38 insertions(+), 4 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 6157dffb..5668c004 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -493,9 +493,18 @@ static void connect_reply(DBusPendingCall *call, void
*user_data)
if (!dbus_pending_call_get_completed(call))
return;
+ if (call != data->call) {
+ connman_error("invalid call %p to VPN connect_reply data %p "
+ " call %p ", call, data, data->call);
+ dbus_pending_call_unref(call);
+ return;
+ }
+
DBG("user_data %p path %s", user_data, cb_data ? cb_data->path : NULL);
reply = dbus_pending_call_steal_reply(call);
+ if (!reply)
+ goto out;
dbus_error_init(&error);
@@ -534,7 +543,11 @@ static void connect_reply(DBusPendingCall *call, void
*user_data)
dbus_message_unref(reply);
- dbus_pending_call_unref(call);
+out:
+ dbus_pending_call_unref(data->call);
+
+ data->call = NULL;
+ data->connect_pending = false;
}
static int connect_provider(struct connection_data *data, void *user_data,
@@ -553,7 +566,10 @@ static int connect_provider(struct connection_data *data,
void *user_data,
return -EINVAL;
}
- data->connect_pending = false;
+ if (data->connect_pending && data->call) {
+ connman_info("connect already pending");
+ return -EALREADY;
+ }
/* We need to pass original dbus sender to connman-vpnd,
* use a Connect2 method for that if the original dbus sender is set.
@@ -587,6 +603,14 @@ static int connect_provider(struct connection_data *data,
void *user_data,
return -EINVAL;
}
+ if (data->call) {
+ dbus_pending_call_cancel(data->call);
+ dbus_pending_call_unref(data->call);
+ }
+
+ data->call = call;
+ data->connect_pending = true;
+
if (cb_data) {
g_free(cb_data->path);
cb_data->path = g_strdup(data->path);
@@ -996,6 +1020,8 @@ static int disconnect_provider(struct connection_data
*data)
static int provider_disconnect(struct connman_provider *provider)
{
+ int err = 0;
+
struct connection_data *data;
DBG("provider %p", provider);
@@ -1005,9 +1031,17 @@ static int provider_disconnect(struct connman_provider
*provider)
return -EINVAL;
if (provider_is_connected(data))
- return disconnect_provider(data);
+ err = disconnect_provider(data);
- return 0;
+ if (data->call) {
+ dbus_pending_call_cancel(data->call);
+ dbus_pending_call_unref(data->call);
+ data->call = NULL;
+ }
+
+ data->connect_pending = false;
+
+ return err;
}
static void configuration_create_reply(DBusPendingCall *call, void *user_data)
--
2.20.1
------------------------------
Date: Tue, 12 Nov 2019 16:00:55 +0200
From: Jussi Laakkonen <[email protected]>
Subject: [PATCH 5/5] service: Clear VPN error and state if AutoConnect
is set on
To: [email protected]
Message-ID: <[email protected]>
Changing AutoConnect value to "on" 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 on clear error and set the service state to idle,
if in failure state. This makes it possible to clear the error and
getting VPN to connect immediately after toggling the AutoConnect on.
Implemented function to disable autoconnect from all but the selected
service. This is required with VPNs as only one VPN should be
automatically connected, hence each transport service can support one
VPN only.
---
src/service.c | 56 +++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 48 insertions(+), 8 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7e1446b7..4aa15ffc 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3588,6 +3588,31 @@ void __connman_service_wispr_start(struct
connman_service *service,
__connman_wispr_start(service, type);
}
+static void disable_autoconnect_for_services(struct connman_service *exclude,
+ enum connman_service_type type)
+{
+ GList* list = NULL;
+
+ for (list = service_list; list; list = list->next) {
+ struct connman_service *service = list->data;
+
+ if (service->type != type)
+ continue;
+
+ if (service == exclude)
+ continue;
+
+ if (connman_service_set_autoconnect(service, false)) {
+ service_save(service);
+ connman_info("disabled autoconnect for %s",
+ service->name);
+ }
+ }
+}
+
+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 +3650,33 @@ 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);
+ 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);
- service->autoconnect = autoconnect;
+ if (service->state == CONNMAN_SERVICE_STATE_FAILURE) {
+ service->state = CONNMAN_SERVICE_STATE_IDLE;
+ state_changed(service);
+ }
- autoconnect_changed(service);
+ /* Disable autoconnect for all other VPN providers
+ * if autoconnect is set true. Only one VPN can have
+ * autoconnect enabled.
+ */
+ disable_autoconnect_for_services(service,
+ service->type);
+ }
- 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
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 49, Issue 16
***************************************
