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: [RFC] service: Move service to state ready if
defaultroute is removed (Patrik Flykt)
2. [PATCH v2 0/7] Clean up gateway selection (Patrik Flykt)
3. [PATCH v2 4/7] service: Create helper function for setting
split routing (Patrik Flykt)
4. [PATCH v2 3/7] connection: Use services to compare gateway
ordering (Patrik Flykt)
5. [PATCH v2 1/7] service: Add function to compare the order
between two services (Patrik Flykt)
6. [PATCH v2 2/7] connection: Use the sorted service list to
find the default gateway (Patrik Flykt)
7. [PATCH v2 5/7] connection: Remove 'order' variable (Patrik Flykt)
8. [PATCH v2 6/7] service: Remove obsolete
__connman_service_update_ordering() function (Patrik Flykt)
----------------------------------------------------------------------
Message: 1
Date: Tue, 19 Jan 2016 13:01:42 +0200
From: Patrik Flykt <[email protected]>
To: Naveen Singh <[email protected]>
Cc: [email protected]
Subject: Re: [RFC] service: Move service to state ready if
defaultroute is removed
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"
Hi,
On Wed, 2016-01-13 at 15:39 -0800, Naveen Singh wrote:
>
>
> On Wed, Jan 13, 2016 at 4:57 AM, Patrik Flykt
> <[email protected]> wrote:
> When the default route is removed, it is more than likely that
> the
> corresponding service cannot be in state online anymore.
> Downgrade
> the affected service state. This in turn will trigger a new
> online
> check and resolve the state back to online if at all possible.
> ---
>
> This might fix the issue with dhcp_failure() reported by
> Naveen Singh.
>
> Please test!
>
> Patrik
>
>
> src/service.c | 23 +++++++++++++++++++----
> 1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/src/service.c b/src/service.c
> index abf0899..3fa5ae5 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -6380,12 +6380,27 @@ static void service_ip_release(struct
> connman_ipconfig *ipconfig,
> settings_changed(service, ipconfig);
> }
>
> -static void service_route_changed(struct connman_ipconfig
> *ipconfig,
> +static void service_defaultroute_set(struct connman_ipconfig
> *ipconfig,
> const char *ifname)
> {
> struct connman_service *service =
> __connman_ipconfig_get_data(ipconfig);
>
> - DBG("%s route changed", ifname);
> + DBG("service %p %s defaultroute set", service,
> ifname);
> +
> + settings_changed(service, ipconfig);
> +}
> +
> +static void service_defaultroute_unset(struct
> connman_ipconfig *ipconfig,
> + const char *ifname)
> +{
> + struct connman_service *service =
> __connman_ipconfig_get_data(ipconfig);
> +
> + DBG("service %p %s defaultroute unset", service,
> ifname);
> +
> + if (service->state == CONNMAN_SERVICE_STATE_ONLINE)
> +
> __connman_service_ipconfig_indicate_state(service,
> + CONNMAN_SERVICE_STATE_READY,
> +
> __connman_ipconfig_get_config_type(ipconfig));
> When the default route is removed why we are changing the state to
> READY. Should not it be CONFIGURE?
A service can be in state ready even if it does not have a gateway. This
is what happens for link-local IPv4 addresses, they have only a subnet
but no default route.
>
> settings_changed(service, ipconfig);
> }
> @@ -6397,8 +6412,8 @@ static const struct connman_ipconfig_ops
> service_ops = {
> .lower_down = service_lower_down,
> .ip_bound = service_ip_bound,
> .ip_release = service_ip_release,
> - .route_set = service_route_changed,
> - .route_unset = service_route_changed,
> + .route_set = service_defaultroute_set,
> + .route_unset = service_defaultroute_unset,
> };
>
> static struct connman_ipconfig *create_ip4config(struct
> connman_service *service,
>
>
> In stead of changing this why do not we use service_ip_bound and
> service_ip_release to do the specific state transitions. These
> functions are the entry point. Right now the service state transition
> to READY is happening from __connman_connection_gateway_add which we
> should remove. This is the sequence that I am proposing:
> 1) In service_ip_bound transition from configuration to ready
> 2) In service_ip_release transition from ready to configuration
>
>
> There is a small check we need to perform specially when we are going
> from 169 address to (DHCP) address. We would like to release the link
> local IP first if it is currently in the link local state and then set
> the IP. This would allow service_ip_release to be called first and
> then service_ip_bound.
IPv6 is then able to remove the state from ready to something else once
a temporary address is removed. Likewise, any IP address the user adds
and later removes would cause the service to drop.
Cheers,
Patrik
------------------------------
Message: 2
Date: Tue, 19 Jan 2016 13:23:30 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 0/7] Clean up gateway selection
Message-ID:
<[email protected]>
Hi,
Patch 4/7 was added to this set, it keeps service order in sync with VPN
split routing, something that was being implicitly updated when calling
update_order() in connection.c. The code currently already sorts the
service list after the new calls to set_split_routing(), that's why there
is no explicit service sorting added by patch 4/7.
The rest of the patches are as before.
Cheers,
Patrik
In order to clean up gateway selection, the gateway code should use the
sorted list of services instead of sorting the gateways by itself.
The 'order' value in the gateway_data structure follows the service list,
i.e. the default service gets an 'order' value of 1, a VPN without split
routing a value of 10 and the rest a value of 0. This means the gateway
'order' variable gets its values so that the sorting order is the same
between gateways and services in the service list. With that, there are
now two entities that are being sorted, which in turn has caused code
duplication and a non-trivial implementation to decipher for the gateway
selection.
To change the current behavior, a function comparing the ordering between
two services is introduced in patch 1/7. This function is then used in
patch 3/7, throwing out the old code as a result. Patch 3/7 also combines
consecutive if statements.
Patch 2/7 gets the default service and uses that to look up the corresponding
gateway_data structure instead of iterating over the hash table.
With the previous ones applied, the 'order' variable is removed in patch 5/7,
the now unused function __connman_service_update_ordering() in patch 6/7 and
the TODO is updated in patch 7/7.
Patrik Flykt (7):
service: Add function to compare the order between two services
connection: Use the sorted service list to find the default gateway
connection: Use services to compare gateway ordering
service: Create helper function for setting split routing
connection: Remove 'order' variable
service: Remove obsolete __connman_service_update_ordering() function
TODO: Mark gateway selection simplification done
TODO | 11 ---------
src/connection.c | 75 ++++++++++++++------------------------------------------
src/connman.h | 4 ++-
src/service.c | 43 ++++++++++++++++++++++----------
4 files changed, 52 insertions(+), 81 deletions(-)
--
2.1.4
------------------------------
Message: 3
Date: Tue, 19 Jan 2016 13:23:34 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 4/7] service: Create helper function for setting
split routing
Message-ID:
<[email protected]>
When setting split routing, the service order must also be updated
to match the functionality provided by __connman_service_get_order().
A VPN with split routing keeps its order at zero like all other services,
but one without split routing needs to be sorted when connected and is
given a order of 10.
---
src/service.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/src/service.c b/src/service.c
index a39db2e..48df58d 100644
--- a/src/service.c
+++ b/src/service.c
@@ -347,6 +347,19 @@ static enum connman_service_proxy_method
string2proxymethod(const char *method)
return CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN;
}
+static void set_split_routing(struct connman_service *service, bool value)
+{
+ if (service->type != CONNMAN_SERVICE_TYPE_VPN)
+ return;
+
+ service->do_split_routing = value;
+
+ if (service->do_split_routing)
+ service->order = 0;
+ else
+ service->order = 10;
+}
+
int __connman_service_load_modifiable(struct connman_service *service)
{
GKeyFile *keyfile;
@@ -367,8 +380,10 @@ int __connman_service_load_modifiable(struct
connman_service *service)
case CONNMAN_SERVICE_TYPE_P2P:
break;
case CONNMAN_SERVICE_TYPE_VPN:
- service->do_split_routing = g_key_file_get_boolean(keyfile,
- service->identifier, "SplitRouting", NULL);
+ set_split_routing(service, g_key_file_get_boolean(keyfile,
+ service->identifier,
+ "SplitRouting", NULL));
+
/* fall through */
case CONNMAN_SERVICE_TYPE_WIFI:
case CONNMAN_SERVICE_TYPE_GADGET:
@@ -421,8 +436,10 @@ static int service_load(struct connman_service *service)
case CONNMAN_SERVICE_TYPE_P2P:
break;
case CONNMAN_SERVICE_TYPE_VPN:
- service->do_split_routing = g_key_file_get_boolean(keyfile,
- service->identifier, "SplitRouting", NULL);
+ set_split_routing(service, g_key_file_get_boolean(keyfile,
+ service->identifier,
+ "SplitRouting", NULL));
+
autoconnect = g_key_file_get_boolean(keyfile,
service->identifier, "AutoConnect", &error);
if (!error)
@@ -4223,11 +4240,11 @@ static DBusMessage *move_service(DBusConnection *conn,
return __connman_error_invalid_service(msg);
}
- target->do_split_routing = true;
+ set_split_routing(target, true);
} else
- target->do_split_routing = false;
+ set_split_routing(target, false);
- service->do_split_routing = false;
+ set_split_routing(service, false);
target4 = __connman_ipconfig_get_method(target->ipconfig_ipv4);
target6 = __connman_ipconfig_get_method(target->ipconfig_ipv6);
--
2.1.4
------------------------------
Message: 4
Date: Tue, 19 Jan 2016 13:23:33 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 3/7] connection: Use services to compare gateway
ordering
Message-ID:
<[email protected]>
Use the service pointer in the gateway data to find out if the candidate
gateway should be chosen over the other gateway.
---
src/connection.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/connection.c b/src/connection.c
index b2d6f5b..414d3ac 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -577,37 +577,35 @@ static bool choose_default_gateway(struct gateway_data
*data,
* this one as default. If the other one is already active
* we mark this one as non default.
*/
- if (data->ipv4_gateway) {
- if (candidate->ipv4_gateway &&
- !candidate->ipv4_gateway->active) {
+ if (data->ipv4_gateway && candidate->ipv4_gateway) {
+
+ if (!candidate->ipv4_gateway->active) {
DBG("ipv4 downgrading %p", candidate);
unset_default_gateway(candidate,
CONNMAN_IPCONFIG_TYPE_IPV4);
}
- if (candidate->ipv4_gateway &&
- candidate->ipv4_gateway->active &&
- candidate->order > data->order) {
+
+ if (candidate->ipv4_gateway->active &&
+ __connman_service_compare(candidate->service,
+ data->service) < 0) {
DBG("ipv4 downgrading this %p", data);
- unset_default_gateway(data,
- CONNMAN_IPCONFIG_TYPE_IPV4);
+ unset_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV4);
downgraded = true;
}
}
- if (data->ipv6_gateway) {
- if (candidate->ipv6_gateway &&
- !candidate->ipv6_gateway->active) {
+ if (data->ipv6_gateway && candidate->ipv6_gateway) {
+ if (!candidate->ipv6_gateway->active) {
DBG("ipv6 downgrading %p", candidate);
unset_default_gateway(candidate,
CONNMAN_IPCONFIG_TYPE_IPV6);
}
- if (candidate->ipv6_gateway &&
- candidate->ipv6_gateway->active &&
- candidate->order > data->order) {
+ if (candidate->ipv6_gateway->active &&
+ __connman_service_compare(candidate->service,
+ data->service) < 0) {
DBG("ipv6 downgrading this %p", data);
- unset_default_gateway(data,
- CONNMAN_IPCONFIG_TYPE_IPV6);
+ unset_default_gateway(data, CONNMAN_IPCONFIG_TYPE_IPV6);
downgraded = true;
}
}
--
2.1.4
------------------------------
Message: 5
Date: Tue, 19 Jan 2016 13:23:31 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 1/7] service: Add function to compare the order
between two services
Message-ID:
<[email protected]>
This function compares the order between two services and returns
-1, 0 or 1 respectively.
---
src/connman.h | 3 +++
src/service.c | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/src/connman.h b/src/connman.h
index ad789c3..a7316ae 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -656,6 +656,9 @@ int __connman_service_load_modifiable(struct
connman_service *service);
void __connman_service_list_struct(DBusMessageIter *iter);
+int __connman_service_compare(const struct connman_service *a,
+ const struct connman_service *b);
+
struct connman_service *__connman_service_lookup_from_index(int index);
struct connman_service *__connman_service_lookup_from_ident(const char
*identifier);
struct connman_service *__connman_service_create_from_network(struct
connman_network *network);
diff --git a/src/service.c b/src/service.c
index abf0899..a39db2e 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4785,6 +4785,12 @@ static void service_list_sort(void)
}
}
+int __connman_service_compare(const struct connman_service *a,
+ const struct connman_service *b)
+{
+ return service_compare(a, b);
+}
+
/**
* connman_service_get_type:
* @service: service structure
--
2.1.4
------------------------------
Message: 6
Date: Tue, 19 Jan 2016 13:23:32 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 2/7] connection: Use the sorted service list to
find the default gateway
Message-ID:
<[email protected]>
The service list is always sorted and thus the service that should have
the default gateway is first in the list. The first service in the list
has coincidentally also the largest 'order' value.
---
src/connection.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)
diff --git a/src/connection.c b/src/connection.c
index bbc5d83..b2d6f5b 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -558,25 +558,13 @@ static void unset_default_gateway(struct gateway_data
*data,
static struct gateway_data *find_default_gateway(void)
{
- struct gateway_data *found = NULL;
- unsigned int order = 0;
- GHashTableIter iter;
- gpointer value, key;
-
- g_hash_table_iter_init(&iter, gateway_hash);
-
- while (g_hash_table_iter_next(&iter, &key, &value)) {
- struct gateway_data *data = value;
-
- if (!found || data->order > order) {
- found = data;
- order = data->order;
+ struct connman_service *service;
- DBG("default %p order %d", found, order);
- }
- }
+ service = __connman_service_get_default();
+ if (!service)
+ return NULL;
- return found;
+ return g_hash_table_lookup(gateway_hash, service);
}
static bool choose_default_gateway(struct gateway_data *data,
--
2.1.4
------------------------------
Message: 7
Date: Tue, 19 Jan 2016 13:23:35 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 5/7] connection: Remove 'order' variable
Message-ID:
<[email protected]>
Ordering is now done according to the order of the service list and
thus the 'order' variable becomes unnecessary.
---
src/connection.c | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/src/connection.c b/src/connection.c
index 414d3ac..f4ecd22 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -46,7 +46,6 @@ struct gateway_config {
struct gateway_data {
int index;
struct connman_service *service;
- unsigned int order;
struct gateway_config *ipv4_gateway;
struct gateway_config *ipv6_gateway;
bool default_checked;
@@ -381,8 +380,6 @@ static struct gateway_data *add_gateway(struct
connman_service *service,
data->service = service;
- data->order = __connman_service_get_order(service);
-
/*
* If the service is already in the hash, then we
* must not replace it blindly but disable the gateway
@@ -741,22 +738,6 @@ static struct gateway_data *find_active_gateway(void)
return NULL;
}
-static void update_order(void)
-{
- GHashTableIter iter;
- gpointer value, key;
-
- DBG("");
-
- g_hash_table_iter_init(&iter, gateway_hash);
-
- while (g_hash_table_iter_next(&iter, &key, &value)) {
- struct gateway_data *data = value;
-
- data->order = __connman_service_get_order(data->service);
- }
-}
-
static void add_host_route(int family, int index, const char *gateway,
enum connman_service_type service_type)
{
@@ -994,8 +975,6 @@ bool __connman_connection_update_gateway(void)
if (!gateway_hash)
return updated;
- update_order();
-
default_gateway = find_default_gateway();
__connman_service_update_ordering();
--
2.1.4
------------------------------
Message: 8
Date: Tue, 19 Jan 2016 13:23:36 +0200
From: Patrik Flykt <[email protected]>
To: [email protected]
Subject: [PATCH v2 6/7] service: Remove obsolete
__connman_service_update_ordering() function
Message-ID:
<[email protected]>
The function __connman_service_update_ordering() does not provide any
features anymore, thus it is removed.
---
src/connection.c | 2 --
src/connman.h | 1 -
src/service.c | 6 ------
3 files changed, 9 deletions(-)
diff --git a/src/connection.c b/src/connection.c
index f4ecd22..6b005e7 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -977,8 +977,6 @@ bool __connman_connection_update_gateway(void)
default_gateway = find_default_gateway();
- __connman_service_update_ordering();
-
DBG("default %p", default_gateway);
/*
diff --git a/src/connman.h b/src/connman.h
index a7316ae..447bdd7 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -683,7 +683,6 @@ const char *__connman_service_get_path(struct
connman_service *service);
const char *__connman_service_get_name(struct connman_service *service);
unsigned int __connman_service_get_order(struct connman_service *service);
enum connman_service_state __connman_service_get_state(struct connman_service
*service);
-void __connman_service_update_ordering(void);
struct connman_network *__connman_service_get_network(struct connman_service
*service);
enum connman_service_security __connman_service_get_security(struct
connman_service *service);
const char *__connman_service_get_phase2(struct connman_service *service);
diff --git a/src/service.c b/src/service.c
index 48df58d..a93aebf 100644
--- a/src/service.c
+++ b/src/service.c
@@ -6623,12 +6623,6 @@ unsigned int __connman_service_get_order(struct
connman_service *service)
return order;
}
-void __connman_service_update_ordering(void)
-{
- if (service_list && service_list->next)
- service_list = g_list_sort(service_list, service_compare);
-}
-
static enum connman_service_type convert_network_type(struct connman_network
*network)
{
enum connman_network_type type = connman_network_get_type(network);
--
2.1.4
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 3, Issue 12
**************************************
