Linux-Bluetooth Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH BlueZ 00/10] Clean up Config Server
@ 2020-07-30 20:38 Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 01/10] mesh: Clean up handling of config subscription messages Inga Stotland
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This patch set cleans up implementation of config server.
Specifically, all the response status messages are sent from
a single point.

Along the way, some behavioral inconsistencies are fixed like
ignoring more malformed messages, returning correct error
status codes.

Inga Stotland (10):
  mesh: Clean up handling of config subscription messages
  mesh: Clean up handling of config model binding messages
  mesh: Clean up handling of config node identity message
  mesh: Clean up handling of config publication messages
  mesh: Clean up handling of config net and app key messages
  mesh: Clean up handling of config relay messages
  mesh: Clean up handling of config poll timeout message
  mesh: Clean up handling of config net transmit messages
  mesh: Clean up handling of config KR phase messages
  mesh: Refactor heartbeat pub/sub

 mesh/cfgmod-server.c    | 1149 ++++++++++++++++-----------------------
 mesh/cfgmod.h           |    2 +-
 mesh/mesh-config-json.c |   12 +-
 mesh/mesh-config.h      |    6 +-
 mesh/model.c            |  227 ++++----
 mesh/model.h            |   21 +-
 mesh/net.c              |  273 +++++++---
 mesh/net.h              |   48 +-
 8 files changed, 851 insertions(+), 887 deletions(-)

-- 
2.26.2


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

* [PATCH BlueZ 01/10] mesh: Clean up handling of config subscription messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 02/10] mesh: Clean up handling of config model binding messages Inga Stotland
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This provides better functional grouping based on whether a group or
a virtual label is used for the subscription address.

Also, use a single point for sending out the composed Config Server
status messages.
---
 mesh/cfgmod-server.c    | 341 +++++++++++++++++-----------------------
 mesh/mesh-config-json.c |  12 +-
 mesh/mesh-config.h      |   6 +-
 mesh/model.c            | 211 ++++++++++++++-----------
 mesh/model.h            |  21 ++-
 5 files changed, 295 insertions(+), 296 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 7672ad3b6..73cf66ffd 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -32,6 +32,10 @@
 #include "mesh/mesh-config.h"
 #include "mesh/cfgmod.h"
 
+#define CFG_SET_ID(vendor, pkt)	((vendor) ?	\
+		(SET_ID(l_get_le16((pkt)), l_get_le16((pkt) + 2))) :	\
+		(SET_ID(SIG_VENDOR, l_get_le16(pkt))))
+
 /* Supported composition pages, sorted high to low */
 /* Only page 0 is currently supported */
 static const uint8_t supported_pages[] = {
@@ -185,237 +189,178 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 				idx, cred_flag, ttl, period, retransmit);
 }
 
-static void send_sub_status(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
-					uint8_t status, uint16_t ele_addr,
-					uint16_t addr, uint32_t id)
-{
-	int n = mesh_model_opcode_set(OP_CONFIG_MODEL_SUB_STATUS, msg);
-
-	msg[n++] = status;
-	l_put_le16(ele_addr, msg + n);
-	n += 2;
-	l_put_le16(addr, msg + n);
-	n += 2;
-
-	if (IS_VENDOR(id)) {
-		l_put_le16(VENDOR_ID(id), msg + n);
-		l_put_le16(MODEL_ID(id), msg + n + 2);
-		n += 4;
-	} else {
-		l_put_le16(MODEL_ID(id), msg + n);
-		n += 2;
-	}
-
-	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
-								false, msg, n);
-}
-
-static bool config_sub_get(struct mesh_node *node, uint16_t net_idx,
+static uint16_t cfg_sub_get_msg(struct mesh_node *node, uint16_t net_idx,
 					uint16_t src, uint16_t dst,
 					const uint8_t *pkt, uint16_t size)
 {
-	uint16_t ele_addr;
+	uint16_t ele_addr, n, sub_len;
 	uint32_t id;
-	uint16_t n = 0;
-	int status;
-	uint8_t *msg_status;
-	uint16_t buf_size;
+	int opcode;
+	bool vendor = (size == 6);
 
-	/* Incoming message has already been size-checked */
 	ele_addr = l_get_le16(pkt);
+	id = CFG_SET_ID(vendor, pkt + 2);
+	opcode = vendor ? OP_CONFIG_VEND_MODEL_SUB_LIST :
+						OP_CONFIG_MODEL_SUB_LIST;
+	n = mesh_model_opcode_set(opcode, msg);
+	memcpy(msg + n + 1, pkt, size);
 
-	switch (size) {
-	default:
-		l_debug("Bad length %d", size);
-		return false;
+	msg[n] = mesh_model_sub_get(node, ele_addr, id, msg + n + 1 + size,
+					MAX_MSG_LEN - (n + 1 + size), &sub_len);
 
-	case 4:
-		id = l_get_le16(pkt + 2);
-		n = mesh_model_opcode_set(OP_CONFIG_MODEL_SUB_LIST, msg);
-		msg_status = msg + n;
-		msg[n++] = 0;
-		l_put_le16(ele_addr, msg + n);
-		n += 2;
-		l_put_le16(id, msg + n);
-		n += 2;
-		id = SET_ID(SIG_VENDOR, id);
-		break;
+	if (msg[n] == MESH_STATUS_SUCCESS)
+		n += sub_len;
 
-	case 6:
-		id = SET_ID(l_get_le16(pkt + 2), l_get_le16(pkt + 4));
-		n = mesh_model_opcode_set(OP_CONFIG_VEND_MODEL_SUB_LIST, msg);
-		msg_status = msg + n;
-		msg[n++] = 0;
-		l_put_le16(ele_addr, msg + n);
-		n += 2;
-		l_put_le16(VENDOR_ID(id), msg + n);
-		n += 2;
-		l_put_le16(MODEL_ID(id), msg + n);
-		n += 2;
-		break;
-	}
-
-	buf_size = sizeof(uint16_t) * MAX_GRP_PER_MOD;
-	status = mesh_model_sub_get(node, ele_addr, id, msg + n, buf_size,
-									&size);
-
-	if (status == MESH_STATUS_SUCCESS)
-		n += size;
-
-	*msg_status = (uint8_t) status;
-
-	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
-								false, msg, n);
-	return true;
+	n += (size + 1);
+	return n;
 }
 
-static bool save_config_sub(struct mesh_node *node, uint16_t ele_addr,
-					uint32_t id, bool vendor,
-					const uint8_t *addr, bool virt,
-					uint16_t grp, uint32_t opcode)
+static bool save_cfg_sub(struct mesh_node *node, uint16_t ele_addr,
+				uint32_t id, bool vendor, const uint8_t *label,
+				bool virt, uint16_t grp, uint32_t opcode)
 {
+	struct mesh_config *cfg = node_config_get(node);
 	struct mesh_config_sub db_sub = {
 				.virt = virt,
-				.src.addr = grp
+				.addr.grp = grp
 				};
 
+	id = (vendor) ? id : MODEL_ID(id);
+
 	if (virt)
-		memcpy(db_sub.src.virt_addr, addr, 16);
+		memcpy(db_sub.addr.label, label, 16);
+
+	if (opcode == OP_CONFIG_MODEL_SUB_VIRT_DELETE &&
+			opcode == OP_CONFIG_MODEL_SUB_DELETE)
+		return mesh_config_model_sub_del(cfg, ele_addr, id, vendor,
+								&db_sub);
 
 	if (opcode == OP_CONFIG_MODEL_SUB_VIRT_OVERWRITE ||
 					opcode == OP_CONFIG_MODEL_SUB_OVERWRITE)
-		mesh_config_model_sub_del_all(node_config_get(node), ele_addr,
-						vendor ? id : MODEL_ID(id),
-									vendor);
 
-	if (opcode != OP_CONFIG_MODEL_SUB_VIRT_DELETE &&
-			opcode != OP_CONFIG_MODEL_SUB_DELETE)
-		return mesh_config_model_sub_add(node_config_get(node),
-					ele_addr, vendor ? id : MODEL_ID(id),
-					vendor, &db_sub);
-	else
-		return mesh_config_model_sub_del(node_config_get(node),
-					ele_addr, vendor ? id : MODEL_ID(id),
-					vendor, &db_sub);
+		if (!mesh_config_model_sub_del_all(cfg, ele_addr, id, vendor))
+			return false;
+
+	return mesh_config_model_sub_add(cfg, ele_addr, id, vendor, &db_sub);
 }
 
