linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes
@ 2014-07-08 12:07 johan.hedberg
  2014-07-08 12:07 ` [PATCH 1/7] Bluetooth: Add flag to track the real advertising state johan.hedberg
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This set of patches tries to improve the current behavior of LE scanning
and advertising so that both features can be used nicely together to the
extent that current LE hardware supports them.

Johan

----------------------------------------------------------------
Johan Hedberg (7):
      Bluetooth: Add flag to track the real advertising state
      Bluetooth: Remove unnecessary mgmt_advertising function
      Bluetooth: Use real advertising state to random address update decision
      Bluetooth: Simplify usage of the enable_advertising function
      Bluetooth: Use the correct flag to decide to disable advertising
      Bluetooth: Stop advertising always before initiating a connection
      Bluetooth: Fix advertising and active scanning co-existence

 include/net/bluetooth/hci.h      |  4 +-
 include/net/bluetooth/hci_core.h |  1 -
 net/bluetooth/hci_conn.c         | 29 +++++++++--
 net/bluetooth/hci_core.c         |  2 +-
 net/bluetooth/hci_event.c        | 16 ++++--
 net/bluetooth/mgmt.c             | 98 ++++++++++++++++---------------------
 6 files changed, 83 insertions(+), 67 deletions(-)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/7] Bluetooth: Add flag to track the real advertising state
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 2/7] Bluetooth: Remove unnecessary mgmt_advertising function johan.hedberg
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Having a single HCI_ADVERTISING flag is problematic since it tries to
track both the real advertising state and the corresponding mgmt
setting. To make the logic simpler and more reliable add a new flag that
only tracks the actual advertising state that has been written to the
controller.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci.h | 4 ++--
 net/bluetooth/hci_event.c   | 4 ++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5481d1c8badb..6ed1f7288f13 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -175,7 +175,7 @@ enum {
 	HCI_UNCONFIGURED,
 	HCI_USER_CHANNEL,
 	HCI_EXT_CONFIGURED,
-
+	HCI_LE_ADV,
 	HCI_LE_SCAN,
 	HCI_SSP_ENABLED,
 	HCI_SC_ENABLED,
@@ -200,7 +200,7 @@ enum {
  * or the HCI device is closed.
  */
 #define HCI_PERSISTENT_MASK (BIT(HCI_LE_SCAN) | BIT(HCI_PERIODIC_INQ) | \
-			      BIT(HCI_FAST_CONNECTABLE))
+			      BIT(HCI_FAST_CONNECTABLE) | BIT(HCI_LE_ADV))
 
 /* HCI ioctl defines */
 #define HCIDEVUP	_IOW('H', 201, int)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a6816498b0b9..10f8de9400bc 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1069,11 +1069,15 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	if (*sent) {
 		struct hci_conn *conn;
 
+		set_bit(HCI_LE_ADV, &hdev->dev_flags);
+
 		conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
 		if (conn)
 			queue_delayed_work(hdev->workqueue,
 					   &conn->le_conn_timeout,
 					   conn->conn_timeout);
+	} else {
+		clear_bit(HCI_LE_ADV, &hdev->dev_flags);
 	}
 
 	mgmt_advertising(hdev, *sent);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/7] Bluetooth: Remove unnecessary mgmt_advertising function
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
  2014-07-08 12:07 ` [PATCH 1/7] Bluetooth: Add flag to track the real advertising state johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 3/7] Bluetooth: Use real advertising state to random address update decision johan.hedberg
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Since the real advertising state is now tracked with its own flag we can
simply set/unset the HCI_ADVERTISING flag in the
set_advertising_complete function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  1 -
 net/bluetooth/hci_event.c        |  2 --
 net/bluetooth/mgmt.c             | 17 +++++------------
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5701d15779dd..c98de309967e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1292,7 +1292,6 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered);
 void mgmt_discoverable_timeout(struct hci_dev *hdev);
 void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable);
 void mgmt_connectable(struct hci_dev *hdev, u8 connectable);
-void mgmt_advertising(struct hci_dev *hdev, u8 advertising);
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status);
 void mgmt_new_link_key(struct hci_dev *hdev, struct link_key *key,
 		       bool persistent);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 10f8de9400bc..8fbf604ba228 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1080,8 +1080,6 @@ static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
 		clear_bit(HCI_LE_ADV, &hdev->dev_flags);
 	}
 
-	mgmt_advertising(hdev, *sent);
-
 	hci_dev_unlock(hdev);
 }
 
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9cc7108f4c45..dda1eb124208 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4034,6 +4034,11 @@ static void set_advertising_complete(struct hci_dev *hdev, u8 status)
 		return;
 	}
 
+	if (test_bit(HCI_LE_ADV, &hdev->dev_flags))
+		set_bit(HCI_ADVERTISING, &hdev->dev_flags);
+	else
+		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+
 	mgmt_pending_foreach(MGMT_OP_SET_ADVERTISING, hdev, settings_rsp,
 			     &match);
 
@@ -5978,18 +5983,6 @@ void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
 		new_settings(hdev, NULL);
 }
 
-void mgmt_advertising(struct hci_dev *hdev, u8 advertising)
-{
-	/* Powering off may stop advertising - don't let that interfere */
-	if (!advertising && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
-		return;
-
-	if (advertising)
-		set_bit(HCI_ADVERTISING, &hdev->dev_flags);
-	else
-		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
-}
-
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status)
 {
 	u8 mgmt_err = mgmt_status(status);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/7] Bluetooth: Use real advertising state to random address update decision
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
  2014-07-08 12:07 ` [PATCH 1/7] Bluetooth: Add flag to track the real advertising state johan.hedberg
  2014-07-08 12:07 ` [PATCH 2/7] Bluetooth: Remove unnecessary mgmt_advertising function johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 4/7] Bluetooth: Simplify usage of the enable_advertising function johan.hedberg
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Now that we have a flag for tracking the real advertising state we
should use that to determine whether it's safe to update the random
address or not. The couple of places that were clearing the flag due to
a pending request need to be updated too.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c | 4 ++--
 net/bluetooth/hci_core.c | 2 +-
 net/bluetooth/mgmt.c     | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 16fd55da9c1d..0db2579ea6c6 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -671,12 +671,12 @@ static void hci_req_directed_advertising(struct hci_request *req,
 	enable = 0x00;
 	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
 
-	/* Clear the HCI_ADVERTISING bit temporarily so that the
+	/* Clear the HCI_LE_ADV bit temporarily so that the
 	 * hci_update_random_address knows that it's safe to go ahead
 	 * and write a new random address. The flag will be set back on
 	 * as soon as the SET_ADV_ENABLE HCI command completes.
 	 */
-	clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+	clear_bit(HCI_LE_ADV, &hdev->dev_flags);
 
 	/* Set require_privacy to false so that the remote device has a
 	 * chance of identifying us.
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f1c5a077e558..8ffaca0290f8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3746,7 +3746,7 @@ static void set_random_addr(struct hci_request *req, bdaddr_t *rpa)
 	 * In this kind of scenario skip the update and let the random
 	 * address be updated at the next cycle.
 	 */
-	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags) ||
+	if (test_bit(HCI_LE_ADV, &hdev->dev_flags) ||
 	    hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT)) {
 		BT_DBG("Deferring random address update");
 		return;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index dda1eb124208..be589d8d437f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1046,12 +1046,12 @@ static void enable_advertising(struct hci_request *req)
 	u8 own_addr_type, enable = 0x01;
 	bool connectable;
 
-	/* Clear the HCI_ADVERTISING bit temporarily so that the
+	/* Clear the HCI_LE_ADV bit temporarily so that the
 	 * hci_update_random_address knows that it's safe to go ahead
 	 * and write a new random address. The flag will be set back on
 	 * as soon as the SET_ADV_ENABLE HCI command completes.
 	 */
-	clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
+	clear_bit(HCI_LE_ADV, &hdev->dev_flags);
 
 	connectable = get_connectable(hdev);
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/7] Bluetooth: Simplify usage of the enable_advertising function
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
                   ` (2 preceding siblings ...)
  2014-07-08 12:07 ` [PATCH 3/7] Bluetooth: Use real advertising state to random address update decision johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 5/7] Bluetooth: Use the correct flag to decide to disable advertising johan.hedberg
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

By adding support for disabling advertising when necessary and doing the
checks for existing LE connections inside the enable_advertising
function we can simplify the calling code quite a lot.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 49 ++++++++++++++++---------------------------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index be589d8d437f..68c0698124fb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1039,6 +1039,13 @@ static bool get_connectable(struct hci_dev *hdev)
 	return test_bit(HCI_CONNECTABLE, &hdev->dev_flags);
 }
 
