linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API
@ 2019-07-17  8:36 Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation Michał Lowas-Rzechonek
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17  8:36 UTC (permalink / raw)
  To: linux-bluetooth

+rebased after mesh-config refactoring
+updated documentation
+removed import_req
+added patch to remove void pointers from managed_obj_request

Jakub Witowski (2):
  mesh: Add ImportLocalNode API documentation
  mesh: Extract read_* functions in mesh-config-json

Michał Lowas-Rzechonek (2):
  mesh: Implement ImportLocalNode() method
  mesh: Convert void pointers to anonymous unions in
    managed_obj_request:wq

 doc/mesh-api.txt        |  52 ++++++++++---
 mesh/mesh-config-json.c | 161 +++++++++++++++++++++++++++++++++++-----
 mesh/mesh-config.h      |  10 +++
 mesh/mesh-defs.h        |   3 +
 mesh/mesh.c             |  90 +++++++++++++++++++++-
 mesh/net.h              |   2 -
 mesh/node.c             | 151 ++++++++++++++++++++++++++-----------
 mesh/node.h             |   5 ++
 8 files changed, 399 insertions(+), 75 deletions(-)

-- 
2.19.1


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

* [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
  2019-07-17  8:36 [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API Michał Lowas-Rzechonek
@ 2019-07-17  8:36 ` Michał Lowas-Rzechonek
  2019-07-17 19:36   ` Stotland, Inga
  2019-07-17  8:36 ` [PATCH BlueZ v5 2/4] mesh: Extract read_* functions in mesh-config-json Michał Lowas-Rzechonek
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jakub Witowski

From: Jakub Witowski <jakub.witowski@silvair.com>

This updates the mesh-api.txt with new ImportLocalNode() API.
---
 doc/mesh-api.txt | 52 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 0ac2fdfd1..7c2a1fafa 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -151,16 +151,31 @@ Methods:
 			org.bluez.mesh.Error.InvalidArguments
 			org.bluez.mesh.Error.AlreadyExists,
 
-	 uint64 token ImportLocalNode(string json_data)
+	uint64 token ImportLocalNode(object app_root, array{byte}[16] uuid,
+							string data_type, array{byte} import_data)
 
 		This method creates a local mesh node based on node
 		configuration that has been generated outside bluetooth-meshd.
 
-		The json_data parameter is a full JSON representation of a node
-		configuration file. The format must conform to the schema
-		defined in "Mesh Node Configuration Schema" section. Any
-		included token will be ignored in favor of a locally generated
-		token value.
+		The app_root parameter is a D-Bus object root path of the
+		application that implements org.bluez.mesh.Application1
+		interface.
+
+		The import_data parameter contains a representation of a
+		provisioned node. Format of this representation depends on
+		value of data_type parameter.
+
+		Allowed data_type values are: "json".
+
+		When data_type is "json", bluetooth-meshd daemon treats
+		import_data is a JSON document following <TBD> schema. See the
+		examples at the end of this document.
+
+		The import_data parameter can contain either minimal, or
+		complete representation of a provisioned node.
+
+		When a complete representation is provided, it is validated
+		against composition data provided by the application.
 
 		The returned token must be preserved by the application in
 		order to authenticate itself to the mesh daemon and attach to
@@ -173,8 +188,8 @@ Methods:
 
 		PossibleErrors:
 			org.bluez.mesh.Error.InvalidArguments,
-			org.bluez.mesh.Error.AlreadyExists
-			org.bluez.mesh.Error.NotFound,
+			org.bluez.mesh.Error.AlreadyExists,
+			org.bluez.mesh.Error.NotSupported,
 			org.bluez.mesh.Error.Failed
 
 Mesh Node Hierarchy
@@ -1064,6 +1079,21 @@ Properties:
 		Uniform Resource Identifier points to out-of-band (OOB)
 		information (e.g., a public key)
 
-Mesh Node Configuration Schema
-==============================
-<TBD>
+Mesh Node Configuration Examples
+================================
+Minimal JSON representation for ImportLocalNode():
+
+{
+	"IVindex":0,
+	"IVupdate":0,
+	"unicastAddress":"0012",
+	"deviceKey":"7daa45cd1e9e11a4b86eeef7d01efa11",
+	"netKeys":[
+		{
+			"index":"0000",
+			"key":"2ddfef86d67144c394428ea3078f86f9",
+			"keyRefresh":0
+		}
+	],
+	"sequenceNumber":15  /* optional */
+}
-- 
2.19.1


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

* [PATCH BlueZ v5 2/4] mesh: Extract read_* functions in mesh-config-json
  2019-07-17  8:36 [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation Michał Lowas-Rzechonek
@ 2019-07-17  8:36 ` Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 3/4] mesh: Implement ImportLocalNode() method Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 4/4] mesh: Convert void pointers to anonymous unions in managed_obj_request:wq Michał Lowas-Rzechonek
  3 siblings, 0 replies; 9+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17  8:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jakub Witowski

From: Jakub Witowski <jakub.witowski@silvair.com>

This enables more code to be reused in ImportLocalNode implementation,
when using 'json' import data format.
---
 mesh/mesh-config-json.c | 76 +++++++++++++++++++++++++++++++----------
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
index 75015e607..6df7f7b3f 100644
--- a/mesh/mesh-config-json.c
+++ b/mesh/mesh-config-json.c
@@ -52,6 +52,8 @@ struct write_info {
 	mesh_config_status_func_t cb;
 };
 
+typedef bool (*read_net_key_cb)(struct mesh_config_netkey *, void*);
+
 static const char *cfg_name = "/node.json";
 static const char *bak_ext = ".bak";
 static const char *tmp_ext = ".tmp";
@@ -297,6 +299,33 @@ static json_object *jarray_key_del(json_object *jarray, int16_t idx)
 	return jarray_new;
 }
 
+static bool read_unicast_address(json_object *jobj,
+							uint16_t *unicast)
+{
+	json_object *jvalue;
+	char *str;
+
+	if (!json_object_object_get_ex(jobj, "unicastAddress", &jvalue))
+		return false;
+
+	str = (char *)json_object_get_string(jvalue);
+	if (sscanf(str, "%04hx", unicast) != 1)
+		return false;
+
+	return true;
+}
+
+static bool read_seq_number(json_object *jobj, uint32_t *seq_number)
+{
+	json_object *jvalue;
+
+	if (!json_object_object_get_ex(jobj, "sequenceNumber", &jvalue))
+		return false;
+
+	*seq_number = json_object_get_int(jvalue);
+	return true;
+}
+
 static bool read_iv_index(json_object *jobj, uint32_t *idx, bool *update)
 {
 	int tmp;
@@ -424,12 +453,35 @@ fail:
 	return false;
 }
 
-static bool read_net_keys(json_object *jobj,  struct mesh_config_node *node)
+static bool read_net_key(struct mesh_config_netkey *net_key,
+							void *user_data)
+{
+	struct mesh_config_node *node = user_data;
+
+	if (!net_key) {
+		l_queue_destroy(node->netkeys, l_free);
+		node->netkeys = NULL;
+		return false;
+	}
+
+	if (!node->netkeys)
+		node->netkeys = l_queue_new();
+
+	l_queue_push_tail(node->netkeys, net_key);
+
+	return true;
+}
+
+static bool read_net_keys(json_object *jobj, read_net_key_cb cb,
+							void *user_data)
 {
 	json_object *jarray;
 	int len;
 	int i;
 
+	if (!cb)
+		return true;
+
 	/* At least one NetKey must be present for a provisioned node */
 	if (!json_object_object_get_ex(jobj, "netKeys", &jarray))
 		return false;
@@ -441,8 +493,6 @@ static bool read_net_keys(json_object *jobj,  struct mesh_config_node *node)
 	if (!len)
 		return false;
 
-	node->netkeys = l_queue_new();
-
 	for (i = 0; i < len; ++i) {
 		json_object *jtemp, *jvalue;
 		char *str;
@@ -480,13 +530,12 @@ static bool read_net_keys(json_object *jobj,  struct mesh_config_node *node)
 		if (!str2hex(str, strlen(str), netkey->key, 16))
 			goto fail;
 
-		l_queue_push_tail(node->netkeys, netkey);
+		cb(netkey, user_data);
 	}
 
 	return true;
 fail:
-	l_queue_destroy(node->netkeys, l_free);
-	node->netkeys = NULL;
+	cb(NULL, user_data);
 
 	return false;
 }
