[PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[Michał Lowas-Rzechonek]

This patch-set implements previously defined properties on Node1
interface and adds two additional properties: list of unicast addresses
claimed by the node and the current sequence number value.

Michał Lowas-Rzechonek (2):
  mesh: Implement properties on org.bluez.mesh.Node1 interface
  mesh: Add properties to Node1 interface

 doc/mesh-api.txt |   9 +++
 mesh/net.c       |   4 ++
 mesh/net.h       |   1 +
 mesh/node.c      | 143 ++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 156 insertions(+), 1 deletion(-)

-- 
2.19.1

[PATCH BlueZ 1/2] mesh: Implement properties on org.bluez.mesh.Node1 interface

[Michał Lowas-Rzechonek]

---
 mesh/net.c  |   4 ++
 mesh/net.h  |   1 +
 mesh/node.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/mesh/net.c b/mesh/net.c
index 7c4049e0e..2785039db 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3945,3 +3945,7 @@ void mesh_net_set_prov(struct mesh_net *net, struct mesh_prov *prov)
 	net->prov = prov;
 }
 
+uint32_t mesh_net_get_instant(struct mesh_net *net)
+{
+	return net->instant;
+}
diff --git a/mesh/net.h b/mesh/net.h
index 55e89ca72..150240ff8 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -359,3 +359,4 @@ void mesh_net_transmit_params_get(struct mesh_net *net, uint8_t *count,
 							uint16_t *interval);
 struct mesh_prov *mesh_net_get_prov(struct mesh_net *net);
 void mesh_net_set_prov(struct mesh_net *net, struct mesh_prov *prov);
+uint32_t mesh_net_get_instant(struct mesh_net *net);
diff --git a/mesh/node.c b/mesh/node.c
index 0d7e45c90..3d9ded3b1 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -24,6 +24,7 @@
 #define _GNU_SOURCE
 #include <dirent.h>
 #include <stdio.h>
+#include <sys/time.h>
 
 #include <ell/ell.h>
 
@@ -2103,6 +2104,100 @@ static struct l_dbus_message *vendor_publish_call(struct l_dbus *dbus,
 	return  l_dbus_message_new_method_return(msg);
 }
 
+static bool features_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	uint8_t friend = node_friend_mode_get(node);
+	uint8_t lpn = node_lpn_mode_get(node);
+	uint8_t proxy = node_proxy_mode_get(node);
+	uint8_t count;
+	uint16_t interval;
+	uint8_t relay = node_relay_mode_get(node, &count, &interval);
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+
+	if (friend != MESH_MODE_UNSUPPORTED)
+		dbus_append_dict_entry_basic(builder, "Friend", "b", &friend);
+
+	if (lpn != MESH_MODE_UNSUPPORTED)
+		dbus_append_dict_entry_basic(builder, "LowPower", "b", &lpn);
+
+	if (proxy != MESH_MODE_UNSUPPORTED)
+		dbus_append_dict_entry_basic(builder, "Proxy", "b", &proxy);
+
+	if (relay != MESH_MODE_UNSUPPORTED)
+		dbus_append_dict_entry_basic(builder, "Relay", "b", &relay);
+
+	l_dbus_message_builder_leave_array(builder);
+
+	return true;
+}
+
+static bool beacon_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	bool beacon_mode = node_beacon_mode_get(node) == MESH_MODE_ENABLED;
+
+	l_dbus_message_builder_append_basic(builder, 'b', &beacon_mode);
+
+	return true;
+}
+
+static bool beaconflags_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
+	uint8_t flags;
+	uint32_t iv_index;
+
+	mesh_net_get_snb_state(net, &flags, &iv_index);
+
+	l_dbus_message_builder_append_basic(builder, 'y', &flags);
+
+	return true;
+}
+
+static bool ivindex_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
+	uint8_t flags;
+	uint32_t iv_index;
+
+	mesh_net_get_snb_state(net, &flags, &iv_index);
+
+	l_dbus_message_builder_append_basic(builder, 'u', &iv_index);
+
+	return true;
+}
+
+static bool lastheard_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
+	struct timeval now;
+	uint32_t last_heard;
+
+	gettimeofday(&now, NULL);
+
+	last_heard = now.tv_sec - mesh_net_get_instant(net);
+
+	l_dbus_message_builder_append_basic(builder, 'u', &last_heard);
+
+	return true;
+
+}
+
 static void setup_node_interface(struct l_dbus_interface *iface)
 {
 	l_dbus_interface_method(iface, "Send", 0, send_call, "", "oqqay",
@@ -2118,7 +2213,15 @@ static void setup_node_interface(struct l_dbus_interface *iface)
 						"", "oqqay", "element_path",
 						"vendor", "model_id", "data");
 
-	/* TODO: Properties */
+	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",
+						beaconflags_getter, NULL);
+	l_dbus_interface_property(iface, "IvIndex", 0, "u", ivindex_getter,
+									NULL);
+	l_dbus_interface_property(iface, "SecondsSinceLastHeard", 0, "u",
+					lastheard_getter, NULL);
 }
 
 bool node_dbus_init(struct l_dbus *bus)
