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: When BackgroundScanning = false, connman's Scan() dbus
method is broken (Daniel Wagner)
2. Re: [PATCH v4 1/5] session: Add parameter Service into
CreateSession call of ConnMan session API (Daniel Wagner)
3. Re: Bug in gdhcp/server.c (Daniel Wagner)
4. Re: [PATCH 1/2] session.c: keep track of addr in fw_snat &
session (Daniel Wagner)
----------------------------------------------------------------------
Message: 1
Date: Mon, 4 Sep 2017 09:31:47 +0200
From: Daniel Wagner <[email protected]>
To: Jose Blanquicet <[email protected]>, Jonah Petri
<[email protected]>, Patrik Flykt <[email protected]>
Cc: Sam Nazarko <[email protected]>, [email protected]
Subject: Re: When BackgroundScanning = false, connman's Scan() dbus
method is broken
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Jose,
On 08/27/2017 04:48 PM, Jose Blanquicet wrote:
> Hi,
>
> On Thu, Aug 24, 2017 at 7:33 PM, Jonah Petri <[email protected]> wrote:
>>> The active scan is useful in order to perform a fast scan looking for the
>>> WiFi networks we have got connected in the past. I think this is
>>> particularly helpful to speed up auto-connect procedure at the start-up of
>>> the system. Instead, when user asks for scanning I think it should always
>>> be a passive scan because users want to see all WiFi networks available in
>>> range. Therefore, I propose to make ConnMan ask wpa_supplicant for an
>>> active scan only at star-up of the system and passive scan when it is
>>> directly asked from users through a Technology.Scan() D-Bus call. Both
>>> things no matter BackgroundScanning's value.
>>>
>>> What do all you think?
>>>
>> (Now it was my turn to be on vacation! Sorry for the delay.)
>
> Don't worry, welcome back!
>
>> Great idea. This is fine for my use case, and the behaviour you propose is
>> what I expected given what Technology.Scan() claims to do.
>
> Well, the scan algorithm is quite complex but I think I finally
> understood how it works and there is something we should change from
> this idea.
>
> We can resume our scenario as following:
>
> * WiFi Driver support active scanning, it means "MaxScanSSID" is greater than
> 1.
> * User has successfully connected to at least one WiFi network in the
> past so the service's information is present in
> {STOREDIR}/connman/wifi_* but none of those networks are currently in
> range, let's call them "known-services".
>
> Under these assumptions, when user asks for scanning the algorithm
> will perform an active scan looking for those known-services in their
> specific frequencies, this kind of scanning is generally faster than a
> passive one which allows to speed up auto-connection procedure.
> However, we have seen that some drivers reply with all the networks in
> the specified frequencies and some other does not reply any network.
> From this point forward, the behaviour changes based on
> "BackgroundScanning" value:
>
> * Enabled: A simple backoff mechanism starting from 10s up to 5
> minutes will run. Of course, using passive scan thus at the end driver
> will reply with all networks in range, as user expects after scanning.
> * Disabled: Nothing else is done. Any additional scan will be
> executed, resulting in a possible empty scan results forever, Johan's
> case.
>
> In my initial idea I proposed to perform passive scan, no matter the
> BackgroundScanning value, when user performs a Scan() D-Bus call to
> ensure all networks are found. However, while trying to implement it
> and due to the reasons I explained before, I realized that it could
> also increase the auto-connection procedure with the default ConnMan's
> configuration, i.e. BackgroundScanning enabled. Therefore, I am now
> limiting this change to users who manually disabled
> BackgroundScanning.
>
> Patrik, Daniel, what do you think?
This behavior is wpa_s specific, right? IIRC, iwd handles all this stuff
on its own.
Anyway it's sound reasonably to me with the assumption that
BackgroundScanning is enabled on most system.
Thanks,
Daniel
------------------------------
Message: 2
Date: Mon, 04 Sep 2017 13:15:08 +0200
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: [email protected], Bjoern Thorwirth
<[email protected]>, [email protected]
Subject: Re: [PATCH v4 1/5] session: Add parameter Service into
CreateSession call of ConnMan session API
Message-ID: <[email protected]>
Content-Type: text/plain
Hi,
[email protected] writes:
> From: Bjoern Thorwirth <[email protected]>
>
> The calling process, that implements the session API is identified by the
> user ID
> as it is runs. All processes of the same user share the same list of allowed
> bearers,
> and the same priority for choosing between available bearers.
>
> This extension allows processes to select a service context dependent
> behaviour
> for which the routing decision is made.
>
> This is an extention to session API interface. Helps to enable a service
> differentiation
> for processes run by the same user. Allows ConnMan to differentiate between
> bearer
> usage permissions and the respective priorities based on the requested
> service type.
I spend some time thinking on this and I think this API is not really
fitting. It doesn't really mix with the rest. It is not clear what
format the argument should have. 'string' can be anything. Also I
struggle a bit with the very generic name.
I can't remember if we had this discussed before but what about
something like AllowedServices? It fit a bit better into the existing
API? The best thing about is, that we can actaully make add a real
implementation to it and don't have this 'empty implementation' of
Service.
Here a rough version (just compiled tested). What do you think about
this?
Thanks,
Daniel
>From 742c500aadc5a32b34d4d6f381edd2858441f2dd Mon Sep 17 00:00:00 2001
From: Daniel Wagner <[email protected]>
Date: Mon, 4 Sep 2017 10:48:14 +0200
Subject: [PATCH] session: Add AllowedServices
The Session API allows the application to limit the choices of
ConnMan's connection algorithm via AllowedBearers and
AllowedInterfaces. These knobs are enough to control which service is
selected if the application just uses one Session. If the application
has several session at the same time, e.g. web browser, the existing
knobs are not enough.
By introducing AllowedService the a multi session application can tell
ConnMan exactly which Service can be assigned to a session.
---
doc/session-api.txt | 20 ++++++
include/session.h | 1 +
src/session.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 193 insertions(+), 4 deletions(-)
diff --git a/doc/session-api.txt b/doc/session-api.txt
index e8da5224dbf7..0f36694b45f3 100644
--- a/doc/session-api.txt
+++ b/doc/session-api.txt
@@ -130,6 +130,26 @@ Settings string State [readonly]
Current IPv6 configuration.
+ array{object} AllowedServices [readwrite] [experimental]
+
+ A list of Services that can be used for this session.
+ In general this list should be empty to indicate that
+ any service is acceptable.
+
+ The order of the entries in AllowedServices matters.
+
+ Also "*" matches any service. This is usefull to
+ prefer certain Service with a fallback to any
+ other available bearer.
+
+ When a session is created and the provided settings
+ dictionary does not contain AllowedServices, a default
+ session with "*" will be created.
+
+ The order of checking if a Service is allowed
+ to be used for a Session is AllowedServices,
+ AllowedBearers.
+
array{string} AllowedBearers [readwrite]
A list of bearers that can be used for this session.
diff --git a/include/session.h b/include/session.h
index 5106e886b9ab..28d73f720ec9 100644
--- a/include/session.h
+++ b/include/session.h
@@ -71,6 +71,7 @@ struct connman_session_config {
enum connman_session_roaming_policy roaming_policy;
enum connman_session_type type;
bool ecall;
+ GSList *allowed_services;
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
diff --git a/src/session.c b/src/session.c
index 9e3c55941ee6..5a0228530854 100644
--- a/src/session.c
+++ b/src/session.c
@@ -56,6 +56,7 @@ struct connman_session {
struct connman_service *service;
struct connman_service *service_last;
struct connman_session_config *policy_config;
+ GSList *user_allowed_services;
GSList *user_allowed_bearers;
char *user_allowed_interface;
@@ -494,6 +495,7 @@ static void free_session(struct connman_session *session)
g_dbus_remove_watch(connection, session->notify_watch);
destroy_policy_config(session);
+ g_slist_free_full(session->info->config.allowed_services, g_free);
g_slist_free(session->info->config.allowed_bearers);
g_free(session->info->config.allowed_interface);
g_free(session->owner);
@@ -533,6 +535,7 @@ static void cleanup_session(gpointer user_data)
session_deactivate(session);
update_session_state(session);
+ g_slist_free_full(session->user_allowed_services, g_free);
g_slist_free(session->user_allowed_bearers);
g_free(session->user_allowed_interface);
@@ -545,6 +548,7 @@ struct creation_data {
/* user config */
enum connman_session_type type;
+ GSList *allowed_services;
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
@@ -558,6 +562,7 @@ static void cleanup_creation_data(struct creation_data
*creation_data)
if (creation_data->pending)
dbus_message_unref(creation_data->pending);
+ g_slist_free_full(creation_data->allowed_services, g_free);
g_slist_free(creation_data->allowed_bearers);
g_free(creation_data->allowed_interface);
g_free(creation_data);
@@ -689,6 +694,34 @@ int connman_session_parse_bearers(const char *token,
GSList **list)
return 0;
}
+static int parse_services(DBusMessageIter *iter, GSList **list)
+{
+ DBusMessageIter array;
+ int type;
+
+ dbus_message_iter_recurse(iter, &array);
+
+ *list = NULL;
+
+ while ((type = dbus_message_iter_get_arg_type(&array)) !=
+ DBUS_TYPE_INVALID) {
+ char *path = NULL;
+
+ if (type != DBUS_TYPE_OBJECT_PATH) {
+ g_slist_free(*list);
+ *list = NULL;
+ return -EINVAL;
+ }
+
+ dbus_message_iter_get_basic(&array, &path);
+ *list = g_slist_append(*list, g_strdup(path));
+
+ dbus_message_iter_next(&array);
+ }
+
+ return 0;
+}
+
static int parse_bearers(DBusMessageIter *iter, GSList **list)
{
DBusMessageIter array;
@@ -723,6 +756,25 @@ static int parse_bearers(DBusMessageIter *iter, GSList
**list)
return 0;
}
+static void filter_service(GSList *services, const char *policy_service,
+ GSList **list)
+{
+ GSList *it;
+
+ if (!services)
+ return;
+
+ for (it = services; it; it = it->next) {
+ const char *service = it->data;
+
+ if (!g_strcmp0(policy_service, service))
+ continue;
+
+ *list = g_slist_append(*list, g_strdup(service));
+ return;
+ }
+}
+
static void filter_bearer(GSList *bearers,
enum connman_service_type policy,
GSList **list)
@@ -744,6 +796,20 @@ static void filter_bearer(GSList *bearers,
}
}
+static void apply_policy_on_services(GSList *policy_services, GSList *services,
+ GSList **list)
+{
+ GSList *it;
+
+ *list = NULL;
+
+ for (it = policy_services; it; it = it->next) {
+ char *policy_service = it->data;
+
+ filter_service(services, policy_service, list);
+ }
+}
+
static void apply_policy_on_bearers(GSList *policy_bearers, GSList *bearers,
GSList **list)
{
@@ -775,6 +841,23 @@ const char *connman_session_get_owner(struct
connman_session *session)
return session->owner;
}
+static void append_allowed_services(DBusMessageIter *iter, void *user_data)
+{
+ struct session_info *info = user_data;
+ GSList *list;
+
+ for (list = info->config.allowed_services;
+ list; list = list->next) {
+ char *path = list->data;
+
+ if (!path)
+ path = "";
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH,
+ &path);
+ }
+}
+
static void append_allowed_bearers(DBusMessageIter *iter, void *user_data)
{
struct session_info *info = user_data;
@@ -907,6 +990,15 @@ static void append_notify(DBusMessageIter *dict,
}
if (session->append_all ||
+ info->config.allowed_services !=
info_last->config.allowed_services) {
+ connman_dbus_dict_append_array(dict, "AllowedServices",
+ DBUS_TYPE_OBJECT_PATH,
+ append_allowed_services,
+ info);
+ info_last->config.allowed_services =
info->config.allowed_services;
+ }
+
+ if (session->append_all ||
info->config.allowed_bearers !=
info_last->config.allowed_bearers) {
connman_dbus_dict_append_array(dict, "AllowedBearers",
DBUS_TYPE_STRING,
@@ -955,7 +1047,8 @@ static bool compute_notifiable_changes(struct
connman_session *session)
info->state >= CONNMAN_SESSION_STATE_CONNECTED)
return true;
- if (info->config.allowed_bearers != info_last->config.allowed_bearers ||
+ if (info->config.allowed_services != info_last->config.allowed_services
||
+ info->config.allowed_bearers !=
info_last->config.allowed_bearers ||
info->config.type != info_last->config.type ||
info->config.allowed_interface !=
info_last->config.allowed_interface ||
info->config.source_ip_rule !=
info_last->config.source_ip_rule)
@@ -1011,6 +1104,7 @@ static void ipconfig_ipv6_changed(struct connman_session
*session)
int connman_session_config_update(struct connman_session *session)
{
struct session_info *info = session->info;
+ GSList *allowed_services;
GSList *allowed_bearers;
char *allowed_interface;
int err;
@@ -1033,6 +1127,11 @@ int connman_session_config_update(struct connman_session
*session)
session->id_type = session->policy_config->id_type;
}
+ apply_policy_on_services(
+ session->policy_config->allowed_services,
+ session->user_allowed_services,
+ &allowed_services);
+
apply_policy_on_bearers(
session->policy_config->allowed_bearers,
session->user_allowed_bearers,
@@ -1048,6 +1147,9 @@ int connman_session_config_update(struct connman_session
*session)
session->active = false;
session_deactivate(session);
+ g_slist_free_full(info->config.allowed_services, g_free);
+ info->config.allowed_services = allowed_services;
+
g_slist_free(info->config.allowed_bearers);
info->config.allowed_bearers = allowed_bearers;
@@ -1132,6 +1234,7 @@ static DBusMessage *change_session(DBusConnection *conn,
DBusMessageIter iter, value;
const char *name;
const char *val;
+ GSList *allowed_services;
GSList *allowed_bearers;
int err;
@@ -1152,7 +1255,28 @@ static DBusMessage *change_session(DBusConnection *conn,
switch (dbus_message_iter_get_arg_type(&value)) {
case DBUS_TYPE_ARRAY:
- if (g_str_equal(name, "AllowedBearers")) {
+ if (g_str_equal(name, "AllowedServices")) {
+ err = parse_services(&value, &allowed_services);
+ if (err < 0)
+ return __connman_error_failed(msg, -err);
+
+ if (session->active)
+ set_active_session(session, false);
+
+ session->active = false;
+ session_deactivate(session);
+ update_session_state(session);
+
+ g_slist_free_full(info->config.allowed_services,
g_free);
+ session->user_allowed_services = allowed_services;
+
+ apply_policy_on_services(
+
session->policy_config->allowed_services,
+ session->user_allowed_services,
+ &info->config.allowed_services);
+
+ session_activate(session);
+ } else if (g_str_equal(name, "AllowedBearers")) {
err = parse_bearers(&value, &allowed_bearers);
if (err < 0)
return __connman_error_failed(msg, -err);
@@ -1346,12 +1470,20 @@ static int session_policy_config_cb(struct
connman_session *session,
info->config.priority = session->policy_config->priority;
info->config.roaming_policy = session->policy_config->roaming_policy;
+ session->user_allowed_services = creation_data->allowed_services;
+ creation_data->allowed_services = NULL;
+
session->user_allowed_bearers = creation_data->allowed_bearers;
creation_data->allowed_bearers = NULL;
session->user_allowed_interface = creation_data->allowed_interface;
creation_data->allowed_interface = NULL;
+ apply_policy_on_services(
+ session->policy_config->allowed_services,
+ session->user_allowed_services,
+ &info->config.allowed_services);
+
apply_policy_on_bearers(
session->policy_config->allowed_bearers,
session->user_allowed_bearers,
@@ -1384,6 +1516,7 @@ static int session_policy_config_cb(struct
connman_session *session,
info_last->state = info->state;
info_last->config.priority = info->config.priority;
info_last->config.roaming_policy = info->config.roaming_policy;
+ info_last->config.allowed_services = info->config.allowed_services;
info_last->config.allowed_bearers = info->config.allowed_bearers;
info_last->config.allowed_interface = info->config.allowed_interface;
info_last->config.source_ip_rule = info->config.source_ip_rule;
@@ -1414,6 +1547,7 @@ int __connman_session_create(DBusMessage *msg)
DBusMessageIter iter, array;
struct connman_session *session = NULL;
struct creation_data *creation_data = NULL;
+ bool user_allowed_services = false;
bool user_allowed_bearers = false;
bool user_connection_type = false;
int err, i;
@@ -1455,7 +1589,14 @@ int __connman_session_create(DBusMessage *msg)
switch (dbus_message_iter_get_arg_type(&value)) {
case DBUS_TYPE_ARRAY:
- if (g_str_equal(key, "AllowedBearers")) {
+ if (g_str_equal(key, "AllowedServices")) {
+ err = parse_services(&value,
+ &creation_data->allowed_services);
+ if (err < 0)
+ goto err;
+
+ user_allowed_services = true;
+ } else if (g_str_equal(key, "AllowedBearers")) {
err = parse_bearers(&value,
&creation_data->allowed_bearers);
if (err < 0)
@@ -1504,8 +1645,13 @@ int __connman_session_create(DBusMessage *msg)
* If the user hasn't provided a configuration, we set
* the default configuration.
*
- * For AllowedBearers this is '*', ...
+ * For AllowedServices and AllowedBearers this is '*', ...
*/
+ if (!user_allowed_services) {
+ creation_data->allowed_services =
+ g_slist_append(NULL, g_strdup("*"));
+ }
+
if (!user_allowed_bearers) {
add_default_bearer_types(&creation_data->allowed_bearers);
if (!creation_data->allowed_bearers) {
@@ -1691,6 +1837,24 @@ static void update_session_state(struct connman_session
*session)
session_notify(session);
}
+static bool allowed_service_match(struct connman_service *service,
+ GSList *allowed_services)
+{
+ GSList *list;
+
+ for (list = allowed_services; list; list = list->next) {
+ const char *path = list->data;
+
+ if (!g_strcmp0(path, "*"))
+ return true;
+
+ if (!g_strcmp0(path, __connman_service_get_path(service)))
+ return true;
+ }
+
+ return false;
+}
+
static bool session_match_service(struct connman_session *session,
struct connman_service *service)
{
@@ -1703,6 +1867,10 @@ static bool session_match_service(struct connman_session
*session,
if (policy && policy->allowed)
return policy->allowed(session, service);
+ if (!allowed_service_match(service,
+ session->info->config.allowed_services))
+ return false;
+
current_service_type = connman_service_get_type(session->service);
for (list = session->info->config.allowed_bearers; list; list =
list->next) {
--
2.9.5
------------------------------
Message: 3
Date: Mon, 04 Sep 2017 13:28:54 +0200
From: Daniel Wagner <[email protected]>
To: Gerald Loacker <[email protected]>
Cc: "connman\@lists.01.org" <[email protected]>
Subject: Re: Bug in gdhcp/server.c
Message-ID: <[email protected]>
Content-Type: text/plain
Hi,
Gerald Loacker <[email protected]> writes:
> Hi,
>
> when using miracast and being group owner of the p2p connection, the
> connection stops after about one
> hour (client was Microsoft surface pro 3). The attached patch fixes this
> issue. Could someone please
> review?
Great catch! I applied the patch.
Thanks,
Daniel
------------------------------
Message: 4
Date: Mon, 04 Sep 2017 13:40:19 +0200
From: Daniel Wagner <[email protected]>
To: Jian Liang <[email protected]>
Cc: [email protected], Jian Liang <[email protected]>
Subject: Re: [PATCH 1/2] session.c: keep track of addr in fw_snat &
session
Message-ID: <[email protected]>
Content-Type: text/plain
Hi Jian,
Jian Liang <[email protected]> writes:
> From: Jian Liang <[email protected]>
>
> When there is more than one session in fw_snat's list of sessions,
> fw_snat failed to be re-created when update-session-state is triggered
> with new IP address. This is because index alone is not sufficient to
> decide if fw_snat needs to be re-created. The solution here is to keep
> a track of IP addr and use it to avoid false lookup of fw_snat.
> ---
> src/session.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index 9e3c559..ca2ddf2 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -65,6 +65,7 @@ struct connman_session {
> struct firewall_context *fw;
> uint32_t mark;
> int index;
> + char *addr;
> char *gateway;
> bool policy_routing;
> bool snat_enabled;
> @@ -79,6 +80,7 @@ struct fw_snat {
> GSList *sessions;
> int id;
> int index;
> + char *addr;
> struct firewall_context *fw;
> };
>
> @@ -200,7 +202,7 @@ static char *service2bearer(enum connman_service_type
> type)
> return "";
> }
>
> -static struct fw_snat *fw_snat_lookup(int index)
> +static struct fw_snat *fw_snat_lookup(int index, const char *addr)
> {
> struct fw_snat *fw_snat;
> GSList *list;
> @@ -208,8 +210,11 @@ static struct fw_snat *fw_snat_lookup(int index)
> for (list = fw_snat_list; list; list = list->next) {
> fw_snat = list->data;
>
> - if (fw_snat->index == index)
> + if (fw_snat->index == index) {
> + if (addr && strcmp (addr, fw_snat->addr))
use g_strcmp() and don't do the extra space after the call.
If fw_snat->addr is never NULL, you could so just use g_strcmp0()
withouth the addition 'if (addr && ...)' part because the g_strcmp0()
can handle NULL.
> + continue;
> return fw_snat;
> + }
> }
> return NULL;
> }
> @@ -224,6 +229,7 @@ static int fw_snat_create(struct connman_session *session,
>
> fw_snat->fw = __connman_firewall_create();
> fw_snat->index = index;
> + fw_snat->addr = g_strdup (addr);
Same here, no whitespace after the function name.
>
> fw_snat->id = __connman_firewall_enable_snat(fw_snat->fw,
> index, ifname, addr);
> @@ -238,6 +244,7 @@ static int fw_snat_create(struct connman_session *session,
> return 0;
> err:
> __connman_firewall_destroy(fw_snat->fw);
> + g_free(fw_snat->addr);
> g_free(fw_snat);
> return err;
> }
> @@ -393,7 +400,7 @@ static void del_nat_rules(struct connman_session *session)
> return;
>
> session->snat_enabled = false;
> - fw_snat = fw_snat_lookup(session->index);
> + fw_snat = fw_snat_lookup(session->index, session->addr);
>
> if (!fw_snat)
> return;
> @@ -420,8 +427,11 @@ static void add_nat_rules(struct connman_session
> *session)
> if (!addr)
> return;
>
> + g_free(session->addr);
> + session->addr = g_strdup (addr);
No space between function name and the braces.
> +
> session->snat_enabled = true;
> - fw_snat = fw_snat_lookup(index);
> + fw_snat = fw_snat_lookup(index, session->addr);
> if (fw_snat) {
> fw_snat_ref(session, fw_snat);
> return;
> @@ -502,6 +512,7 @@ static void free_session(struct connman_session *session)
> g_free(session->info);
> g_free(session->info_last);
> g_free(session->gateway);
> + g_free(session->addr);
>
> g_free(session);
> }
Apart of my nitpicking, the rest looks good.
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 23, Issue 2
**************************************