@@ -1294,7 +1343,6 @@ static bool read_net_transmit(json_object *jobj, struct mesh_config_node *node)
 static bool read_node(json_object *jnode, struct mesh_config_node *node)
 {
 	json_object *jvalue;
-	char *str;
 
 	if (!read_iv_index(jnode, &node->iv_index, &node->iv_update)) {
 		l_info("Failed to read IV index");
@@ -1318,14 +1366,7 @@ static bool read_node(json_object *jnode, struct mesh_config_node *node)
 
 	parse_features(jnode, node);
 
-	if (!json_object_object_get_ex(jnode, "unicastAddress", &jvalue)) {
-		l_info("Bad config: Unicast address must be present");
-		return false;
-	}
-
-	str = (char *)json_object_get_string(jvalue);
-	if (sscanf(str, "%04hx", &node->unicast) != 1)
-		return false;
+	read_unicast_address(jnode, &node->unicast);
 
 	if (json_object_object_get_ex(jnode, "defaultTTL", &jvalue)) {
 		int ttl = json_object_get_int(jvalue);
@@ -1335,8 +1376,7 @@ static bool read_node(json_object *jnode, struct mesh_config_node *node)
 		node->ttl = (uint8_t) ttl;
 	}
 
-	if (json_object_object_get_ex(jnode, "sequenceNumber", &jvalue))
-		node->seq_number = json_object_get_int(jvalue);
+	read_seq_number(jnode, &node->seq_number);
 
 	/* Check for required "elements" property */
 	if (!json_object_object_get_ex(jnode, "elements", &jvalue))
@@ -1347,7 +1387,7 @@ static bool read_node(json_object *jnode, struct mesh_config_node *node)
 		return false;
 	}
 
-	if (!read_net_keys(jnode, node)) {
+	if (!read_net_keys(jnode, read_net_key, node)) {
 		l_info("Failed to read net keys");
 		return false;
 	}
-- 
2.19.1


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

* [PATCH BlueZ v5 3/4] mesh: Implement ImportLocalNode() method
  2019-07-17  8:36 [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 2/4] mesh: Extract read_* functions in mesh-config-json Michał Lowas-Rzechonek
@ 2019-07-17  8:36 ` Michał Lowas-Rzechonek
  2019-07-17  8:36 ` [PATCH BlueZ v5 4/4] mesh: Convert void pointers to anonymous unions in managed_obj_request:wq Michał Lowas-Rzechonek
  3 siblings, 0 replies; 9+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17  8:36 UTC (permalink / raw)
  To: linux-bluetooth

This implements ImportLocalNode() method of org.bluez.mesh.Network1
interface, allowing applications to create provisioned nodes based on
passed import data, without relying on external provisioner.
---
 mesh/mesh-config-json.c |  85 ++++++++++++++++++++++++++++++++
 mesh/mesh-config.h      |  10 ++++
 mesh/mesh-defs.h        |   3 ++
 mesh/mesh.c             |  90 +++++++++++++++++++++++++++++++++-
 mesh/net.h              |   2 -
 mesh/node.c             | 106 ++++++++++++++++++++++++++++++++++------
 mesh/node.h             |   5 ++
 7 files changed, 282 insertions(+), 19 deletions(-)

diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
index 6df7f7b3f..e80df19a5 100644
--- a/mesh/mesh-config-json.c
+++ b/mesh/mesh-config-json.c
@@ -472,6 +472,25 @@ static bool read_net_key(struct mesh_config_netkey *net_key,
 	return true;
 }
 
+static bool import_net_key(struct mesh_config_netkey *net_key,
+							void *user_data)
+{
+	struct mesh_config_import *import = user_data;
+
+	if (!net_key)
+		return false;
+
+	if (import->net_key.idx != UNUSED_KEY_IDX)
+		return false;
+
+	if (net_key->phase != KEY_REFRESH_PHASE_NONE)
+		return false;
+
+	memcpy(&import->net_key, net_key, sizeof(import->net_key));
+
+	return true;
+}
+
 static bool read_net_keys(json_object *jobj, read_net_key_cb cb,
 							void *user_data)
 {
@@ -2256,3 +2275,69 @@ void mesh_config_destroy(struct mesh_config *cfg)
 	/* Release node config object */
 	mesh_config_release(cfg);
 }
+
+
+struct mesh_config_import *mesh_config_import_create(const char *import_data)
+{
+	json_object *jobj = json_tokener_parse(import_data);
+	struct mesh_config_import *import = NULL;
+
+	if (!jobj || json_object_get_type(jobj) != json_type_object)
+		goto fail;
+
+	import = l_new(struct mesh_config_import, 1);
+	import->node = l_new(struct mesh_config_node, 1);
+	import->net_key.idx = UNUSED_KEY_IDX;
+
+	if (!read_device_key(jobj, import->dev_key)) {
+		l_error("Failed to parse imported device key");
+		goto fail;
+	}
+
+	json_object_object_del(jobj, "deviceKey");
+
+	if (!read_unicast_address(jobj, &import->node->unicast)) {
+		l_error("Failed to parse imported unicast");
+		goto fail;
+	}
+
+	json_object_object_del(jobj, "unicastAddress");
+
+	if (!read_iv_index(jobj, &import->iv_index,
+							&import->iv_update)) {
+		l_error("Failed to parse iv_index");
+		goto fail;
+	}
+
+	json_object_object_del(jobj, "IVindex");
+	json_object_object_del(jobj, "IVupdate");
+
+	if (!read_net_keys(jobj, import_net_key, import)) {
+		l_error("Failed to parse imported network key");
+		goto fail;
+	}
+
+	json_object_object_del(jobj, "netKeys");
+
+	if (!read_seq_number(jobj, &import->node->seq_number))
+		import->node->seq_number = DEFAULT_SEQUENCE_NUMBER;
+
+	json_object_object_del(jobj, "sequenceNumber");
+
+	if (json_object_object_length(jobj) != 0) {
+		l_error("Unexpected keys in import data");
+		goto fail;
+	}
+
+	return import;
+fail:
+	if (jobj)
+		json_object_put(jobj);
+
+	if (import) {
+		l_free(import->node);
+		l_free(import);
+	}
+
+	return NULL;
+}
diff --git a/mesh/mesh-config.h b/mesh/mesh-config.h
index 44e3b3ad6..0061862b5 100644
--- a/mesh/mesh-config.h
+++ b/mesh/mesh-config.h
@@ -104,6 +104,15 @@ struct mesh_config_node {
 	uint8_t dev_key[16];
 	uint8_t token[8];
 };
+
+struct mesh_config_import {
+       struct mesh_config_node *node;
+       struct mesh_config_netkey net_key;
+       uint8_t dev_key[16];
+       uint32_t iv_index;
+       bool iv_update;
+};
+
 typedef void (*mesh_config_status_func_t)(void *user_data, bool result);
 typedef bool (*mesh_config_node_func_t)(struct mesh_config_node *node,
 							const uint8_t uuid[16],
@@ -119,6 +128,7 @@ bool mesh_config_save(struct mesh_config *cfg, bool no_wait,
 struct mesh_config *mesh_config_create(const char *cfg_path,
 						const uint8_t uuid[16],
 						struct mesh_config_node *node);
+struct mesh_config_import *mesh_config_import_create(const char *import_data);
 
 bool mesh_config_write_net_transmit(struct mesh_config *cfg, uint8_t cnt,
 							uint16_t interval);
diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
index 1a199f156..79b38c56c 100644
--- a/mesh/mesh-defs.h
+++ b/mesh/mesh-defs.h
@@ -37,6 +37,7 @@
 #define KEY_REFRESH_PHASE_THREE	0x03
 
 #define DEFAULT_TTL		0xff
+#define DEFAULT_SEQUENCE_NUMBER	0
 
 /* Supported algorithms for provisioning */
 #define ALG_FIPS_256_ECC	0x0001
@@ -76,6 +77,8 @@
 
 #define PRIMARY_NET_IDX		0x0000
 #define MAX_KEY_IDX		0x0fff
+#define UNUSED_KEY_IDX		0xffff
+
 #define MAX_MODEL_COUNT		0xff
 #define MAX_ELE_COUNT		0xff
 
diff --git a/mesh/mesh.c b/mesh/mesh.c
index 9c6b9a70e..ca268e4c9 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -33,6 +33,7 @@
 #include "mesh/error.h"
 #include "mesh/agent.h"
 #include "mesh/mesh.h"
+#include "mesh/mesh-config.h"
 
 /*
  * The default values for mesh configuration. Can be
@@ -69,6 +70,10 @@ struct join_data{
 	uint8_t *uuid;
 };
 
+static const char * const supported_import_data_types[] = {
+	"json"
+};
+
 static struct bt_mesh mesh;
 
 /* We allow only one outstanding Join request */
@@ -383,6 +388,18 @@ fail:
 	free_pending_join_call(true);
 }
 
+static bool validate_data_type(const char *data_type)
+{
+	uint8_t idx = 0;
+	uint8_t len = L_ARRAY_SIZE(supported_import_data_types);
+
+	for (idx = 0; idx < len; idx++) {
+		if (strcmp(data_type, supported_import_data_types[idx]) == 0)
+			return true;
+	}
+	return false;
+}
+
 static struct l_dbus_message *join_network_call(struct l_dbus *dbus,
 						struct l_dbus_message *msg,
 						void *user_data)
@@ -536,7 +553,7 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
 	return l_dbus_message_new_method_return(msg);
 }
 
-static void create_network_ready_cb(void *user_data, int status,
+static void create_node_ready_cb(void *user_data, int status,
 							struct mesh_node *node)
 {
 	struct l_dbus_message *reply;
@@ -593,12 +610,74 @@ static struct l_dbus_message *create_network_call(struct l_dbus *dbus,
 
 	l_queue_push_tail(pending_queue, pending_msg);
 
-	node_create(app_path, sender, uuid, create_network_ready_cb,
+	node_create(app_path, sender, uuid, create_node_ready_cb,
 								pending_msg);
 
 	return NULL;
 }
 
+static struct l_dbus_message *import_local_node_call(struct l_dbus *dbus,
+						struct l_dbus_message *msg,
+						void *user_data)
+{
+	const char *app_path, *sender;
+	struct l_dbus_message *pending_msg;
+	struct l_dbus_message_iter iter_uuid, iter_import_data;
+	struct mesh_config_import *import;
+	const char *data_type, *import_data;
+	uint8_t *uuid;
+	uint32_t n;
+
+	l_debug("Import local node request");
+
+	if (!l_dbus_message_get_arguments(msg, "oaysay", &app_path, &iter_uuid,
+						&data_type, &iter_import_data))
+		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
+
+	if (!validate_data_type(data_type))
+		return dbus_error(msg, MESH_ERROR_NOT_IMPLEMENTED,
+						"Unsupported data type");
+
+	if (!l_dbus_message_iter_get_fixed_array(&iter_uuid, &uuid, &n) ||
+									n != 16)
+		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, "Bad dev UUID");
+
+	if (node_find_by_uuid(uuid))
+		return dbus_error(msg, MESH_ERROR_ALREADY_EXISTS,
+							"Node already exists");
+
+	if (!l_dbus_message_iter_get_fixed_array(&iter_import_data,
+							&import_data, &n))
+		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
+							"Bad import_data");
+
+	import = mesh_config_import_create(import_data);
+
+	if (!import)
+		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
+						"Failed to parse import_data");
+
+	sender = l_dbus_message_get_sender(msg);
+	pending_msg = l_dbus_message_ref(msg);
+
+	if (!pending_queue)
+		pending_queue = l_queue_new();
+
+	l_queue_push_tail(pending_queue, pending_msg);
+
+	if (!node_import(app_path, sender, import, uuid, create_node_ready_cb,
+								pending_msg))
+		goto fail;
+
+	return NULL;
+
+fail:
+	l_free(import);
+	l_dbus_message_unref(msg);
+	l_queue_remove(pending_queue, pending_msg);
+	return dbus_error(msg, MESH_ERROR_INVALID_ARGS, "Node import failed");
+}
+
 static void setup_network_interface(struct l_dbus_interface *iface)
 {
 	l_dbus_interface_method(iface, "Join", 0, join_network_call, "",
@@ -612,8 +691,15 @@ static void setup_network_interface(struct l_dbus_interface *iface)
 
 	l_dbus_interface_method(iface, "Leave", 0, leave_call, "", "t",
 								"token");
+
 	l_dbus_interface_method(iface, "CreateNetwork", 0, create_network_call,
 					"t", "oay", "token", "app", "uuid");
+
+	l_dbus_interface_method(iface, "ImportLocalNode", 0,
+					import_local_node_call,
+					"t", "oaysay", "token",
+					"app", "uuid", "data_type",
+					"import_data");
 }
 
 bool mesh_dbus_init(struct l_dbus *dbus)
diff --git a/mesh/net.h b/mesh/net.h
index 8848e6df0..7e6af8714 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -26,8 +26,6 @@ struct mesh_node;
 
 #define DEV_ID	0
 
-#define UNUSED_KEY_IDX	0xffff
-
 #define APP_ID_DEV	0
 #define APP_ID_ANY	((unsigned int) -1)
 #define NET_ID_ANY	(APP_ID_ANY - 1)
diff --git a/mesh/node.c b/mesh/node.c
index 652551756..d1b37a5da 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -56,11 +56,13 @@
 #define DEFAULT_LOCATION 0x0000
 
 #define DEFAULT_CRPL 10
-#define DEFAULT_SEQUENCE_NUMBER 0
 
-#define REQUEST_TYPE_JOIN 0
-#define REQUEST_TYPE_ATTACH 1
-#define REQUEST_TYPE_CREATE 2
+enum request_type {
+	REQUEST_TYPE_JOIN = 0,
+	REQUEST_TYPE_ATTACH,
+	REQUEST_TYPE_CREATE,
+	REQUEST_TYPE_IMPORT,
+};
 
 struct node_element {
 	char *path;
@@ -112,8 +114,9 @@ struct mesh_node {
 struct managed_obj_request {
 	void *data;
 	void *cb;
-	void *user_data;
-	uint8_t type;
+	struct l_dbus_message *pending_msg;
+	enum request_type type;
+	struct mesh_config_import *import;
 };
 
 static struct l_queue *nodes;
@@ -1568,14 +1571,13 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	}
 
 	if (is_new) {
-		node = l_new(struct mesh_node, 1);
+		node = node_new(req->data);
 		node->elements = l_queue_new();
 	} else {
 		node = req->data;
 	}
 
 	num_ele = 0;
-
 	while (l_dbus_message_iter_next_entry(&objects, &path, &interfaces)) {
 		struct l_dbus_message_iter properties;
 		const char *interface;
@@ -1652,7 +1654,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 
 			node->disc_watch = l_dbus_add_disconnect_watch(bus,
 					node->owner, app_disc_cb, node, NULL);
-			cb(req->user_data, MESH_ERROR_NONE, node);
+			cb(req->pending_msg, MESH_ERROR_NONE, node);
 		} else
 			goto fail;
 
@@ -1674,6 +1676,51 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 
 		cb(node, agent);
 
+	} else if (req->type == REQUEST_TYPE_IMPORT) {
+
+		node_ready_func_t cb = req->cb;
+		struct mesh_config_import *import = req->import;
+		struct keyring_net_key net_key;
+
+		if (!agent) {
+			l_error("Interface %s not found",
+						MESH_PROVISION_AGENT_INTERFACE);
+			goto fail;
+		}
+
+		node->num_ele = num_ele;
+		set_defaults(node);
+
+		if (node->seq_number != import->node->seq_number)
+			node->seq_number = import->node->seq_number;
+
+		memcpy(node->uuid, req->data, 16);
+
+		if (!create_node_config(node, node->uuid))
+			goto fail;
+
+		if (!add_local_node(node, import->node->unicast, false,
+					import->iv_update, import->iv_index,
+					import->dev_key, import->net_key.idx,
+					import->net_key.key))
+			goto fail;
+
+		memcpy(net_key.old_key, import->net_key.key, 16);
+		net_key.net_idx = import->net_key.idx;
+		net_key.phase = KEY_REFRESH_PHASE_NONE;
+
+		/* Initialize directory for storing keyring info */
+		init_storage_dir(node);
+
+		if (!keyring_put_remote_dev_key(node, import->node->unicast,
+						num_ele, import->dev_key))
+			goto fail;
+
+		if (!keyring_put_net_key(node, import->net_key.idx, &net_key))
+			goto fail;
+
+		cb(req->pending_msg, MESH_ERROR_NONE, node);
+
 	} else {
 		/* Callback for create node request */
 		node_ready_func_t cb = req->cb;
@@ -1709,7 +1756,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		if (!keyring_put_net_key(node, PRIMARY_NET_IDX, &net_key))
 			goto fail;
 
-		cb(req->user_data, MESH_ERROR_NONE, node);
+		cb(req->pending_msg, MESH_ERROR_NONE, node);
 	}
 
 	return;
@@ -1723,9 +1770,9 @@ fail:
 
 		free_node_dbus_resources(node);
 
-		cb(req->user_data, MESH_ERROR_FAILED, node);
+		cb(req->pending_msg, MESH_ERROR_FAILED, node);
 	} else {
-		/* Handle failed Join and Create requests */
+		/* Handle failed Join, Create and Import requests */
 		if (node)
 			node_remove(node);
 
@@ -1736,7 +1783,7 @@ fail:
 		} else {
 			node_ready_func_t cb = req->cb;
 
-			cb(req->user_data, MESH_ERROR_FAILED, NULL);
+			cb(req->pending_msg, MESH_ERROR_FAILED, NULL);
 		}
 	}
 }
