[PATCH 00/17] LE auto connection

[PATCH 00/17] LE auto connection

[Andre Guedes]

Hi all,

Since the latest reviews of this "LE auto connection" series were basically
minor changes, I replaced the 'RFC' label by 'PATCH'.

This patch set differs from previous one by the hci_connect() refactoring
(Patches 5 and 6) requested in the last review.

BR,

Andre

PS: The patch "[PATCH] tools/btmgmt: Add command to set scan parameters"
(already sent to ML) can help with testing different scanning parameters.

Andre Guedes (17):
  Bluetooth: Create hci_req_add_le_scan_disable helper
  Bluetooth: Declare le_conn_failed in hci_core.h
  Bluetooth: Stop scanning on LE connection
  Bluetooth: Remove unused function
  Bluetooth: Refactor HCI connection code
  Bluetooth: Move address type conversion to outside hci_connect_le
  Bluetooth: Introduce hdev->pend_le_conn list
  Bluetooth: Introduce LE auto connection infrastructure
  Bluetooth: Introduce LE auto connect options
  Bluetooth: Connection parameters and auto connection
  Bluetooth: Temporarily stop background scanning on discovery
  Bluetooth: Auto connection and power on
  Bluetooth: Connection parameters and resolvable address
  Bluetooth: Support resolvable private addresses
  Bluetooth: Add le_auto_conn file on debugfs
  Bluetooth: Create hci_req_add_le_passive_scan helper
  Bluetooth: Update background scan parameters

 include/net/bluetooth/hci.h      |   1 +
 include/net/bluetooth/hci_core.h |  43 ++++-
 net/bluetooth/hci_conn.c         | 115 +++++++-----
 net/bluetooth/hci_core.c         | 369 +++++++++++++++++++++++++++++++++++++--
 net/bluetooth/hci_event.c        |  72 +++++++-
 net/bluetooth/l2cap_core.c       |  19 +-
 net/bluetooth/mgmt.c             |  68 +++++---
 7 files changed, 589 insertions(+), 98 deletions(-)

-- 
1.8.5.4

[PATCH 01/17] Bluetooth: Create hci_req_add_le_scan_disable helper

[Andre Guedes]

This patch moves stop LE scanning duplicate code to one single
place and reuses it. This will avoid more duplicate code in
upcoming patches.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_core.c         | 14 ++++++++++----
 net/bluetooth/mgmt.c             | 12 ++----------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 269c820..bef65d0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1129,6 +1129,8 @@ void hci_req_add_ev(struct hci_request *req, u16 opcode, u32 plen,
 		    const void *param, u8 event);
 void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
 
+void hci_req_add_le_scan_disable(struct hci_request *req);
+
 struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen,
 			       const void *param, u32 timeout);
 struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 669c76e..9a078cf 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3318,7 +3318,6 @@ static void le_scan_disable_work(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev,
 					    le_scan_disable.work);
-	struct hci_cp_le_set_scan_enable cp;
 	struct hci_request req;
 	int err;
 
@@ -3326,9 +3325,7 @@ static void le_scan_disable_work(struct work_struct *work)
 
 	hci_req_init(&req, hdev);
 
-	memset(&cp, 0, sizeof(cp));
-	cp.enable = LE_SCAN_DISABLE;
-	hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+	hci_req_add_le_scan_disable(&req);
 
 	err = hci_req_run(&req, le_scan_disable_work_complete);
 	if (err)
@@ -4872,3 +4869,12 @@ static void hci_cmd_work(struct work_struct *work)
 		}
 	}
 }
+
+void hci_req_add_le_scan_disable(struct hci_request *req)
+{
+	struct hci_cp_le_set_scan_enable cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = LE_SCAN_DISABLE;
+	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d6e2692..cfcaf97 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1052,11 +1052,7 @@ static int clean_up_hci_state(struct hci_dev *hdev)
 		disable_advertising(&req);
 
 	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
-		struct hci_cp_le_set_scan_enable cp;
-
-		memset(&cp, 0, sizeof(cp));
-		cp.enable = LE_SCAN_DISABLE;
-		hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+		hci_req_add_le_scan_disable(&req);
 	}
 
 	list_for_each_entry(conn, &hdev->conn_hash.list, list) {
@@ -3527,7 +3523,6 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 	struct hci_cp_remote_name_req_cancel cp;
 	struct inquiry_entry *e;
 	struct hci_request req;
-	struct hci_cp_le_set_scan_enable enable_cp;
 	int err;
 
 	BT_DBG("%s", hdev->name);
@@ -3563,10 +3558,7 @@ static int stop_discovery(struct sock *sk, struct hci_dev *hdev, void *data,
 		} else {
 			cancel_delayed_work(&hdev->le_scan_disable);
 
-			memset(&enable_cp, 0, sizeof(enable_cp));
-			enable_cp.enable = LE_SCAN_DISABLE;
-			hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE,
-				    sizeof(enable_cp), &enable_cp);
+			hci_req_add_le_scan_disable(&req);
 		}
 
 		break;
-- 
1.8.5.4

[PATCH 02/17] Bluetooth: Declare le_conn_failed in hci_core.h

[Andre Guedes]

This patch adds the "hci_" prefix to le_conn_failed() helper and
declares it in hci_core.h so it can be reused in hci_event.c.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 2 ++
 net/bluetooth/hci_conn.c         | 4 ++--
 net/bluetooth/hci_event.c        | 6 +-----
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index bef65d0..4253bdf 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -653,6 +653,8 @@ int hci_conn_switch_role(struct hci_conn *conn, __u8 role);
 
 void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
 
+void hci_le_conn_failed(struct hci_conn *conn, u8 status);
+
 /*
  * hci_conn_get() and hci_conn_put() are used to control the life-time of an
  * "hci_conn" object. They do not guarantee that the hci_conn object is running,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 3d6b1cf..dc8aad9 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -515,7 +515,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
 EXPORT_SYMBOL(hci_get_route);
 
 /* This function requires the caller holds hdev->lock */
-static void le_conn_failed(struct hci_conn *conn, u8 status)
+void hci_le_conn_failed(struct hci_conn *conn, u8 status)
 {
 	struct hci_dev *hdev = conn->hdev;
 
@@ -545,7 +545,7 @@ static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
 	if (!conn)
 		goto done;
 
-	le_conn_failed(conn, status);
+	hci_le_conn_failed(conn, status);
 
 done:
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 877cee8..eaa6965 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3658,11 +3658,7 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	}
 
 	if (ev->status) {
-		mgmt_connect_failed(hdev, &conn->dst, conn->type,
-				    conn->dst_type, ev->status);
-		hci_proto_connect_cfm(conn, ev->status);
-		conn->state = BT_CLOSED;
-		hci_conn_del(conn);
+		hci_le_conn_failed(conn, ev->status);
 		goto unlock;
 	}
 
-- 
1.8.5.4

[PATCH 03/17] Bluetooth: Stop scanning on LE connection

[Andre Guedes]

Some LE controllers don't support scanning and creating a connection
at the same time. So we should always stop scanning in order to
establish the connection.

Since we may prematurely stop the discovery procedure in favor of
the connection establishment, we should also cancel hdev->le_scan_
disable delayed work and set the discovery state to DISCOVERY_STOPPED.

This change does a small improvement since it is not mandatory the
user stops scanning before connecting anymore. Moreover, this change
is required by upcoming LE auto connection mechanism in order to work
properly with controllers that don't support background scanning and
connection establishment at the same time.

In future, we might want to do a small optimization by checking if
controller is able to scan and connect at the same time. For now,
we want the simplest approach so we always stop scanning (even if
the controller is able to carry out both operations).

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_conn.c    | 89 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 1bb45a4..c3834d3 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -356,6 +356,7 @@ enum {
 
 /* ---- HCI Error Codes ---- */
 #define HCI_ERROR_AUTH_FAILURE		0x05
+#define HCI_ERROR_MEMORY_EXCEEDED	0x07
 #define HCI_ERROR_CONNECTION_TIMEOUT	0x08
 #define HCI_ERROR_REJ_BAD_ADDR		0x0f
 #define HCI_ERROR_REMOTE_USER_TERM	0x13
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index dc8aad9..ae2c3e1 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -594,12 +594,83 @@ static int hci_create_le_conn(struct hci_conn *conn)
 	return 0;
 }
 
+static void hci_req_add_le_create_conn(struct hci_request *req,
+				       struct hci_conn *conn)
+{
+	struct hci_cp_le_create_conn cp;
+	struct hci_dev *hdev = conn->hdev;
+	u8 own_addr_type;
+
+	memset(&cp, 0, sizeof(cp));
+
+	/* Update random address, but set require_privacy to false so
+	 * that we never connect with an unresolvable address.
+	 */
+	if (hci_update_random_address(req, false, &own_addr_type))
+		return;
+
+	conn->src_type = own_addr_type;
+
+	cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
+	cp.scan_window = cpu_to_le16(hdev->le_scan_window);
+	bacpy(&cp.peer_addr, &conn->dst);
+	cp.peer_addr_type = conn->dst_type;
+	cp.own_address_type = conn->src_type;
+	cp.conn_interval_min = cpu_to_le16(conn->le_conn_min_interval);
+	cp.conn_interval_max = cpu_to_le16(conn->le_conn_max_interval);
+	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
+	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
+	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
+
+	hci_req_add(req, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
+}
+
+static void stop_scan_complete(struct hci_dev *hdev, u8 status)
+{
+	struct hci_request req;
+	struct hci_conn *conn;
+	int err;
+
+	conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+	if (!conn)
+		return;
+
+	if (status) {
+		BT_DBG("HCI request failed to stop scanning: status 0x%2.2x",
+		       status);
+
+		hci_dev_lock(hdev);
+		hci_le_conn_failed(conn, status);
+		hci_dev_unlock(hdev);
+		return;
+	}
+
+	/* Since we may have prematurely stopped discovery procedure, we should
+	 * update discovery state.
+	 */
+	cancel_delayed_work(&hdev->le_scan_disable);
+	hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+
+	hci_req_init(&req, hdev);
+
+	hci_req_add_le_create_conn(&req, conn);
+
+	err = hci_req_run(&req, create_le_conn_complete);
+	if (err) {
+		hci_dev_lock(hdev);
+		hci_le_conn_failed(conn, HCI_ERROR_MEMORY_EXCEEDED);
+		hci_dev_unlock(hdev);
+		return;
+	}
+}
+
 static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 				    u8 dst_type, u8 sec_level, u8 auth_type)
 {
 	struct hci_conn_params *params;
 	struct hci_conn *conn;
 	struct smp_irk *irk;
+	struct hci_request req;
 	int err;
 
 	if (test_bit(HCI_ADVERTISING, &hdev->flags))
@@ -675,9 +746,23 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 		conn->le_conn_max_interval = hdev->le_conn_max_interval;
 	}
 
-	err = hci_create_le_conn(conn);
-	if (err)
+	hci_req_init(&req, hdev);
+
+	/* If controller is scanning, we stop it since some controllers are
+	 * not able to scan and connect at the same time.
+	 */
+	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
+		hci_req_add_le_scan_disable(&req);
+		err = hci_req_run(&req, stop_scan_complete);
+	} else {
+		hci_req_add_le_create_conn(&req, conn);
+		err = hci_req_run(&req, create_le_conn_complete);
+	}
+
+	if (err) {
+		hci_conn_del(conn);
 		return ERR_PTR(err);
+	}
 
 done:
 	hci_conn_hold(conn);
-- 
1.8.5.4

Re: [PATCH 03/17] Bluetooth: Stop scanning on LE connection

[Marcel Holtmann]

Hi Andre,

