[PATCH 0/7 v2] Bluetooth: LE single-mode & peripheral role fixes

[PATCH 0/7 v2] Bluetooth: LE single-mode & peripheral role fixes

[Johan Hedberg]

Hi,

This is a combination of one patch from my very first set that had
conflicts (1/7 here) as well as a re-sorted set of patches from my
second set that I got an ack on. There's still two missing patches from
my second set that are not here (AD data writing and mgmt_set_le support
for the peripheral mode) which I'll send in a separate set probably
later tonight.

Johan

[PATCH 1/7] Bluetooth: Fix HCI command sending when powering on LE-only adapters

[Johan Hedberg]

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

This patch makes sure that we don't send BR/EDR-only commands for
LE-only adapters when they get powered on. Doing this would just cause
command errors.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c |   34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index b41f3bc..19cb899 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2872,6 +2872,21 @@ static void settings_rsp(struct pending_cmd *cmd, void *data)
 	mgmt_pending_free(cmd);
 }
 
+static int set_bredr_scan(struct hci_dev *hdev)
+{
+	u8 scan = 0;
+
+	if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
+		scan |= SCAN_PAGE;
+	if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
+		scan |= SCAN_INQUIRY;
+
+	if (!scan)
+		return 0;
+
+	return hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
+}
+
 int mgmt_powered(struct hci_dev *hdev, u8 powered)
 {
 	struct cmd_lookup match = { NULL, hdev };
@@ -2883,16 +2898,6 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 	mgmt_pending_foreach(MGMT_OP_SET_POWERED, hdev, settings_rsp, &match);
 
 	if (powered) {
-		u8 scan = 0;
-
-		if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
-			scan |= SCAN_PAGE;
-		if (test_bit(HCI_DISCOVERABLE, &hdev->dev_flags))
-			scan |= SCAN_INQUIRY;
-
-		if (scan)
-			hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
-
 		if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {
 			u8 ssp = 1;
 
@@ -2909,9 +2914,12 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 				     sizeof(cp), &cp);
 		}
 
-		update_class(hdev);
-		update_name(hdev, hdev->dev_name);
-		update_eir(hdev);
+		if (lmp_bredr_capable(hdev)) {
+			set_bredr_scan(hdev);
+			update_class(hdev);
+			update_name(hdev, hdev->dev_name);
+			update_eir(hdev);
+		}
 	} else {
 		u8 status = MGMT_STATUS_NOT_POWERED;
 		mgmt_pending_foreach(0, hdev, cmd_status_rsp, &status);
-- 
1.7.10.4

[PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Johan Hedberg]

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

This patch makes sure that settings which are specific for BR/EDR
capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
Instead, a "not supported" error is returned of such a setting is
attempted to be set for a non-BR/EDR adapter.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/mgmt.c |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 19cb899..6215c97 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
 	u32 settings = 0;
 
 	settings |= MGMT_SETTING_POWERED;
-	settings |= MGMT_SETTING_CONNECTABLE;
-	settings |= MGMT_SETTING_FAST_CONNECTABLE;
-	settings |= MGMT_SETTING_DISCOVERABLE;
 	settings |= MGMT_SETTING_PAIRABLE;
 
 	if (lmp_ssp_capable(hdev))
 		settings |= MGMT_SETTING_SSP;
 
 	if (lmp_bredr_capable(hdev)) {
+		settings |= MGMT_SETTING_CONNECTABLE;
+		settings |= MGMT_SETTING_FAST_CONNECTABLE;
+		settings |= MGMT_SETTING_DISCOVERABLE;
 		settings |= MGMT_SETTING_BREDR;
 		settings |= MGMT_SETTING_LINK_SECURITY;
 	}
@@ -868,6 +868,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+				 MGMT_STATUS_NOT_SUPPORTED);
+
 	timeout = __le16_to_cpu(cp->timeout);
 	if (!cp->val && timeout > 0)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -963,6 +967,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
