Re: [PATCH BlueZ V2] Mesh: Fix TTL in Config Heartbeat Publication Set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On 03/23/2018 05:08 PM, Robert Lubas wrote:
Hi Brian,

On 03/23/2018 01:37 PM, Gix, Brian wrote:
Hi Robert,

On Mar 23, 2018, at 8:20 AM, Robert Lubas <robert.lubas@xxxxxxxxxxx> wrote:

Hi Brian,

On 03/22/2018 10:33 PM, Gix, Brian wrote:
I am unable to look at all the code in context at the moment, but using the DEFAULT_TTL (0xff) for any TTL value, will automatically get replaced with whatever the default TTL has been set to. It is a handy value which means “use the system setting” without needing to look it up.
On Mar 22, 2018, at 3:45 AM, Robert Lubaś <robert.lubas@xxxxxxxxxxx> wrote:

In Mesh Profile spec 4.2.17.4 Heartbeat Publication TTL value range is
0x00-0x7F. In cmd_hb_pub_set heartbeat ttl was set to DEFAULT_TTL 0xFF, this
patch fix this by adding ttl param to hb-pub-set.
---
mesh/config-client.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/mesh/config-client.c b/mesh/config-client.c
index 19e617d62..0b5b8677b 100644
--- a/mesh/config-client.c
+++ b/mesh/config-client.c
@@ -1042,7 +1042,7 @@ static void cmd_hb_pub_set(int argc, char *argv[])
    n = mesh_opcode_set(OP_CONFIG_HEARTBEAT_PUB_SET, msg);

    parm_cnt = read_input_parameters(argc, argv);
-    if (parm_cnt != 5) {
+    if (parm_cnt != 6) {
        bt_shell_printf("Bad arguments: %s\n", argv[1]);
        return bt_shell_noninteractive_quit(EXIT_FAILURE);
    }
@@ -1056,12 +1056,12 @@ static void cmd_hb_pub_set(int argc, char *argv[])
    /* Period Log */
    msg[n++] = parms[2];
    /* Heartbeat TTL */
-    msg[n++] = DEFAULT_TTL;
+    msg[n++] = parms[3];
    /* Features */
-    put_le16(parms[3], msg + n);
+    put_le16(parms[4], msg + n);
    n += 2;
    /* NetKey Index */
-    put_le16(parms[4], msg + n);
+    put_le16(parms[5], msg + n);
    n += 2;

    if (!config_send(msg, n)) {
@@ -1167,8 +1167,8 @@ static const struct bt_shell_menu cfg_menu = {
                        "Set relay"},
    {"relay-get",           NULL,                   cmd_relay_get,
                        "Get relay"},
-    {"hb-pub-set", "<pub_addr> <count> <period> <features> <net_idx>",
-                cmd_hb_pub_set,     "Set heartbeat publish"},
+    {"hb-pub-set", "<pub_addr> <count> <period> <ttl> <features> <net_idx>",
+                cmd_hb_pub_set,    "Set heartbeat publish"},
    {"hb-pub-get",           NULL,                   cmd_hb_pub_get,
                        "Get heartbeat publish"},
    {"hb-sub-set", "<src_addr> <dst_addr> <period>",
--
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
I see your point. You know the mesh specification very well, but I will just refer to it to construct the reply. The Mesh Profile specification says: if the publish TTL is set to 0xFF the message use the Default TTL (4.2.2.5). The Default TTL valid values are 0x00, 0x02-0x7F. Prohibited are 0x01, 0x80-0x7FF (4.2.7).

However in terms of Heartbeat publication TTL the valid values are 0x00-0x7F. Prohibited are 0x80-0xFF (4.2.17.4). Taking into account valid ranges the heartbeat TTL should be masked with 0x7F or in case of 0xFF the node Default TTL from db should be used.

I think that it will be helpful e.g. to assess the basic reliability of the mesh network having the heartbeat publication TTL as a parameter. Then, for example you can very easy build the metric which depends on TTL.

You are correct that you should never see anything greater than 0x7f over-the-air.... but the intent was that the lower layers would handle the translation to the system setting.... or in the case of heartbeat, the control layer.

Can you expand a little bit more about handling the translation to the system setting?

The Default TTL is applied to access layer, the heartbeat operates on upper transport layer - so from Configuration Client point of view all above 0x7F are just prohibited.The current implementation of heartbeat publication set does not work.
The problem is with the TTL field in the message (not valid value is hard codded). I proposed a fix by passing ttl into cmd_hb_pub_set (user has to be aware of valid values of the TTL as well as of another parameters in this function)

In the 4.4.1.2.15 Heartbeat Publication state (Mesh Profile spec) in the 3rd paragraph we can read that if the message is successfully processed (no invalid key index error) we should update the Heartbeat Publication state to the corresponding field values (defined in Table 4.120). Going to this table the Heartbeat Publication TTL state should be updated using message field TTL. The message field TTL has to be set according to the 4.2.17.4 Heartbeat Publication TTL (0x00-0x7F values are valid).

In the spec there is nothing about translation to the system settings mechanism in case of heartbeat.

Kindly asking for the review.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux