2020-06-03 17:15:32

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH v2 0/7] LL Privacy support

Based on the input the patches modified and verifed accordingly.

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 (4):
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

include/net/bluetooth/hci.h | 8 ++-
include/net/bluetooth/hci_core.h | 3 +
net/bluetooth/hci_conn.c | 6 +-
net/bluetooth/hci_core.c | 17 +++++
net/bluetooth/hci_event.c | 20 ++++++
net/bluetooth/hci_request.c | 119 ++++++++++++++++++++++++++-----
net/bluetooth/hci_request.h | 3 +-
net/bluetooth/mgmt.c | 2 +-
8 files changed, 157 insertions(+), 21 deletions(-)

--
2.17.1


2020-06-03 17:16:09

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 5/7] Bluetooth: Let controller creates RPA during le create conn

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

Signed-off-by: Sathish Narasimman <[email protected]>
---
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 bcd64dec9989..6e1036cc8b1e 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -2183,7 +2183,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

2020-06-03 17:16:10

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 7/7] Bluetooth: Enable RPA Timeout

Signed-off-by: Sathish Narasimman <[email protected]>
---
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 1165dfea56a9..12005fbe6e09 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 f4ceda1f1454..67e1434c3f31 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -763,6 +763,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

2020-06-03 17:17:34

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 4/7] Bluetooth: Translate additional address type during le_conn

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 <[email protected]>
---
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 cfeaee347db3..9c9c434844f2 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

2020-06-03 17:17:56

by Sathish Narasimman

[permalink] [raw]
Subject: [PATCH 6/7] Bluetooth: Enable/Disable address resolution during le create conn

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 <[email protected]>
---
net/bluetooth/hci_conn.c | 6 ++++-
net/bluetooth/hci_event.c | 4 ++++
net/bluetooth/hci_request.c | 46 ++++++++++++++++++++++++++++---------
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 307800fd18e6..bbd6d06bc900 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1008,6 +1008,10 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst,
struct smp_irk *irk;
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)) {
@@ -1109,7 +1113,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 9c9c434844f2..1866de712bb2 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5225,6 +5225,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 6e1036cc8b1e..8552a37976ec 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -429,7 +429,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 {
@@ -448,7 +448,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);

@@ -653,7 +653,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;

@@ -677,8 +677,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);
}
@@ -1054,7 +1055,7 @@ static void hci_req_set_event_filter(struct hci_request *req)
static void hci_req_config_le_suspend_scan(struct hci_request *req)
{
/* Can't change params without disabling first */
- 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);
@@ -1121,7 +1122,7 @@ void hci_req_prepare_suspend(struct hci_dev *hdev, enum suspended_state next)
hci_req_add(&req, HCI_OP_WRITE_SCAN_ENABLE, 1, &page_scan);

/* Disable LE passive 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);
@@ -1682,6 +1683,29 @@ 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);
@@ -2608,7 +2632,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;
}

@@ -2706,7 +2730,7 @@ static int le_scan_restart(struct hci_request *req, unsigned long opt)
if (!hci_dev_test_flag(hdev, HCI_LE_SCAN))
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;
@@ -2797,7 +2821,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
@@ -2911,14 +2935,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 9e8a3cccc6ca..97eb4f3a160b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4848,7 +4848,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

2020-06-08 06:31:10

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] LL Privacy support

Hi

On Wed, Jun 3, 2020 at 10:44 PM Sathish Narasimman <[email protected]> wrote:
>
> Based on the input the patches modified and verifed accordingly.
>
> 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 (4):
> 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
>
> include/net/bluetooth/hci.h | 8 ++-
> include/net/bluetooth/hci_core.h | 3 +
> net/bluetooth/hci_conn.c | 6 +-
> net/bluetooth/hci_core.c | 17 +++++
> net/bluetooth/hci_event.c | 20 ++++++
> net/bluetooth/hci_request.c | 119 ++++++++++++++++++++++++++-----
> net/bluetooth/hci_request.h | 3 +-
> net/bluetooth/mgmt.c | 2 +-
> 8 files changed, 157 insertions(+), 21 deletions(-)
>
> --
> 2.17.1
>

Gentle Reminder
Also, the present approach will only work for the central role.

I am planning to work in a peripheral role.
need help with a design approach on the peripheral.


Regards
Sathish N

2020-06-19 07:59:14

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] LL Privacy support

Hi Sathish,

> Based on the input the patches modified and verifed accordingly.
>
> 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 (4):
> 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
>
> include/net/bluetooth/hci.h | 8 ++-
> include/net/bluetooth/hci_core.h | 3 +
> net/bluetooth/hci_conn.c | 6 +-
> net/bluetooth/hci_core.c | 17 +++++
> net/bluetooth/hci_event.c | 20 ++++++
> net/bluetooth/hci_request.c | 119 ++++++++++++++++++++++++++-----
> net/bluetooth/hci_request.h | 3 +-
> net/bluetooth/mgmt.c | 2 +-
> 8 files changed, 157 insertions(+), 21 deletions(-)

can you please rebase this against the latest bluetooth-next tree. With the introduction of the advertising monitor, we now have a few conflicts that you need to resolve.

Also as discussed, we wanted to hide this initially after an experimental feature flag so it would need to be enabled first. And I think once enabled, we need to take the support of “advertising” aka peripheral away from the controller until we can fully ensure it works for scanning and advertising at the same time.

Regards

Marcel

2020-07-01 10:02:50

by Sathish Narasimman

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] LL Privacy support

Hi Marcel

On Fri, Jun 19, 2020 at 1:28 PM Marcel Holtmann <[email protected]> wrote:
>
> Hi Sathish,
>
> > Based on the input the patches modified and verifed accordingly.
> >
> > 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 (4):
> > 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
> >
> > include/net/bluetooth/hci.h | 8 ++-
> > include/net/bluetooth/hci_core.h | 3 +
> > net/bluetooth/hci_conn.c | 6 +-
> > net/bluetooth/hci_core.c | 17 +++++
> > net/bluetooth/hci_event.c | 20 ++++++
> > net/bluetooth/hci_request.c | 119 ++++++++++++++++++++++++++-----
> > net/bluetooth/hci_request.h | 3 +-
> > net/bluetooth/mgmt.c | 2 +-
> > 8 files changed, 157 insertions(+), 21 deletions(-)
>
> can you please rebase this against the latest bluetooth-next tree. With the introduction of the advertising monitor, we now have a few conflicts that you need to resolve.
>
> Also as discussed, we wanted to hide this initially after an experimental feature flag so it would need to be enabled first. And I think once enabled, we need to take the support of “advertising” aka peripheral away from the controller until we can fully ensure it works for scanning and advertising at the same time.

Updated the v3 rebased and the flag is introduced to enable LL
privacy from the host using mgmt set_privacy interface.

what I understood regarding removing of advertising was when we enable
LL privacy using mgmt interface. the advertisement should not work
(correct me please).

>
> Regards
>
> Marcel
>

Regards
Sathish N