> Some LE controllers don't support scanning and creating a connection
> at the same time. So we should always stop scanning in order to
> establish the connection.
> 
> Since we may prematurely stop the discovery procedure in favor of
> the connection establishment, we should also cancel hdev->le_scan_
> disable delayed work and set the discovery state to DISCOVERY_STOPPED.
> 
> This change does a small improvement since it is not mandatory the
> user stops scanning before connecting anymore. Moreover, this change
> is required by upcoming LE auto connection mechanism in order to work
> properly with controllers that don't support background scanning and
> connection establishment at the same time.
> 
> In future, we might want to do a small optimization by checking if
> controller is able to scan and connect at the same time. For now,
> we want the simplest approach so we always stop scanning (even if
> the controller is able to carry out both operations).
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_conn.c    | 89 ++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 1bb45a4..c3834d3 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -356,6 +356,7 @@ enum {
> 
> /* ---- HCI Error Codes ---- */
> #define HCI_ERROR_AUTH_FAILURE		0x05
> +#define HCI_ERROR_MEMORY_EXCEEDED	0x07
> #define HCI_ERROR_CONNECTION_TIMEOUT	0x08
> #define HCI_ERROR_REJ_BAD_ADDR		0x0f
> #define HCI_ERROR_REMOTE_USER_TERM	0x13
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index dc8aad9..ae2c3e1 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -594,12 +594,83 @@ static int hci_create_le_conn(struct hci_conn *conn)
> 	return 0;
> }
> 
> +static void hci_req_add_le_create_conn(struct hci_request *req,
> +				       struct hci_conn *conn)
> +{
> +	struct hci_cp_le_create_conn cp;
> +	struct hci_dev *hdev = conn->hdev;
> +	u8 own_addr_type;
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	/* Update random address, but set require_privacy to false so
> +	 * that we never connect with an unresolvable address.
> +	 */
> +	if (hci_update_random_address(req, false, &own_addr_type))
> +		return;
> +
> +	conn->src_type = own_addr_type;
> +
> +	cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
> +	cp.scan_window = cpu_to_le16(hdev->le_scan_window);
> +	bacpy(&cp.peer_addr, &conn->dst);
> +	cp.peer_addr_type = conn->dst_type;
> +	cp.own_address_type = conn->src_type;

the reason why you get the own_addr_type when setting the random address is to actually use it here.

This is important since in cases where LE Privacy is enabled and we are using RPA, we want the random address used.

Regards

Marcel

Re: [PATCH 03/17] Bluetooth: Stop scanning on LE connection

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:23 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> Some LE controllers don't support scanning and creating a connection
>> at the same time. So we should always stop scanning in order to
>> establish the connection.
>>
>> Since we may prematurely stop the discovery procedure in favor of
>> the connection establishment, we should also cancel hdev->le_scan_
>> disable delayed work and set the discovery state to DISCOVERY_STOPPED.
>>
>> This change does a small improvement since it is not mandatory the
>> user stops scanning before connecting anymore. Moreover, this change
>> is required by upcoming LE auto connection mechanism in order to work
>> properly with controllers that don't support background scanning and
>> connection establishment at the same time.
>>
>> In future, we might want to do a small optimization by checking if
>> controller is able to scan and connect at the same time. For now,
>> we want the simplest approach so we always stop scanning (even if
>> the controller is able to carry out both operations).
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci.h |  1 +
>> net/bluetooth/hci_conn.c    | 89 ++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 88 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>> index 1bb45a4..c3834d3 100644
>> --- a/include/net/bluetooth/hci.h
>> +++ b/include/net/bluetooth/hci.h
>> @@ -356,6 +356,7 @@ enum {
>>
>> /* ---- HCI Error Codes ---- */
>> #define HCI_ERROR_AUTH_FAILURE                0x05
>> +#define HCI_ERROR_MEMORY_EXCEEDED    0x07
>> #define HCI_ERROR_CONNECTION_TIMEOUT  0x08
>> #define HCI_ERROR_REJ_BAD_ADDR                0x0f
>> #define HCI_ERROR_REMOTE_USER_TERM    0x13
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index dc8aad9..ae2c3e1 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -594,12 +594,83 @@ static int hci_create_le_conn(struct hci_conn *conn)
>>       return 0;
>> }
>>
>> +static void hci_req_add_le_create_conn(struct hci_request *req,
>> +                                    struct hci_conn *conn)
>> +{
>> +     struct hci_cp_le_create_conn cp;
>> +     struct hci_dev *hdev = conn->hdev;
>> +     u8 own_addr_type;
>> +
>> +     memset(&cp, 0, sizeof(cp));
>> +
>> +     /* Update random address, but set require_privacy to false so
>> +      * that we never connect with an unresolvable address.
>> +      */
>> +     if (hci_update_random_address(req, false, &own_addr_type))
>> +             return;
>> +
>> +     conn->src_type = own_addr_type;
>> +
>> +     cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
>> +     cp.scan_window = cpu_to_le16(hdev->le_scan_window);
>> +     bacpy(&cp.peer_addr, &conn->dst);
>> +     cp.peer_addr_type = conn->dst_type;
>> +     cp.own_address_type = conn->src_type;
>
> the reason why you get the own_addr_type when setting the random address is to actually use it here.
>
> This is important since in cases where LE Privacy is enabled and we are using RPA, we want the random address used.

Year, I'm aware of it. 'own_addr_type' is assigned to
'conn->src_type'. Then, 'conn->src_type' is used in
'cp.own_address_type'.

IOW:
        conn->src_type = own_addr_type;
        ...
        cp.own_address_type = conn->src_type;

Or am I missing something?

BR,

Andre

Re: [PATCH 03/17] Bluetooth: Stop scanning on LE connection

[Marcel Holtmann]

Hi Andre,

>>> Some LE controllers don't support scanning and creating a connection
>>> at the same time. So we should always stop scanning in order to
>>> establish the connection.
>>> 
>>> Since we may prematurely stop the discovery procedure in favor of
>>> the connection establishment, we should also cancel hdev->le_scan_
>>> disable delayed work and set the discovery state to DISCOVERY_STOPPED.
>>> 
>>> This change does a small improvement since it is not mandatory the
>>> user stops scanning before connecting anymore. Moreover, this change
>>> is required by upcoming LE auto connection mechanism in order to work
>>> properly with controllers that don't support background scanning and
>>> connection establishment at the same time.
>>> 
>>> In future, we might want to do a small optimization by checking if
>>> controller is able to scan and connect at the same time. For now,
>>> we want the simplest approach so we always stop scanning (even if
>>> the controller is able to carry out both operations).
>>> 
>>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>>> ---
>>> include/net/bluetooth/hci.h |  1 +
>>> net/bluetooth/hci_conn.c    | 89 ++++++++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 88 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index 1bb45a4..c3834d3 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -356,6 +356,7 @@ enum {
>>> 
>>> /* ---- HCI Error Codes ---- */
>>> #define HCI_ERROR_AUTH_FAILURE                0x05
>>> +#define HCI_ERROR_MEMORY_EXCEEDED    0x07
>>> #define HCI_ERROR_CONNECTION_TIMEOUT  0x08
>>> #define HCI_ERROR_REJ_BAD_ADDR                0x0f
>>> #define HCI_ERROR_REMOTE_USER_TERM    0x13
>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>> index dc8aad9..ae2c3e1 100644
>>> --- a/net/bluetooth/hci_conn.c
>>> +++ b/net/bluetooth/hci_conn.c
>>> @@ -594,12 +594,83 @@ static int hci_create_le_conn(struct hci_conn *conn)
>>>      return 0;
>>> }
>>> 
>>> +static void hci_req_add_le_create_conn(struct hci_request *req,
>>> +                                    struct hci_conn *conn)
>>> +{
>>> +     struct hci_cp_le_create_conn cp;
>>> +     struct hci_dev *hdev = conn->hdev;
>>> +     u8 own_addr_type;
>>> +
>>> +     memset(&cp, 0, sizeof(cp));
>>> +
>>> +     /* Update random address, but set require_privacy to false so
>>> +      * that we never connect with an unresolvable address.
>>> +      */
>>> +     if (hci_update_random_address(req, false, &own_addr_type))
>>> +             return;
>>> +
>>> +     conn->src_type = own_addr_type;
>>> +
>>> +     cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
>>> +     cp.scan_window = cpu_to_le16(hdev->le_scan_window);
>>> +     bacpy(&cp.peer_addr, &conn->dst);
>>> +     cp.peer_addr_type = conn->dst_type;
>>> +     cp.own_address_type = conn->src_type;
>> 
>> the reason why you get the own_addr_type when setting the random address is to actually use it here.
>> 
>> This is important since in cases where LE Privacy is enabled and we are using RPA, we want the random address used.
> 
> Year, I'm aware of it. 'own_addr_type' is assigned to
> 'conn->src_type'. Then, 'conn->src_type' is used in
> 'cp.own_address_type'.
> 
> IOW:
>        conn->src_type = own_addr_type;
>        ...
>        cp.own_address_type = conn->src_type;
> 
> Or am I missing something?

the own_addr_type refers to the identity address. That is either the public address or a static random address. Based on which one it is the identity address type aka own_addr_type is either 0x00 or 0x01.

However in the case of enabled privacy, the address type used for HCI commands will be always 0x01 since we are always using the resolvable private address. The remote side can resolve this random address later back into our identity address.

So while our identity address type might be 0x00, then address type used on HCI will be 0x01 since we are using a resolvable address there.

Even while we would be using the RPA to establish a connection, internally we track it with the identity address and identity address type. So every notification send over sockets or mgmt will contain the identity address and not the random address.

Meaning whatever you get out of hci_update_random_address function is the address type you need to give to the HCI command.

Regards

Marcel

Re: [PATCH 03/17] Bluetooth: Stop scanning on LE connection

[Johan Hedberg]

Hi Marcel,

On Wed, Feb 26, 2014, Marcel Holtmann wrote:
> >>> +static void hci_req_add_le_create_conn(struct hci_request *req,
> >>> +                                    struct hci_conn *conn)
> >>> +{
> >>> +     struct hci_cp_le_create_conn cp;
> >>> +     struct hci_dev *hdev = conn->hdev;
> >>> +     u8 own_addr_type;
> >>> +
> >>> +     memset(&cp, 0, sizeof(cp));
> >>> +
> >>> +     /* Update random address, but set require_privacy to false so
> >>> +      * that we never connect with an unresolvable address.
> >>> +      */
> >>> +     if (hci_update_random_address(req, false, &own_addr_type))
> >>> +             return;
> >>> +
> >>> +     conn->src_type = own_addr_type;
> >>> +
> >>> +     cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
> >>> +     cp.scan_window = cpu_to_le16(hdev->le_scan_window);
> >>> +     bacpy(&cp.peer_addr, &conn->dst);
> >>> +     cp.peer_addr_type = conn->dst_type;
> >>> +     cp.own_address_type = conn->src_type;
> >> 
> >> the reason why you get the own_addr_type when setting the random
> >> address is to actually use it here.
> >> 
> >> This is important since in cases where LE Privacy is enabled and we
> >> are using RPA, we want the random address used.
> > 
> > Year, I'm aware of it. 'own_addr_type' is assigned to
> > 'conn->src_type'. Then, 'conn->src_type' is used in
> > 'cp.own_address_type'.
> > 
> > IOW:
> >        conn->src_type = own_addr_type;
> >        ...
> >        cp.own_address_type = conn->src_type;
> > 
> > Or am I missing something?
> 
> the own_addr_type refers to the identity address.

No, it refers to whatever we want to pass to the HCI command.

> That is either the public address or a static random address. Based on
> which one it is the identity address type aka own_addr_type is either
> 0x00 or 0x01.

What's a bit confusing here is that in all other places we assign the
returned value directly to cp.own_addr_type and the hci_create_le_conn
function always takes care of updating the conn->src_type value. Now
that the hci_create_le_conn function gets removed in the patch set
however this new function becomes responsible for updating
conn->src_type. Nevertheless, I'd keep the original style from
hci_create_le_conn which assigns the same own_addr_type variable to both
conn->src_type as well as cp.own_addr_type.

In general what would help a lot in avoiding this kind of confusion is
to have proper code comments explaining what we're doing and more
importantly why we're doing it. The essential part here is that we track
the type used for HCI_LE_Create_Connection as long as the connection
attempt is in progress and update it to match the Identity Address as
soon as the connection is successful.

Johan

[PATCH 04/17] Bluetooth: Remove unused function

[Andre Guedes]

This patch removes hci_create_le_conn() since it is not used anymore.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c | 43 -------------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ae2c3e1..a823d30 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -551,49 +551,6 @@ done:
 	hci_dev_unlock(hdev);
 }
 
-static int hci_create_le_conn(struct hci_conn *conn)
-{
-	struct hci_dev *hdev = conn->hdev;
-	struct hci_cp_le_create_conn cp;
-	struct hci_request req;
-	u8 own_addr_type;
-	int err;
-
-	hci_req_init(&req, hdev);
-
-	memset(&cp, 0, sizeof(cp));
-
-	/* Update random address, but set require_privacy to false so
-	 * that we never connect with an unresolvable address.
-	 */
-	err = hci_update_random_address(&req, false, &own_addr_type);
-	if (err < 0)
-		return err;
-
-	conn->src_type = own_addr_type;
-
-	cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
-	cp.scan_window = cpu_to_le16(hdev->le_scan_window);
-	bacpy(&cp.peer_addr, &conn->dst);
-	cp.peer_addr_type = conn->dst_type;
-	cp.own_address_type = own_addr_type;
-	cp.conn_interval_min = cpu_to_le16(conn->le_conn_min_interval);
-	cp.conn_interval_max = cpu_to_le16(conn->le_conn_max_interval);
-	cp.supervision_timeout = __constant_cpu_to_le16(0x002a);
-	cp.min_ce_len = __constant_cpu_to_le16(0x0000);
-	cp.max_ce_len = __constant_cpu_to_le16(0x0000);
-
-	hci_req_add(&req, HCI_OP_LE_CREATE_CONN, sizeof(cp), &cp);
-
-	err = hci_req_run(&req, create_le_conn_complete);
-	if (err) {
-		hci_conn_del(conn);
-		return err;
-	}
-
-	return 0;
-}
-
 static void hci_req_add_le_create_conn(struct hci_request *req,
 				       struct hci_conn *conn)
 {
-- 
1.8.5.4

[PATCH 05/17] Bluetooth: Refactor HCI connection code

[Andre Guedes]

hci_connect() is a very simple and useless wrapper of hci_connect_acl
and hci_connect_le functions. Addtionally, all places where hci_connect
is called the link type value is passed explicitly. This way, we can
safely delete hci_connect, declare hci_connect_acl and hci_connect_le
in hci_core.h and call them directly.

No functionality is changed by this patch.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  6 ++++--
 net/bluetooth/hci_conn.c         | 24 ++++--------------------
 net/bluetooth/l2cap_core.c       |  7 +++----
 net/bluetooth/mgmt.c             |  8 ++++----
 4 files changed, 15 insertions(+), 30 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4253bdf..20bdb2e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -641,8 +641,10 @@ 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(struct hci_dev *hdev, int type, bdaddr_t *dst,
-			     __u8 dst_type, __u8 sec_level, __u8 auth_type);
+struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
+				u8 dst_type, u8 sec_level, u8 auth_type);
+struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
+				 u8 sec_level, u8 auth_type);
 struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
 				 __u16 setting);
 int hci_conn_check_link_mode(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index a823d30..7fbcc7d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -621,8 +621,8 @@ static void stop_scan_complete(struct hci_dev *hdev, u8 status)
 	}
 }
 
-static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
-				    u8 dst_type, u8 sec_level, u8 auth_type)
+struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
+				u8 dst_type, u8 sec_level, u8 auth_type)
 {
 	struct hci_conn_params *params;
 	struct hci_conn *conn;
@@ -726,8 +726,8 @@ done:
 	return conn;
 }
 
-static struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
-						u8 sec_level, u8 auth_type)
+struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
+				 u8 sec_level, u8 auth_type)
 {
 	struct hci_conn *acl;
 
@@ -796,22 +796,6 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
 	return sco;
 }
 
-/* Create SCO, ACL or LE connection. */
-struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
-			     __u8 dst_type, __u8 sec_level, __u8 auth_type)
-{
-	BT_DBG("%s dst %pMR type 0x%x", hdev->name, dst, type);
-
-	switch (type) {
-	case LE_LINK:
-		return hci_connect_le(hdev, dst, dst_type, sec_level, auth_type);
-	case ACL_LINK:
-		return hci_connect_acl(hdev, dst, sec_level, auth_type);
-	}
-
-	return ERR_PTR(-EINVAL);
-}
-
 /* Check link security requirement */
 int hci_conn_check_link_mode(struct hci_conn *conn)
 {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7bd78c5..308945e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7109,11 +7109,10 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 	auth_type = l2cap_get_auth_type(chan);
 
 	if (bdaddr_type_is_le(dst_type))
-		hcon = hci_connect(hdev, LE_LINK, dst, dst_type,
-				   chan->sec_level, auth_type);
+		hcon = hci_connect_le(hdev, dst, dst_type, chan->sec_level,
+				      auth_type);
 	else
-		hcon = hci_connect(hdev, ACL_LINK, dst, dst_type,
-				   chan->sec_level, auth_type);
+		hcon = hci_connect_acl(hdev, dst, chan->sec_level, auth_type);
 
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index cfcaf97..9fc7c1d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2816,11 +2816,11 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		auth_type = HCI_AT_DEDICATED_BONDING_MITM;
 
 	if (cp->addr.type == BDADDR_BREDR)
-		conn = hci_connect(hdev, ACL_LINK, &cp->addr.bdaddr,
-				   cp->addr.type, sec_level, auth_type);
+		conn = hci_connect_acl(hdev, &cp->addr.bdaddr, sec_level,
+				       auth_type);
 	else
-		conn = hci_connect(hdev, LE_LINK, &cp->addr.bdaddr,
-				   cp->addr.type, sec_level, auth_type);
+		conn = hci_connect_le(hdev, &cp->addr.bdaddr, cp->addr.type,
+				      sec_level, auth_type);
 
 	if (IS_ERR(conn)) {
 		int status;
-- 
1.8.5.4

[PATCH 06/17] Bluetooth: Move address type conversion to outside hci_connect_le

[Andre Guedes]

This patch moves address type conversion (L2CAP address type to HCI
address type) to outside hci_connect_le. This way, we avoid back and
forth address type conversion in a comming patch.

So hci_connect_le() now expects 'dst_type' parameter in HCI address
type convention.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c   |  6 ------
 net/bluetooth/l2cap_core.c | 12 ++++++++++--
 net/bluetooth/mgmt.c       | 16 +++++++++++++---
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 7fbcc7d..ccf4a4f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -656,12 +656,6 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	if (conn)
 		return ERR_PTR(-EBUSY);
 
-	/* Convert from L2CAP channel address type to HCI address type */
-	if (dst_type == BDADDR_LE_PUBLIC)
-		dst_type = ADDR_LE_DEV_PUBLIC;
-	else
-		dst_type = ADDR_LE_DEV_RANDOM;
-
 	/* When given an identity address with existing identity
 	 * resolving key, the connection needs to be established
 	 * to a resolvable random address.
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 308945e..9ce386a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -7108,11 +7108,19 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 
 	auth_type = l2cap_get_auth_type(chan);
 
-	if (bdaddr_type_is_le(dst_type))
+	if (bdaddr_type_is_le(dst_type)) {
+		/* Convert from L2CAP channel address type to HCI address type
+		 */
+		if (dst_type == BDADDR_LE_PUBLIC)
+			dst_type = ADDR_LE_DEV_PUBLIC;
+		else
+			dst_type = ADDR_LE_DEV_RANDOM;
+
 		hcon = hci_connect_le(hdev, dst, dst_type, chan->sec_level,
 				      auth_type);
-	else
+	} else {
 		hcon = hci_connect_acl(hdev, dst, chan->sec_level, auth_type);
+	}
 
 	if (IS_ERR(hcon)) {
 		err = PTR_ERR(hcon);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9fc7c1d..bad23d5 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2815,12 +2815,22 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	else
 		auth_type = HCI_AT_DEDICATED_BONDING_MITM;
 
-	if (cp->addr.type == BDADDR_BREDR)
+	if (cp->addr.type == BDADDR_BREDR) {
 		conn = hci_connect_acl(hdev, &cp->addr.bdaddr, sec_level,
 				       auth_type);
-	else
-		conn = hci_connect_le(hdev, &cp->addr.bdaddr, cp->addr.type,
+	} else {
+		u8 addr_type;
+
+		/* Convert from L2CAP channel address type to HCI address type
+		 */
+		if (cp->addr.type == BDADDR_LE_PUBLIC)
+			addr_type = ADDR_LE_DEV_PUBLIC;
+		else
+			addr_type = ADDR_LE_DEV_RANDOM;
+
+		conn = hci_connect_le(hdev, &cp->addr.bdaddr, addr_type,
 				      sec_level, auth_type);
+	}
 
 	if (IS_ERR(conn)) {
 		int status;
-- 
1.8.5.4

[PATCH 07/17] Bluetooth: Introduce hdev->pend_le_conn list

[Andre Guedes]

This patch introduces the hdev->pend_le_conn list which holds the
device addresses the kernel should autonomously connect. It also
introduces some helper functions to manipulate the list.

The list and helper functions will be used by the next patch which
implements the LE auto connection infrastructure.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  7 +++++
 net/bluetooth/hci_core.c         | 68 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 20bdb2e..e08405d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -284,6 +284,7 @@ struct hci_dev {
 	struct list_head	identity_resolving_keys;
 	struct list_head	remote_oob_data;
 	struct list_head	le_conn_params;
+	struct list_head	pend_le_conns;
 
 	struct hci_dev_stats	stat;
 
@@ -799,6 +800,12 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_conn_params_clear(struct hci_dev *hdev);
 
+struct bdaddr_list *hci_pend_le_conn_lookup(struct hci_dev *hdev,
+					    bdaddr_t *addr, u8 addr_type);
+void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
+void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
+void hci_pend_le_conns_clear(struct hci_dev *hdev);
+
 void hci_uuids_clear(struct hci_dev *hdev);
 
 void hci_link_keys_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9a078cf..142ecd8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3259,6 +3259,72 @@ void hci_conn_params_clear(struct hci_dev *hdev)
 	BT_DBG("All LE connection parameters were removed");
 }
 
+/* This function requires the caller holds hdev->lock */
+struct bdaddr_list *hci_pend_le_conn_lookup(struct hci_dev *hdev,
+					    bdaddr_t *addr, u8 addr_type)
+{
+	struct bdaddr_list *entry;
+
+	list_for_each_entry(entry, &hdev->pend_le_conns, list) {
+		if (bacmp(&entry->bdaddr, addr) == 0 &&
+		    entry->bdaddr_type == addr_type)
+			return entry;
+	}
+
+	return NULL;
+}
+
+/* This function requires the caller holds hdev->lock */
+void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
+{
+	struct bdaddr_list *entry;
+
+	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
+	if (entry)
+		return;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry) {
+		BT_ERR("Out of memory");
+		return;
+	}
+
+	bacpy(&entry->bdaddr, addr);
+	entry->bdaddr_type = addr_type;
+
+	list_add(&entry->list, &hdev->pend_le_conns);
+
+	BT_DBG("addr %pMR (type %u)", addr, addr_type);
+}
+
+/* This function requires the caller holds hdev->lock */
+void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
+{
+	struct bdaddr_list *entry;
+
+	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
+	if (!entry)
+		return;
+
+	list_del(&entry->list);
+	kfree(entry);
+
+	BT_DBG("addr %pMR (type %u)", addr, addr_type);
+}
+
+/* This function requires the caller holds hdev->lock */
+void hci_pend_le_conns_clear(struct hci_dev *hdev)
+{
+	struct bdaddr_list *entry, *tmp;
+
+	list_for_each_entry_safe(entry, tmp, &hdev->pend_le_conns, list) {
+		list_del(&entry->list);
+		kfree(entry);
+	}
+
+	BT_DBG("All LE pending connections cleared");
+}
+
 static void inquiry_complete(struct hci_dev *hdev, u8 status)
 {
 	if (status) {
@@ -3441,6 +3507,7 @@ struct hci_dev *hci_alloc_dev(void)
 	INIT_LIST_HEAD(&hdev->identity_resolving_keys);
 	INIT_LIST_HEAD(&hdev->remote_oob_data);
 	INIT_LIST_HEAD(&hdev->le_conn_params);
+	INIT_LIST_HEAD(&hdev->pend_le_conns);
 	INIT_LIST_HEAD(&hdev->conn_hash.list);
 
 	INIT_WORK(&hdev->rx_work, hci_rx_work);
@@ -3642,6 +3709,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	hci_smp_irks_clear(hdev);
 	hci_remote_oob_data_clear(hdev);
 	hci_conn_params_clear(hdev);
+	hci_pend_le_conns_clear(hdev);
 	hci_dev_unlock(hdev);
 
 	hci_dev_put(hdev);
-- 
1.8.5.4

[PATCH 08/17] Bluetooth: Introduce LE auto connection infrastructure

[Andre Guedes]

This patch introduces the LE auto connection infrastructure which
will be used to implement the LE auto connection options.

In summary, the auto connection mechanism works as follows: Once the
first pending LE connection is created, the background scanning is
started. When the target device is found in range, the kernel
autonomously starts the connection attempt. If connection is
established successfully, that pending LE connection is deleted and
the background is stopped.

To achieve that, this patch introduces the hci_update_background_scan()
which controls the background scanning state. This function starts or
stops the background scanning based on the hdev->pend_le_conns list. If
there is no pending LE connection, the background scanning is stopped.
Otherwise, we start the background scanning.

Then, every time a pending LE connection is added we call hci_update_
background_scan() so the background scanning is started (in case it is
not already running). Likewise, every time a pending LE connection is
deleted we call hci_update_background_scan() so the background scanning
is stopped (in case this was the last pending LE connection) or it is
started again (in case we have more pending LE connections). Finally,
we also call hci_update_background_scan() in hci_le_conn_failed() so
the background scan is restarted in case the connection establishment
fails. This way the background scanning keeps running until all pending
LE connection are established.

At this point, resolvable addresses are not support by this
infrastructure. The proper support is added in upcoming patches.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  2 +
 net/bluetooth/hci_conn.c         |  5 +++
 net/bluetooth/hci_core.c         | 93 +++++++++++++++++++++++++++++++++++++++-
 net/bluetooth/hci_event.c        | 38 ++++++++++++++++
 4 files changed, 136 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e08405d..617cf49 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -806,6 +806,8 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_pend_le_conns_clear(struct hci_dev *hdev);
 
+void hci_update_background_scan(struct hci_dev *hdev);
+
 void hci_uuids_clear(struct hci_dev *hdev);
 
 void hci_link_keys_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ccf4a4f..e79351c 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -527,6 +527,11 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
 	hci_proto_connect_cfm(conn, status);
 
 	hci_conn_del(conn);
+
+	/* Since we may have temporarily stopped the background scanning in
+	 * favor of connection establishment, we should restart it.
+	 */
+	hci_update_background_scan(hdev);
 }
 
 static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 142ecd8..d242217 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3281,7 +3281,7 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
 
 	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
 	if (entry)
-		return;
+		goto done;
 
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry) {
@@ -3295,6 +3295,9 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
 	list_add(&entry->list, &hdev->pend_le_conns);
 
 	BT_DBG("addr %pMR (type %u)", addr, addr_type);
+
+done:
+	hci_update_background_scan(hdev);
 }
 
 /* This function requires the caller holds hdev->lock */
@@ -3304,12 +3307,15 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
 
 	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
 	if (!entry)
-		return;
+		goto done;
 
 	list_del(&entry->list);
 	kfree(entry);
 
 	BT_DBG("addr %pMR (type %u)", addr, addr_type);
+
+done:
+	hci_update_background_scan(hdev);
 }
 
 /* This function requires the caller holds hdev->lock */
@@ -4946,3 +4952,86 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 	cp.enable = LE_SCAN_DISABLE;
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 }
+
+static void update_background_scan_complete(struct hci_dev *hdev, u8 status)
+{
+	if (status)
+		BT_DBG("HCI request failed to update background scanning: "
+		       "status 0x%2.2x", status);
+}
+
+/* This function controls the background scanning based on hdev->pend_le_conns
+ * list. If there are pending LE connection we start the background scanning,
+ * otherwise we stop it.
+ *
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_update_background_scan(struct hci_dev *hdev)
+{
+	struct hci_cp_le_set_scan_param param_cp;
+	struct hci_cp_le_set_scan_enable enable_cp;
+	struct hci_request req;
+	struct hci_conn *conn;
+	int err;
+
+	hci_req_init(&req, hdev);
+
+	if (list_empty(&hdev->pend_le_conns)) {
+		/* If there is no pending LE connections, we should stop
+		 * the background scanning.
+		 */
+
+		/* If controller is not scanning we are done. */
+		if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+			return;
+
+		hci_req_add_le_scan_disable(&req);
+
+		BT_DBG("%s stopping background scanning", hdev->name);
+	} else {
+		u8 own_addr_type;
+
+		/* If there is at least one pending LE connection, we should
+		 * keep the background scan running.
+		 */
+
+		/* If controller is already scanning we are done. */
+		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+			return;
+
+		/* If controller is connecting, we should not start scanning
+		 * since some controllers are not able to scan and connect at
+		 * the same time.
+		 */
+		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
+		if (conn)
+			return;
+
+		/* Use private address since remote doesn't need to identify us.
+		 * Strictly speaking, this is not required since no SCAN_REQ is
+		 * sent in passive scanning.
+		 */
+		if (hci_update_random_address(&req, true, &own_addr_type))
+			return;
+
+		memset(&param_cp, 0, sizeof(param_cp));
+		param_cp.type = LE_SCAN_PASSIVE;
+		param_cp.interval = cpu_to_le16(hdev->le_scan_interval);
+		param_cp.window = cpu_to_le16(hdev->le_scan_window);
+		param_cp.own_address_type = own_addr_type;
+		hci_req_add(&req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
+			    &param_cp);
+
+		memset(&enable_cp, 0, sizeof(enable_cp));
+		enable_cp.enable = LE_SCAN_ENABLE;
+		enable_cp.filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
+		hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
+			    &enable_cp);
+
+		BT_DBG("%s starting background scanning", hdev->name);
+	}
+
+	err = hci_req_run(&req, update_background_scan_complete);
+	if (err)
+		BT_ERR("Failed to run HCI request: err %d", err);
+}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index eaa6965..b6631d7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3677,25 +3677,63 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hci_proto_connect_cfm(conn, ev->status);
 
