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 v3 6/7] iwd: Add/remove ConnMan devices (Daniel Wagner)
   2. Re: Spurious failures starting ConnMan with systemd (Sam Nazarko)
   3. Re: Spurious failures starting ConnMan with systemd
      (Robert Tiemann)
   4. Re: Spurious failures starting ConnMan with systemd
      (Daniel Wagner)
   5. Re: Spurious failures starting ConnMan with systemd
      (Robert Tiemann)
   6. Re: [RFC 2/4] main: Make -d option repeatable (Daniel Wagner)
   7. Re: [PATCH v3 0/7] IWD plugin (Daniel Wagner)


----------------------------------------------------------------------

Message: 1
Date: Tue, 15 Nov 2016 14:58:47 +0100
From: Daniel Wagner <[email protected]>
To: [email protected]
Cc: Daniel Wagner <[email protected]>
Subject: [PATCH v3 6/7] iwd: Add/remove ConnMan devices
Message-ID: <[email protected]>

From: Daniel Wagner <[email protected]>

---
 plugins/iwd.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

diff --git a/plugins/iwd.c b/plugins/iwd.c
index ba93710e94b4..1cb2f0d63508 100644
--- a/plugins/iwd.c
+++ b/plugins/iwd.c
@@ -26,12 +26,14 @@
 #include <errno.h>
 #include <string.h>
 #include <stdbool.h>
+#include <linux/if_ether.h>
 
 #define CONNMAN_API_SUBJECT_TO_CHANGE
 #include <connman/plugin.h>
 #include <connman/dbus.h>
 #include <connman/network.h>
 #include <connman/technology.h>
+#include <connman/inet.h>
 #include <gdbus.h>
 
 static DBusConnection *connection;
@@ -78,6 +80,8 @@ struct iwd_device {
        enum iwd_device_state state;
        bool powered;
        bool scanning;
+
+       struct connman_device *device;
 };
 
 struct iwd_network {
@@ -147,6 +151,17 @@ static bool proxy_get_bool(GDBusProxy *proxy, const char 
*property)
        return value;
 }
 
+static void address2ident(const char *address, char *ident)
+{
+       int i;
+
+       for (i = 0; i < ETH_ALEN; i++) {
+               ident[i * 2] = address[i * 3];
+               ident[i * 2 + 1] = address[i * 3 + 1];
+       }
+       ident[ETH_ALEN * 2] = '\0';
+}
+
 static int cm_network_probe(struct connman_network *network)
 {
        return -EOPNOTSUPP;
@@ -172,6 +187,18 @@ static struct connman_network_driver network_driver = {
 
 static int cm_device_probe(struct connman_device *device)
 {
+       GHashTableIter iter;
+       gpointer key, value;
+
+       g_hash_table_iter_init(&iter, devices);
+
+       while (g_hash_table_iter_next(&iter, &key, &value)) {
+               struct iwd_device *iwdd = value;
+
+               if (device == iwdd->device)
+                       return 0;
+       }
+
        return -EOPNOTSUPP;
 }
 
@@ -179,8 +206,49 @@ static void cm_device_remove(struct connman_device *device)
 {
 }
 
+struct dev_cb_data {
+       char *path;
+       bool powered;
+};
+
+static void device_powered_cb(const DBusError *error, void *user_data)
+{
+       struct dev_cb_data *cbd = user_data;
+       struct iwd_device *iwdd;
+
+       iwdd = g_hash_table_lookup(devices, cbd->path);
+       if (!iwdd)
+               goto out;
+
+       if (dbus_error_is_set(error)) {
+               connman_warn("WiFi device %s not enabled %s",
+                               cbd->path, error->message);
+               goto out;
+       }
+
+       connman_device_set_powered(iwdd->device, cbd->powered);
+out:
+       g_free(cbd->path);
+       g_free(cbd);
+}
+
 static int set_device_powered(struct connman_device *device, bool powered)
 {
+       struct iwd_device *iwdd = connman_device_get_data(device);
+       dbus_bool_t device_powered = powered;
+       struct dev_cb_data *cbd;
+
+       if (proxy_get_bool(iwdd->proxy, "Powered"))
+               return -EALREADY;
+
+       cbd = g_new(struct dev_cb_data, 1);
+       cbd->path = g_strdup(iwdd->path);
+       cbd->powered = powered;
+
+       g_dbus_proxy_set_property_basic(iwdd->proxy, "Powered",
+                       DBUS_TYPE_BOOLEAN, &device_powered,
+                       device_powered_cb, cbd, NULL);
+
        return -EINPROGRESS;
 }
 
@@ -219,6 +287,36 @@ static struct connman_technology_driver tech_driver = {
        .remove         = cm_tech_remove,
 };
 
+static void add_device(const char *path, struct iwd_device *iwdd)
+{
+       char ident[ETH_ALEN * 2 + 1];
+
+       iwdd->device = connman_device_create("wifi", CONNMAN_DEVICE_TYPE_WIFI);
+       if (!iwdd->device)
+               return;
+
+       connman_device_set_data(iwdd->device, iwdd);
+
+       address2ident(iwdd->address, ident);
+       connman_device_set_ident(iwdd->device, ident);
+
+       if (connman_device_register(iwdd->device) < 0) {
+               g_hash_table_remove(devices, path);
+               return;
+       }
+
+       connman_device_set_powered(iwdd->device, iwdd->powered);
+}
+
+static void remove_device(struct iwd_device *iwdd)
+{
+       if (!iwdd->device)
+               return;
+
+       connman_device_unref(iwdd->device);
+       iwdd->device = NULL;
+}
+
 static void adapter_property_change(GDBusProxy *proxy, const char *name,
                DBusMessageIter *iter, void *user_data)
 {
@@ -328,6 +426,8 @@ static void device_free(gpointer data)
                iwdd->proxy = NULL;
        }
 
+       remove_device(iwdd);
+
        g_free(iwdd->path);
        g_free(iwdd->adapter);
        g_free(iwdd->name);
@@ -422,6 +522,8 @@ static void create_device(GDBusProxy *proxy)
 
        g_dbus_proxy_set_property_watch(iwdd->proxy,
                        device_property_change, NULL);
+
+       add_device(path, iwdd);
 }
 
 static void unregister_agent();
-- 
2.7.4


------------------------------

Message: 2
Date: Tue, 15 Nov 2016 14:37:03 +0000
From: Sam Nazarko <[email protected]>
To: "[email protected]" <[email protected]>
Subject: Re: Spurious failures starting ConnMan with systemd
Message-ID: <[email protected]>
Content-Type: text/plain; charset="iso-8859-1"

>Probably basic.target is something that is reached no matter what, but
>an After=dbus.service is not. In my system I see an additional
>dependency only on dbus.socket, and all works fine. This with systemd
>231 and Debian.

Not sure if this helps, but in OSMC (Debian Jessie) we use:

After=dbus.service network-pre.target
Wants=network.target remote-fs-pre.target
Before=remote-fs-pre.target network.target

Sam Nazarko
_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

Message: 3
Date: Tue, 15 Nov 2016 15:46:42 +0100
From: Robert Tiemann <[email protected]>
To: [email protected]
Subject: Re: Spurious failures starting ConnMan with systemd
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed

Hi!

On 11/15/2016 03:37 PM, Sam Nazarko wrote:

> Not sure if this helps, but in OSMC (Debian Jessie) we use:
>
> After=dbus.service network-pre.target
> Wants=network.target remote-fs-pre.target
> Before=remote-fs-pre.target network.target

This is more or less what was shipped with ConnMan v1.30. It works for
me as well.

> Sam Nazarko

Kind regards,
Robert


------------------------------

Message: 4
Date: Tue, 15 Nov 2016 16:02:07 +0100
From: Daniel Wagner <[email protected]>
To: Colin Guthrie <[email protected]>, Robert Tiemann <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: Spurious failures starting ConnMan with systemd
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8

Hi Colin,

On 11/15/2016 03:46 PM, Colin Guthrie wrote:
> Hope you're keeping well?

Doing fine! How is live in the north? 
 
> So, by default, dbus should be socket activated. That means that when
> dbus.service eventually starts shouldn't really matter, provided it is
> eventually started. This is because it's actually dbus.socket that's the
> important unit. It should be started by sockets.target which is pulled
> in as part of the default dependencies that all units get automatically
> (provided they've not disabled this)

Okay, that makes sense.

> So, check for dbus.socket, and check that connman.service doesn't
> disable default deps.

I think we do, see below.

> If socket activation is used, then there shouldn't be any need to
> mention dbus.socket/service in connman.service at all.

"""
[Unit]
Description=Connection service
DefaultDependencies=false
Conflicts=shutdown.target
RequiresMountsFor=/var/lib/connman
After=dbus.service network-pre.target systemd-sysusers.service
Before=network.target multi-user.target shutdown.target
Wants=network.target

[Service]
Type=dbus
BusName=net.connman
Restart=on-failure
ExecStart=/usr/sbin/connmand -n
StandardOutput=null
CapabilityBoundingSet=CAP_KILL CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW 
CAP_SYS_TIME CAP_SYS_MODULE
ProtectHome=true
ProtectSystem=full

[Install]
WantedBy=multi-user.target
"""

Okay, so the DefaultDependencies=false is causing this problem then.
In our commit history I found following explanation for it:

"""
commit 09aa0243aac40ec4e5bd0fbe41e702be4952a382
Author: Patrik Flykt <[email protected]>
Date:   Thu Sep 17 10:42:46 2015 +0300

    connman.service: Fix dependencies for early boot
    
    Unset default dependencies in order to properly run at early boot and
    require the save directory to be mounted before starting.
    
    See the systemd.unit man page, Debian's wiki page
    https://wiki.debian.org/Teams/pkg-systemd/rcSMigration and the upstream
    systemd-networkd.service file for details.

diff --git a/src/connman.service.in b/src/connman.service.in
index 8f7f3429f7dc..0a8f15c9f90b 100644
--- a/src/connman.service.in
+++ b/src/connman.service.in
@@ -1,7 +1,10 @@
 [Unit]
 Description=Connection service
+DefaultDependencies=false
+Conflicts=shutdown.target
+RequiresMountsFor=@localstatedir@/lib/connman
 After=dbus.service network-pre.target
-Before=network.target remote-fs-pre.target
+Before=network.target shutdown.target remote-fs-pre.target
 Wants=network.target remote-fs-pre.target
 
 [Service]
"""

Hmm, now I am confused...

Thanks,
Daniel


------------------------------

Message: 5
Date: Tue, 15 Nov 2016 16:13:37 +0100
From: Robert Tiemann <[email protected]>
To: Patrik Flykt <[email protected]>, Daniel Wagner
        <[email protected]>, [email protected], [email protected]
Subject: Re: Spurious failures starting ConnMan with systemd
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8; format=flowed

>       Hi,

Hi!

> Probably basic.target is something that is reached no matter what, but
> an After=dbus.service is not. In my system I see an additional
> dependency only on dbus.socket, and all works fine. This with systemd
> 231 and Debian.

Thanks for the hint! This seems to be my problem: no automatic
dependency on dbus.socket is added, which, as already mentioned by
Colin, should be enough. I've checked the systemd git repo for
suspicious commits between v225 and v231, and I found

45f06b3 - core: pull in dbus.socket from Type=dbus services

This patch was applied between v227 and v228 about a year ago and
implements the documented behavior. I guess the best would be to keep
my change local until I can upgrade to a later systemd version. Sorry
for the noise.

If you still want a patch from me to support pre-v228 systemd systems,
I'll happily prepare one for you.

> Cheers,
>
>       Patrik

Kind regards,
Robert


------------------------------

Message: 6
Date: Tue, 15 Nov 2016 19:02:40 +0100
From: Daniel Wagner <[email protected]>
To: Slava Monich <[email protected]>, [email protected]
Subject: Re: [RFC 2/4] main: Make -d option repeatable
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed

Hi Slava,

On 10/06/2016 03:37 PM, Slava Monich wrote:
> On 06/10/16 15:48, Daniel Wagner wrote:
>> From: Slava Monich <[email protected]>
>>
>> Concatenating the patterns makes more sense than using the last
>> supplied value and leaking the previous allocated patterns.
>>
>> [wagi: make also -d option without argument repeatable]
>> ---
>>   src/main.c | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/main.c b/src/main.c
>> index fdb4f724cf75..db97cd54fb43 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -476,10 +476,17 @@ static gboolean option_version = FALSE;
>>   static bool parse_debug(const char *key, const char *value,
>>                       gpointer user_data, GError **error)
>>   {
>> -    if (value)
>> +    if (!value)
>> +        value = "*";
>> +
>> +    if (option_debug) {
>> +        char *prev = option_debug;
>> +
>> +        option_debug = g_strconcat(prev, ",", value, NULL);
>> +        g_free(prev);
>> +    } else {
>>           option_debug = g_strdup(value);
>> -    else
>> -        option_debug = g_strdup("*");
>> +    }
>>         return true;
>>   }
>
>
> Hmm,
>
> The only difference from my patch is that if no value is passed in, my
> patch would overwrite the existing pattern with a single "*", and yours
> would append "*" to it. Why is that important?

Can you resend your patch? I dropped my patch because it is just too ugly.

cheers,
daniel


------------------------------

Message: 7
Date: Tue, 15 Nov 2016 19:44:43 +0100
From: Daniel Wagner <[email protected]>
To: [email protected]
Subject: Re: [PATCH v3 0/7] IWD plugin
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed

All patches applied.


------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 13, Issue 24
***************************************

Reply via email to