-static void config_sub_set(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
-					const uint8_t *pkt, uint16_t size,
-					bool virt, uint32_t opcode)
+static uint16_t cfg_sub_add_msg(struct mesh_node *node, const uint8_t *pkt,
+					bool vendor, uint32_t opcode)
 {
-	uint16_t grp, ele_addr;
+	uint16_t addr, ele_addr, n;
 	uint32_t id;
-	const uint8_t *addr = NULL;
-	int status = MESH_STATUS_SUCCESS;
-	bool vendor = false;
 
-	switch (size) {
-	default:
-		l_error("Bad length: %d", size);
-		return;
-	case 4:
-		if (opcode != OP_CONFIG_MODEL_SUB_DELETE_ALL)
-			return;
+	addr = l_get_le16(pkt + 2);
 
-		id = SET_ID(SIG_VENDOR, l_get_le16(pkt + 2));
-		break;
-	case 6:
-		if (virt)
-			return;
+	if (!IS_GROUP(addr))
+		return 0;
 
-		if (opcode != OP_CONFIG_MODEL_SUB_DELETE_ALL) {
-			id = SET_ID(SIG_VENDOR, l_get_le16(pkt + 4));
-		} else {
-			id = SET_ID(l_get_le16(pkt + 2), l_get_le16(pkt + 4));
-			vendor = true;
-		}
+	ele_addr = l_get_le16(pkt);
+	id = CFG_SET_ID(vendor, pkt + 4);
 
-		break;
-	case 8:
-		if (virt)
-			return;
+	n = mesh_model_opcode_set(OP_CONFIG_MODEL_SUB_STATUS, msg);
 
-		id = SET_ID(l_get_le16(pkt + 4), l_get_le16(pkt + 6));
-		vendor = true;
-		break;
-	case 20:
-		if (!virt)
-			return;
+	if (opcode == OP_CONFIG_MODEL_SUB_OVERWRITE)
+		msg[n] = mesh_model_sub_ovrt(node, ele_addr, id, addr);
+	else if (opcode == OP_CONFIG_MODEL_SUB_ADD)
+		msg[n] = mesh_model_sub_add(node, ele_addr, id, addr);
+	else
+		msg[n] = mesh_model_sub_del(node, ele_addr, id, addr);
 
-		id = SET_ID(SIG_VENDOR, l_get_le16(pkt + 18));
-		break;
-	case 22:
-		if (!virt)
-			return;
+	if (msg[n] == MESH_STATUS_SUCCESS &&
+			!save_cfg_sub(node, ele_addr, id, vendor, NULL, false,
+								addr, opcode))
+		msg[n] = MESH_STATUS_STORAGE_FAIL;
 
-		vendor = true;
-		id = SET_ID(l_get_le16(pkt + 18), l_get_le16(pkt + 20));
-		break;
+	if (vendor) {
+		memcpy(msg + n + 1, pkt, 8);
+		n += 9;
+	} else {
+		memcpy(msg + n + 1, pkt, 6);
+		n += 7;
 	}
 
+	return n;
+}
+
+static uint16_t cfg_virt_sub_add_msg(struct mesh_node *node, const uint8_t *pkt,
+						bool vendor, uint32_t opcode)
+{
+	uint16_t addr, ele_addr, n;
+	uint32_t id;
+	const uint8_t *label;
+
+	n = mesh_model_opcode_set(OP_CONFIG_MODEL_SUB_STATUS, msg);
+
 	ele_addr = l_get_le16(pkt);
+	label = pkt + 2;
+	id = CFG_SET_ID(vendor, pkt + 18);
+
+	if (opcode == OP_CONFIG_MODEL_SUB_VIRT_OVERWRITE)
+		msg[n] = mesh_model_virt_sub_ovrt(node, ele_addr, id, label,
+									&addr);
+	else if (opcode == OP_CONFIG_MODEL_SUB_VIRT_ADD)
+		msg[n] = mesh_model_virt_sub_add(node, ele_addr, id, label,
+									&addr);
+	else
+		msg[n] = mesh_model_virt_sub_del(node, ele_addr, id, label,
+									&addr);
 
-	if (opcode != OP_CONFIG_MODEL_SUB_DELETE_ALL) {
-		addr = pkt + 2;
-		grp = l_get_le16(addr);
-	} else
-		grp = UNASSIGNED_ADDRESS;
+	if (msg[n] == MESH_STATUS_SUCCESS &&
+				!save_cfg_sub(node, ele_addr, id, vendor,
+						label, true, addr, opcode))
+		msg[n] = MESH_STATUS_STORAGE_FAIL;
 
-	switch (opcode) {
-	default:
-		l_debug("Bad opcode: %x", opcode);
-		return;
+	l_put_le16(ele_addr, msg + n + 1);
+	l_put_le16(addr, msg + n + 3);
 
-	case OP_CONFIG_MODEL_SUB_DELETE_ALL:
-		status = mesh_model_sub_del_all(node, ele_addr, id);
+	if (vendor) {
+		l_put_le16(VENDOR_ID(id), msg + n + 5);
+		l_put_le16(MODEL_ID(id), msg + n + 7);
+		n += 9;
+	} else {
+		l_put_le16(MODEL_ID(id), msg + n + 5);
+		n += 7;
+	}
 
-		if (status == MESH_STATUS_SUCCESS)
-			mesh_config_model_sub_del_all(node_config_get(node),
-					ele_addr, vendor ? id : MODEL_ID(id),
-									vendor);
-		break;
+	return n;
+}
 
-	case OP_CONFIG_MODEL_SUB_VIRT_OVERWRITE:
-		grp = UNASSIGNED_ADDRESS;
-		/* Fall Through */
-	case OP_CONFIG_MODEL_SUB_OVERWRITE:
-		status = mesh_model_sub_ovr(node, ele_addr, id,
-							addr, virt, &grp);
+static uint16_t config_sub_del_all(struct mesh_node *node, const uint8_t *pkt,
+								bool vendor)
+{
+	uint16_t ele_addr, n, grp = UNASSIGNED_ADDRESS;
+	uint32_t id;
 
-		if (status == MESH_STATUS_SUCCESS)
-			save_config_sub(node, ele_addr, id, vendor, addr,
-							virt, grp, opcode);
-		break;
-	case OP_CONFIG_MODEL_SUB_VIRT_ADD:
-		grp = UNASSIGNED_ADDRESS;
-		/* Fall Through */
-	case OP_CONFIG_MODEL_SUB_ADD:
-		status = mesh_model_sub_add(node, ele_addr, id,
-							addr, virt, &grp);
+	n = mesh_model_opcode_set(OP_CONFIG_MODEL_SUB_STATUS, msg);
 
-		if (status == MESH_STATUS_SUCCESS &&
-				!save_config_sub(node, ele_addr, id, vendor,
-						addr, virt, grp, opcode))
-			status = MESH_STATUS_STORAGE_FAIL;
+	ele_addr = l_get_le16(pkt);
+	id = CFG_SET_ID(vendor, pkt + 2);
 
-		break;
-	case OP_CONFIG_MODEL_SUB_VIRT_DELETE:
-		grp = UNASSIGNED_ADDRESS;
-		/* Fall Through */
-	case OP_CONFIG_MODEL_SUB_DELETE:
-		status = mesh_model_sub_del(node, ele_addr, id, addr, virt,
-									&grp);
+	msg[n] = mesh_model_sub_del_all(node, ele_addr, id);
 
-		if (status == MESH_STATUS_SUCCESS)
-			save_config_sub(node, ele_addr, id, vendor, addr,
-							virt, grp, opcode);
+	if (msg[n] == MESH_STATUS_SUCCESS) {
+		struct mesh_config *cfg = node_config_get(node);
 
-		break;
+		if (!mesh_config_model_sub_del_all(cfg, ele_addr,
+						vendor ? id : MODEL_ID(id),
+						vendor))
+			msg[n] = MESH_STATUS_STORAGE_FAIL;
+	}
+
+	l_put_le16(ele_addr, msg + n + 1);
+	l_put_le16(grp, msg + n + 3);
+
+	if (vendor) {
+		l_put_le16(VENDOR_ID(id), msg + n + 5);
+		l_put_le16(MODEL_ID(id), msg + n + 7);
+		n += 9;
+	} else {
+		l_put_le16(MODEL_ID(id), msg + n + 5);
+		n += 7;
 	}
 
-	send_sub_status(node, net_idx, src, dst, status, ele_addr, grp, id);
+	return n;
 }
 
 static void send_model_app_status(struct mesh_node *node, uint16_t net_idx,
@@ -797,28 +742,38 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	case OP_CONFIG_VEND_MODEL_SUB_GET:
 		if (size != 6)
 			return true;
-
-		config_sub_get(node, net_idx, src, dst, pkt, size);
-		break;
+		/* Fall Through */
 
 	case OP_CONFIG_MODEL_SUB_GET:
-		if (size != 4)
+		if (size != 4 && opcode == OP_CONFIG_MODEL_SUB_GET)
 			return true;
 
-		config_sub_get(node, net_idx, src, dst, pkt, size);
+		n = cfg_sub_get_msg(node, net_idx, src, dst, pkt, size);
 		break;
 
 	case OP_CONFIG_MODEL_SUB_VIRT_OVERWRITE:
 	case OP_CONFIG_MODEL_SUB_VIRT_DELETE:
 	case OP_CONFIG_MODEL_SUB_VIRT_ADD:
-		virt = true;
-		/* Fall Through */
+		if (size != 20 && size != 22)
+			return true;
+
+		n = cfg_virt_sub_add_msg(node, pkt, size == 22, opcode);
+		break;
+
 	case OP_CONFIG_MODEL_SUB_OVERWRITE:
 	case OP_CONFIG_MODEL_SUB_DELETE:
 	case OP_CONFIG_MODEL_SUB_ADD:
+		if (size != 6 && size != 8)
+			return true;
+
+		n = cfg_sub_add_msg(node, pkt, size == 8, opcode);
+		break;
+
 	case OP_CONFIG_MODEL_SUB_DELETE_ALL:
-		config_sub_set(node, net_idx, src, dst, pkt, size, virt,
-									opcode);
+		if (size != 4 && size != 6)
+			return true;
+
+		n = config_sub_del_all(node, pkt, size == 6);
 		break;
 
 	case OP_CONFIG_RELAY_SET:
diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
index deb0019f9..a40f92c01 100644
--- a/mesh/mesh-config-json.c
+++ b/mesh/mesh-config-json.c
@@ -1069,11 +1069,11 @@ static bool parse_model_subscriptions(json_object *jsubs,
 
 		switch (len) {
 		case 4:
-			if (sscanf(str, "%04hx", &subs[i].src.addr) != 1)
+			if (sscanf(str, "%04hx", &subs[i].addr.grp) != 1)
 				goto fail;
 		break;
 		case 32:
-			if (!str2hex(str, len, subs[i].src.virt_addr, 16))
+			if (!str2hex(str, len, subs[i].addr.label, 16))
 				goto fail;
 			subs[i].virt = true;
 			break;
@@ -2068,10 +2068,10 @@ bool mesh_config_model_sub_add(struct mesh_config *cfg, uint16_t ele_addr,
 		return false;
 
 	if (!sub->virt) {
-		snprintf(buf, 5, "%4.4x", sub->src.addr);
+		snprintf(buf, 5, "%4.4x", sub->addr.grp);
 		len = 4;
 	} else {
-		hex2str(sub->src.virt_addr, 16, buf, 33);
+		hex2str(sub->addr.label, 16, buf, 33);
 		len = 32;
 	}
 
@@ -2122,10 +2122,10 @@ bool mesh_config_model_sub_del(struct mesh_config *cfg, uint16_t ele_addr,
 		return true;
 
 	if (!sub->virt) {
-		snprintf(buf, 5, "%4.4x", sub->src.addr);
+		snprintf(buf, 5, "%4.4x", sub->addr.grp);
 		len = 4;
 	} else {
-		hex2str(sub->src.virt_addr, 16, buf, 33);
+		hex2str(sub->addr.label, 16, buf, 33);
 		len = 32;
 	}
 
diff --git a/mesh/mesh-config.h b/mesh/mesh-config.h
index 7dfa9f20c..f15f3f376 100644
--- a/mesh/mesh-config.h
+++ b/mesh/mesh-config.h
@@ -24,9 +24,9 @@ struct mesh_config;
 struct mesh_config_sub {
 	bool virt;
 	union {
-		uint16_t	addr;
-		uint8_t	virt_addr[16];
-	} src;
+		uint16_t grp;
+		uint8_t	label[16];
+	} addr;
 };
 
 struct mesh_config_pub {
diff --git a/mesh/model.c b/mesh/model.c
index ef7668147..3c9b6577a 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -664,7 +664,7 @@ static int update_binding(struct mesh_node *node, uint16_t addr, uint32_t id,
 		return MESH_STATUS_SUCCESS;
 	}
 
-	if (l_queue_length(mod->bindings) >= MAX_BINDINGS)
+	if (l_queue_length(mod->bindings) >= MAX_MODEL_BINDINGS)
 		return MESH_STATUS_INSUFF_RESOURCES;
 
 	if (!mesh_config_model_binding_add(node_config_get(node), addr,
@@ -737,7 +737,7 @@ static int set_virt_pub(struct mesh_model *mod, const uint8_t *label,
 }
 
 static int add_virt_sub(struct mesh_net *net, struct mesh_model *mod,
-			     const uint8_t *label, uint16_t *dst)
+				const uint8_t *label, uint16_t *addr)
 {
 	struct mesh_virtual *virt = l_queue_find(mod->virtuals,
 						find_virt_by_label, label);
@@ -745,40 +745,35 @@ static int add_virt_sub(struct mesh_net *net, struct mesh_model *mod,
 	if (!virt) {
 		virt = add_virtual(label);
 		if (!virt)
-			return MESH_STATUS_STORAGE_FAIL;
+			return MESH_STATUS_INSUFF_RESOURCES;
 
 		l_queue_push_head(mod->virtuals, virt);
 		mesh_net_dst_reg(net, virt->addr);
 		l_debug("Added virtual sub addr %4.4x", virt->addr);
 	}
 
-	if (dst)
-		*dst = virt->addr;
+	if (addr)
+		*addr = virt->addr;
 
 	return MESH_STATUS_SUCCESS;
 }
 
 static int add_sub(struct mesh_net *net, struct mesh_model *mod,
-			const uint8_t *group, bool b_virt, uint16_t *dst)
+							uint16_t addr)
 {
-	uint16_t grp;
-
-	if (b_virt)
-		return add_virt_sub(net, mod, group, dst);
-
-	grp = l_get_le16(group);
-	if (dst)
-		*dst = grp;
+	if (!mod->subs)
+		mod->subs = l_queue_new();
 
-	if (!l_queue_find(mod->subs, simple_match, L_UINT_TO_PTR(grp))) {
+	if (l_queue_find(mod->subs, simple_match, L_UINT_TO_PTR(addr)))
+		return MESH_STATUS_SUCCESS;
 
-		if (!mod->subs)
-			mod->subs = l_queue_new();
+	if ((l_queue_length(mod->subs) + l_queue_length(mod->virtuals)) >=
+						MAX_MODEL_SUBS)
+		return MESH_STATUS_INSUFF_RESOURCES;
 
-		l_queue_push_tail(mod->subs, L_UINT_TO_PTR(grp));
-		mesh_net_dst_reg(net, grp);
-		l_debug("Added group subscription %4.4x", grp);
-	}
+	l_queue_push_tail(mod->subs, L_UINT_TO_PTR(addr));
+	mesh_net_dst_reg(net, addr);
+	l_debug("Added group subscription %4.4x", addr);
 
 	return MESH_STATUS_SUCCESS;
 }
@@ -1454,8 +1449,8 @@ int mesh_model_sub_get(struct mesh_node *node, uint16_t addr, uint32_t id,
 	return MESH_STATUS_SUCCESS;
 }
 
-int mesh_model_sub_add(struct mesh_node *node, uint16_t addr, uint32_t id,
-			const uint8_t *group, bool is_virt, uint16_t *dst)
+int mesh_model_sub_add(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
+								uint16_t addr)
 {
 	struct mesh_model *mod;
 	int status, ele_idx = node_get_element_idx(node, addr);
@@ -1470,7 +1465,35 @@ int mesh_model_sub_add(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
 		return MESH_STATUS_NOT_SUB_MOD;
 
-	status = add_sub(node_get_net(node), mod, group, is_virt, dst);
+	status = add_sub(node_get_net(node), mod, addr);
+	if (status != MESH_STATUS_SUCCESS)
+		return status;
+
+	if (!mod->cbs)
+		/* External models */
+		cfg_update_model_subs(node, ele_idx, mod);
+
+	return MESH_STATUS_SUCCESS;
+}
+
+int mesh_model_virt_sub_add(struct mesh_node *node, uint16_t ele_addr,
+				uint32_t id, const uint8_t *label,
+				uint16_t *pub_addr)
+{
+	struct mesh_model *mod;
+	int status, ele_idx = node_get_element_idx(node, ele_addr);
+
+	if (ele_idx < 0)
+		return MESH_STATUS_INVALID_ADDRESS;
+
+	mod = get_model(node, (uint8_t) ele_idx, id);
+	if (!mod)
+		return MESH_STATUS_INVALID_MODEL;
+
+	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
+		return MESH_STATUS_NOT_SUB_MOD;
+
+	status = add_virt_sub(node_get_net(node), mod, label, pub_addr);
 
 	if (status != MESH_STATUS_SUCCESS)
 		return status;
@@ -1482,12 +1505,11 @@ int mesh_model_sub_add(struct mesh_node *node, uint16_t addr, uint32_t id,
 	return MESH_STATUS_SUCCESS;
 }
 
-int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
-			const uint8_t *group, bool is_virt, uint16_t *dst)
+int mesh_model_sub_ovrt(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
+								uint16_t addr)
 {
-	struct l_queue *virtuals, *subs;
 	struct mesh_model *mod;
-	int status, ele_idx = node_get_element_idx(node, addr);
+	int ele_idx = node_get_element_idx(node, addr);
 
 	if (ele_idx < 0)
 		return MESH_STATUS_INVALID_ADDRESS;
@@ -1499,36 +1521,39 @@ int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
 		return MESH_STATUS_NOT_SUB_MOD;
 
-	subs = mod->subs;
-	virtuals = mod->virtuals;
-	mod->subs = l_queue_new();
-	mod->virtuals = l_queue_new();
+	l_queue_clear(mod->subs, NULL);
+	l_queue_clear(mod->virtuals, unref_virt);
 
-	if (!mod->subs || !mod->virtuals)
-		return MESH_STATUS_INSUFF_RESOURCES;
+	add_sub(node_get_net(node), mod, addr);
 
-	status = add_sub(node_get_net(node), mod, group, is_virt, dst);
+	if (!mod->cbs)
+		/* External models */
+		cfg_update_model_subs(node, ele_idx, mod);
 
-	if (status != MESH_STATUS_SUCCESS) {
-		/* Adding new group failed, so revert to old lists */
-		l_queue_destroy(mod->subs, NULL);
-		mod->subs = subs;
-		l_queue_destroy(mod->virtuals, unref_virt);
-		mod->virtuals = virtuals;
-	} else {
-		const struct l_queue_entry *entry;
-		struct mesh_net *net = node_get_net(node);
+	return MESH_STATUS_SUCCESS;
+}
+
+int mesh_model_virt_sub_ovrt(struct mesh_node *node, uint16_t ele_addr,
+					uint32_t id, const uint8_t *label,
+					uint16_t *addr)
+{
+	struct mesh_model *mod;
+	int status, ele_idx = node_get_element_idx(node, ele_addr);
+
+	if (ele_idx < 0)
+		return MESH_STATUS_INVALID_ADDRESS;
+
+	mod = get_model(node, (uint8_t) ele_idx, id);
+	if (!mod)
+		return MESH_STATUS_INVALID_MODEL;
 
-		entry = l_queue_get_entries(subs);
+	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
+		return MESH_STATUS_NOT_SUB_MOD;
 
-		for (; entry; entry = entry->next)
-			mesh_net_dst_unreg(net,
-					(uint16_t) L_PTR_TO_UINT(entry->data));
+	l_queue_clear(mod->subs, NULL);
+	l_queue_clear(mod->virtuals, unref_virt);
 
-		/* Destroy old lists */
-		l_queue_destroy(subs, NULL);
-		l_queue_destroy(virtuals, unref_virt);
-	}
+	status = add_virt_sub(node_get_net(node), mod, label, addr);
 
 	if (!mod->cbs)
 		/* External models */
@@ -1537,10 +1562,9 @@ int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
 	return status;
 }
 
-int mesh_model_sub_del(struct mesh_node *node, uint16_t addr, uint32_t id,
-			const uint8_t *group, bool is_virt, uint16_t *dst)
+int mesh_model_sub_del(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
+								uint16_t addr)
 {
-	uint16_t grp;
 	struct mesh_model *mod;
 	int ele_idx = node_get_element_idx(node, addr);
 
@@ -1554,26 +1578,47 @@ int mesh_model_sub_del(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
 		return MESH_STATUS_NOT_SUB_MOD;
 
-	if (is_virt) {
-		struct mesh_virtual *virt;
+	if (l_queue_remove(mod->subs, L_UINT_TO_PTR(addr))) {
+		mesh_net_dst_unreg(node_get_net(node), addr);
 
-		virt = l_queue_find(mod->virtuals, find_virt_by_label, group);
-		if (virt) {
-			l_queue_remove(mod->virtuals, virt);
-			grp = virt->addr;
-			unref_virt(virt);
-		} else {
-			if (!mesh_crypto_virtual_addr(group, &grp))
-				return MESH_STATUS_STORAGE_FAIL;
-		}
-	} else {
-		grp = l_get_le16(group);
+		if (!mod->cbs)
+			/* External models */
+			cfg_update_model_subs(node, ele_idx, mod);
 	}
 
-	*dst = grp;
+	return MESH_STATUS_SUCCESS;
+}
 
-	if (l_queue_remove(mod->subs, L_UINT_TO_PTR(grp))) {
-		mesh_net_dst_unreg(node_get_net(node), grp);
+int mesh_model_virt_sub_del(struct mesh_node *node, uint16_t ele_addr,
+					uint32_t id, const uint8_t *label,
+					uint16_t *addr)
+{
+	struct mesh_model *mod;
+	struct mesh_virtual *virt;
+	int ele_idx = node_get_element_idx(node, ele_addr);
+
+	if (ele_idx < 0)
+		return MESH_STATUS_INVALID_ADDRESS;
+
+	mod = get_model(node, (uint8_t) ele_idx, id);
+	if (!mod)
+		return MESH_STATUS_INVALID_MODEL;
+
+	if (!mod->sub_enabled || (mod->cbs && !(mod->cbs->sub)))
+		return MESH_STATUS_NOT_SUB_MOD;
+
+	virt = l_queue_remove_if(mod->virtuals, find_virt_by_label, label);
+
+	if (virt) {
+		*addr = virt->addr;
+		unref_virt(virt);
+	} else {
+		*addr = UNASSIGNED_ADDRESS;
+		return MESH_STATUS_SUCCESS;
+	}
+
+	if (l_queue_remove(mod->subs, L_UINT_TO_PTR(*addr))) {
+		mesh_net_dst_unreg(node_get_net(node), *addr);
 
 		if (!mod->cbs)
 			/* External models */
@@ -1614,9 +1659,9 @@ static struct mesh_model *model_setup(struct mesh_net *net, uint8_t ele_idx,
 	struct mesh_config_pub *pub = db_mod->pub;
 	uint32_t i;
 
-	if (db_mod->num_bindings > MAX_BINDINGS) {
+	if (db_mod->num_bindings > MAX_MODEL_BINDINGS) {
 		l_warn("Binding list too long %u (max %u)",
-					db_mod->num_bindings, MAX_BINDINGS);
+				db_mod->num_bindings, MAX_MODEL_BINDINGS);
 		return NULL;
 	}
 
@@ -1670,22 +1715,12 @@ static struct mesh_model *model_setup(struct mesh_net *net, uint8_t ele_idx,
 		return mod;
 
 	for (i = 0; i < db_mod->num_subs; i++) {
-		uint16_t group;
-		uint8_t *src;
+		struct mesh_config_sub *sub = &db_mod->subs[i];
 
-		/*
-		 * To keep calculations for virtual label coherent,
-		 * convert to little endian.
-		 */
-		l_put_le16(db_mod->subs[i].src.addr, &group);
-		src = db_mod->subs[i].virt ? db_mod->subs[i].src.virt_addr :
-			(uint8_t *) &group;
-
-		if (add_sub(net, mod, src, db_mod->subs[i].virt, NULL) !=
-							MESH_STATUS_SUCCESS) {
-			mesh_model_free(mod);
-			return NULL;
-		}
+		if (!sub->virt)
+			add_sub(net, mod, sub->addr.grp);
+		else
+			add_virt_sub(net, mod, sub->addr.label, NULL);
 	}
 
 	return mod;
diff --git a/mesh/model.h b/mesh/model.h
index 0d8dddf92..3221379af 100644
--- a/mesh/model.h
+++ b/mesh/model.h
@@ -19,8 +19,8 @@
 
 struct mesh_model;
 
-#define MAX_BINDINGS	10
-#define MAX_GRP_PER_MOD	10
+#define MAX_MODEL_BINDINGS	10
+#define MAX_MODEL_SUBS		10
 
 #define ACTION_ADD	1
 #define ACTION_UPDATE	2
@@ -89,12 +89,21 @@ int mesh_model_get_bindings(struct mesh_node *node, uint16_t ele_addr,
 				uint32_t id, uint8_t *buf, uint16_t buf_sz,
 								uint16_t *len);
 int mesh_model_sub_add(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
-				const uint8_t *grp, bool b_virt, uint16_t *dst);
+								uint16_t grp);
+int mesh_model_virt_sub_add(struct mesh_node *node, uint16_t ele_addr,
+					uint32_t id, const uint8_t *label,
+					uint16_t *addr);
 int mesh_model_sub_del(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
-				const uint8_t *grp, bool b_virt, uint16_t *dst);
+								uint16_t grp);
+int mesh_model_virt_sub_del(struct mesh_node *node, uint16_t ele_addr,
+					uint32_t id, const uint8_t *label,
+					uint16_t *addr);
 int mesh_model_sub_del_all(struct mesh_node *node, uint16_t addr, uint32_t id);
-int mesh_model_sub_ovr(struct mesh_node *node, uint16_t addr, uint32_t id,
-				const uint8_t *grp, bool b_virt, uint16_t *dst);
+int mesh_model_sub_ovrt(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
+								uint16_t addr);
+int mesh_model_virt_sub_ovrt(struct mesh_node *node, uint16_t ele_addr,
+					uint32_t id, const uint8_t *label,
+					uint16_t *addr);
 int mesh_model_sub_get(struct mesh_node *node, uint16_t ele_addr, uint32_t id,
 			uint8_t *buf, uint16_t buf_size, uint16_t *size);
 uint16_t mesh_model_cfg_blk(uint8_t *pkt);
-- 
2.26.2


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

* [PATCH BlueZ 02/10] mesh: Clean up handling of config model binding messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 01/10] mesh: Clean up handling of config subscription messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 03/10] mesh: Clean up handling of config node identity message Inga Stotland
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 128 +++++++++++--------------------------------
 mesh/model.c         |  16 +-----
 2 files changed, 35 insertions(+), 109 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 73cf66ffd..6621d0935 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -112,8 +112,7 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 	uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
 	const uint8_t *pub_addr;
 	uint16_t test_addr;
-	uint8_t ttl, period;
-	uint8_t retransmit;
+	uint8_t ttl, period, retransmit;
 	int status;
 	bool cred_flag;
 
@@ -363,115 +362,52 @@ static uint16_t config_sub_del_all(struct mesh_node *node, const uint8_t *pkt,
 	return n;
 }
 
-static void send_model_app_status(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
-					uint8_t status, uint16_t addr,
-					uint32_t id, uint16_t idx)
-{
-	size_t n = mesh_model_opcode_set(OP_MODEL_APP_STATUS, msg);
-
-	msg[n++] = status;
-	l_put_le16(addr, msg + n);
-	n += 2;
-	l_put_le16(idx, msg + n);
-	n += 2;
-
-	if (IS_VENDOR(id)) {
-		l_put_le16(VENDOR_ID(id), msg + n);
-		n += 2;
-	}
-
-	l_put_le16(MODEL_ID(id), msg + n);
-	n += 2;
-
-	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
-								false, msg, n);
-}
-
-static void model_app_list(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
+static uint16_t model_app_list(struct mesh_node *node,
 					const uint8_t *pkt, uint16_t size)
 {
-	uint16_t ele_addr;
+	uint16_t ele_addr, n, bnd_len;
 	uint32_t id;
-	uint8_t *status;
-	uint16_t n;
-	int result;
+	int opcode;
 
+	opcode = (size == 4) ? OP_MODEL_APP_LIST : OP_VEND_MODEL_APP_LIST;
 	ele_addr = l_get_le16(pkt);
 
-	switch (size) {
-	default:
-		return;
-	case 4:
-		n = mesh_model_opcode_set(OP_MODEL_APP_LIST, msg);
-		status = msg + n;
-		id = l_get_le16(pkt + 2);
-		l_put_le16(ele_addr, msg + 1 + n);
-		l_put_le16((uint16_t) id, msg + 3 + n);
-		id = SET_ID(SIG_VENDOR, id);
-		n += 5;
-		break;
-	case 6:
-		n = mesh_model_opcode_set(OP_VEND_MODEL_APP_LIST, msg);
-		status = msg + n;
-		id = SET_ID(l_get_le16(pkt + 2), l_get_le16(pkt + 4));
+	n = mesh_model_opcode_set(opcode, msg);
+	memcpy(msg + n + 1, pkt, size);
 
-		l_put_le16(ele_addr, msg + 1 + n);
-		l_put_le16((uint16_t) VENDOR_ID(id), msg + 3 + n);
-		l_put_le16((uint16_t) MODEL_ID(id), msg + 5 + n);
-		n += 7;
-		break;
-	}
+	id = CFG_SET_ID(size == 6, pkt + 2);
 
-	result = mesh_model_get_bindings(node, ele_addr, id, msg + n,
-						MAX_MSG_LEN - n, &size);
-	n += size;
+	msg[n] = mesh_model_get_bindings(node, ele_addr, id, msg + n + 1 + size,
+					MAX_MSG_LEN - (n + 1 + size), &bnd_len);
 
-	if (result >= 0) {
-		*status = result;
-		mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx,
-						DEFAULT_TTL, false, msg, n);
-	}
+	if (msg[n] == MESH_STATUS_SUCCESS)
+		n += bnd_len;
+
+	n += (size + 1);
+	return n;
 }
 
-static bool model_app_bind(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
-					const uint8_t *pkt, uint16_t size,
-					bool unbind)
+static uint16_t model_app_bind(struct mesh_node *node, const uint8_t *pkt,
+						uint16_t size, bool unbind)
 {
-	uint16_t ele_addr;
+	uint16_t ele_addr, idx, n;
 	uint32_t id;
-	uint16_t idx;
-	int result;
-
-	switch (size) {
-	default:
-		return false;
-
-	case 6:
-		id = SET_ID(SIG_VENDOR, l_get_le16(pkt + 4));
-		break;
-	case 8:
-		id = SET_ID(l_get_le16(pkt + 4), l_get_le16(pkt + 6));
-		break;
-	}
 
 	ele_addr = l_get_le16(pkt);
 	idx = l_get_le16(pkt + 2);
+	id = CFG_SET_ID(size == 8, pkt + 4);
 
-	if (idx > 0xfff)
-		return false;
+	n = mesh_model_opcode_set(OP_MODEL_APP_STATUS, msg);
 
 	if (unbind)
-		result = mesh_model_binding_del(node, ele_addr, id, idx);
+		msg[n] = mesh_model_binding_del(node, ele_addr, id, idx);
 	else
-		result = mesh_model_binding_add(node, ele_addr, id, idx);
+		msg[n] = mesh_model_binding_add(node, ele_addr, id, idx);
 
-	send_model_app_status(node, net_idx, src, dst, result, ele_addr,
-								id, idx);
+	memcpy(msg + n + 1, pkt, size);
+	n += (size + 1);
 
-	return true;
+	return n;
 }
 
 static void hb_pub_timeout_func(struct l_timeout *timeout, void *user_data)
@@ -704,8 +640,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		if (size != 1 || pkt[0] > TTL_MASK || pkt[0] == 1)
 			return true;
 
-		if (pkt[0] <= TTL_MASK)
-			node_default_ttl_set(node, pkt[0]);
+		node_default_ttl_set(node, pkt[0]);
 		/* Fall Through */
 
 	case OP_CONFIG_DEFAULT_TTL_GET:
@@ -1049,22 +984,25 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 
 	case OP_MODEL_APP_BIND:
 	case OP_MODEL_APP_UNBIND:
-		model_app_bind(node, net_idx, src, dst, pkt, size,
-				opcode != OP_MODEL_APP_BIND);
+		if (size != 6 && size != 8)
+			return true;
+
+		n = model_app_bind(node, pkt, size,
+						opcode != OP_MODEL_APP_BIND);
 		break;
 
 	case OP_VEND_MODEL_APP_GET:
 		if (size != 6)
 			return true;
 
-		model_app_list(node, net_idx, src, dst, pkt, size);
+		n = model_app_list(node, pkt, size);
 		break;
 
 	case OP_MODEL_APP_GET:
 		if (size != 4)
 			return true;
 
-		model_app_list(node, net_idx, src, dst, pkt, size);
+		n = model_app_list(node, pkt, size);
 		break;
 
 	case OP_CONFIG_HEARTBEAT_PUB_SET:
diff --git a/mesh/model.c b/mesh/model.c
index 3c9b6577a..e2cadfe36 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -111,13 +111,7 @@ static bool simple_match(const void *a, const void *b)
 
 static bool has_binding(struct l_queue *bindings, uint16_t idx)
 {
-	const struct l_queue_entry *l;
-
-	for (l = l_queue_get_entries(bindings); l; l = l->next) {
-		if (L_PTR_TO_UINT(l->data) == idx)
-			return true;
-	}
-	return false;
+	return l_queue_find(bindings, simple_match, L_UINT_TO_PTR(idx)) != NULL;
 }
 
 static bool find_virt_by_label(const void *a, const void *b)
@@ -628,7 +622,6 @@ static int update_binding(struct mesh_node *node, uint16_t addr, uint32_t id,
 						uint16_t app_idx, bool unbind)
 {
 	struct mesh_model *mod;
-	bool is_present;
 	int ele_idx = node_get_element_idx(node, addr);
 
 	if (ele_idx < 0)
@@ -645,12 +638,7 @@ static int update_binding(struct mesh_node *node, uint16_t addr, uint32_t id,
 	if (!appkey_have_key(node_get_net(node), app_idx))
 		return MESH_STATUS_INVALID_APPKEY;
 
-	is_present = has_binding(mod->bindings, app_idx);
-
-	if (!is_present && unbind)
-		return MESH_STATUS_SUCCESS;
-
-	if (is_present && !unbind)
+	if (unbind ^ has_binding(mod->bindings, app_idx))
 		return MESH_STATUS_SUCCESS;
 
 	if (unbind) {
-- 
2.26.2


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

* [PATCH BlueZ 03/10] mesh: Clean up handling of config node identity message
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 01/10] mesh: Clean up handling of config subscription messages Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 02/10] mesh: Clean up handling of config model binding messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 04/10] mesh: Clean up handling of config publication messages Inga Stotland
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 6621d0935..1cd0bcdd5 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -774,11 +774,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		break;
 
 	case OP_NODE_IDENTITY_SET:
-		if (size != 3 || pkt[2] > 0x01)
-			return true;
-
-		n_idx = l_get_le16(pkt);
-		if (n_idx > 0xfff)
+		if (size != 3)
 			return true;
 
 		/* Currently setting node identity not supported */
@@ -786,18 +782,13 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		/* Fall Through */
 
 	case OP_NODE_IDENTITY_GET:
-		if (size < 2)
+		if (opcode == OP_NODE_IDENTITY_GET && size != 2)
 			return true;
 
 		n_idx = l_get_le16(pkt);
-		if (n_idx > 0xfff)
-			return true;
 
 		n = mesh_model_opcode_set(OP_NODE_IDENTITY_STATUS, msg);
-
-		status = mesh_net_get_identity_mode(net, n_idx, &state);
-
-		msg[n++] = status;
+		msg[n++] = mesh_net_get_identity_mode(net, n_idx, &state);
 
 		l_put_le16(n_idx, msg + n);
 		n += 2;
-- 
2.26.2


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

* [PATCH BlueZ 04/10] mesh: Clean up handling of config publication messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (2 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 03/10] mesh: Clean up handling of config node identity message Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 05/10] mesh: Clean up handling of config net and app key messages Inga Stotland
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 70 ++++++++++++++++----------------------------
 1 file changed, 25 insertions(+), 45 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 1cd0bcdd5..871ecf878 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -44,11 +44,9 @@ static const uint8_t supported_pages[] = {
 
 static uint8_t msg[MAX_MSG_LEN];
 
-static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
-			uint16_t src, uint16_t dst,
-			uint8_t status, uint16_t ele_addr, uint32_t id,
-			uint16_t pub_addr, uint16_t idx, bool cred_flag,
-			uint8_t ttl, uint8_t period, uint8_t retransmit)
+static uint16_t set_pub_status(uint8_t status, uint16_t ele_addr, uint32_t id,
+				uint16_t pub_addr, uint16_t idx, bool cred_flag,
+				uint8_t ttl, uint8_t period, uint8_t retransmit)
 {
 	size_t n;
 
@@ -72,41 +70,32 @@ static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
 		n += 4;
 	}
 
-	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
-								false, msg, n);
+	return n;
 }
 
-static void config_pub_get(struct mesh_node *node, uint16_t net_idx,
-					uint16_t src, uint16_t dst,
-					const uint8_t *pkt, uint16_t size)
+static uint16_t config_pub_get(struct mesh_node *node, const uint8_t *pkt,
+								bool vendor)
 {
 	uint32_t id;
 	uint16_t ele_addr;
 	struct mesh_model_pub *pub;
 	int status;
 
-	if (size == 4) {
-		id = SET_ID(SIG_VENDOR, l_get_le16(pkt + 2));
-	} else if (size == 6) {
-		id = SET_ID(l_get_le16(pkt + 2), l_get_le16(pkt + 4));
-	} else
-		return;
-
 	ele_addr = l_get_le16(pkt);
+	id = CFG_SET_ID(vendor, pkt + 2);
+
 	pub = mesh_model_pub_get(node, ele_addr, id, &status);
 
 	if (pub && status == MESH_STATUS_SUCCESS)
-		send_pub_status(node, net_idx, src, dst, status, ele_addr,
-				id, pub->addr, pub->idx, pub->credential,
-				pub->ttl, pub->period, pub->retransmit);
+		return set_pub_status(status, ele_addr, id, pub->addr, pub->idx,
+					pub->credential, pub->ttl, pub->period,
+					pub->retransmit);
 	else
-		send_pub_status(node, net_idx, src, dst, status, ele_addr,
-				id, 0, 0, 0, 0, 0, 0);
+		return set_pub_status(status, ele_addr, id, 0, 0, 0, 0, 0, 0);
 }
 
-static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
-				uint16_t src, uint16_t dst,
-				const uint8_t *pkt, bool virt, bool vendor)
+static uint16_t config_pub_set(struct mesh_node *node, const uint8_t *pkt,
+							bool virt, bool vendor)
 {
 	uint32_t id;
 	uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
@@ -125,17 +114,12 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 	ttl = pkt[6];
 	period = pkt[7];
 	retransmit = pkt[8];
-	id = l_get_le16(pkt + 9);
-
-	if (!vendor)
-		id = SET_ID(SIG_VENDOR, id);
-	else
-		id = SET_ID(id, l_get_le16(pkt + 11));
+	id = CFG_SET_ID(vendor, pkt + 9);
 
 	/* Don't accept virtual seeming addresses */
 	test_addr = l_get_le16(pub_addr);
 	if (!virt && IS_VIRTUAL(test_addr))
-		return;
+		return 0;
 
 	cred_flag = !!(CREDFLAG_MASK & idx);
 	idx &= APP_IDX_MASK;
@@ -144,15 +128,11 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 					cred_flag, ttl, period, retransmit,
 					virt, &ota);
 
-	l_debug("pub_set: status %d, ea %4.4x, ota: %4.4x, mod: %x, idx: %3.3x",
+	l_debug("pub_set: status %d, ea %4.4x, ota: %4.4x, id: %x, idx: %3.3x",
 					status, ele_addr, ota, id, idx);
 
-	if (status != MESH_STATUS_SUCCESS) {
-		send_pub_status(node, net_idx, src, dst, status, ele_addr,
-						id, 0, 0, 0, 0, 0, 0);
-
-		return;
-	}
+	if (status != MESH_STATUS_SUCCESS)
+		return set_pub_status(status, ele_addr, id, 0, 0, 0, 0, 0, 0);
 
 	if (IS_UNASSIGNED(test_addr) && !virt) {
 		ttl = period = idx = 0;
@@ -180,12 +160,12 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 		/* Save model publication to config file */
 		if (!mesh_config_model_pub_add(node_config_get(node), ele_addr,
 						vendor ? id : MODEL_ID(id),
-							vendor, &db_pub))
+						vendor, &db_pub))
 			status = MESH_STATUS_STORAGE_FAIL;
 	}
 
-	send_pub_status(node, net_idx, src, dst, status, ele_addr, id, ota,
-				idx, cred_flag, ttl, period, retransmit);
+	return set_pub_status(status, ele_addr, id, ota, idx, cred_flag, ttl,
+							period, retransmit);
 }
 
 static uint16_t cfg_sub_get_msg(struct mesh_node *node, uint16_t net_idx,
@@ -664,14 +644,14 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		if (!virt && (size != 11 && size != 13))
 			return true;
 
-		config_pub_set(node, net_idx, src, dst, pkt, virt,
-						size == 13 || size == 27);
+		n = config_pub_set(node, pkt, virt, size == 13 || size == 27);
 		break;
 
 	case OP_CONFIG_MODEL_PUB_GET:
 		if (size != 4 && size != 6)
 			return true;
-		config_pub_get(node, net_idx, src, dst, pkt, size);
+
+		n = config_pub_get(node, pkt, size == 6);
 		break;
 
 	case OP_CONFIG_VEND_MODEL_SUB_GET:
-- 
2.26.2


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

* [PATCH BlueZ 05/10] mesh: Clean up handling of config net and app key messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (3 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 04/10] mesh: Clean up handling of config publication messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 06/10] mesh: Clean up handling of config relay messages Inga Stotland
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 147 ++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 72 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 871ecf878..67e320eda 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -541,6 +541,73 @@ static void node_reset(void *user_data)
 	node_remove(node);
 }
 
+static uint16_t cfg_appkey_msg(struct mesh_node *node, const uint8_t *pkt,
+								int opcode)
+{
+	uint16_t n_idx, a_idx, n;
+	struct mesh_net *net = node_get_net(node);
+
+	n_idx = l_get_le16(pkt) & 0xfff;
+	a_idx = l_get_le16(pkt + 1) >> 4;
+
+	n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);
+
+	if (opcode == OP_APPKEY_ADD)
+		msg[n] = appkey_key_add(net, n_idx, a_idx, pkt + 3);
+	else if (opcode == OP_APPKEY_UPDATE)
+		msg[n] = appkey_key_update(net, n_idx, a_idx, pkt + 3);
+	else
+		msg[n] = appkey_key_delete(net, n_idx, a_idx);
+
+	l_debug("AppKey Command %s: Net_Idx %3.3x, App_Idx %3.3x",
+			(msg[n] == MESH_STATUS_SUCCESS) ? "success" : "fail",
+								n_idx, a_idx);
+
+	memcpy(msg + n + 1, &pkt[0], 3);
+
+	return n + 4;
+}
+
+static uint16_t cfg_netkey_msg(struct mesh_node *node, const uint8_t *pkt,
+								int opcode)
+{
+	uint16_t n_idx, n;
+	struct mesh_net *net = node_get_net(node);
+
+	n_idx = l_get_le16(pkt);
+	n = mesh_model_opcode_set(OP_NETKEY_STATUS, msg);
+
+	if (opcode == OP_NETKEY_ADD)
+		msg[n] = mesh_net_add_key(net, n_idx, pkt + 2);
+	else if (opcode == OP_NETKEY_UPDATE)
+		msg[n] = mesh_net_update_key(net, n_idx, pkt + 2);
+	else
+		msg[n] = mesh_net_del_key(net, n_idx);
+
+	l_debug("NetKey Command %s: Net_Idx %3.3x",
+			(msg[n] == MESH_STATUS_SUCCESS) ? "success" : "fail",
+									n_idx);
+
+	memcpy(msg + n + 1, &pkt[0], 2);
+
+	return n + 3;
+}
+
+static uint16_t cfg_get_appkeys_msg(struct mesh_node *node, const uint8_t *pkt)
+{
+	uint16_t n_idx, sz, n;
+
+	n_idx = l_get_le16(pkt);
+
+	n = mesh_model_opcode_set(OP_APPKEY_LIST, msg);
+	l_put_le16(n_idx, msg + n + 1);
+
+	msg[n] = appkey_list(node_get_net(node), n_idx, msg + n + 3,
+						MAX_MSG_LEN - (n + 3), &sz);
+
+	return n + 3 + sz;
+}
+
 static uint16_t get_composition(struct mesh_node *node, uint8_t page,
 								uint8_t *buf)
 {
@@ -579,7 +646,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	uint32_t opcode, tmp32;
 	int b_res = MESH_STATUS_SUCCESS;
 	struct mesh_net_heartbeat *hb;
-	uint16_t n_idx, a_idx;
+	uint16_t n_idx;
 	uint8_t state, status;
 	uint8_t phase;
 	bool virt = false;
@@ -850,60 +917,19 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		if (size != 19)
 			return true;
 
-		n_idx = l_get_le16(pkt) & 0xfff;
-		a_idx = l_get_le16(pkt + 1) >> 4;
-
-		if (opcode == OP_APPKEY_ADD)
-			b_res = appkey_key_add(net, n_idx, a_idx, pkt + 3);
-		else
-			b_res = appkey_key_update(net, n_idx, a_idx,
-								pkt + 3);
-
-		l_debug("Add/Update AppKey %s: Net_Idx %3.3x, App_Idx %3.3x",
-			(b_res == MESH_STATUS_SUCCESS) ? "success" : "fail",
-							n_idx, a_idx);
-
-
-		n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);
-
-		msg[n++] = b_res;
-		msg[n++] = pkt[0];
-		msg[n++] = pkt[1];
-		msg[n++] = pkt[2];
-		break;
-
+		/* Fall Through */
 	case OP_APPKEY_DELETE:
-		if (size != 3)
+		if (opcode == OP_APPKEY_DELETE && size != 3)
 			return true;
 
-		n_idx = l_get_le16(pkt) & 0xfff;
-		a_idx = l_get_le16(pkt + 1) >> 4;
-		b_res = appkey_key_delete(net, n_idx, a_idx);
-		l_debug("Delete AppKey %s Net_Idx %3.3x to App_Idx %3.3x",
-			(b_res == MESH_STATUS_SUCCESS) ? "success" : "fail",
-							n_idx, a_idx);
-
-		n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);
-		msg[n++] = b_res;
-		msg[n++] = pkt[0];
-		msg[n++] = pkt[1];
-		msg[n++] = pkt[2];
+		n = cfg_appkey_msg(node, pkt, opcode);
 		break;
 
 	case OP_APPKEY_GET:
 		if (size != 2)
 			return true;
 
-		n_idx = l_get_le16(pkt);
-
-		n = mesh_model_opcode_set(OP_APPKEY_LIST, msg);
-
-		status = appkey_list(net, n_idx, msg + n + 3,
-						MAX_MSG_LEN - n - 3, &size);
-
-		msg[n] = status;
-		l_put_le16(n_idx, msg + n + 1);
-		n += (size + 3);
+		n = cfg_get_appkeys_msg(node, pkt);
 		break;
 
 	case OP_NETKEY_ADD:
@@ -911,35 +937,12 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		if (size != 18)
 			return true;
 
-		n_idx = l_get_le16(pkt);
-
-		if (opcode == OP_NETKEY_ADD)
-			b_res = mesh_net_add_key(net, n_idx, pkt + 2);
-		else
-			b_res = mesh_net_update_key(net, n_idx, pkt + 2);
-
-		l_debug("NetKey Add/Update %s",
-			(b_res == MESH_STATUS_SUCCESS) ? "success" : "fail");
-
-		n = mesh_model_opcode_set(OP_NETKEY_STATUS, msg);
-		msg[n++] = b_res;
-		l_put_le16(l_get_le16(pkt), msg + n);
-		n += 2;
-		break;
-
+		/* Fall Through */
 	case OP_NETKEY_DELETE:
-		if (size != 2)
+		if (opcode == OP_NETKEY_DELETE && size != 2)
 			return true;
 
-		b_res = mesh_net_del_key(net, l_get_le16(pkt));
-
-		l_debug("NetKey delete %s",
-			(b_res == MESH_STATUS_SUCCESS) ? "success" : "fail");
-
-		n = mesh_model_opcode_set(OP_NETKEY_STATUS, msg);
-		msg[n++] = b_res;
-		l_put_le16(l_get_le16(pkt), msg + n);
-		n += 2;
+		n = cfg_netkey_msg(node, pkt, opcode);
 		break;
 
 	case OP_NETKEY_GET:
-- 
2.26.2


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

* [PATCH BlueZ 06/10] mesh: Clean up handling of config relay messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (4 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 05/10] mesh: Clean up handling of config net and app key messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 07/10] mesh: Clean up handling of config poll timeout message Inga Stotland
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 67e320eda..f1237b0fe 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -390,6 +390,30 @@ static uint16_t model_app_bind(struct mesh_node *node, const uint8_t *pkt,
 	return n;
 }
 
+static uint16_t cfg_relay_msg(struct mesh_node *node, const uint8_t *pkt,
+								int opcode)
+{
+	uint8_t count;
+	uint16_t interval;
+	uint16_t n;
+
+	if (pkt[0] > 0x01)
+		return 0;
+
+	if (opcode == OP_CONFIG_RELAY_SET) {
+		count = (pkt[1] & 0x7) + 1;
+		interval = ((pkt[1] >> 3) + 1) * 10;
+		node_relay_mode_set(node, !!pkt[0], count, interval);
+	}
+
+	n = mesh_model_opcode_set(OP_CONFIG_RELAY_STATUS, msg);
+
+	msg[n++] = node_relay_mode_get(node, &count, &interval);
+	msg[n++] = (count - 1) + ((interval/10 - 1) << 3);
+
+	return n;
+}
+
 static void hb_pub_timeout_func(struct l_timeout *timeout, void *user_data)
 {
 	struct mesh_net *net = user_data;
@@ -759,24 +783,14 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		break;
 
 	case OP_CONFIG_RELAY_SET:
-		if (size != 2 || pkt[0] > 0x01)
+		if (size != 2)
 			return true;
-
-		count = (pkt[1] & 0x7) + 1;
-		interval = ((pkt[1] >> 3) + 1) * 10;
-		node_relay_mode_set(node, !!pkt[0], count, interval);
 		/* Fall Through */
-
 	case OP_CONFIG_RELAY_GET:
 		if (opcode == OP_CONFIG_RELAY_GET && size != 0)
 			return true;
 
-		n = mesh_model_opcode_set(OP_CONFIG_RELAY_STATUS, msg);
-
-		msg[n++] = node_relay_mode_get(node, &count, &interval);
-		msg[n++] = (count - 1) + ((interval/10 - 1) << 3);
-
-		l_debug("Get/Set Relay Config (%d)", msg[n-1]);
+		n = cfg_relay_msg(node, pkt, opcode);
 		break;
 
 	case OP_CONFIG_NETWORK_TRANSMIT_SET:
-- 
2.26.2


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

* [PATCH BlueZ 07/10] mesh: Clean up handling of config poll timeout message
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (5 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 06/10] mesh: Clean up handling of config relay messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 08/10] mesh: Clean up handling of config net transmit messages Inga Stotland
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 34 +++++++++++++++++++++++-----------
 mesh/cfgmod.h        |  2 +-
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index f1237b0fe..bc564f0ef 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -632,6 +632,25 @@ static uint16_t cfg_get_appkeys_msg(struct mesh_node *node, const uint8_t *pkt)
 	return n + 3 + sz;
 }
 
