[RFC 0/7] LE connection routine

[RFC 0/7] LE connection routine

[Andre Guedes]

Hi all,

In order to better support LE connection requirements, such as multiple
connections and re-connections, we should support the general connection
establishment procedure in kernel side. Today, we support only the direct
connection establishment procedure which has some limitations and, therefore,
requires extra connection management at user-space.

According to the spec (Vol 3, Part C, section 9.3.6), the general procedure is
described as follows: The host starts scanning for LE devices, once the device
we want to connect to is in-range, the host stops scanning and initiates a
connection. The procedure is terminated when the connection is established or
when the host terminates the procedure.

This RFC series implements the basic support for general connection procedure.
The first patches do simple changes required to implement this new LE
connection routine. It has not been well tested, but the basic LE connection
and disconnection cases have been covered.

This is an initial work, so it doesn't support multiple LE connection attempts
at the same time (current kernel doesn't support too) and doesn't handle
concurrent device discovery properly.

The next steps are the following:
1. Cover some corner cases in this series.
2. Add support for multiple LE connection attempts.
3. Handle concurrent LE connections and device discovery
4. Add better support for controller which a capable of scanning and initiating
LE connection at the same time.
5. Add API so userspace is able to configure connection parameters.
6. Remove LE connection management code from bluetoothd.

Feedback is welcome.

Best regards,

Andre


Andre Guedes (7):
  Bluetooth: Add new connection states
  Bluetooth: Make hci_le_create_connection non-static
  Bluetooth: Setup LE scan with no timeout
  Bluetooth: Add LE scan type macros
  Bluetooth: Change LE connection routine
  Bluetooth: Handle hci_conn timeout in BT_SCAN state
  Bluetooth: Track the number of hci_conn in BT_SCAN

 include/net/bluetooth/bluetooth.h |  8 +++++++-
 include/net/bluetooth/hci_core.h  |  9 +++++++++
 net/bluetooth/hci_conn.c          | 16 ++++++++++++++--
 net/bluetooth/hci_core.c          | 19 +++++++++++--------
 net/bluetooth/hci_event.c         | 40 +++++++++++++++++++++++++++++++++++++--
 net/bluetooth/mgmt.c              |  5 ++---
 6 files changed, 81 insertions(+), 16 deletions(-)

-- 
1.8.1.1

[RFC 1/7] Bluetooth: Add new connection states

[Andre Guedes]

This patch adds two new connection states which will be used by
hci_conn to establish LE connections.

BT_SCAN state means the controller is scanning for LE devices. Once
the target device is found, the connection goes to BT_DEV_FOUND
state, and then to BT_CONNECT state.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 include/net/bluetooth/bluetooth.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9531bee..add5721 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -126,7 +126,9 @@ enum {
 	BT_CONNECT2,
 	BT_CONFIG,
 	BT_DISCONN,
-	BT_CLOSED
+	BT_CLOSED,
+	BT_SCAN,
+	BT_DEV_FOUND
 };
 
 /* If unused will be removed by compiler */
@@ -151,6 +153,10 @@ static inline const char *state_to_string(int state)
 		return "BT_DISCONN";
 	case BT_CLOSED:
 		return "BT_CLOSED";
+	case BT_SCAN:
+		return "BT_SCAN";
+	case BT_DEV_FOUND:
+		return "BT_DEV_FOUND";
 	}
 
 	return "invalid state";
-- 
1.8.1.1

Re: [RFC 1/7] Bluetooth: Add new connection states

[Marcel Holtmann]

Hi Andre,

> This patch adds two new connection states which will be used by
> hci_conn to establish LE connections.
> 
> BT_SCAN state means the controller is scanning for LE devices. Once
> the target device is found, the connection goes to BT_DEV_FOUND
> state, and then to BT_CONNECT state.
> 
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  include/net/bluetooth/bluetooth.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9531bee..add5721 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -126,7 +126,9 @@ enum {
>  	BT_CONNECT2,
>  	BT_CONFIG,
>  	BT_DISCONN,
> -	BT_CLOSED
> +	BT_CLOSED,
> +	BT_SCAN,
> +	BT_DEV_FOUND
>  };

