Add a pointer to the associated server structure in the L2CAP accept()
callback. This allows the callee to know which server an incoming L2CAP
connection is associated with.
Signed-off-by: Donatien Garnier <donatien.garnier@blecon.net>
Modify the signature of the k_mem_slab_free() function with a new one,
replacing the old void **mem with void *mem as a parameter.
The following function:
void k_mem_slab_free(struct k_mem_slab *slab, void **mem);
has the wrong signature. mem is only used as a regular pointer, so there
is no need to use a double-pointer. The correct signature should be:
void k_mem_slab_free(struct k_mem_slab *slab, void *mem);
The issue with the current signature, although functional, is that it is
extremely confusing. I myself, a veteran Zephyr developer, was confused
by this parameter when looking at it recently.
All in-tree uses of the function have been adapted.
Fixes#61888.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Using a different channel for responding to a request is forbidden by spec.
The allocator was especially flawed as it iterated over all the EATT
channels to find one w/ a big enough MTU, but the sending was still done
over the same channel as the REQ.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
This to ensure we don't fail to send a response and never get an ATT
TIMEOUT due to ACL TX buffer starvation caused by other users of the stack.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
The ATT module has provisions to queue a packet/buffer for sending later if
it can't send it right away. For example if the conn.c tx context
allocation fails.
This unfortunately doesn't work if the buffer can't get allocated in the
first place, or if the ATT metadata can't also be allocated.
The metadata is allocated from a global pool set to the same number as
conn.c TX contexts. That can lead to a situation where other users of ATT
manage to queue a bunch of buffers (e.g. the app spamming GATT
notifications), depleting the number of ATT metadata slots so that none are
available.
When none are available, and we receive an ATT REQ, we try to allocate one,
fail, and drop the buffer (!). That pretty much guarantees an ATT timeout.
As a workaround for this, use a per-channel metadata slot, that is only
used for completing transactions.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
This is useful in multilink scenarios, especially since there is not user
callback when the ATT channel times out.
Adding a user-facing callback should ideally also be done, but just logging
the address already provides useful insight.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Easier to debug that way.
Ideally we'd have more error codes/logging instead of just returning
UNLIKELY for most errors.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Make `bt_eatt_count()` return what it says on the tin, ie. the number of
channels that are actually connected.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
This fixes missing `static` function specifier.
The bt_att_chan_create_pdu is not called outside of att.c.
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
If the remote responds with and security related error the stack tries
to increase the security level to satisfy the remote permissions.
This fixes missing ATT timer reset on security related ATT Error
Response as the ATT operation is considered as complete.
< ACL Data TX: Handle 0 flags 0x00 dlen 7
ATT: Read Request (0x0a) len 2
Handle: 0x0084
TMAS: Role
> ACL Data RX: Handle 0 flags 0x02 dlen 9
ATT: Error Response (0x01) len 4
Read Request (0x0a)
Handle: 0x0084
Error: Insufficient Authentication (0x05)
TMAS: Role
Error code: 0x05
< ACL Data TX: Handle 0 flags 0x00 dlen 6
SMP: Security Request (0x0b) len 1
Authentication requirement: Bonding, No MITM, SC, No Keypresses
= bt: bt_att: ATT Timeout
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
The ATT_PENDING_SENT flag was still not being cleared in all cases.
Also reset `data->att_chan` when not able to send on a given channel.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Refactoring. All occurences of `atomic_test_bit((.*)->flags,
ATT_ENHANCED)` are replaced with `bt_att_is_enhanced($1)`.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
The EATT MTU in Zephyr is static. We know it at channel creation time,
so we should communicate the MTU as part of channel creation.
Side note: With this approach, it should no longer be neccessary or
useful to do a channel reconfigure.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
The previous approach with `cap_eatt` was flawed. It would overwrite
`le_chan->tx.mtu`, losing its true value. (It is supposed to be the
L2CAP MTU as reported by the remote side.)
The previous approach worked out for UATT because the locally triggered
exchange always carries the remote MTU in the response, so we did not
need to keep track of the remote MTU.
But, unlike the UATT MTU exchange, the L2CAP reconfigure only exchanges
the MTU in one direction. If the remote does the first reconfigure, we
would correctly cap the ATT MTU to our local MTU. But, we would
incorrectly store this as the remote's MTU. When we then increase our
local MTU using `bt_eatt_reconfigure`, we correctly set and send our
MTU. But we have an incorrect notion that the remote MTU is the value
that we ourselves limited. And mistake would incorrectly limit the
negotiated ATT MTU locally.
The simplest solution is to not mess with `le_chan->tx.mtu` and just
calculate the ATT MTU like Spec intended.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
This is a refactor. There is no behavioral change because
`BT_LOCAL_ATT_MTU_UATT` is `BT_LOCAL_ATT_MTU_EATT + 2` is
`BT_ATT_BUF_SIZE` is the old `BT_ATT_MTU`.
The old `BT_ATT_MTU` was wrong for EATT bearers. EATT MTU is two bytes
less because of ECRED overhead.
Instead of the old `BT_ATT_MTU`, we define one for each bearer type. We
also define the max of them to use as a convenience for allocating
buffers that fit either.
To avoid confusion, 'LOCAL' has been added to the name. This is to
differentiate it from what the spec calls 'ATT MTU', which is a
negotiated property. (It is the minimum of the two side's local ATT
MTU.)
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
This is a refactoring, only visible inside `att.c`.
Give the expression `chan->chan.tx.mtu` the name `bt_att_mtu`. Use it
when the intention is to get the ATT MTU property of a channel.
This is done in preparation a more complex expression for `bt_att_mtu`,
since the expression currently incorrect. The fix will come in a later
commit in the same PR.
Signed-off-by: Aleksander Wasaznik <aleksander.wasaznik@nordicsemi.no>
In the case of a constrained system that uses dynamic channels (ie, not
much initial credits), the user's log would be flooded with those three
messages.
This is not really an error per se as the stack will handle it properly and
reschedule the sending of the buffers as soon as more credits arrive.
Thus downgrading the severity of
- the l2cap messages, as they are only compiled when dynamic channels are
enabled
- the conn message. Reporting the error belongs in the upper layers.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
`ATT_PENDING_SENT` wasn't cleared when L2CAP reported an error when sending
the packet. This resulted in the channel being unusable for ever, since we
only clear that bit on a response (that will never be sent).
Found when setting `CONFIG_SYS_CLOCK_TICKS_PER_SEC=32768` in the
`notify_multiple` test. The kicker was the bug didn't manifest when EATT
wasn't enabled:
- we were queuing two unsubscribes back to back in the test
- on UATT we have to wait for a req-rsp pair before enqueuing a new one
- on UATT we only have one channel anyways
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
Clear the `ATT_CONNECTED` flag when a channel is detached (could be after
an ATT timeout).
Also convert the assert checking it in `chan send` to an `if` test, since
the channel could be disconnected from a different thread than the one
triggering `chan_send`.
Fixes#53247.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
The macro BT_CONN_INTERVAL_TO_MS was used a fair amount
of places, but it often was used with integers. This meant
that sometimes the resulting (integer) value would be
incorrect, as something like 7.5ms interval would not
be properly stored as a integer in millisecond units.
Adding BT_CONN_INTERVAL_TO_US allows users to still use
integers to store the result, but in a more accurate unit.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
This fixes deadlock that happened waiting for meta data in system
workqueue.
The meta data always get freed in the system workqueue,
so if we're in the same workqueue but there are no immediate
contexts available, there's no chance we'll get one by waiting.
Fixes: #53455
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
The `bluetooth/common/log.h` and `bluetooth/common/log.c` files have been
removed. Files that were using them have been updated to use
`zephyr/logging/log.h` instead.
Those replacement have been done consequently:
- `/BT_DBG/LOG_DBG/`
- `/BT_ERR/LOG_ERR/`
- `/BT_WARN/LOG_WRN/`
- `/BT_INFO/LOG_INF/`
- `/BT_HEXDUMP_DBG/LOG_HEXDUMP_DBG/`
- `/BT_DBG_OBJ_ID/LOG_DBG_OBJ_ID/`
Also, some files were relying on the `common/log.h` include to include
`zephyr/bluetooth/hci.h`, in those cases the include of `hci.h` has
been added.
For files that were including `common/log.h` but not using any logs,
the include has been removed and not replaced.
Signed-off-by: Théo Battrel <theo.battrel@nordicsemi.no>
bt_eatt_init currently registers a l2cap server, however should not
register a new one if it has already been registered - for example in
bt_enable previously.
Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
In bt_att_init, bt_conn_init and bt_l2cap_init, init the fifos and
slists before use. This means that they can be used after disable
without memory leakage.
Signed-off-by: Sean Madigan <sean.madigan@nordicsemi.no>
Remove the `bt_hex`, `bt_addr_str`, `bt_addr_le_str` and `bt_uuid_str`
macros from `bt_str.h` as they were just aliases for the `*_real` functions
in the same file.
The functions has been renamed without the `_real` suffix.
Some files were using the functions and not the macros, they have been
updated to use the new name of the functions.
Signed-off-by: Théo Battrel <theo.battrel@nordicsemi.no>
Functions related to string manipulation that were defined in
`common/log.h` has been moved to the `common/bt_str.h` file and their
implementation in `common/bt_str.c`.
Files that were using those functions has been updated consequently.
Signed-off-by: Théo Battrel <theo.battrel@nordicsemi.no>
- Downgrade the warning emitted when we run out of channels.
- Only log a warning when failing to establish channels and the error is
not -ENOMEM.
- Add a debug log message in that case to hint what might've happened.
This should make it less confusing to the average user not aware of the
stack internals.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
If a device sends an MTU that is bigger than our maximum tx buffer size,
that could cause assertion failures down the line.
This PR limits it to the maximum we support (CONFIG_BT_L2CAP_TX_MTU).
The issue has been observed with a gatt discovery procedure, but is likely
present in other places in att.c.
To reproduce it, we need two zephyr shell devices, with one having a larger
MTU than the other:
- connect
- do data length update to the bigger MTU
- set security to 2, EATT channels get connected
- launch a gatt discovery from the device with the larger MTU
- observe kernel panic on the other device when it attempts to add too much
memory to a net buf.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
As of today <zephyr/zephyr.h> is 100% equivalent to <zephyr/kernel.h>.
This patch proposes to then include <zephyr/kernel.h> instead of
<zephyr/zephyr.h> since it is more clear that you are including the
Kernel APIs and (probably) nothing else. <zephyr/zephyr.h> sounds like a
catch-all header that may be confusing. Most applications need to
include a bunch of other things to compile, e.g. driver headers or
subsystem headers like BT, logging, etc.
The idea of a catch-all header in Zephyr is probably not feasible
anyway. Reason is that Zephyr is not a library, like it could be for
example `libpython`. Zephyr provides many utilities nowadays: a kernel,
drivers, subsystems, etc and things will likely grow. A catch-all header
would be massive, difficult to keep up-to-date. It is also likely that
an application will only build a small subset. Note that subsystem-level
headers may use a catch-all approach to make things easier, though.
NOTE: This patch is **NOT** removing the header, just removing its usage
in-tree. I'd advocate for its deprecation (add a #warning on it), but I
understand many people will have concerns.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
When sending an ATT packet, do not just try to send the first packet in
the queue, but find the first one that can be sent using the channel.
This makes sure that eg. packets that shall be sent on an unenhanced ATT
channel are not blocked by packets that shall be sent on enhanced ATT
channels.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
bt_eatt_connect was documented to return -EINVAL if conn is NULL, but
this was not the case. Instead it lead to undefined behavior.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
Disallow trying to send requests or notifications on only EATT channels
if no EATT channels are connected or the link is not encrypted. In these
cases the operation will always fail, so it should not be queued.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
A channel option field has been added to the bt_gatt_*_params structs.
This allows the application to choose wether Unenhanced ATT, Enhanced
ATT, or Any ATT channel shall be used for the
request/command/indication/notification.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
This API now becomes a low-level API, in the sense that it should only be
used if the app developer knows what he is doing and wants to ensure a
ATT_MULTIPLE_HANDLE_VALUE_NTF PDU goes on-air.
For the other 99% of use cases, `bt_gatt_notify` should instead be used, as
it will automatically upgrade to ATT_MULTIPLE_HANDLE_VALUE_NTF when
possible.
One can disable the batching of notifications when using `bt_gatt_notify`
by setting CONFIG_BT_GATT_NOTIFY_MULTIPLE_FLUSH_MS=0 .
This API doesn't support lookup by UUID any more.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
In the case a call to `bt_gatt_notify_cb` gets its attribute data
batched with others to form an ATT_MULTIPLE_HANDLE_VALUE_NTF PDU, the
application developer would still expect one callback per API call.
Signed-off-by: Jonathan Rico <jonathan.rico@nordicsemi.no>
This reverts commit f3444ce00b.
The check is not needed anymore, as the EATT channels are available on
encrypted link only.
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
Core Vol 3, Part G, Section 5.3.2 Channel Requirements states that
"The channel shall be encrypted". It does not mention any additional
security requirements that can be specified bt higher layer profiles.
This enables link encryption requirement for EATT channel.
Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
When the client receives att error rsp, but the error code
is an illegal value, such as 0, an exception will be triggered.
gatt_read_type --> gatt_read_type_rsp --> parse_characteristic
`switch (rsp->len) {` null address access.
Signed-off-by: Lingao Meng <menglingao@xiaomi.com>
The automatic elevation of security and retry of ATT requests interferes
with some tests that expect authentication failures.
Affecting GATT/CL/GAR/BI-42-C
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
EATT is not a requirement for the Multiple Variable Length Read
procedure, but previously one had to enable CONFIG_BT_EATT to enable
support for it.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
The API is documented as being blocking. Making it nonblocking was an
unintentional API change.
Signed-off-by: Herman Berget <herman.berget@nordicsemi.no>
The L2CAP channel section is sorted lexicographically. Make sure
that ATT fixed channel will be placed as the last one to ensure
that SMP channel is properly initialized before bt_att_connected
tries to send security request.
Fixes#45820
Signed-off-by: Marek Pieta <Marek.Pieta@nordicsemi.no>
