[PATCH 1/4] Bluetooth: Remove unneeded mgmt_connectable function

[PATCH 1/4] Bluetooth: Remove unneeded mgmt_connectable function

[johan.hedberg]

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

The mgmt_connectable function has been used to ensure that the right
actions to HCI_CONNECTABLE are taken when the HCI_Write_Scan_Enable
command is triggered by something else than mgmt. The only other user
that we really care about is the HCISETSCAN ioctl code, so we can
actually more simply perform the needed changes there instead.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e69c2b08c0c6..76675c55be15 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1288,7 +1288,6 @@ void mgmt_set_powered_failed(struct hci_dev *hdev, int err);
 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_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_core.c b/net/bluetooth/hci_core.c
index 8f9df768f250..3844eeb85453 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2717,9 +2717,9 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 				   HCI_INIT_TIMEOUT);
 
 		/* Ensure that the connectable state gets correctly
-		 * notified if the whitelist is in use.
+		 * modified as this was a non-mgmt change.
 		 */
-		if (!err && !list_empty(&hdev->whitelist)) {
+		if (!err) {
 			bool changed;
 
 			if ((dr.dev_opt & SCAN_PAGE))
@@ -2729,7 +2729,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 				changed = test_and_clear_bit(HCI_CONNECTABLE,
 							     &hdev->dev_flags);
 
-			if (changed)
+			if (changed && test_bit(HCI_MGMT, &hdev->dev_flags))
 				mgmt_new_settings(hdev);
 		}
 		break;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index c8ae9ee3cb12..38a0e457eaf9 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -330,12 +330,8 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	} else if (old_iscan)
 		mgmt_discoverable(hdev, 0);
 
-	if (param & SCAN_PAGE) {
+	if (param & SCAN_PAGE)
 		set_bit(HCI_PSCAN, &hdev->flags);
-		if (!old_pscan)
-			mgmt_connectable(hdev, 1);
-	} else if (old_pscan)
-		mgmt_connectable(hdev, 0);
 
 done:
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 38f05386bc0c..9f9f11c8488b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6068,38 +6068,6 @@ void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
 	}
 }
 
-void mgmt_connectable(struct hci_dev *hdev, u8 connectable)
-{
-	bool changed;
-
-	/* Nothing needed here if there's a pending command since that
-	 * commands request completion callback takes care of everything
-	 * necessary.
-	 */
-	if (mgmt_pending_find(MGMT_OP_SET_CONNECTABLE, hdev))
-		return;
-
-	/* Powering off may clear the scan mode - don't let that interfere */
-	if (!connectable && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
-		return;
-
-	/* If something else than mgmt changed the page scan state we
-	 * can't differentiate this from a change triggered by adding
-	 * the first element to the whitelist. Therefore, avoid
-	 * incorrectly setting HCI_CONNECTABLE.
-	 */
-	if (connectable && !list_empty(&hdev->whitelist))
-		return;
-
-	if (connectable)
-		changed = !test_and_set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
-	else
-		changed = test_and_clear_bit(HCI_CONNECTABLE, &hdev->dev_flags);
-
-	if (changed)
-		new_settings(hdev, NULL);
-}
-
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status)
 {
 	u8 mgmt_err = mgmt_status(status);
-- 
1.9.3

[PATCH 2/4] Bluetooth: Refactor ioctl scan state update to its own function

[johan.hedberg]

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

With subsequent patches we'll also need to update the discoverable
state. As the code grows bigger it's better to move this out from the
switch statement into its own function.

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

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3844eeb85453..27c40e4901a3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2655,6 +2655,26 @@ done:
 	return ret;
 }
 
