extmod/nimble: Improve the flow control for l2cap recv path.
If the _IRQ_L2CAP_RECV handler does the actual consumption of the incoming data (i.e. via l2cap_recvinto), rather than setting a flag for non-scheduler-context to handle it later, then two things can happen: - It can starve the VM (i.e. the scheduled task never terminates). This is because calling l2cap_recvinto will empty the rx buffer, which will grant more credits to the channel (an HCI command), meaning more data can arrive. This means that the loop in hal_uart.c that keeps reading HCI data from the uart and executing NimBLE events as they are created will not terminate, preventing other VM code from running. - There's no flow control (i.e. data will arrive too quickly). The channel shouldn't be given credits until after we return from scheduler context. It's preferable that no work is done in scheduler/IRQ context. But to prevent this being a problem this commit changes l2cap_recvinto so that if it is called in IRQ context, and the Python handler empties the rx buffer, then don't grant credits until the Python handler is complete. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
This commit is contained in:
parent
0f9a9129da
commit
47d02b3104
@ -1355,6 +1355,7 @@ typedef struct _mp_bluetooth_nimble_l2cap_channel_t {
|
|||||||
struct os_mbuf_pool sdu_mbuf_pool;
|
struct os_mbuf_pool sdu_mbuf_pool;
|
||||||
struct os_mempool sdu_mempool;
|
struct os_mempool sdu_mempool;
|
||||||
struct os_mbuf *rx_pending;
|
struct os_mbuf *rx_pending;
|
||||||
|
bool irq_in_progress;
|
||||||
uint16_t mtu;
|
uint16_t mtu;
|
||||||
os_membuf_t sdu_mem[];
|
os_membuf_t sdu_mem[];
|
||||||
} mp_bluetooth_nimble_l2cap_channel_t;
|
} mp_bluetooth_nimble_l2cap_channel_t;
|
||||||
@ -1441,7 +1442,23 @@ STATIC int l2cap_channel_event(struct ble_l2cap_event *event, void *arg) {
|
|||||||
chan->chan->coc_rx.sdu = sdu_rx;
|
chan->chan->coc_rx.sdu = sdu_rx;
|
||||||
|
|
||||||
ble_l2cap_get_chan_info(event->receive.chan, &info);
|
ble_l2cap_get_chan_info(event->receive.chan, &info);
|
||||||
|
|
||||||
|
// Don't allow granting more credits until after the IRQ is handled.
|
||||||
|
chan->irq_in_progress = true;
|
||||||
|
|
||||||
mp_bluetooth_gattc_on_l2cap_recv(event->receive.conn_handle, info.scid);
|
mp_bluetooth_gattc_on_l2cap_recv(event->receive.conn_handle, info.scid);
|
||||||
|
chan->irq_in_progress = false;
|
||||||
|
|
||||||
|
// If all data has been consumed by the IRQ handler, then now allow
|
||||||
|
// more credits. If the IRQ handler doesn't consume all available data
|
||||||
|
// then rx_pending will be still set.
|
||||||
|
if (!chan->rx_pending) {
|
||||||
|
struct os_mbuf *sdu_rx = chan->chan->coc_rx.sdu;
|
||||||
|
assert(sdu_rx);
|
||||||
|
if (sdu_rx) {
|
||||||
|
ble_l2cap_recv_ready(chan->chan, sdu_rx);
|
||||||
|
}
|
||||||
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
case BLE_L2CAP_EVENT_COC_TX_UNSTALLED: {
|
case BLE_L2CAP_EVENT_COC_TX_UNSTALLED: {
|
||||||
@ -1508,6 +1525,7 @@ STATIC int create_l2cap_channel(uint16_t mtu, mp_bluetooth_nimble_l2cap_channel_
|
|||||||
|
|
||||||
chan->mtu = mtu;
|
chan->mtu = mtu;
|
||||||
chan->rx_pending = NULL;
|
chan->rx_pending = NULL;
|
||||||
|
chan->irq_in_progress = false;
|
||||||
|
|
||||||
int err = os_mempool_init(&chan->sdu_mempool, buf_blocks, L2CAP_BUF_BLOCK_SIZE, chan->sdu_mem, "l2cap_sdu_pool");
|
int err = os_mempool_init(&chan->sdu_mempool, buf_blocks, L2CAP_BUF_BLOCK_SIZE, chan->sdu_mem, "l2cap_sdu_pool");
|
||||||
if (err != 0) {
|
if (err != 0) {
|
||||||
@ -1635,13 +1653,17 @@ int mp_bluetooth_l2cap_recvinto(uint16_t conn_handle, uint16_t cid, uint8_t *buf
|
|||||||
os_mbuf_free_chain(chan->rx_pending);
|
os_mbuf_free_chain(chan->rx_pending);
|
||||||
chan->rx_pending = NULL;
|
chan->rx_pending = NULL;
|
||||||
|
|
||||||
// We've already given the channel a new mbuf in l2cap_channel_event above, so
|
// If we're in the call stack of the l2cap_channel_event handler, then don't
|
||||||
// re-use that mbuf in the call to ble_l2cap_recv_ready. This will just
|
// re-enable receiving yet (as we need to complete the rest of IRQ handler first).
|
||||||
// give the channel more credits.
|
if (!chan->irq_in_progress) {
|
||||||
struct os_mbuf *sdu_rx = chan->chan->coc_rx.sdu;
|
// We've already given the channel a new mbuf in l2cap_channel_event above, so
|
||||||
assert(sdu_rx);
|
// re-use that mbuf in the call to ble_l2cap_recv_ready. This will just
|
||||||
if (sdu_rx) {
|
// give the channel more credits.
|
||||||
ble_l2cap_recv_ready(chan->chan, sdu_rx);
|
struct os_mbuf *sdu_rx = chan->chan->coc_rx.sdu;
|
||||||
|
assert(sdu_rx);
|
||||||
|
if (sdu_rx) {
|
||||||
|
ble_l2cap_recv_ready(chan->chan, sdu_rx);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Trim the used bytes from the start of the mbuf.
|
// Trim the used bytes from the start of the mbuf.
|
||||||
|
Loading…
Reference in New Issue
Block a user