V3.6.99 ncs2 #2051
Open
V3.6.99 ncs2 #2051
+42,961
−5,982
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 current TX pattern in the host is to try to push a buffer through all the layers up until it is ingested by the controller. Since sending can fail at any layer, we need error-handling and separate retry logic on pretty much all layers. That logic obscures the "happy path" for people trying ot understand the code. This commit inverts the control, in a way that doesn't require changing the host or HCI driver API (yet): Layers don't send buffers synchronously, they instead put their buffer in a private queue of their own and raise a TX flag on the lower layer. Think of it as a `READY` interrupt line that has to be serviced by the lower layer. Sending is now non-blocking, rate depends on the size of buffer pools. There is a single TX processing function. This can be thought as the Interrupt Service Routine that will handle the `READY` interrupt from the layers above. That `tx_processor()` will then attempt to allocate enough resources in order to send the buffer through to the controller. This allocation logic does not block. After acquiring all the resources, the TX processor will attempt to pull data from the upper layer. The upper layer has to figure out which buffer to pass to the controller. This is a good spot to put scheduling or QoS logic in the upper layer. Notes: - user-facing API for tuning QoS will be implemented in a future patch - this scheme could (and probably will) be extended to upper layers (e.g. ATT, L2CAP CoC segmentation). - this patch removes the `pending_no_cb()` memory optimization for clarity/correctness. It might get re-implemented after a stabilization period. Hopefully with more documentation. Signed-off-by: Jonathan Rico <[email protected]> Co-authored-by: Aleksander Wasaznik <[email protected]> (cherry picked from commit 28535fe)
… TX is done" This reverts commit d74e0b5. Signed-off-by: Rubin Gerritsen <[email protected]>
This API replaces `bt_l2cap_send()` and `bt_l2cap_send_cb()`. The difference is that it takes the `struct bt_l2cap_le_chan` object directly instead of a connection + CID. We need the channel object in order to put the PDU on the TX queue. It is inefficient to do a search for every PDU when the caller knows the channel object's address and can just pass it down. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 38820ef)
We don't need it thanks to the new TX architecture. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 48d1cff)
We don't need the TX thread anymore. Generalizing the pull-based architecture (ie. `tx_processor`) to HCI commands makes it possible to run the whole TX path from the the system workqueue, or any workqueue really. There is an edge-case, where we call `bt_hci_cmd_send_sync()` from the syswq, stalling the system. The proposed mitigation is to attempt to drain the command queue from within `bt_hci_cmd_send_sync()`. My spidey sense tingles however, and it would be better to just remove the capability of calling this fn from the syswq. But doing this requires refactoring a bunch of synchronous procedures in the stack (e.g. stack init, connection establishment, address setting etc), dragging in more work. I will do it, but in a later patch. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 28be890)
We can get rid of the view pool for SDU segments :) We have to make the code slightly more complex :'( The basic idea is always giving the original SDU buffer to `conn.c` for it to pull ACL fragments from. In order to do this, we need to add the PDU headers just-in-time. `bt_l2cap_send_pdu()` does not add them before putting the PDU on the queue anymore. They are added by `l2cap_data_pull()` right before the data leaves `l2cap.c` for `conn.c`. We also have to inform `conn.c` "out of band" of the real L2CAP PDU size so it doesn't fragment across segment boundaries. This oob is the new `length` parameter to the `.pull()` method. This is the added complexity mentioned above. Since SDU segmentation concerns only LE-L2CAP, ISO and Classic L2CAP don't need this extra logic. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit b6cdf10)
…variables In order to suppress compiler warnings w/o using void/ifdef. Suggested in #72854 Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 5a7ef42)
View buffers are now also a limited resource. Acquire them before attempting to pull data. `CONFIG_BT_CONN_FRAG_COUNT` should be tuned on a per-application basis to avoid this. A possible optimization, that was present before, is to not create a frag when the original buffer fits the controller's HCI size. I prefer deferring this optimization to a future patchset. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 9b3f41d)
The same way as `bt_hci_get_adv_handle` and `bt_hci_get_conn_handle` add a function to get the handle of a periodic advertising sync. Signed-off-by: Théo Battrel <[email protected]> (cherry picked from commit 7c3a5d5)
Adds missing doc on public member. Signed-off-by: Knut Eldhuset <[email protected]> (cherry picked from commit db9308d)
…I APIs Add versioning to the new HCI API so that it shows up officially as unstable, and add a reference to the new API from the old API. Signed-off-by: Johan Hedberg <[email protected]> (cherry picked from commit 1c53726)
ISO connections that were in the TX queue were not getting serviced in time. This happens because `iso_data_pull()` returns `NULL` when that particular connection (`conn`) is done sending. But it doesn't trigger the TX processor again to process other channels in the queue. This patch fixes that by calling `bt_tx_irq_raise()`. We can't do this from `conn.c` as we don't know if the `NULL` returned is because the current channel is out of data or because it has data but it can't send it (e.g. the current buf is being "viewed" already). Fixes zephyrproject-rtos/zephyr#74321 Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 8af7180)
Similar to ISO connections, ACL connections are not serviced as fast as possible. Change this, and try to send as much as we have resources for. Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit c6345c6)
This API converts a SMP error code to a string. This can be useful if application developers want to print them in the applications. BT_SMP_ERR_SUCCESS was added for completeness. Later we can also use them in the host to improve debuggability. Signed-off-by: Rubin Gerritsen <[email protected]> (cherry picked from commit b25985a)
This can be useful if application developers want to print them in the applications. Later we can also use them in the host to improve debuggability. Signed-off-by: Rubin Gerritsen <[email protected]> (cherry picked from commit 69fb606)
Use K_WORK defined. This delayed work is never used with any other timeout than K_NO_WAIT, so the "delayed" part of it is never actually needed Signed-off-by: Lingao Meng <[email protected]> (cherry picked from commit cfd79e8)
…end_sync` `cmd(buf)` depends on this since it uses `net_buf_id`, which would alias multiple pools. Signed-off-by: Aleksander Wasaznik <[email protected]> (cherry picked from commit a9c95c5)
The `rsp` params actually not used, so removed. Signed-off-by: Lingao Meng <[email protected]> (cherry picked from commit b11c43c)
…sage If func:`bt_hci_cmd_send_sync` return no-zero value, indicate that `cmd(buf)->status` not zero, in this condition, rsp not to assign anything, so remove it. Signed-off-by: Lingao Meng <[email protected]> (cherry picked from commit f6480db)
Fix Bluetooth initialization problem caused by PR#72090 for at least ST boards that are using BlueNRG BLE modules. For more information, please refer to issue #74528. Signed-off-by: Ali Hozhabri <[email protected]> (cherry picked from commit 3b726de)
Modified to check the length of the remaining data in buffer before processing the next report. The length check is missing in the cont routine. Signed-off-by: Eunkyu Lee <[email protected]> (cherry picked from commit e491f22)
…d warnings
This commit fixes compilation warnings that are present when compiling
with CONFIG_BT_SMP_OOB_LEGACY_PAIR_ONLY as can be seen in this
compiler log:
"""
In file included from /zephyr-sdk-0.16.1/arm-zephyr-eabi/picolibc/
include/string.h:215,
from /zephyr/subsys/bluetooth/host/smp.c:15:
In function '__memcpy_ichk',
inlined from 'sc_send_public_key' at /zephyr/subsys/bluetooth/host/
smp.c:3006:2:
/zephyr-sdk-0.16.1/arm-zephyr-eabi/picolibc/include/ssp/string.h:83:1:
warning: argument 2 null where non-null expected [-Wnonnull]
83 | __ssp_bos_icheck3_restrict(memcpy, void *, const void *)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
/zephyr-sdk-0.16.1/arm-zephyr-eabi/picolibc/include/ssp/string.h:83:1:
note: in a call to built-in function '__builtin_memcpy'
/zephyr/subsys/bluetooth/host/smp.c: In function 'smp_public_key':
/zephyr/subsys/bluetooth/host/smp.c:4214:21: warning: argument 2
null where non-null expected [-Wnonnull]
4214 | memcmp(smp->pkey, sc_public_key, BT_PUB_KEY_COORD_LEN) == 0) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/zephyr-sdk-0.16.1/arm-zephyr-eabi/picolibc/include/string.h:62:10: note:
in a call to function 'memcmp' declared 'nonnull'
62 | int memcmp (const void *, const void *, size_t);
| ^~~~~~
"""
The warning is caused by the potential use of NULL "sc_public_key"
global pointer that is not assigned a value in "smp_init()" if
CONFIG_BT_SMP_OOB_LEGACY_PAIR_ONLY is enabled. This commit
conditionally changes the behavior of function "smp_public_key()"
if CONFIG_BT_SMP_OOB_LEGACY_PAIR_ONLY is activated to simply return
and not use the "sc_public_key" variable. Other functions that are not
called anymore by "smp_public_key()" are also conditionally
deactivated when CONFIG_BT_SMP_OOB_LEGACY_PAIR_ONLY is enabled
Signed-off-by: Sebastian Panceac <[email protected]>
(cherry picked from commit 9ce338d)
`bt_le_ext_adv_start` does not modify the `param` argument, which can therefore be marked as `const`. This allows the struct to exist purely in ROM. Signed-off-by: Jordan Yates <[email protected]> (cherry picked from commit 40eeded)
The rationale behind that change is that the Application can use the `bt_conn_le_param_update()` API to signal the controller to reschedule the link. Even if the new connection params are within the old ones, the controller would be free to choose an e.g. smaller interval. The host API should not prevent this usage. Fixes zephyrproject-rtos/zephyr#74292 Co-authored-by: Knut Eldhuset <[email protected]> Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit ac37d64)
When syncing to a PA using PAST then the sync_info.recv_enabled was always just set to true, regardless of what mode was set during the subscribe parameters. The mode(s) are now stored in an array (with the default value as well) so that we can retrieve that information when the PA has synced via PAST. It was considered to put the `mode` value into the `bt_conn` struct, but that would require an API change as the `bt_conn` parameter for the subcribe function uses `const`. This commit also modifies the guard for PAST to be the more correct value CONFIG_BT_PER_ADV_SYNC_TRANSFER_RECEIVER instead of just CONFIG_BT_CONN. Signed-off-by: Emil Gydesen <[email protected]> (cherry picked from commit 711b42a)
…_pool `struct acl_data` is used even when Host flow control is not enabled. It is written to through the `acl(buf)` accessor in `conn.c:hci_acl()`. Hopefully no netbufs were harmed by that :/ Signed-off-by: Jonathan Rico <[email protected]> (cherry picked from commit 792ae68)
…eset timeout Some HCI drivers issue HCI reset when disabling, like the IPC HCI driver. We need to keep the RX thread running to allow receiving the command complete. This commit postpones aborting the RX thread until this is done. The issue happens started occuring after commit d0e75ab87c4b53d66008c941c38709a2fca9dbea. Fixes #76202. Upstream PR: zephyrproject-rtos/zephyr#76203 Signed-off-by: Rubin Gerritsen <[email protected]>
By default, the BLE stack calls sent callback for ATT data when the data is passed to BLE controller for transmission. Enabling this Kconfig option delays calling the sent callback until data transmission is finished by BLE controller (the callback is delayed until receiving the num complete packets event). Jira: NCSDK-27422 Signed-off-by: Marek Pieta <[email protected]> (cherry picked from commit d74e0b5)
The function `bt_hci_le_past_received_v2()` is not compiled in for this configuration, so the reference needs to be removed. Fixes #76268. Upstream PR: zephyrproject-rtos/zephyr#76269 Signed-off-by: Rubin Gerritsen <[email protected]>
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]>
…tate callback Adds an optional callback which can be used to append custom fields to the image slot state command response Signed-off-by: Jamie McCrae <[email protected]> (cherry picked from commit 031f9f2)
For nrf54h20 a range of combinations exist to configure the test and debug domains data sources and sinks. Expose them in DTS to allow configuring them. Also drop the previous style which was too rigid to extend to cover all cases cleanly. The old style was only used in a single sample application so far. Signed-off-by: Karsten Koenig <[email protected]> (cherry picked from commit 43f9488)
We don't want an exact version match, a minimum one is enough. This aligns it also with the upstream copy of this file. Signed-off-by: Karsten Koenig <[email protected]>
|
none Note: This comment is automatically posted and updated by the Contribs GitHub Action. |
NordicBuilder
requested review from
gmarull,
hermabe,
jori-nordic,
Thalley and
theob-pro
Test did not support PPR core due to failing GRTC channel allocation. This change fixes the channel allocation and enables PPR core test. Signed-off-by: Bartosz Miller <[email protected]> (cherry picked from commit e26b975)
rlubos
requested review from
anangl,
carlescufi,
ankuns,
de-nordic,
nordic-krch,
aescolar,
nordicjm and
a team
as code owners
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.
No description provided.