linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/8] LL Privacy Support
@ 2020-07-13  6:22 Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 1/8] Bluetooth: Translate additional address type correctly Sathish Narasimman
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

V4: patches are rebased
Added support to use set Experimental feature to enable controller
address resolution

Marcel Holtmann (3):
  Bluetooth: Translate additional address type correctly
  Bluetooth: Configure controller address resolution if available
  Bluetooth: Update resolving list when updating whitelist

Sathish Narasimman (5):
  Bluetooth: Translate additional address type during le_conn
  Bluetooth: Let controller creates RPA during le create conn
  Bluetooth: Enable/Disable address resolution during le create conn
  Bluetooth: Enable RPA Timeout
  Bluetooth: Enable controller RPA resolution using Experimental feature

 include/net/bluetooth/hci.h      |   9 ++-
 include/net/bluetooth/hci_core.h |   3 +
 net/bluetooth/hci_conn.c         |   7 +-
 net/bluetooth/hci_core.c         |  17 +++++
 net/bluetooth/hci_event.c        |  21 ++++++
 net/bluetooth/hci_request.c      | 120 ++++++++++++++++++++++++++-----
 net/bluetooth/hci_request.h      |   3 +-
 net/bluetooth/mgmt.c             |  54 +++++++++++++-
 8 files changed, 213 insertions(+), 21 deletions(-)

-- 
2.17.1


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

* [PATCH v4 1/8] Bluetooth: Translate additional address type correctly
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 2/8] Bluetooth: Configure controller address resolution if available Sathish Narasimman
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Sathish Narsimman

From: Marcel Holtmann <marcel@holtmann.org>

When using controller based address resolution, then the new address
types 0x02 and 0x03 are used. These types need to be converted back into
either public address or random address types.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
---
 include/net/bluetooth/hci.h | 6 ++++--
 net/bluetooth/hci_core.c    | 9 +++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 1f18f71363e9..abab8b5981a7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2268,8 +2268,10 @@ struct hci_ev_le_conn_complete {
 #define LE_EXT_ADV_SCAN_RSP		0x0008
 #define LE_EXT_ADV_LEGACY_PDU		0x0010
 
-#define ADDR_LE_DEV_PUBLIC	0x00
-#define ADDR_LE_DEV_RANDOM	0x01
+#define ADDR_LE_DEV_PUBLIC		0x00
+#define ADDR_LE_DEV_RANDOM		0x01
+#define ADDR_LE_DEV_PUBLIC_RESOLVED	0x02
+#define ADDR_LE_DEV_RANDOM_RESOLVED	0x03
 
 #define HCI_EV_LE_ADVERTISING_REPORT	0x02
 struct hci_ev_le_advertising_info {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6509f785dd14..4af208b82138 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3290,6 +3290,15 @@ struct hci_conn_params *hci_pend_le_action_lookup(struct list_head *list,
 {
 	struct hci_conn_params *param;
 
+	switch (addr_type) {
+	case ADDR_LE_DEV_PUBLIC_RESOLVED:
+		addr_type = ADDR_LE_DEV_PUBLIC;
+		break;
+	case ADDR_LE_DEV_RANDOM_RESOLVED:
+		addr_type = ADDR_LE_DEV_RANDOM;
+		break;
+	}
+
 	list_for_each_entry(param, list, action) {
 		if (bacmp(&param->addr, addr) == 0 &&
 		    param->addr_type == addr_type)
-- 
2.17.1


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

* [PATCH v4 2/8] Bluetooth: Configure controller address resolution if available
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 1/8] Bluetooth: Translate additional address type correctly Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 3/8] Bluetooth: Update resolving list when updating whitelist Sathish Narasimman
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Sathish Narsimman

From: Marcel Holtmann <marcel@holtmann.org>

When the LL Privacy support is available, then as part of enabling or
disabling passive background scanning, it is required to set up the
controller based address resolution as well.

Since only passive background scanning is utilizing the whitelist, the
address resolution is now bound to the whitelist and passive background
scanning. All other resolution can be easily done by the host stack.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
---
 include/net/bluetooth/hci_core.h |  3 +++
 net/bluetooth/hci_request.c      | 26 +++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 34ad5b207598..065250242a1b 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1359,6 +1359,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
 #define scan_coded(dev) (((dev)->le_tx_def_phys & HCI_LE_SET_PHY_CODED) || \
 			 ((dev)->le_rx_def_phys & HCI_LE_SET_PHY_CODED))
 
+/* Use LL Privacy based address resolution if supported */
+#define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)
+
 /* Use ext scanning if set ext scan param and ext scan enable is supported */
 #define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
 			   ((dev)->commands[37] & 0x40))
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 770b93758112..d3c7ddbcff33 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -675,6 +675,12 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 		cp.enable = LE_SCAN_DISABLE;
 		hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 	}
+
+	if (use_ll_privacy(hdev) &&
+	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION)) {
+		__u8 enable = 0x00;
+		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
+	}
 }
 
 static void del_from_white_list(struct hci_request *req, bdaddr_t *bdaddr,
@@ -816,7 +822,8 @@ static bool scan_use_rpa(struct hci_dev *hdev)
 }
 
 static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
-			       u16 window, u8 own_addr_type, u8 filter_policy)
+			       u16 window, u8 own_addr_type, u8 filter_policy,
+			       bool addr_resolv)
 {
 	struct hci_dev *hdev = req->hdev;
 
@@ -825,6 +832,11 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 		return;
 	}
 
+	if (use_ll_privacy(hdev) && addr_resolv) {
+		u8 enable = 0x01;
+		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
+	}
+
 	/* Use ext scanning if set ext scan param and ext scan enable is
 	 * supported
 	 */
@@ -898,12 +910,18 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 	}
 }
 
+/* Ensure to call hci_req_add_le_scan_disable() first to disable the
+ * controller based address resolution to be able to reconfigure
+ * resolving list.
+ */
 void hci_req_add_le_passive_scan(struct hci_request *req)
 {
 	struct hci_dev *hdev = req->hdev;
 	u8 own_addr_type;
 	u8 filter_policy;
 	u16 window, interval;
+	/* Background scanning should run with address resolution */
+	bool addr_resolv = true;
 
 	if (hdev->scanning_paused) {
 		bt_dev_dbg(hdev, "Scanning is paused for suspend");
@@ -949,7 +967,7 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
 
 	bt_dev_dbg(hdev, "LE passive scan with whitelist = %d", filter_policy);
 	hci_req_start_scan(req, LE_SCAN_PASSIVE, interval, window,
-			   own_addr_type, filter_policy);
+			   own_addr_type, filter_policy, addr_resolv);
 }
 
 static u8 get_adv_instance_scan_rsp_len(struct hci_dev *hdev, u8 instance)
@@ -2748,6 +2766,8 @@ static int active_scan(struct hci_request *req, unsigned long opt)
 	u8 own_addr_type;
 	/* White list is not used for discovery */
 	u8 filter_policy = 0x00;
+	/* Discovery doesn't require controller address resolution */
+	bool addr_resolv = false;
 	int err;
 
 	BT_DBG("%s", hdev->name);
@@ -2770,7 +2790,7 @@ static int active_scan(struct hci_request *req, unsigned long opt)
 
 	hci_req_start_scan(req, LE_SCAN_ACTIVE, interval,
 			   hdev->le_scan_window_discovery, own_addr_type,
-			   filter_policy);
+			   filter_policy, addr_resolv);
 	return 0;
 }
 
-- 
2.17.1


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

* [PATCH v4 3/8] Bluetooth: Update resolving list when updating whitelist
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 1/8] Bluetooth: Translate additional address type correctly Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 2/8] Bluetooth: Configure controller address resolution if available Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 4/8] Bluetooth: Translate additional address type during le_conn Sathish Narasimman
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Marcel Holtmann, Sathish Narsimman

From: Marcel Holtmann <marcel@holtmann.org>

When the whitelist is updated, then also update the entries of the
resolving list for devices where IRKs are available.

Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Sathish Narsimman <sathish.narasimman@intel.com>
---
 net/bluetooth/hci_request.c | 41 +++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index d3c7ddbcff33..2dc00604412c 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -694,6 +694,21 @@ static void del_from_white_list(struct hci_request *req, bdaddr_t *bdaddr,
 	bt_dev_dbg(req->hdev, "Remove %pMR (0x%x) from whitelist", &cp.bdaddr,
 		   cp.bdaddr_type);
 	hci_req_add(req, HCI_OP_LE_DEL_FROM_WHITE_LIST, sizeof(cp), &cp);
+
+	if (use_ll_privacy(req->hdev)) {
+		struct smp_irk *irk;
+
+		irk = hci_find_irk_by_addr(req->hdev, bdaddr, bdaddr_type);
+		if (irk) {
+			struct hci_cp_le_del_from_resolv_list cp;
+
+			cp.bdaddr_type = bdaddr_type;
+			bacpy(&cp.bdaddr, bdaddr);
+
+			hci_req_add(req, HCI_OP_LE_DEL_FROM_RESOLV_LIST,
+				    sizeof(cp), &cp);
+		}
+	}
 }
 
 /* Adds connection to white list if needed. On error, returns -1. */
@@ -714,7 +729,7 @@ static int add_to_white_list(struct hci_request *req,
 		return -1;
 
 	/* White list can not be used with RPAs */
-	if (!allow_rpa &&
+	if (!allow_rpa && !use_ll_privacy(hdev) &&
 	    hci_find_irk_by_addr(hdev, &params->addr, params->addr_type)) {
 		return -1;
 	}
@@ -732,6 +747,28 @@ static int add_to_white_list(struct hci_request *req,
 		   cp.bdaddr_type);
 	hci_req_add(req, HCI_OP_LE_ADD_TO_WHITE_LIST, sizeof(cp), &cp);
 
+	if (use_ll_privacy(hdev)) {
+		struct smp_irk *irk;
+
+		irk = hci_find_irk_by_addr(hdev, &params->addr,
+					   params->addr_type);
+		if (irk) {
+			struct hci_cp_le_add_to_resolv_list cp;
+
+			cp.bdaddr_type = params->addr_type;
+			bacpy(&cp.bdaddr, &params->addr);
+			memcpy(cp.peer_irk, irk->val, 16);
+
+			if (hci_dev_test_flag(hdev, HCI_PRIVACY))
+				memcpy(cp.local_irk, hdev->irk, 16);
+			else
+				memset(cp.local_irk, 0, 16);
+
+			hci_req_add(req, HCI_OP_LE_ADD_TO_RESOLV_LIST,
+				    sizeof(cp), &cp);
+		}
+	}
+
 	return 0;
 }
 
@@ -772,7 +809,7 @@ static u8 update_white_list(struct hci_request *req)
 		}
 
 		/* White list can not be used with RPAs */
-		if (!allow_rpa &&
+		if (!allow_rpa && !use_ll_privacy(hdev) &&
 		    hci_find_irk_by_addr(hdev, &b->bdaddr, b->bdaddr_type)) {
 			return 0x00;
 		}
-- 
2.17.1


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

* [PATCH v4 4/8] Bluetooth: Translate additional address type during le_conn
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (2 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 3/8] Bluetooth: Update resolving list when updating whitelist Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 5/8] Bluetooth: Let controller creates RPA during le create conn Sathish Narasimman
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

When using controller based address resolution, then the new address
types 0x02 and 0x03 are used. These types need to be converted back into
either public address or random address types.

This patch is specially during LE_CREATE_CONN if using own_add_type as 0x02
or 0x03.

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 net/bluetooth/hci_event.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 927bde511170..eae5bd4a53ac 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2296,6 +2296,22 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr,
 	if (!conn)
 		return;
 
+	/* When using controller based address resolution, then the new
+	 * address types 0x02 and 0x03 are used. These types need to be
+	 * converted back into either public address or random address type
+	 */
+	if (use_ll_privacy(hdev) &&
+	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION)) {
+		switch (own_address_type) {
+		case ADDR_LE_DEV_PUBLIC_RESOLVED:
+			own_address_type = ADDR_LE_DEV_PUBLIC;
+			break;
+		case ADDR_LE_DEV_RANDOM_RESOLVED:
+			own_address_type = ADDR_LE_DEV_RANDOM;
+			break;
+		}
+	}
+
 	/* Store the initiator and responder address information which
 	 * is needed for SMP. These values will not change during the
 	 * lifetime of the connection.
-- 
2.17.1


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

* [PATCH v4 5/8] Bluetooth: Let controller creates RPA during le create conn
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (3 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 4/8] Bluetooth: Translate additional address type during le_conn Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 6/8] Bluetooth: Enable/Disable address resolution " Sathish Narasimman
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

When address resolution is enabled and set_privacy is enabled let's
use own address type as 0x03

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 net/bluetooth/hci_request.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 2dc00604412c..e16d5154b2cf 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -2201,7 +2201,13 @@ int hci_update_random_address(struct hci_request *req, bool require_privacy,
 	if (use_rpa) {
 		int to;
 
-		*own_addr_type = ADDR_LE_DEV_RANDOM;
+		/* If Controller supports LL Privacy use own address type is
+		 * 0x03
+		 */
+		if (use_ll_privacy(hdev))
+			*own_addr_type = ADDR_LE_DEV_RANDOM_RESOLVED;
+		else
+			*own_addr_type = ADDR_LE_DEV_RANDOM;
 
 		if (!hci_dev_test_and_clear_flag(hdev, HCI_RPA_EXPIRED) &&
 		    !bacmp(&hdev->random_addr, &hdev->rpa))
-- 
2.17.1


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

* [PATCH v4 6/8] Bluetooth: Enable/Disable address resolution during le create conn
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (4 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 5/8] Bluetooth: Let controller creates RPA during le create conn Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 7/8] Bluetooth: Enable RPA Timeout Sathish Narasimman
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

In this patch if le_create_conn process is started restrict to
disable address resolution and same is disabled during
le_enh_connection_complete

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 net/bluetooth/hci_conn.c    |  7 +++++-
 net/bluetooth/hci_event.c   |  4 ++++
 net/bluetooth/hci_request.c | 45 ++++++++++++++++++++++++++++---------
 net/bluetooth/hci_request.h |  3 ++-
 net/bluetooth/mgmt.c        |  2 +-
 5 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 8805d68e65f2..caf1598758bf 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1003,6 +1003,11 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	struct hci_request req;
 	int err;
 
+	/* This ensures that during disable le_scan address resolution
+	 * will not be disabled if it is followed by le_create_conn
+	 */
+	bool rpa_le_conn = true;
+
 	/* Let's make sure that le is enabled.*/
 	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED)) {
 		if (lmp_le_capable(hdev))
@@ -1103,7 +1108,7 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
 	 * state.
 	 */
 	if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
-		hci_req_add_le_scan_disable(&req);
+		hci_req_add_le_scan_disable(&req, rpa_le_conn);
 		hci_dev_set_flag(hdev, HCI_LE_SCAN_INTERRUPTED);
 	}
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index eae5bd4a53ac..684c68cb5c76 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5221,6 +5221,10 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
 			     le16_to_cpu(ev->interval),
 			     le16_to_cpu(ev->latency),
 			     le16_to_cpu(ev->supervision_timeout));
+
+	if (use_ll_privacy(hdev) &&
+	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
+		hci_req_disable_address_resolution(hdev);
 }
 
 static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, struct sk_buff *skb)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index e16d5154b2cf..c3193f7f9ff0 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -428,7 +428,7 @@ static void __hci_update_background_scan(struct hci_request *req)
 		if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
 			return;
 
-		hci_req_add_le_scan_disable(req);
+		hci_req_add_le_scan_disable(req, false);
 
 		BT_DBG("%s stopping background scanning", hdev->name);
 	} else {
@@ -447,7 +447,7 @@ static void __hci_update_background_scan(struct hci_request *req)
 		 * don't miss any advertising (due to duplicates filter).
 		 */
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
-			hci_req_add_le_scan_disable(req);
+			hci_req_add_le_scan_disable(req, false);
 
 		hci_req_add_le_passive_scan(req);
 
@@ -652,7 +652,7 @@ void __hci_req_update_eir(struct hci_request *req)
 	hci_req_add(req, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
 }
 
-void hci_req_add_le_scan_disable(struct hci_request *req)
+void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)
 {
 	struct hci_dev *hdev = req->hdev;
 
@@ -676,8 +676,9 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
 		hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
 	}
 
+	/* Disable address resolution */
 	if (use_ll_privacy(hdev) &&
-	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION)) {
+	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) && !rpa_le_conn) {
 		__u8 enable = 0x00;
 		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
 	}
@@ -1072,7 +1073,7 @@ static void hci_req_config_le_suspend_scan(struct hci_request *req)
 {
 	/* Before changing params disable scan if enabled */
 	if (hci_dev_test_flag(req->hdev, HCI_LE_SCAN))
-		hci_req_add_le_scan_disable(req);
+		hci_req_add_le_scan_disable(req, false);
 
 	/* Configure params and enable scanning */
 	hci_req_add_le_passive_scan(req);
@@ -1140,7 +1141,7 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
 
 		/* Disable LE passive scan if enabled */
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
-			hci_req_add_le_scan_disable(&req);
+			hci_req_add_le_scan_disable(&req, false);
 
 		/* Mark task needing completion */
 		set_bit(SUSPEND_SCAN_DISABLE, hdev->suspend_tasks);
@@ -1701,6 +1702,28 @@ int hci_req_update_adv_data(struct hci_dev *hdev, u8 instance)
 	return hci_req_run(&req, NULL);
 }
 
+static void enable_addr_resolution_complete(struct hci_dev *hdev, u8 status,
+					    u16 opcode)
+{
+	BT_DBG("%s status %u", hdev->name, status);
+}
+
+void hci_req_disable_address_resolution(struct hci_dev *hdev)
+{
+	struct hci_request req;
+	__u8 enable = 0x00;
+
+	if (!use_ll_privacy(hdev) &&
+	    !hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
+		return;
+
+	hci_req_init(&req, hdev);
+
+	hci_req_add(&req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
+
+	hci_req_run(&req, enable_addr_resolution_complete);
+}
+
 static void adv_enable_complete(struct hci_dev *hdev, u8 status, u16 opcode)
 {
 	BT_DBG("%s status %u", hdev->name, status);
@@ -2626,7 +2649,7 @@ static void bg_scan_update(struct work_struct *work)
 
 static int le_scan_disable(struct hci_request *req, unsigned long opt)
 {
-	hci_req_add_le_scan_disable(req);
+	hci_req_add_le_scan_disable(req, false);
 	return 0;
 }
 
@@ -2729,7 +2752,7 @@ static int le_scan_restart(struct hci_request *req, unsigned long opt)
 		return 0;
 	}
 
-	hci_req_add_le_scan_disable(req);
+	hci_req_add_le_scan_disable(req, false);
 
 	if (use_ext_scan(hdev)) {
 		struct hci_cp_le_set_ext_scan_enable ext_enable_cp;
@@ -2820,7 +2843,7 @@ static int active_scan(struct hci_request *req, unsigned long opt)
 	 * discovery scanning parameters.
 	 */
 	if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
-		hci_req_add_le_scan_disable(req);
+		hci_req_add_le_scan_disable(req, false);
 
 	/* All active scans will be done with either a resolvable private
 	 * address (when privacy feature has been enabled) or non-resolvable
@@ -2935,14 +2958,14 @@ bool hci_req_stop_discovery(struct hci_request *req)
 
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
 			cancel_delayed_work(&hdev->le_scan_disable);
-			hci_req_add_le_scan_disable(req);
+			hci_req_add_le_scan_disable(req, false);
 		}
 
 		ret = true;
 	} else {
 		/* Passive scanning */
 		if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
-			hci_req_add_le_scan_disable(req);
+			hci_req_add_le_scan_disable(req, false);
 			ret = true;
 		}
 	}
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 0e81614d235e..12bea10e7d70 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -65,11 +65,12 @@ void __hci_req_write_fast_connectable(struct hci_request *req, bool enable);
 void __hci_req_update_name(struct hci_request *req);
 void __hci_req_update_eir(struct hci_request *req);
 
-void hci_req_add_le_scan_disable(struct hci_request *req);
+void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn);
 void hci_req_add_le_passive_scan(struct hci_request *req);
 
 void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next);
 