@@ -1061,6 +1069,10 @@ static int set_link_security(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
@@ -2595,6 +2607,10 @@ static int set_fast_connectable(struct sock *sk, struct hci_dev *hdev,
 
 	BT_DBG("%s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	if (!hdev_is_powered(hdev))
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
 				  MGMT_STATUS_NOT_POWERED);
-- 
1.7.10.4

[PATCH 3/7] Bluetooth: Fix updating host feature bits for LE

[Johan Hedberg]

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

When LE has been enabled with the simultaneous BR/EDR & LE parameter set
to true we should also update the host features stored in struct hci_dev
accordingly.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_event.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0b9e646..aa325ee 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1287,6 +1287,11 @@ static void hci_cc_write_le_host_supported(struct hci_dev *hdev,
 			hdev->host_features[0] |= LMP_HOST_LE;
 		else
 			hdev->host_features[0] &= ~LMP_HOST_LE;
+
+		if (sent->simul)
+			hdev->host_features[0] |= LMP_HOST_LE_BREDR;
+		else
+			hdev->host_features[0] &= ~LMP_HOST_LE_BREDR;
 	}
 
 	if (test_bit(HCI_MGMT, &hdev->dev_flags) &&
-- 
1.7.10.4

[PATCH 4/7] Bluetooth: Add missing feature test macros

[Johan Hedberg]

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

This patch adds missing feature test macros needed for various use cases
and also sorts the macros according to the feature bit location in the
feature mask (for easy lookup).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci_core.h |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c885e54..6642b3c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -750,18 +750,28 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define SET_HCIDEV_DEV(hdev, pdev) ((hdev)->dev.parent = (pdev))
 
 /* ----- LMP capabilities ----- */
-#define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
 #define lmp_encrypt_capable(dev)   ((dev)->features[0] & LMP_ENCRYPT)
+#define lmp_rswitch_capable(dev)   ((dev)->features[0] & LMP_RSWITCH)
+#define lmp_hold_capable(dev)      ((dev)->features[0] & LMP_HOLD)
 #define lmp_sniff_capable(dev)     ((dev)->features[0] & LMP_SNIFF)
-#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
+#define lmp_park_capable(dev)      ((dev)->features[1] & LMP_PARK)
+#define lmp_inq_rssi_capable(dev)  ((dev)->features[3] & LMP_RSSI_INQ)
 #define lmp_esco_capable(dev)      ((dev)->features[3] & LMP_ESCO)
+#define lmp_bredr_capable(dev)     (!((dev)->features[4] & LMP_NO_BREDR))
+#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
+#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
+#define lmp_pause_enc_capable(dev) ((dev)->features[5] & LMP_PAUSE_ENC)
+#define lmp_ext_inq_capable(dev)   ((dev)->features[6] & LMP_EXT_INQ)
+#define lmp_le_br_capable(dev)     ((dev)->features[6] & LMP_SIMUL_LE_BR)
 #define lmp_ssp_capable(dev)       ((dev)->features[6] & LMP_SIMPLE_PAIR)
 #define lmp_no_flush_capable(dev)  ((dev)->features[6] & LMP_NO_FLUSH)
-#define lmp_le_capable(dev)        ((dev)->features[4] & LMP_LE)
-#define lmp_bredr_capable(dev)     (!((dev)->features[4] & LMP_NO_BREDR))
+#define lmp_lsto_capable(dev)      ((dev)->features[7] & LMP_LSTO)
+#define lmp_inq_tx_pwr_capable(dev) ((dev)->features[7] & LMP_INQ_TX_PWR)
+#define lmp_ext_feat_capable(dev)  ((dev)->features[7] & LMP_EXTFEATURES)
 
 /* ----- Extended LMP capabilities ----- */
 #define lmp_host_le_capable(dev)   ((dev)->host_features[0] & LMP_HOST_LE)
+#define lmp_host_le_br_capable(dev) ((dev)->host_features[0] & LMP_HOST_LE_BREDR)
 
 /* ----- HCI protocols ----- */
 static inline int hci_proto_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr,
-- 
1.7.10.4

[PATCH 5/7] Bluetooth: Make use feature test macros

[Johan Hedberg]

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

For better code readability and avoiding simple bugs of checking the
wrong byte of the features make use of feature test macros whenever
possible.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_event.c |   26 +++++++++++++-------------
 net/bluetooth/mgmt.c      |   10 +++++-----
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index aa325ee..aae8053 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -460,10 +460,10 @@ static void hci_cc_write_ssp_mode(struct hci_dev *hdev, struct sk_buff *skb)
 
 static u8 hci_get_inquiry_mode(struct hci_dev *hdev)
 {
-	if (hdev->features[6] & LMP_EXT_INQ)
+	if (lmp_ext_inq_capable(hdev))
 		return 2;
 
-	if (hdev->features[3] & LMP_RSSI_INQ)
+	if (lmp_inq_rssi_capable(hdev))
 		return 1;
 
 	if (hdev->manufacturer == 11 && hdev->hci_rev == 0x00 &&
@@ -515,22 +515,22 @@ static void hci_setup_event_mask(struct hci_dev *hdev)
 		events[5] |= 0x10; /* Synchronous Connection Changed */
 	}
 
-	if (hdev->features[3] & LMP_RSSI_INQ)
+	if (lmp_inq_rssi_capable(hdev))
 		events[4] |= 0x02; /* Inquiry Result with RSSI */
 
 	if (lmp_sniffsubr_capable(hdev))
 		events[5] |= 0x20; /* Sniff Subrating */
 
-	if (hdev->features[5] & LMP_PAUSE_ENC)
+	if (lmp_pause_enc_capable(hdev))
 		events[5] |= 0x80; /* Encryption Key Refresh Complete */
 
-	if (hdev->features[6] & LMP_EXT_INQ)
+	if (lmp_ext_inq_capable(hdev))
 		events[5] |= 0x40; /* Extended Inquiry Result */
 
 	if (lmp_no_flush_capable(hdev))
 		events[7] |= 0x01; /* Enhanced Flush Complete */
 
-	if (hdev->features[7] & LMP_LSTO)
+	if (lmp_lsto_capable(hdev))
 		events[6] |= 0x80; /* Link Supervision Timeout Changed */
 
 	if (lmp_ssp_capable(hdev)) {
@@ -633,13 +633,13 @@ static void hci_setup(struct hci_dev *hdev)
 		}
 	}
 
-	if (hdev->features[3] & LMP_RSSI_INQ)
+	if (lmp_inq_rssi_capable(hdev))
 		hci_setup_inquiry_mode(hdev);
 
-	if (hdev->features[7] & LMP_INQ_TX_PWR)
+	if (lmp_inq_tx_pwr_capable(hdev))
 		hci_send_cmd(hdev, HCI_OP_READ_INQ_RSP_TX_POWER, 0, NULL);
 
-	if (hdev->features[7] & LMP_EXTFEATURES) {
+	if (lmp_ext_feat_capable(hdev)) {
 		struct hci_cp_read_local_ext_features cp;
 
 		cp.page = 0x01;
@@ -686,11 +686,11 @@ static void hci_setup_link_policy(struct hci_dev *hdev)
 
 	if (lmp_rswitch_capable(hdev))
 		link_policy |= HCI_LP_RSWITCH;
-	if (hdev->features[0] & LMP_HOLD)
+	if (lmp_hold_capable(hdev))
 		link_policy |= HCI_LP_HOLD;
 	if (lmp_sniff_capable(hdev))
 		link_policy |= HCI_LP_SNIFF;
-	if (hdev->features[1] & LMP_PARK)
+	if (lmp_park_capable(hdev))
 		link_policy |= HCI_LP_PARK;
 
 	cp.policy = cpu_to_le16(link_policy);
@@ -780,10 +780,10 @@ static void hci_set_le_support(struct hci_dev *hdev)
 
 	if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
 		cp.le = 1;
-		cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+		cp.simul = !!lmp_le_br_capable(hdev);
 	}
 
-	if (cp.le != !!(hdev->host_features[0] & LMP_HOST_LE))
+	if (cp.le != !!lmp_host_le_capable(hdev))
 		hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(cp),
 			     &cp);
 }
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 6215c97..0ee150e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -566,7 +566,7 @@ static int update_eir(struct hci_dev *hdev)
 	if (!hdev_is_powered(hdev))
 		return 0;
 
-	if (!(hdev->features[6] & LMP_EXT_INQ))
+	if (!lmp_ext_inq_capable(hdev))
 		return 0;
 
 	if (!test_bit(HCI_SSP_ENABLED, &hdev->dev_flags))
@@ -1226,7 +1226,7 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 	}
 
 	val = !!cp->val;
-	enabled = !!(hdev->host_features[0] & LMP_HOST_LE);
+	enabled = !!lmp_host_le_capable(hdev);
 
 	if (!hdev_is_powered(hdev) || val == enabled) {
 		bool changed = false;
@@ -1262,7 +1262,7 @@ static int set_le(struct sock *sk, struct hci_dev *hdev, void *data, u16 len)
 
 	if (val) {
 		hci_cp.le = val;
-		hci_cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+		hci_cp.simul = !!lmp_le_br_capable(hdev);
 	}
 
 	err = hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED, sizeof(hci_cp),
@@ -2924,7 +2924,7 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 			struct hci_cp_write_le_host_supported cp;
 
 			cp.le = 1;
-			cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+			cp.simul = !!lmp_le_br_capable(hdev);
 
 			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
 				     sizeof(cp), &cp);
@@ -3384,7 +3384,7 @@ static int clear_eir(struct hci_dev *hdev)
 {
 	struct hci_cp_write_eir cp;
 
-	if (!(hdev->features[6] & LMP_EXT_INQ))
+	if (!lmp_ext_inq_capable(hdev))
 		return 0;
 
 	memset(hdev->eir, 0, sizeof(hdev->eir));
-- 
1.7.10.4

[PATCH 6/7] Bluetooth: Add flag for LE GAP Peripheral role

[Johan Hedberg]

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

This patch adds a flag to be used for LE GAP Peripheral role. In this
role undirected advertising will be enabled and operations not allowed
in Peripheral role (such as scanning and initiating connections) will be
disallowed.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 include/net/bluetooth/hci.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 348f4bf..6c414f4 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -115,6 +115,7 @@ enum {
 	HCI_SSP_ENABLED,
 	HCI_HS_ENABLED,
 	HCI_LE_ENABLED,
+	HCI_LE_PERIPHERAL,
 	HCI_CONNECTABLE,
 	HCI_DISCOVERABLE,
 	HCI_LINK_SECURITY,
-- 
1.7.10.4

Re: [PATCH 6/7] Bluetooth: Add flag for LE GAP Peripheral role

[Marcel Holtmann]

Hi Johan,

> This patch adds a flag to be used for LE GAP Peripheral role. In this
> role undirected advertising will be enabled and operations not allowed
> in Peripheral role (such as scanning and initiating connections) will be
> disallowed.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  include/net/bluetooth/hci.h |    1 +
>  1 file changed, 1 insertion(+)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel

[PATCH 7/7] Bluetooth: Disallow LE scanning and connecting in peripheral role

[Johan Hedberg]

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

When an adapter is in the LE peripheral role scanning for other devices
or initiating connections to them is not allowed. This patch makes sure
that such attempts will result in appropriate error returns.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_conn.c |    3 +++
 net/bluetooth/hci_core.c |    3 +++
 2 files changed, 6 insertions(+)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 6dcf452..dc331ce 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -502,6 +502,9 @@ static struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 {
 	struct hci_conn *le;
 
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->flags))
+		return ERR_PTR(-ENOTSUPP);
+
 	le = hci_conn_hash_lookup_ba(hdev, LE_LINK, dst);
 	if (!le) {
 		le = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5a3400d..515d0c3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1577,6 +1577,9 @@ int hci_le_scan(struct hci_dev *hdev, u8 type, u16 interval, u16 window,
 
 	BT_DBG("%s", hdev->name);
 
+	if (test_bit(HCI_LE_PERIPHERAL, &hdev->dev_flags))
+		return -ENOTSUPP;
+
 	if (work_busy(&hdev->le_scan))
 		return -EINPROGRESS;
 
-- 
1.7.10.4

Re: [PATCH 7/7] Bluetooth: Disallow LE scanning and connecting in peripheral role

[Gustavo Padovan]

Hi Johan,

* Johan Hedberg <johan.hedberg@gmail.com> [2012-10-24 21:12:03 +0300]:

> From: Johan Hedberg <johan.hedberg@intel.com>
> 
> When an adapter is in the LE peripheral role scanning for other devices
> or initiating connections to them is not allowed. This patch makes sure
> that such attempts will result in appropriate error returns.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/hci_conn.c |    3 +++
>  net/bluetooth/hci_core.c |    3 +++
>  2 files changed, 6 insertions(+)

All 7 patches have been applied  to bluetooth-next. Thanks.

	Gustavo

[PATCH 0/7] Bluetooth: Improved single-mode and LE peripheral support

[Johan Hedberg]

Hi,

This set of patches builds on top of my previous one. It introduces a
few more fixes for single-mode controllers as well as adds a new feature
to the management interface to switch a controller to LE peripheral
mode.  User space already has code to support this and you can enable
the new mode using "tools/btmgmt le peripheral".

Johan

[PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Johan Hedberg]

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

This patch makes sure that settings which are specific for BR/EDR
capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
Instead, a "not supported" error is returned of such a setting is
attempted to be set for a non-BR/EDR adapter.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1d79979..eaa6fdc 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
 	u32 settings = 0;
 
 	settings |= MGMT_SETTING_POWERED;
-	settings |= MGMT_SETTING_CONNECTABLE;
-	settings |= MGMT_SETTING_FAST_CONNECTABLE;
-	settings |= MGMT_SETTING_DISCOVERABLE;
 	settings |= MGMT_SETTING_PAIRABLE;
 
 	if (lmp_ssp_capable(hdev))
 		settings |= MGMT_SETTING_SSP;
 
 	if (lmp_bredr_capable(hdev)) {
+		settings |= MGMT_SETTING_CONNECTABLE;
+		settings |= MGMT_SETTING_FAST_CONNECTABLE;
+		settings |= MGMT_SETTING_DISCOVERABLE;
 		settings |= MGMT_SETTING_BREDR;
 		settings |= MGMT_SETTING_LINK_SECURITY;
 	}
@@ -874,6 +874,10 @@ static int set_discoverable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
+				 MGMT_STATUS_NOT_SUPPORTED);
+
 	timeout = __le16_to_cpu(cp->timeout);
 	if (!cp->val && timeout > 0)
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_DISCOVERABLE,
@@ -969,6 +973,10 @@ static int set_connectable(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_CONNECTABLE,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
@@ -1067,6 +1075,10 @@ static int set_link_security(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	BT_DBG("request for %s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_LINK_SECURITY,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	hci_dev_lock(hdev);
 
 	if (!hdev_is_powered(hdev)) {
@@ -2647,6 +2659,10 @@ static int set_fast_connectable(struct sock *sk, struct hci_dev *hdev,
 
 	BT_DBG("%s", hdev->name);
 
+	if (!lmp_bredr_capable(hdev))
+		return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
+				  MGMT_STATUS_NOT_SUPPORTED);
+
 	if (!hdev_is_powered(hdev))
 		return cmd_status(sk, hdev->id, MGMT_OP_SET_FAST_CONNECTABLE,
 				  MGMT_STATUS_NOT_POWERED);
-- 
1.7.10.4

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Anderson Lizardo]

Hi Johan,

On Tue, Oct 23, 2012 at 12:53 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> @@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
>         u32 settings = 0;
>
>         settings |= MGMT_SETTING_POWERED;
> -       settings |= MGMT_SETTING_CONNECTABLE;
> -       settings |= MGMT_SETTING_FAST_CONNECTABLE;
> -       settings |= MGMT_SETTING_DISCOVERABLE;
>         settings |= MGMT_SETTING_PAIRABLE;
>
>         if (lmp_ssp_capable(hdev))
>                 settings |= MGMT_SETTING_SSP;
>
>         if (lmp_bredr_capable(hdev)) {
> +               settings |= MGMT_SETTING_CONNECTABLE;
> +               settings |= MGMT_SETTING_FAST_CONNECTABLE;
> +               settings |= MGMT_SETTING_DISCOVERABLE;
>                 settings |= MGMT_SETTING_BREDR;
>                 settings |= MGMT_SETTING_LINK_SECURITY;

"Discoverable" (at least as a Discovery mode from GAP) is mandatory
for LE Peripheral role (see page 1698). Same for "Connectable" (see
page 1706).

Unless you have other plans to implement this for single mode LE
peripherals running bluez?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Marcel Holtmann]

Hi Anderson,

> > @@ -377,15 +377,15 @@ static u32 get_supported_settings(struct hci_dev *hdev)
> >         u32 settings = 0;
> >
> >         settings |= MGMT_SETTING_POWERED;
> > -       settings |= MGMT_SETTING_CONNECTABLE;
> > -       settings |= MGMT_SETTING_FAST_CONNECTABLE;
> > -       settings |= MGMT_SETTING_DISCOVERABLE;
> >         settings |= MGMT_SETTING_PAIRABLE;
> >
> >         if (lmp_ssp_capable(hdev))
> >                 settings |= MGMT_SETTING_SSP;
> >
> >         if (lmp_bredr_capable(hdev)) {
> > +               settings |= MGMT_SETTING_CONNECTABLE;
> > +               settings |= MGMT_SETTING_FAST_CONNECTABLE;
> > +               settings |= MGMT_SETTING_DISCOVERABLE;
> >                 settings |= MGMT_SETTING_BREDR;
> >                 settings |= MGMT_SETTING_LINK_SECURITY;
> 
> "Discoverable" (at least as a Discovery mode from GAP) is mandatory
> for LE Peripheral role (see page 1698). Same for "Connectable" (see
> page 1706).
> 
> Unless you have other plans to implement this for single mode LE
> peripherals running bluez?

at this moment, we plan to keep LE a bit independent from BR/EDR.

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Marcel Holtmann]

Hi Johan,

> This patch makes sure that settings which are specific for BR/EDR
> capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
> Instead, a "not supported" error is returned of such a setting is
> attempted to be set for a non-BR/EDR adapter.
> 
> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
> ---
>  net/bluetooth/mgmt.c |   22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)

can we lead with this patch. It seems to be useful no matter what we do
for LE peripheral or single-mode controllers.

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Anderson Lizardo]

Hi Marcel,

On Tue, Oct 23, 2012 at 3:02 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Johan,
>
>> This patch makes sure that settings which are specific for BR/EDR
>> capable adapters are not allowed for non-BR/EDR (e.g. LE-only) adapters.
>> Instead, a "not supported" error is returned of such a setting is
>> attempted to be set for a non-BR/EDR adapter.
>>
>> Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
>> ---
>>  net/bluetooth/mgmt.c |   22 +++++++++++++++++++---
>>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> can we lead with this patch. It seems to be useful no matter what we do
> for LE peripheral or single-mode controllers.

I agree that current upstream will be more consistent if these
settings are made "not supported" for LE (as they indeed are not
implemented yet). I was just confused that the same patch series that
improve LE peripheral support does not include properly support for
Connectable/Discoverable modes on this role (which may restrict the
usefulness?).

Johan: do you have further plans on improving LE peripheral support
after these patches? If yes, please keep in mind that we still have
ongoing plans to push broadcaster/observer roles upstream, which also
requires hability to enable/disable advertising and set advertising
data/parameters.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Johan Hedberg]

Hi Lizardo,

On Tue, Oct 23, 2012, Anderson Lizardo wrote:
> Johan: do you have further plans on improving LE peripheral support
> after these patches? If yes, please keep in mind that we still have
> ongoing plans to push broadcaster/observer roles upstream, which also
> requires hability to enable/disable advertising and set advertising
> data/parameters.

Looking through the various "shall" and "shall not" clauses in the core
spec it seems to me that only Central, Observer and Broadcaster roles
are compatible with each other, and Peripheral is mutually exclusive
with anything else. E.g. section 9.2.4.2 (page 1700):

"While a device is in the Broadcaster, Observer or Central role the
device shall not support the general discoverable mode."

and section 9.3.4.2 (page 1709):

"While a device is in the Broadcaster, Observer, or the Central role
the device shall not support the undirected connectable mode."

Both general discoverable and undirected connectable are needed by
Peripheral role, i.e. essentially excluding the other roles.

Extending the other roles could indeed be a matter of interpreting the
mgmt_set_le parameter as a bit mask and then returning an error if an
incompatible combination of roles is attempted. Also, once support for
new roles is added it should be easy to extend the current state checks
to make a distinction on exactly what kind of scanning/advertising is
done instead of just looking at "advertising or not" on a general level.

Johan

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Marcel Holtmann]

Hi Johan,

> > Johan: do you have further plans on improving LE peripheral support
> > after these patches? If yes, please keep in mind that we still have
> > ongoing plans to push broadcaster/observer roles upstream, which also
> > requires hability to enable/disable advertising and set advertising
> > data/parameters.
> 
> Looking through the various "shall" and "shall not" clauses in the core
> spec it seems to me that only Central, Observer and Broadcaster roles
> are compatible with each other, and Peripheral is mutually exclusive
> with anything else. E.g. section 9.2.4.2 (page 1700):
> 
> "While a device is in the Broadcaster, Observer or Central role the
> device shall not support the general discoverable mode."
> 
> and section 9.3.4.2 (page 1709):
> 
> "While a device is in the Broadcaster, Observer, or the Central role
> the device shall not support the undirected connectable mode."
> 
> Both general discoverable and undirected connectable are needed by
> Peripheral role, i.e. essentially excluding the other roles.
> 
> Extending the other roles could indeed be a matter of interpreting the
> mgmt_set_le parameter as a bit mask and then returning an error if an
> incompatible combination of roles is attempted. Also, once support for
> new roles is added it should be easy to extend the current state checks
> to make a distinction on exactly what kind of scanning/advertising is
> done instead of just looking at "advertising or not" on a general level.

the way I see this now (which might change), we are doing LE on/off. And
by default this means actually central. For us it is a good assumption
that we are central if LE is enabled. And this means if the LE flag is
set, then we are a central. As it has been so far. In case we want to
switch this to peripheral, then we have to set PERIPHERAL flag. The LE
flag stays on to indicate LE is enabled. !PERIPHERAL flag here means we
are central.

That is why I like the explanation above, if we set ourselves as
peripheral, then we are peripheral and everything else is out of the
question. So either LE is enabled and we act as central or we are acting
as peripheral.

So broadcaster and observer are extra features on top of central. For me
that means we have treat them independent. How much you can be
broadcaster and observer and central at the same time is then again a
different question that we need to answer.

Regards

Marcel

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Anderson Lizardo]

Hi Marcel,

On Tue, Oct 23, 2012 at 5:46 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> That is why I like the explanation above, if we set ourselves as
> peripheral, then we are peripheral and everything else is out of the
> question. So either LE is enabled and we act as central or we are acting
> as peripheral.
>
> So broadcaster and observer are extra features on top of central. For me
> that means we have treat them independent. How much you can be
> broadcaster and observer and central at the same time is then again a
> different question that we need to answer.

So do you suggest we keep Broadcaster/Observer as separate settings
(which actually we prefer, given that we have been working on
supporting those modes for BR/EDR only adapters, by using EIR for
broadcasting data allowed on EIR, such as Manufacturer Specific Data,
and parsing them from Device Found mgmt event when on Observer mode),
and keep "Set Low Energy" setting just for Central/Peripheral modes?

Regards
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 2/7] Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters

[Marcel Holtmann]

Hi Anderson,

> > That is why I like the explanation above, if we set ourselves as
> > peripheral, then we are peripheral and everything else is out of the
> > question. So either LE is enabled and we act as central or we are acting
> > as peripheral.
> >
> > So broadcaster and observer are extra features on top of central. For me
> > that means we have treat them independent. How much you can be
> > broadcaster and observer and central at the same time is then again a
> > different question that we need to answer.
> 
> So do you suggest we keep Broadcaster/Observer as separate settings
> (which actually we prefer, given that we have been working on
> supporting those modes for BR/EDR only adapters, by using EIR for
> broadcasting data allowed on EIR, such as Manufacturer Specific Data,
> and parsing them from Device Found mgmt event when on Observer mode),
> and keep "Set Low Energy" setting just for Central/Peripheral modes?

if we can stay in central role and do broadcaster/observer as an
additional feature, then yes. If they are mutually exclusive, we have to
find something else.

But with all of this, I do not have the final answer until we tried it
and had a couple of review rounds.

Regards

Marcel