+static uint16_t cfg_poll_timeout_msg(struct mesh_node *node, const uint8_t *pkt)
+{
+	uint16_t n, addr = l_get_le16(pkt);
+	uint32_t poll_to;
+
+	if (!IS_UNICAST(addr))
+		return 0;
+
+	n = mesh_model_opcode_set(OP_CONFIG_POLL_TIMEOUT_STATUS, msg);
+	l_put_le16(addr, msg + n);
+	n += 2;
+
+	poll_to = mesh_net_friend_timeout(node_get_net(node), addr);
+	msg[n++] = poll_to;
+	msg[n++] = poll_to >> 8;
+	msg[n++] = poll_to >> 16;
+	return n;
+}
+
 static uint16_t get_composition(struct mesh_node *node, uint8_t page,
 								uint8_t *buf)
 {
@@ -667,7 +686,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	struct mesh_net *net;
 	const uint8_t *pkt = data;
 	struct timeval time_now;
-	uint32_t opcode, tmp32;
+	uint32_t opcode;
 	int b_res = MESH_STATUS_SUCCESS;
 	struct mesh_net_heartbeat *hb;
 	uint16_t n_idx;
@@ -1103,18 +1122,11 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		msg[n++] = hb->sub_max_hops;
 		break;
 
-	case OP_CONFIG_POLL_TIMEOUT_LIST:
-		if (size != 2 || l_get_le16(pkt) == 0 ||
-						l_get_le16(pkt) > 0x7fff)
+	case OP_CONFIG_POLL_TIMEOUT_GET:
+		if (size != 2)
 			return true;
 
-		n = mesh_model_opcode_set(OP_CONFIG_POLL_TIMEOUT_STATUS, msg);
-		l_put_le16(l_get_le16(pkt), msg + n);
-		n += 2;
-		tmp32 = mesh_net_friend_timeout(net, l_get_le16(pkt));
-		msg[n++] = tmp32;
-		msg[n++] = tmp32 >> 8;
-		msg[n++] = tmp32 >> 16;
+		n = cfg_poll_timeout_msg(node, pkt);
 		break;
 
 	case OP_NODE_RESET:
diff --git a/mesh/cfgmod.h b/mesh/cfgmod.h
index 7b6a95807..6d73656a7 100644
--- a/mesh/cfgmod.h
+++ b/mesh/cfgmod.h
@@ -66,7 +66,7 @@
 #define OP_CONFIG_MODEL_SUB_LIST		0x802A
 #define OP_CONFIG_VEND_MODEL_SUB_GET		0x802B
 #define OP_CONFIG_VEND_MODEL_SUB_LIST		0x802C
-#define OP_CONFIG_POLL_TIMEOUT_LIST		0x802D
+#define OP_CONFIG_POLL_TIMEOUT_GET		0x802D
 #define OP_CONFIG_POLL_TIMEOUT_STATUS		0x802E
 /* Health opcodes in health-mod.h */
 #define OP_CONFIG_HEARTBEAT_PUB_GET		0x8038
-- 
2.26.2


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

* [PATCH BlueZ 08/10] mesh: Clean up handling of config net transmit messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (6 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 07/10] mesh: Clean up handling of config poll timeout message Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 09/10] mesh: Clean up handling of config KR phase messages Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 10/10] mesh: Refactor heartbeat pub/sub Inga Stotland
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index bc564f0ef..a7369e435 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -651,6 +651,28 @@ static uint16_t cfg_poll_timeout_msg(struct mesh_node *node, const uint8_t *pkt)
 	return n;
 }
 