+void hci_req_disable_address_resolution(struct hci_dev *hdev);
 void hci_req_reenable_advertising(struct hci_dev *hdev);
 void __hci_req_enable_advertising(struct hci_request *req);
 void __hci_req_disable_advertising(struct hci_request *req);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 686ef4792831..c292d5de4dc3 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5226,7 +5226,7 @@ static int set_scan_params(struct sock *sk, struct hci_dev *hdev,
 
 		hci_req_init(&req, hdev);
 
-		hci_req_add_le_scan_disable(&req);
+		hci_req_add_le_scan_disable(&req, false);
 		hci_req_add_le_passive_scan(&req);
 
 		hci_req_run(&req, NULL);
-- 
2.17.1


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

* [PATCH v4 7/8] Bluetooth: Enable RPA Timeout
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (5 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 6/8] Bluetooth: Enable/Disable address resolution " Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-13  6:22 ` [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature Sathish Narasimman
  2020-07-16  5:12 ` [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

Enable RPA timeout during bluetooth initialization.
The RPA timeout value is used from hdev, which initialized from
debug_fs

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 include/net/bluetooth/hci.h | 2 ++
 net/bluetooth/hci_core.c    | 8 ++++++++
 2 files changed, 10 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index abab8b5981a7..4ff2fc4498f3 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1637,6 +1637,8 @@ struct hci_rp_le_read_resolv_list_size {
 
 #define HCI_OP_LE_SET_ADDR_RESOLV_ENABLE 0x202d
 
+#define HCI_OP_LE_SET_RPA_TIMEOUT	0x202e
+
 #define HCI_OP_LE_READ_MAX_DATA_LEN	0x202f
 struct hci_rp_le_read_max_data_len {
 	__u8	status;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4af208b82138..2030536cc5d8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -762,6 +762,14 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
 			hci_req_add(req, HCI_OP_LE_CLEAR_RESOLV_LIST, 0, NULL);
 		}
 
+		if (hdev->commands[35] & 0x40) {
+			__le16 rpa_timeout = cpu_to_le16(hdev->rpa_timeout);
+
+			/* Set RPA timeout */
+			hci_req_add(req, HCI_OP_LE_SET_RPA_TIMEOUT, 2,
+				    &rpa_timeout);
+		}
+
 		if (hdev->le_features[0] & HCI_LE_DATA_LEN_EXT) {
 			/* Read LE Maximum Data Length */
 			hci_req_add(req, HCI_OP_LE_READ_MAX_DATA_LEN, 0, NULL);
-- 
2.17.1


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

* [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (6 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 7/8] Bluetooth: Enable RPA Timeout Sathish Narasimman
@ 2020-07-13  6:22 ` Sathish Narasimman
  2020-07-16  7:13   ` Marcel Holtmann
  2020-07-16  5:12 ` [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
  8 siblings, 1 reply; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-13  6:22 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Sathish Narasimman

This patch adds support to enable the use of RPA Address resolution
using expermental feature mgmt command.

Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_event.c   |  3 ++-
 net/bluetooth/hci_request.c |  6 +++--
 net/bluetooth/mgmt.c        | 52 +++++++++++++++++++++++++++++++++++++
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 4ff2fc4498f3..cb284365b4c1 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -307,6 +307,7 @@ enum {
 	HCI_FORCE_BREDR_SMP,
 	HCI_FORCE_STATIC_ADDR,
 	HCI_LL_RPA_RESOLUTION,
+	HCI_ENABLE_RPA_RESOLUTION,
 	HCI_CMD_PENDING,
 	HCI_FORCE_NO_MITM,
 
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 684c68cb5c76..c8a5e1e4dba2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5222,7 +5222,8 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
 			     le16_to_cpu(ev->latency),
 			     le16_to_cpu(ev->supervision_timeout));
 
-	if (use_ll_privacy(hdev) &&
+	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
+	    use_ll_privacy(hdev) &&
 	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
 		hci_req_disable_address_resolution(hdev);
 }
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index c3193f7f9ff0..cb44b83539e6 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -677,7 +677,8 @@ void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)
 	}
 
 	/* Disable address resolution */
-	if (use_ll_privacy(hdev) &&
+	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
+	    use_ll_privacy(hdev) &&
 	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) && !rpa_le_conn) {
 		__u8 enable = 0x00;
 		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
@@ -870,7 +871,8 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
 		return;
 	}
 
-	if (use_ll_privacy(hdev) && addr_resolv) {
+	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
+	    use_ll_privacy(hdev) && addr_resolv) {
 		u8 enable = 0x01;
 		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
 	}
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index c292d5de4dc3..fbe02ab5fa05 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3759,6 +3759,12 @@ static const u8 simult_central_periph_uuid[16] = {
 	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
 };
 
+/* 15c0a148-c273-11ea-b3de-0242ac130004 */
+static const u8 rpa_resolution_uuid[16] = {
+	0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
+	0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15,
+};
+
 static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 				  void *data, u16 data_len)
 {
@@ -3795,6 +3801,17 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
 		idx++;
 	}
 
+	if (hdev) {
+		if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION))
+			flags = BIT(0);
+		else
+			flags = 0;
+
+		memcpy(rp->features[idx].uuid, rpa_resolution_uuid, 16);
+		rp->features[idx].flags = cpu_to_le32(flags);
+		idx++;
+	}
+
 	rp->feature_count = cpu_to_le16(idx);
 
 	/* After reading the experimental features information, enable
@@ -3895,6 +3912,41 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
 	}
 #endif
 
+	if (!memcmp(cp->uuid, rpa_resolution_uuid, 16)) {
+		bool val;
+		int err;
+
+		/* Parameters are limited to a single octet */
+		if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
+			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+					       MGMT_OP_SET_EXP_FEATURE,
+					       MGMT_STATUS_INVALID_PARAMS);
+
+		/* Only boolean on/off is supported */
+		if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
+			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
+					       MGMT_OP_SET_EXP_FEATURE,
+					       MGMT_STATUS_INVALID_PARAMS);
+
+		val = !!cp->param[0];
+
+		if (val)
+			hci_dev_set_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
+		else
+			hci_dev_clear_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
+
+		memcpy(rp.uuid, rpa_resolution_uuid, 16);
+		rp.flags = cpu_to_le32(val ? BIT(0) : 0);
+
+		hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
+
+		err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
+					MGMT_OP_SET_EXP_FEATURE, 0,
+					&rp, sizeof(rp));
+
+		return err;
+	}
+
 	return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
 			       MGMT_OP_SET_EXP_FEATURE,
 			       MGMT_STATUS_NOT_SUPPORTED);
-- 
2.17.1


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

* Re: [PATCH v4 0/8] LL Privacy Support
  2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
                   ` (7 preceding siblings ...)
  2020-07-13  6:22 ` [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature Sathish Narasimman
@ 2020-07-16  5:12 ` Sathish Narasimman
  8 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-16  5:12 UTC (permalink / raw)
  To: Bluez mailing list; +Cc: Sathish Narasimman, Sathish N, Chethan T N

Hi

Gentle reminder

On Mon, Jul 13, 2020 at 11:48 AM Sathish Narasimman
<nsathish41@gmail.com> wrote:
>
> V4: patches are rebased
> Added support to use set Experimental feature to enable controller
> address resolution
>
> Marcel Holtmann (3):
>   Bluetooth: Translate additional address type correctly
>   Bluetooth: Configure controller address resolution if available
>   Bluetooth: Update resolving list when updating whitelist
>
> Sathish Narasimman (5):
>   Bluetooth: Translate additional address type during le_conn
>   Bluetooth: Let controller creates RPA during le create conn
>   Bluetooth: Enable/Disable address resolution during le create conn
>   Bluetooth: Enable RPA Timeout
>   Bluetooth: Enable controller RPA resolution using Experimental feature
>
>  include/net/bluetooth/hci.h      |   9 ++-
>  include/net/bluetooth/hci_core.h |   3 +
>  net/bluetooth/hci_conn.c         |   7 +-
>  net/bluetooth/hci_core.c         |  17 +++++
>  net/bluetooth/hci_event.c        |  21 ++++++
>  net/bluetooth/hci_request.c      | 120 ++++++++++++++++++++++++++-----
>  net/bluetooth/hci_request.h      |   3 +-
>  net/bluetooth/mgmt.c             |  54 +++++++++++++-
>  8 files changed, 213 insertions(+), 21 deletions(-)
>
> --
> 2.17.1
>
Regards
Sathish N

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

* Re: [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature
  2020-07-13  6:22 ` [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature Sathish Narasimman
@ 2020-07-16  7:13   ` Marcel Holtmann
  2020-07-23 12:23     ` Sathish Narasimman
  0 siblings, 1 reply; 12+ messages in thread
From: Marcel Holtmann @ 2020-07-16  7:13 UTC (permalink / raw)
  To: Sathish Narasimman; +Cc: linux-bluetooth, Sathish Narasimman

Hi Sathish,

> This patch adds support to enable the use of RPA Address resolution
> using expermental feature mgmt command.

everything looks fine, except for this patch. I just prefer to only apply the others if we can apply this one as well.

> Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> ---
> include/net/bluetooth/hci.h |  1 +
> net/bluetooth/hci_event.c   |  3 ++-
> net/bluetooth/hci_request.c |  6 +++--
> net/bluetooth/mgmt.c        | 52 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 4ff2fc4498f3..cb284365b4c1 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -307,6 +307,7 @@ enum {
> 	HCI_FORCE_BREDR_SMP,
> 	HCI_FORCE_STATIC_ADDR,
> 	HCI_LL_RPA_RESOLUTION,
> +	HCI_ENABLE_RPA_RESOLUTION,

I would call this ENABLE_LL_PRIVAY. It put its more in line with use_ll_privacy and clearly distinct from the LL_RPA_RESOLUTION with is a HCI operational mode.

> 	HCI_CMD_PENDING,
> 	HCI_FORCE_NO_MITM,
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 684c68cb5c76..c8a5e1e4dba2 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5222,7 +5222,8 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
> 			     le16_to_cpu(ev->latency),
> 			     le16_to_cpu(ev->supervision_timeout));
> 
> -	if (use_ll_privacy(hdev) &&
> +	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> +	    use_ll_privacy(hdev) &&
> 	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))

I would leave use_ll_privacy at the top and add the new one after it.

> 		hci_req_disable_address_resolution(hdev);
> }
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index c3193f7f9ff0..cb44b83539e6 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -677,7 +677,8 @@ void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)
> 	}
> 
> 	/* Disable address resolution */
> -	if (use_ll_privacy(hdev) &&
> +	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> +	    use_ll_privacy(hdev) &&

Same here.

> 	    hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) && !rpa_le_conn) {
> 		__u8 enable = 0x00;
> 		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> @@ -870,7 +871,8 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
> 		return;
> 	}
> 
> -	if (use_ll_privacy(hdev) && addr_resolv) {
> +	if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> +	    use_ll_privacy(hdev) && addr_resolv) {

And here.

> 		u8 enable = 0x01;
> 		hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> 	}
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index c292d5de4dc3..fbe02ab5fa05 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -3759,6 +3759,12 @@ static const u8 simult_central_periph_uuid[16] = {
> 	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
> };
> 
> +/* 15c0a148-c273-11ea-b3de-0242ac130004 */
> +static const u8 rpa_resolution_uuid[16] = {
> +	0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
> +	0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15,
> +};
> +
> static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> 				  void *data, u16 data_len)
> {
> @@ -3795,6 +3801,17 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> 		idx++;
> 	}
> 
> +	if (hdev) {

If use_ll_privacy is not available, then we should also not expose this experimental feature.

> +		if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION))
> +			flags = BIT(0);
> +		else
> +			flags = 0;
> +

