From ea04fd95f939b8115358560df5887bb5f2baeab8 Mon Sep 17 00:00:00 2001 From: Jonathan Rico Date: Wed, 29 Nov 2023 13:14:31 +0100 Subject: [PATCH] Bluetooth: ATT: remove `BT_ATT_ENFORCE_FLOW` Enforcing the peer's behavior is not strictly necessary. All the host should do is make sure it is resilient to a spec-violating peer. Moreover, a growing number of platforms were disabling the check, as the spec allows "batching" HCI num complete packets events, stalling ATT RX. Signed-off-by: Jonathan Rico Co-authored-by: Aleksander Wasaznik --- drivers/bluetooth/hci/Kconfig.infineon | 5 ----- samples/bluetooth/peripheral_ht/prj.conf | 1 - subsys/bluetooth/host/Kconfig.gatt | 11 ----------- subsys/bluetooth/host/att.c | 23 ----------------------- 4 files changed, 40 deletions(-) diff --git a/drivers/bluetooth/hci/Kconfig.infineon b/drivers/bluetooth/hci/Kconfig.infineon index 1f4193bd28a..6204ed4f8a2 100644 --- a/drivers/bluetooth/hci/Kconfig.infineon +++ b/drivers/bluetooth/hci/Kconfig.infineon @@ -129,11 +129,6 @@ config AIROC_CUSTOM_FIRMWARE_HCD_BLOB config BT_BUF_CMD_TX_SIZE default 255 -# Disable ATT_ENFORCE_FLOW feature, CYW43XX informs about frees buffer -# (HCL Number Of Completed Packets event) after second packet. -config BT_ATT_ENFORCE_FLOW - default n - endif # BT_AIROC if BT_PSOC6_BLESS diff --git a/samples/bluetooth/peripheral_ht/prj.conf b/samples/bluetooth/peripheral_ht/prj.conf index d25259ec660..b8a3da4f5b5 100644 --- a/samples/bluetooth/peripheral_ht/prj.conf +++ b/samples/bluetooth/peripheral_ht/prj.conf @@ -7,5 +7,4 @@ CONFIG_BT_DIS_PNP=n CONFIG_BT_BAS=y CONFIG_BT_DEVICE_NAME="Zephyr Health Thermometer" CONFIG_BT_DEVICE_APPEARANCE=768 -CONFIG_BT_ATT_ENFORCE_FLOW=n CONFIG_CBPRINTF_FP_SUPPORT=y diff --git a/subsys/bluetooth/host/Kconfig.gatt b/subsys/bluetooth/host/Kconfig.gatt index 9209368f943..53b482a0a38 100644 --- a/subsys/bluetooth/host/Kconfig.gatt +++ b/subsys/bluetooth/host/Kconfig.gatt @@ -5,17 +5,6 @@ menu "ATT and GATT Options" -config BT_ATT_ENFORCE_FLOW - bool "Enforce strict flow control semantics for incoming PDUs" - default y if !(BOARD_QEMU_CORTEX_M3 || BOARD_QEMU_X86 || ARCH_POSIX || BT_SPI_BLUENRG) - help - Enforce flow control rules on incoming PDUs, preventing a peer - from sending new requests until a previous one has been responded - or sending a new indication until a previous one has been - confirmed. This may need to be disabled to avoid potential race - conditions arising from a USB based HCI transport that splits - HCI events and ACL data to separate endpoints. - config BT_ATT_PREPARE_COUNT int "Number of ATT prepare write buffers" default 0 diff --git a/subsys/bluetooth/host/att.c b/subsys/bluetooth/host/att.c index 70fc717aff6..2b5206bcd0f 100644 --- a/subsys/bluetooth/host/att.c +++ b/subsys/bluetooth/host/att.c @@ -72,8 +72,6 @@ K_MEM_SLAB_DEFINE(req_slab, sizeof(struct bt_att_req), CONFIG_BT_L2CAP_TX_BUF_COUNT, __alignof__(struct bt_att_req)); enum { - ATT_PENDING_RSP, - ATT_PENDING_CFM, ATT_CONNECTED, ATT_ENHANCED, ATT_PENDING_SENT, @@ -540,10 +538,6 @@ static void chan_cfm_sent(struct bt_conn *conn, struct bt_att_tx_meta_data *user LOG_DBG("chan %p", chan); - if (IS_ENABLED(CONFIG_BT_ATT_ENFORCE_FLOW)) { - atomic_clear_bit(chan->flags, ATT_PENDING_CFM); - } - tx_meta_data_free(data); } @@ -554,10 +548,6 @@ static void chan_rsp_sent(struct bt_conn *conn, struct bt_att_tx_meta_data *user LOG_DBG("chan %p", chan); - if (IS_ENABLED(CONFIG_BT_ATT_ENFORCE_FLOW)) { - atomic_clear_bit(chan->flags, ATT_PENDING_RSP); - } - tx_meta_data_free(data); } @@ -2908,19 +2898,6 @@ static int bt_att_recv(struct bt_l2cap_chan *chan, struct net_buf *buf) return 0; } - if (IS_ENABLED(CONFIG_BT_ATT_ENFORCE_FLOW)) { - if (handler->type == ATT_REQUEST && - atomic_test_and_set_bit(att_chan->flags, ATT_PENDING_RSP)) { - LOG_WRN("Ignoring unexpected request"); - return 0; - } else if (handler->type == ATT_INDICATION && - atomic_test_and_set_bit(att_chan->flags, - ATT_PENDING_CFM)) { - LOG_WRN("Ignoring unexpected indication"); - return 0; - } - } - if (buf->len < handler->expect_len) { LOG_ERR("Invalid len %u for code 0x%02x", buf->len, hdr->code); err = BT_ATT_ERR_INVALID_PDU;