+static void disable_advertising(struct hci_request *req)
+{
+	u8 enable = 0x00;
+
+	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
+}
+
 static void enable_advertising(struct hci_request *req)
 {
 	struct hci_dev *hdev = req->hdev;
@@ -1046,6 +1053,12 @@ static void enable_advertising(struct hci_request *req)
 	u8 own_addr_type, enable = 0x01;
 	bool connectable;
 
+	if (hci_conn_num(hdev, LE_LINK) > 0)
+		return;
+
+	if (test_bit(HCI_LE_ADV, &hdev->dev_flags))
+		disable_advertising(req);
+
 	/* Clear the HCI_LE_ADV bit temporarily so that the
 	 * hci_update_random_address knows that it's safe to go ahead
 	 * and write a new random address. The flag will be set back on
@@ -1074,13 +1087,6 @@ static void enable_advertising(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
 }
 
-static void disable_advertising(struct hci_request *req)
-{
-	u8 enable = 0x00;
-
-	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
-}
-
 static void service_cache_off(struct work_struct *work)
 {
 	struct hci_dev *hdev = container_of(work, struct hci_dev,
@@ -1112,19 +1118,14 @@ static void rpa_expired(struct work_struct *work)
 
 	set_bit(HCI_RPA_EXPIRED, &hdev->dev_flags);
 
-	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags) ||
-	    hci_conn_num(hdev, LE_LINK) > 0)
+	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags))
 		return;
 
 	/* The generation of a new RPA and programming it into the
 	 * controller happens in the enable_advertising() function.
 	 */
-
 	hci_req_init(&req, hdev);
-
-	disable_advertising(&req);
 	enable_advertising(&req);
-
 	hci_req_run(&req, NULL);
 }
 
@@ -1864,10 +1865,8 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 		write_fast_connectable(&req, false);
 
 	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags) &&
-	    hci_conn_num(hdev, LE_LINK) == 0) {
-		disable_advertising(&req);
+	    !test_bit(HCI_LE_ADV, &hdev->dev_flags))
 		enable_advertising(&req);
-	}
 
 	err = hci_req_run(&req, set_connectable_complete);
 	if (err < 0) {
@@ -6809,32 +6808,16 @@ void mgmt_discovering(struct hci_dev *hdev, u8 discovering)
 static void adv_enable_complete(struct hci_dev *hdev, u8 status)
 {
 	BT_DBG("%s status %u", hdev->name, status);
-
-	/* Clear the advertising mgmt setting if we failed to re-enable it */
-	if (status) {
-		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
-		new_settings(hdev, NULL);
-	}
 }
 
 void mgmt_reenable_advertising(struct hci_dev *hdev)
 {
 	struct hci_request req;
 
-	if (hci_conn_num(hdev, LE_LINK) > 0)
-		return;
-
 	if (!test_bit(HCI_ADVERTISING, &hdev->dev_flags))
 		return;
 
 	hci_req_init(&req, hdev);
 	enable_advertising(&req);
-
-	/* If this fails we have no option but to let user space know
-	 * that we've disabled advertising.
-	 */
-	if (hci_req_run(&req, adv_enable_complete) < 0) {
-		clear_bit(HCI_ADVERTISING, &hdev->dev_flags);
-		new_settings(hdev, NULL);
-	}
+	hci_req_run(&req, adv_enable_complete);
 }
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/7] Bluetooth: Use the correct flag to decide to disable advertising
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
                   ` (3 preceding siblings ...)
  2014-07-08 12:07 ` [PATCH 4/7] Bluetooth: Simplify usage of the enable_advertising function johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 6/7] Bluetooth: Stop advertising always before initiating a connection johan.hedberg
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

When deciding to call disable_advertising() we're interested in the real
state instead of the mgmt setting. Use therefore HCI_LE_ADV instead of
the HCI_ADVERTISING flag.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/mgmt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 68c0698124fb..9549d7366da2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1301,7 +1301,7 @@ static int clean_up_hci_state(struct hci_dev *hdev)
 		hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 	}
 
-	if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
+	if (test_bit(HCI_LE_ADV, &hdev->dev_flags))
 		disable_advertising(&req);
 
 	hci_stop_discovery(&req);