+static void hci_update_scan_state(struct hci_dev *hdev, u8 scan)
+{
+	bool conn_changed;
+
+	BT_DBG("%s scan 0x%02x", hdev->name, scan);
+
+	if ((scan & SCAN_PAGE))
+		conn_changed = !test_and_set_bit(HCI_CONNECTABLE,
+						 &hdev->dev_flags);
+	else
+		conn_changed = test_and_clear_bit(HCI_CONNECTABLE,
+						  &hdev->dev_flags);
+
+	if (!test_bit(HCI_MGMT, &hdev->dev_flags))
+		return;
+
+	if (conn_changed)
+		mgmt_new_settings(hdev);
+}
+
 int hci_dev_cmd(unsigned int cmd, void __user *arg)
 {
 	struct hci_dev *hdev;
@@ -2719,19 +2739,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 		/* Ensure that the connectable state gets correctly
 		 * modified as this was a non-mgmt change.
 		 */
-		if (!err) {
-			bool changed;
-
-			if ((dr.dev_opt & SCAN_PAGE))
-				changed = !test_and_set_bit(HCI_CONNECTABLE,
-							    &hdev->dev_flags);
-			else
-				changed = test_and_clear_bit(HCI_CONNECTABLE,
-							     &hdev->dev_flags);
-
-			if (changed && test_bit(HCI_MGMT, &hdev->dev_flags))
-				mgmt_new_settings(hdev);
-		}
+		if (!err)
+			hci_update_scan_state(hdev, dr.dev_opt);
 		break;
 
 	case HCISETLINKPOL:
-- 
1.9.3

Re: [PATCH 2/4] Bluetooth: Refactor ioctl scan state update to its own function

[Marcel Holtmann]

Hi Johan,

> With subsequent patches we'll also need to update the discoverable
> state. As the code grows bigger it's better to move this out from the
> switch statement into its own function.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> net/bluetooth/hci_core.c | 35 ++++++++++++++++++++++-------------
> 1 file changed, 22 insertions(+), 13 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH 3/4] Bluetooth: Remove unneeded mgmt_discoverable function

[johan.hedberg]

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

Since the HCISETSCAN ioctl is the only non-mgmt user we care about for
setting the right discoverable state we can simply do the necessary
updates in the ioctl handler function instead. This then allows the
removal of the mgmt_discoverable function and should simplify that state
handling considerably.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci_core.h |  2 +-
 net/bluetooth/hci_core.c         | 24 +++++++++++---
 net/bluetooth/hci_event.c        | 19 +++---------
 net/bluetooth/mgmt.c             | 67 ++++++++++++++--------------------------
 4 files changed, 50 insertions(+), 62 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 76675c55be15..b65efb22be54 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1286,8 +1286,8 @@ void mgmt_index_added(struct hci_dev *hdev);
 void mgmt_index_removed(struct hci_dev *hdev);
 void mgmt_set_powered_failed(struct hci_dev *hdev, int err);
 int mgmt_powered(struct hci_dev *hdev, u8 powered);
+int mgmt_update_adv_data(struct hci_dev *hdev);
 void mgmt_discoverable_timeout(struct hci_dev *hdev);
-void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable);
 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_core.c b/net/bluetooth/hci_core.c
index 27c40e4901a3..3321c65c73ac 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2657,7 +2657,7 @@ done:
 
 static void hci_update_scan_state(struct hci_dev *hdev, u8 scan)
 {
-	bool conn_changed;
+	bool conn_changed, discov_changed;
 
 	BT_DBG("%s scan 0x%02x", hdev->name, scan);
 
@@ -2668,11 +2668,27 @@ static void hci_update_scan_state(struct hci_dev *hdev, u8 scan)
 		conn_changed = test_and_clear_bit(HCI_CONNECTABLE,
 						  &hdev->dev_flags);
 
+	if ((scan & SCAN_INQUIRY)) {
+		discov_changed = !test_and_set_bit(HCI_DISCOVERABLE,
+						   &hdev->dev_flags);
+	} else {
+		clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
+		discov_changed = test_and_clear_bit(HCI_DISCOVERABLE,
+						    &hdev->dev_flags);
+	}
+
 	if (!test_bit(HCI_MGMT, &hdev->dev_flags))
 		return;
 
-	if (conn_changed)
+	if (conn_changed || discov_changed) {
+		/* In case this was disabled through mgmt */
+		set_bit(HCI_BREDR_ENABLED, &hdev->dev_flags);
+
+		if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags))
+			mgmt_update_adv_data(hdev);
+
 		mgmt_new_settings(hdev);
+	}
 }
 
 int hci_dev_cmd(unsigned int cmd, void __user *arg)
@@ -2736,8 +2752,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
 		err = hci_req_sync(hdev, hci_scan_req, dr.dev_opt,
 				   HCI_INIT_TIMEOUT);
 
-		/* Ensure that the connectable state gets correctly
-		 * modified as this was a non-mgmt change.
+		/* Ensure that the connectable and discoverable states
+		 * get correctly modified as this was a non-mgmt change.
 		 */
 		if (!err)
 			hci_update_scan_state(hdev, dr.dev_opt);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 38a0e457eaf9..760bbd88f3c5 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -296,7 +296,6 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	__u8 status = *((__u8 *) skb->data);
 	__u8 param;
