2019-08-20 17:02:18

by Carey Sonsino

[permalink] [raw]
Subject: [PATCH 1/1] bluetooth: update default BLE connection interval bounds

Update the default BLE connection interval min/max bounds to the full
range of permitted values (6-3200, corresponding to 7.25-4000ms).

Commit c49a8682fc5d298d44e8d911f4fa14690ea9485e introduced a bounds
check on connection interval update requests, but the default min/max
values were left at 24-40 (30-50ms) which caused problems for devices
that want to negotiate connection intervals outside of those bounds.

Setting the default min/max connection interval to the full allowable
range in the bluetooth specification restores the default Linux behavior
of allowing remote devices to negotiate their desired connection
interval, while still permitting the system administrator to later
narrow the range.

Fixes c49a8682fc5d: (validate BLE connection interval updates)

Signed-off-by: Carey Sonsino <[email protected]>

---

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 04bc79359a17..f4f2f712c527 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3181,8 +3181,8 @@ struct hci_dev *hci_alloc_dev(void)
     hdev->le_adv_max_interval = 0x0800;
     hdev->le_scan_interval = 0x0060;
     hdev->le_scan_window = 0x0030;
-    hdev->le_conn_min_interval = 0x0018;
-    hdev->le_conn_max_interval = 0x0028;
+    hdev->le_conn_min_interval = 0x0006;
+    hdev->le_conn_max_interval = 0x0c80;
     hdev->le_conn_latency = 0x0000;
     hdev->le_supv_timeout = 0x002a;
     hdev->le_def_tx_len = 0x001b;


2019-08-23 23:03:37

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds

Hi,

On 20 Aug 2019, at 20.01, Carey Sonsino <[email protected]> wrote:
>
> Update the default BLE connection interval min/max bounds to the full range of permitted values (6-3200, corresponding to 7.25-4000ms).
>
> Commit c49a8682fc5d298d44e8d911f4fa14690ea9485e introduced a bounds check on connection interval update requests, but the default min/max values were left at 24-40 (30-50ms) which caused problems for devices that want to negotiate connection intervals outside of those bounds.
>
> Setting the default min/max connection interval to the full allowable range in the bluetooth specification restores the default Linux behavior of allowing remote devices to negotiate their desired connection interval, while still permitting the system administrator to later narrow the range.
>
> Fixes c49a8682fc5d: (validate BLE connection interval updates)
>
> Signed-off-by: Carey Sonsino <[email protected]>
>
> ---
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 04bc79359a17..f4f2f712c527 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3181,8 +3181,8 @@ struct hci_dev *hci_alloc_dev(void)
> hdev->le_adv_max_interval = 0x0800;
> hdev->le_scan_interval = 0x0060;
> hdev->le_scan_window = 0x0030;
> - hdev->le_conn_min_interval = 0x0018;
> - hdev->le_conn_max_interval = 0x0028;
> + hdev->le_conn_min_interval = 0x0006;
> + hdev->le_conn_max_interval = 0x0c80;
> hdev->le_conn_latency = 0x0000;
> hdev->le_supv_timeout = 0x002a;
> hdev->le_def_tx_len = 0x001b;

This looks fine to me, except the commit message line lengths need fixing (max 72-74 or so). It seems we’d want this through the Bluetooth stable tree, i.e. still into the 5.3-rc series, correct? Marcel, do you agree?

Johan

2019-08-23 23:23:47

by Andreas Kemnade

[permalink] [raw]
Subject: Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds

Hi,

On Tue, 20 Aug 2019 11:01:41 -0600
Carey Sonsino <[email protected]> wrote:

> Update the default BLE connection interval min/max bounds to the full
> range of permitted values (6-3200, corresponding to 7.25-4000ms).
>
> Commit c49a8682fc5d298d44e8d911f4fa14690ea9485e introduced a bounds
> check on connection interval update requests, but the default min/max
> values were left at 24-40 (30-50ms) which caused problems for devices
> that want to negotiate connection intervals outside of those bounds.
>
> Setting the default min/max connection interval to the full allowable
> range in the bluetooth specification restores the default Linux behavior
> of allowing remote devices to negotiate their desired connection
> interval, while still permitting the system administrator to later
> narrow the range.
>
> Fixes c49a8682fc5d: (validate BLE connection interval updates)
>
Trying pair XX:XX:XX:XX:XX:XX in bluetoothctl
leads to create connection commands containing
le_conn_max_interval > le_supv_timeout (4000ms > 420ms) which the
controller does not like and is imho not allowed.

< HCI Command: LE Create Connection (0x08|0x000d) plen 25
bdaddr XX:XX:XX:XX:XX:XX type 0
interval 96 window 96 initiator_filter 0
own_bdaddr_type 0 min_interval 6 max_interval 3200
latency 0 supervision_to 42 min_ce 0 max_ce 0
> HCI Event: Command Status (0x0f) plen 4
LE Create Connection (0x08|0x000d) status 0x12 ncmd 1
Error: Invalid HCI Command Parameters


