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
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
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
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
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
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
Hello:
This series was applied to bluetooth/bluez.git (master)
by Brian Gix <[email protected]>:
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