[PATCH BlueZ 0/4] Mesh: Fix heartbeat publication/subscription

[PATCH BlueZ 0/4] Mesh: Fix heartbeat publication/subscription

[Isak Westin]

Hi,

These patches include some fixes to the heartbeat publication and subscription procedures.
Found during following PTS tests:
- MESH/NODE/CFG/HBS/*
- MESH/NODE/CFG/HBP/*

Best regards,
Isak

Isak Westin (4):
  mesh: Correct u32 to u8 log transformation
  mesh: Reply to HB pub set with same fields
  mesh: Correct HB sub state updates
  mesh: Clear HB sub status field if disabled

 mesh/cfgmod-server.c | 48 ++++++++++++++++++++++++++++++++++++--------
 mesh/net.c           | 20 ++++--------------
 2 files changed, 44 insertions(+), 24 deletions(-)

-- 
2.20.1

[PATCH BlueZ 1/4] mesh: Correct u32 to u8 log transformation

[Isak Westin]

Fixed the log transformation to correctly follow the value mapping
defined in the mesh profile (section 4.1.2).
---
 mesh/cfgmod-server.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 9bc2f1c97..33796d05a 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -455,14 +455,14 @@ done:
 static uint8_t uint32_to_log(uint32_t value)
 {
 	uint32_t val = 1;
-	uint8_t ret = 1;
+	uint8_t ret = 0;
 
 	if (!value)
 		return 0;
 	else if (value > 0x10000)
 		return 0xff;
 
-	while (val < value) {
+	while (val <= value) {
 		val <<= 1;
 		ret++;
 	}
@@ -495,7 +495,7 @@ static uint16_t hb_subscription_get(struct mesh_node *node, int status)
 	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 != 0xffff ? uint32_to_log(sub->count) : 0xff;
 	msg[n++] = sub->count ? sub->min_hops : 0;
 	msg[n++] = sub->max_hops;
 
@@ -538,7 +538,7 @@ static uint16_t hb_publication_get(struct mesh_node *node, int status)
 	msg[n++] = status;
 	l_put_le16(pub->dst, msg + n);
 	n += 2;
-	msg[n++] = uint32_to_log(pub->count);
+	msg[n++] = pub->count != 0xffff ? uint32_to_log(pub->count) : 0xff;
 	msg[n++] = uint32_to_log(pub->period);
 	msg[n++] = pub->ttl;
 	l_put_le16(pub->features, msg + n);
-- 
2.20.1

[PATCH BlueZ 2/4] mesh: Reply to HB pub set with same fields

[Isak Westin]

If a Config Heartbeat Publication Set message is unsuccessfully
processed, the fields in the status reply should be the same as in the
original message. See MshPRFv1.0.1 section 4.4.1.2.15.
---
 mesh/cfgmod-server.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 33796d05a..9c5edf551 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -575,7 +575,17 @@ static uint16_t hb_publication_set(struct mesh_node *node, const uint8_t *pkt)
 	status = mesh_net_set_heartbeat_pub(net, dst, features, net_idx, ttl,
 						count_log, period_log);
 
-	return hb_publication_get(node, status);
+	if (status != MESH_STATUS_SUCCESS) {
+		uint16_t n;
+
+		n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_PUB_STATUS, msg);
+		msg[n++] = status;
+		memcpy(msg + n, pkt, 9);
+		n += 9;
+
+		return n;
+	} else
+		return hb_publication_get(node, status);
 }
 
 static void node_reset(void *user_data)
-- 
2.20.1

[PATCH BlueZ 3/4] mesh: Correct HB sub state updates

[Isak Westin]

If heartbeat subscription is disabled, all fields should be set to zero
but collected data should be preserved. If HB subscription is enabled,
the collected data should be reset (which includes Min Hops = 0x7f).
HB subscription is disabled by setting any of the following fields to
zero: Source, destination or period log.
HB subscription is enabled by setting all the same fields to valid values.
---
 mesh/cfgmod-server.c |  2 +-
 mesh/net.c           | 20 ++++----------------
 2 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 9c5edf551..55a2d896b 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -496,7 +496,7 @@ static uint16_t hb_subscription_get(struct mesh_node *node, int status)
 	n += 2;
 	msg[n++] = uint32_to_log(time_now.tv_sec);
 	msg[n++] = sub->count != 0xffff ? uint32_to_log(sub->count) : 0xff;
-	msg[n++] = sub->count ? sub->min_hops : 0;
+	msg[n++] = sub->min_hops;
 	msg[n++] = sub->max_hops;
 
 	return n;
diff --git a/mesh/net.c b/mesh/net.c
index 699469284..7fec98531 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3608,24 +3608,14 @@ int mesh_net_set_heartbeat_sub(struct mesh_net *net, uint16_t src, uint16_t dst,
 		return MESH_STATUS_UNSPECIFIED_ERROR;
 
 	/* Check if the subscription should be disabled */
-	if (IS_UNASSIGNED(src) || IS_UNASSIGNED(dst)) {
+	if (IS_UNASSIGNED(src) || IS_UNASSIGNED(dst) || !period_log) {
 		if (IS_GROUP(sub->dst))
 			mesh_net_dst_unreg(net, sub->dst);
 
+		/* Preserve collected data, but disable */
 		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) {
-		if (IS_GROUP(sub->dst))
-			mesh_net_dst_unreg(net, sub->dst);
-
-		/* Preserve collected data, but disable */
-		sub->enabled = false;
 		sub->period = 0;
 
 	} else {
@@ -3637,12 +3627,12 @@ int mesh_net_set_heartbeat_sub(struct mesh_net *net, uint16_t src, uint16_t dst,
 				mesh_net_dst_reg(net, dst);
 		}
 
-		sub->enabled = !!period_log;
+		sub->enabled = true;
 		sub->src = src;
 		sub->dst = dst;
 		sub->count = 0;
 		sub->period = log_to_uint32(period_log);
-		sub->min_hops = 0x00;
+		sub->min_hops = 0x7f;
 		sub->max_hops = 0x00;
 		gettimeofday(&time_now, NULL);
 		sub->start = time_now.tv_sec;
@@ -3656,8 +3646,6 @@ int mesh_net_set_heartbeat_sub(struct mesh_net *net, uint16_t src, uint16_t dst,
 		return MESH_STATUS_SUCCESS;
 	}
 
-	sub->min_hops = 0xff;
-
 	if (!sub->timer)
 		sub->timer = l_timeout_create(sub->period, hb_sub_timeout_func,
 								net, NULL);
-- 
2.20.1

[PATCH BlueZ 4/4] mesh: Clear HB sub status field if disabled

[Isak Westin]

When replying to a HB subscription get message, and the current state of
source or destination fields is zero (which means that HB subscription
is disabled), all fields in the status reply should be zero.
---
 mesh/cfgmod-server.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 55a2d896b..7044b670d 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -470,7 +470,7 @@ static uint8_t uint32_to_log(uint32_t value)
 	return ret;
 }
 
-static uint16_t hb_subscription_get(struct mesh_node *node, int status)
+static uint16_t hb_subscription_status(struct mesh_node *node, int status)
 {
 	struct mesh_net *net = node_get_net(node);
 	struct mesh_net_heartbeat_sub *sub = mesh_net_get_heartbeat_sub(net);
@@ -502,6 +502,28 @@ static uint16_t hb_subscription_get(struct mesh_node *node, int status)
 	return n;
 }
 
+static uint16_t hb_subscription_get(struct mesh_node *node, int status)
+{
+	struct mesh_net *net = node_get_net(node);
+	struct mesh_net_heartbeat_sub *sub = mesh_net_get_heartbeat_sub(net);
+
+	/*
+	 * MshPRFv1.0.1 section 4.4.1.2.16, Heartbeat Subscription state:
+	 * If this is a GET request and the source or destination is unassigned,
+	 * all fields shall be set to zero in the status reply.
+	 */
+	if (IS_UNASSIGNED(sub->src) || IS_UNASSIGNED(sub->dst)) {
+		uint16_t n;
+
+		n = mesh_model_opcode_set(OP_CONFIG_HEARTBEAT_SUB_STATUS, msg);
+		memset(msg + n, 0, 9);
+		n += 9;
+		return n;
+	}
+
+	return hb_subscription_status(node, status);
+}
+
 static uint16_t hb_subscription_set(struct mesh_node *node, const uint8_t *pkt)
 {
 	uint16_t src, dst;
@@ -525,7 +547,7 @@ static uint16_t hb_subscription_set(struct mesh_node *node, const uint8_t *pkt)
 
 	status = mesh_net_set_heartbeat_sub(net, src, dst, period_log);
 
-	return hb_subscription_get(node, status);
+	return hb_subscription_status(node, status);
 }
 
 static uint16_t hb_publication_get(struct mesh_node *node, int status)
-- 
2.20.1

RE: Mesh: Fix heartbeat publication/subscription

[bluez.test.bot]

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=680522

---Test result---

Test Summary:
CheckPatch                    PASS      3.06 seconds
GitLint                       PASS      2.07 seconds
Prep - Setup ELL              PASS      35.30 seconds
Build - Prep                  PASS      0.83 seconds
Build - Configure             PASS      10.94 seconds
Build - Make                  PASS      1231.41 seconds
Make Check                    PASS      13.40 seconds
Make Check w/Valgrind         PASS      380.05 seconds
Make Distcheck                PASS      324.29 seconds
Build w/ext ELL - Configure   PASS      11.27 seconds
Build w/ext ELL - Make        PASS      115.80 seconds
Incremental Build w/ patches  PASS      538.07 seconds
Scan Build                    PASS      738.03 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 0/4] Mesh: Fix heartbeat publication/subscription

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Brian Gix <brian.gix@intel.com>:

On Mon, 26 Sep 2022 15:01:06 +0200 you wrote:
> Hi,
> 
> These patches include some fixes to the heartbeat publication and subscription procedures.
> Found during following PTS tests:
> - MESH/NODE/CFG/HBS/*
> - MESH/NODE/CFG/HBP/*
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/4] mesh: Correct u32 to u8 log transformation
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5b569e3d14a3
  - [BlueZ,2/4] mesh: Reply to HB pub set with same fields
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1ef221ca0205
  - [BlueZ,3/4] mesh: Correct HB sub state updates
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=902389f3e7a3
  - [BlueZ,4/4] mesh: Clear HB sub status field if disabled
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d763bfa4d089

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html