All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/3] mesh: clean up
@ 2019-12-06 20:57 Inga Stotland
  2019-12-06 20:57 ` [PATCH BlueZ 1/3] mesh: Delete unused function Inga Stotland
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Inga Stotland @ 2019-12-06 20:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This set of patches claens up some cruft in node.c and some in net.c
The dtaaflow is a bit more deterministic now.

Inga Stotland (3):
  mesh: Delete unused function
  mesh: Clean up node.c
  mesh: Initialize net modes based on node configuration

 mesh/mesh-defs.h |   2 +
 mesh/model.c     |  17 +-
 mesh/model.h     |   4 +-
 mesh/net.c       |  15 +-
 mesh/node.c      | 503 ++++++++++++++++++-----------------------------
 mesh/node.h      |   1 -
 6 files changed, 200 insertions(+), 342 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH BlueZ 1/3] mesh: Delete unused function
  2019-12-06 20:57 [PATCH BlueZ 0/3] mesh: clean up Inga Stotland
@ 2019-12-06 20:57 ` Inga Stotland
  2019-12-06 20:57 ` [PATCH BlueZ 2/3] mesh: Clean up node.c Inga Stotland
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Inga Stotland @ 2019-12-06 20:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This removes node_parse_composition() implementation as it is not used
anywhere in the rest of the code base.
---
 mesh/node.c | 131 ----------------------------------------------------
 mesh/node.h |   1 -
 2 files changed, 132 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index 7b4ee0505..b8c30c30a 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -994,137 +994,6 @@ element_done:
 	return n;
 }
 
-
-#define MIN_COMPOSITION_LEN 16
-
-bool node_parse_composition(struct mesh_node *node, uint8_t *data,
-								uint16_t len)
-{
-	struct node_composition *comp;
-	uint16_t features;
-	uint8_t num_ele;
-	bool mode;
-
-	if (!len)
-		return false;
-
-	/* Skip page -- We only support Page Zero */
-	data++;
-	len--;
-
-	if (len < MIN_COMPOSITION_LEN)
-		return false;
-
-	comp = l_new(struct node_composition, 1);
-	if (!comp)
-		return false;
-
-	node->elements = l_queue_new();
-	if (!node->elements) {
-		l_free(comp);
-		return false;
-	}
-
-	node->comp = l_new(struct node_composition, 1);
-	comp->cid = l_get_le16(&data[0]);
-	comp->pid = l_get_le16(&data[2]);
-	comp->vid = l_get_le16(&data[4]);
-	comp->crpl = l_get_le16(&data[6]);
-	features = l_get_le16(&data[8]);
-	data += 10;
-	len -= 10;
-
-	mode = !!(features & FEATURE_PROXY);
-	node->proxy = mode ? MESH_MODE_DISABLED : MESH_MODE_UNSUPPORTED;
-
-	mode = !!(features & FEATURE_LPN);
-	node->lpn = mode ? MESH_MODE_DISABLED : MESH_MODE_UNSUPPORTED;
-
-	mode = !!(features & FEATURE_FRIEND);
-	node->friend = mode ? MESH_MODE_DISABLED : MESH_MODE_UNSUPPORTED;
-
-	mode = !!(features & FEATURE_RELAY);
-	node->relay.mode = mode ? MESH_MODE_DISABLED : MESH_MODE_UNSUPPORTED;
-
-	num_ele = 0;
-
-	do {
-		uint8_t m, v;
-		uint16_t mod_id;
-		uint16_t vendor_id;
-		struct node_element *ele;
-		struct mesh_model *mod;
-
-		ele = l_new(struct node_element, 1);
-		if (!ele)
-			return false;
-
-		ele->idx = num_ele;
-		ele->location = l_get_le16(data);
-		len -= 2;
-		data += 2;
-
-		m = *data++;
-		v = *data++;
-		len -= 2;
-
-		/* Parse SIG models */
-		while (len >= 2 && m--) {
-			mod_id = l_get_le16(data);
-			mod = mesh_model_new(ele->idx, mod_id);
-			if (!mod || !element_add_model(ele, mod)) {
-				mesh_model_free(mod);
-				element_free(ele);
-				goto fail;
-			}
-
-			data += 2;
-			len -= 2;
-		}
-
-		if (v && len < 4) {
-			element_free(ele);
-			goto fail;
-		}
-
-		/* Parse vendor models */
-		while (len >= 4 && v--) {
-			mod_id = l_get_le16(data + 2);
-			vendor_id = l_get_le16(data);
-			mod_id |= (vendor_id << 16);
-			mod = mesh_model_vendor_new(ele->idx, vendor_id,
-									mod_id);
-			if (!mod || !element_add_model(ele, mod)) {
-				mesh_model_free(mod);
-				element_free(ele);
-				goto fail;
-			}
-
-			data += 4;
-			len -= 4;
-		}
-
-		num_ele++;
-		l_queue_push_tail(node->elements, ele);
-
-	} while (len >= 6);
-
-	/* Check the consistency for the remote node */
-	if (node->num_ele > num_ele)
-		goto fail;
-
-	node->comp = comp;
-	node->num_ele = num_ele;
-
-	return true;
-
-fail:
-	l_queue_destroy(node->elements, element_free);
-	l_free(comp);
-
-	return false;
-}
-
 static void attach_io(void *a, void *b)
 {
 	struct mesh_node *node = a;
diff --git a/mesh/node.h b/mesh/node.h
index be57d5e67..7448756ae 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -50,7 +50,6 @@ 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);
-bool node_parse_composition(struct mesh_node *node, uint8_t *buf, uint16_t len);
 bool node_add_binding(struct mesh_node *node, uint8_t ele_idx,
 			uint32_t model_id, uint16_t app_idx);
 bool node_del_binding(struct mesh_node *node, uint8_t ele_idx,
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH BlueZ 2/3] mesh: Clean up node.c
  2019-12-06 20:57 [PATCH BlueZ 0/3] mesh: clean up Inga Stotland
  2019-12-06 20:57 ` [PATCH BlueZ 1/3] mesh: Delete unused function Inga Stotland
@ 2019-12-06 20:57 ` Inga Stotland
  2019-12-11 15:48   ` Michał Lowas-Rzechonek
  2019-12-06 20:57 ` [PATCH BlueZ 3/3] mesh: Initialize net modes based on node configuration Inga Stotland
  2019-12-10 18:40 ` [PATCH BlueZ 0/3] mesh: clean up Gix, Brian
  3 siblings, 1 reply; 6+ messages in thread
From: Inga Stotland @ 2019-12-06 20:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This change makes the node initialization a bit easier to follow.
Replace if-else with switch when processing request type, descriptive
function names, more predictable code flow.
---
 mesh/model.c |  17 +--
 mesh/model.h |   4 +-
 mesh/node.c  | 331 ++++++++++++++++++++++++++-------------------------
 3 files changed, 171 insertions(+), 181 deletions(-)

diff --git a/mesh/model.c b/mesh/model.c
index 4091881ec..acdd94f7b 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -1249,7 +1249,7 @@ void mesh_model_free(void *data)
 	l_free(mod);
 }
 
-static struct mesh_model *model_new(uint8_t ele_idx, uint32_t id)
+struct mesh_model *mesh_model_new(uint8_t ele_idx, uint32_t id)
 {
 	struct mesh_model *mod = l_new(struct mesh_model, 1);
 
@@ -1259,19 +1259,6 @@ static struct mesh_model *model_new(uint8_t ele_idx, uint32_t id)
 	return mod;
 }
 
-struct mesh_model *mesh_model_new(uint8_t ele_idx, uint16_t id)
-{
-	return model_new(ele_idx, id | VENDOR_ID_MASK);
-}
-
-struct mesh_model *mesh_model_vendor_new(uint8_t ele_idx, uint16_t vendor_id,
-								uint16_t mod_id)
-{
-	uint32_t id = mod_id | (vendor_id << 16);
-
-	return model_new(ele_idx, id);
-}
-
 /* Internal models only */
 static void restore_model_state(struct mesh_model *mod)
 {
@@ -1600,7 +1587,7 @@ struct mesh_model *mesh_model_setup(struct mesh_node *node, uint8_t ele_idx,
 		return NULL;
 	}
 
-	mod = model_new(ele_idx, db_mod->vendor ? db_mod->id :
+	mod = mesh_model_new(ele_idx, db_mod->vendor ? db_mod->id :
 						db_mod->id | VENDOR_ID_MASK);
 
 	/* Implicitly bind config server model to device key */
diff --git a/mesh/model.h b/mesh/model.h
index f30ca2e38..9c7ce9334 100644
--- a/mesh/model.h
+++ b/mesh/model.h
@@ -62,9 +62,7 @@ struct mesh_model_ops {
 	mesh_model_sub_cb sub;
 };
 
-struct mesh_model *mesh_model_new(uint8_t ele_idx, uint16_t mod_id);
-struct mesh_model *mesh_model_vendor_new(uint8_t ele_idx, uint16_t vendor_id,
-							uint16_t mod_id);
+struct mesh_model *mesh_model_new(uint8_t ele_idx, uint32_t mod_id);
 void mesh_model_free(void *data);
 uint32_t mesh_model_get_model_id(const struct mesh_model *model);
 bool mesh_model_register(struct mesh_node *node, uint8_t ele_idx,
diff --git a/mesh/node.c b/mesh/node.c
index b8c30c30a..edf6fce37 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -219,7 +219,6 @@ static int compare_model_id(const void *a, const void *b, void *user_data)
 	return 0;
 }
 
-
 struct mesh_node *node_find_by_addr(uint16_t addr)
 {
 	if (!IS_UNICAST(addr))
@@ -245,13 +244,46 @@ uint8_t *node_uuid_get(struct mesh_node *node)
 	return node->uuid;
 }
 
+static void add_internal_model(struct mesh_node *node, uint32_t mod_id,
+								uint8_t ele_idx)
+{
+	struct node_element *ele;
+	struct mesh_model *mod;
+
+	ele = l_queue_find(node->elements, match_element_idx,
+							L_UINT_TO_PTR(ele_idx));
+	if (!ele)
+		return;
+
+	if (l_queue_find(ele->models, match_model_id, L_UINT_TO_PTR(mod_id)))
+		return;
+
+	mod = mesh_model_new(ele_idx, mod_id);
+
+	l_queue_insert(ele->models, mod, compare_model_id, NULL);
+}
+
+static void set_defaults(struct mesh_node *node)
+{
+	/* TODO: these values should come from mesh.conf */
+	node->lpn = MESH_MODE_UNSUPPORTED;
+	node->proxy = MESH_MODE_UNSUPPORTED;
+	node->friend = MESH_MODE_UNSUPPORTED;
+	node->beacon = MESH_MODE_DISABLED;
+	node->relay.mode = MESH_MODE_DISABLED;
+	node->ttl = TTL_MASK;
+	node->seq_number = DEFAULT_SEQUENCE_NUMBER;
+}
+
 static struct mesh_node *node_new(const uint8_t uuid[16])
 {
 	struct mesh_node *node;
 
 	node = l_new(struct mesh_node, 1);
 	node->net = mesh_net_new(node);
+	node->elements = l_queue_new();
 	memcpy(node->uuid, uuid, sizeof(node->uuid));
+	set_defaults(node);
 
 	return node;
 }
@@ -344,18 +376,8 @@ void node_remove(struct mesh_node *node)
 	free_node_resources(node);
 }
 
-static bool element_add_model(struct node_element *ele, struct mesh_model *mod)
-{
-	uint32_t mod_id = mesh_model_get_model_id(mod);
-
-	if (l_queue_find(ele->models, match_model_id, L_UINT_TO_PTR(mod_id)))
-		return false;
-
-	l_queue_insert(ele->models, mod, compare_model_id, NULL);
-	return true;
-}
-
-static bool add_models(struct mesh_node *node, struct node_element *ele,
+static bool add_models_from_storage(struct mesh_node *node,
+					struct node_element *ele,
 					struct mesh_config_element *db_ele)
 {
 	const struct l_queue_entry *entry;
@@ -364,52 +386,31 @@ static bool add_models(struct mesh_node *node, struct node_element *ele,
 		ele->models = l_queue_new();
 
 	entry = l_queue_get_entries(db_ele->models);
+
 	for (; entry; entry = entry->next) {
 		struct mesh_model *mod;
 		struct mesh_config_model *db_mod;
+		uint32_t id;
 
 		db_mod = entry->data;
+
+		id = db_mod->vendor ? db_mod->id : db_mod->id | VENDOR_ID_MASK;
+
+		if (l_queue_find(ele->models, match_model_id,
+							L_UINT_TO_PTR(id)))
+			return false;
+
 		mod = mesh_model_setup(node, ele->idx, db_mod);
 		if (!mod)
 			return false;
 
-		if (!element_add_model(ele, mod)) {
-			mesh_model_free(mod);
-			return false;
-		}
+		l_queue_insert(ele->models, mod, compare_model_id, NULL);
 	}
 
 	return true;
 }
 
-static void add_internal_model(struct mesh_node *node, uint32_t mod_id,
-								uint8_t ele_idx)
-{
-	struct node_element *ele;
-	struct mesh_model *mod;
-	struct mesh_config_model db_mod;
-
-	ele = l_queue_find(node->elements, match_element_idx,
-							L_UINT_TO_PTR(ele_idx));
-
-	if (!ele)
-		return;
-
-	memset(&db_mod, 0, sizeof(db_mod));
-	db_mod.id = mod_id;
-
-	mod = mesh_model_setup(node, ele_idx, &db_mod);
-	if (!mod)
-		return;
-
-	if (!ele->models)
-		ele->models = l_queue_new();
-
-	if (!element_add_model(ele, mod))
-		mesh_model_free(mod);
-}
-
-static bool add_element(struct mesh_node *node,
+static bool add_element_from_storage(struct mesh_node *node,
 					struct mesh_config_element *db_ele)
 {
 	struct node_element *ele;
@@ -421,26 +422,26 @@ static bool add_element(struct mesh_node *node,
 	ele->idx = db_ele->index;
 	ele->location = db_ele->location;
 
-	if (!db_ele->models || !add_models(node, ele, db_ele))
+	if (!db_ele->models || !add_models_from_storage(node, ele, db_ele))
 		return false;
 
 	l_queue_push_tail(node->elements, ele);
 	return true;
 }
 
-static bool add_elements(struct mesh_node *node,
+static bool add_elements_from_storage(struct mesh_node *node,
 					struct mesh_config_node *db_node)
 {
 	const struct l_queue_entry *entry;
 
-	if (!node->elements)
-		node->elements = l_queue_new();
-
 	entry = l_queue_get_entries(db_node->elements);
 	for (; entry; entry = entry->next)
-		if (!add_element(node, entry->data))
+		if (!add_element_from_storage(node, entry->data))
 			return false;
 
+	/* Add configuration server model on the primary element */
+	add_internal_model(node, CONFIG_SRV_MODEL, PRIMARY_ELE_IDX);
+
 	return true;
 }
 
@@ -462,13 +463,35 @@ static void set_appkey(void *a, void *b)
 						appkey->key, appkey->new_key);
 }
 
+static bool init_storage_dir(struct mesh_node *node)
+{
+	char uuid[33];
+	char dir_name[PATH_MAX];
+
+	if (node->storage_dir)
+		return true;
+
+	if (!hex2str(node->uuid, 16, uuid, sizeof(uuid)))
+		return false;
+
+	snprintf(dir_name, PATH_MAX, "%s/%s", mesh_get_storage_dir(), uuid);
+
+	if (strlen(dir_name) >= PATH_MAX)
+		return false;
+
+	create_dir(dir_name);
+
+	node->storage_dir = l_strdup(dir_name);
+
+	return true;
+}
+
 static bool init_from_storage(struct mesh_config_node *db_node,
 			const uint8_t uuid[16], struct mesh_config *cfg,
 			void *user_data)
 {
 	unsigned int num_ele;
 	uint8_t mode;
-
 	struct mesh_node *node = node_new(uuid);
 
 	if (!nodes)
@@ -504,7 +527,7 @@ static bool init_from_storage(struct mesh_config_node *db_node,
 
 	node->num_ele = num_ele;
 
-	if (num_ele != 0 && !add_elements(node, db_node))
+	if (num_ele != 0 && !add_elements_from_storage(node, db_node))
 		goto fail;
 
 	node->primary = db_node->unicast;
@@ -512,6 +535,10 @@ static bool init_from_storage(struct mesh_config_node *db_node,
 	if (!db_node->netkeys)
 		goto fail;
 
+	if (!IS_UNASSIGNED(node->primary) &&
+		!mesh_net_register_unicast(node->net, node->primary, num_ele))
+		goto fail;
+
 	mesh_net_set_iv_index(node->net, db_node->iv_index, db_node->iv_update);
 
 	if (db_node->net_transmit)
@@ -544,15 +571,14 @@ static bool init_from_storage(struct mesh_config_node *db_node,
 	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
 		mesh_net_set_beacon_mode(node->net, mode == MESH_MODE_ENABLED);
 
-	if (!IS_UNASSIGNED(node->primary) &&
-		!mesh_net_register_unicast(node->net, node->primary, num_ele))
-		goto fail;
-
 	/* Initialize configuration server model */
 	cfgmod_server_init(node, PRIMARY_ELE_IDX);
 
 	node->cfg = cfg;
 
+	/* Initialize directory for storing keyring info */
+	init_storage_dir(node);
+
 	return true;
 fail:
 	node_remove(node);
@@ -876,8 +902,6 @@ uint16_t node_generate_comp(struct mesh_node *node, uint8_t *buf, uint16_t sz)
 {
 	uint16_t n, features;
 	uint16_t num_ele = 0;
-	uint8_t *cfgmod_idx = NULL;
-
 	const struct l_queue_entry *ele_entry;
 
 	if (!node || !node->comp || sz < MIN_COMP_SIZE)
@@ -940,9 +964,6 @@ uint16_t node_generate_comp(struct mesh_node *node, uint8_t *buf, uint16_t sz)
 			mod_id = mesh_model_get_model_id(
 					(const struct mesh_model *) mod);
 
-			if (mod_id == CONFIG_SRV_MODEL)
-				cfgmod_idx = &ele->idx;
-
 			if ((mod_id & VENDOR_ID_MASK) == VENDOR_ID_MASK) {
 				if (n + 2 > sz)
 					goto element_done;
@@ -988,9 +1009,6 @@ element_done:
 	if (!num_ele)
 		return 0;
 
-	if (!cfgmod_idx || *cfgmod_idx != PRIMARY_ELE_IDX)
-		return 0;
-
 	return n;
 }
 
@@ -1056,39 +1074,57 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
 	free_node_dbus_resources(node);
 }
 
-static void get_models_from_properties(struct node_element *ele,
-					struct l_dbus_message_iter *property,
-								bool vendor)
+static void get_sig_models_from_properties(struct node_element *ele,
+					struct l_dbus_message_iter *property)
 {
 	struct l_dbus_message_iter ids;
-	uint16_t mod_id, vendor_id;
-	const char *signature = !vendor ? "aq" : "a(qq)";
+	uint16_t mod_id;
 
 	if (!ele->models)
 		ele->models = l_queue_new();
 
-	if (!l_dbus_message_iter_get_variant(property, signature, &ids))
+	if (!l_dbus_message_iter_get_variant(property, "aq", &ids))
 		return;
 
 	/* Bluetooth SIG defined models */
-	if (!vendor) {
-		while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
-			struct mesh_model *mod;
+	while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
+		struct mesh_model *mod;
+		uint32_t id = mod_id | VENDOR_ID_MASK;
 
-			mod = mesh_model_new(ele->idx, mod_id);
-			if (!element_add_model(ele, mod))
-				mesh_model_free(mod);
-		}
-		return;
+		if (l_queue_find(ele->models, match_model_id,
+						L_UINT_TO_PTR(id)))
+			continue;
+
+		mod = mesh_model_new(ele->idx, id);
+
+		l_queue_insert(ele->models, mod, compare_model_id, NULL);
 	}
+}
+
+static void get_vendor_models_from_properties(struct node_element *ele,
+					struct l_dbus_message_iter *property)
+{
+	struct l_dbus_message_iter ids;
+	uint16_t mod_id, vendor_id;
+
+	if (!ele->models)
+		ele->models = l_queue_new();
+
+	if (!l_dbus_message_iter_get_variant(property, "a(qq)", &ids))
+		return;
 
 	/* Vendor defined models */
 	while (l_dbus_message_iter_next_entry(&ids, &vendor_id, &mod_id)) {
 		struct mesh_model *mod;
+		uint32_t id = mod_id | (vendor_id << 16);
+
+		if (l_queue_find(ele->models, match_model_id,
+							L_UINT_TO_PTR(id)))
+			continue;
 
-		mod = mesh_model_vendor_new(ele->idx, vendor_id, mod_id);
-		if (!element_add_model(ele, mod))
-			mesh_model_free(mod);
+		mod = mesh_model_new(ele->idx, id);
+
+		l_queue_insert(ele->models, mod, compare_model_id, NULL);
 	}
 }
 
@@ -1116,13 +1152,13 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
 		}
 
 		if (!mods && !strcmp(key, "Models")) {
-			get_models_from_properties(ele, &var, false);
+			get_sig_models_from_properties(ele, &var);
 			mods = true;
 			continue;
 		}
 
 		if (!vendor_mods && !strcmp(key, "VendorModels")) {
-			get_models_from_properties(ele, &var, true);
+			get_vendor_models_from_properties(ele, &var);
 			vendor_mods = true;
 			continue;
 		}
@@ -1146,6 +1182,16 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
 
 	ele->path = l_strdup(path);
 
+	/*
+	 * Add configuration server model on the primary element.
+	 * We allow the application not to specify the presense of
+	 * the Configuration Server model, since it's implemented by the
+	 * daemon. If the model is present in the application properties,
+	 * the operation below will be a "no-op".
+	 */
+	if (ele->idx == PRIMARY_ELE_IDX)
+		add_internal_model(node, CONFIG_SRV_MODEL, PRIMARY_ELE_IDX);
+
 	return true;
 fail:
 	l_free(ele);
@@ -1218,6 +1264,9 @@ static bool create_node_config(struct mesh_node *node, const uint8_t uuid[16])
 	storage_dir = mesh_get_storage_dir();
 	node->cfg = mesh_config_create(storage_dir, uuid, &db_node);
 
+	if (node->cfg)
+		init_storage_dir(node);
+
 	/* Free temporarily allocated resources */
 	entry = l_queue_get_entries(db_node.elements);
 	for (; entry; entry = entry->next) {
@@ -1231,21 +1280,6 @@ static bool create_node_config(struct mesh_node *node, const uint8_t uuid[16])
 	return node->cfg != NULL;
 }
 
-static void set_defaults(struct mesh_node *node)
-{
-	/* TODO: these values should come from mesh.conf */
-	node->lpn = MESH_MODE_UNSUPPORTED;
-	node->proxy = MESH_MODE_UNSUPPORTED;
-	node->friend = MESH_MODE_UNSUPPORTED;
-	node->beacon = MESH_MODE_DISABLED;
-	node->relay.mode = MESH_MODE_DISABLED;
-	node->ttl = TTL_MASK;
-	node->seq_number = DEFAULT_SEQUENCE_NUMBER;
-
-	/* Add configuration server model on primary element */
-	add_internal_model(node, CONFIG_SRV_MODEL, PRIMARY_ELE_IDX);
-}
-
 static bool get_app_properties(struct mesh_node *node, const char *path,
 					struct l_dbus_message_iter *properties)
 {
@@ -1357,29 +1391,6 @@ static bool add_local_node(struct mesh_node *node, uint16_t unicast, bool kr,
 	return true;
 }
 
-static bool init_storage_dir(struct mesh_node *node)
-{
-	char uuid[33];
-	char dir_name[PATH_MAX];
-
-	if (node->storage_dir)
-		return true;
-
-	if (!hex2str(node->uuid, 16, uuid, sizeof(uuid)))
-		return false;
-
-	snprintf(dir_name, PATH_MAX, "%s/%s", mesh_get_storage_dir(), uuid);
-
-	if (strlen(dir_name) >= PATH_MAX)
-		return false;
-
-	create_dir(dir_name);
-
-	node->storage_dir = l_strdup(dir_name);
-
-	return true;
-}
-
 static bool check_req_node(struct managed_obj_request *req)
 {
 	uint8_t node_comp[MAX_MSG_LEN - 2];
@@ -1409,12 +1420,19 @@ static bool check_req_node(struct managed_obj_request *req)
 	return true;
 }
 
-static struct mesh_node *attach_req_node(struct mesh_node *attach,
-							struct mesh_node *node)
+static bool attach_req_node(struct mesh_node *attach, struct mesh_node *node)
 {
 	const struct l_queue_entry *attach_entry;
 	const struct l_queue_entry *node_entry;
 
+	attach->obj_path = node->obj_path;
+	node->obj_path = NULL;
+
+	if (!register_node_object(attach)) {
+		free_node_dbus_resources(attach);
+		return false;
+	}
+
 	attach_entry = l_queue_get_entries(attach->elements);
 	node_entry = l_queue_get_entries(node->elements);
 
@@ -1447,7 +1465,7 @@ static struct mesh_node *attach_req_node(struct mesh_node *attach,
 
 	node_remove(node);
 
-	return attach;
+	return true;
 }
 
 static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
@@ -1456,9 +1474,12 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	struct managed_obj_request *req = user_data;
 	const char *path;
 	struct mesh_node *node = req->node;
+	struct node_import *import;
 	void *agent = NULL;
 	bool have_app = false;
 	unsigned int num_ele;
+	struct keyring_net_key net_key;
+	uint8_t dev_key[16];
 
 	if (l_dbus_message_is_error(msg)) {
 		l_error("Failed to get app's dbus objects");
@@ -1470,9 +1491,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		goto fail;
 	}
 
-	if (!node->elements)
-		node->elements = l_queue_new();
-
 	while (l_dbus_message_iter_next_entry(&objects, &path, &interfaces)) {
 		struct l_dbus_message_iter properties;
 		const char *interface;
@@ -1531,7 +1549,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		goto fail;
 	}
 
-	set_defaults(node);
 	num_ele = l_queue_length(node->elements);
 
 	if (num_ele > MAX_ELE_COUNT)
@@ -1542,26 +1559,19 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 	if (!check_req_node(req))
 		goto fail;
 
-	if (req->type == REQUEST_TYPE_ATTACH) {
-		struct l_dbus *bus = dbus_get_bus();
-
-		node = attach_req_node(req->attach, node);
-
-		if (!register_node_object(node))
+	switch (req->type) {
+	case REQUEST_TYPE_ATTACH:
+		if (!attach_req_node(req->attach, node))
 			goto fail;
 
-		/*
-		 * TODO: For now always initialize directory for storing
-		 * keyring info. Need to figure out what checks need
-		 * to be performed to do this conditionally, i.e., presence of
-		 * Provisioner interface, etc.
-		 */
-		init_storage_dir(node);
+		req->attach->disc_watch = l_dbus_add_disconnect_watch(
+					dbus_get_bus(), req->attach->owner,
+					app_disc_cb, req->attach, NULL);
 
-		node->disc_watch = l_dbus_add_disconnect_watch(bus,
-				node->owner, app_disc_cb, node, NULL);
+		req->ready_cb(req->pending_msg, MESH_ERROR_NONE, req->attach);
+		return;
 
-	} else if (req->type == REQUEST_TYPE_JOIN) {
+	case REQUEST_TYPE_JOIN:
 		if (!node->agent) {
 			l_error("Interface %s not found",
 						MESH_PROVISION_AGENT_INTERFACE);
@@ -1571,26 +1581,27 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 		if (!create_node_config(node, node->uuid))
 			goto fail;
 
-	} else if (req->type == REQUEST_TYPE_IMPORT) {
-		struct node_import *import = req->import;
+		req->join_ready_cb(node, node->agent);
+
+		return;
 
+	case REQUEST_TYPE_IMPORT:
 		if (!create_node_config(node, node->uuid))
 			goto fail;
 
+		import = req->import;
 		if (!add_local_node(node, import->unicast, import->flags.kr,
 					import->flags.ivu,
 					import->iv_index, import->dev_key,
 					import->net_idx, import->net_key))
 			goto fail;
 
-		/* Initialize directory for storing keyring info */
-		init_storage_dir(node);
+		req->ready_cb(req->pending_msg, MESH_ERROR_NONE, node);
+		l_free(import);
 
-	} else {
-		/* Callback for create node request */
-		struct keyring_net_key net_key;
-		uint8_t dev_key[16];
+		return;
 
+	case REQUEST_TYPE_CREATE:
 		if (!create_node_config(node, node->uuid))
 			goto fail;
 
@@ -1606,9 +1617,6 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
 						net_key.old_key))
 			goto fail;
 
-		/* Initialize directory for storing keyring info */
-		init_storage_dir(node);
-
 		if (!keyring_put_remote_dev_key(node, DEFAULT_NEW_UNICAST,
 						node->num_ele, dev_key))
 			goto fail;
@@ -1616,14 +1624,12 @@ 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;
 
-	}
-
-	if (req->type == REQUEST_TYPE_JOIN)
-		req->join_ready_cb(node, node->agent);
-	else
 		req->ready_cb(req->pending_msg, MESH_ERROR_NONE, node);
+		return;
 
-	goto done;
+	default:
+		goto fail;
+	}
 
 fail:
 	if (agent)
@@ -1638,7 +1644,6 @@ fail:
 	else
 		req->ready_cb(req->pending_msg, MESH_ERROR_FAILED, NULL);
 
-done:
 	if (req->type == REQUEST_TYPE_IMPORT)
 		l_free(req->import);
 }
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH BlueZ 3/3] mesh: Initialize net modes based on node configuration
  2019-12-06 20:57 [PATCH BlueZ 0/3] mesh: clean up Inga Stotland
  2019-12-06 20:57 ` [PATCH BlueZ 1/3] mesh: Delete unused function Inga Stotland
  2019-12-06 20:57 ` [PATCH BlueZ 2/3] mesh: Clean up node.c Inga Stotland
@ 2019-12-06 20:57 ` Inga Stotland
  2019-12-10 18:40 ` [PATCH BlueZ 0/3] mesh: clean up Gix, Brian
  3 siblings, 0 replies; 6+ messages in thread
From: Inga Stotland @ 2019-12-06 20:57 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This correctly initializes net settings related to node features
based on node configuration: either defaults in the case of
a newly node created/provisioned/imported node or the configured
values read from stored existing node.
---
 mesh/mesh-defs.h |  2 ++
 mesh/net.c       | 15 ++-------------
 mesh/node.c      | 45 +++++++++++++++++++++++++++------------------
 3 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
index 8f28fc89b..9353d7351 100644
--- a/mesh/mesh-defs.h
+++ b/mesh/mesh-defs.h
@@ -109,6 +109,8 @@
 #define APP_IDX_DEV_REMOTE	0x6fff
 #define APP_IDX_DEV_LOCAL	0x7fff
 
+#define DEFAULT_SEQUENCE_NUMBER 0x000000
+
 #define IS_UNASSIGNED(x)	((x) == UNASSIGNED_ADDRESS)
 #define IS_UNICAST(x)		(((x) > UNASSIGNED_ADDRESS) && \
 					((x) < VIRTUAL_ADDRESS_LOW))
diff --git a/mesh/net.c b/mesh/net.c
index 17dbf2ec2..f662d8a91 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -679,20 +679,9 @@ struct mesh_net *mesh_net_new(struct mesh_node *node)
 	net = l_new(struct mesh_net, 1);
 
 	net->node = node;
-	net->pkt_id = 0;
-	net->bea_id = 0;
-
-	net->beacon_enable = true;
-	net->proxy_enable = false;
-	net->relay.enable = false;
-
-	net->seq_num = 0x000000;
-	net->src_addr = 0x0000;
-	net->default_ttl = 0x7f;
-
-	net->provisioner = false;
+	net->seq_num = DEFAULT_SEQUENCE_NUMBER;
+	net->default_ttl = TTL_MASK;
 
-	net->test_mode = false;
 	memset(&net->prov_caps, 0, sizeof(net->prov_caps));
 	net->prov_caps.algorithms = 1;
 
diff --git a/mesh/node.c b/mesh/node.c
index edf6fce37..f8acc78c3 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -57,7 +57,6 @@
 #define DEFAULT_LOCATION 0x0000
 
 #define DEFAULT_CRPL 10
-#define DEFAULT_SEQUENCE_NUMBER 0
 
 enum request_type {
 	REQUEST_TYPE_JOIN,
@@ -219,6 +218,7 @@ static int compare_model_id(const void *a, const void *b, void *user_data)
 	return 0;
 }
 
+
 struct mesh_node *node_find_by_addr(uint16_t addr)
 {
 	if (!IS_UNICAST(addr))
@@ -486,12 +486,34 @@ static bool init_storage_dir(struct mesh_node *node)
 	return true;
 }
 
+static void update_net_settings(struct mesh_node *node)
+{
+	uint8_t mode;
+
+	mode = node->proxy;
+	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
+		mesh_net_set_proxy_mode(node->net, mode == MESH_MODE_ENABLED);
+
+	mode = node->friend;
+	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
+		mesh_net_set_friend_mode(node->net, mode == MESH_MODE_ENABLED);
+
+	mode = node->relay.mode;
+	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
+		mesh_net_set_relay_mode(node->net, mode == MESH_MODE_ENABLED,
+					node->relay.cnt, node->relay.interval);
+
+	mode = node->beacon;
+	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
+		mesh_net_set_beacon_mode(node->net, mode == MESH_MODE_ENABLED);
+}
+
 static bool init_from_storage(struct mesh_config_node *db_node,
 			const uint8_t uuid[16], struct mesh_config *cfg,
 			void *user_data)
 {
 	unsigned int num_ele;
-	uint8_t mode;
+
 	struct mesh_node *node = node_new(uuid);
 
 	if (!nodes)
@@ -554,22 +576,7 @@ static bool init_from_storage(struct mesh_config_node *db_node,
 	mesh_net_set_seq_num(node->net, node->seq_number);
 	mesh_net_set_default_ttl(node->net, node->ttl);
 
-	mode = node->proxy;
-	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
-		mesh_net_set_proxy_mode(node->net, mode == MESH_MODE_ENABLED);
-
-	mode = node->friend;
-	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
-		mesh_net_set_friend_mode(node->net, mode == MESH_MODE_ENABLED);
-
-	mode = node->relay.mode;
-	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
-		mesh_net_set_relay_mode(node->net, mode == MESH_MODE_ENABLED,
-					node->relay.cnt, node->relay.interval);
-
-	mode = node->beacon;
-	if (mode == MESH_MODE_ENABLED || mode == MESH_MODE_DISABLED)
-		mesh_net_set_beacon_mode(node->net, mode == MESH_MODE_ENABLED);
+	update_net_settings(node);
 
 	/* Initialize configuration server model */
 	cfgmod_server_init(node, PRIMARY_ELE_IDX);
@@ -1383,6 +1390,8 @@ static bool add_local_node(struct mesh_node *node, uint16_t unicast, bool kr,
 			return false;
 	}
 
+	update_net_settings(node);
+
 	mesh_config_save(node->cfg, true, NULL, NULL);
 
 	/* Initialize configuration server model */
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ 0/3] mesh: clean up
  2019-12-06 20:57 [PATCH BlueZ 0/3] mesh: clean up Inga Stotland
                   ` (2 preceding siblings ...)
  2019-12-06 20:57 ` [PATCH BlueZ 3/3] mesh: Initialize net modes based on node configuration Inga Stotland
@ 2019-12-10 18:40 ` Gix, Brian
  3 siblings, 0 replies; 6+ messages in thread
From: Gix, Brian @ 2019-12-10 18:40 UTC (permalink / raw)
  To: linux-bluetooth, Stotland, Inga

Patchset Applied

On Fri, 2019-12-06 at 12:57 -0800, Inga Stotland wrote:
> This set of patches claens up some cruft in node.c and some in net.c
> The dtaaflow is a bit more deterministic now.
> 
> Inga Stotland (3):
>   mesh: Delete unused function
>   mesh: Clean up node.c
>   mesh: Initialize net modes based on node configuration
> 
>  mesh/mesh-defs.h |   2 +
>  mesh/model.c     |  17 +-
>  mesh/model.h     |   4 +-
>  mesh/net.c       |  15 +-
>  mesh/node.c      | 503 ++++++++++++++++++-----------------------------
>  mesh/node.h      |   1 -
>  6 files changed, 200 insertions(+), 342 deletions(-)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH BlueZ 2/3] mesh: Clean up node.c
  2019-12-06 20:57 ` [PATCH BlueZ 2/3] mesh: Clean up node.c Inga Stotland
@ 2019-12-11 15:48   ` Michał Lowas-Rzechonek
  0 siblings, 0 replies; 6+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-12-11 15:48 UTC (permalink / raw)
  To: Inga Stotland; +Cc: linux-bluetooth, brian.gix

Hi Inga,

On 12/06, Inga Stotland wrote:
> @@ -876,8 +902,6 @@ uint16_t node_generate_comp(struct mesh_node *node, uint8_t *buf, uint16_t sz)
>  {
>  	uint16_t n, features;
>  	uint16_t num_ele = 0;
> -	uint8_t *cfgmod_idx = NULL;
> -
>  	const struct l_queue_entry *ele_entry;
>  
>  	if (!node || !node->comp || sz < MIN_COMP_SIZE)
> @@ -940,9 +964,6 @@ uint16_t node_generate_comp(struct mesh_node *node, uint8_t *buf, uint16_t sz)
>  			mod_id = mesh_model_get_model_id(
>  					(const struct mesh_model *) mod);
>  
> -			if (mod_id == CONFIG_SRV_MODEL)
> -				cfgmod_idx = &ele->idx;
> -
>  			if ((mod_id & VENDOR_ID_MASK) == VENDOR_ID_MASK) {
>  				if (n + 2 > sz)
>  					goto element_done;
> @@ -988,9 +1009,6 @@ element_done:
>  	if (!num_ele)
>  		return 0;
>  
> -	if (!cfgmod_idx || *cfgmod_idx != PRIMARY_ELE_IDX)
> -		return 0;
> -
>  	return n;
>  }

Note that this cause the daemon to no longer reject applications that
declare presence of Config Server Model on non-primary element.

-- 
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, other threads:[~2019-12-11 15:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06 20:57 [PATCH BlueZ 0/3] mesh: clean up Inga Stotland
2019-12-06 20:57 ` [PATCH BlueZ 1/3] mesh: Delete unused function Inga Stotland
2019-12-06 20:57 ` [PATCH BlueZ 2/3] mesh: Clean up node.c Inga Stotland
2019-12-11 15:48   ` Michał Lowas-Rzechonek
2019-12-06 20:57 ` [PATCH BlueZ 3/3] mesh: Initialize net modes based on node configuration Inga Stotland
2019-12-10 18:40 ` [PATCH BlueZ 0/3] mesh: clean up Gix, Brian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.