Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Reply-To: Carey Sonsino <csonsino@gmail.com>
Subject: Re: [PATCH 1/1] bluetooth: update default BLE connection interval
 bounds
To:     Andreas Kemnade <andreas@kemnade.info>
Cc:     Marcel Holtmann <marcel@holtmann.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Jamie Mccrae <Jamie.Mccrae@lairdconnect.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
References: <1c707bb9-999b-7ca3-3c91-7b183dacfdd5@gmail.com>
 <20190823154559.1f203fac@aktux>
From:   Carey Sonsino <csonsino@gmail.com>
Message-ID: <f5976cc8-736f-74d7-a8ec-c5a860e89ae0@gmail.com>
Date:   Fri, 23 Aug 2019 11:50:24 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20190823154559.1f203fac@aktux>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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
>