And since we only support the RPA resolution for central mode at the moment, we really now need to disable advertising support. So this one needs to indicate the the supported settings will change when enabled.

> +		memcpy(rp->features[idx].uuid, rpa_resolution_uuid, 16);
> +		rp->features[idx].flags = cpu_to_le32(flags);
> +		idx++;
> +	}
> +
> 	rp->feature_count = cpu_to_le16(idx);
> 
> 	/* After reading the experimental features information, enable
> @@ -3895,6 +3912,41 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> 	}
> #endif
> 
> +	if (!memcmp(cp->uuid, rpa_resolution_uuid, 16)) {
> +		bool val;
> +		int err;
> +
> +		/* Parameters are limited to a single octet */
> +		if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> +			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> +					       MGMT_OP_SET_EXP_FEATURE,
> +					       MGMT_STATUS_INVALID_PARAMS);
> +
> +		/* Only boolean on/off is supported */
> +		if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> +			return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> +					       MGMT_OP_SET_EXP_FEATURE,
> +					       MGMT_STATUS_INVALID_PARAMS);
> +
> +		val = !!cp->param[0];
> +
> +		if (val)
> +			hci_dev_set_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
> +		else
> +			hci_dev_clear_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
> +
> +		memcpy(rp.uuid, rpa_resolution_uuid, 16);
> +		rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> +
> +		hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> +
> +		err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
> +					MGMT_OP_SET_EXP_FEATURE, 0,
> +					&rp, sizeof(rp));

The exp_feature_changed event is missing. In addition you need to handle the ZERO_KEY branch which means it will reset all experimental features back to default.

> +
> +		return err;
> +	}
> +
> 	return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
> 			       MGMT_OP_SET_EXP_FEATURE,
> 			       MGMT_STATUS_NOT_SUPPORTED);

Regards

Marcel


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

* Re: [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature
  2020-07-16  7:13   ` Marcel Holtmann
@ 2020-07-23 12:23     ` Sathish Narasimman
  0 siblings, 0 replies; 12+ messages in thread
From: Sathish Narasimman @ 2020-07-23 12:23 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Bluez mailing list, Sathish Narasimman

Hi Marcel


On Thu, Jul 16, 2020 at 12:43 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Sathish,
>
> > This patch adds support to enable the use of RPA Address resolution
> > using expermental feature mgmt command.
>
> everything looks fine, except for this patch. I just prefer to only apply the others if we can apply this one as well.
>
> > Signed-off-by: Sathish Narasimman <sathish.narasimman@intel.com>
> > ---
> > include/net/bluetooth/hci.h |  1 +
> > net/bluetooth/hci_event.c   |  3 ++-
> > net/bluetooth/hci_request.c |  6 +++--
> > net/bluetooth/mgmt.c        | 52 +++++++++++++++++++++++++++++++++++++
> > 4 files changed, 59 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 4ff2fc4498f3..cb284365b4c1 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -307,6 +307,7 @@ enum {
> >       HCI_FORCE_BREDR_SMP,
> >       HCI_FORCE_STATIC_ADDR,
> >       HCI_LL_RPA_RESOLUTION,
> > +     HCI_ENABLE_RPA_RESOLUTION,
>
> I would call this ENABLE_LL_PRIVAY. It put its more in line with use_ll_privacy and clearly distinct from the LL_RPA_RESOLUTION with is a HCI operational mode.
>
Will change it

> >       HCI_CMD_PENDING,
> >       HCI_FORCE_NO_MITM,
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 684c68cb5c76..c8a5e1e4dba2 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -5222,7 +5222,8 @@ static void hci_le_enh_conn_complete_evt(struct hci_dev *hdev,
> >                            le16_to_cpu(ev->latency),
> >                            le16_to_cpu(ev->supervision_timeout));
> >
> > -     if (use_ll_privacy(hdev) &&
> > +     if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> > +         use_ll_privacy(hdev) &&
> >           hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION))
>
> I would leave use_ll_privacy at the top and add the new one after it.
>
> >               hci_req_disable_address_resolution(hdev);
> > }
> > diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> > index c3193f7f9ff0..cb44b83539e6 100644
> > --- a/net/bluetooth/hci_request.c
> > +++ b/net/bluetooth/hci_request.c
> > @@ -677,7 +677,8 @@ void hci_req_add_le_scan_disable(struct hci_request *req, bool rpa_le_conn)
> >       }
> >
> >       /* Disable address resolution */
> > -     if (use_ll_privacy(hdev) &&
> > +     if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> > +         use_ll_privacy(hdev) &&
>
> Same here.
>
> >           hci_dev_test_flag(hdev, HCI_LL_RPA_RESOLUTION) && !rpa_le_conn) {
> >               __u8 enable = 0x00;
> >               hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> > @@ -870,7 +871,8 @@ static void hci_req_start_scan(struct hci_request *req, u8 type, u16 interval,
> >               return;
> >       }
> >
> > -     if (use_ll_privacy(hdev) && addr_resolv) {
> > +     if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION) &&
> > +         use_ll_privacy(hdev) && addr_resolv) {
>
> And here.
>
> >               u8 enable = 0x01;
> >               hci_req_add(req, HCI_OP_LE_SET_ADDR_RESOLV_ENABLE, 1, &enable);
> >       }
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index c292d5de4dc3..fbe02ab5fa05 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -3759,6 +3759,12 @@ static const u8 simult_central_periph_uuid[16] = {
> >       0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
> > };
> >
> > +/* 15c0a148-c273-11ea-b3de-0242ac130004 */
> > +static const u8 rpa_resolution_uuid[16] = {
> > +     0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
> > +     0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15,
> > +};
> > +
> > static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> >                                 void *data, u16 data_len)
> > {
> > @@ -3795,6 +3801,17 @@ static int read_exp_features_info(struct sock *sk, struct hci_dev *hdev,
> >               idx++;
> >       }
> >
> > +     if (hdev) {
>
> If use_ll_privacy is not available, then we should also not expose this experimental feature.
>
> > +             if (hci_dev_test_flag(hdev, HCI_ENABLE_RPA_RESOLUTION))
> > +                     flags = BIT(0);
> > +             else
> > +                     flags = 0;
> > +
>
> And since we only support the RPA resolution for central mode at the moment, we really now need to disable advertising support. So this one needs to indicate the the supported settings will change when enabled.
>
Does disable advertising support means clearing HCI_ADVERTISING flag?
Or __hci_req_disable_advertising
Please review the next version of the changes where i updated clearing the flag

> > +             memcpy(rp->features[idx].uuid, rpa_resolution_uuid, 16);
> > +             rp->features[idx].flags = cpu_to_le32(flags);
> > +             idx++;
> > +     }
> > +
> >       rp->feature_count = cpu_to_le16(idx);
> >
> >       /* After reading the experimental features information, enable
> > @@ -3895,6 +3912,41 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> >       }
> > #endif
> >
> > +     if (!memcmp(cp->uuid, rpa_resolution_uuid, 16)) {
> > +             bool val;
> > +             int err;
> > +
> > +             /* Parameters are limited to a single octet */
> > +             if (data_len != MGMT_SET_EXP_FEATURE_SIZE + 1)
> > +                     return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > +                                            MGMT_OP_SET_EXP_FEATURE,
> > +                                            MGMT_STATUS_INVALID_PARAMS);
> > +
> > +             /* Only boolean on/off is supported */
> > +             if (cp->param[0] != 0x00 && cp->param[0] != 0x01)
> > +                     return mgmt_cmd_status(sk, MGMT_INDEX_NONE,
> > +                                            MGMT_OP_SET_EXP_FEATURE,
> > +                                            MGMT_STATUS_INVALID_PARAMS);
> > +
> > +             val = !!cp->param[0];
> > +
> > +             if (val)
> > +                     hci_dev_set_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
> > +             else
> > +                     hci_dev_clear_flag(hdev, HCI_ENABLE_RPA_RESOLUTION);
> > +
> > +             memcpy(rp.uuid, rpa_resolution_uuid, 16);
> > +             rp.flags = cpu_to_le32(val ? BIT(0) : 0);
> > +
> > +             hci_sock_set_flag(sk, HCI_MGMT_EXP_FEATURE_EVENTS);
> > +
> > +             err = mgmt_cmd_complete(sk, MGMT_INDEX_NONE,
> > +                                     MGMT_OP_SET_EXP_FEATURE, 0,
> > +                                     &rp, sizeof(rp));
>
> The exp_feature_changed event is missing. In addition you need to handle the ZERO_KEY branch which means it will reset all experimental features back to default.

Changes done please help to review version 5
>
> > +
> > +             return err;
> > +     }
> > +
> >       return mgmt_cmd_status(sk, hdev ? hdev->id : MGMT_INDEX_NONE,
> >                              MGMT_OP_SET_EXP_FEATURE,
> >                              MGMT_STATUS_NOT_SUPPORTED);
>
> Regards
>
> Marcel
>

Regards
Sathish N

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

end of thread, other threads:[~2020-07-23 12:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-13  6:22 [PATCH v4 0/8] LL Privacy Support Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 1/8] Bluetooth: Translate additional address type correctly Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 2/8] Bluetooth: Configure controller address resolution if available Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 3/8] Bluetooth: Update resolving list when updating whitelist Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 4/8] Bluetooth: Translate additional address type during le_conn Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 5/8] Bluetooth: Let controller creates RPA during le create conn Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 6/8] Bluetooth: Enable/Disable address resolution " Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 7/8] Bluetooth: Enable RPA Timeout Sathish Narasimman
2020-07-13  6:22 ` [PATCH v4 8/8] Bluetooth: Enable controller RPA resolution using Experimental feature Sathish Narasimman
2020-07-16  7:13   ` Marcel Holtmann
2020-07-23 12:23     ` Sathish Narasimman
2020-07-16  5:12 ` [PATCH v4 0/8] LL Privacy Support Sathish Narasimman

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).