[RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[Michał Lowas-Rzechonek]

This patchset aims to make the API a bit more consistent.

Side question: at the moment none of the node properties emit
PropertiesChanged signal. I think this violates D-Bus spec, since all
properties are assumed to emit these signals by default [1] [2].

Unfortunately, at the moment ELL does not support "EmitsChangedSignal"
annotation, so I'd like to add this to ELL, annotate node
properties with:
    - Features: const
    - Beacon: true
    - BeaconFlags: true
    - IvIndex: true
    - SecondsSinceLastHeard: false (for performance reasons)
    - Addresses: const

And also emit PropertiesChanged where required.

Thoughts?


[1] https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties

    Each property (or the parent interface) must be annotated with the
    org.freedesktop.DBus.Property.EmitsChangedSignal annotation to
    convey this (usually the default value true is sufficient meaning
    that the annotation does not need to be used). See the section
    called “Introspection Data Format” for details on this annotation.

[2] https://dbus.freedesktop.org/doc/dbus-specification.html#introspection-format

    org.freedesktop.DBus.Property.EmitsChangedSignal:
        true,invalidates,const,false	

    If set to false, the
    org.freedesktop.DBus.Properties.PropertiesChanged signal, see the
    section called “org.freedesktop.DBus.Properties” is not guaranteed
    to be emitted if the property changes.

    If set to const the property never changes value during the lifetime
    of the object it belongs to, and hence the signal is never emitted
    for it.

    If set to invalidates the signal is emitted but the value is not
    included in the signal.

    If set to true the signal is emitted with the value included.

    The value for the annotation defaults to true if the enclosing
    interface element does not specify the annotation. Otherwise it
    defaults to the value specified in the enclosing interface element.

    This annotation is intended to be used by code generators to
    implement client-side caching of property values. For all properties
    for which the annotation is set to const, invalidates or true the
    client may unconditionally cache the values as the properties don't
    change or notifications are generated for them if they do.

Michał Lowas-Rzechonek (2):
  mesh: Rename IVUpdate import flag to IvUpdate
  mesh: Change BeaconFlags property type to a dict

 doc/mesh-api.txt | 20 +++++++++++++++-----
 mesh/mesh.c      |  2 +-
 mesh/node.c      | 13 +++++++++++--
 3 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.19.1

[RFC BlueZ 1/2] mesh: Rename IVUpdate import flag to IvUpdate

[Michał Lowas-Rzechonek]

Name change for consistency with "IvIndex" property.
---
 doc/mesh-api.txt | 2 +-
 mesh/mesh.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 6f9531019..b33c24e12 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -188,7 +188,7 @@ Methods:
 		The flags parameter is a dictionary containing provisioning
 		flags. Supported values are:
 
-			boolean IVUpdate
+			boolean IvUpdate
 
 				When true, indicates that the network is in the
 				middle of IV Index Update procedure.
diff --git a/mesh/mesh.c b/mesh/mesh.c
index 6d2f86b6d..972beecc9 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -777,7 +777,7 @@ static struct l_dbus_message *import_call(struct l_dbus *dbus,
 							"Bad net index");
 
 	while (l_dbus_message_iter_next_entry(&iter_flags, &key, &var)) {
-		if (!strcmp(key, "IVUpdate")) {
+		if (!strcmp(key, "IvUpdate")) {
 			if (!l_dbus_message_iter_get_variant(&var, "b",
 								&ivu))
 				goto fail;
-- 
2.19.1

[RFC BlueZ 2/2] mesh: Change BeaconFlags property type to a dict

[Michał Lowas-Rzechonek]

Instead of a binary value, return a dictionary consistent with
"flags" argument used in Import().
---
 doc/mesh-api.txt | 18 ++++++++++++++----
 mesh/node.c      | 13 +++++++++++--
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index b33c24e12..33077bcfc 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -432,11 +432,21 @@ Properties:
 		This property indicates whether the periodic beaconing is
 		enabled (true) or disabled (false).
 
-	uint8 BeaconFlags [read-only]
+	dict BeaconFlags [read-only]
 
-		This property may be read at any time to determine the flag
-		field setting on sent and received beacons of the primary
-		network key.
+		This property may be read at any time to determine the flags
+		used in network beacons of the primary network key. Supported
+		values are:
+
+			boolean IvUpdate
+
+				When true, indicates that the network is in the
+				middle of IV Index Update procedure.
+
+			boolean KeyRefresh
+
+				When true, indicates that the network is in the
+				middle of a key refresh procedure.
 
 	uint32 IvIndex [read-only]
 
diff --git a/mesh/node.c b/mesh/node.c
index 0ad935105..0e28c650b 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -32,6 +32,7 @@
 #include "mesh/mesh-defs.h"
 #include "mesh/mesh.h"
 #include "mesh/net.h"
+#include "mesh/net-keys.h"
 #include "mesh/appkey.h"
 #include "mesh/mesh-config.h"
 #include "mesh/provision.h"
@@ -2233,10 +2234,18 @@ static bool beaconflags_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
 	struct mesh_net *net = node_get_net(node);
 	uint8_t flags;
 	uint32_t iv_index;
+	bool ivu;
+	bool kr;
 
 	mesh_net_get_snb_state(net, &flags, &iv_index);
 
-	l_dbus_message_builder_append_basic(builder, 'y', &flags);
+	ivu = flags & IV_INDEX_UPDATE;
+	kr = flags & KEY_REFRESH;
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+	dbus_append_dict_entry_basic(builder, "IvUpdate", "b", &ivu);
+	dbus_append_dict_entry_basic(builder, "KeyRefresh", "b", &kr);
+	l_dbus_message_builder_leave_array(builder);
 
 	return true;
 }
@@ -2337,7 +2346,7 @@ static void setup_node_interface(struct l_dbus_interface *iface)
 	l_dbus_interface_property(iface, "Features", 0, "a{sv}", features_getter,
 									NULL);
 	l_dbus_interface_property(iface, "Beacon", 0, "b", beacon_getter, NULL);
-	l_dbus_interface_property(iface, "BeaconFlags", 0, "b",
+	l_dbus_interface_property(iface, "BeaconFlags", 0, "a{sv}",
 						beaconflags_getter, NULL);
 	l_dbus_interface_property(iface, "IvIndex", 0, "u", ivindex_getter,
 									NULL);
-- 
2.19.1

Re: [RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[Michał Lowas-Rzechonek]

Hi Brian, Inga,

On 01/09, Michał Lowas-Rzechonek wrote:
> This patchset aims to make the API a bit more consistent.
> 
> Side question: at the moment none of the node properties emit
> PropertiesChanged signal. I think this violates D-Bus spec, since all
> properties are assumed to emit these signals by default [1] [2].
> 
> Unfortunately, at the moment ELL does not support "EmitsChangedSignal"
> annotation, so I'd like to add this to ELL, annotate node
> properties with:
>     - Features: const
>     - Beacon: true
>     - BeaconFlags: true
>     - IvIndex: true
>     - SecondsSinceLastHeard: false (for performance reasons)
>     - Addresses: const
> 
> And also emit PropertiesChanged where required.
> 
> Thoughts?

Any comments about this idea, or should I just go ahead and send a
patch?

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[Stotland, Inga]

Hi Michal,

On Tue, 2020-01-21 at 21:15 +0100, Michał Lowas-Rzechonek wrote:
> Hi Brian, Inga,
> 
> On 01/09, Michał Lowas-Rzechonek wrote:
> > This patchset aims to make the API a bit more consistent.
> > 
> > Side question: at the moment none of the node properties emit
> > PropertiesChanged signal. I think this violates D-Bus spec, since all
> > properties are assumed to emit these signals by default [1] [2].
> > 
> > Unfortunately, at the moment ELL does not support "EmitsChangedSignal"
> > annotation, so I'd like to add this to ELL, annotate node
> > properties with:
> >     - Features: const
> >     - Beacon: true
> >     - BeaconFlags: true
> >     - IvIndex: true
> >     - SecondsSinceLastHeard: false (for performance reasons)
> >     - Addresses: const
> > 
> > And also emit PropertiesChanged where required.
> > 
> > Thoughts?
> 
> Any comments about this idea, or should I just go ahead and send a
> patch?
> 

I think this is a good idea. I agree with the proposed node properties
annotations.

Thanks,
Inga

Re: [RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[Gix, Brian]

Hi Inga & Michał,

On Wed, 2020-01-22 at 00:44 +0000, Stotland, Inga wrote:
> Hi Michal,
> 
> On Tue, 2020-01-21 at 21:15 +0100, Michał Lowas-Rzechonek wrote:
> > Hi Brian, Inga,
> > 
> > On 01/09, Michał Lowas-Rzechonek wrote:
> > > This patchset aims to make the API a bit more consistent.
> > > 
> > > Side question: at the moment none of the node properties emit
> > > PropertiesChanged signal. I think this violates D-Bus spec, since all
> > > properties are assumed to emit these signals by default [1] [2].
> > > 
> > > Unfortunately, at the moment ELL does not support "EmitsChangedSignal"
> > > annotation, so I'd like to add this to ELL, annotate node
> > > properties with:
> > >     - Features: const
> > >     - Beacon: true
> > >     - BeaconFlags: true
> > >     - IvIndex: true
> > >     - SecondsSinceLastHeard: false (for performance reasons)
> > >     - Addresses: const
> > > 
> > > And also emit PropertiesChanged where required.
> > > 
> > > Thoughts?
> > 
> > Any comments about this idea, or should I just go ahead and send a
> > patch?
> > 
> 
> I think this is a good idea. I agree with the proposed node properties
> annotations.


I honestly would like to never emit any signals for Mesh, and if it is legally possible for
SecondsSinceLastHeard (a property which literally changes value every second) then I hope it is possible and
legal to never emit anything....   They should be polled only.

Unless someone can come up with a use case that requires signals to notify a specific piece of information has
changed, Signal Emissions are Information Leaks that provide no benefit...  they are sent globally to anyone on
the DBus, and signals to anyone who may be eavesdropping that *something* is happening, even if the details of
the change are "hidden".

Are there any use cases that necessitate DBus Signals in Mesh?

--Brian

Re: [RFC BlueZ 0/2] Fix types and names of beacon/import flags.

[michal.lowas-rzechonek]

Brian, Inga,

On 01/22, Gix, Brian wrote:
> Signal Emissions are Information Leaks

No, they aren't. Flags and IV Index value is public information, sent in
open text over the air in Secure Network Beacons.

> that provide no benefit...  they are sent globally to anyone on the
> DBus, and signals to anyone who may be eavesdropping that *something*
> is happening

Yeah, and you get the same info from listening to beacons. I would just
like to make it slightly easier to listen to locally. At the moment one
needs to wait for the beacon to be transmitted.


Anyway: what's your opinion about the proposed API modifications (IvUpdate
rename and BeaconFlags type change)?

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND