Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/2] mesh: Add sequence nr getter to the doc
@ 2020-01-14 11:49 Jakub Witowski
  2020-01-14 11:49 ` [PATCH 2/2] mesh: Add sequence nr getter code Jakub Witowski
  2020-01-14 14:50 ` [PATCH 1/2] mesh: Add sequence nr getter to the doc Gix, Brian
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Witowski @ 2020-01-14 11:49 UTC (permalink / raw)
  To: linux-bluetooth

---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mesh: Add sequence nr getter code
  2020-01-14 11:49 [PATCH 1/2] mesh: Add sequence nr getter to the doc Jakub Witowski
@ 2020-01-14 11:49 ` Jakub Witowski
  2020-01-14 14:50 ` [PATCH 1/2] mesh: Add sequence nr getter to the doc Gix, Brian
  1 sibling, 0 replies; 6+ messages in thread
From: Jakub Witowski @ 2020-01-14 11:49 UTC (permalink / raw)
  To: linux-bluetooth

---
 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc
  2020-01-14 11:49 [PATCH 1/2] mesh: Add sequence nr getter to the doc Jakub Witowski
  2020-01-14 11:49 ` [PATCH 2/2] mesh: Add sequence nr getter code Jakub Witowski
@ 2020-01-14 14:50 ` Gix, Brian
  2020-01-14 15:40   ` Michał Lowas-Rzechonek
  1 sibling, 1 reply; 6+ messages in thread
From: Gix, Brian @ 2020-01-14 14:50 UTC (permalink / raw)
  To: jakub.witowski, linux-bluetooth; +Cc: Stotland, Inga

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc
  2020-01-14 14:50 ` [PATCH 1/2] mesh: Add sequence nr getter to the doc Gix, Brian
@ 2020-01-14 15:40   ` Michał Lowas-Rzechonek
  2020-01-14 16:39     ` Gix, Brian
  0 siblings, 1 reply; 6+ messages in thread
From: Michał Lowas-Rzechonek @ 2020-01-14 15:40 UTC (permalink / raw)
  To: Gix, Brian; +Cc: jakub.witowski, linux-bluetooth, Stotland, Inga

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc
  2020-01-14 15:40   ` Michał Lowas-Rzechonek
@ 2020-01-14 16:39     ` Gix, Brian
  2020-01-14 16:53       ` michal.lowas-rzechonek
  0 siblings, 1 reply; 6+ messages in thread
From: Gix, Brian @ 2020-01-14 16:39 UTC (permalink / raw)
  To: michal.lowas-rzechonek; +Cc: jakub.witowski, linux-bluetooth, Stotland, Inga

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.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] mesh: Add sequence nr getter to the doc
  2020-01-14 16:39     ` Gix, Brian
@ 2020-01-14 16:53       ` michal.lowas-rzechonek
  0 siblings, 0 replies; 6+ messages in thread
From: michal.lowas-rzechonek @ 2020-01-14 16:53 UTC (permalink / raw)
  To: Gix, Brian; +Cc: jakub.witowski, linux-bluetooth, Stotland, Inga

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 11:49 [PATCH 1/2] mesh: Add sequence nr getter to the doc Jakub Witowski
2020-01-14 11:49 ` [PATCH 2/2] mesh: Add sequence nr getter code Jakub Witowski
2020-01-14 14:50 ` [PATCH 1/2] mesh: Add sequence nr getter to the doc Gix, Brian
2020-01-14 15:40   ` Michał Lowas-Rzechonek
2020-01-14 16:39     ` Gix, Brian
2020-01-14 16:53       ` michal.lowas-rzechonek

Linux-Bluetooth Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-bluetooth/0 linux-bluetooth/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-bluetooth linux-bluetooth/ https://lore.kernel.org/linux-bluetooth \
		linux-bluetooth@vger.kernel.org
	public-inbox-index linux-bluetooth

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-bluetooth


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git