2020-01-30 14:35:59

by Jakub Witowski

[permalink] [raw]
Subject: [PATCH BlueZ v2 0/4] Allow to reattach with new composition data

This patch allows the application to modify the CID, PID and VID in the composition data.

Version 2: Do not allow to change CRPL in the composition data.
Additionaly verify the device key when updating comp data and remove
unused function in the 3rd patch.

Version 1: According the Mesh Profile (2.3.4 Elements) the modification of fields
other than "Elements" is not prohibited.

Also in my opinion (as you can see in the 1st patch), there is no need to use pointer to
the node_composition struct. The static is more clear and less problematic.

Jakub Witowski (4):
mesh: use static node_comp instead of the pointer
mesh: add cid/pid/vid setter
mesh: remove unused node_set_device_key()
mesh: allow to reattach with new composition data

mesh/mesh-config-json.c | 40 ++++++++++++----
mesh/mesh-config.h | 2 +
mesh/node.c | 100 +++++++++++++++++++++++++---------------
mesh/node.h | 1 -
4 files changed, 96 insertions(+), 47 deletions(-)

--
2.20.1


2020-01-30 14:36:36

by Jakub Witowski

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/4] mesh: use static node_comp instead of the pointer

There is no need to use the pointer to the node_comp data.
This pach uses static node_comp instead.
---
mesh/node.c | 56 +++++++++++++++++++++++------------------------------
1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index de6e74c4f..6fe70742d 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -90,7 +90,7 @@ struct mesh_node {
uint32_t seq_number;
bool provisioner;
uint16_t primary;
- struct node_composition *comp;
+ struct node_composition comp;
struct {
uint16_t interval;
uint8_t cnt;
@@ -336,7 +336,6 @@ static void free_node_resources(void *data)
free_node_dbus_resources(node);

mesh_net_free(node->net);
- l_free(node->comp);
l_free(node->storage_dir);
l_free(node);
}
@@ -503,11 +502,10 @@ static bool init_from_storage(struct mesh_config_node *db_node,

l_queue_push_tail(nodes, node);

- node->comp = l_new(struct node_composition, 1);
- node->comp->cid = db_node->cid;
- node->comp->pid = db_node->pid;
- node->comp->vid = db_node->vid;
- node->comp->crpl = db_node->crpl;
+ node->comp.cid = db_node->cid;
+ node->comp.pid = db_node->pid;
+ node->comp.vid = db_node->vid;
+ node->comp.crpl = db_node->crpl;
node->lpn = db_node->modes.lpn;

node->proxy = db_node->modes.proxy;
@@ -753,7 +751,7 @@ uint16_t node_get_crpl(struct mesh_node *node)
if (!node)
return 0;

- return node->comp->crpl;
+ return node->comp.crpl;
}

uint8_t node_relay_mode_get(struct mesh_node *node, uint8_t *count,
@@ -886,18 +884,18 @@ uint16_t node_generate_comp(struct mesh_node *node, uint8_t *buf, uint16_t sz)
uint16_t num_ele = 0;
const struct l_queue_entry *ele_entry;

- if (!node || !node->comp || sz < MIN_COMP_SIZE)
+ if (!node || sz < MIN_COMP_SIZE)
return 0;

n = 0;

- l_put_le16(node->comp->cid, buf + n);
+ l_put_le16(node->comp.cid, buf + n);
n += 2;
- l_put_le16(node->comp->pid, buf + n);
+ l_put_le16(node->comp.pid, buf + n);
n += 2;
- l_put_le16(node->comp->vid, buf + n);
+ l_put_le16(node->comp.vid, buf + n);
n += 2;
- l_put_le16(node->comp->crpl, buf + n);
+ l_put_le16(node->comp.crpl, buf + n);
n += 2;

features = 0;
@@ -1198,10 +1196,10 @@ static void convert_node_to_storage(struct mesh_node *node,
{
const struct l_queue_entry *entry;

- db_node->cid = node->comp->cid;
- db_node->pid = node->comp->pid;
- db_node->vid = node->comp->vid;
- db_node->crpl = node->comp->crpl;
+ db_node->cid = node->comp.cid;
+ db_node->pid = node->comp.pid;
+ db_node->vid = node->comp.vid;
+ db_node->crpl = node->comp.crpl;
db_node->modes.lpn = node->lpn;
db_node->modes.proxy = node->proxy;

@@ -1285,29 +1283,28 @@ static bool get_app_properties(struct mesh_node *node, const char *path,

l_debug("path %s", path);

- node->comp = l_new(struct node_composition, 1);
- node->comp->crpl = mesh_get_crpl();
+ node->comp.crpl = mesh_get_crpl();

while (l_dbus_message_iter_next_entry(properties, &key, &variant)) {
if (!cid && !strcmp(key, "CompanyID")) {
if (!l_dbus_message_iter_get_variant(&variant, "q",
- &node->comp->cid))
- goto fail;
+ &node->comp.cid))
+ return false;
cid = true;
continue;
}

if (!pid && !strcmp(key, "ProductID")) {
if (!l_dbus_message_iter_get_variant(&variant, "q",
- &node->comp->pid))
- goto fail;
+ &node->comp.pid))
+ return false;
pid = true;
continue;
}

if (!vid && !strcmp(key, "VersionID")) {
if (!l_dbus_message_iter_get_variant(&variant, "q",
- &node->comp->vid))
+ &node->comp.vid))
return false;
vid = true;
continue;
@@ -1315,21 +1312,16 @@ static bool get_app_properties(struct mesh_node *node, const char *path,

if (!strcmp(key, "CRPL")) {
if (!l_dbus_message_iter_get_variant(&variant, "q",
- &node->comp->crpl))
- goto fail;
+ &node->comp.crpl))
+ return false;
continue;
}
}

if (!cid || !pid || !vid)
- goto fail;
+ return false;

return true;
-fail:
- l_free(node->comp);
- node->comp = NULL;
-
- return false;
}

static bool add_local_node(struct mesh_node *node, uint16_t unicast, bool kr,
--
2.20.1

2020-01-30 14:36:36

by Jakub Witowski

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/4] mesh: remove unused node_set_device_key()

This patch removes node_set_device_key() function,
because it is unused.
---
mesh/node.c | 5 -----
mesh/node.h | 1 -
2 files changed, 6 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index 6fe70742d..d4be070fa 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -628,11 +628,6 @@ uint16_t node_get_primary(struct mesh_node *node)
return node->primary;
}

-void node_set_device_key(struct mesh_node *node, uint8_t key[16])
-{
- memcpy(node->dev_key, key, 16);
-}
-
const uint8_t *node_get_device_key(struct mesh_node *node)
{
if (!node)
diff --git a/mesh/node.h b/mesh/node.h
index a6bc4a2a6..38aea138f 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -46,7 +46,6 @@ uint16_t node_get_primary(struct mesh_node *node);
uint16_t node_get_primary_net_idx(struct mesh_node *node);
void node_set_token(struct mesh_node *node, uint8_t token[8]);
const uint8_t *node_get_token(struct mesh_node *node);
-void node_set_device_key(struct mesh_node *node, uint8_t key[16]);
const uint8_t *node_get_device_key(struct mesh_node *node);
void node_set_num_elements(struct mesh_node *node, uint8_t num_ele);
uint8_t node_get_num_elements(struct mesh_node *node);
--
2.20.1

2020-01-31 18:30:27

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 0/4] Allow to reattach with new composition data

Hi Jakub,
On Thu, 2020-01-30 at 15:34 +0100, Jakub Witowski wrote:
> This patch allows the application to modify the CID, PID and VID in the composition data.
>
> Version 2: Do not allow to change CRPL in the composition data.
> Additionaly verify the device key when updating comp data and remove
> unused function in the 3rd patch.
>
> Version 1: According the Mesh Profile (2.3.4 Elements) the modification of fields
> other than "Elements" is not prohibited.
>
> Also in my opinion (as you can see in the 1st patch), there is no need to use pointer to
> the node_composition struct. The static is more clear and less problematic.
>
> Jakub Witowski (4):
> mesh: use static node_comp instead of the pointer
> mesh: add cid/pid/vid setter
> mesh: remove unused node_set_device_key()
> mesh: allow to reattach with new composition data

Patches 1 and 3 of this patchset have been applied, as they are non-controversial.

I would like to wait a little while, as the Working group weighs in, on modifying composition data.

I am actually ready today to allow an App to Attach to an existing node, with modifications to it's CID/PID/VID
(drop daemon validation) with the understanding that the composition stored in node.json is not changed. But
anything that changes OTA behavior, I would like blessed by the WG and the SIG.


>
> mesh/mesh-config-json.c | 40 ++++++++++++----
> mesh/mesh-config.h | 2 +
> mesh/node.c | 100 +++++++++++++++++++++++++---------------
> mesh/node.h | 1 -
> 4 files changed, 96 insertions(+), 47 deletions(-)
>