> Signed-off-by: Carey Sonsino <[email protected]>
>
> ---
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 04bc79359a17..f4f2f712c527 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3181,8 +3181,8 @@ struct hci_dev *hci_alloc_dev(void)
>      hdev->le_adv_max_interval = 0x0800;
>      hdev->le_scan_interval = 0x0060;
>      hdev->le_scan_window = 0x0030;
> -    hdev->le_conn_min_interval = 0x0018;
> -    hdev->le_conn_max_interval = 0x0028;
> +    hdev->le_conn_min_interval = 0x0006;
> +    hdev->le_conn_max_interval = 0x0c80;
>      hdev->le_conn_latency = 0x0000;
>      hdev->le_supv_timeout = 0x002a;
>      hdev->le_def_tx_len = 0x001b;

hmm, what happened with the tabs here? I needed to manually apply it.

Regards,
Andreas

2019-08-23 23:35:45

by Carey Sonsino

[permalink] [raw]
Subject: Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds

Well that is certainly a problem. According to the bluetooth core spec
(5.1), section 4.5.2 Supervision timeout:

    The connSupervisionTimeout shall be a multiple of 10 ms in the
range of 100 ms to 32.0 s and it shall be larger than (1 +
connSlaveLatency ) * connInterval * 2.

So in theory that value should be able to go up to 3200 (ms * the 10ms
multiple = 32000ms).  I see that le_supv_timeout is set to 0x2a (42
*10ms = 420ms) so that explains the value in the error you're seeing,
but I'm not sure what effect it would have to set the value to 0xc80
(3200) -- kind of seems like a bad idea.  I haven't played around with
the supervision timeout, and I'm not sure if
le_supv_timeout/supervision_timeout specifies the exact value to use or
if it's a maximum and the actual value is calculated based upon the
negotiated connection interval and slave latency as shown above.

Will have to look into that and the other issues with the patch (commit
message length & tabs).

Carey

On 8/23/19 7:45 AM, Andreas Kemnade wrote:
> Hi,
>
> On Tue, 20 Aug 2019 11:01:41 -0600
> Carey Sonsino <[email protected]> wrote:
>
>> Update the default BLE connection interval min/max bounds to the full
>> range of permitted values (6-3200, corresponding to 7.25-4000ms).
>>
>> Commit c49a8682fc5d298d44e8d911f4fa14690ea9485e introduced a bounds
>> check on connection interval update requests, but the default min/max
>> values were left at 24-40 (30-50ms) which caused problems for devices
>> that want to negotiate connection intervals outside of those bounds.
>>
>> Setting the default min/max connection interval to the full allowable
>> range in the bluetooth specification restores the default Linux behavior
>> of allowing remote devices to negotiate their desired connection
>> interval, while still permitting the system administrator to later
>> narrow the range.
>>
>> Fixes c49a8682fc5d: (validate BLE connection interval updates)
>>
> Trying pair XX:XX:XX:XX:XX:XX in bluetoothctl
> leads to create connection commands containing
> le_conn_max_interval > le_supv_timeout (4000ms > 420ms) which the
> controller does not like and is imho not allowed.
>
> < HCI Command: LE Create Connection (0x08|0x000d) plen 25
> bdaddr XX:XX:XX:XX:XX:XX type 0
> interval 96 window 96 initiator_filter 0
> own_bdaddr_type 0 min_interval 6 max_interval 3200
> latency 0 supervision_to 42 min_ce 0 max_ce 0
>> HCI Event: Command Status (0x0f) plen 4
> LE Create Connection (0x08|0x000d) status 0x12 ncmd 1
> Error: Invalid HCI Command Parameters
>
>
>> Signed-off-by: Carey Sonsino <[email protected]>
>>
>> ---
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 04bc79359a17..f4f2f712c527 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3181,8 +3181,8 @@ struct hci_dev *hci_alloc_dev(void)
>>      hdev->le_adv_max_interval = 0x0800;
>>      hdev->le_scan_interval = 0x0060;
>>      hdev->le_scan_window = 0x0030;
>> -    hdev->le_conn_min_interval = 0x0018;
>> -    hdev->le_conn_max_interval = 0x0028;
>> +    hdev->le_conn_min_interval = 0x0006;
>> +    hdev->le_conn_max_interval = 0x0c80;
>>      hdev->le_conn_latency = 0x0000;
>>      hdev->le_supv_timeout = 0x002a;
>>      hdev->le_def_tx_len = 0x001b;
> hmm, what happened with the tabs here? I needed to manually apply it.
>
> Regards,
> Andreas
>