[PATCH 1/2] mesh: Add sequence nr getter to the doc

[PATCH 1/2] mesh: Add sequence nr getter to the doc

[Jakub Witowski]

---
 doc/mesh-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index ebff8492a..d83c68bdc 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -441,6 +441,11 @@ Properties:
 
 		This property contains unicast addresses of node's elements.
 
+	uint32 SequenceNumber [read-only]
+
+		This property may be read at any time to determine the
+		sequence number.
+
 Mesh Provisioning Hierarchy
 ============================
 Service		org.bluez.mesh
-- 
2.20.1

[PATCH 2/2] mesh: Add sequence nr getter code

[Jakub Witowski]

---
 mesh/node.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/mesh/node.c b/mesh/node.c
index 3154d6bf4..723869cc0 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -2215,6 +2215,19 @@ static bool ivindex_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
 	return true;
 }
 
+static bool seq_num_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);
+	uint32_t seq_nr = mesh_net_get_seq_num(net);
+
+	l_dbus_message_builder_append_basic(builder, 'u', &seq_nr);
+
+	return true;
+}
+
 static bool lastheard_getter(struct l_dbus *dbus, struct l_dbus_message *msg,
 					struct l_dbus_message_builder *builder,
 					void *user_data)
@@ -2284,6 +2297,8 @@ static void setup_node_interface(struct l_dbus_interface *iface)
 						beaconflags_getter, NULL);
 	l_dbus_interface_property(iface, "IvIndex", 0, "u", ivindex_getter,
 									NULL);
+	l_dbus_interface_property(iface, "SequenceNumber", 0, "u",
+							seq_num_getter, NULL);
 	l_dbus_interface_property(iface, "SecondsSinceLastHeard", 0, "u",
 					lastheard_getter, NULL);
 	l_dbus_interface_property(iface, "Addresses", 0, "aq", addresses_getter,
-- 
2.20.1

Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc

[Gix, Brian]

Hi Jakub,

On Tue, 2020-01-14 at 12:49 +0100, Jakub Witowski wrote:
> ---
>  doc/mesh-api.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index ebff8492a..d83c68bdc 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -441,6 +441,11 @@ Properties:
>  
>  		This property contains unicast addresses of node's elements.
>  
> +	uint32 SequenceNumber [read-only]
> +
> +		This property may be read at any time to determine the
> +		sequence number.
> +

Is there a use case justification for exposing this value?  Why an Application would need this?


>  Mesh Provisioning Hierarchy
>  ============================
>  Service		org.bluez.mesh

Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc

[Michał Lowas-Rzechonek]

Hi Brian,

On 01/14, Gix, Brian wrote:
> > +	uint32 SequenceNumber [read-only]
> > +
> > +		This property may be read at any time to determine the
> > +		sequence number.
> > +
> 
> Is there a use case justification for exposing this value?  Why an Application would need this?

There are 2 use cases:
 - debugging and monitoring RPL behaviour
 - we'd like to add another API to increase the sequence number - this
   is useful when you Import() a node from another database and would
   like to bump its sequence number up to a previously known value, so
   that the rest of the network can talk to it

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

Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc

[Gix, Brian]

Hi Michał & Jakub,

On Tue, 2020-01-14 at 16:40 +0100, Michał Lowas-Rzechonek wrote:
> Hi Brian,
> 
> On 01/14, Gix, Brian wrote:
> > > +	uint32 SequenceNumber [read-only]
> > > +
> > > +		This property may be read at any time to determine the
> > > +		sequence number.
> > > +
> > 
> > Is there a use case justification for exposing this value?  Why an Application would need this?
> 
> There are 2 use cases:
>  - debugging and monitoring RPL behaviour

I need to think about this a bit and discuss with Inga

>  - we'd like to add another API to increase the sequence number - this
>    is useful when you Import() a node from another database and would
>    like to bump its sequence number up to a previously known value, so
>    that the rest of the network can talk to it

I have some serious discomfort with an API to increase sequence numbers.  On import,
the sequence number should be part of the data being imported, so I don't see a
direct need there to bump up the value afterwards.

Plus, we handle sequence numbers differently than many other settings in the
system.  To prevent storage thrashing, we "Pre-Reserve" a chunk of sequence
numbers that we store in node.json, and then real-time use these sequence
numbers for outbound packets without having to write to storage each time
(a power failure or other reset would then pick up after the reserved chunk).
And then it also feeds into the IV Index update feature as well.

Giving an App the ability to arbitrarily increase it's sequence number puts
it into conflict with the natural usage of sequence numbers, and when we request
the IV Index Update.

Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc

[michal.lowas-rzechonek]

Hi,

On 01/14, Gix, Brian wrote:
> I have some serious discomfort with an API to increase sequence numbers.  On import,
> the sequence number should be part of the data being imported, so I don't see a
> direct need there to bump up the value afterwards.
> 
> Plus, we handle sequence numbers differently than many other settings in the
> system.  To prevent storage thrashing, we "Pre-Reserve" a chunk of sequence
> numbers that we store in node.json, and then real-time use these sequence
> numbers for outbound packets without having to write to storage each time
> (a power failure or other reset would then pick up after the reserved chunk).
> And then it also feeds into the IV Index update feature as well.
> 
> Giving an App the ability to arbitrarily increase it's sequence number puts
> it into conflict with the natural usage of sequence numbers, and when we request
> the IV Index Update.

Ok, I appreciate that. Let's take a look from a different angle.

As I mentioned some time ago, we're working on an automated test suite
for the daemon. We need to know the current value of sequence number to
verify payloads send to radio adapter. One can say that we're
*eventually* aiming for, is a "test mode" of sorts.

A significant part of the suite is checking the IV Update logic. As
you've seen, there was/is a fair number of issues with the current
implementation. We know from (painful) experience, that this part of the
system is notoriously difficult to implement correctly and efficiently.
One of the problems is verifying time-based behaviour.

Mesh spec actually mandates that the device shall implement the test
mode by removing state timers, but it doesn't say anything about
triggering the node to start IV Update.

So maybe we in the end we could enable SequenceNumber etc. access only
when daemon is running with a certain configuration option (either
commandline, of from the file)?

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