+static uint16_t cfg_net_tx_msg(struct mesh_node *node, const uint8_t *pkt,
+								int opcode)
+{
+	uint8_t cnt;
+	uint16_t interval, n;
+	struct mesh_net *net = node_get_net(node);
+
+	cnt = (pkt[0] & 0x7) + 1;
+	interval = ((pkt[0] >> 3) + 1) * 10;
+
+	if (opcode == OP_CONFIG_NETWORK_TRANSMIT_SET &&
+			mesh_config_write_net_transmit(node_config_get(node),
+							cnt, interval))
+		mesh_net_transmit_params_set(net, cnt, interval);
+
+	n = mesh_model_opcode_set(OP_CONFIG_NETWORK_TRANSMIT_STATUS, msg);
+
+	mesh_net_transmit_params_get(net, &cnt, &interval);
+	msg[n++] = (cnt - 1) + ((interval/10 - 1) << 3);
+	return n;
+}
+
 static uint16_t get_composition(struct mesh_node *node, uint8_t page,
 								uint8_t *buf)
 {
@@ -693,8 +715,6 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	uint8_t state, status;
 	uint8_t phase;
 	bool virt = false;
-	uint8_t count;
-	uint16_t interval;
 	uint16_t n;
 
 	if (app_idx != APP_IDX_DEV_LOCAL)
@@ -815,25 +835,13 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	case OP_CONFIG_NETWORK_TRANSMIT_SET:
 		if (size != 1)
 			return true;
-
-		count = (pkt[0] & 0x7) + 1;
-		interval = ((pkt[0] >> 3) + 1) * 10;
-
-		if (mesh_config_write_net_transmit(node_config_get(node), count,
-								interval))
-			mesh_net_transmit_params_set(net, count, interval);
 		/* Fall Through */
 
 	case OP_CONFIG_NETWORK_TRANSMIT_GET:
 		if (opcode == OP_CONFIG_NETWORK_TRANSMIT_GET && size != 0)
 			return true;
 
-		n = mesh_model_opcode_set(OP_CONFIG_NETWORK_TRANSMIT_STATUS,
-									msg);
-		mesh_net_transmit_params_get(net, &count, &interval);
-		msg[n++] = (count - 1) + ((interval/10 - 1) << 3);
-
-		l_debug("Get/Set Network Transmit Config");
+		n = cfg_net_tx_msg(node, pkt, opcode);
 		break;
 
 	case OP_CONFIG_PROXY_SET:
-- 
2.26.2


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

* [PATCH BlueZ 09/10] mesh: Clean up handling of config KR phase messages
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (7 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 08/10] mesh: Clean up handling of config net transmit messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  2020-07-30 20:38 ` [PATCH BlueZ 10/10] mesh: Refactor heartbeat pub/sub Inga Stotland
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

This modification allows using a single point for sending out
the composed status messages by the Config Server.
---
 mesh/cfgmod-server.c | 55 +++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index a7369e435..cacc8b6ea 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -414,6 +414,31 @@ static uint16_t cfg_relay_msg(struct mesh_node *node, const uint8_t *pkt,
 	return n;
 }
 
+static uint16_t cfg_key_refresh_phase(struct mesh_node *node,
+						const uint8_t *pkt, int opcode)
+{
+	struct mesh_net *net = node_get_net(node);
+	uint16_t n, idx = l_get_le16(pkt);
+	uint8_t phase;
+
+	n = mesh_model_opcode_set(OP_CONFIG_KEY_REFRESH_PHASE_STATUS, msg);
+
+	if (opcode == OP_CONFIG_KEY_REFRESH_PHASE_SET) {
+		phase = pkt[2];
+		msg[n] = mesh_net_key_refresh_phase_set(net, idx, phase);
+		l_debug("Set KR Phase: net=%3.3x transition=%d", idx, phase);
+	} else {
+		msg[n] = mesh_net_key_refresh_phase_get(net, idx, &phase);
+		l_debug("Get KR Phase: net=%3.3x phase=%d", idx, phase);
+	}
+
+	l_put_le16(idx, msg + n);
+	msg[n + 2] = (msg[n] != MESH_STATUS_SUCCESS) ?
+						KEY_REFRESH_PHASE_NONE : phase;
+
+	return n + 3;
+}
+
 static void hb_pub_timeout_func(struct l_timeout *timeout, void *user_data)
 {
 	struct mesh_net *net = user_data;
@@ -712,8 +737,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	int b_res = MESH_STATUS_SUCCESS;
 	struct mesh_net_heartbeat *hb;
 	uint16_t n_idx;
-	uint8_t state, status;
-	uint8_t phase;
+	uint8_t state;
 	bool virt = false;
 	uint16_t n;
 
@@ -920,37 +944,16 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		break;
 
 	case OP_CONFIG_KEY_REFRESH_PHASE_SET:
-		if (size != 3 || pkt[2] > 0x03)
+		if (size != 3 || pkt[2] > KEY_REFRESH_PHASE_THREE)
 			return true;
 
-		b_res = mesh_net_key_refresh_phase_set(net, l_get_le16(pkt),
-							pkt[2]);
-		size = 2;
 		/* Fall Through */
 
 	case OP_CONFIG_KEY_REFRESH_PHASE_GET:
-		if (size != 2)
+		if (size != 2 && opcode == OP_CONFIG_KEY_REFRESH_PHASE_GET)
 			return true;
 
-		n_idx = l_get_le16(pkt);
-
-		n = mesh_model_opcode_set(OP_CONFIG_KEY_REFRESH_PHASE_STATUS,
-						msg);
-
-		/* State: 0x00-0x03 phase of key refresh */
-		status = mesh_net_key_refresh_phase_get(net, n_idx,
-							&phase);
-		if (status != MESH_STATUS_SUCCESS) {
-			b_res = status;
-			phase = KEY_REFRESH_PHASE_NONE;
-		}
-
-		msg[n++] = b_res;
-		l_put_le16(n_idx, msg + n);
-		n += 2;
-		msg[n++] = phase;
-
-		l_debug("Get/Set Key Refresh State (%d)", msg[n-1]);
+		n = cfg_key_refresh_phase(node, pkt, opcode);
 		break;
 
 	case OP_APPKEY_ADD:
-- 
2.26.2


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

* [PATCH BlueZ 10/10] mesh: Refactor heartbeat pub/sub
  2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
                   ` (8 preceding siblings ...)
  2020-07-30 20:38 ` [PATCH BlueZ 09/10] mesh: Clean up handling of config KR phase messages Inga Stotland
@ 2020-07-30 20:38 ` Inga Stotland
  9 siblings, 0 replies; 11+ messages in thread
From: Inga Stotland @ 2020-07-30 20:38 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

Move heartbeat publication/subscription timers and housekeeping
to net.c since this is where the trigger events and control messages
are handled. Configuration server (cfgmod-server.c) stays
responsible for parsing the set pub/sub message parameters and
assemblying the pub/sub status messages.

Also, make sure that the correct message status is reported.
---
 mesh/cfgmod-server.c | 307 ++++++++++++++-----------------------------
 mesh/net.c           | 273 ++++++++++++++++++++++++++++----------
 mesh/net.h           |  48 ++++---
 3 files changed, 328 insertions(+), 300 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index cacc8b6ea..a447a3524 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -439,50 +439,6 @@ static uint16_t cfg_key_refresh_phase(struct mesh_node *node,
 	return n + 3;
 }
 
-static void hb_pub_timeout_func(struct l_timeout *timeout, void *user_data)
-{
-	struct mesh_net *net = user_data;
-	struct mesh_net_heartbeat *hb = mesh_net_heartbeat_get(net);
-
-	mesh_net_heartbeat_send(net);
-
-	if (hb->pub_count != 0xffff)
-		hb->pub_count--;
-	if (hb->pub_count > 0)
-		l_timeout_modify(hb->pub_timer, hb->pub_period);
-	else {
-		l_timeout_remove(hb->pub_timer);
-		hb->pub_timer = NULL;
-	}
-}
-
-static void update_hb_pub_timer(struct mesh_net *net,
-						struct mesh_net_heartbeat *hb)
-{
-	if (IS_UNASSIGNED(hb->pub_dst) || hb->pub_count == 0) {
-		l_timeout_remove(hb->pub_timer);
-		hb->pub_timer = NULL;
-		return;
-	}
-
-	if (!hb->pub_timer)
-		hb->pub_timer = l_timeout_create(hb->pub_period,
-					hb_pub_timeout_func, net, NULL);
-	else
-		l_timeout_modify(hb->pub_timer, hb->pub_period);
-}
-
-static void hb_sub_timeout_func(struct l_timeout *timeout, void *user_data)
-{
-	struct mesh_net *net = user_data;
-	struct mesh_net_heartbeat *hb = mesh_net_heartbeat_get(net);
-
-	l_debug("HB Subscription Ended");
-	l_timeout_remove(hb->sub_timer);
-	hb->sub_timer = NULL;
-	hb->sub_enabled = false;
-}
-
 static uint8_t uint32_to_log(uint32_t value)
 {
 	uint32_t val = 1;
@@ -501,85 +457,112 @@ static uint8_t uint32_to_log(uint32_t value)
 	return ret;
 }
 
-static uint32_t log_to_uint32(uint8_t log, uint8_t offset)
+static uint16_t hb_subscription_get(struct mesh_node *node, int status)
 {
-	if (!log)
-		return 0x0000;
-	else if (log > 0x11)
-		return 0xffff;
+	struct mesh_net *net = node_get_net(node);
+	struct mesh_net_heartbeat_sub *sub = mesh_net_get_heartbeat_sub(net);
+	struct timeval time_now;
+	uint16_t n;
+
+	gettimeofday(&time_now, NULL);
+	time_now.tv_sec -= sub->start;
+
+	if (time_now.tv_sec >= (long int) sub->period)
+		time_now.tv_sec = 0;
 	else
-		return (1 << (log - offset));
-}
+		time_now.tv_sec = sub->period - time_now.tv_sec;
 
+	l_debug("Sub Period (Log %2.2x) %d sec", uint32_to_log(time_now.tv_sec),
+							(int) time_now.tv_sec);
 
-static int hb_subscription_set(struct mesh_net *net, uint16_t src,
-					uint16_t dst, uint8_t period_log)
+	n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_SUB_STATUS, msg);
+	msg[n++] = status;
+	l_put_le16(sub->src, msg + n);
+	n += 2;
+	l_put_le16(sub->dst, msg + n);
+	n += 2;
+	msg[n++] = uint32_to_log(time_now.tv_sec);
+	msg[n++] = uint32_to_log(sub->count);
+	msg[n++] = sub->count ? sub->min_hops : 0;
+	msg[n++] = sub->max_hops;
+
+	return n;
+}
+
+static uint16_t hb_subscription_set(struct mesh_node *node, const uint8_t *pkt)
 {
-	struct mesh_net_heartbeat *hb = mesh_net_heartbeat_get(net);
-	struct timeval time_now;
+	uint16_t src, dst;
+	uint8_t period_log;
+	struct mesh_net *net;
+	int status;
+
+	src = l_get_le16(pkt);
+	dst = l_get_le16(pkt + 2);
 
 	/* SRC must be Unicast, DST can be any legal address except Virtual */
 	if ((!IS_UNASSIGNED(src) && !IS_UNICAST(src)) || IS_VIRTUAL(dst))
-		return -1;
-
-	/* Check if the subscription should be disabled */
-	if (IS_UNASSIGNED(src) || IS_UNASSIGNED(dst)) {
-		if (IS_GROUP(hb->sub_dst))
-			mesh_net_dst_unreg(net, hb->sub_dst);
-
-		l_timeout_remove(hb->sub_timer);
-		hb->sub_timer = NULL;
-		hb->sub_enabled = false;
-		hb->sub_dst = UNASSIGNED_ADDRESS;
-		hb->sub_src = UNASSIGNED_ADDRESS;
-		hb->sub_count = 0;
-		hb->sub_period = 0;
-		hb->sub_min_hops = 0;
-		hb->sub_max_hops = 0;
-		return MESH_STATUS_SUCCESS;
-
-	} else if (!period_log && src == hb->sub_src && dst == hb->sub_dst) {
-		/* Preserve collected data, but disable */
-		l_timeout_remove(hb->sub_timer);
-		hb->sub_timer = NULL;
-		hb->sub_enabled = false;
-		hb->sub_period = 0;
-		return MESH_STATUS_SUCCESS;
-	}
+		return 0;
 
-	if (hb->sub_dst != dst) {
-		if (IS_GROUP(hb->sub_dst))
-			mesh_net_dst_unreg(net, hb->sub_dst);
-		if (IS_GROUP(dst))
-			mesh_net_dst_reg(net, dst);
-	}
+	period_log = pkt[4];
 
-	hb->sub_enabled = !!period_log;
-	hb->sub_src = src;
-	hb->sub_dst = dst;
-	hb->sub_count = 0;
-	hb->sub_period = log_to_uint32(period_log, 1);
-	hb->sub_min_hops = 0x00;
-	hb->sub_max_hops = 0x00;
+	if (period_log > 0x11)
+		return 0;
 
-	gettimeofday(&time_now, NULL);
-	hb->sub_start = time_now.tv_sec;
+	net = node_get_net(node);
 
-	if (!hb->sub_enabled) {
-		l_timeout_remove(hb->sub_timer);
-		hb->sub_timer = NULL;
-		return MESH_STATUS_SUCCESS;
-	}
+	status = mesh_net_set_heartbeat_sub(net, src, dst, period_log);
 
-	hb->sub_min_hops = 0xff;
+	return hb_subscription_get(node, status);
+}
 
-	if (!hb->sub_timer)
-		hb->sub_timer = l_timeout_create(hb->sub_period,
-						hb_sub_timeout_func, net, NULL);
-	else
-		l_timeout_modify(hb->sub_timer, hb->sub_period);
+static uint16_t hb_publication_get(struct mesh_node *node, int status)
+{
+	struct mesh_net *net = node_get_net(node);
+	struct mesh_net_heartbeat_pub *pub = mesh_net_get_heartbeat_pub(net);
+	uint16_t n;
 
-	return MESH_STATUS_SUCCESS;
+	n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_PUB_STATUS, msg);
+	msg[n++] = status;
+	l_put_le16(pub->dst, msg + n);
+	n += 2;
+	msg[n++] = uint32_to_log(pub->count);
+	msg[n++] = uint32_to_log(pub->period);
+	msg[n++] = pub->ttl;
+	l_put_le16(pub->features, msg + n);
+	n += 2;
+	l_put_le16(pub->net_idx, msg + n);
+	n += 2;
+
+	return n;
+}
+
+static uint16_t hb_publication_set(struct mesh_node *node, const uint8_t *pkt)
+{
+	uint16_t dst, features, net_idx;
+	uint8_t period_log, count_log, ttl;
+	struct mesh_net *net;
+	int status;
+
+	dst = l_get_le16(pkt);
+	count_log = pkt[2];
+	period_log = pkt[3];
+	ttl = pkt[4];
+
+	if (count_log > 0x11 && count_log != 0xff)
+		return 0;
+
+	if (period_log > 0x11 || ttl > TTL_MASK || IS_VIRTUAL(dst))
+		return 0;
+
+	features = l_get_le16(pkt + 5) & 0xf;
+	net_idx = l_get_le16(pkt + 7);
+
+	net = node_get_net(node);
+
+	status = mesh_net_set_heartbeat_pub(net, dst, features, net_idx, ttl,
+						count_log, period_log);
+
+	return hb_publication_get(node, status);
 }
 
 static void node_reset(void *user_data)
