L2CAP: static size of BLE_L2CAP_COC_MTU inefficient?!

[haukepetersen]

Lately, I was playing with some stress-tests for sending data over raw l2cap connection oriented channels, and I had a lot of trouble with overflowing buffers. It took me a while to understand NimBLE's internal buffering for l2cap, but I think I have it figured out...

However, when looking more closely into the code, I found that BLE_L2CAP_COC_MTU in ble_l2cap_coc_priv.h is statically set to 100. If I understand it correctly, this value is only used for the computation of the initial credits for a connection (in ble_l2cap_coc_chan_alloc()). These credits then define the maximum number of 'mbuf's, that are allocated in a row from the msys pool, when ble_l2cap_coc_continue_tx() is called.

Example:

• open a l2cap channel with a MTU of 5000 byte -> the channel will initial have 50 credits (5000 + (BLE_L2CAP_COC_MTU - 1)) / BLE_L2CAP_COC_MTU
• now I want to send a chunk 5000 byte through the channel:
  • put the 5000 byte into an application specific mbuf pool
  • hand the data to NimBLE using ble_l2cap_send()
  • ble_l2cap_coc_continue_tx() is called. This function will allocate 50 mbufs from the msys pool, putting BLE_L2CAP_COC_MTU -> 100 byte of user data into each mbuf

-> on RIOT, MYNEWT_VAL_MSYS_1_BLOCK_SIZE is per default set to 292. This leads to a lot of wasted memory, as each of the 50 mbufs allocated above uses only 100 of these 292 byes, leaving 50 * ~190 byte -> ~9.5kb of RAM unused

So is there any reason I missed for setting BLE_L2CAP_COC_MTU statically to 100? I would suggest to set it simply to the value of MYNEWT_VAL_MSYS_1_BLOCK_SIZE, possibly minus a little overhead. Would that make sense?

[rymanluk]

@haukepetersen I guess you have issue when testing against 2 devices using Nimble, is this correct?

Anyway, I don't know why it is 100 but note that L2CAP can take os_mbuf not only from MSYS_1.
I would do it as MYNEWT_VAL and set it by default to MSYS_1_BLOCK_SIZE - headers as you suggested.

Could you do such a patch?

[haukepetersen]

I guess you have issue when testing against 2 devices using Nimble, is this correct?

yes, 5-10 devices running Nimble :-)

Anyway, I don't know why it is 100 but note that L2CAP can take os_mbuf not only from MSYS_1.

But did I understand correctly, that the MSYS buffers are ordered by their block size, with MSYS_1 having the smallest block size? So that would mean that setting it at least to MSYS_1_BLOCK_SIZE would indeed make sense.

Could you do such a patch?

I can certainly for the RIOT port, but I am not sure if I sufficient enough with mynewt to include such a fix into the rest of the code tree. But I guess I can at least give it a try :-)

[haukepetersen]

So here is my try: #315