Host privacy feature configuration #13662
Conversation
paul-szczepanek-arm
changed the base branch from
master
to
feature-ble-host-privacy
|
@paul-szczepanek-arm, thank you for your changes. |
paul-szczepanek-arm
force-pushed
the
host-privacy-feature
branch
2 times, most recently
from
fbd3de8 to
3b97267
Compare
There was a problem hiding this comment.
Thanks Paul, the changes looks good however I question if we shouldn't have a single compile time flag that indicates what type of privacy should be used. I'm thinking of three case:
- When it is determined at runtime based on controller capabilities.
- Host based, removing all controller based privacy support
- Controller based removing host based privacy support except for address generation.
Another option is to set the right privacy configuration in the target configuration.
| "ble-gap-host-privacy-resolving-list-size": { | ||
| "help": "Used for host privacy. How many pairs of resolvable private addresses and identity address to store.", | ||
| "value": 8, | ||
| "macro_name": "BLE_GAP_HOST_PRIVACY_RESOLVED_LIST_SIZE" |
There was a problem hiding this comment.
It would made sense for it to be equal to BLE_SECURITY_DATABASE_MAX_ENTRIES There won't be more entries than there is in the security DB.
There was a problem hiding this comment.
replaced with BLE_SECURITY_DATABASE_MAX_ENTRIES
| { | ||
| #if (BLE_GAP_HOST_BASED_PRIVACY == 0) | ||
| /* we need either privacy on host or controller to enable it */ | ||
| if (enable && !_address_registry.is_controller_privacy_supported()) { |
There was a problem hiding this comment.
Why do we check for enable if host base privacy is not enabled ?
There was a problem hiding this comment.
narrowed down the meaning to address resolution and fixed ifdefs accordingly
| #endif // BLE_ROLE_OBSERVER | ||
| } | ||
|
|
||
| #if BLE_GAP_HOST_BASED_PRIVACY |
There was a problem hiding this comment.
Can it be return early or execute the body ? Computations above are pointless if host base privacy is enabled.
There was a problem hiding this comment.
narrowed down the meaning to address resolution and fixed ifdefs accordingly
| }, | ||
| "ble-gap-host-based-privacy": { | ||
| "help": "Perform address resolution on the host, not the controller. Controller based privacy is preferred as it happens lower down the stack but this can be used in case controller based privacy is unavailable. If this is enabled the controller will not be used for privacy.", | ||
| "value": false, |
There was a problem hiding this comment.
I'd expect this value to be on by default.
There was a problem hiding this comment.
changed to true
| bool PrivateAddressController::is_controller_privacy_supported() | ||
| { | ||
| #if BLE_GAP_HOST_BASED_PRIVACY | ||
| return false; |
There was a problem hiding this comment.
I'm not sure this is accurate host based privacy may be enabled on a board with LL privacy available.
There was a problem hiding this comment.
narrowed down the meaning to address resolution and fixed ifdefs accordingly
|
This PR cannot be merged due to conflicts. Please rebase to resolve them. |
paul-szczepanek-arm
force-pushed
the
host-privacy-feature
branch
from
f4e93fd to
082f732
Compare
| } | ||
| } | ||
| #else | ||
| if (!apply_peripheral_privacy_connection_policy(event)) { |
There was a problem hiding this comment.
That will cause an issue if BLE_FEATURE_PRIVACY is not set as apply_peripheral_privacy_connection_policy won't be available.
4 participants
Summary of changes
Add configuration for host privacy feature.
Impact of changes
Migration actions required
Documentation
none
Pull request type
Test results
Reviewers