I am actually against this. These states where originally socket states
and not general states that we just pile on top of.

You need to create separate state handling for LE connection handling. I
am really not happy that we always try to shoe-horn this onto something
else.

Regards

Marcel

Re: [RFC 1/7] Bluetooth: Add new connection states

[Andre Guedes]

Hi Marcel,

On Fri, Feb 1, 2013 at 9:51 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Andre,
>
>> This patch adds two new connection states which will be used by
>> hci_conn to establish LE connections.
>>
>> BT_SCAN state means the controller is scanning for LE devices. Once
>> the target device is found, the connection goes to BT_DEV_FOUND
>> state, and then to BT_CONNECT state.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  include/net/bluetooth/bluetooth.h | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
>> index 9531bee..add5721 100644
>> --- a/include/net/bluetooth/bluetooth.h
>> +++ b/include/net/bluetooth/bluetooth.h
>> @@ -126,7 +126,9 @@ enum {
>>       BT_CONNECT2,
>>       BT_CONFIG,
>>       BT_DISCONN,
>> -     BT_CLOSED
>> +     BT_CLOSED,
>> +     BT_SCAN,
>> +     BT_DEV_FOUND
>>  };
>
> I am actually against this. These states where originally socket states
> and not general states that we just pile on top of.
>
> You need to create separate state handling for LE connection handling. I
> am really not happy that we always try to shoe-horn this onto something
> else.

Ok, I'll create a separated state for LE connection handling.

Regards,

Andre

[RFC 2/7] Bluetooth: Make hci_le_create_connection non-static

[Andre Guedes]

Make hci_le_create_connection helper non-static so it can be called
from outside hci_conn.c.

The helper will be used to create a LE connection once the target
device is in-range.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 90cf75a..00923ef 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -577,6 +577,7 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
 void hci_setup_sync(struct hci_conn *conn, __u16 handle);
 void hci_sco_setup(struct hci_conn *conn, __u8 status);
+void hci_le_create_connection(struct hci_conn *conn);
 
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
 int hci_conn_del(struct hci_conn *conn);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 25bfce0..bb9a88d 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -31,7 +31,7 @@
 #include <net/bluetooth/a2mp.h>
 #include <net/bluetooth/smp.h>
 
-static void hci_le_create_connection(struct hci_conn *conn)
+void hci_le_create_connection(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_cp_le_create_conn cp;
-- 
1.8.1.1

[RFC 3/7] Bluetooth: Setup LE scan with no timeout

[Andre Guedes]

This patch modifies hci_do_le_scan and hci_cancel_le_scan helpers so
we are able to start and stop LE scan with no timeout. This feature
will be used by the LE connection routine.

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

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22e77a7..3aa0345 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1618,26 +1618,27 @@ static int hci_do_le_scan(struct hci_dev *hdev, u8 type, u16 interval,
 	if (err < 0)
 		return err;
 
-	queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
-			   msecs_to_jiffies(timeout));
+	if (timeout > 0)
+		queue_delayed_work(hdev->workqueue, &hdev->le_scan_disable,
+				   msecs_to_jiffies(timeout));
 
 	return 0;
 }
 
 int hci_cancel_le_scan(struct hci_dev *hdev)
 {
+	struct hci_cp_le_set_scan_enable cp;
+
 	BT_DBG("%s", hdev->name);
 
 	if (!test_bit(HCI_LE_SCAN, &hdev->dev_flags))
 		return -EALREADY;
 
-	if (cancel_delayed_work(&hdev->le_scan_disable)) {
-		struct hci_cp_le_set_scan_enable cp;
+	cancel_delayed_work(&hdev->le_scan_disable);
 
-		/* Send HCI command to disable LE Scan */
-		memset(&cp, 0, sizeof(cp));
-		hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
-	}
+	/* Send HCI command to disable LE Scan */
+	memset(&cp, 0, sizeof(cp));
+	hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 
 	return 0;
 }
-- 
1.8.1.1

[RFC 4/7] Bluetooth: Add LE scan type macros

[Andre Guedes]