@@ -732,10 +715,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 	struct mesh_node *node = (struct mesh_node *) user_data;
 	struct mesh_net *net;
 	const uint8_t *pkt = data;
-	struct timeval time_now;
 	uint32_t opcode;
-	int b_res = MESH_STATUS_SUCCESS;
-	struct mesh_net_heartbeat *hb;
 	uint16_t n_idx;
 	uint8_t state;
 	bool virt = false;
@@ -751,7 +731,7 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		return false;
 
 	net = node_get_net(node);
-	hb = mesh_net_heartbeat_get(net);
+
 	l_debug("CONFIG-SRV-opcode 0x%x size %u idx %3.3x", opcode, size,
 								net_idx);
 
@@ -1024,113 +1004,35 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		break;
 
 	case OP_CONFIG_HEARTBEAT_PUB_SET:
-		l_debug("OP_CONFIG_HEARTBEAT_PUB_SET");
+		l_debug("Config Heartbeat Publication Set");
 		if (size != 9)
 			return true;
 
-		if (pkt[2] > 0x11 || pkt[3] > 0x10 || pkt[4] > 0x7f)
-			return true;
-		else if (IS_VIRTUAL(l_get_le16(pkt)))
-			b_res = MESH_STATUS_INVALID_ADDRESS;
-		else if (l_get_le16(pkt + 7) != mesh_net_get_primary_idx(net))
-			/* Future work: check for valid subnets */
-			b_res = MESH_STATUS_INVALID_NETKEY;
-
-		n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_PUB_STATUS,
-						msg);
-		msg[n++] = b_res;
-
-		memcpy(&msg[n], pkt, 9);
-
-		/* Ignore RFU bits in features */
-		l_put_le16(l_get_le16(pkt + 5) & 0xf, &msg[n + 5]);
-
-		/* Add octet count to status */
-		n += 9;
-
-		if (b_res != MESH_STATUS_SUCCESS)
-			break;
-
-		hb->pub_dst = l_get_le16(pkt);
-		if (hb->pub_dst == UNASSIGNED_ADDRESS ||
-				pkt[2] == 0 || pkt[3] == 0) {
-			/*
-			 * We might still have a pub_dst here in case
-			 * we need it for State Change heartbeat
-			 */
-			hb->pub_count = 0;
-			hb->pub_period = 0;
-		} else {
-			hb->pub_count = (pkt[2] != 0xff) ?
-				log_to_uint32(pkt[2], 1) : 0xffff;
-			hb->pub_period = log_to_uint32(pkt[3], 1);
-		}
-
-		hb->pub_ttl = pkt[4];
-		hb->pub_features = l_get_le16(pkt + 5) & 0xf;
-		hb->pub_net_idx = l_get_le16(pkt + 7);
-		update_hb_pub_timer(net, hb);
-
+		n = hb_publication_set(node, pkt);
 		break;
 
 	case OP_CONFIG_HEARTBEAT_PUB_GET:
 		if (size != 0)
 			return true;
 
-		n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_PUB_STATUS, msg);
-		msg[n++] = b_res;
-		l_put_le16(hb->pub_dst, msg + n);
-		n += 2;
-		msg[n++] = uint32_to_log(hb->pub_count);
-		msg[n++] = uint32_to_log(hb->pub_period);
-		msg[n++] = hb->pub_ttl;
-		l_put_le16(hb->pub_features, msg + n);
-		n += 2;
-		l_put_le16(hb->pub_net_idx, msg + n);
-		n += 2;
+		n = hb_publication_get(node, MESH_STATUS_SUCCESS);
 		break;
 
 	case OP_CONFIG_HEARTBEAT_SUB_SET:
 		if (size != 5)
 			return true;
 
-		l_debug("Set Sub Period (Log %2.2x) %d sec",
-				pkt[4], log_to_uint32(pkt[4], 1));
-
-		b_res = hb_subscription_set(net, l_get_le16(pkt),
-						l_get_le16(pkt + 2),
-						pkt[4]);
-		if (b_res < 0)
-			return true;
+		l_debug("Set HB Sub Period Log %2.2x", pkt[4]);
 
-		/* Fall through */
+		n = hb_subscription_set(node, pkt);
+		break;
 
 	case OP_CONFIG_HEARTBEAT_SUB_GET:
-		if (opcode == OP_CONFIG_HEARTBEAT_SUB_GET && size != 0)
-			return true;
-
-		gettimeofday(&time_now, NULL);
-		time_now.tv_sec -= hb->sub_start;
 
-		if (time_now.tv_sec >= (long int) hb->sub_period)
-			time_now.tv_sec = 0;
-		else
-			time_now.tv_sec = hb->sub_period - time_now.tv_sec;
-
-		l_debug("Sub Period (Log %2.2x) %d sec",
-				uint32_to_log(time_now.tv_sec),
-				(int) time_now.tv_sec);
+		if (size != 0)
+			return true;
 
-		n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_SUB_STATUS, msg);
-		msg[n++] = b_res;
-		l_put_le16(hb->sub_src, msg + n);
-		n += 2;
-		l_put_le16(hb->sub_dst, msg + n);
-		n += 2;
-		msg[n++] = uint32_to_log(time_now.tv_sec);
-		msg[n++] = uint32_to_log(hb->sub_count);
-		msg[n++] = hb->sub_count ? hb->sub_min_hops : 0;
-		msg[n++] = hb->sub_max_hops;
+		n = hb_subscription_get(node, MESH_STATUS_SUCCESS);
 		break;
 
 	case OP_CONFIG_POLL_TIMEOUT_GET:
@@ -1160,13 +1062,6 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 
 static void cfgmod_srv_unregister(void *user_data)
 {
-	struct mesh_node *node = user_data;
-	struct mesh_net *net = node_get_net(node);
-	struct mesh_net_heartbeat *hb = mesh_net_heartbeat_get(net);
-
-	l_timeout_remove(hb->pub_timer);
-	l_timeout_remove(hb->sub_timer);
-	hb->pub_timer = hb->sub_timer = NULL;
 }
 
 static const struct mesh_model_ops ops = {
diff --git a/mesh/net.c b/mesh/net.c
index b54c647cb..5bfaa181a 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -23,6 +23,8 @@
 
 #define _GNU_SOURCE
 
+#include <sys/time.h>
+
 #include <ell/ell.h>
 
 #include "mesh/mesh-defs.h"
@@ -132,7 +134,10 @@ struct mesh_net {
 		uint8_t count;
 	} relay;
 
-	struct mesh_net_heartbeat heartbeat;
+	/* Heartbeat info */
+	struct mesh_net_heartbeat_sub hb_sub;
+	struct mesh_net_heartbeat_pub hb_pub;
+	uint16_t features;
 
 	struct l_queue *subnets;
 	struct l_queue *msg_cache;
@@ -255,35 +260,46 @@ static bool match_friend_key_id(const void *a, const void *b)
 					(key_id == friend->net_key_upd);
 }
 