+	hci_pend_le_conn_del(hdev, &conn->dst, conn->dst_type);
+
 unlock:
 	hci_dev_unlock(hdev);
 }
 
+/* This function requires the caller holds hdev->lock */
+static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
+				  u8 addr_type)
+{
+	struct hci_conn *conn;
+
+	if (!hci_pend_le_conn_lookup(hdev, addr, addr_type))
+		return;
+
+	conn = hci_connect_le(hdev, addr, addr_type, BT_SECURITY_LOW,
+			      HCI_AT_NO_BONDING);
+	if (!IS_ERR(conn))
+		return;
+
+	switch (PTR_ERR(conn)) {
+	case -EBUSY:
+		/* If hci_connect() returns -EBUSY it means there is already
+		 * an LE connection attempt going on. Since controllers don't
+		 * support more than one connection attempt at the time, we
+		 * don't consider this an error case.
+		 */
+		break;
+	default:
+		BT_DBG("Failed to connect: err %ld", PTR_ERR(conn));
+	}
+}
+
 static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	u8 num_reports = skb->data[0];
 	void *ptr = &skb->data[1];
 	s8 rssi;
 
+	hci_dev_lock(hdev);
+
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
 
+		if (ev->evt_type == LE_ADV_IND ||
+		    ev->evt_type == LE_ADV_DIRECT_IND)
+			check_pending_le_conn(hdev, &ev->bdaddr,
+					      ev->bdaddr_type);
+
 		rssi = ev->data[ev->length];
 		mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
 				  NULL, rssi, 0, 1, ev->data, ev->length);
 
 		ptr += sizeof(*ev) + ev->length + 1;
 	}
+
+	hci_dev_unlock(hdev);
 }
 
 static void hci_le_ltk_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
-- 
1.8.5.4

Re: [PATCH 08/17] Bluetooth: Introduce LE auto connection infrastructure

[Marcel Holtmann]

Hi Andre,

