Return-Path: Subject: Re: [PATCH BlueZ V2] Mesh: Fix TTL in Config Heartbeat Publication Set To: "Gix, Brian" Cc: "linux-bluetooth@vger.kernel.org" References: <20180322104442.27650-1-robert.lubas@silvair.com>

<4AD7CCB7-A68D-4570-B575-34E8A32E5422@intel.com> From: Robert Lubas Message-ID: <9748004b-5fe5-f768-0fca-15bb83aec51a@silvair.com> Date: Fri, 23 Mar 2018 17:08:40 +0100 MIME-Version: 1.0 In-Reply-To: <4AD7CCB7-A68D-4570-B575-34E8A32E5422@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Brian, On 03/23/2018 01:37 PM, Gix, Brian wrote: > Hi Robert, > >> On Mar 23, 2018, at 8:20 AM, Robert Lubas 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ś 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", " ", >>>> - cmd_hb_pub_set, "Set heartbeat publish"}, >>>> + {"hb-pub-set", " ", >>>> + cmd_hb_pub_set, "Set heartbeat publish"}, >>>> {"hb-pub-get", NULL, cmd_hb_pub_get, >>>> "Get heartbeat publish"}, >>>> {"hb-sub-set", " ", >>>> -- >>>> 2.11.0 >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in >>>> the body of a message to majordomo@vger.kernel.org >>>> 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.