@@ -1764,7 +1811,7 @@ int node_attach(const char *app_path, const char *sender, uint64_t token,
 	req = l_new(struct managed_obj_request, 1);
 	req->data = node;
 	req->cb = cb;
-	req->user_data = user_data;
+	req->pending_msg = user_data;
 	req->type = REQUEST_TYPE_ATTACH;
 
 	l_dbus_method_call(dbus_get_bus(), sender, app_path,
@@ -1797,6 +1844,35 @@ void node_join(const char *app_path, const char *sender, const uint8_t *uuid,
 					req, l_free);
 }
 
+bool node_import(const char *app_path, const char *sender,
+				struct mesh_config_import *import,
+				const uint8_t *uuid, node_ready_func_t cb,
+				void *user_data)
+{
+	struct managed_obj_request *req;
+
+	l_debug("");
+
+	/* TODO: implement full import */
+	if (import->node->elements)
+		return false;
+
+	req = l_new(struct managed_obj_request, 1);
+
+	req->data = (void *) uuid;
+	req->cb = cb;
+	req->pending_msg = user_data;
+	req->import = import;
+	req->type = REQUEST_TYPE_IMPORT;
+
+	l_dbus_method_call(dbus_get_bus(), sender, app_path,
+						L_DBUS_INTERFACE_OBJECT_MANAGER,
+						"GetManagedObjects", NULL,
+						get_managed_objects_cb,
+						req, l_free);
+	return true;
+}
+
 void node_create(const char *app_path, const char *sender, const uint8_t *uuid,
 					node_ready_func_t cb, void *user_data)
 {
@@ -1807,7 +1883,7 @@ void node_create(const char *app_path, const char *sender, const uint8_t *uuid,
 	req = l_new(struct managed_obj_request, 1);
 	req->data = (void *) uuid;
 	req->cb = cb;
-	req->user_data = user_data;
+	req->pending_msg = user_data;
 	req->type = REQUEST_TYPE_CREATE;
 
 	l_dbus_method_call(dbus_get_bus(), sender, app_path,
diff --git a/mesh/node.h b/mesh/node.h
index 56ca796cd..fad034f42 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -23,6 +23,7 @@ struct mesh_io;
 struct mesh_agent;
 struct mesh_config;
 struct mesh_config_node;
+struct mesh_config_import;
 
 /* To prevent local node JSON cache thrashing, minimum update times */
 #define MIN_SEQ_TRIGGER	32
@@ -95,6 +96,10 @@ void node_build_attach_reply(struct mesh_node *node,
 						struct l_dbus_message *reply);
 void node_create(const char *app_path, const char *sender, const uint8_t *uuid,
 					node_ready_func_t cb, void *user_data);
+bool node_import(const char *app_path, const char *sender,
+				struct mesh_config_import *import,
+				const uint8_t *uuid, node_ready_func_t cb,
+				void *user_data);
 void node_id_set(struct mesh_node *node, uint16_t node_id);
 uint16_t node_id_get(struct mesh_node *node);
 bool node_dbus_init(struct l_dbus *bus);
-- 
2.19.1


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

* [PATCH BlueZ v5 4/4] mesh: Convert void pointers to anonymous unions in managed_obj_request:wq
  2019-07-17  8:36 [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API Michał Lowas-Rzechonek
                   ` (2 preceding siblings ...)
  2019-07-17  8:36 ` [PATCH BlueZ v5 3/4] mesh: Implement ImportLocalNode() method Michał Lowas-Rzechonek
@ 2019-07-17  8:36 ` Michał Lowas-Rzechonek
  3 siblings, 0 replies; 9+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17  8:36 UTC (permalink / raw)
  To: linux-bluetooth

---
 mesh/node.c | 69 +++++++++++++++++++++++------------------------------
 1 file changed, 30 insertions(+), 39 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index d1b37a5da..d631b9324 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -112,8 +112,14 @@ struct mesh_node {
 };
 
 struct managed_obj_request {
-	void *data;
-	void *cb;
+	union {
+		const uint8_t *uuid;
+		struct mesh_node *node;
+	};
+	union {
+		node_ready_func_t ready_cb;
+		node_join_ready_func_t join_ready_cb;
+	};
 	struct l_dbus_message *pending_msg;
 	enum request_type type;
 	struct mesh_config_import *import;
@@ -1571,10 +1577,10 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	}
 
 	if (is_new) {
-		node = node_new(req->data);
+		node = node_new(req->uuid);
 		node->elements = l_queue_new();
 	} else {
-		node = req->data;
+		node = req->node;
 	}
 
 	num_ele = 0;
@@ -1644,8 +1650,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	}
 
 	if (req->type == REQUEST_TYPE_ATTACH) {
-		node_ready_func_t cb = req->cb;
-
 		if (num_ele != node->num_ele)
 			goto fail;
 
@@ -1654,13 +1658,11 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 
 			node->disc_watch = l_dbus_add_disconnect_watch(bus,
 					node->owner, app_disc_cb, node, NULL);
-			cb(req->pending_msg, MESH_ERROR_NONE, node);
+			req->ready_cb(req->pending_msg, MESH_ERROR_NONE, node);
 		} else
 			goto fail;
 
 	} else if (req->type == REQUEST_TYPE_JOIN) {
-		node_join_ready_func_t cb = req->cb;
-
 		if (!agent) {
 			l_error("Interface %s not found",
 						MESH_PROVISION_AGENT_INTERFACE);
@@ -1669,16 +1671,14 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 
 		node->num_ele = num_ele;
 		set_defaults(node);
-		memcpy(node->uuid, req->data, 16);
+		memcpy(node->uuid, req->uuid, 16);
 
 		if (!create_node_config(node, node->uuid))
 			goto fail;
 
-		cb(node, agent);
+		req->join_ready_cb(node, agent);
 
 	} else if (req->type == REQUEST_TYPE_IMPORT) {
-
-		node_ready_func_t cb = req->cb;
 		struct mesh_config_import *import = req->import;
 		struct keyring_net_key net_key;
 
@@ -1694,7 +1694,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		if (node->seq_number != import->node->seq_number)
 			node->seq_number = import->node->seq_number;
 
-		memcpy(node->uuid, req->data, 16);
+		memcpy(node->uuid, req->uuid, 16);
 
 		if (!create_node_config(node, node->uuid))
 			goto fail;
@@ -1719,17 +1719,16 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		if (!keyring_put_net_key(node, import->net_key.idx, &net_key))
 			goto fail;
 
-		cb(req->pending_msg, MESH_ERROR_NONE, node);
+		req->ready_cb(req->pending_msg, MESH_ERROR_NONE, node);
 
 	} else {
 		/* Callback for create node request */
-		node_ready_func_t cb = req->cb;
 		struct keyring_net_key net_key;
 		uint8_t dev_key[16];
 
 		node->num_ele = num_ele;
 		set_defaults(node);
-		memcpy(node->uuid, req->data, 16);
+		memcpy(node->uuid, req->uuid, 16);
 
 		if (!create_node_config(node, node->uuid))
 			goto fail;
@@ -1756,7 +1755,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		if (!keyring_put_net_key(node, PRIMARY_NET_IDX, &net_key))
 			goto fail;
 
-		cb(req->pending_msg, MESH_ERROR_NONE, node);
+		req->ready_cb(req->pending_msg, MESH_ERROR_NONE, node);
 	}
 
 	return;
@@ -1765,26 +1764,18 @@ fail:
 		mesh_agent_remove(agent);
 
 	if (!is_new) {
-		/* Handle failed Attach request */
-		node_ready_func_t cb = req->cb;
-
 		free_node_dbus_resources(node);
 
-		cb(req->pending_msg, MESH_ERROR_FAILED, node);
+		req->ready_cb(req->pending_msg, MESH_ERROR_FAILED, node);
 	} else {
 		/* Handle failed Join, Create and Import requests */
 		if (node)
 			node_remove(node);
 
-		if (req->type == REQUEST_TYPE_JOIN) {
-			node_join_ready_func_t cb = req->cb;
-
-			cb(NULL, NULL);
-		} else {
-			node_ready_func_t cb = req->cb;
-
-			cb(req->pending_msg, MESH_ERROR_FAILED, NULL);
-		}
+		if (req->type == REQUEST_TYPE_JOIN)
+			req->join_ready_cb(NULL, NULL);
+		else
+			req->ready_cb(req->pending_msg, MESH_ERROR_FAILED, NULL);
 	}
 }
 
@@ -1809,8 +1800,8 @@ int node_attach(const char *app_path, const char *sender, uint64_t token,
 	node->owner = l_strdup(sender);
 
 	req = l_new(struct managed_obj_request, 1);
-	req->data = node;
-	req->cb = cb;
+	req->node = node;
+	req->ready_cb = cb;
 	req->pending_msg = user_data;
 	req->type = REQUEST_TYPE_ATTACH;
 
@@ -1833,8 +1824,8 @@ void node_join(const char *app_path, const char *sender, const uint8_t *uuid,
 	l_debug("");
 
 	req = l_new(struct managed_obj_request, 1);
-	req->data = (void *) uuid;
-	req->cb = cb;
+	req->uuid = uuid;
+	req->join_ready_cb = cb;
 	req->type = REQUEST_TYPE_JOIN;
 
 	l_dbus_method_call(dbus_get_bus(), sender, app_path,
@@ -1859,8 +1850,8 @@ bool node_import(const char *app_path, const char *sender,
 
 	req = l_new(struct managed_obj_request, 1);
 
-	req->data = (void *) uuid;
-	req->cb = cb;
+	req->uuid = uuid;
+	req->ready_cb = cb;
 	req->pending_msg = user_data;
 	req->import = import;
 	req->type = REQUEST_TYPE_IMPORT;
@@ -1881,8 +1872,8 @@ void node_create(const char *app_path, const char *sender, const uint8_t *uuid,
 	l_debug("");
 
 	req = l_new(struct managed_obj_request, 1);
-	req->data = (void *) uuid;
-	req->cb = cb;
+	req->uuid = uuid;
+	req->ready_cb = cb;
 	req->pending_msg = user_data;
 	req->type = REQUEST_TYPE_CREATE;
 
-- 
2.19.1


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

* Re: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
  2019-07-17  8:36 ` [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation Michał Lowas-Rzechonek
@ 2019-07-17 19:36   ` Stotland, Inga
  2019-07-17 19:47     ` michal.lowas-rzechonek
  0 siblings, 1 reply; 9+ messages in thread
From: Stotland, Inga @ 2019-07-17 19:36 UTC (permalink / raw)
  To: michal.lowas-rzechonek, linux-bluetooth; +Cc: jakub.witowski, Gix, Brian

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

Hi Mical, Jakub, Brian...


On Wed, 2019-07-17 at 10:36 +0200, Michał Lowas-Rzechonek wrote:
> From: Jakub Witowski <jakub.witowski@silvair.com>
> 
> This updates the mesh-api.txt with new ImportLocalNode() API.
> ---
>  doc/mesh-api.txt | 52 ++++++++++++++++++++++++++++++++++++++------
> ----
>  1 file changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index 0ac2fdfd1..7c2a1fafa 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -151,16 +151,31 @@ Methods:
>  			org.bluez.mesh.Error.InvalidArguments
>  			org.bluez.mesh.Error.AlreadyExists,
>  
> -	 uint64 token ImportLocalNode(string json_data)
> +	uint64 token ImportLocalNode(object app_root, array{byte}[16]
> uuid,
> +							string
> data_type, array{byte} import_data)


I apologize for the backtracking, but I would like to revisit this API.

I feel like having "object app_root" is unnecessary and also, creates
some gnarly pathways within the code.

What exactly is the problem for requiring the composition data to be
part of the import_data? It's just weird to say "oh, it may be there or
it may be not".

Getting rid of the app_root and mandating the composition to be part of
the import_data allows:

1) Avoid checking whether this is a "full" configuration or the
"minimal" one

2) Efficiently re-use the existing code:
Adding an API call like this one may be sufficient

mesh_config_import(const char *cfg_dir,
                   const uint8_t uuid[16],
                   const uint8 *import_data, <import__len>?,
                   mesh_config_node_func_t cb,
                   void *user_data)

We can just re-factor the code that parses and populates a single node
from the stored configuration. user_data may contain whatever we need
to preserve in order to respond to d-bus call.

>  
>  		This method creates a local mesh node based on node
>  		configuration that has been generated outside
> bluetooth-meshd.
>  
> -		The json_data parameter is a full JSON representation
> of a node
> -		configuration file. The format must conform to the
> schema
> -		defined in "Mesh Node Configuration Schema" section.
> Any
> -		included token will be ignored in favor of a locally
> generated
> -		token value.
> +		The app_root parameter is a D-Bus object root path of
> the
> +		application that implements org.bluez.mesh.Application1
> +		interface.
> +
> +		The import_data parameter contains a representation of
> a
> +		provisioned node. Format of this representation depends
> on
> +		value of data_type parameter.
> +
> +		Allowed data_type values are: "json".
> +
> +		When data_type is "json", bluetooth-meshd daemon treats
> +		import_data is a JSON document following <TBD> schema.
> See the
> +		examples at the end of this document.
> +
> +		The import_data parameter can contain either minimal,
> or
> +		complete representation of a provisioned node.
> +
> +		When a complete representation is provided, it is
> validated
> +		against composition data provided by the application.
>  
>  		The returned token must be preserved by the application
> in
>  		order to authenticate itself to the mesh daemon and
> attach to
> @@ -173,8 +188,8 @@ Methods:
>  
>  		PossibleErrors:
>  			org.bluez.mesh.Error.InvalidArguments,
> -			org.bluez.mesh.Error.AlreadyExists
> -			org.bluez.mesh.Error.NotFound,
> +			org.bluez.mesh.Error.AlreadyExists,
> +			org.bluez.mesh.Error.NotSupported,
>  			org.bluez.mesh.Error.Failed
>  
>  Mesh Node Hierarchy
> @@ -1064,6 +1079,21 @@ Properties:
>  		Uniform Resource Identifier points to out-of-band (OOB)
>  		information (e.g., a public key)
>  
> -Mesh Node Configuration Schema
> -==============================
> -<TBD>
> +Mesh Node Configuration Examples
> +================================
> +Minimal JSON representation for ImportLocalNode():
> +
> +{
> +	"IVindex":0,
> +	"IVupdate":0,
> +	"unicastAddress":"0012",
> +	"deviceKey":"7daa45cd1e9e11a4b86eeef7d01efa11",
> +	"netKeys":[
> +		{
> +			"index":"0000",
> +			"key":"2ddfef86d67144c394428ea3078f86f9",
> +			"keyRefresh":0
> +		}
> +	],
> +	"sequenceNumber":15  /* optional */
> +}

Regards, Inga

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

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

* Re: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
  2019-07-17 19:36   ` Stotland, Inga
@ 2019-07-17 19:47     ` michal.lowas-rzechonek
  2019-07-17 21:14       ` Gix, Brian
  0 siblings, 1 reply; 9+ messages in thread
From: michal.lowas-rzechonek @ 2019-07-17 19:47 UTC (permalink / raw)
  To: Stotland, Inga; +Cc: linux-bluetooth, jakub.witowski, Gix, Brian

Hello,

On 07/17, Stotland, Inga wrote:
> I feel like having "object app_root" is unnecessary and also, creates
> some gnarly pathways within the code.
> 
> What exactly is the problem for requiring the composition data to be
> part of the import_data? It's just weird to say "oh, it may be there or
> it may be not".

The main issue lies on the application side. In order to properly
Attach(), the application must expose element structure via D-Bus.

If we say that it must also do the same via JSON, to call
ImportLocalNode, it leads to code duplication on the application side.

Moreover, the app still needs to be queried via D-Bus to check that the
passed JSON matches the D-Bus structure - otherwise the app would then
fail to Attach() and the user would be in deep trouble.

> Getting rid of the app_root and mandating the composition to be part of
> the import_data allows:
> 
> 1) Avoid checking whether this is a "full" configuration or the
> "minimal" one

I'm not convinced that the "full" configuration is even needed. We
certaintly don't use it in our use case, but it might be required in the
future.

> 2) Efficiently re-use the existing code:
> Adding an API call like this one may be sufficient
> 
> mesh_config_import(const char *cfg_dir,
>                    const uint8_t uuid[16],
>                    const uint8 *import_data, <import__len>?,
>                    mesh_config_node_func_t cb,
>                    void *user_data)
> 
> We can just re-factor the code that parses and populates a single node
> from the stored configuration. user_data may contain whatever we need
> to preserve in order to respond to d-bus call.

After refactoring node validation to byte-compare composition data, the
code also becomes significantly simpler, and execution paths for Join(),
Attach(), CreateNetwork() and ImportLocalNode() converge.

I've implemented this validation method on top of this patch-set. I'll
send it as RFC shortly.

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] 9+ messages in thread

* RE: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
  2019-07-17 19:47     ` michal.lowas-rzechonek
@ 2019-07-17 21:14       ` Gix, Brian
  2019-07-18  8:16         ` michal.lowas-rzechonek
  0 siblings, 1 reply; 9+ messages in thread
From: Gix, Brian @ 2019-07-17 21:14 UTC (permalink / raw)
  To: michal.lowas-rzechonek, Stotland, Inga; +Cc: linux-bluetooth, jakub.witowski

Hi Inga & Michal,

> On 07/17, Stotland, Inga wrote:
> > I feel like having "object app_root" is unnecessary and also, creates
> > some gnarly pathways within the code.
> >
> > What exactly is the problem for requiring the composition data to be
> > part of the import_data? It's just weird to say "oh, it may be there
> > or it may be not".
> 
> The main issue lies on the application side. In order to properly Attach(), the
> application must expose element structure via D-Bus.

I understand this concern, but I think I am agreeing with Inga on this one.

At the time of inception, Not only the Prov Data (net key, net idx, iv index, flags, unicast, dev key) must be present, but also so must the "Composition Data"...  all of the data which is returned by the "GET COMPOSITION DATA" opcode.  This data needs to be present as soon as the Provisioner starts querying and setting states on the (new or migrated) fresh node.

An important point to remember here is that *the application does not need to be attached" while the remote Provisioner/ConfigClient is interacting with the nodes Config Server.  One of our Design Goals, in fact, was that the node always exists on the network, regardless of whether the App is attached...  This is why the Config Server model is part of the daemon....   It is always "OnLine".

For this reason, I am supporting Inga here, that the JSON input file should contain everything Jakub has included:
Dev Key
Net Key
Net Index
IV Index
Flags
Unicast 

*And* it should include the pure Composition Data including
CID/VID/PID  (optional .. Can be set with BlueZ defaults)
CRPL  (optional ...  Can be set with internal defaults)
Features (optional ...  missing are created as either "Unsupported" or "Disabled")
array of Elements, containing array of Models. (Mandatory ... Needed for Cfg Server to function)

The application has no need to even exist at this point, as long as it can attach to the token at some point in the future.
But this *does* enable the ability to have a *generic* application that can inject nodes (fully configured, or "New") into the daemon.

But worries that the App might not immediately be able to "Attach" go away.



> If we say that it must also do the same via JSON, to call ImportLocalNode, it
> leads to code duplication on the application side.
> 
> Moreover, the app still needs to be queried via D-Bus to check that the passed
> JSON matches the D-Bus structure - otherwise the app would then fail to
> Attach() and the user would be in deep trouble.

The composition as reflected by the GetManagedObjects() call is "sanity checked" against the internal storage *every* time the App attaches...  I think Inga is concerned with code complexity and bloat to repeat this during ImportLocalNode(), simply as a "second way" attain the composition data.  This is different from the Join() case, in my opinion, where the JSON (or other storage) is being created *totally* from scratch, via the provisioning interaction with the remote Provisioner. In that case, yes... the "owning application" needs to be present on the D-Bus.

 
> > Getting rid of the app_root and mandating the composition to be part
> > of the import_data allows:
> >
> > 1) Avoid checking whether this is a "full" configuration or the
> > "minimal" one
> 
> I'm not convinced that the "full" configuration is even needed. We certaintly
> don't use it in our use case, but it might be required in the future.

We *definitely* need an option for importing/migrating a fully configured node.  Phones are retired and replaced... Workstations are retired and replaced...  Some nodes publications will inevitably need to pick up the "current conversation" with the migrated node, where the old conversation left off. And this will almost certainly be a rare (but important) operation, and a "Utility" application to perform the operation (that does not itself need to have the model/element arrays implemented) will be easier to write and maintain.


> 
> > 2) Efficiently re-use the existing code:
> > Adding an API call like this one may be sufficient
> >
> > mesh_config_import(const char *cfg_dir,
> >                    const uint8_t uuid[16],
> >                    const uint8 *import_data, <import__len>?,
> >                    mesh_config_node_func_t cb,
> >                    void *user_data)
> >
> > We can just re-factor the code that parses and populates a single node
> > from the stored configuration. user_data may contain whatever we need
> > to preserve in order to respond to d-bus call.
> 
> After refactoring node validation to byte-compare composition data, the code
> also becomes significantly simpler, and execution paths for Join(), Attach(),
> CreateNetwork() and ImportLocalNode() converge.

Again, I cannot think of any situations where Join/Attach/Create would ever exist in the absence of the Application.

This is an easy and obvious use case with Import.

--Brian

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

* Re: [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation
  2019-07-17 21:14       ` Gix, Brian
@ 2019-07-18  8:16         ` michal.lowas-rzechonek
  0 siblings, 0 replies; 9+ messages in thread
From: michal.lowas-rzechonek @ 2019-07-18  8:16 UTC (permalink / raw)
  To: Gix, Brian; +Cc: Stotland, Inga, linux-bluetooth, jakub.witowski

Brian, Inga,

On 07/17, Gix, Brian wrote:
> The application has no need to even exist at this point, as long as it 
> can attach to the token at some point in the future.  But this *does* 
> enable the ability to have a *generic* application that can inject 
> nodes (fully configured, or "New") into the daemon.

The same thing can be said about CreateNetwork() call - you don't really 
*need* to have an attached (of even "attachable") application in order 
to create a new node.

In fact, CreateNetwork() and ImportLocalNode() calls are very similar in 
this regard, it's just that CreateNetwork generates keys, addresses and 
starting sequence number on the daemon side, while ImportLocalNode() 
allows passing them from the application.

Since CreateNetwork() does query the app via D-Bus, I think it's more 
consistent to query the app during ImportLocalNode() as well.

That, or replace both calls with a more generic CreateNode() call that 
would accept create/import data as an agument, and not query the app at 
all.

> > If we say that it must also do the same via JSON, to call 
> > ImportLocalNode, it leads to code duplication on the application 
> > side.
> >
> > Moreover, the app still needs to be queried via D-Bus to check that 
> > the passed JSON matches the D-Bus structure - otherwise the app 
> > would then fail to Attach() and the user would be in deep trouble.
>
> The composition as reflected by the GetManagedObjects() call is 
> "sanity checked" against the internal storage *every* time the App 
> attaches...  I think Inga is concerned with code complexity and bloat 
> to repeat this during ImportLocalNode(),

This is covered by the same code path as other calls. I think it's 
cleaner to always perform GetManagedObjects() dance for all 4 calls.

And, as I mentioned before, this code path becomes even simpler when we 
refactor "sanity checking" to compare serialized composition data.

> This is different from the Join() case, in my opinion, where the JSON 
> (or other storage) is being created *totally* from scratch, via the 
> provisioning interaction with the remote Provisioner. In that case, 
> yes... the "owning application" needs to be present on the D-Bus.

Not necessarily. The application needs to provide ProvisionAgent 
interface only, and only in cases where provisioner selects 
authentication method that requires the app to perform some OOB action.

It is entirely possible for a joining node to say it doesn't support any 
OOB actions, and the mesh daemon can accept provisioning wihout any 
interaction with the app.

> > I'm not convinced that the "full" configuration is even needed. We 
> > certaintly don't use it in our use case, but it might be required in 
> > the future.
>
> We *definitely* need an option for importing/migrating a fully 
> configured node.  Phones are retired and replaced... Workstations are 
> retired and replaced...  Some nodes publications will inevitably need 
> to pick up the "current conversation" with the migrated node, where 
> the old conversation left off. And this will almost certainly be a 
> rare (but important) operation, and a "Utility" application to perform 
> the operation (that does not itself need to have the model/element 
> arrays implemented) will be easier to write and maintain.
(...)
> Again, I cannot think of any situations where Join/Attach/Create would 
> ever exist in the absence of the Application.
>
> This is an easy and obvious use case with Import.

I don't think it makes sense to have a utility application that would 
migrate the node without having anything that would Attach() to it.

IMO the migration operation should be a part of application logic, 
because this operation requires talking to external Provisioner, and 
Provisioner API is vendor specific. So I don't think you can't really 
create a 'generic migration tool'.

In case of replaced device, the use case I see is re-installing the same 
application on a new device, and having it re-create the node either 
from data received from the Provisioner, or exported from the previous 
instance of the application. I don't see a need to instantiate the node 
on the replaces device without reinstalling the application - such a 
node would not be operational anyway.

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

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

end of thread, other threads:[~2019-07-18  8:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-17  8:36 [PATCH BlueZ v5 0/4] Implement ImportLocalNode D-Bus API Michał Lowas-Rzechonek
2019-07-17  8:36 ` [PATCH BlueZ v5 1/4] mesh: Add ImportLocalNode API documentation Michał Lowas-Rzechonek
2019-07-17 19:36   ` Stotland, Inga
2019-07-17 19:47     ` michal.lowas-rzechonek
2019-07-17 21:14       ` Gix, Brian
2019-07-18  8:16         ` michal.lowas-rzechonek
2019-07-17  8:36 ` [PATCH BlueZ v5 2/4] mesh: Extract read_* functions in mesh-config-json Michał Lowas-Rzechonek
2019-07-17  8:36 ` [PATCH BlueZ v5 3/4] mesh: Implement ImportLocalNode() method Michał Lowas-Rzechonek
2019-07-17  8:36 ` [PATCH BlueZ v5 4/4] mesh: Convert void pointers to anonymous unions in managed_obj_request:wq Michał Lowas-Rzechonek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).