Bluetooth: Host: Fix ATT PDU alloc being non-blocking. #46247
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The API is documented as being blocking. Making it nonblocking was an unintentional API change. Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
hermabe
commented
Jun 3, 2022
| @@ -542,18 +543,19 @@ struct net_buf *bt_att_chan_create_pdu(struct bt_att_chan *chan, uint8_t op, | |||
| case ATT_RESPONSE: | |||
| case ATT_CONFIRMATION: | |||
| /* Use a timeout only when responding/confirming */ | |||
| buf = bt_l2cap_create_pdu_timeout(NULL, 0, BT_ATT_TIMEOUT); | |||
| timeout = BT_ATT_TIMEOUT; | |||
There was a problem hiding this comment.
One issue here is that the timeout is used for both allocations. Potentially, the total timeout is greater than BT_ATT_TIMEOUT.
jori-nordic
reviewed
Jun 3, 2022
| break; | ||
| default: | ||
| buf = bt_l2cap_create_pdu(NULL, 0); | ||
| timeout = K_FOREVER; |
There was a problem hiding this comment.
is there a reason to not use BT_ATT_TIMEOUT for all allocations ?
There was a problem hiding this comment.
The ATT timeout is just from a request is sent to the response is received. It is ok to wait before sending a request (and the API is documented as blocking).
jori-nordic
approved these changes
Jun 3, 2022
|
I see this (relates to this #25917): And this (seems new!): But does not cause any faults, the modified central_hr continues normal after the error messages. |
jhedberg
approved these changes
Jun 3, 2022
cvinayak
approved these changes
Jun 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The API is documented as being blocking. Making it non-blocking was an unintentional API change.
Introduced in #44269.