@@ -2230,7 +2230,7 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 		hci_cp.le = val;
 		hci_cp.simul = lmp_le_br_capable(hdev);
 	} else {
-		if (test_bit(HCI_ADVERTISING, &hdev->dev_flags))
+		if (test_bit(HCI_LE_ADV, &hdev->dev_flags))
 			disable_advertising(&req);
 	}
 
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/7] Bluetooth: Stop advertising always before initiating a connection
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
                   ` (4 preceding siblings ...)
  2014-07-08 12:07 ` [PATCH 5/7] Bluetooth: Use the correct flag to decide to disable advertising johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:07 ` [PATCH 7/7] Bluetooth: Fix advertising and active scanning co-existence johan.hedberg
  2014-07-08 12:32 ` [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes Marcel Holtmann
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Most controllers do not support advertising while initiating an LE
connection. We also have to first disable current advertising if the
initiation is going to happen through direct advertising. Therefore,
simply stop advertising as the first thing when starting to issue
commands to establish an LE connection.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0db2579ea6c6..1517f1549f85 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -668,9 +668,6 @@ static void hci_req_directed_advertising(struct hci_request *req,
 	u8 own_addr_type;
 	u8 enable;
 
-	enable = 0x00;
-	hci_req_add(req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable), &enable);
-
 	/* Clear the HCI_LE_ADV bit temporarily so that the
 	 * hci_update_random_address knows that it's safe to go ahead
 	 * and write a new random address. The flag will be set back on
@@ -761,6 +758,18 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 
 	hci_req_init(&req, hdev);
 
+	/* Disable advertising if we're active. For master role
+	 * connections most controllers will refuse to connect if
+	 * advertising is enabled, and for slave role connections we
+	 * anyway have to disable it in order to start directed
+	 * advertising.
+	 */
+	if (test_bit(HCI_LE_ADV, &hdev->dev_flags)) {
+		u8 enable = 0x00;
+		hci_req_add(&req, HCI_OP_LE_SET_ADV_ENABLE, sizeof(enable),
+			    &enable);
+	}
+
 	/* If requested to connect as slave use directed advertising */
 	if (!master) {
 		hci_req_directed_advertising(&req, conn);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/7] Bluetooth: Fix advertising and active scanning co-existence
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
                   ` (5 preceding siblings ...)
  2014-07-08 12:07 ` [PATCH 6/7] Bluetooth: Stop advertising always before initiating a connection johan.hedberg
@ 2014-07-08 12:07 ` johan.hedberg
  2014-07-08 12:32 ` [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes Marcel Holtmann
  7 siblings, 0 replies; 9+ messages in thread
From: johan.hedberg @ 2014-07-08 12:07 UTC (permalink / raw)
  To: linux-bluetooth

From: Johan Hedberg <johan.hedberg@intel.com>

Many controllers allow simultaneous active scanning and advertising
(e.g. Intel and Broadcom) but some do not (e.g. CSR). It's therefore
safest to implement mutual exclusion of these states in the kernel.

This patch ensures that the two states are never entered simultaneously.
Extra precaution needs to be taken for outgoing connection attempts in
slave role (i.e. through directed advertising) in which case the
operation that came first has precedence and the one that comes after
gets a rejection.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/hci_conn.c  | 10 ++++++++++
 net/bluetooth/hci_event.c | 10 +++++++++-
 net/bluetooth/mgmt.c      | 24 ++++++++++++++++++------
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 1517f1549f85..490ee8846d9e 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -772,6 +772,16 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 
 	/* If requested to connect as slave use directed advertising */
 	if (!master) {
+		/* If we're active scanning most controllers are unable
+		 * to initiate advertising. Simply reject the attempt.
+		 */
+		if (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
+		    hdev->le_scan_type == LE_SCAN_ACTIVE) {
+			skb_queue_purge(&req.cmd_q);
+			hci_conn_del(conn);
+			return ERR_PTR(-EBUSY);
+		}
+
 		hci_req_directed_advertising(&req, conn);
 		goto create_conn;
 	}
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8fbf604ba228..5d3095d7d4b0 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1176,13 +1176,21 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev,
 		cancel_delayed_work(&hdev->le_scan_disable);
 
 		clear_bit(HCI_LE_SCAN, &hdev->dev_flags);
+
 		/* The HCI_LE_SCAN_INTERRUPTED flag indicates that we
 		 * interrupted scanning due to a connect request. Mark
-		 * therefore discovery as stopped.
+		 * therefore discovery as stopped. If this was not
+		 * because of a connect request advertising might have
+		 * been disabled because of active scanning, so
+		 * re-enable it again if necessary.
 		 */
 		if (test_and_clear_bit(HCI_LE_SCAN_INTERRUPTED,
 				       &hdev->dev_flags))
 			hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
+		else if (!test_bit(HCI_LE_ADV, &hdev->dev_flags) &&
+			 hdev->discovery.state != DISCOVERY_STARTING)
+			mgmt_reenable_advertising(hdev);
+
 		break;
 
 	default:
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9549d7366da2..944e6463fd61 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3726,11 +3726,21 @@ static int start_discovery(struct sock *sk, struct hci_dev *hdev,
 			goto failed;
 		}
 
-		if (test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
-			err = cmd_status(sk, hdev->id, MGMT_OP_START_DISCOVERY,
-					 MGMT_STATUS_REJECTED);
-			mgmt_pending_remove(cmd);
-			goto failed;
+		if (test_bit(HCI_LE_ADV, &hdev->dev_flags)) {
+			/* Don't let discovery abort an outgoing
+			 * connection attempt that's using directed
+			 * advertising.
+			 */
+			if (hci_conn_hash_lookup_state(hdev, LE_LINK,
+						       BT_CONNECT)) {
+				err = cmd_status(sk, hdev->id,
+						 MGMT_OP_START_DISCOVERY,
+						 MGMT_STATUS_REJECTED);
+				mgmt_pending_remove(cmd);
+				goto failed;
+			}
+
+			disable_advertising(&req);
 		}
 
 		/* If controller is scanning, it means the background scanning
@@ -4078,7 +4088,9 @@ static int set_advertising(struct sock *sk, struct hci_dev *hdev, void *data,
 	 * necessary).
 	 */
 	if (!hdev_is_powered(hdev) || val == enabled ||
-	    hci_conn_num(hdev, LE_LINK) > 0) {
+	    hci_conn_num(hdev, LE_LINK) > 0 ||
+	    (test_bit(HCI_LE_SCAN, &hdev->dev_flags) &&
+	     hdev->le_scan_type == LE_SCAN_ACTIVE)) {
 		bool changed = false;
 
 		if (val != test_bit(HCI_ADVERTISING, &hdev->dev_flags)) {
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes
  2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
                   ` (6 preceding siblings ...)
  2014-07-08 12:07 ` [PATCH 7/7] Bluetooth: Fix advertising and active scanning co-existence johan.hedberg
@ 2014-07-08 12:32 ` Marcel Holtmann
  7 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2014-07-08 12:32 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

Hi Johan,

> This set of patches tries to improve the current behavior of LE scanning
> and advertising so that both features can be used nicely together to the
> extent that current LE hardware supports them.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (7):
>      Bluetooth: Add flag to track the real advertising state
>      Bluetooth: Remove unnecessary mgmt_advertising function
>      Bluetooth: Use real advertising state to random address update decision
>      Bluetooth: Simplify usage of the enable_advertising function
>      Bluetooth: Use the correct flag to decide to disable advertising
>      Bluetooth: Stop advertising always before initiating a connection
>      Bluetooth: Fix advertising and active scanning co-existence
> 
> include/net/bluetooth/hci.h      |  4 +-
> include/net/bluetooth/hci_core.h |  1 -
> net/bluetooth/hci_conn.c         | 29 +++++++++--
> net/bluetooth/hci_core.c         |  2 +-
> net/bluetooth/hci_event.c        | 16 ++++--
> net/bluetooth/mgmt.c             | 98 ++++++++++++++++---------------------
> 6 files changed, 83 insertions(+), 67 deletions(-)

all 7 patches have been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-07-08 12:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-08 12:07 [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes johan.hedberg
2014-07-08 12:07 ` [PATCH 1/7] Bluetooth: Add flag to track the real advertising state johan.hedberg
2014-07-08 12:07 ` [PATCH 2/7] Bluetooth: Remove unnecessary mgmt_advertising function johan.hedberg
2014-07-08 12:07 ` [PATCH 3/7] Bluetooth: Use real advertising state to random address update decision johan.hedberg
2014-07-08 12:07 ` [PATCH 4/7] Bluetooth: Simplify usage of the enable_advertising function johan.hedberg
2014-07-08 12:07 ` [PATCH 5/7] Bluetooth: Use the correct flag to decide to disable advertising johan.hedberg
2014-07-08 12:07 ` [PATCH 6/7] Bluetooth: Stop advertising always before initiating a connection johan.hedberg
2014-07-08 12:07 ` [PATCH 7/7] Bluetooth: Fix advertising and active scanning co-existence johan.hedberg
2014-07-08 12:32 ` [PATCH 0/7] Bluetooth: Scanning & advertising co-existence fixes Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).