linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] bluetooth: update default BLE connection interval bounds
@ 2019-08-20 17:01 Carey Sonsino
  2019-08-23 10:52 ` Johan Hedberg
  2019-08-23 13:45 ` Andreas Kemnade
  0 siblings, 2 replies; 4+ messages in thread
From: Carey Sonsino @ 2019-08-20 17:01 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg
  Cc: Andreas Kemnade, Jamie Mccrae, linux-bluetooth

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 <csonsino@gmail.com>

---

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;


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds
  2019-08-20 17:01 [PATCH 1/1] bluetooth: update default BLE connection interval bounds Carey Sonsino
@ 2019-08-23 10:52 ` Johan Hedberg
  2019-08-23 13:45 ` Andreas Kemnade
  1 sibling, 0 replies; 4+ messages in thread
From: Johan Hedberg @ 2019-08-23 10:52 UTC (permalink / raw)
  To: Carey Sonsino, Marcel Holtmann
  Cc: Andreas Kemnade, Jamie Mccrae, linux-bluetooth

Hi,

On 20 Aug 2019, at 20.01, Carey Sonsino <csonsino@gmail.com> 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 <csonsino@gmail.com>
> 
> ---
> 
> 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds
  2019-08-20 17:01 [PATCH 1/1] bluetooth: update default BLE connection interval bounds Carey Sonsino
  2019-08-23 10:52 ` Johan Hedberg
@ 2019-08-23 13:45 ` Andreas Kemnade
  2019-08-23 17:50   ` Carey Sonsino
  1 sibling, 1 reply; 4+ messages in thread
From: Andreas Kemnade @ 2019-08-23 13:45 UTC (permalink / raw)
  To: Carey Sonsino
  Cc: Marcel Holtmann, Johan Hedberg, Jamie Mccrae, linux-bluetooth

Hi,

On Tue, 20 Aug 2019 11:01:41 -0600
Carey Sonsino <csonsino@gmail.com> 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 <csonsino@gmail.com>
> 
> ---
> 
> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] bluetooth: update default BLE connection interval bounds
  2019-08-23 13:45 ` Andreas Kemnade
@ 2019-08-23 17:50   ` Carey Sonsino
  0 siblings, 0 replies; 4+ messages in thread
From: Carey Sonsino @ 2019-08-23 17:50 UTC (permalink / raw)
  To: Andreas Kemnade
  Cc: Marcel Holtmann, Johan Hedberg, Jamie Mccrae, linux-bluetooth

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 <csonsino@gmail.com> 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 <csonsino@gmail.com>
>>
>> ---
>>
>> 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
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-23 17:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 17:01 [PATCH 1/1] bluetooth: update default BLE connection interval bounds Carey Sonsino
2019-08-23 10:52 ` Johan Hedberg
2019-08-23 13:45 ` Andreas Kemnade
2019-08-23 17:50   ` Carey Sonsino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).