> This patch introduces the LE auto connection infrastructure which
> will be used to implement the LE auto connection options.
> 
> In summary, the auto connection mechanism works as follows: Once the
> first pending LE connection is created, the background scanning is
> started. When the target device is found in range, the kernel
> autonomously starts the connection attempt. If connection is
> established successfully, that pending LE connection is deleted and
> the background is stopped.
> 
> To achieve that, this patch introduces the hci_update_background_scan()
> which controls the background scanning state. This function starts or
> stops the background scanning based on the hdev->pend_le_conns list. If
> there is no pending LE connection, the background scanning is stopped.
> Otherwise, we start the background scanning.
> 
> Then, every time a pending LE connection is added we call hci_update_
> background_scan() so the background scanning is started (in case it is
> not already running). Likewise, every time a pending LE connection is
> deleted we call hci_update_background_scan() so the background scanning
> is stopped (in case this was the last pending LE connection) or it is
> started again (in case we have more pending LE connections). Finally,
> we also call hci_update_background_scan() in hci_le_conn_failed() so
> the background scan is restarted in case the connection establishment
> fails. This way the background scanning keeps running until all pending
> LE connection are established.
> 
> At this point, resolvable addresses are not support by this
> infrastructure. The proper support is added in upcoming patches.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h |  2 +
> net/bluetooth/hci_conn.c         |  5 +++
> net/bluetooth/hci_core.c         | 93 +++++++++++++++++++++++++++++++++++++++-
> net/bluetooth/hci_event.c        | 38 ++++++++++++++++
> 4 files changed, 136 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e08405d..617cf49 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -806,6 +806,8 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
> void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
> void hci_pend_le_conns_clear(struct hci_dev *hdev);
> 
> +void hci_update_background_scan(struct hci_dev *hdev);
> +
> void hci_uuids_clear(struct hci_dev *hdev);
> 
> void hci_link_keys_clear(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index ccf4a4f..e79351c 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -527,6 +527,11 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
> 	hci_proto_connect_cfm(conn, status);
> 
> 	hci_conn_del(conn);
> +
> +	/* Since we may have temporarily stopped the background scanning in
> +	 * favor of connection establishment, we should restart it.
> +	 */
> +	hci_update_background_scan(hdev);
> }
> 
> static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 142ecd8..d242217 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3281,7 +3281,7 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
> 
> 	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
> 	if (entry)
> -		return;
> +		goto done;
> 
> 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> 	if (!entry) {
> @@ -3295,6 +3295,9 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
> 	list_add(&entry->list, &hdev->pend_le_conns);
> 
> 	BT_DBG("addr %pMR (type %u)", addr, addr_type);
> +
> +done:
> +	hci_update_background_scan(hdev);
> }
> 
> /* This function requires the caller holds hdev->lock */
> @@ -3304,12 +3307,15 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
> 
> 	entry = hci_pend_le_conn_lookup(hdev, addr, addr_type);
> 	if (!entry)
> -		return;
> +		goto done;
> 
> 	list_del(&entry->list);
> 	kfree(entry);
> 
> 	BT_DBG("addr %pMR (type %u)", addr, addr_type);
> +
> +done:
> +	hci_update_background_scan(hdev);
> }
> 
> /* This function requires the caller holds hdev->lock */
> @@ -4946,3 +4952,86 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
> 	cp.enable = LE_SCAN_DISABLE;
> 	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
> }
> +
> +static void update_background_scan_complete(struct hci_dev *hdev, u8 status)
> +{
> +	if (status)
> +		BT_DBG("HCI request failed to update background scanning: "
> +		       "status 0x%2.2x", status);
> +}
> +
> +/* This function controls the background scanning based on hdev->pend_le_conns
> + * list. If there are pending LE connection we start the background scanning,
> + * otherwise we stop it.
> + *
> + * This function requires the caller holds hdev->lock.
> + */
> +void hci_update_background_scan(struct hci_dev *hdev)
> +{
> +	struct hci_cp_le_set_scan_param param_cp;
> +	struct hci_cp_le_set_scan_enable enable_cp;
> +	struct hci_request req;
> +	struct hci_conn *conn;
> +	int err;
> +
> +	hci_req_init(&req, hdev);
> +
> +	if (list_empty(&hdev->pend_le_conns)) {
> +		/* If there is no pending LE connections, we should stop
> +		 * the background scanning.
> +		 */
> +
> +		/* If controller is not scanning we are done. */
> +		if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> +			return;
> +
> +		hci_req_add_le_scan_disable(&req);
> +
> +		BT_DBG("%s stopping background scanning", hdev->name);
> +	} else {
> +		u8 own_addr_type;
> +
> +		/* If there is at least one pending LE connection, we should
> +		 * keep the background scan running.
> +		 */
> +
> +		/* If controller is already scanning we are done. */
> +		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
> +			return;
> +
> +		/* If controller is connecting, we should not start scanning
> +		 * since some controllers are not able to scan and connect at
> +		 * the same time.
> +		 */
> +		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> +		if (conn)
> +			return;
> +
> +		/* Use private address since remote doesn't need to identify us.
> +		 * Strictly speaking, this is not required since no SCAN_REQ is
> +		 * sent in passive scanning.
> +		 */
> +		if (hci_update_random_address(&req, true, &own_addr_type))
> +			return;

the comment above is confusing. Reason is simple. We might use a RPA if LE Privacy has been enabled. The require_privacy == true will fallback to URPA in case privacy is not enabled. If privacy is enabled, then no matter what require_privacy is set to, we will use a random address.

It should say something along the lines of this:

	/* Set require_privacy to true to avoid identification from
	 * unknown peer devices. Since this is passive scanning, no
	 * SCAN_REQ using the local identity should be sent. Mandating
	 * privacy is just an extra precaution.
	 */

Regards

Marcel

Re: [PATCH 08/17] Bluetooth: Introduce LE auto connection infrastructure

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:31 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> This patch introduces the LE auto connection infrastructure which
>> will be used to implement the LE auto connection options.
>>
>> In summary, the auto connection mechanism works as follows: Once the
>> first pending LE connection is created, the background scanning is
>> started. When the target device is found in range, the kernel
>> autonomously starts the connection attempt. If connection is
>> established successfully, that pending LE connection is deleted and
>> the background is stopped.
>>
>> To achieve that, this patch introduces the hci_update_background_scan()
>> which controls the background scanning state. This function starts or
>> stops the background scanning based on the hdev->pend_le_conns list. If
>> there is no pending LE connection, the background scanning is stopped.
>> Otherwise, we start the background scanning.
>>
>> Then, every time a pending LE connection is added we call hci_update_
>> background_scan() so the background scanning is started (in case it is
>> not already running). Likewise, every time a pending LE connection is
>> deleted we call hci_update_background_scan() so the background scanning
>> is stopped (in case this was the last pending LE connection) or it is
>> started again (in case we have more pending LE connections). Finally,
>> we also call hci_update_background_scan() in hci_le_conn_failed() so
>> the background scan is restarted in case the connection establishment
>> fails. This way the background scanning keeps running until all pending
>> LE connection are established.
>>
>> At this point, resolvable addresses are not support by this
>> infrastructure. The proper support is added in upcoming patches.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |  2 +
>> net/bluetooth/hci_conn.c         |  5 +++
>> net/bluetooth/hci_core.c         | 93 ++++++++++++++++++++++++++++++++++=
+++++-
>> net/bluetooth/hci_event.c        | 38 ++++++++++++++++
>> 4 files changed, 136 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index e08405d..617cf49 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -806,6 +806,8 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bdad=
dr_t *addr, u8 addr_type);
>> void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_=
type);
>> void hci_pend_le_conns_clear(struct hci_dev *hdev);
>>
>> +void hci_update_background_scan(struct hci_dev *hdev);
>> +
>> void hci_uuids_clear(struct hci_dev *hdev);
>>
>> void hci_link_keys_clear(struct hci_dev *hdev);
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index ccf4a4f..e79351c 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -527,6 +527,11 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 s=
tatus)
>>       hci_proto_connect_cfm(conn, status);
>>
>>       hci_conn_del(conn);
>> +
>> +     /* Since we may have temporarily stopped the background scanning i=
n
>> +      * favor of connection establishment, we should restart it.
>> +      */
>> +     hci_update_background_scan(hdev);
>> }
>>
>> static void create_le_conn_complete(struct hci_dev *hdev, u8 status)
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 142ecd8..d242217 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -3281,7 +3281,7 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bd=
addr_t *addr, u8 addr_type)
>>
>>       entry =3D hci_pend_le_conn_lookup(hdev, addr, addr_type);
>>       if (entry)
>> -             return;
>> +             goto done;
>>
>>       entry =3D kzalloc(sizeof(*entry), GFP_KERNEL);
>>       if (!entry) {
>> @@ -3295,6 +3295,9 @@ void hci_pend_le_conn_add(struct hci_dev *hdev, bd=
addr_t *addr, u8 addr_type)
>>       list_add(&entry->list, &hdev->pend_le_conns);
>>
>>       BT_DBG("addr %pMR (type %u)", addr, addr_type);
>> +
>> +done:
>> +     hci_update_background_scan(hdev);
>> }
>>
>> /* This function requires the caller holds hdev->lock */
>> @@ -3304,12 +3307,15 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, =
bdaddr_t *addr, u8 addr_type)
>>
>>       entry =3D hci_pend_le_conn_lookup(hdev, addr, addr_type);
>>       if (!entry)
>> -             return;
>> +             goto done;
>>
>>       list_del(&entry->list);
>>       kfree(entry);
>>
>>       BT_DBG("addr %pMR (type %u)", addr, addr_type);
>> +
>> +done:
>> +     hci_update_background_scan(hdev);
>> }
>>
>> /* This function requires the caller holds hdev->lock */
>> @@ -4946,3 +4952,86 @@ void hci_req_add_le_scan_disable(struct hci_reque=
st *req)
>>       cp.enable =3D LE_SCAN_DISABLE;
>>       hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
>> }
>> +
>> +static void update_background_scan_complete(struct hci_dev *hdev, u8 st=
atus)
>> +{
>> +     if (status)
>> +             BT_DBG("HCI request failed to update background scanning: =
"
>> +                    "status 0x%2.2x", status);
>> +}
>> +
>> +/* This function controls the background scanning based on hdev->pend_l=
e_conns
>> + * list. If there are pending LE connection we start the background sca=
nning,
>> + * otherwise we stop it.
>> + *
>> + * This function requires the caller holds hdev->lock.
>> + */
>> +void hci_update_background_scan(struct hci_dev *hdev)
>> +{
>> +     struct hci_cp_le_set_scan_param param_cp;
>> +     struct hci_cp_le_set_scan_enable enable_cp;
>> +     struct hci_request req;
>> +     struct hci_conn *conn;
>> +     int err;
>> +
>> +     hci_req_init(&req, hdev);
>> +
>> +     if (list_empty(&hdev->pend_le_conns)) {
>> +             /* If there is no pending LE connections, we should stop
>> +              * the background scanning.
>> +              */
>> +
>> +             /* If controller is not scanning we are done. */
>> +             if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
>> +                     return;
>> +
>> +             hci_req_add_le_scan_disable(&req);
>> +
>> +             BT_DBG("%s stopping background scanning", hdev->name);
>> +     } else {
>> +             u8 own_addr_type;
>> +
>> +             /* If there is at least one pending LE connection, we shou=
ld
>> +              * keep the background scan running.
>> +              */
>> +
>> +             /* If controller is already scanning we are done. */
>> +             if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
>> +                     return;
>> +
>> +             /* If controller is connecting, we should not start scanni=
ng
>> +              * since some controllers are not able to scan and connect=
 at
>> +              * the same time.
>> +              */
>> +             conn =3D hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONN=
ECT);
>> +             if (conn)
>> +                     return;
>> +
>> +             /* Use private address since remote doesn't need to identi=
fy us.
>> +              * Strictly speaking, this is not required since no SCAN_R=
EQ is
>> +              * sent in passive scanning.
>> +              */
>> +             if (hci_update_random_address(&req, true, &own_addr_type))
>> +                     return;
>
> the comment above is confusing. Reason is simple. We might use a RPA if L=
E Privacy has been enabled. The require_privacy =3D=3D true will fallback t=
o URPA in case privacy is not enabled. If privacy is enabled, then no matte=
r what require_privacy is set to, we will use a random address.
>
> It should say something along the lines of this:
>
>         /* Set require_privacy to true to avoid identification from
>          * unknown peer devices. Since this is passive scanning, no
>          * SCAN_REQ using the local identity should be sent. Mandating
>          * privacy is just an extra precaution.
>          */

Ok, I'll fix this comment.

BR,

Andre

[PATCH 09/17] Bluetooth: Introduce LE auto connect options

[Andre Guedes]

This patch introduces the LE auto connection options: HCI_AUTO_CONN_
ALWAYS and HCI_AUTO_CONN_LINK_LOSS. Their working mechanism are
described as follows:

The HCI_AUTO_CONN_ALWAYS option configures the kernel to always re-
establish the connection, no matter the reason the connection was
terminated. This feature is required by some LE profiles such as
HID over GATT, Health Thermometer and Blood Pressure. These profiles
require the host autonomously connect to the device as soon as it
enters in connectable mode (start advertising) so the device is able
to delivery notifications or indications.

The BT_AUTO_CONN_LINK_LOSS option configures the kernel to re-
establish the connection in case the connection was terminated due
to a link loss. This feature is required by the majority of LE
profiles such as Proximity, Find Me, Cycling Speed and Cadence and
Time.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  9 ++++++++-
 net/bluetooth/hci_core.c         | 11 +++++++----
 net/bluetooth/hci_event.c        | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 617cf49..b159810 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -402,6 +402,12 @@ struct hci_conn_params {
 
 	u16 conn_min_interval;
 	u16 conn_max_interval;
+
+	enum {
+		HCI_AUTO_CONN_DISABLED,
+		HCI_AUTO_CONN_ALWAYS,
+		HCI_AUTO_CONN_LINK_LOSS,
+	} auto_connect;
 };
 
 extern struct list_head hci_dev_list;
@@ -796,7 +802,8 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 					       bdaddr_t *addr, u8 addr_type);
 void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
-			 u16 conn_min_interval, u16 conn_max_interval);
+			 u8 auto_connect, u16 conn_min_interval,
+			 u16 conn_max_interval);
 void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_conn_params_clear(struct hci_dev *hdev);
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d242217..7b4f315 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3202,7 +3202,8 @@ struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 
 /* This function requires the caller holds hdev->lock */
 void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
-			 u16 conn_min_interval, u16 conn_max_interval)
+			 u8 auto_connect, u16 conn_min_interval,
+			 u16 conn_max_interval)
 {
 	struct hci_conn_params *params;
 
@@ -3210,6 +3211,7 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 	if (params) {
 		params->conn_min_interval = conn_min_interval;
 		params->conn_max_interval = conn_max_interval;
+		params->auto_connect = auto_connect;
 		return;
 	}
 
@@ -3223,12 +3225,13 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 	params->addr_type = addr_type;
 	params->conn_min_interval = conn_min_interval;
 	params->conn_max_interval = conn_max_interval;
+	params->auto_connect = auto_connect;
 
 	list_add(&params->list, &hdev->le_conn_params);
 
-	BT_DBG("addr %pMR (type %u) conn_min_interval 0x%.4x "
-	       "conn_max_interval 0x%.4x", addr, addr_type, conn_min_interval,
-	       conn_max_interval);
+	BT_DBG("addr %pMR (type %u) auto_connect %u conn_min_interval 0x%.4x "
+	       "conn_max_interval 0x%.4x", addr, addr_type, auto_connect,
+	       conn_min_interval, conn_max_interval);
 }
 
 /* This function requires the caller holds hdev->lock */
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index b6631d7..46da8b6 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1841,6 +1841,7 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_ev_disconn_complete *ev = (void *) skb->data;
 	u8 reason = hci_to_mgmt_reason(ev->reason);
+	struct hci_conn_params *params;
 	struct hci_conn *conn;
 	bool mgmt_connected;
 	u8 type;
@@ -1868,6 +1869,23 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	if (conn->type == ACL_LINK && conn->flush_key)
 		hci_remove_link_key(hdev, &conn->dst);
 
+	params = hci_conn_params_lookup(hdev, &conn->dst, conn->dst_type);
+	if (params) {
+		switch (params->auto_connect) {
+		case HCI_AUTO_CONN_LINK_LOSS:
+			if (ev->reason != HCI_ERROR_CONNECTION_TIMEOUT)
+				break;
+			/* Fall through */
+
+		case HCI_AUTO_CONN_ALWAYS:
+			hci_pend_le_conn_add(hdev, &conn->dst, conn->dst_type);
+			break;
+
+		default:
+			break;
+		}
+	}
+
 	type = conn->type;
 
 	hci_proto_disconn_cfm(conn, ev->reason);
-- 
1.8.5.4

[PATCH 10/17] Bluetooth: Connection parameters and auto connection

[Andre Guedes]

This patch modifies hci_conn_params_add() and hci_conn_params_del() so
they also add/delete pending LE connections according to the auto_
connect option. This way, background scan is automatically triggered/
untriggered when connection parameters are added/removed.

For instance, when a new connection parameters with HCI_AUTO_CONN_ALWAYS
option is added and we are not connected to the device, we add a pending
LE connection for that device.

Likewise, when the connection parameters are updated we add or delete
a pending LE connection according to its new auto_connect option.

Finally, when the connection parameter is deleted we also delete the
pending LE connection (if any).

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7b4f315..82042d3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3200,6 +3200,23 @@ struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 	return NULL;
 }
 
+static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
+{
+	struct hci_conn *conn;
+
+	conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, addr);
+	if (!conn)
+		return false;
+
+	if (conn->dst_type != type)
+		return false;
+
+	if (conn->state != BT_CONNECTED)
+		return false;
+
+	return true;
+}
+
 /* This function requires the caller holds hdev->lock */
 void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 			 u8 auto_connect, u16 conn_min_interval,
@@ -3208,12 +3225,8 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 	struct hci_conn_params *params;
 
 	params = hci_conn_params_lookup(hdev, addr, addr_type);
-	if (params) {
-		params->conn_min_interval = conn_min_interval;
-		params->conn_max_interval = conn_max_interval;
-		params->auto_connect = auto_connect;
-		return;
-	}
+	if (params)
+		goto update;
 
 	params = kzalloc(sizeof(*params), GFP_KERNEL);
 	if (!params) {
@@ -3223,11 +3236,24 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 
 	bacpy(&params->addr, addr);
 	params->addr_type = addr_type;
+
+	list_add(&params->list, &hdev->le_conn_params);
+
+update:
 	params->conn_min_interval = conn_min_interval;
 	params->conn_max_interval = conn_max_interval;
 	params->auto_connect = auto_connect;
 
-	list_add(&params->list, &hdev->le_conn_params);
+	switch (auto_connect) {
+	case HCI_AUTO_CONN_DISABLED:
+	case HCI_AUTO_CONN_LINK_LOSS:
+		hci_pend_le_conn_del(hdev, addr, addr_type);
+		break;
+	case HCI_AUTO_CONN_ALWAYS:
+		if (!is_connected(hdev, addr, addr_type))
+			hci_pend_le_conn_add(hdev, addr, addr_type);
+		break;
+	}
 
 	BT_DBG("addr %pMR (type %u) auto_connect %u conn_min_interval 0x%.4x "
 	       "conn_max_interval 0x%.4x", addr, addr_type, auto_connect,
@@ -3243,6 +3269,8 @@ void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type)
 	if (!params)
 		return;
 
+	hci_pend_le_conn_del(hdev, addr, addr_type);
+
 	list_del(&params->list);
 	kfree(params);
 
-- 
1.8.5.4

Re: [PATCH 10/17] Bluetooth: Connection parameters and auto connection

[Marcel Holtmann]

Hi Andre,

> This patch modifies hci_conn_params_add() and hci_conn_params_del() so
> they also add/delete pending LE connections according to the auto_
> connect option. This way, background scan is automatically triggered/
> untriggered when connection parameters are added/removed.
> 
> For instance, when a new connection parameters with HCI_AUTO_CONN_ALWAYS
> option is added and we are not connected to the device, we add a pending
> LE connection for that device.
> 
> Likewise, when the connection parameters are updated we add or delete
> a pending LE connection according to its new auto_connect option.
> 
> Finally, when the connection parameter is deleted we also delete the
> pending LE connection (if any).

what about disconnecting an existing connection for a device we have in our auto-connect list. I think that should happen as well.

Regards

Marcel

Re: [PATCH 10/17] Bluetooth: Connection parameters and auto connection

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:33 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patch modifies hci_conn_params_add() and hci_conn_params_del() so
>> they also add/delete pending LE connections according to the auto_
>> connect option. This way, background scan is automatically triggered/
>> untriggered when connection parameters are added/removed.
>>
>> For instance, when a new connection parameters with HCI_AUTO_CONN_ALWAYS
>> option is added and we are not connected to the device, we add a pending
>> LE connection for that device.
>>
>> Likewise, when the connection parameters are updated we add or delete
>> a pending LE connection according to its new auto_connect option.
>>
>> Finally, when the connection parameter is deleted we also delete the
>> pending LE connection (if any).
>
> what about disconnecting an existing connection for a device we have in our auto-connect list. I think that should happen as well.

This kind of logic seems to be more suitable if we implement it in the
upper layer (mgmt.c). The function that will handle connection
parameters removal would also terminate the connection in case there
is a connection for that device.

BR,

Andre

Re: [PATCH 10/17] Bluetooth: Connection parameters and auto connection

[Marcel Holtmann]

Hi Andre,

>>> This patch modifies hci_conn_params_add() and hci_conn_params_del() so
>>> they also add/delete pending LE connections according to the auto_
>>> connect option. This way, background scan is automatically triggered/
>>> untriggered when connection parameters are added/removed.
>>> 
>>> For instance, when a new connection parameters with HCI_AUTO_CONN_ALWAYS
>>> option is added and we are not connected to the device, we add a pending
>>> LE connection for that device.
>>> 
>>> Likewise, when the connection parameters are updated we add or delete
>>> a pending LE connection according to its new auto_connect option.
>>> 
>>> Finally, when the connection parameter is deleted we also delete the
>>> pending LE connection (if any).
>> 
>> what about disconnecting an existing connection for a device we have in our auto-connect list. I think that should happen as well.
> 
> This kind of logic seems to be more suitable if we implement it in the
> upper layer (mgmt.c). The function that will handle connection
> parameters removal would also terminate the connection in case there
> is a connection for that device.

not 100% convinced here. If the connection has been triggered by an actually connect() socket call, then sure it is there to stay until disconnect by the user.

However in case the connection has been established by the auto-connect handling and we either remove the value or set it back to disabled, then why not also terminate that connection. This might need further thinking. So lets go ahead without doing it right now.

Regards

Marcel

[PATCH 11/17] Bluetooth: Temporarily stop background scanning on discovery

[Andre Guedes]

If the user sends a mgmt start discovery command while the background
scanning is running, we should temporarily stop it. Once the discovery
finishes, we start the background scanning again.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c |  2 ++
 net/bluetooth/mgmt.c     | 12 ++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 82042d3..3c985f2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1786,6 +1786,8 @@ void hci_discovery_set_state(struct hci_dev *hdev, int state)
 
 	switch (state) {
 	case DISCOVERY_STOPPED:
+		hci_update_background_scan(hdev);
+
 		if (hdev->discovery.state != DISCOVERY_STARTING)
 			mgmt_discovering(hdev, 0);
 		break;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index bad23d5..a62e22c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3439,12 +3439,12 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags)) {
-			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
-					 MGMT_STATUS_BUSY);
-			mgmt_pending_remove(cmd);
-			goto failed;
-		}
+		/* If controller is scanning, it means the background scanning
+		 * is running. Thus, we should temporarily stop it in order to
+		 * set the discovery scanning parameters.
+		 */
+		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags))
+			hci_req_add_le_scan_disable(&req);
 
 		memset(&param_cp, 0, sizeof(param_cp));
 
-- 
1.8.5.4

[PATCH 12/17] Bluetooth: Auto connection and power on

[Andre Guedes]

When hdev is closed (e.g. Mgmt power off command, RFKILL or controller
is reset), the ongoing active connections are silently dropped by the
controller (no Disconnection Complete Event is sent to host). For that
reason, the devices that require HCI_AUTO_CONN_ALWAYS are not added to
hdev->pend_le_conns list and they won't auto connect.

So to fix this issue, during hdev closing, we remove all pending LE
connections. After adapter is powered on, we add a pending LE connection
for each HCI_AUTO_CONN_ALWAYS address.

This way, the auto connection mechanism works propely after a power
off and power on sequence as well as RFKILL block/unblock.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c |  1 +
 net/bluetooth/mgmt.c     | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3c985f2..692637b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2266,6 +2266,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 	hci_dev_lock(hdev);
 	hci_inquiry_cache_flush(hdev);
 	hci_conn_hash_flush(hdev);
+	hci_pend_le_conns_clear(hdev);
 	hci_dev_unlock(hdev);
 
 	hci_notify(hdev, HCI_DEV_DOWN);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a62e22c..f878267 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4669,6 +4669,17 @@ void mgmt_index_removed(struct hci_dev *hdev)
 	mgmt_event(MGMT_EV_INDEX_REMOVED, hdev, NULL, 0, NULL);
 }
 
+/* This function requires the caller holds hdev->lock */
+static void restart_le_auto_conns(struct hci_dev *hdev)
+{
+	struct hci_conn_params *p;
+
+	list_for_each_entry(p, &hdev->le_conn_params, list) {
+		if (p->auto_connect == HCI_AUTO_CONN_ALWAYS)
+			hci_pend_le_conn_add(hdev, &p->addr, p->addr_type);
+	}
+}
+
 static void powered_complete(struct hci_dev *hdev, u8 status)
 {
 	struct cmd_lookup match = { NULL, hdev };
@@ -4677,6 +4688,8 @@ static void powered_complete(struct hci_dev *hdev, u8 status)
 
 	hci_dev_lock(hdev);
 
+	restart_le_auto_conns(hdev);
+
 	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
 
 	new_settings(hdev, match.sk);
-- 
1.8.5.4

[PATCH 13/17] Bluetooth: Connection parameters and resolvable address

[Andre Guedes]

We should only add connection parameters for public, random static and
random private resolvable with IRK. If we allow non-resolvable or
resolvable without IRK, the background scan may run indefinitely. So, to
avoid this undesired behavior, we should check the address type in
hci_conn_params_add().

Additionally, since the IRK is removed during unpair, we should also
remove the connection parameters from that device.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h | 18 +++++++++++++++---
 net/bluetooth/hci_core.c         | 24 ++++++++++++++++++++----
 net/bluetooth/mgmt.c             |  2 ++
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b159810..e0e0eab 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -801,9 +801,9 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 type);
 
 struct hci_conn_params *hci_conn_params_lookup(struct hci_dev *hdev,
 					       bdaddr_t *addr, u8 addr_type);
-void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
-			 u8 auto_connect, u16 conn_min_interval,
-			 u16 conn_max_interval);
+int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
+			u8 auto_connect, u16 conn_min_interval,
+			u16 conn_max_interval);
 void hci_conn_params_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_conn_params_clear(struct hci_dev *hdev);
 
@@ -1119,6 +1119,18 @@ static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
 	return false;
 }
 
+/* Check if address is "random private non-resolvable" type */
+static inline bool hci_bdaddr_is_non_rpa(bdaddr_t *bdaddr, u8 addr_type)
+{
+	if (addr_type != 0x01)
+		return false;
+
+	if ((bdaddr->b[5] & 0xc0) == 0x00)
+		return true;
+
+	return false;
+}
+
 static inline struct smp_irk *hci_get_irk(struct hci_dev *hdev,
 					  bdaddr_t *bdaddr, u8 addr_type)
 {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 692637b..e17ea87 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3221,12 +3221,26 @@ static bool is_connected(struct hci_dev *hdev, bdaddr_t *addr, u8 type)
 }
 
 /* This function requires the caller holds hdev->lock */
