2019-07-01 12:40:49

by Jakub Witowski

[permalink] [raw]

Subject: [PATCH] mesh: Allow to set-up the CRPL with application

---

This implementation adds possibility of adding CRPL to the node
via application in the same way as CIP VID or PID.
---
doc/mesh-api.txt | 4 ++++
mesh/node.c | 12 ++++++++++--
2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 4e0a8bff1..45fc431fa 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -724,6 +724,10 @@ Properties:

A 16-bit vendor-assigned product version identifier

+ uint16 CRPL [read-only]
+
+ A 16-bit reply protection value
+

Mesh Element Hierarchy
======================
diff --git a/mesh/node.c b/mesh/node.c
index 4e35bb3ff..9372d540a 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -55,7 +55,6 @@
/* Default element location: unknown */
#define DEFAULT_LOCATION 0x0000

-#define DEFAULT_CRPL 10
#define DEFAULT_SEQUENCE_NUMBER 0

#define REQUEST_TYPE_JOIN 0
@@ -1302,7 +1301,6 @@ static void set_defaults(struct mesh_node *node)
if (!node->comp)
node->comp = l_new(struct node_composition, 1);

- node->comp->crpl = DEFAULT_CRPL;
node->lpn = MESH_MODE_UNSUPPORTED;
node->proxy = MESH_MODE_UNSUPPORTED;
node->friend = MESH_MODE_UNSUPPORTED;
@@ -1359,6 +1357,16 @@ static bool get_app_properties(struct mesh_node *node, const char *path,
return false;

node->comp->vid = value;
+
+ } else if (!strcmp(key, "CRPL")) {
+ if (!l_dbus_message_iter_get_variant(&variant, "q",
+ &value))
+ return false;
+
+ if (!is_new && node->comp->crpl != value)
+ return false;
+
+ node->comp->crpl = value;
}
}

--
2.20.1

2019-07-02 05:24:33

by Stotland, Inga

[permalink] [raw]

Subject: Re: [PATCH] mesh: Allow to set-up the CRPL with application

---

Hi Jakub,

On Mon, 2019-07-01 at 13:42 +0200, Jakub Witowski wrote:
> This implementation adds possibility of adding CRPL to the node
> via application in the same way as CIP VID or PID.

Basically, you're exposing CRPL as a property so let's rephrase
the commit message as something like:
"This adds an optional CRPL property to org.bluez.mesh.Application1
interface, allowing to indicate the depth of reply protection list."

> ---
> doc/mesh-api.txt | 4 ++++
> mesh/node.c | 12 ++++++++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index 4e0a8bff1..45fc431fa 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -724,6 +724,10 @@ Properties:
>
> A 16-bit vendor-assigned product version identifier
>
> + uint16 CRPL [read-only]

[read-only] -> [read-only, optional]

I'd prefer this to be an optional property. If not found, the daemon
uses its default setting (see comment below)

> +
> + A 16-bit reply protection value

A 16-bit minimum number of replay protection list entries

> +
>
> Mesh Element Hierarchy
> ======================
> diff --git a/mesh/node.c b/mesh/node.c
> index 4e35bb3ff..9372d540a 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -55,7 +55,6 @@
> /* Default element location: unknown */
> #define DEFAULT_LOCATION 0x0000
>
> -#define DEFAULT_CRPL 10

Let's keep the default value and make the property optional.

> #define DEFAULT_SEQUENCE_NUMBER 0
>
> #define REQUEST_TYPE_JOIN 0
> @@ -1302,7 +1301,6 @@ static void set_defaults(struct mesh_node
> *node)
> if (!node->comp)
> node->comp = l_new(struct node_composition, 1);
>
> - node->comp->crpl = DEFAULT_CRPL;

Let's remove the node->comp allocation here altogether.
Instead, in get_app_properties, add the default value setting:

if (is_new) {
node->comp = l_new(struct node_composition, 1);
node->comp->crpl = DEFAULT_CRPL;
}

If CRPL property is present, the default is overwritten.

> node->lpn = MESH_MODE_UNSUPPORTED;
> node->proxy = MESH_MODE_UNSUPPORTED;
> node->friend = MESH_MODE_UNSUPPORTED;
> @@ -1359,6 +1357,16 @@ static bool get_app_properties(struct
> mesh_node *node, const char *path,
> return false;
>
> node->comp->vid = value;
> +
> + } else if (!strcmp(key, "CRPL")) {
> + if (!l_dbus_message_iter_get_variant(&variant,
> "q",
> +
> &value))
> + return false;
> +
> + if (!is_new && node->comp->crpl != value)
> + return false;
> +
> + node->comp->crpl = value;
> }
> }
>

---

Attachments:

smime.p7s (3.19 kB)