This patch adds macros for active and passive LE scan type values. It
also removes the LE_SCAN_TYPE macro since it is not used anymore.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 00923ef..b089968 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -117,6 +117,9 @@ struct oob_data {
 	u8 randomizer[16];
 };
 
+#define LE_SCAN_PASSIVE		0x00
+#define LE_SCAN_ACTIVE		0x01
+
 struct le_scan_params {
 	u8 type;
 	u16 interval;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 39395c7..43db12e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -106,7 +106,6 @@ static const u16 mgmt_events[] = {
  * These LE scan and inquiry parameters were chosen according to LE General
  * Discovery Procedure specification.
  */
-#define LE_SCAN_TYPE			0x01
 #define LE_SCAN_WIN			0x12
 #define LE_SCAN_INT			0x12
 #define LE_SCAN_TIMEOUT_LE_ONLY		10240	/* TGAP(gen_disc_scan_min) */
@@ -2485,7 +2484,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT,
+		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT,
 				  LE_SCAN_WIN, LE_SCAN_TIMEOUT_LE_ONLY);
 		break;
 
@@ -2497,7 +2496,7 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		err = hci_le_scan(hdev, LE_SCAN_TYPE, LE_SCAN_INT, LE_SCAN_WIN,
+		err = hci_le_scan(hdev, LE_SCAN_ACTIVE, LE_SCAN_INT, LE_SCAN_WIN,
 				  LE_SCAN_TIMEOUT_BREDR_LE);
 		break;
 
-- 
1.8.1.1

[RFC 5/7] Bluetooth: Change LE connection routine

[Andre Guedes]

In order to better support LE connection requirements, such as
multiple connections and re-connections, we should support the
general connection establishment procedure described in Core
spec. Today, we support only the direct connection establishment
procedure which has some limitations and, therefore, requires
extra connection management at user-space in order to support
LE connection requirements.

According to the spec, the general procedure is described as
follows: The host starts scanning for LE devices, once the
device we want to connect to is in-range, the host stops scanning
and initiates a connection. The procedure is terminated when the
connection is established or when the host terminates the procedure.

This patch changes the LE connection routine so we carry out the
general procedure instead of the direct procedure.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index bb9a88d..fa2caf2 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -516,7 +516,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 			return ERR_PTR(-ENOMEM);
 
 		le->dst_type = bdaddr_to_le(dst_type);
-		hci_le_create_connection(le);
+		le->state = BT_SCAN;
+
+		hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
 	}
 
 	le->pending_sec_level = sec_level;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 477726a..c9d2b71 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1260,6 +1260,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 {
 	struct hci_cp_le_set_scan_enable *cp;
 	__u8 status = *((__u8 *) skb->data);
+	struct hci_conn *hcon;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
 
@@ -1295,8 +1296,15 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
-		if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
-		    hdev->discovery.state == DISCOVERY_FINDING) {
+		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_DEV_FOUND);
+		if (hcon) {
+			hci_dev_lock(hdev);
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+			hci_dev_unlock(hdev);
+
+			hci_le_create_connection(hcon);
+		} else if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED &&
+			   hdev->discovery.state == DISCOVERY_FINDING) {
 			mgmt_interleaved_discovery(hdev);
 		} else {
 			hci_dev_lock(hdev);
@@ -3987,10 +3995,17 @@ 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;
+	struct hci_conn *hcon;
 
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
 
+		hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
+		if (hcon && hcon->dst_type == ev->bdaddr_type) {
+			hcon->state = BT_DEV_FOUND;
+			hci_cancel_le_scan(hdev);
+		}
+
 		rssi = ev->data[ev->length];
 		mgmt_device_found(hdev, &ev->bdaddr, LE_LINK, ev->bdaddr_type,
 				  NULL, rssi, 0, 1, ev->data, ev->length);
-- 
1.8.1.1

[RFC 6/7] Bluetooth: Handle hci_conn timeout in BT_SCAN state

[Andre Guedes]

If occurs a hci_conn timeout in BT_SCAN state we should stop the
ongoing LE scan.

Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
---
 net/bluetooth/hci_conn.c  |  6 ++++++
 net/bluetooth/hci_event.c | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fa2caf2..ac45725 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -269,6 +269,12 @@ static void hci_conn_timeout(struct work_struct *work)
 		return;
 
 	switch (conn->state) {
+	case BT_SCAN:
+		if (conn->type == LE_LINK) {
+			conn->state = BT_CLOSED;
+			hci_cancel_le_scan(conn->hdev);
+		}
+		break;
 	case BT_CONNECT:
 	case BT_CONNECT2:
 		if (conn->out) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c9d2b71..63c5d10 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1296,6 +1296,22 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
 
+		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CLOSED);
+		if (hcon) {
+			mgmt_connect_failed(hdev, &hcon->dst, hcon->type,
+					    hcon->dst_type,
+					    HCI_ERROR_LOCAL_HOST_TERM);
+
+			hci_proto_connect_cfm(hcon, HCI_ERROR_LOCAL_HOST_TERM);
+			hci_conn_del(hcon);
+
+			hci_dev_lock(hdev);
+			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+			hci_dev_unlock(hdev);
+
+			return;
+		}
+
 		hcon = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_DEV_FOUND);
 		if (hcon) {
 			hci_dev_lock(hdev);
-- 
1.8.1.1

[RFC 7/7] Bluetooth: Track the number of hci_conn in BT_SCAN

[Andre Guedes]

In order to avoid traversing the hci_conn list every time we get an
advertising report, we should keep a counter of connections in
BT_SCAN state. This way, we only traverse the list if the counter
is greater from zero.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b089968..b371434 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -290,6 +290,11 @@ struct hci_dev {
 	__u8			adv_data[HCI_MAX_AD_LENGTH];
 	__u8			adv_data_len;
 
+	/* This counter tracks the number of LE connections in scanning
+	 * state (BT_SCAN).
+	 */
+	atomic_t		le_conn_scan_cnt;
+
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ac45725..58fd681 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -262,6 +262,7 @@ static void hci_conn_timeout(struct work_struct *work)
 {
 	struct hci_conn *conn = container_of(work, struct hci_conn,
 					     disc_work.work);
+	struct hci_dev *hdev = conn->hdev;
 
 	BT_DBG("hcon %p state %s", conn, state_to_string(conn->state));
 
@@ -272,7 +273,8 @@ static void hci_conn_timeout(struct work_struct *work)
 	case BT_SCAN:
 		if (conn->type == LE_LINK) {
 			conn->state = BT_CLOSED;
-			hci_cancel_le_scan(conn->hdev);
+			atomic_dec(&hdev->le_conn_scan_cnt);
+			hci_cancel_le_scan(hdev);
 		}
 		break;
 	case BT_CONNECT:
@@ -524,6 +526,8 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 		le->dst_type = bdaddr_to_le(dst_type);
 		le->state = BT_SCAN;
 
+		atomic_inc(&hdev->le_conn_scan_cnt);
+
 		hci_le_scan(hdev, LE_SCAN_PASSIVE, 0x60, 0x30, 0);
 	}
 
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3aa0345..f5c2b01 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1742,6 +1742,8 @@ struct hci_dev *hci_alloc_dev(void)
 	hci_init_sysfs(hdev);
 	discovery_init(hdev);
 
+	atomic_set(&hdev->le_conn_scan_cnt, 0);
+
 	return hdev;
 }
 EXPORT_SYMBOL(hci_alloc_dev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 63c5d10..1464e5e 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4016,10 +4016,15 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
 	while (num_reports--) {
 		struct hci_ev_le_advertising_info *ev = ptr;
 
-		hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
-		if (hcon && hcon->dst_type == ev->bdaddr_type) {
-			hcon->state = BT_DEV_FOUND;
-			hci_cancel_le_scan(hdev);
+		if (atomic_read(&hdev->le_conn_scan_cnt) != 0) {
+			hcon = hci_conn_hash_lookup_ba(hdev, LE_LINK,
+						       &ev->bdaddr);
+
+			if (hcon && hcon->dst_type == ev->bdaddr_type) {
+				hcon->state = BT_DEV_FOUND;
+				atomic_dec(&hdev->le_conn_scan_cnt);
+				hci_cancel_le_scan(hdev);
+			}
 		}
 
 		rssi = ev->data[ev->length];
-- 
1.8.1.1