-void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
-			 u8 auto_connect, u16 conn_min_interval,
-			 u16 conn_max_interval)
+int hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
+			u8 auto_connect, u16 conn_min_interval,
+			u16 conn_max_interval)
 {
 	struct hci_conn_params *params;
 
+	if (hci_bdaddr_is_non_rpa(addr, addr_type))
+		return -EINVAL;
+
+	if (hci_bdaddr_is_rpa(addr, addr_type)) {
+		struct smp_irk *irk;
+
+		irk = hci_get_irk(hdev, addr, addr_type);
+		if (!irk)
+			return -EINVAL;
+
+		addr = &irk->bdaddr;
+		addr_type = irk->addr_type;
+	}
+
 	params = hci_conn_params_lookup(hdev, addr, addr_type);
 	if (params)
 		goto update;
@@ -3234,7 +3248,7 @@ void hci_conn_params_add(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type,
 	params = kzalloc(sizeof(*params), GFP_KERNEL);
 	if (!params) {
 		BT_ERR("Out of memory");
-		return;
+		return -ENOMEM;
 	}
 
 	bacpy(&params->addr, addr);
@@ -3261,6 +3275,8 @@ update:
 	BT_DBG("addr %pMR (type %u) auto_connect %u conn_min_interval 0x%.4x "
 	       "conn_max_interval 0x%.4x", addr, addr_type, auto_connect,
 	       conn_min_interval, conn_max_interval);
+
+	return 0;
 }
 
 /* This function requires the caller holds hdev->lock */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f878267..2e6564e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2416,6 +2416,8 @@ static int unpair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 		hci_remove_irk(hdev, &cp->addr.bdaddr, addr_type);
 
+		hci_conn_params_del(hdev, &cp->addr.bdaddr, addr_type);
+
 		err = hci_remove_ltk(hdev, &cp->addr.bdaddr, addr_type);
 	}
 
-- 
1.8.5.4

Re: [PATCH 13/17] Bluetooth: Connection parameters and resolvable address

[Marcel Holtmann]

Hi Andre,

> We should only add connection parameters for public, random static and
> random private resolvable with IRK. If we allow non-resolvable or
> resolvable without IRK, the background scan may run indefinitely. So, to
> avoid this undesired behavior, we should check the address type in
> hci_conn_params_add().

this makes no sense. We should only allow auto-connection from public and static random addresses. These two are identity addresses.

Every IRK has an identity address assigned to it. If you want to auto-connect a device using a resolvable private address, then the identity address needs to be programmed into our auto-connection list. The RPA should never ever go there.

That is how connect() works actually. You give it the identity address and it will use the IRK to match it to the RPA in use. We need to do exactly the same.

In addition please keep in mind that userspace only knows about RPA as long as they are not identified. Once they are identified, the kernel will only tell us about identity addresses. The RPA will be in all mgmt commands and events automatically resolved.

Regards

Marcel

Re: [PATCH 13/17] Bluetooth: Connection parameters and resolvable address

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:37 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> We should only add connection parameters for public, random static and
>> random private resolvable with IRK. If we allow non-resolvable or
>> resolvable without IRK, the background scan may run indefinitely. So, to
>> avoid this undesired behavior, we should check the address type in
>> hci_conn_params_add().
>
> this makes no sense. We should only allow auto-connection from public and=
 static random addresses. These two are identity addresses.
>
> Every IRK has an identity address assigned to it. If you want to auto-con=
nect a device using a resolvable private address, then the identity address=
 needs to be programmed into our auto-connection list. The RPA should never=
 ever go there.
>
> That is how connect() works actually. You give it the identity address an=
d it will use the IRK to match it to the RPA in use. We need to do exactly =
the same.
>
> In addition please keep in mind that userspace only knows about RPA as lo=
ng as they are not identified. Once they are identified, the kernel will on=
ly tell us about identity addresses. The RPA will be in all mgmt commands a=
nd events automatically resolved.

Ok, so I'll change hci_conn_params_add() to accept public and random
static addresses only.

BR,

Andre

[PATCH 14/17] Bluetooth: Support resolvable private addresses

[Andre Guedes]

Only identity addresses are inserted into hdev->pend_le_conns. So,
in order to support resolvable private addresses in auto connection
mechanism, we should resolve the address before checking for pending
connections.

Thus, this patch adds an extra check in check_pending_le_conn() and
updates 'addr' and 'addr_type' variables before hci_pend_le_conn_
lookup().

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_event.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 46da8b6..cda92db 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3706,6 +3706,16 @@ static void check_pending_le_conn(struct hci_dev *hdev, bdaddr_t *addr,
 				  u8 addr_type)
 {
 	struct hci_conn *conn;
+	struct smp_irk *irk;
+
+	/* If this is a resolvable address, we should resolve it and then
+	 * update address and address type variables.
+	 */
+	irk = hci_get_irk(hdev, addr, addr_type);
+	if (irk) {
+		addr = &irk->bdaddr;
+		addr_type = irk->addr_type;
+	}
 
 	if (!hci_pend_le_conn_lookup(hdev, addr, addr_type))
 		return;
-- 
1.8.5.4

[PATCH 15/17] Bluetooth: Add le_auto_conn file on debugfs

[Andre Guedes]

This patch adds to debugfs the le_auto_conn file. This file will be
used to test LE auto connection infrastructure.

To add a new auto connection address we write on le_auto_conn file
following the format <address> <address type> <auto_connect>.

The <address type> values are:
  * 0 for public address
  * 1 for random address

The <auto_connect> values are (for more details see struct hci_
conn_params):
  * 0 for disabled
  * 1 for always
  * 2 for link loss

So for instance, if you want the kernel autonomously establishes
connections with device AA:BB:CC:DD:EE:FF (public address) every
time the device enters in connectable mode (starts advertising),
you should run the command:
$ echo "AA:BB:CC:DD:EE:FF 0 1" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn

To get the list of connection parameters configured in kernel, read
the le_auto_conn file:
$ cat /sys/kernel/debug/bluetooth/hci0/le_auto_conn

Finally, to clear the connection parameters list, write an empty
string:
$ echo "" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn

This file is created only if LE is enabled.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 92 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e17ea87..fb4c961 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -896,6 +896,96 @@ static const struct file_operations lowpan_debugfs_fops = {
 	.llseek		= default_llseek,
 };
 
+static int le_auto_conn_show(struct seq_file *sf, void *ptr)
+{
+	struct hci_dev *hdev = sf->private;
+	struct hci_conn_params *p;
+
+	hci_dev_lock(hdev);
+
+	list_for_each_entry(p, &hdev->le_conn_params, list) {
+		seq_printf(sf, "%pMR %u %u\n", &p->addr, p->addr_type,
+			   p->auto_connect);
+	}
+
+	hci_dev_unlock(hdev);
+
+	return 0;
+}
+
+static int le_auto_conn_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, le_auto_conn_show, inode->i_private);
+}
+
+static ssize_t le_auto_conn_write(struct file *file, const char __user *data,
+				  size_t count, loff_t *offset)
+{
+	struct seq_file *sf = file->private_data;
+	struct hci_dev *hdev = sf->private;
+	u8 auto_connect;
+	bdaddr_t addr;
+	u8 addr_type;
+	char *buf;
+	int err;
+	int n;
+
+	/* Don't allow partial write */
+	if (*offset != 0)
+		return -EINVAL;
+
+	/* If empty string, clear the connection parameters and pending LE
+	 * connection list.
+	 */
+	if (count == 1) {
+		hci_dev_lock(hdev);
+		hci_conn_params_clear(hdev);
+		hci_pend_le_conns_clear(hdev);
+		hci_update_background_scan(hdev);
+		hci_dev_unlock(hdev);
+		return count;
+	}
+
+	buf = kzalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	if (copy_from_user(buf, data, count)) {
+		kfree(buf);
+		return -EFAULT;
+	}
+
+	n = sscanf(buf, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx %hhu %hhu", &addr.b[5],
+		   &addr.b[4], &addr.b[3], &addr.b[2], &addr.b[1], &addr.b[0],
+		   &addr_type, &auto_connect);
+	if (n != 8) {
+		kfree(buf);
+		return -EINVAL;
+	}
+
+	hci_dev_lock(hdev);
+	err = hci_conn_params_add(hdev, &addr, addr_type, auto_connect,
+				  hdev->le_conn_min_interval,
+				  hdev->le_conn_max_interval);
+	hci_dev_unlock(hdev);
+
+	if (err) {
+		kfree(buf);
+		return err;
+	}
+
+	kfree(buf);
+	return count;
+}
+
+static const struct file_operations le_auto_conn_fops = {
+	.open		= le_auto_conn_open,
+	.read		= seq_read,
+	.write		= le_auto_conn_write,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 /* ---- HCI requests ---- */
 
 static void hci_req_sync_complete(struct hci_dev *hdev, u8 result)
@@ -1694,6 +1784,8 @@ static int __hci_init(struct hci_dev *hdev)
 				    hdev, &adv_channel_map_fops);
 		debugfs_create_file("6lowpan", 0644, hdev->debugfs, hdev,
 				    &lowpan_debugfs_fops);
+		debugfs_create_file("le_auto_conn", 0644, hdev->debugfs, hdev,
+				    &le_auto_conn_fops);
 	}
 
 	return 0;
-- 
1.8.5.4

Re: [PATCH 15/17] Bluetooth: Add le_auto_conn file on debugfs

[Marcel Holtmann]

Hi Andre,

> This patch adds to debugfs the le_auto_conn file. This file will be
> used to test LE auto connection infrastructure.
> 
> To add a new auto connection address we write on le_auto_conn file
> following the format <address> <address type> <auto_connect>.
> 
> The <address type> values are:
>  * 0 for public address
>  * 1 for random address
> 
> The <auto_connect> values are (for more details see struct hci_
> conn_params):
>  * 0 for disabled
>  * 1 for always
>  * 2 for link loss
> 
> So for instance, if you want the kernel autonomously establishes
> connections with device AA:BB:CC:DD:EE:FF (public address) every
> time the device enters in connectable mode (starts advertising),
> you should run the command:
> $ echo "AA:BB:CC:DD:EE:FF 0 1" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn
> 
> To get the list of connection parameters configured in kernel, read
> the le_auto_conn file:
> $ cat /sys/kernel/debug/bluetooth/hci0/le_auto_conn
> 
> Finally, to clear the connection parameters list, write an empty
> string:
> $ echo "" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn
> 
> This file is created only if LE is enabled.

I wonder if this should be prefixed with a command. For example like this:

	“add <address> <address_type> [auto_connect]”
	“del <address> <address_type>”
	“clr”

Regards

Marcel

Re: [PATCH 15/17] Bluetooth: Add le_auto_conn file on debugfs

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:41 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patch adds to debugfs the le_auto_conn file. This file will be
>> used to test LE auto connection infrastructure.
>>
>> To add a new auto connection address we write on le_auto_conn file
>> following the format <address> <address type> <auto_connect>.
>>
>> The <address type> values are:
>>  * 0 for public address
>>  * 1 for random address
>>
>> The <auto_connect> values are (for more details see struct hci_
>> conn_params):
>>  * 0 for disabled
>>  * 1 for always
>>  * 2 for link loss
>>
>> So for instance, if you want the kernel autonomously establishes
>> connections with device AA:BB:CC:DD:EE:FF (public address) every
>> time the device enters in connectable mode (starts advertising),
>> you should run the command:
>> $ echo "AA:BB:CC:DD:EE:FF 0 1" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn
>>
>> To get the list of connection parameters configured in kernel, read
>> the le_auto_conn file:
>> $ cat /sys/kernel/debug/bluetooth/hci0/le_auto_conn
>>
>> Finally, to clear the connection parameters list, write an empty
>> string:
>> $ echo "" > /sys/kernel/debug/bluetooth/hci0/le_auto_conn
>>
>> This file is created only if LE is enabled.
>
> I wonder if this should be prefixed with a command. For example like this:
>
>         "add <address> <address_type> [auto_connect]"
>         "del <address> <address_type>"
>         "clr"

Year, no problem. I'll do like that.

BR,

Andre

[PATCH 16/17] Bluetooth: Create hci_req_add_le_passive_scan helper

[Andre Guedes]

This patches creates the hci_req_add_le_passive_scan helper so it can
be re-used in the next patch.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_core.c | 54 +++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fb4c961..e776624 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -5095,6 +5095,35 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 }
 