-- 
2.19.1

[PATCH BlueZ 2/2] mesh: Add properties to Node1 interface

[Michał Lowas-Rzechonek]

---
 doc/mesh-api.txt |  9 +++++++++
 mesh/node.c      | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 255104ab6..b639ae719 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -423,6 +423,15 @@ Properties:
 		seconds since mesh network layer traffic was last detected on
 		this node's network.
 
+	array{uint16} Addresses [read-only]
+
+		This property contains unicast addresses node's elements.
+
+	uint32 SequenceNumber [read-only]
+
+		This property may be read at any time to determine currently
+		used sequence number.
+
 Mesh Provisioning Hierarchy
 ============================
 Service		org.bluez.mesh
diff --git a/mesh/node.c b/mesh/node.c
index 3d9ded3b1..789759b6f 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -2198,6 +2198,40 @@ static bool lastheard_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
 
 }
 
+static bool addresses_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	const struct l_queue_entry *entry;
+
+	l_dbus_message_builder_enter_array(builder, "q");
+
+	entry = l_queue_get_entries(node->elements);
+	for (; entry; entry = entry->next) {
+		const struct node_element *ele = entry->data;
+		uint16_t address = node->primary + ele->idx;
+
+		l_dbus_message_builder_append_basic(builder, 'q', &address);
+	}
+
+	l_dbus_message_builder_leave_array(builder);
+
+	return true;
+}
+
+static bool seqnumber_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct mesh_node *node = user_data;
+	uint32_t seq_number = node_get_sequence_number(node);
+
+	l_dbus_message_builder_append_basic(builder, 'u', &seq_number);
+
+	return true;
+}
+
 static void setup_node_interface(struct l_dbus_interface *iface)
 {
 	l_dbus_interface_method(iface, "Send", 0, send_call, "", "oqqay",
@@ -2222,6 +2256,10 @@ static void setup_node_interface(struct l_dbus_interface *iface)
 									NULL);
 	l_dbus_interface_property(iface, "SecondsSinceLastHeard", 0, "u",
 					lastheard_getter, NULL);
+	l_dbus_interface_property(iface, "Addresses", 0, "aq", addresses_getter,
+									NULL);
+	l_dbus_interface_property(iface, "SequenceNumber", 0, "u",
+							seqnumber_getter, NULL);
 }
 
 bool node_dbus_init(struct l_dbus *bus)
-- 
2.19.1

Re: [PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[Stotland, Inga]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

Hi Michal,

On Tue, 2019-08-27 at 11:08 +0200, Michał Lowas-Rzechonek wrote:
> This patch-set implements previously defined properties on Node1
> interface and adds two additional properties: list of unicast
> addresses
> claimed by the node and the current sequence number value.
> 

Could you please explain the justification for adding these two new
properties?

> Michał Lowas-Rzechonek (2):
>   mesh: Implement properties on org.bluez.mesh.Node1 interface
>   mesh: Add properties to Node1 interface
> 
>  doc/mesh-api.txt |   9 +++
>  mesh/net.c       |   4 ++
>  mesh/net.h       |   1 +
>  mesh/node.c      | 143
> ++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 156 insertions(+), 1 deletion(-)
> 

Thanks,
Inga

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

Re: [PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[michal.lowas-rzechonek]

Hi,

On 08/27, Stotland, Inga wrote:
> > adds two additional properties: list of unicast addresses
> > claimed by the node and the current sequence number value.
> Could you please explain the justification for adding these two new
> properties?

Sure thing.

The address part is useful when application would like to talk to its
own node's Config or Health Server. At the moment the address is known
when calling Import or CreateNetwork (even though the application would
then need to store it somewhere, so we end up with two sources of
truth), but after Join() the application won't know the address assigned
to it.

As for sequence number part, reading is mostly for debugging and
verification. A few our users had trouble identifying a problem in their
setup when their node was listed in other nodes' RPL.

In the end I would like to make it writable (increment-only) to enable
address reuse, but as this stage I'm still looking for a way to
implement this without causing a race condition, so I left it readonly
for now.

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

Re: [PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[Gix, Brian]

On Tue, 2019-08-27 at 20:23 +0200, michal.lowas-rzechonek@silvair.com wrote:
> Hi,
> 
> On 08/27, Stotland, Inga wrote:
> > > adds two additional properties: list of unicast addresses
> > > claimed by the node and the current sequence number value.
> > Could you please explain the justification for adding these two new
> > properties?
> 
> Sure thing.
> 
> The address part is useful when application would like to talk to its
> own node's Config or Health Server. At the moment the address is known
> when calling Import or CreateNetwork (even though the application would
> then need to store it somewhere, so we end up with two sources of
> truth), but after Join() the application won't know the address assigned
> to it.

I think I am OK with this...   It is hard to make the argument that this would be an information leak when the
information has been revealed before. And certain nodes (if they are authorized) need to be able to talk to
their own config servers.

> 
> As for sequence number part, reading is mostly for debugging and
> verification. A few our users had trouble identifying a problem in their
> setup when their node was listed in other nodes' RPL.

I am kind of sympathetic to this for debugging purposes, but I would point out that during the debugging
process, many tools are available, including adding debug logging to the daemon as needed. So adding this info
to the dmsg log for example, would be acceptable...  Especially so that it could be easily turned off at non-
debug times.

However, I don't see a reason for any *deployed* application needing this information.

> In the end I would like to make it writable (increment-only) to enable
> address reuse, but as this stage I'm still looking for a way to
> implement this without causing a race condition, so I left it readonly
> for now.

This is where I start to see actual danger, and difficulty when considering the NVM system is "pre-reserving"
sequence numbers to prevent NVM thrashing during heavy use. In the spirit of keeping an API as small as
possible, giving applications the ability to adjust the SeqNum (even in the legal direction) should have a rock
solid justification.

> regards

Re: [PATCH BlueZ 0/2] mesh: Implement org.bluez.mesh.Node1 properties

[michal.lowas-rzechonek]

Brian, Inga,

On 08/27, Gix, Brian wrote:
> > The address part is useful when application would like to talk to its
> > own node's Config or Health Server.
> > (...)
> I think I am OK with this...   It is hard to make the argument that
> this would be an information leak when the information has been
> revealed before. And certain nodes (if they are authorized) need to be
> able to talk to their own config servers.

Ack.

> > As for sequence number part (...) I would like to make it writable
> > (increment-only) to enable address reuse, but as this stage I'm
> > still looking for a way to implement this without causing a race
> > condition, so I left it readonly for now.
> 
> This is where I start to see actual danger, and difficulty when
> considering the NVM system is "pre-reserving" sequence numbers to
> prevent NVM thrashing during heavy use. In the spirit of keeping an
> API as small as possible, giving applications the ability to adjust
> the SeqNum (even in the legal direction) should have a rock solid
> justification.

Ok, fair enough. I'm gonna drop this property in v2.

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