Stop BLE log streaming from starving the NimBLE notify pool (#11253)
sendLog() notified once per firmware log line with no backpressure. With debug_log_api_enabled and a subscribed client, a logging burst drains the msys_1 mbuf pool that every GATT notification and ATT response allocates from, so ble_gatts_notify_custom starts returning BLE_HS_ENOMEM (rc=6) -- for the fromNum doorbell too, not just the log line that exhausted it. Each failure then prints an error over serial at ~12ms apiece, which is itself enough to throttle the main loop during a burst. Register a callback on the log characteristic so the host's own ERROR_GATT verdict pauses log notifies for 250ms and lets the pool refill. Keying off the failure rather than a free-block count keeps this correct however the pools are sized. Also restore CONFIG_BT_NIMBLE_MSYS_1_BLOCK_COUNT to the IDF default of 12. Pool selection is by block size and a notify request always resolves to the smallest pool, so msys_1 is the only pool GATT draws from; 8 was thin once anything chains or fragments. msys_2 and the ACL/EVT transport pools are left trimmed -- neither is on the notify path, and restoring the full defaults would re-spend the contiguous allocation that #10741 was fixing on heap-tight boards. sendLog() also lacked the null check onNowHasData() already has, so a log line arriving during BLE teardown dereferenced a freed characteristic. Fixes #11245
This commit is contained in:
@@ -637,6 +637,21 @@ class NimbleBluetoothFromRadioCallback : public BLECharacteristicCallbacks
|
||||
}
|
||||
};
|
||||
|
||||
// One log notify per log line shares the msys_1 mbuf pool with the fromNum doorbell and ATT
|
||||
// responses, so a logging burst starves them; back off on rejection until the pool refills.
|
||||
static constexpr uint32_t LOG_NOTIFY_BACKOFF_MS = 250;
|
||||
static std::atomic<uint32_t> lastLogNotifyFailureMs{0};
|
||||
|
||||
class NimbleBluetoothLogRadioCallback : public BLECharacteristicCallbacks
|
||||
{
|
||||
void onStatus(BLECharacteristic *, Status s, uint32_t) override
|
||||
{
|
||||
// ERROR_GATT is the only status meaning the host refused it; the rest never allocated.
|
||||
if (s == Status::ERROR_GATT)
|
||||
lastLogNotifyFailureMs.store(millis());
|
||||
}
|
||||
};
|
||||
|
||||
class NimbleBluetoothSecurityCallback : public BLESecurityCallbacks
|
||||
{
|
||||
void onPassKeyNotify(uint32_t passkey) override
|
||||
@@ -1005,6 +1020,9 @@ void NimbleBluetooth::setupService()
|
||||
static NimbleBluetoothFromRadioCallback fromRadioCallbacks;
|
||||
FromRadioCharacteristic->setCallbacks(&fromRadioCallbacks);
|
||||
|
||||
static NimbleBluetoothLogRadioCallback logRadioCallbacks;
|
||||
logRadioCharacteristic->setCallbacks(&logRadioCallbacks);
|
||||
|
||||
bleService->start();
|
||||
|
||||
// Setup the battery service
|
||||
@@ -1056,6 +1074,11 @@ void NimbleBluetooth::sendLog(const uint8_t *logMessage, size_t length)
|
||||
if (!isConnected() || length > 512) {
|
||||
return;
|
||||
}
|
||||
if (!logRadioCharacteristic) // BLE may have been torn down; never notify a freed characteristic
|
||||
return;
|
||||
// Pool still under pressure; drop this line rather than spend a buffer fromNum needs.
|
||||
if (Throttle::isWithinTimespanMs(lastLogNotifyFailureMs.load(), LOG_NOTIFY_BACKOFF_MS))
|
||||
return;
|
||||
logRadioCharacteristic->setValue(logMessage, length);
|
||||
logRadioCharacteristic->notify();
|
||||
}
|
||||
|
||||
@@ -291,7 +291,9 @@ custom_sdkconfig =
|
||||
CONFIG_BT_CTRL_BLE_MAX_ACT=2
|
||||
CONFIG_BT_CTRL_SCAN_DUPL_CACHE_SIZE=10
|
||||
CONFIG_BT_NIMBLE_WHITELIST_SIZE=1
|
||||
CONFIG_BT_NIMBLE_MSYS_1_BLOCK_COUNT=8
|
||||
# msys_1 is the only pool GATT allocates from, and 8 blocks left no headroom for a notify burst
|
||||
# (#11245); back to the IDF default of 12 for +1KB. Raising msys_2 would not help.
|
||||
CONFIG_BT_NIMBLE_MSYS_1_BLOCK_COUNT=12
|
||||
CONFIG_BT_NIMBLE_MSYS_2_BLOCK_COUNT=8
|
||||
CONFIG_BT_NIMBLE_TRANSPORT_ACL_FROM_LL_COUNT=8
|
||||
CONFIG_BT_NIMBLE_TRANSPORT_EVT_COUNT=12
|
||||
|
||||
Reference in New Issue
Block a user