+static void hci_req_add_le_passive_scan(struct hci_request *req,
+					struct hci_dev *hdev)
+{
+	struct hci_cp_le_set_scan_param param_cp;
+	struct hci_cp_le_set_scan_enable enable_cp;
+	u8 own_addr_type;
+
+	/* Use private address since remote doesn't need to identify us.
+	 * Strictly speaking, this is not required since no SCAN_REQ is
+	 * sent in passive scanning.
+	 */
+	if (hci_update_random_address(req, true, &own_addr_type))
+		return;
+
+	memset(&param_cp, 0, sizeof(param_cp));
+	param_cp.type = LE_SCAN_PASSIVE;
+	param_cp.interval = cpu_to_le16(hdev->le_scan_interval);
+	param_cp.window = cpu_to_le16(hdev->le_scan_window);
+	param_cp.own_address_type = own_addr_type;
+	hci_req_add(req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
+		    &param_cp);
+
+	memset(&enable_cp, 0, sizeof(enable_cp));
+	enable_cp.enable = LE_SCAN_ENABLE;
+	enable_cp.filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
+	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
+		    &enable_cp);
+}
+
 static void update_background_scan_complete(struct hci_dev *hdev, u8 status)
 {
 	if (status)
@@ -5110,8 +5139,6 @@ static void update_background_scan_complete(struct hci_dev *hdev, u8 status)
  */
 void hci_update_background_scan(struct hci_dev *hdev)
 {
-	struct hci_cp_le_set_scan_param param_cp;
-	struct hci_cp_le_set_scan_enable enable_cp;
 	struct hci_request req;
 	struct hci_conn *conn;
 	int err;
@@ -5131,8 +5158,6 @@ void hci_update_background_scan(struct hci_dev *hdev)
 
 		BT_DBG("%s stopping background scanning", hdev->name);
 	} else {
-		u8 own_addr_type;
-
 		/* If there is at least one pending LE connection, we should
 		 * keep the background scan running.
 		 */
@@ -5149,26 +5174,7 @@ void hci_update_background_scan(struct hci_dev *hdev)
 		if (conn)
 			return;
 
-		/* Use private address since remote doesn't need to identify us.
-		 * Strictly speaking, this is not required since no SCAN_REQ is
-		 * sent in passive scanning.
-		 */
-		if (hci_update_random_address(&req, true, &own_addr_type))
-			return;
-
-		memset(&param_cp, 0, sizeof(param_cp));
-		param_cp.type = LE_SCAN_PASSIVE;
-		param_cp.interval = cpu_to_le16(hdev->le_scan_interval);
-		param_cp.window = cpu_to_le16(hdev->le_scan_window);
-		param_cp.own_address_type = own_addr_type;
-		hci_req_add(&req, HCI_OP_LE_SET_SCAN_PARAM, sizeof(param_cp),
-			    &param_cp);
-
-		memset(&enable_cp, 0, sizeof(enable_cp));
-		enable_cp.enable = LE_SCAN_ENABLE;
-		enable_cp.filter_dup = LE_SCAN_FILTER_DUP_DISABLE;
-		hci_req_add(&req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(enable_cp),
-			    &enable_cp);
+		hci_req_add_le_passive_scan(&req, hdev);
 
 		BT_DBG("%s starting background scanning", hdev->name);
 	}
-- 
1.8.5.4

Re: [PATCH 16/17] Bluetooth: Create hci_req_add_le_passive_scan helper

[Marcel Holtmann]

Hi Andre,

> This patches creates the hci_req_add_le_passive_scan helper so it can
> be re-used in the next patch.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> net/bluetooth/hci_core.c | 54 +++++++++++++++++++++++++++---------------------
> 1 file changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index fb4c961..e776624 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -5095,6 +5095,35 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
> 	hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
> }
> 
> +static void hci_req_add_le_passive_scan(struct hci_request *req,
> +					struct hci_dev *hdev)
> +{

	struct hci_dev *hdev = req->hdev;

Regards

Marcel

Re: [PATCH 16/17] Bluetooth: Create hci_req_add_le_passive_scan helper

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:44 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patches creates the hci_req_add_le_passive_scan helper so it can
>> be re-used in the next patch.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> net/bluetooth/hci_core.c | 54 +++++++++++++++++++++++++++---------------------
>> 1 file changed, 30 insertions(+), 24 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index fb4c961..e776624 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -5095,6 +5095,35 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
>>       hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
>> }
>>
>> +static void hci_req_add_le_passive_scan(struct hci_request *req,
>> +                                     struct hci_dev *hdev)
>> +{
>
>         struct hci_dev *hdev = req->hdev;

Sure, my bad.

Thanks,

Andre

[PATCH 17/17] Bluetooth: Update background scan parameters

[Andre Guedes]

If new scanning parameters are set while background scan is running,
we should restart background scanning so these parameters are updated.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/hci_core.h |  1 +
 net/bluetooth/hci_core.c         | 26 ++++++++++++++++++++++++++
 net/bluetooth/mgmt.c             |  7 +++++++
 3 files changed, 34 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e0e0eab..8f0ddc8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -814,6 +814,7 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
 void hci_pend_le_conns_clear(struct hci_dev *hdev);
 
 void hci_update_background_scan(struct hci_dev *hdev);
+void hci_restart_background_scan(struct hci_dev *hdev);
 
 void hci_uuids_clear(struct hci_dev *hdev);
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e776624..f5c0e97 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -5183,3 +5183,29 @@ void hci_update_background_scan(struct hci_dev *hdev)
 	if (err)
 		BT_ERR("Failed to run HCI request: err %d", err);
 }
+
+static void restart_background_scan_complete(struct hci_dev *hdev, u8 status)
+{
+	if (status)
+		BT_DBG("HCI request failed to restart background scan: "
+		       "status 0x%2.2x", status);
+}
+
+void hci_restart_background_scan(struct hci_dev *hdev)
+{
+	struct hci_request req;
+	int err;
+
+	hci_req_init(&req, hdev);
+
+	hci_req_add_le_scan_disable(&req);
+	hci_req_add_le_passive_scan(&req, hdev);
+
+	err = hci_req_run(&req, restart_background_scan_complete);
+	if (err) {
+		BT_ERR("Failed to run HCI request: err %d", err);
+		return;
+	}
+
+	BT_DBG("%s re-starting background scanning", hdev->name);
+}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 2e6564e..08d52f2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3924,6 +3924,13 @@ static int set_scan_params(struct sock *sk, struct hci_dev *hdev,
 
 	err = cmd_complete(sk, hdev->id, MGMT_OP_SET_SCAN_PARAMS, 0, NULL, 0);
 
+	/* If background scan is running, restart it so new parameters are
+	 * loaded.
+	 */
+	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
+	    hdev->discovery.state == DISCOVERY_STOPPED)
+		hci_restart_background_scan(hdev);
+
 	hci_dev_unlock(hdev);
 
 	return err;
-- 
1.8.5.4

Re: [PATCH 17/17] Bluetooth: Update background scan parameters

[Marcel Holtmann]

Hi Andre,

> If new scanning parameters are set while background scan is running,
> we should restart background scanning so these parameters are updated.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
> include/net/bluetooth/hci_core.h |  1 +
> net/bluetooth/hci_core.c         | 26 ++++++++++++++++++++++++++
> net/bluetooth/mgmt.c             |  7 +++++++
> 3 files changed, 34 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e0e0eab..8f0ddc8 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -814,6 +814,7 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, bdaddr_t *addr, u8 addr_type);
> void hci_pend_le_conns_clear(struct hci_dev *hdev);
> 
> void hci_update_background_scan(struct hci_dev *hdev);
> +void hci_restart_background_scan(struct hci_dev *hdev);
> 
> void hci_uuids_clear(struct hci_dev *hdev);
> 
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index e776624..f5c0e97 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -5183,3 +5183,29 @@ void hci_update_background_scan(struct hci_dev *hdev)
> 	if (err)
> 		BT_ERR("Failed to run HCI request: err %d", err);
> }
> +
> +static void restart_background_scan_complete(struct hci_dev *hdev, u8 status)
> +{
> +	if (status)
> +		BT_DBG("HCI request failed to restart background scan: "
> +		       "status 0x%2.2x", status);
> +}
> +
> +void hci_restart_background_scan(struct hci_dev *hdev)
> +{
> +	struct hci_request req;
> +	int err;
> +
> +	hci_req_init(&req, hdev);
> +
> +	hci_req_add_le_scan_disable(&req);
> +	hci_req_add_le_passive_scan(&req, hdev);
> +
> +	err = hci_req_run(&req, restart_background_scan_complete);
> +	if (err) {
> +		BT_ERR("Failed to run HCI request: err %d", err);
> +		return;
> +	}
> +
> +	BT_DBG("%s re-starting background scanning", hdev->name);
> +}
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 2e6564e..08d52f2 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3924,6 +3924,13 @@ static int set_scan_params(struct sock *sk, struct hci_dev *hdev,
> 
> 	err = cmd_complete(sk, hdev->id, MGMT_OP_SET_SCAN_PARAMS, 0, NULL, 0);
> 
> +	/* If background scan is running, restart it so new parameters are
> +	 * loaded.
> +	 */
> +	if (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
> +	    hdev->discovery.state == DISCOVERY_STOPPED)
> +		hci_restart_background_scan(hdev);
> +

unless there are some other plans, then put the hci_req directly into the body of the if statement. No need to split this out into a public function. And even you want to use this later, I rather see this confined right now and keep public function to a minimum.

Regards

Marcel

Re: [PATCH 17/17] Bluetooth: Update background scan parameters

[Andre Guedes]

Hi Marcel,

On Wed, Feb 26, 2014 at 3:47 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andre,
>
>> If new scanning parameters are set while background scan is running,
>> we should restart background scanning so these parameters are updated.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>> include/net/bluetooth/hci_core.h |  1 +
>> net/bluetooth/hci_core.c         | 26 ++++++++++++++++++++++++++
>> net/bluetooth/mgmt.c             |  7 +++++++
>> 3 files changed, 34 insertions(+)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index e0e0eab..8f0ddc8 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -814,6 +814,7 @@ void hci_pend_le_conn_del(struct hci_dev *hdev, bdad=
dr_t *addr, u8 addr_type);
>> void hci_pend_le_conns_clear(struct hci_dev *hdev);
>>
>> void hci_update_background_scan(struct hci_dev *hdev);
>> +void hci_restart_background_scan(struct hci_dev *hdev);
>>
>> void hci_uuids_clear(struct hci_dev *hdev);
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index e776624..f5c0e97 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -5183,3 +5183,29 @@ void hci_update_background_scan(struct hci_dev *h=
dev)
>>       if (err)
>>               BT_ERR("Failed to run HCI request: err %d", err);
>> }
>> +
>> +static void restart_background_scan_complete(struct hci_dev *hdev, u8 s=
tatus)
>> +{
>> +     if (status)
>> +             BT_DBG("HCI request failed to restart background scan: "
>> +                    "status 0x%2.2x", status);
>> +}
>> +
>> +void hci_restart_background_scan(struct hci_dev *hdev)
>> +{
>> +     struct hci_request req;
>> +     int err;
>> +
>> +     hci_req_init(&req, hdev);
>> +
>> +     hci_req_add_le_scan_disable(&req);
>> +     hci_req_add_le_passive_scan(&req, hdev);
>> +
>> +     err =3D hci_req_run(&req, restart_background_scan_complete);
>> +     if (err) {
>> +             BT_ERR("Failed to run HCI request: err %d", err);
>> +             return;
>> +     }
>> +
>> +     BT_DBG("%s re-starting background scanning", hdev->name);
>> +}
>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>> index 2e6564e..08d52f2 100644
>> --- a/net/bluetooth/mgmt.c
>> +++ b/net/bluetooth/mgmt.c
>> @@ -3924,6 +3924,13 @@ static int set_scan_params(struct sock *sk, struc=
t hci_dev *hdev,
>>
>>       err =3D cmd_complete(sk, hdev->id, MGMT_OP_SET_SCAN_PARAMS, 0, NUL=
L, 0);
>>
>> +     /* If background scan is running, restart it so new parameters are
>> +      * loaded.
>> +      */
>> +     if (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
>> +         hdev->discovery.state =3D=3D DISCOVERY_STOPPED)
>> +             hci_restart_background_scan(hdev);
>> +
>
> unless there are some other plans, then put the hci_req directly into the=
 body of the if statement. No need to split this out into a public function=
. And even you want to use this later, I rather see this confined right now=
 and keep public function to a minimum.

I'll fix this.

BR,

Andre