[PATCH 0/4] LE connection improvements

[PATCH 0/4] LE connection improvements

[Andre Guedes]

Hi all,

Currently, in order to establish a LE connection, the user must actively scan
for LE devices (e.g. discovery or hcitool lescan command) before trying to
connect to any device.

This approach is fine in common discovery + connect scenario (the user lists
in-range devices and then choose one to connect to), but it isn't suitable for
LE GATT profiles. In this later scenario, we're not interested in performing
discovery because dual mode devices will waste 5.12 seconds performing inquiry
which results we're not interested at all.

To address this issue we've come up with some LE connection improvements which
we plan to take in two steps:
	1. Passive scan trigger
	2. Early stop scan

1. Passive scan trigger
During LE connection, the destination address (the address we want to connect
to) may not be in the advertising cache. Instead of returning error, we trigger
the LE passive scan.

The scan window and interval is 30 ms and 60 ms, respectively, and it runs for
3869 ms. These parameters were chosen based on some simulation results sent to
Bluetooth SIG mailing list (GATT Profile Architecture Working Group) in April
2011.

Once the passive scan is finished, we check if we have the advertising entry
from the destination device. If so, we create the connection, otherwise the
connection attempt fails.

2. Early stop scan
Once we find the LE advertising from the destination address, we don't need to
continue scanning LE devices. If that happens, we cancel the ongoing scanning
and create the connection.


After those improvements we'll have a sort of optimized version of the General
Connection Establishment Procedure describe in Bluetooth spec (volume 3,
session 9.3.6).

The optimization is done by always checking the advertising cache before
triggering the passive scan. The idea is: if we already have cached the
advertising report from the destination device we don't need to carry out the
passive scan. This way, we have faster LE connection establishment.

Those steps are a beginning effort to improve the LE connection establishment
procedure. This RFC series* implements the "Passive scan trigger" step.

Feedback are welcome.

Regards,

Andre Guedes.

[*] Patch 1/4 isn't originally part of this patch series, but it may be needed
to understand the following patches. We're waiting for discovery patches been
pushed upstream to send it to the ML.

Andre Guedes (4):
  Bluetooth: hci_cc_le_set_scan_enable() critical region
  Bluetooth: Add hci_do_le_scan() helper function
  Bluetooth: Extend hci_conn_check_pending()
  Bluetooth: LE passive scan trigger in hci_connect()

 include/net/bluetooth/hci_core.h |    4 ++-
 net/bluetooth/hci_conn.c         |   53 ++++++++++++++++++++++++++++++++-----
 net/bluetooth/hci_core.c         |   46 +++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |   35 +++++++++++++-----------
 4 files changed, 113 insertions(+), 25 deletions(-)

-- 
1.7.5.2

[PATCH 1/4] Bluetooth: hci_cc_le_set_scan_enable() critical region

[Andre Guedes]

This patch reduces the critial region (protected by hdev->lock) in
hci_cc_le_set_scan_enable(). This way, only really needed code is
protected.

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

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 55872ff..9910f81 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -894,23 +894,24 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 	if (!cp)
 		return;
 
-	hci_dev_lock(hdev);
-
 	if (cp->enable == 0x01) {
 		if (status) {
 			mgmt_start_discovery_failed(hdev->id,
 							bt_to_errno(status));
-			goto unlock;
+			return;
 		}
 
 		set_bit(HCI_LE_SCAN, &hdev->flags);
 
 		del_timer(&hdev->adv_timer);
+
+		hci_dev_lock(hdev);
 		hci_adv_entries_clear(hdev);
+		hci_dev_unlock(hdev);
 
 		if (mgmt_has_pending_stop_discov(hdev->id)) {
 			mgmt_cancel_discovery(hdev->id);
-			goto unlock;
+			return;
 		}
 
 		if (!mgmt_is_interleaved_discovery(hdev->id))
@@ -924,7 +925,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 				mgmt_start_discovery_failed(hdev->id,
 							bt_to_errno(status));
 
-			goto unlock;
+			return;
 		}
 
 		clear_bit(HCI_LE_SCAN, &hdev->flags);
@@ -933,14 +934,14 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 
 		mgmt_discovering(hdev->id, 0);
 
-		if (mgmt_has_pending_stop_discov(hdev->id))
+		if (mgmt_has_pending_stop_discov(hdev->id)) {
 			mgmt_stop_discovery_complete(hdev->id);
-		else
+		} else {
+			hci_dev_lock(hdev);
 			mgmt_start_discovery_complete(hdev->id);
+			hci_dev_unlock(hdev);
+		}
 	}
