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: [PATCH] inet: Prevent glib source from being removed
twice (Daniel Wagner)
2. Re: [PATCH] service: Add EnableOnlineCheck config option
(Daniel Wagner)
3. Re: [PATCH] inet: Prevent glib source from being removed
twice (Daniel Wagner)
4. Re: [PATCH v4 5/8] session: Add source ip rule (Daniel Wagner)
5. Re: [PATCH v4 2/8] session: Add AllowedInterface config
parameter (Daniel Wagner)
6. Re: [PATCH] session: Set allowed_bearers to NULL before
reusing list (Daniel Wagner)
7. Re: [PATCH] session: Maintain bearer priority in policy file
and check for other services when service lost (Daniel Wagner)
8. Re: ofono: cellular service disconnect makes it disappear
(Daniel Wagner)
9. Re: ofono: cellular service disconnect makes it disappear
(Daniel Wagner)
10. Re: Service remains on service list even if it is not longer
available (Jose Blanquicet)
----------------------------------------------------------------------
Message: 1
Date: Fri, 3 Feb 2017 07:20:33 +0100
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] inet: Prevent glib source from being removed
twice
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Slava,
>> Maybe it's a good time to make those all path the same and then we add
>> a comment saying what the return value means. Maybe adding an enum or
>> using error codes. What do you think?
>
> I'll submit another patch with inet_rtnl_event doing 'if (ret == 0)
> return TRUE' just as it's done in all other places. Hopefully, I get the
> logic right this time. As for the other improvements, the whole thing is
> so much not my style that I don't feel myself to be in a position to
> propose anything. It can all be done in so many difference ways. It's up
> to you.
No worries. If you just make it consistent it is already an big
improvement.
Thanks,
Daniel
------------------------------
Message: 2
Date: Fri, 3 Feb 2017 07:23:47 +0100
From: Daniel Wagner <[email protected]>
To: Lukasz Nowak <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] service: Add EnableOnlineCheck config option
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Lukasz,
On 02/01/2017 08:09 AM, Daniel Wagner wrote:
>>> and we either add wispr.c an version for --disable-wispr in a #ifdef
>>> block or maybe even better have a wispr-disabled.c (better name) which
>>> contains just the READY to ONLINE transistion. And the configure
>>> script just adds one or the other to the build.
>>
>> If that's the decision, I can code it this way, no problem, in the
>> next few days.
>
> We have already parts like this. So it is consistent and actually makes
> --disable-wispr work. So yes, let's go this way. Thanks a for doing the
> work!
Actually after thinking for a bit longer, what you are about to do is to
disable the online check. So --disable-wispr works as expected, the
thing you wont do to is to introduce --disable-online-check. That is
fine for me, but we should not do the trick with READY -> ONLINE state
transition in this case, just let it stay in READY.
Thanks,
Daniel
------------------------------
Message: 3
Date: Fri, 3 Feb 2017 07:31:52 +0100
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>, [email protected]
Subject: Re: [PATCH] inet: Prevent glib source from being removed
twice
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Slava,
On 02/01/2017 02:37 PM, Slava Monich wrote:
> If GIOFunc callback returns FALSE, the event source is removed
> by glib. We either call g_source_remove or return FALSE from the
> callback. If we do both, the source is getting removed twice which
> glib considers a critical error.
Patch applied.
Thanks,
Daniel
------------------------------
Message: 4
Date: Fri, 3 Feb 2017 07:39:23 +0100
From: Daniel Wagner <[email protected]>
To: Lukasz Nowak <[email protected]>, [email protected]
Subject: Re: [PATCH v4 5/8] session: Add source ip rule
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Lukasz,
On 01/31/2017 03:13 PM, Lukasz Nowak wrote:
> + break;
> + case DBUS_TYPE_BOOLEAN:
> + if (g_str_equal(key, "SourceIPRule")) {
> + dbus_bool_t source_ip_rule;
> + dbus_message_iter_get_basic(&value,
> &source_ip_rule);
> + creation_data->source_ip_rule = source_ip_rule;
> + } else {
> + err = -EINVAL;
> + goto err;
> + }
> + break;
> + default:
> + goto err;
> }
gcc 6 still complains here:
src/session.c: In function ?__connman_session_create?:
src/session.c:1583:9: error: ?err? may be used uninitialized in this
function [-Werror=maybe-uninitialized]
return err;
Thanks,
Daniel
------------------------------
Message: 5
Date: Fri, 3 Feb 2017 07:39:40 +0100
From: Daniel Wagner <[email protected]>
To: Lukasz Nowak <[email protected]>, [email protected]
Subject: Re: [PATCH v4 2/8] session: Add AllowedInterface config
parameter
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Lukasz,
On 01/31/2017 03:13 PM, Lukasz Nowak wrote:
> @@ -1105,6 +1141,26 @@ static DBusMessage *change_session(DBusConnection
> *conn,
> info->config.type = apply_policy_on_type(
> session->policy_config->type,
> connman_session_parse_connection_type(val));
> + } else if (g_str_equal(name, "AllowedInterface")) {
> + dbus_message_iter_get_basic(&value, &val);
> + if (session->active)
> + set_active_session(session, false);
> +
> + session->active = false;
> + session_deactivate(session);
> +
> + g_free(session->user_allowed_interface);
> + /* empty string means allow any interface */
> + if (0 == g_strcmp0(val, ""))
if (!g_strcmp0(...))
because of consistency
Thanks,
Daniel
------------------------------
Message: 6
Date: Fri, 3 Feb 2017 07:44:47 +0100
From: Daniel Wagner <[email protected]>
To: Daryl Nebrich <[email protected]>, [email protected]
Subject: Re: [PATCH] session: Set allowed_bearers to NULL before
reusing list
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Daryl
On 02/02/2017 03:31 PM, Daryl Nebrich wrote:
> Crash when modifying session policy file with connman already running. Reset
> allowed_bearers list to NULL before reusing. NULL considered empty list.
Patch applied after splitting the commit message into several lines :)
Thanks,
Daniel
> ---
> src/session.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/session.c b/src/session.c
> index b1e9e1b..267477b 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -600,6 +600,7 @@ void connman_session_set_default_config(struct
> connman_session_config *config)
> config->ecall = FALSE;
>
> g_slist_free(config->allowed_bearers);
> + config->allowed_bearers = NULL;
> add_default_bearer_types(&config->allowed_bearers);
> }
>
>
------------------------------
Message: 7
Date: Fri, 3 Feb 2017 07:50:50 +0100
From: Daniel Wagner <[email protected]>
To: Daryl Nebrich <[email protected]>, [email protected]
Subject: Re: [PATCH] session: Maintain bearer priority in policy file
and check for other services when service lost
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Daryl,
On 02/02/2017 04:05 PM, Daryl Nebrich wrote:
> The session_policy_local files can have multiple services listed in
> AllowedBearers.
> The order was not being maintained by apply_policy_on_bearers(). Reversed
> loop
> order so that the priority order in policy file is maintained.
>
> session_match_service() now checks the priority of a new service before
> switching.
>
> When a session service is lost, handle_service_state_offline() removes the
> old service
> but does not check for other available services. Added call to
> session_activate().
Looks pretty good. Though I would prefer to have two patches one which
does the logic fix in the filtering and the second which introduces the
additional check. Actually make it three patches, because the last one
'added call to session_activate()' deserves an description in the commit
message why you do it and we don't need to comment then.
> ---
> src/session.c | 32 ++++++++++++++++++++++----------
> 1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index 267477b..d40db0f 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -686,18 +686,18 @@ static int parse_bearers(DBusMessageIter *iter, GSList
> **list)
> return 0;
> }
>
> -static void filter_bearer(GSList *policy_bearers,
> - enum connman_service_type bearer,
> +static void filter_bearer(GSList *bearers,
> + enum connman_service_type policy,
> GSList **list)
> {
> - enum connman_service_type policy;
> + enum connman_service_type bearer;
> GSList *it;
>
> - if (!policy_bearers)
> + if (!bearers)
> return;
>
> - for (it = policy_bearers; it; it = it->next) {
> - policy = GPOINTER_TO_INT(it->data);
> + for (it = bearers; it; it = it->next) {
> + bearer = GPOINTER_TO_INT(it->data);
>
> if (policy != bearer)
> continue;
> @@ -710,15 +710,15 @@ static void filter_bearer(GSList *policy_bearers,
> static void apply_policy_on_bearers(GSList *policy_bearers, GSList *bearers,
> GSList **list)
> {
> - enum connman_service_type bearer;
> + enum connman_service_type policy_bearer;
> GSList *it;
>
> *list = NULL;
>
> - for (it = bearers; it; it = it->next) {
> - bearer = GPOINTER_TO_INT(it->data);
> + for (it = policy_bearers; it; it = it->next) {
> + policy_bearer = GPOINTER_TO_INT(it->data);
>
> - filter_bearer(policy_bearers, bearer, list);
> + filter_bearer(bearers, policy_bearer, list);
> }
> }
So this above is the first patch.
> @@ -1549,15 +1549,24 @@ static bool session_match_service(struct
> connman_session *session,
> {
> enum connman_service_type bearer_type;
> enum connman_service_type service_type;
> + enum connman_service_type current_service_type;
> GSList *list;
>
> if (policy && policy->allowed)
> return policy->allowed(session, service);
>
> + /* Handle multiple bearers and priorities in session policy. */
> + current_service_type = connman_service_get_type(session->service);
> +
> for (list = session->info->config.allowed_bearers; list; list =
> list->next) {
> bearer_type = GPOINTER_TO_INT(list->data);
> service_type = connman_service_get_type(service);
>
> + /* Keep using current service if higher priority. */
> + if (bearer_type == current_service_type) {
> + return false;
> + }
> +
Second one.
And for one liners you can leave the brackets away, so just do
if (bearer_type == current_service_type)
return false;
> if (bearer_type == service_type)
> return true;
> }
> @@ -1689,6 +1698,9 @@ static void handle_service_state_offline(struct
> connman_service *service,
>
> session->service = NULL;
> update_session_state(session);
> +
> + /* Check if another allowed bearer is available. */
> + session_activate(session);
> }
> }
Third patch. If we have a nice commit message we don't need the comment
here.
Thanks,
Daniel
------------------------------
Message: 8
Date: Fri, 3 Feb 2017 07:56:27 +0100
From: Daniel Wagner <[email protected]>
To: Andr? Draszik <[email protected]>, [email protected]
Subject: Re: ofono: cellular service disconnect makes it disappear
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
Hi Andr?,
On 02/02/2017 10:49 AM, Andr? Draszik wrote:
> We're using connman 1.33 & ofono 1.19 to manage cellular connections using a
> Telit LE910 modem, and once we disconnect the cellular service using
> connman, it completely disappears from connman's list of services (the
> cellular technology remains known to connman).
As Dayrl pointed out, best thing would be to try the current HEAD of
ConnMan first. We had some bug fixes since 1.33. Also oFono.
> To make the service known to connman again we have to instruct ofono to
> disable the modem and re-enable it again.
That sounds strange indeed.
> Is this expected behaviour? I would guess that it isn't, but I am no expert
> in this area and would like confirmation one way or another please.
>
> I could imagine this to be an issue in either connman, ofono, or ofono's
> modem driver, or even the hardware (modem firmware?). Any thoughts on that?
There are few things you can try for debugging. There is monitor-ofono
which shows you want oFono is sending over D-Bus to ConnMan. Check with
list-modes what oFono is showing ConnMan. And you might want to enable
the debug output in ConnMan to see what ConnMan is doing:
connmand -n -d plugins/ofono.c
But I would strongly suggest to try the current head first.
> Do other people see similar behaviour?
No, it should work as you expect it.
Thanks,
Daniel
------------------------------
Message: 9
Date: Fri, 3 Feb 2017 07:58:27 +0100
From: Daniel Wagner <[email protected]>
To: Daryl Nebrich <[email protected]>, Andr? Draszik
<[email protected]>
Cc: [email protected]
Subject: Re: ofono: cellular service disconnect makes it disappear
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed
Hi Daryl,
On 02/02/2017 03:45 PM, Daryl Nebrich wrote:
> Hi,
>
> I'm using connman tip, ofono 1.18 with a Telit LE920 modem. I don't
> see the service go away. When first connected, the cellular service
> shows up in connmanctl as *AO or *AR. After disconnecting it, it's
> still listed as *A. I can then reconnect it.
>
> However, I've made a change to src/config.c in connman to auto-connect
> cellular. This might explain the different behavior I'm seeing. Not
> sure if my change is the right way, but cellular wasn't auto
> connecting without it, which is what I need. When searching the old
> forum messages, I recall seeing discussion of changes in cellular
> connect behavior. Not sure of the back story. I pasted my unofficial
> local patch below.
Cellular should also autoconnect if you ask me. So if that snippet below
is needed to get it rolling, I suggest to send it as proper patch and I
will review it :)
Thanks,
Daniel
------------------------------
Message: 10
Date: Fri, 3 Feb 2017 09:20:09 +0100
From: Jose Blanquicet <[email protected]>
To: [email protected], "MANIEZZO Marco (MM)"
<[email protected]>
Subject: Re: Service remains on service list even if it is not longer
available
Message-ID:
<CAFC8iJKrPDEMZ_R98-SMwJaS8X5kp8rjLP3Y+6ccWUfR=ru...@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
On Fri, Jan 27, 2017 at 4:28 PM, Jose Blanquicet <[email protected]> wrote:
> When we get disconnected because of AP we are connected to goes out of
> range. ConnMan keeps showing such AP in the services list even though
> it is not longer available (We cannot connect to it). In addition,
> such AP will continue in our service list even if we connect to a new
> one. What is the propose of keeping that AP in the services list? From
> our point of view it creates confusion to the user. We would prefer to
> not do that, what do people think?
Ping? Is it how it supposed to be?
Cheers,
Jose Blanquicet
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman
------------------------------
End of connman Digest, Vol 16, Issue 3
**************************************
