[fromtree] bluetooth: fix bug when destroying tx buffers on disconnected #3025
Conversation
This file contains hidden or 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
alwa-nordic
force-pushed
the
l2cap_check_before_destroy
branch
from
7e248df to
b417cbb
Compare
…nd_buf" This reverts commit 827eb44. Signed-off-by: Aleksander Wasaznik <[email protected]>
…p_send_pdu" This reverts commit 685c53a. Signed-off-by: Aleksander Wasaznik <[email protected]>
alwa-nordic
force-pushed
the
l2cap_check_before_destroy
branch
from
b417cbb to
300ffe2
Compare
… disconnect Channel tx_queue purging on disconnect was inconsistently handled by the different channels: iso, l2cap, l2cap_br. iso channels handled purging in the tx_data_pull hook. l2cap and l2cap_br did the purging in channel delete functions and did not expect tx_data_pull to be called for a disconnected channel. Their data_pull functions could return a ptr to a net_buf that was still on the tx_queue, which is problematic when the conn tx_processor unrefs the returned buffer resulting in multiple calls to the buf destroy function. To make things consistent and correct, remove the code that tries to purge tx_queues in the tx_processor and only do purging in the channels themselves when they are deleted/disconnected. Also refactor and clarify referencing of the net_buf returned by tx_data_pull. It was confusing who had a reference and when, which could vary depending on the length of the original buffer. There are three cases: the buffer length is less than the tx.mps, greater the mps but less than the mtu so requiring segementation but not fragmentation, or greater than both mps and mtu so requiring both segmentation and fragmentation. The conn layer would increase the refcnt if the length was greater than the mtu, but not have any awareness of whether the net_buf was still on the tx_queue or not. Now it is the tx_data_pull callbacks responsibitity to increment the reference count if it is returning a pointer to a net_buf that it is still keeping on the tx_queue for segmentation purposes. The conn layer will now always transfer that reference into a fragment view and not conditional it on the length relative to the mtu, and always decrement the reference to the parent when the fragment is destroyed. So there is no risk of decrementing a reference to a net buf that might still be on a tx_queue, which simplifies error handling in particular. Also add error handling paths for when asserts are not enabled. Signed-off-by: Mike J. Chen <[email protected]> (cherry picked from commit a392c33)
Allow for an additional buffer reference if callback is provided. This can be used to extend lifetime of the net buffer until the data transmission is confirmed by ACK of the remote. Signed-off-by: Marek Pieta <[email protected]> (cherry picked from commit d6539f4) (cherry picked from commit 685c53a)
If ATT sent callback is delayed until data transmission is done by BLE controller, the transmitted buffer may have an additional reference. The reference is used to extend lifetime of the net buffer until the data transmission is confirmed by ACK of the remote. send_buf function can be called multiple times, if buffer has to be fragmented over HCI. In that case, the callback is provided as an argument only for the last transmitted fragment. The `buf->ref == 1` check is skipped because it's impossible to properly validate number of references for the sent fragments if buffers may have the additional reference. Jira: NCSDK-28624 Signed-off-by: Marek Pieta <[email protected]> (cherry picked from commit e589307) (cherry picked from commit 827eb44)
alwa-nordic
force-pushed
the
l2cap_check_before_destroy
branch
from
300ffe2 to
3f54299
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Cherry-pick commit from upstream #90975