-
-unlock:
-	hci_dev_unlock(hdev);
 }
 
 static void hci_cc_le_ltk_reply(struct hci_dev *hdev, struct sk_buff *skb)
-- 
1.7.5.2

[PATCH 2/4] Bluetooth: Add hci_do_le_scan() helper function

[Andre Guedes]

This patch adds a helper function in hci_core to perform LE scan.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1bc1c3a..8611e2e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -543,6 +543,8 @@ int hci_get_conn_list(void __user *arg);
 int hci_get_conn_info(struct hci_dev *hdev, void __user *arg);
 int hci_get_auth_info(struct hci_dev *hdev, void __user *arg);
 int hci_inquiry(void __user *arg);
+int hci_do_le_scan(struct hci_dev *hdev, int timeout, u8 type, u16 interval,
+								u16 window);
 
 struct bdaddr_list *hci_blacklist_lookup(struct hci_dev *hdev, bdaddr_t *bdaddr);
 int hci_blacklist_clear(struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8f9ed8c..dbca996 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2431,3 +2431,49 @@ static void hci_cmd_task(unsigned long arg)
 		}
 	}
 }
+
+static int set_le_scan_param(struct hci_dev *hdev, u8 type, u16 interval,
+								u16 window)
+{
+	struct hci_cp_le_set_scan_param cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.type = type;
+	cp.interval = cpu_to_le16(interval);
+	cp.window = cpu_to_le16(window);
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_PARAM, sizeof(cp), &cp);
+}
+
+static int le_scan(struct hci_dev *hdev, u8 enable)
+{
+	struct hci_cp_le_set_scan_enable cp;
+
+	memset(&cp, 0, sizeof(cp));
+	cp.enable = enable;
+
+	return hci_send_cmd(hdev, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
+}
+
+int hci_do_le_scan(struct hci_dev *hdev, int timeout, u8 type, u16 interval,
+								u16 window)
+{
+	int err;
+
+	BT_DBG("hdev %s", hdev->name);
+
+	if (test_bit(HCI_LE_SCAN, &hdev->flags))
+		return -EPERM;
+
+	err = set_le_scan_param(hdev, type, interval, window);
+	if (err < 0)
+		return err;
+
+	err = le_scan(hdev, 1);
+	if (err < 0)
+		return err;
+
+	mod_timer(&hdev->le_scan_timer, jiffies + msecs_to_jiffies(timeout));
+
+	return 0;
+}
-- 
1.7.5.2

[PATCH 3/4] Bluetooth: Extend hci_conn_check_pending()

[Andre Guedes]

This patch adds a new parameter to the hci_conn_check_pending().
This function was modified to check for pending hci connections
according to the link type.

In case there is a pending LE connection, we should check if the
LE advertising of the destination device was cached. If so, we
create a connection, otherwise the connection attempt fails.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8611e2e..6b3b632 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -450,7 +450,7 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status);
 struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst);
 int hci_conn_del(struct hci_conn *conn);
 void hci_conn_hash_flush(struct hci_dev *hdev);
-void hci_conn_check_pending(struct hci_dev *hdev);
+void hci_conn_check_pending(struct hci_dev *hdev, u8 type);
 
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 						__u8 sec_level, __u8 auth_type);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index fa6820e..cc36358 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -825,19 +825,46 @@ void hci_conn_hash_flush(struct hci_dev *hdev)
 }
 
 /* Check pending connect attempts */
-void hci_conn_check_pending(struct hci_dev *hdev)
+void hci_conn_check_pending(struct hci_dev *hdev, u8 type)
 {
 	struct hci_conn *conn;
+	struct adv_entry *entry;
 
 	BT_DBG("hdev %s", hdev->name);
 
 	hci_dev_lock(hdev);
 
-	conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2);
-	if (conn)
-		hci_acl_connect(conn);
+	switch (type) {
+	case ACL_LINK:
+		conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2);
+		if (conn)
+			hci_acl_connect(conn);
+
+		break;
+	case LE_LINK:
+		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_OPEN);
+		if (!conn)
+			goto unlock;
+
+		entry = hci_find_adv_entry(hdev, &conn->dst);
+		if (!entry) {
+			u8 status = 0x04; /* mapped into EHOSTDOWN errno */
+			mgmt_connect_failed(hdev->id, &conn->dst, status);
+			hci_proto_connect_cfm(conn, status);
+			conn->state = BT_CLOSED;
+			hci_conn_del(conn);
+			goto unlock;
+		}
 
