On Wed, Jul 29, 2015 at 5:37 PM, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
> Hi Jakub,
>
>>>> Currently, when trying to connect to already paired device that just
>>>> rotated its RPA MAC address, old address would be used and connection
>>>> would fail. In order to fix that, kernel must scan and receive
>>>> advertisement with fresh RPA before connecting.
>>>>
>>>> This patch adds hci_connect_le_scan with dependencies, new method that
>>>> will be used to connect to remote LE devices. Instead of just sending
>>>> connect request, it adds a device to whitelist. Later patches will make
>>>> use of this whitelist to send conenct request when advertisement is
>>>> received, and properly handle timeouts.
>>>>
>>>> Signed-off-by: Jakub Pawlowski <jpawlowski@xxxxxxxxxx>
>>>> ---
>>>> include/net/bluetooth/hci_core.h | 5 ++
>>>> net/bluetooth/hci_conn.c | 170 +++++++++++++++++++++++++++++++++++++++
>>>> net/bluetooth/hci_core.c | 32 ++++++++
>>>> 3 files changed, 207 insertions(+)
>>>>
>>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>>> index c8d2b5a..5d19794 100644
>>>> --- a/include/net/bluetooth/hci_core.h
>>>> +++ b/include/net/bluetooth/hci_core.h
>>>> @@ -826,6 +826,9 @@ void hci_chan_del(struct hci_chan *chan);
>>>> void hci_chan_list_flush(struct hci_conn *conn);
>>>> struct hci_chan *hci_chan_lookup_handle(struct hci_dev *hdev, __u16 handle);
>>>>
>>>> +struct hci_conn *hci_connect_le_scan(struct hci_dev *hdev, bdaddr_t *dst,
>>>> + u8 dst_type, u8 sec_level,
>>>> + u16 conn_timeout, u8 role);
>>>> struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
>>>> u8 dst_type, u8 sec_level, u16 conn_timeout,
>>>> u8 role);
>>>> @@ -991,6 +994,8 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev);
>>>> struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
>>>> bdaddr_t *addr,
>>>> u8 addr_type);
>>>> +struct hci_conn_params *hci_pend_le_explicit_connect_lookup(
>>>> + struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
>>>
>>> this is kinda violating the coding style. I wonder if we can shorten the function name or just go over 80 characters this time.
>>
>> will rename it to hci_explicit_connect_lookup
>>>
>>>> void hci_uuids_clear(struct hci_dev *hdev);
>>>>
>>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>>> index 1ba8240..2a13214 100644
>>>> --- a/net/bluetooth/hci_conn.c
>>>> +++ b/net/bluetooth/hci_conn.c
>>>> @@ -64,6 +64,36 @@ static void hci_le_create_connection_cancel(struct hci_conn *conn)
>>>> hci_send_cmd(conn->hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL);
>>>> }
>>>>
>>>> +/* This function requires the caller holds hdev->lock */
>>>> +static void hci_connect_le_scan_cleanup(struct hci_conn *conn)
>>>> +{
>>>> + struct hci_conn_params *params;
>>>> +
>>>> + params = hci_pend_le_explicit_connect_lookup(conn->hdev, &conn->dst,
>>>> + conn->dst_type);
>>>> + if (!params)
>>>> + return;
>>>> +
>>>> + /* The connection attempt was doing scan for new RPA, and is
>>>> + * in scan phase. If params are not associated with any other
>>>> + * autoconnect action, remove them completely. If they are, just unmark
>>>> + * them as awaiting for connection, by clearing explicit_connect field.
>>>> + */
>>>> + if (params->auto_connect == HCI_AUTO_CONN_EXPLICIT)
>>>> + hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
>>>> + else
>>>> + params->explicit_connect = false;
>>>> +}
>>>
>>> What happens if we have two explicit connection requests here. I mean we can have two L2CAP socket trying to connect to two peripherals at the same time.
>>>
>>
>> So right now if we have two sockets connecting to two different
>> peripherals, you'll get same behavior as before, that is first request
>> will succeed, second will fail (if they're triggered at same time and
>> connection is not established before second is attempted). I'm willing
>> to further improve this behavior in future.
>
>
> essentially we just need to add both connect requests to the auto-connect list and it will sort itself out. Which means however we need to make sure we track this correct.
>
> Regards
>
> Marcel
>
No, If you try again both sockets would use same conn object. call to
connect on each socket will result in hci_connect_le_scan. First call
to this method will create conn object, second call will re-use the
old object for second socket. But now I found a bug - if one of
sockets get closed, second will also get closed, because I don't keep
count of sockets trying to connect. I'll fix that by changing
explicit_connect from boolean to integer and increment it on each
connect attempt.
Plus I'll add a test case for that.
--
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
