bluetooth: host: Avoid sending duplicate device to RL. #48641
Conversation
ibaz-nordic
requested review from
alwa-nordic,
jhedberg,
Vudentz and
sjanc
as code owners
There was a problem hiding this comment.
Couple of inline comments. Also, could you explain what parts of bt_id_add() functionality is still critical for a duplicate entry? My initial (and probably naïve) thought is that do we need to call the function at all in the case of a duplicate?
Another way to avoid the controller error would be to always remove the entry via HCI before trying to add it. Yet another option is to simply cleanly deal with the specific HCI error code that results from a duplicate.
subsys/bluetooth/host/id.c
Outdated
| @@ -864,13 +864,19 @@ void bt_id_add(struct bt_keys *keys) | |||
| goto done; | |||
| } | |||
|
|
|||
| if (duplicate_rl && !IS_ENABLED(CONFIG_BT_PRIVACY)) { | |||
There was a problem hiding this comment.
What about the rl_entries == rl_size branch above? That would still increment rl_entries even for a duplicate.
Also (minor issue): you have two spaces after &&
There was a problem hiding this comment.
The way I see it, rl_entries is used to check if there's any address resolution. In this situation, it should have been counted before. So there's no need to increment.
ibaz-nordic
force-pushed
the
zephyr_duplicate_rl
branch
from
665849f to
923a1ec
Compare
From Core 5.3 p2424 BT_HCI_OP_LE_ADD_DEV_TO_RL should return Invalid HCI Command I think identifying a duplicate address and not adding it is a better solution than deleting and then adding because a flash cycle is saved. |
Core v5.3, Vol 4, Part E, 7.8.38 Controller has the option to accept or reject when the same device is added to RL. In this patch host will not add the same device to controller RL. Signed-off-by: Azizah Ibrahim <[email protected]>
ibaz-nordic
force-pushed
the
zephyr_duplicate_rl
branch
from
923a1ec to
b940195
Compare
There was a problem hiding this comment.
I think this is a completely wrong approach to fix the problem.
Abstractly, each local identity has its own bond database.
Each entry in the bond database must have a remote identity address and optionally a remote identity resolving key.
Both the remote identity address and the corresponding identity resolve key must each be unique within that bond database. This is because must exist a two way mapping between them.
A remote identity should be considered bonded only when it's has been added to the bond database. Before that, the bond is incomplete.
When we obtain an identity address and everything else necessary for a bond, we should try to realize the bond by adding it to the bond database. The database must refuse if there is a conflict in the uniqueness constraints. When that happens, we can prompt the application to handle the problem of a conflicting bond. The application can choose to use the normal bond deletion mechanism to delete the conflicting bond, in which case the new bond can be stored. Or it an disconnect to refuse.
If the application deletes the conflicting bond, one of the things that should happen is that the old identity address and identity resolving key are removed from the controller resolve list. This solves the problem this PR is trying to address.
I will take some time now to sketch out a possible realization of this, and give you the details in case you want to implement this.
|
@alwa-nordic will look into a different solution |
Fixes: #45827
Signed-off-by: Azizah Ibrahim [email protected]