-static void idle_mesh_heartbeat_send(void *net)
+static void send_hb_publication(void *data)
 {
-	mesh_net_heartbeat_send(net);
+	struct mesh_net *net = data;
+	struct mesh_net_heartbeat_pub *pub = &net->hb_pub;
+	uint8_t msg[4];
+	int n = 0;
+
+	if (pub->dst == UNASSIGNED_ADDRESS)
+		return;
+
+	msg[n++] = NET_OP_HEARTBEAT;
+	msg[n++] = pub->ttl;
+	l_put_be16(net->features, msg + n);
+	n += 2;
+
+	mesh_net_transport_send(net, 0, 0, mesh_net_get_iv_index(net),
+					pub->ttl, 0, 0, pub->dst, msg, n);
 }
 
 static void trigger_heartbeat(struct mesh_net *net, uint16_t feature,
-								bool in_use)
+								bool enable)
 {
-	struct mesh_net_heartbeat *hb = &net->heartbeat;
-
-	l_debug("%s: %4.4x --> %d", __func__, feature, in_use);
+	l_debug("HB: %4.4x --> %d", feature, enable);
 
-	if (in_use) {
-		if (net->heartbeat.features & feature)
+	if (enable) {
+		if (net->features & feature)
 			return; /* no change */
 
-		hb->features |= feature;
+		net->features |= feature;
 	} else {
-		if (!(hb->features & feature))
+		if (!(net->features & feature))
 			return; /* no change */
 
-		hb->features &= ~feature;
+		net->features &= ~feature;
 	}
 
-	if (!(hb->pub_features & feature))
-		return; /* not interested in this feature */
-
-	l_idle_oneshot(idle_mesh_heartbeat_send, net, NULL);
+	if (!(net->hb_pub.features & feature))
+		return; /* no interest in this feature */
 
+	l_idle_oneshot(send_hb_publication, net, NULL);
 }
 
 static bool match_by_friend(const void *a, const void *b)
@@ -616,8 +632,6 @@ struct mesh_net *mesh_net_new(struct mesh_node *node)
 	net->destinations = l_queue_new();
 	net->app_keys = l_queue_new();
 
-	memset(&net->heartbeat, 0, sizeof(net->heartbeat));
-
 	if (!nets)
 		nets = l_queue_new();
 
@@ -813,8 +827,8 @@ int mesh_net_del_key(struct mesh_net *net, uint16_t idx)
 	appkey_delete_bound_keys(net, idx);
 
 	/* Disable hearbeat publication on this subnet */
-	if (idx == net->heartbeat.pub_net_idx)
-		net->heartbeat.pub_dst = UNASSIGNED_ADDRESS;
+	if (idx == net->hb_pub.net_idx)
+		net->hb_pub.dst = UNASSIGNED_ADDRESS;
 
 	/* TODO: cancel beacon_enable on this subnet */
 
@@ -2017,25 +2031,23 @@ static bool ctl_received(struct mesh_net *net, uint16_t key_id,
 		break;
 
 	case NET_OP_HEARTBEAT:
-		if (net->heartbeat.sub_enabled &&
-				src == net->heartbeat.sub_src) {
+		if (net->hb_sub.enabled && src == net->hb_sub.src) {
 			uint8_t hops = pkt[0] - ttl + 1;
 
 			print_packet("Rx-NET_OP_HEARTBEAT", pkt, len);
 
-			if (net->heartbeat.sub_count != 0xffff)
-				net->heartbeat.sub_count++;
+			if (net->hb_sub.count != 0xffff)
+				net->hb_sub.count++;
 
-			if (net->heartbeat.sub_min_hops > hops)
-				net->heartbeat.sub_min_hops = hops;
+			if (net->hb_sub.min_hops > hops)
+				net->hb_sub.min_hops = hops;
 
-			if (net->heartbeat.sub_max_hops < hops)
-				net->heartbeat.sub_max_hops = hops;
+			if (net->hb_sub.max_hops < hops)
+				net->hb_sub.max_hops = hops;
 
 			l_debug("HB: cnt:%4.4x min:%2.2x max:%2.2x",
-					net->heartbeat.sub_count,
-					net->heartbeat.sub_min_hops,
-					net->heartbeat.sub_max_hops);
+					net->hb_sub.count, net->hb_sub.min_hops,
+							net->hb_sub.max_hops);
 		}
 		break;
 	}
@@ -3276,52 +3288,14 @@ int mesh_net_update_key(struct mesh_net *net, uint16_t idx,
 	return MESH_STATUS_SUCCESS;
 }
 
-static uint16_t get_features(struct mesh_net *net)
-{
-	uint16_t features = 0;
-
-	if (net->relay.enable)
-		features |= FEATURE_RELAY;
-
-	if (net->proxy_enable)
-		features |= FEATURE_PROXY;
-
-	if (net->friend_enable)
-		features |= FEATURE_FRIEND;
-
-	return features;
-}
-
-struct mesh_net_heartbeat *mesh_net_heartbeat_get(struct mesh_net *net)
-{
-	return &net->heartbeat;
-}
-
-void mesh_net_heartbeat_send(struct mesh_net *net)
+struct mesh_net_heartbeat_sub *mesh_net_get_heartbeat_sub(struct mesh_net *net)
 {
-	struct mesh_net_heartbeat *hb = &net->heartbeat;
-	uint8_t msg[4];
-	int n = 0;
-
-	if (hb->pub_dst == UNASSIGNED_ADDRESS)
-		return;
-
-	msg[n++] = NET_OP_HEARTBEAT;
-	msg[n++] = hb->pub_ttl;
-	l_put_be16(hb->features, msg + n);
-	n += 2;
-
-	mesh_net_transport_send(net, 0, 0, mesh_net_get_iv_index(net),
-					hb->pub_ttl, 0, 0, hb->pub_dst, msg, n);
+	return &net->hb_sub;
 }
 
-void mesh_net_heartbeat_init(struct mesh_net *net)
+struct mesh_net_heartbeat_pub *mesh_net_get_heartbeat_pub(struct mesh_net *net)
 {
-	struct mesh_net_heartbeat *hb = &net->heartbeat;
-
-	memset(hb, 0, sizeof(struct mesh_net_heartbeat));
-	hb->sub_min_hops = 0xff;
-	hb->features = get_features(net);
+	return &net->hb_pub;
 }
 
 void mesh_net_set_iv_index(struct mesh_net *net, uint32_t index, bool update)
@@ -3559,3 +3533,156 @@ void net_msg_add_replay_cache(struct mesh_net *net, uint16_t src, uint32_t seq,
 	/* Optimize so that most recent conversations stay earliest in cache */
 	l_queue_push_head(net->replay_cache, rpe);
 }
+
+static void hb_sub_timeout_func(struct l_timeout *timeout, void *user_data)
+{
+	struct mesh_net *net = user_data;
+	struct mesh_net_heartbeat_sub *sub = &net->hb_sub;
+
+	l_debug("HB Subscription Ended");
+	l_timeout_remove(sub->timer);
+	sub->timer = NULL;
+	sub->enabled = false;
+}
+
+static uint32_t log_to_uint32(uint8_t log)
+{
+	if (!log)
+		return 0x0000;
+
+	return (1 << (log - 1));
+}
+
+int mesh_net_set_heartbeat_sub(struct mesh_net *net, uint16_t src, uint16_t dst,
+							uint8_t period_log)
+{
+	struct mesh_net_heartbeat_sub *sub = &net->hb_sub;
+	struct timeval time_now;
+
+	if (!net)
+		return MESH_STATUS_UNSPECIFIED_ERROR;
+
+	/* Check if the subscription should be disabled */
+	if (IS_UNASSIGNED(src) || IS_UNASSIGNED(dst)) {
+		if (IS_GROUP(sub->dst))
+			mesh_net_dst_unreg(net, sub->dst);
+
+		sub->enabled = false;
+		sub->dst = UNASSIGNED_ADDRESS;
+		sub->src = UNASSIGNED_ADDRESS;
+		sub->count = 0;
+		sub->period = 0;
+		sub->min_hops = 0;
+		sub->max_hops = 0;
+
+	} else if (!period_log && src == sub->src && dst == sub->dst) {
+		/* Preserve collected data, but disable */
+		sub->enabled = false;
+		sub->period = 0;
+
+	} else if (sub->dst != dst) {
+		if (IS_GROUP(sub->dst))
+			mesh_net_dst_unreg(net, sub->dst);
+
+		if (IS_GROUP(dst))
+			mesh_net_dst_reg(net, dst);
+
+		sub->enabled = !!period_log;
+		sub->src = src;
+		sub->dst = dst;
+		sub->count = 0;
+		sub->period = log_to_uint32(period_log);
+		sub->min_hops = 0x00;
+		sub->max_hops = 0x00;
+		gettimeofday(&time_now, NULL);
+		sub->start = time_now.tv_sec;
+	}
+
+	/* TODO: Save to node config */
+
+	if (!sub->enabled) {
+		l_timeout_remove(sub->timer);
+		sub->timer = NULL;
+		return MESH_STATUS_SUCCESS;
+	}
+
+	sub->min_hops = 0xff;
+
+	if (!sub->timer)
+		sub->timer = l_timeout_create(sub->period, hb_sub_timeout_func,
+								net, NULL);
+	else
+		l_timeout_modify(sub->timer, sub->period);
+
+	return MESH_STATUS_SUCCESS;
+}
+
+static void hb_pub_timeout_func(struct l_timeout *timeout, void *user_data)
+{
+	struct mesh_net *net = user_data;
+	struct mesh_net_heartbeat_pub *pub = &net->hb_pub;
+
+	send_hb_publication(net);
+
+	if (pub->count != 0xffff)
+		pub->count--;
+
+	if (pub->count > 0)
+		l_timeout_modify(pub->timer, pub->period);
+	else {
+		l_timeout_remove(pub->timer);
+		pub->timer = NULL;
+	}
+}
+
+static void update_hb_pub_timer(struct mesh_net *net,
+					struct mesh_net_heartbeat_pub *pub)
+{
+	if (IS_UNASSIGNED(pub->dst) || pub->count == 0) {
+		l_timeout_remove(pub->timer);
+		pub->timer = NULL;
+		return;
+	}
+
+	if (!pub->timer)
+		pub->timer = l_timeout_create(pub->period,
+					hb_pub_timeout_func, net, NULL);
+	else
+		l_timeout_modify(pub->timer, pub->period);
+}
+
+int mesh_net_set_heartbeat_pub(struct mesh_net *net, uint16_t dst,
+				uint16_t features, uint16_t idx, uint8_t ttl,
+				uint8_t count_log, uint8_t period_log)
+{
+	struct mesh_subnet *subnet;
+	struct mesh_net_heartbeat_pub *pub = &net->hb_pub;
+
+	if (!net)
+		return MESH_STATUS_UNSPECIFIED_ERROR;
+
+	subnet = l_queue_find(net->subnets, match_key_index,
+							L_UINT_TO_PTR(idx));
+	if (!subnet)
+		return MESH_STATUS_INVALID_NETKEY;
+
+	pub->dst = dst;
+
+	if (pub->dst == UNASSIGNED_ADDRESS) {
+		pub->count = 0;
+		pub->period = 0;
+		pub->ttl = 0;
+	} else {
+		pub->count = (count_log != 0xff) ?
+					log_to_uint32(count_log) : 0xffff;
+		pub->period = log_to_uint32(period_log);
+	}
+
+	pub->ttl = ttl;
+	pub->features = features;
+	pub->net_idx = idx;
+	update_hb_pub_timer(net, pub);
+
+	/* TODO: Save to node config */
+	return MESH_STATUS_SUCCESS;
+}
diff --git a/mesh/net.h b/mesh/net.h
index 7117f1a47..0e36ab068 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -131,25 +131,27 @@ struct mesh_net_prov_caps {
 	uint16_t input_action;
 } __packed;
 
-struct mesh_net_heartbeat {
-	struct l_timeout *pub_timer;
-	struct l_timeout *sub_timer;
-	struct timeval sub_time;
-	bool sub_enabled;
-	uint32_t pub_period;
-	uint32_t sub_period;
-	uint32_t sub_start;
-	uint16_t pub_dst;
-	uint16_t pub_count;
-	uint16_t pub_features;
+struct mesh_net_heartbeat_sub {
+	struct l_timeout *timer;
+	uint32_t start;
+	uint32_t period;
 	uint16_t features;
-	uint16_t pub_net_idx;
-	uint16_t sub_src;
-	uint16_t sub_dst;
-	uint16_t sub_count;
-	uint8_t pub_ttl;
-	uint8_t sub_min_hops;
-	uint8_t sub_max_hops;
+	uint16_t src;
+	uint16_t dst;
+	uint16_t count;
+	bool enabled;
+	uint8_t min_hops;
+	uint8_t max_hops;
+};
+
+struct mesh_net_heartbeat_pub {
+	struct l_timeout *timer;
+	uint32_t period;
+	uint16_t dst;
+	uint16_t count;
+	uint16_t features;
+	uint16_t net_idx;
+	uint8_t ttl;
 };
 
 struct mesh_key_set {
@@ -330,9 +332,13 @@ void mesh_net_send_seg(struct mesh_net *net, uint32_t key_id,
 				uint32_t iv_index, uint8_t ttl, uint32_t seq,
 				uint16_t src, uint16_t dst, uint32_t hdr,
 				const void *seg, uint16_t seg_len);
-struct mesh_net_heartbeat *mesh_net_heartbeat_get(struct mesh_net *net);
-void mesh_net_heartbeat_init(struct mesh_net *net);
-void mesh_net_heartbeat_send(struct mesh_net *net);
+struct mesh_net_heartbeat_sub *mesh_net_get_heartbeat_sub(struct mesh_net *net);
+int mesh_net_set_heartbeat_sub(struct mesh_net *net, uint16_t src, uint16_t dst,
+							uint8_t period_log);
+struct mesh_net_heartbeat_pub *mesh_net_get_heartbeat_pub(struct mesh_net *net);
+int mesh_net_set_heartbeat_pub(struct mesh_net *net, uint16_t dst,
+				uint16_t features, uint16_t idx, uint8_t ttl,
+				uint8_t count_log, uint8_t period_log);
 bool mesh_net_key_list_get(struct mesh_net *net, uint8_t *buf, uint16_t *count);
 uint16_t mesh_net_get_primary_idx(struct mesh_net *net);
 uint32_t mesh_net_friend_timeout(struct mesh_net *net, uint16_t addr);
-- 
2.26.2


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

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 20:38 [PATCH BlueZ 00/10] Clean up Config Server Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 01/10] mesh: Clean up handling of config subscription messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 02/10] mesh: Clean up handling of config model binding messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 03/10] mesh: Clean up handling of config node identity message Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 04/10] mesh: Clean up handling of config publication messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 05/10] mesh: Clean up handling of config net and app key messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 06/10] mesh: Clean up handling of config relay messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 07/10] mesh: Clean up handling of config poll timeout message Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 08/10] mesh: Clean up handling of config net transmit messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 09/10] mesh: Clean up handling of config KR phase messages Inga Stotland
2020-07-30 20:38 ` [PATCH BlueZ 10/10] mesh: Refactor heartbeat pub/sub Inga Stotland

Linux-Bluetooth Archive on lore.kernel.org

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

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

Example config snippet for mirrors

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


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