-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Bluetooth: controller: split: Fix response to unexpected LL_FEATURE_RSP #23495
Conversation
Fix response to unexpected LL_FEATURE_RSP for the case that BT_CTLR_SLAVE_FEAT_REQ is disabled. Fixes LL/PAC/SLA/BV-01 for such a configuration. Signed-off-by: Wolfgang Puffitsch <wopu@demant.com>
@@ -5407,7 +5407,9 @@ static inline int ctrl_rx(memq_link_t *link, struct node_rx_pdu **rx, | |||
#endif /* CONFIG_BT_CTLR_SLAVE_FEAT_REQ */ | |||
|
|||
case PDU_DATA_LLCTRL_TYPE_FEATURE_RSP: | |||
if (!pdu_len_cmp(PDU_DATA_LLCTRL_TYPE_FEATURE_RSP, | |||
if ((!IS_ENABLED(CONFIG_BT_CTLR_SLAVE_FEAT_REQ) && |
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.
Conditional compile the complete block and other associated code (feature_rsp_recv
) with:
#if defined(CONFIG_BT_CTLR_SLAVE_FEAT_REQ) || defined(CONFIG_BT_CENTRAL)
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.
That would not work in a configuration with CONFIG_BT_CENTRAL=y
, CONFIG_BT_PERIPHERAL=y
, and CONFIG_BT_CTLR_SLAVE_FEAT_REQ=n
(i.e., where the same device can be master and slave).
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.
if CONFIG_BT_CTLR_SLAVE_FEAT_REQ=n
then only central role is to receive the PDU_DATA_LLCTRL_TYPE_FEATURE_RSP
, and the default
case will generate the unknown response in slave role.
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 stand corrected. The original changes look good (where the same device can be master and slave, but not support slave feature req).
@@ -5407,7 +5407,9 @@ static inline int ctrl_rx(memq_link_t *link, struct node_rx_pdu **rx, | |||
#endif /* CONFIG_BT_CTLR_SLAVE_FEAT_REQ */ | |||
|
|||
case PDU_DATA_LLCTRL_TYPE_FEATURE_RSP: | |||
if (!pdu_len_cmp(PDU_DATA_LLCTRL_TYPE_FEATURE_RSP, | |||
if ((!IS_ENABLED(CONFIG_BT_CTLR_SLAVE_FEAT_REQ) && |
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 stand corrected. The original changes look good (where the same device can be master and slave, but not support slave feature req).
Fix response to unexpected LL_FEATURE_RSP for the case that
BT_CTLR_SLAVE_FEAT_REQ is disabled. Fixes LL/PAC/SLA/BV-01 for such a
configuration.
Fixes #23494