Skip to content

Commit

Permalink
extmod/nimble: Add "memory stalling" mechanism for l2cap_send.
Browse files Browse the repository at this point in the history
When l2cap_send detects that the sys mempool is running low (used to store
the outgoing HCI payloads), it will report stalled back to the application,
and then only unstall once these HCI payloads have been sent.

This prevents a situation where a remote receiver with very large MTU can
cause NimBLE to queue up more than MYNEWT_VAL_MSYS_1_BLOCK_COUNT (i.e. 12)
payloads, causing further attempts to send to fail with ENOMEM (even though
the channel is not stalled and we have room in the channel mbufs). The
regular credit/stall flow control is not effective here because the
receiver's MTU is large enough that it will not activate (i.e. there are
lots of credits available).

Thresholds of 1/2 (stall) and 1/4 (unstall) chosen to allow headroom for
other payloads (e.g. notifications) and that when a regular stall occurs it
might keep sending (and creating more payloads) in the background.
  • Loading branch information
jimmo authored and dpgeorge committed Jul 23, 2021
1 parent edfb5d5 commit 341158c
Showing 1 changed file with 42 additions and 6 deletions.
48 changes: 42 additions & 6 deletions extmod/nimble/modbluetooth_nimble.c
Expand Up @@ -1392,7 +1392,16 @@ int mp_bluetooth_gattc_exchange_mtu(uint16_t conn_handle) {

#endif // MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT

#if MICROPY_PY_BLUETOOTH_ENABLE_L2CAP_CHANNELS
STATIC void unstall_l2cap_channel(void);
#endif

void mp_bluetooth_nimble_sent_hci_packet(void) {
#if MICROPY_PY_BLUETOOTH_ENABLE_L2CAP_CHANNELS
if (os_msys_num_free() >= os_msys_count() * 3 / 4) {
unstall_l2cap_channel();
}
#endif
}

#if MICROPY_PY_BLUETOOTH_ENABLE_L2CAP_CHANNELS
Expand All @@ -1416,6 +1425,7 @@ typedef struct _mp_bluetooth_nimble_l2cap_channel_t {
struct os_mempool sdu_mempool;
struct os_mbuf *rx_pending;
bool irq_in_progress;
bool mem_stalled;
uint16_t mtu;
os_membuf_t sdu_mem[];
} mp_bluetooth_nimble_l2cap_channel_t;
Expand All @@ -1433,6 +1443,19 @@ STATIC void destroy_l2cap_channel() {
}
}

STATIC void unstall_l2cap_channel(void) {
// Whenever we send an HCI packet and the sys mempool is now less than 1/4 full,
// we can unstall the L2CAP channel if it was marked as "mem_stalled" by
// mp_bluetooth_l2cap_send. (This happens if the pool is half-empty).
mp_bluetooth_nimble_l2cap_channel_t *chan = MP_STATE_PORT(bluetooth_nimble_root_pointers)->l2cap_chan;
if (!chan || !chan->mem_stalled) {
return;
}
DEBUG_printf("unstall_l2cap_channel: count %d, free: %d\n", os_msys_count(), os_msys_num_free());
chan->mem_stalled = false;
mp_bluetooth_on_l2cap_send_ready(chan->chan->conn_handle, chan->chan->scid, 0);
}

STATIC int l2cap_channel_event(struct ble_l2cap_event *event, void *arg) {
DEBUG_printf("l2cap_channel_event: type=%d\n", event->type);
mp_bluetooth_nimble_l2cap_channel_t *chan = (mp_bluetooth_nimble_l2cap_channel_t *)arg;
Expand Down Expand Up @@ -1528,9 +1551,13 @@ STATIC int l2cap_channel_event(struct ble_l2cap_event *event, void *arg) {
}
case BLE_L2CAP_EVENT_COC_TX_UNSTALLED: {
DEBUG_printf("l2cap_channel_event: tx_unstalled: conn_handle=%d status=%d\n", event->tx_unstalled.conn_handle, event->tx_unstalled.status);
ble_l2cap_get_chan_info(event->receive.chan, &info);
// Map status to {0,1} (i.e. "sent everything", or "partial send").
mp_bluetooth_on_l2cap_send_ready(event->tx_unstalled.conn_handle, info.scid, event->tx_unstalled.status == 0 ? 0 : 1);
assert(event->tx_unstalled.conn_handle == chan->chan->conn_handle);
// Don't unstall if we're still waiting for room in the sys pool.
if (!chan->mem_stalled) {
ble_l2cap_get_chan_info(event->receive.chan, &info);
// Map status to {0,1} (i.e. "sent everything", or "partial send").
mp_bluetooth_on_l2cap_send_ready(event->tx_unstalled.conn_handle, info.scid, event->tx_unstalled.status == 0 ? 0 : 1);
}
break;
}
case BLE_L2CAP_EVENT_COC_RECONFIG_COMPLETED: {
Expand Down Expand Up @@ -1678,26 +1705,35 @@ int mp_bluetooth_l2cap_send(uint16_t conn_handle, uint16_t cid, const uint8_t *b
return MP_ENOMEM;
}

*stalled = false;

err = ble_l2cap_send(chan->chan, sdu_tx);
if (err == BLE_HS_ESTALLED) {
// Stalled means that this one will still send but any future ones
// will fail until we receive an unstalled event.
DEBUG_printf("mp_bluetooth_l2cap_send: credit stall\n");
*stalled = true;
err = 0;
} else {
if (err) {
// Anything except stalled means it won't attempt to send,
// so free the mbuf (we're failing the op entirely).
os_mbuf_free_chain(sdu_tx);
} else {
*stalled = false;
}
}

if (os_msys_num_free() <= os_msys_count() / 2) {
// If the sys mempool is less than half-full, then back off sending more
// on this channel.
DEBUG_printf("mp_bluetooth_l2cap_send: forcing mem stall: count %d, free: %d\n", os_msys_count(), os_msys_num_free());
chan->mem_stalled = true;
*stalled = true;
}

// Sometimes we see what looks like BLE_HS_EAGAIN (but it's actually
// OS_ENOMEM in disguise). Fixed in NimBLE v1.4.
if (err == OS_ENOMEM) {
return MP_ENOMEM;
err = BLE_HS_ENOMEM;
}

// Other error codes such as BLE_HS_EBUSY (we're stalled) or BLE_HS_EBADDATA (bigger than MTU).
Expand Down

0 comments on commit 341158c

Please sign in to comment.