+		conn->dst_type = entry->bdaddr_type;
+		hci_le_connect(conn);
+
+		break;
+	}
+
+unlock:
 	hci_dev_unlock(hdev);
+
 }
 
 void hci_conn_hold_device(struct hci_conn *conn)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9910f81..6bbf4d7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -64,7 +64,7 @@ static void hci_cc_inquiry_cancel(struct hci_dev *hdev, struct sk_buff *skb)
 
 	hci_req_complete(hdev, HCI_OP_INQUIRY_CANCEL, status);
 
-	hci_conn_check_pending(hdev);
+	hci_conn_check_pending(hdev, ACL_LINK);
 
 	mgmt_discovering(hdev->id, 0);
 	mgmt_stop_discovery_complete(hdev->id);
@@ -79,7 +79,7 @@ static void hci_cc_exit_periodic_inq(struct hci_dev *hdev, struct sk_buff *skb)
 	if (status)
 		return;
 
-	hci_conn_check_pending(hdev);
+	hci_conn_check_pending(hdev, ACL_LINK);
 }
 
 static void hci_cc_remote_name_req_cancel(struct hci_dev *hdev, struct sk_buff *skb)
@@ -989,7 +989,7 @@ static inline void hci_cs_inquiry(struct hci_dev *hdev, __u8 status)
 
 	if (status) {
 		hci_req_complete(hdev, HCI_OP_INQUIRY, status);
-		hci_conn_check_pending(hdev);
+		hci_conn_check_pending(hdev, ACL_LINK);
 
 		mgmt_start_discovery_failed(hdev->id, bt_to_errno(status));
 
@@ -1381,7 +1381,7 @@ static inline void hci_inquiry_complete_evt(struct hci_dev *hdev, struct sk_buff
 
 	BT_DBG("%s status %d", hdev->name, status);
 
-	hci_conn_check_pending(hdev);
+	hci_conn_check_pending(hdev, ACL_LINK);
 
 	if (!test_bit(HCI_INQUIRY, &hdev->flags))
 		return;
@@ -1514,7 +1514,7 @@ static inline void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *s
 unlock:
 	hci_dev_unlock(hdev);
 
-	hci_conn_check_pending(hdev);
+	hci_conn_check_pending(hdev, ACL_LINK);
 }
 
 static inline void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1693,7 +1693,7 @@ static inline void hci_remote_name_evt(struct hci_dev *hdev, struct sk_buff *skb
 
 	BT_DBG("%s", hdev->name);
 
-	hci_conn_check_pending(hdev);
+	hci_conn_check_pending(hdev, ACL_LINK);
 
 	hci_dev_lock(hdev);
 
-- 
1.7.5.2

[PATCH 4/4] Bluetooth: LE passive scan trigger in hci_connect()

[Andre Guedes]

During LE connection, if the entry isn't found in the advertising
cache the passive scan is triggered and the connection creation is
postponed until the passive scan finishes.

The scan parameters were chosen based on some simulation results sent
to Bluetooth SIG mailing list (GATT Profile Architecture Working Group)
in April 2011.

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

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index cc36358..1f404cf 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -507,14 +507,24 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
 		if (le)
 			return ERR_PTR(-EBUSY);
 
-		entry = hci_find_adv_entry(hdev, dst);
-		if (!entry)
-			return ERR_PTR(-EHOSTUNREACH);
-
 		le = hci_conn_add(hdev, LE_LINK, dst);
 		if (!le)
 			return ERR_PTR(-ENOMEM);
 
+		entry = hci_find_adv_entry(hdev, dst);
+		if (!entry) {
+			int err;
+
+			err = hci_do_le_scan(hdev, 3869, 0x00, 0x60, 0x30);
+			if (err < 0) {
+				hci_conn_del(le);
+				return ERR_PTR(err);
+			}
+
+			hci_conn_hold(le);
+			return le;
+		}
+
 		le->dst_type = entry->bdaddr_type;
 
 		hci_le_connect(le);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 6bbf4d7..540ee81 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -941,6 +941,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 			mgmt_start_discovery_complete(hdev->id);
 			hci_dev_unlock(hdev);
 		}
+
+		hci_conn_check_pending(hdev, LE_LINK);
 	}
 }
 
-- 
1.7.5.2

Re: [PATCH 4/4] Bluetooth: LE passive scan trigger in hci_connect()

[Sumit Bajpai]

Hi Andre,

Is there any update on this patch acceptance from community ?

Best Regards
Hemant

On Wed, Aug 10, 2011 at 4:22 AM, Andre Guedes
<andre.guedes@openbossa.org> wrote:
> During LE connection, if the entry isn't found in the advertising
> cache the passive scan is triggered and the connection creation is
> postponed until the passive scan finishes.
>
> The scan parameters were chosen based on some simulation results sent
> to Bluetooth SIG mailing list (GATT Profile Architecture Working Group)
> in April 2011.
>
> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> ---
>  net/bluetooth/hci_conn.c  |   18 ++++++++++++++----
>  net/bluetooth/hci_event.c |    2 ++
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index cc36358..1f404cf 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -507,14 +507,24 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>                if (le)
>                        return ERR_PTR(-EBUSY);
>
> -               entry = hci_find_adv_entry(hdev, dst);
> -               if (!entry)
> -                       return ERR_PTR(-EHOSTUNREACH);
> -
>                le = hci_conn_add(hdev, LE_LINK, dst);
>                if (!le)
>                        return ERR_PTR(-ENOMEM);
>
> +               entry = hci_find_adv_entry(hdev, dst);
> +               if (!entry) {
> +                       int err;
> +
> +                       err = hci_do_le_scan(hdev, 3869, 0x00, 0x60, 0x30);
> +                       if (err < 0) {
> +                               hci_conn_del(le);
> +                               return ERR_PTR(err);
> +                       }
> +
> +                       hci_conn_hold(le);
> +                       return le;
> +               }
> +
>                le->dst_type = entry->bdaddr_type;
>
>                hci_le_connect(le);
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 6bbf4d7..540ee81 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -941,6 +941,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>                        mgmt_start_discovery_complete(hdev->id);
>                        hci_dev_unlock(hdev);
>                }
> +
> +               hci_conn_check_pending(hdev, LE_LINK);
>        }
>  }
>
> --
> 1.7.5.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] Bluetooth: LE passive scan trigger in hci_connect()

[Andre Guedes]

Hi Hemant,

This work depends on discovery patches which are not upstream yet. I'm
rebasing discovery series and I'll resend it by the end of this week.

As soon as we have discovery upstream we'll work on this LE connection
improvements.

BR,

Andre

On Tue, Dec 20, 2011 at 7:04 AM, Sumit Bajpai <sumit.btle@gmail.com> wrote:
> Hi Andre,
>
> Is there any update on this patch acceptance from community ?
>
> Best Regards
> Hemant
>
> On Wed, Aug 10, 2011 at 4:22 AM, Andre Guedes
> <andre.guedes@openbossa.org> wrote:
>> During LE connection, if the entry isn't found in the advertising
>> cache the passive scan is triggered and the connection creation is
>> postponed until the passive scan finishes.
>>
>> The scan parameters were chosen based on some simulation results sent
>> to Bluetooth SIG mailing list (GATT Profile Architecture Working Group)
>> in April 2011.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
>> ---
>>  net/bluetooth/hci_conn.c  |   18 ++++++++++++++----
>>  net/bluetooth/hci_event.c |    2 ++
>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index cc36358..1f404cf 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -507,14 +507,24 @@ struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst, __u8
>>                if (le)
>>                        return ERR_PTR(-EBUSY);
>>
>> -               entry = hci_find_adv_entry(hdev, dst);
>> -               if (!entry)
>> -                       return ERR_PTR(-EHOSTUNREACH);
>> -
>>                le = hci_conn_add(hdev, LE_LINK, dst);
>>                if (!le)
>>                        return ERR_PTR(-ENOMEM);
>>
>> +               entry = hci_find_adv_entry(hdev, dst);
>> +               if (!entry) {
>> +                       int err;
>> +
>> +                       err = hci_do_le_scan(hdev, 3869, 0x00, 0x60, 0x30);
>> +                       if (err < 0) {
>> +                               hci_conn_del(le);
>> +                               return ERR_PTR(err);
>> +                       }
>> +
>> +                       hci_conn_hold(le);
>> +                       return le;
>> +               }
>> +
>>                le->dst_type = entry->bdaddr_type;
>>
>>                hci_le_connect(le);
>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>> index 6bbf4d7..540ee81 100644
>> --- a/net/bluetooth/hci_event.c
>> +++ b/net/bluetooth/hci_event.c
>> @@ -941,6 +941,8 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
>>                        mgmt_start_discovery_complete(hdev->id);
>>                        hci_dev_unlock(hdev);
>>                }
>> +
>> +               hci_conn_check_pending(hdev, LE_LINK);
>>        }
>>  }
>>
>> --
>> 1.7.5.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html