Bluetooth: controller: Ignore connections from same peer #32700
Conversation
wopu-ot
requested review from
carlescufi,
cvinayak,
kruithofa and
thoh-ot
as code owners
Rename the peer's address in the advertising set struct from `id_addr` to `peer_addr` to clarify what the address refers to. Signed-off-by: Wolfgang Puffitsch <[email protected]>
wopu-ot
force-pushed
the
ignore_same_peer
branch
from
d953e15
to
6b0db82
Compare
subsys/bluetooth/controller/Kconfig
Outdated
| config BT_CTLR_IGNORE_SAME_PEER_CONN | ||
| bool "Ignore connection requests from same peer" | ||
| default y | ||
| help | ||
| Ignored connection requests from the same peer. While the | ||
| Bluetooth specification does not allow multiple connections | ||
| with the same peer, allowing such connections is useful | ||
| while debugging multiple connections. |
There was a problem hiding this comment.
If the spec specifies a shall for this, why is it a Kconfig? Wouldn't disabling this cause the stack to be non-compliant?
There was a problem hiding this comment.
This was suggested by @carlescufi #26172 (comment)
There was a problem hiding this comment.
I see. Then I would suggest only allowing it to be turned on for TEST/DEBUG builds, e.g. by only allowing it be be n if e.g. BT_TESTING is enabled.
I don't think it's a good idea to intentionally allowing a non-spec compliant build without some sort of warning or making sure that whoever disables this is aware of the consequences.
There was a problem hiding this comment.
I could make it dependent on BT_CTLR_ADVANCED_FEATURES, though I believe I would have to flip the polarity (...ALLOW... instead of ...IGNORE... and !defined instead of defined) to get the correct behavior if the former is not defined.
There was a problem hiding this comment.
Alternatively we could add similar warnings to disabling this as we have when e.g. we enable SMP debug keys. See #26186
There was a problem hiding this comment.
I moved the user-controller flag it under BT_CTLR_ADVANCED_FEATURES and added a WARNING to the description. I also made the configuration now dependent on having more than one connection to not punish implementations that allow only a single connection anyway.
wopu-ot
force-pushed
the
ignore_same_peer
branch
from
6b0db82
to
7d0215e
Compare
Ignore connection indications from peers that are already connected. This is to bring the behavior of the controller in accordance with [5.2, Vol 6, Part B, 4.5 Connection state]: "If an advertiser receives a connection request from an initiator it is already connected to, it shall ignore that request." Signed-off-by: Wolfgang Puffitsch <[email protected]>
wopu-ot
force-pushed
the
ignore_same_peer
branch
from
7d0215e
to
6955b9c
Compare
| WARNING: This option enables behavior that violates the Bluetooth | ||
| specification. |
There was a problem hiding this comment.
I would like @carlescufi input on whether this is enough, or if we also should add build warnings like was done for other such configurations in #26186
There was a problem hiding this comment.
I would like @carlescufi input on whether this is enough, or if we also should add build warnings like was done for other such configurations in #26186
@carlescufi What is your take in this?
There was a problem hiding this comment.
@wopu-ot Is this something that is critical to get in? I can mark it as approved, and then we can consider adding additional warnings (if needed) afterwards in a separate PR.
Ignore connection indications from peers that are already
connected. This is to bring the behavior of the controller in
accordance with [5.2, Vol 6, Part B, 4.5 Connection state]:
"If an advertiser receives a connection request from an initiator it
is already connected to, it shall ignore that request."
Signed-off-by: Wolfgang Puffitsch [email protected]
Fixes #26172