-
Notifications
You must be signed in to change notification settings - Fork 676
Bluetooth: Host: Bonding with the same Central using multiple identities #2963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
subsys/bluetooth/host/Kconfig
Outdated
bool "Automatically swap conflicting entries in the Resolving List" | ||
depends on !BT_ID_UNPAIR_MATCHING_BONDS | ||
depends on BT_PRIVACY && BT_PERIPHERAL && !BT_CENTRAL | ||
select EXPERIMENTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I need to keep this option as experimental.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think experimental should be removed as this goes into production by customer.
358652b
to
024e250
Compare
Just curious how are implemented bsim tests going to be checked if CI bsim in downstream isn't run? |
Manually |
187ccc8
to
92987f9
Compare
Added persistent storage test to verify that re-connection works fine after recovering bonding data from flash. |
subsys/bluetooth/host/id.c
Outdated
struct bt_id_conflict { | ||
struct bt_keys *candidate; | ||
struct bt_keys *found; | ||
bool all; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one nitpick: all
seems to me a bit ambiguous. Perhaps better to call it check_all_irk
or similar? Feel free to disregard. Otherwise to me this PR looks good, although with the caveat that I am still lacking quite some knowledge of the host, nor did I manually run the bsim tests you added to check.
@@ -630,6 +630,24 @@ config BT_ID_UNPAIR_MATCHING_BONDS | |||
link-layer. The Host does not have control over this acknowledgment, | |||
and the order of distribution is fixed by the specification. | |||
|
|||
config BT_ID_AUTO_SWAP_MATCHING_BONDS | |||
bool "Automatically swap conflicting entries in the Resolving List" | |||
depends on !BT_ID_UNPAIR_MATCHING_BONDS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "!BT_CENTRAL" is unnecessary. You may have central which is dealing with other non-private connections with other peripherals, and that would work without any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't even want to think what will happen if BT_CENTRAL
is enabled. The current use case is for Peripheral role only. Therefore it is easier to prevent compiling with BT_CENTRAL
. If a customer comes with request to support Central role as well, we will look at this again.
Addressed comments. Also found bug when removing last conflicting key during unpairing. |
750b10f
to
447cebe
Compare
Test commit is dropped. Tests are moved to the internal test framework. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated: bt_le_adv_start_ext could be made static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the github actions should be fixed.
subsys/bluetooth/host/Kconfig
Outdated
bool "Automatically swap conflicting entries in the Resolving List" | ||
depends on !BT_ID_UNPAIR_MATCHING_BONDS | ||
depends on BT_PRIVACY && BT_PERIPHERAL && !BT_CENTRAL | ||
select EXPERIMENTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think experimental should be removed as this goes into production by customer.
aeab23c
to
966aa7a
Compare
This commit adds a new Kconfig option by enabling which Host will keep bonding with the same Central instead of rejecting pairing. Brief implementation details: This implementation adds a new flag to bt_keys struct: BT_KEYS_ID_CONFLICT. The flag is set, when: - bonding with the same peer and conflict identified - when loading conflicting keys from persistent storage. When bonding and conflict is identified, the new keys aren't added to the Resolving List immediately. Instead, the old keys stay in the Resolving List. When start advertising, Host finds conflicting keys that are already added to the Resolving List and substitues them. If, however, there is another advertiser already started for the added keys, the new request is reject and advertising start function returns -EPERM. This is supported by Peripheral role only for now. Signed-off-by: Pavel Vasilyev <[email protected]>
|
This PR adds a feature to allow bonding with the same Central using multiple identities.
The PR consists of 2 commits.
The first commit adds a new Kconfig option by enabling which Host will keep
bonding with the same Central instead of rejecting pairing.
Brief implementation details:
This implementation adds a new flag to bt_keys struct:
BT_KEYS_ID_CONFLICT. The flag is set, when:
When bonding and conflict is identified, the new keys aren't added to
the Resolving List immediately. Instead, the old keys stay in the
Resolving List.
When start advertising, Host finds conflicting keys that are already
added to the Resolving List and substitues them.
If, however, there is another advertiser already started for the added
keys, the new request is reject and advertising start function returns
-EPERM.
The second adds the following tests: