Bluetooth: host: Add RPA in directed advertisement support #14984

joerchan · 2019-03-28T14:23:20Z

In order to advertise directed to a privacy enabled central the
initiator field of the directed adv packet needs to set to an RPA.
To instruct the controller to use an RPA in the initiator field own
address type should be set to either 0x02 or 0x03.
Since it is not certain that a remote device supports address resolution
of the initiator address we add an option to turn this on and give the
application the responsibility to check if peer supports this.

Fixes #14743

Signed-off-by: Joakim Andersson [email protected]

jhedberg · 2019-03-28T14:26:46Z

subsys/bluetooth/host/hci_core.c

+			if (IS_ENABLED(CONFIG_BT_SMP) &&
+			    !IS_ENABLED(CONFIG_BT_PRIVACY) &&
+			    BT_FEAT_LE_PRIVACY(bt_dev.le.features) &&
+			    param->options & BT_LE_ADV_OPT_DIR_ADDR_RPA) {


put parentheses around (foo & bar) to make it clear that you didn't intend to use &&

jhedberg · 2019-03-28T14:27:37Z

subsys/bluetooth/host/settings.c

-#define ID_SIZE_MAX sizeof(bt_dev.irk)
-#else
-#define ID_SIZE_MAX sizeof(bt_dev.id_addr)
-#endif


This doesn't seem related, and it was actually removed by one of my own PRs recently.

codecov-io · 2019-03-28T15:09:16Z

Codecov Report

Merging #14984 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #14984      +/-   ##
==========================================
+ Coverage   52.92%   52.93%   +0.01%     
==========================================
  Files         309      309              
  Lines       45268    45267       -1     
  Branches    10451    10451              
==========================================
+ Hits        23956    23960       +4     
+ Misses      16544    16542       -2     
+ Partials     4768     4765       -3

Impacted Files	Coverage Δ
include/bluetooth/hci.h	`77.77% <ø> (ø)`	⬆️
include/bluetooth/bluetooth.h	`31.57% <ø> (ø)`	⬆️
subsys/bluetooth/host/hci_core.c	`44.02% <100%> (-0.04%)`	⬇️
subsys/testsuite/ztest/src/ztest.c	`72.72% <0%> (+4.13%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e81cbe...1245d12. Read the comment docs.

jhedberg · 2019-04-01T10:30:53Z

There's something I don't understand here. The HCI spec says the following about values 0x02 and 0x03 for the Own_Address_Type parameter:
"
0x02 Controller generates the Resolvable Private Address based on the local IRK from the resolving list. If the resolving list contains no matching entry, use the public address.

0x03 Controller generates the Resolvable Private Address based on the local IRK from the resolving list. If the resolving list contains no matching entry, use the random address from LE_Set_Advertising_Set_Random_Address.
"

Now, as you say we don't have a local IRK, so based on the above our behaviour should be exactly the same as if 0x00 or 0x01 would have been given, right? So why do we go through the trouble to say 0x02 or 0x03 when we know the controller will fall back to just normal public/random address?

Another thing that's bothering me is that it should be abundantly clear what API features are workarounds for broken devices (which seems to be the case here - please correct me if I'm wrong), however it's not at all clear from the public API additions. To me the text you've added in the API sounds like this is normal behaviour for privacy-enabled centrals. Am I misunderstanding something here?

carlescufi

Looks good, just doc issues

carlescufi · 2019-04-01T10:34:05Z

include/bluetooth/bluetooth.h

+	 *  supports address resolution of directed advertisement.
+	 *  It is the responsibility of the application to check that the remote
+	 *  device supports address resolution of directed advertisements by
+	 *  reading it's Central Address Resolution characteristic.


@joerchan this doesn't seem to be fixed yet?

carlescufi · 2019-04-01T10:34:14Z

include/bluetooth/bluetooth.h

+
+	/** Enable use of Private Resolvable Address in directed advertisements
+	 *  when CONFIG_BT_PRIVACY is not enabled.
+	 *  This is required if the remote device is privacy enabled and


privacy-enabled

carlescufi · 2019-04-01T10:35:18Z

include/bluetooth/bluetooth.h

@@ -277,6 +277,16 @@ enum {
 	 *  bt_conn_create_slave_le().
 	 */
 	BT_LE_ADV_OPT_DIR_MODE_LOW_DUTY = BIT(4),
+
+	/** Enable use of Private Resolvable Address in directed advertisements


*Resolvable Private Address (RPA)
Please specify that this only affects TargetA and not AdvA

jhedberg · 2019-04-01T11:55:11Z

@joerchan for the record, @carlescufi helped clarify the situation with own_addr_type and the peer_addr being linked even when we don't have a local IRK. I.e. 0x02/0x03 is not quite the same as 0x00/0x01 when we don't have an IRK ourselves. The requested clarifications by @carlescufi are helpful in this regard.

carlescufi

LGTM now, thanks!

jhedberg

Besides the typo that's not yet fixed this PR is fine with me

jhedberg · 2019-04-02T10:30:12Z

include/bluetooth/bluetooth.h

+	 *  supports address resolution of directed advertisement.
+	 *  It is the responsibility of the application to check that the remote
+	 *  device supports address resolution of directed advertisements by
+	 *  reading it's Central Address Resolution characteristic.


@joerchan this doesn't seem to be fixed yet?

In order to advertise directed to a privacy enabled central the initiator field of the directed adv packet needs to set to an RPA. To instruct the controller to use an RPA in the initiator field own address type should be set to either 0x02 or 0x03. Since it is not certain that a remote device supports address resolution of the initiator address we add an option to turn this on and give the application the responsibility to check if peer supports this. Fixes zephyrproject-rtos#14743 Signed-off-by: Joakim Andersson <[email protected]>

joerchan requested review from jhedberg, sjanc and Vudentz as code owners March 28, 2019 14:23

jhedberg requested changes Mar 28, 2019

View reviewed changes

joerchan added the area: Bluetooth label Mar 28, 2019

joerchan force-pushed the adv_dir_rpa_support branch from b1fc94b to 1245d12 Compare March 29, 2019 09:12

joerchan requested a review from jhedberg April 1, 2019 09:24

jhedberg requested review from cvinayak and carlescufi April 1, 2019 10:31

carlescufi requested changes Apr 1, 2019

View reviewed changes

joerchan force-pushed the adv_dir_rpa_support branch 2 times, most recently from 24fe883 to f96e87f Compare April 2, 2019 10:10

carlescufi approved these changes Apr 2, 2019

View reviewed changes

jhedberg approved these changes Apr 2, 2019

View reviewed changes

joerchan force-pushed the adv_dir_rpa_support branch from f96e87f to cead2e8 Compare April 2, 2019 10:42

carlescufi merged commit 710adac into zephyrproject-rtos:master Apr 2, 2019

joerchan deleted the adv_dir_rpa_support branch September 24, 2019 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Bluetooth: host: Add RPA in directed advertisement support #14984

Bluetooth: host: Add RPA in directed advertisement support #14984

Uh oh!

joerchan commented Mar 28, 2019

Uh oh!

jhedberg Mar 28, 2019

Uh oh!

jhedberg Mar 28, 2019

Uh oh!

codecov-io commented Mar 28, 2019 •

edited

Loading

Uh oh!

jhedberg commented Apr 1, 2019

Uh oh!

carlescufi left a comment

Uh oh!

carlescufi Apr 1, 2019

Uh oh!

jhedberg Apr 2, 2019

Uh oh!

carlescufi Apr 1, 2019

Uh oh!

carlescufi Apr 1, 2019 •

edited

Loading

Uh oh!

jhedberg commented Apr 1, 2019

Uh oh!

carlescufi left a comment

Uh oh!

jhedberg left a comment

Uh oh!

jhedberg Apr 2, 2019

Uh oh!

Uh oh!

Bluetooth: host: Add RPA in directed advertisement support #14984

Bluetooth: host: Add RPA in directed advertisement support #14984

Uh oh!

Conversation

joerchan commented Mar 28, 2019

Uh oh!

jhedberg Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

jhedberg Mar 28, 2019

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Mar 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jhedberg commented Apr 1, 2019

Uh oh!

carlescufi left a comment

Choose a reason for hiding this comment

Uh oh!

carlescufi Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

jhedberg Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

carlescufi Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

carlescufi Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhedberg commented Apr 1, 2019

Uh oh!

carlescufi left a comment

Choose a reason for hiding this comment

Uh oh!

jhedberg left a comment

Choose a reason for hiding this comment

Uh oh!

jhedberg Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-io commented Mar 28, 2019 •

edited

Loading

carlescufi Apr 1, 2019 •

edited

Loading