extmod/nimble: Add "memory stalling" mechanism for l2cap_send.

jimmo · dpgeorge · commit 341158c251c0 · 2021-07-23T21:58:10.000+10:00
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.
diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c
@@ -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
@@ -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;
@@ -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;
@@ -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: {
@@ -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).