-	int old_pscan, old_iscan;
 	void *sent;
 
 	BT_DBG("%s status 0x%2.2x", hdev->name, status);
@@ -315,23 +314,15 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 		goto done;
 	}
 
-	/* We need to ensure that we set this back on if someone changed
-	 * the scan mode through a raw HCI socket.
-	 */
-	set_bit(HCI_BREDR_ENABLED, &hdev->dev_flags);
-
-	old_pscan = test_and_clear_bit(HCI_PSCAN, &hdev->flags);
-	old_iscan = test_and_clear_bit(HCI_ISCAN, &hdev->flags);
-
-	if (param & SCAN_INQUIRY) {
+	if (param & SCAN_INQUIRY)
 		set_bit(HCI_ISCAN, &hdev->flags);
-		if (!old_iscan)
-			mgmt_discoverable(hdev, 1);
-	} else if (old_iscan)
-		mgmt_discoverable(hdev, 0);
+	else
+		clear_bit(HCI_ISCAN, &hdev->flags);
 
 	if (param & SCAN_PAGE)
 		set_bit(HCI_PSCAN, &hdev->flags);
+	else
+		clear_bit(HCI_ISCAN, &hdev->flags);
 
 done:
 	hci_dev_unlock(hdev);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9f9f11c8488b..1dad7bffc6af 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -906,6 +906,16 @@ static void update_adv_data(struct hci_request *req)
 	hci_req_add(req, HCI_OP_LE_SET_ADV_DATA, sizeof(cp), &cp);
 }
 
+int mgmt_update_adv_data(struct hci_dev *hdev)
+{
+	struct hci_request req;
+
+	hci_req_init(&req, hdev);
+	update_adv_data(&req);
+
+	return hci_req_run(&req, NULL);
+}
+
 static void create_eir(struct hci_dev *hdev, u8 *data)
 {
 	u8 *ptr = data;
@@ -1743,7 +1753,7 @@ static void set_connectable_complete(struct hci_dev *hdev, u8 status)
 {
 	struct pending_cmd *cmd;
 	struct mgmt_mode *cp;
-	bool changed;
+	bool conn_changed, discov_changed;
 
 	BT_DBG("status 0x%02x", status);
 
@@ -1760,15 +1770,23 @@ static void set_connectable_complete(struct hci_dev *hdev, u8 status)
 	}
 
 	cp = cmd->param;
-	if (cp->val)
-		changed = !test_and_set_bit(HCI_CONNECTABLE, &hdev->dev_flags);
-	else
-		changed = test_and_clear_bit(HCI_CONNECTABLE, &hdev->dev_flags);
+	if (cp->val) {
+		conn_changed = !test_and_set_bit(HCI_CONNECTABLE,
+						 &hdev->dev_flags);
+		discov_changed = false;
+	} else {
+		conn_changed = test_and_clear_bit(HCI_CONNECTABLE,
+						  &hdev->dev_flags);
+		discov_changed = test_and_clear_bit(HCI_DISCOVERABLE,
+						    &hdev->dev_flags);
+	}
 
 	send_settings_rsp(cmd->sk, MGMT_OP_SET_CONNECTABLE, hdev);
 
-	if (changed) {
+	if (conn_changed || discov_changed) {
 		new_settings(hdev, cmd->sk);
+		if (discov_changed)
+			mgmt_update_adv_data(hdev);
 		hci_update_background_scan(hdev);
 	}
 
@@ -6031,43 +6049,6 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	hci_dev_unlock(hdev);
 }
 
-void mgmt_discoverable(struct hci_dev *hdev, u8 discoverable)
-{
-	bool changed;
-
-	/* Nothing needed here if there's a pending command since that
-	 * commands request completion callback takes care of everything
-	 * necessary.
-	 */
-	if (mgmt_pending_find(MGMT_OP_SET_DISCOVERABLE, hdev))
-		return;
-
-	/* Powering off may clear the scan mode - don't let that interfere */
-	if (!discoverable && mgmt_pending_find(MGMT_OP_SET_POWERED, hdev))
-		return;
-
-	if (discoverable) {
-		changed = !test_and_set_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
-	} else {
-		clear_bit(HCI_LIMITED_DISCOVERABLE, &hdev->dev_flags);
-		changed = test_and_clear_bit(HCI_DISCOVERABLE, &hdev->dev_flags);
-	}
-
-	if (changed) {
-		struct hci_request req;
-
-		/* In case this change in discoverable was triggered by
-		 * a disabling of connectable there could be a need to
-		 * update the advertising flags.
-		 */
-		hci_req_init(&req, hdev);
-		update_adv_data(&req);
-		hci_req_run(&req, NULL);
-
-		new_settings(hdev, NULL);
-	}
-}
-
 void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status)
 {
 	u8 mgmt_err = mgmt_status(status);
-- 
1.9.3

Re: [PATCH 3/4] Bluetooth: Remove unneeded mgmt_discoverable function

[Marcel Holtmann]

Hi Johan,

> Since the HCISETSCAN ioctl is the only non-mgmt user we care about for
> setting the right discoverable state we can simply do the necessary
> updates in the ioctl handler function instead. This then allows the
> removal of the mgmt_discoverable function and should simplify that state
> handling considerably.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h |  2 +-
> net/bluetooth/hci_core.c         | 24 +++++++++++---
> net/bluetooth/hci_event.c        | 19 +++---------
> net/bluetooth/mgmt.c             | 67 ++++++++++++++--------------------------
> 4 files changed, 50 insertions(+), 62 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

[PATCH 4/4] Bluetooth: Remove unneeded mgmt_write_scan_failed function

[johan.hedberg]

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

The Set Connectable/Discoverable mgmt handlers use a hci_request with a
proper callback to handle the HCI command sending. It makes therefore
little sense to have this extra function to be called from hci_event.c
for command failures.

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

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b65efb22be54..7e9e95633a85 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1288,7 +1288,6 @@ void mgmt_set_powered_failed(struct hci_dev *hdev, int err);
 int mgmt_powered(struct hci_dev *hdev, u8 powered);
 int mgmt_update_adv_data(struct hci_dev *hdev);
 void mgmt_discoverable_timeout(struct hci_dev *hdev);
-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);
 void mgmt_device_connected(struct hci_dev *hdev, bdaddr_t *bdaddr, u8 link_type,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 760bbd88f3c5..a62e918d2641 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -309,7 +309,6 @@ static void hci_cc_write_scan_enable(struct hci_dev *hdev, struct sk_buff *skb)
 	hci_dev_lock(hdev);
 
 	if (status) {
-		mgmt_write_scan_failed(hdev, param, status);
 		hdev->discov_timeout = 0;
 		goto done;
 	}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1dad7bffc6af..7703b72653ff 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6049,19 +6049,6 @@ void mgmt_discoverable_timeout(struct hci_dev *hdev)
 	hci_dev_unlock(hdev);
 }
 
