Bluetooth: L2CAP: Prepend SDU header immediately

Previously it was not always possible to prepend the header.

It was not possible if the application neglected to reserve the space
for headers.  This is bad because it forces a buffer segment allocation
even if the buffer had enough room for the headers. E.g. a payload of 10
bytes in a netbuf of 30 bytes would have been segmented.

We now explicitly reject the buffer if it does not have the headroom.

This allows us to do a nice thing; simplify L2CAP segmentation.

We convert the SDU from the application into a PDU payload, by
prepending the SDU header, i.e. the SDU length in the original buffer.

This PDU payload is ready to be chunked into PDUs without having to keep
track of where in the SDU we are. This has the effect of removing a
bunch of logic in the segmentation machine.

Signed-off-by: Jonathan Rico <[email protected]>
Signed-off-by: Aleksander Wasaznik <[email protected]>

doc/releases/migration-guide-3.6.rst

@@ -245,6 +245,9 @@ Bluetooth
   parameter when segmenting SDUs into PDUs. In order to reproduce the previous behavior, the
   application should register the `alloc_seg` channel callback and allocate from the same pool as
   `buf`.
+* The :c:func:`bt_l2cap_chan_send` API now requires the application to reserve
+  enough bytes for the L2CAP headers. Call ``net_buf_reserve(buf,
+  BT_L2CAP_SDU_CHAN_SEND_RESERVE);`` at buffer allocation time to do so.
 
 * Mesh
 

include/zephyr/bluetooth/l2cap.h

@@ -579,14 +579,12 @@ int bt_l2cap_chan_disconnect(struct bt_l2cap_chan *chan);
  *  size the buffers for the for the outgoing buffer pool.
  *
  *  When sending L2CAP data over an LE connection the application is sending
- *  L2CAP SDUs. The application can optionally reserve
+ *  L2CAP SDUs. The application shall reserve
  *  @ref BT_L2CAP_SDU_CHAN_SEND_RESERVE bytes in the buffer before sending.
- *  By reserving bytes in the buffer the stack can use this buffer as a segment
- *  directly, otherwise it will have to allocate a new segment for the first
- *  segment.
- *  If the application is reserving the bytes it should use the
- *  BT_L2CAP_BUF_SIZE() helper to correctly size the buffers for the for the
- *  outgoing buffer pool.
+ *
+ *  The application can use the BT_L2CAP_SDU_BUF_SIZE() helper to correctly size
+ *  the buffer to account for the reserved headroom.
+ *
  *  When segmenting an L2CAP SDU into L2CAP PDUs the stack will first attempt to
  *  allocate buffers from the channel's `alloc_seg` callback and will fallback
  *  on the stack's global buffer pool (sized

subsys/bluetooth/host/l2cap.c

@@ -2067,29 +2067,11 @@ static int l2cap_chan_le_send_sdu(struct bt_l2cap_le_chan *ch,
 
 	total_len = net_buf_frags_len(*buf) + sent;
 
-	if (total_len > ch->tx.mtu) {
-		return -EMSGSIZE;
-	}
-
 	frag = *buf;
 	if (!frag->len && frag->frags) {
 		frag = frag->frags;
 	}
 
-	if (!sent) {
-		/* Add SDU length for the first segment */
-		ret = l2cap_chan_le_send(ch, frag, BT_L2CAP_SDU_HDR_SIZE);
-		if (ret < 0) {
-			if (ret == -EAGAIN) {
-				/* Store sent data into user_data */
-				l2cap_tx_meta_data(frag)->sent = sent;
-			}
-			*buf = frag;
-			return ret;
-		}
-		sent = ret;
-	}
-
 	/* Send remaining segments */
 	for (ret = 0; sent < total_len; sent += ret) {
 		/* Proceed to next fragment */
@@ -3139,6 +3121,7 @@ int bt_l2cap_chan_send(struct bt_l2cap_chan *chan, struct net_buf *buf)
 	struct bt_l2cap_le_chan *le_chan = BT_L2CAP_LE_CHAN(chan);
 	struct l2cap_tx_meta_data *data;
 	void *old_user_data = l2cap_tx_meta_data(buf);
+	uint16_t sdu_len;
 	int err;
 
 	if (!buf) {
@@ -3160,12 +3143,48 @@ int bt_l2cap_chan_send(struct bt_l2cap_chan *chan, struct net_buf *buf)
 		return bt_l2cap_br_chan_send_cb(chan, buf, NULL, NULL);
 	}
 
+	sdu_len = net_buf_frags_len(buf);
+
+	if (sdu_len > le_chan->tx.mtu) {
+		return -EMSGSIZE;
+	}
+
+	if (net_buf_headroom(buf) < BT_L2CAP_SDU_CHAN_SEND_RESERVE) {
+		/* Call `net_buf_reserve(buf, BT_L2CAP_SDU_CHAN_SEND_RESERVE)`
+		 * when allocating buffers intended for bt_l2cap_chan_send().
+		 */
+		LOG_DBG("Not enough headroom in buf %p", buf);
+		return -EINVAL;
+	}
+
 	data = alloc_tx_meta_data();
 	if (!data) {
 		LOG_WRN("Unable to allocate TX context");
 		return -ENOBUFS;
 	}
 
+	/* Prepend SDU "header".
+	 *
+	 * L2CAP LE CoC SDUs are segmented into PDUs and sent over so-called
+	 * K-frames that each have their own L2CAP header (ie channel, PDU
+	 * length).
+	 *
+	 * The SDU header is right before the data that will be segmented and is
+	 * only present in the first segment/PDU. Here's an example:
+	 *
+	 * Sent data payload of 50 bytes over channel 0x4040 with MPS of 30 bytes:
+	 * First PDU / segment / K-frame:
+	 * | L2CAP K-frame header        | K-frame payload                 |
+	 * | PDU length  | Channel ID    | SDU header   | SDU payload      |
+	 * | 30          | 0x4040        | 50           | 28 bytes of data |
+	 *
+	 * Second and last PDU / segment / K-frame:
+	 * | L2CAP K-frame header        | K-frame payload     |
+	 * | PDU length  | Channel ID    | rest of SDU payload |
+	 * | 22          | 0x4040        | 22 bytes of data    |
+	 */
+	net_buf_push_le16(buf, sdu_len);
+
 	data->sent = 0;
 	data->cid = le_chan->tx.cid;
 	data->cb = NULL;

tests/bsim/bluetooth/host/l2cap/credits/src/main.c

@@ -23,7 +23,8 @@ CREATE_FLAG(flag_l2cap_connected);
 #define L2CAP_MTU       (2 * SDU_LEN)
 
 /* Only one SDU transmitted or received at a time */
-NET_BUF_POOL_DEFINE(sdu_pool, 1, L2CAP_MTU, CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
+NET_BUF_POOL_DEFINE(sdu_pool, 1, BT_L2CAP_SDU_BUF_SIZE(L2CAP_MTU),
+		    CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
 
 static uint8_t tx_data[SDU_LEN];
 static uint16_t rx_cnt;

tests/bsim/bluetooth/host/l2cap/credits_seg_recv/src/main.c

@@ -21,7 +21,8 @@ CREATE_FLAG(flag_l2cap_connected);
 #define L2CAP_MTU (2 * SDU_LEN)
 
 /* Only one SDU transmitted or received at a time */
-NET_BUF_POOL_DEFINE(sdu_pool, 1, L2CAP_MTU, CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
+NET_BUF_POOL_DEFINE(sdu_pool, 1, BT_L2CAP_SDU_BUF_SIZE(L2CAP_MTU),
+		    CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
 
 static uint8_t tx_data[SDU_LEN];
 static uint16_t rx_cnt;

tests/bsim/bluetooth/host/l2cap/general/src/main_l2cap_ecred.c

@@ -33,9 +33,9 @@ static const struct bt_data ad[] = {
 #define SHORT_MSG_CHAN_IDX 1
 
 NET_BUF_POOL_FIXED_DEFINE(rx_data_pool, L2CAP_CHANNELS, BT_L2CAP_BUF_SIZE(DATA_BUF_SIZE), 8, NULL);
-NET_BUF_POOL_FIXED_DEFINE(tx_data_pool_0, 1, BT_L2CAP_BUF_SIZE(DATA_MTU),
+NET_BUF_POOL_FIXED_DEFINE(tx_data_pool_0, 1, BT_L2CAP_SDU_BUF_SIZE(DATA_MTU),
 			  CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
-NET_BUF_POOL_FIXED_DEFINE(tx_data_pool_1, 1, BT_L2CAP_BUF_SIZE(DATA_MTU),
+NET_BUF_POOL_FIXED_DEFINE(tx_data_pool_1, 1, BT_L2CAP_SDU_BUF_SIZE(DATA_MTU),
 			  CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);
 
 static struct bt_l2cap_server servers[SERVERS];
@@ -379,7 +379,7 @@ static void send_sdu(int iteration, int chan_idx, int bytes)
 	}
 
 	channels[chan_idx].buf = buf;
-	net_buf_reserve(buf, BT_L2CAP_CHAN_SEND_RESERVE);
+	net_buf_reserve(buf, BT_L2CAP_SDU_CHAN_SEND_RESERVE);
 	net_buf_add_mem(buf, channels[chan_idx].payload, bytes);
 
 	LOG_DBG("bt_l2cap_chan_sending ch: %i bytes: %i iteration: %i", chan_idx, bytes, iteration);