-void mgmt_write_scan_failed(struct hci_dev *hdev, u8 scan, u8 status)
-{
-	u8 mgmt_err = mgmt_status(status);
-
-	if (scan & SCAN_PAGE)
-		mgmt_pending_foreach(MGMT_OP_SET_CONNECTABLE, hdev,
-				     cmd_status_rsp, &mgmt_err);
-
-	if (scan & SCAN_INQUIRY)
-		mgmt_pending_foreach(MGMT_OP_SET_DISCOVERABLE, hdev,
-				     cmd_status_rsp, &mgmt_err);
-}
-
 void mgmt_new_link_key(struct hci_dev *hdev, struct link_key *key,
 		       bool persistent)
 {
-- 
1.9.3

Re: [PATCH 4/4] Bluetooth: Remove unneeded mgmt_write_scan_failed function

[Marcel Holtmann]

Hi Johan,

> The Set Connectable/Discoverable mgmt handlers use a hci_request with a
> proper callback to handle the HCI command sending. It makes therefore
> little sense to have this extra function to be called from hci_event.c
> for command failures.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h |  1 -
> net/bluetooth/hci_event.c        |  1 -
> net/bluetooth/mgmt.c             | 13 -------------
> 3 files changed, 15 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH 1/4] Bluetooth: Remove unneeded mgmt_connectable function

[Marcel Holtmann]

Hi Johan,

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> The mgmt_connectable function has been used to ensure that the right
> actions to HCI_CONNECTABLE are taken when the HCI_Write_Scan_Enable
> command is triggered by something else than mgmt. The only other user
> that we really care about is the HCISETSCAN ioctl code, so we can
> actually more simply perform the needed changes there instead.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
> include/net/bluetooth/hci_core.h |  1 -
> net/bluetooth/hci_core.c         |  6 +++---
> net/bluetooth/hci_event.c        |  6 +-----
> net/bluetooth/mgmt.c             | 32 --------------------------------
> 4 files changed, 4 insertions(+), 41 deletions(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel