[nrf fromlist] Bluetooth: Host: L2CAP: Add explicit MPS parameter to …

…reconfigure API

Add explicit MPS parameter to `bt_l2cap_ecred_chan_reconfigure`. This is
the only straightforward way to reconfigure MPS channels that use
`seg_recv`, where the application is in control of the MPS.

The implementation of `bt_l2cap_ecred_chan_reconfigure` before this
patch essentially had an implicit MPS parameter equal to `MIN(mtu +
BT_L2CAP_SDU_HDR_SIZE, BT_L2CAP_RX_MTU)`. This choice maximizes
throughput within the constrains of the Host implementation.

To upgrade without change, applications must simply use that expression
as the new argument, the the benefit that the choice of MPS is now
explicit.

I am not aware of any real-world uses of
`bt_l2cap_ecred_chan_reconfigure`. I believe we can impose the small
inconvenience of a search-replace without going through a deprecation
cycle.

For non-`seg_recv` channels, an explicit MPS parameter is useful if
the application wants to ensure a small MPS. This might be useful for
minimizing the impact this channel has on other channels on the same
connection.

Upstream PR #: 80883

Signed-off-by: Lang Xie <lang.xie@nordicsemi.no>

include/zephyr/bluetooth/l2cap.h

@@ -547,10 +547,11 @@ int bt_l2cap_ecred_chan_connect(struct bt_conn *conn,
  *  @param chans Array of channel objects. Null-terminated. Elements after the
  *               first 5 are silently ignored.
  *  @param mtu Channel MTU to reconfigure to.
+ *  @param mps Channel MPS to reconfigure to.
  *
  *  @return 0 in case of success or negative value in case of error.
  */
-int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu);
+int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu, uint16_t mps);
 
 /** @brief Connect L2CAP channel
  *

subsys/bluetooth/host/att.c

@@ -3773,6 +3773,7 @@ int bt_eatt_reconfigure(struct bt_conn *conn, uint16_t mtu)
 	struct bt_l2cap_chan *chans[CONFIG_BT_EATT_MAX + 1] = {};
 	size_t offset = 0;
 	size_t i = 0;
+	uint16_t mps;
 	int err;
 
 	SYS_SLIST_FOR_EACH_CONTAINER(&att->chans, att_chan, node) {
@@ -3783,10 +3784,12 @@ int bt_eatt_reconfigure(struct bt_conn *conn, uint16_t mtu)
 	}
 
 	while (offset < i) {
+		mps = MIN(mtu + BT_L2CAP_SDU_HDR_SIZE, BT_L2CAP_RX_MTU);
+
 		/* bt_l2cap_ecred_chan_reconfigure() uses the first L2CAP_ECRED_CHAN_MAX_PER_REQ
 		 * elements of the array or until a null-terminator is reached.
 		 */
-		err = bt_l2cap_ecred_chan_reconfigure(&chans[offset], mtu);
+		err = bt_l2cap_ecred_chan_reconfigure(&chans[offset], mtu, mps);
 		if (err < 0) {
 			return err;
 		}

subsys/bluetooth/host/l2cap.c

@@ -2944,12 +2944,13 @@ static struct bt_l2cap_le_chan *l2cap_find_pending_reconf(struct bt_conn *conn)
 	return NULL;
 }
 
-int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu)
+int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu, uint16_t mps)
 {
 	struct bt_l2cap_ecred_reconf_req *req;
 	struct bt_conn *conn = NULL;
 	struct bt_l2cap_le_chan *ch;
 	struct net_buf *buf;
+	bool multiple_chan;
 	uint8_t ident;
 	int i;
 
@@ -2959,6 +2960,16 @@ int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu)
 		return -EINVAL;
 	}
 
+	if (chans[0] == NULL) {
+		return -EINVAL;
+	}
+
+	if (mps > BT_L2CAP_RX_MTU) {
+		return -EINVAL;
+	}
+
+	multiple_chan = chans[1] != NULL;
+
 	for (i = 0; i < L2CAP_ECRED_CHAN_MAX_PER_REQ; i++) {
 		if (!chans[i]) {
 			break;
@@ -2977,10 +2988,13 @@ int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu)
 		if (mtu < BT_L2CAP_LE_CHAN(chans[i])->rx.mtu) {
 			return -EINVAL;
 		}
-	}
 
-	if (i == 0) {
-		return -EINVAL;
+		/* MPS is not allowed to decrease when reconfiguring multiple channels.
+		 * Core Specification 3.A.4.27 v6.0
+		 */
+		if (multiple_chan && mps < BT_L2CAP_LE_CHAN(chans[i])->rx.mps) {
+			return -EINVAL;
+		}
 	}
 
 	if (!conn) {
@@ -3007,12 +3021,7 @@ int bt_l2cap_ecred_chan_reconfigure(struct bt_l2cap_chan **chans, uint16_t mtu)
 
 	req = net_buf_add(buf, sizeof(*req));
 	req->mtu = sys_cpu_to_le16(mtu);
-
-	/* MPS shall not be bigger than MTU + BT_L2CAP_SDU_HDR_SIZE
-	 * as the remaining bytes cannot be used.
-	 */
-	req->mps = sys_cpu_to_le16(MIN(mtu + BT_L2CAP_SDU_HDR_SIZE,
-				       BT_L2CAP_RX_MTU));
+	req->mps = sys_cpu_to_le16(mps);
 
 	for (int j = 0; j < i; j++) {
 		ch = BT_L2CAP_LE_CHAN(chans[j]);

subsys/bluetooth/shell/l2cap.c

@@ -282,6 +282,7 @@ static int cmd_ecred_reconfigure(const struct shell *sh, size_t argc, char *argv
 {
 	struct bt_l2cap_chan *l2cap_ecred_chans[] = { &l2ch_chan.ch.chan, NULL };
 	uint16_t mtu;
+	uint16_t mps;
 	int err = 0;
 
 	if (!default_conn) {
@@ -301,7 +302,8 @@ static int cmd_ecred_reconfigure(const struct shell *sh, size_t argc, char *argv
 		return -ENOEXEC;
 	}
 
-	err = bt_l2cap_ecred_chan_reconfigure(l2cap_ecred_chans, mtu);
+	mps = MIN(mtu + BT_L2CAP_SDU_HDR_SIZE, BT_L2CAP_RX_MTU);
+	err = bt_l2cap_ecred_chan_reconfigure(l2cap_ecred_chans, mtu, mps);
 	if (err < 0) {
 		shell_error(sh, "Unable to reconfigure channel (err %d)", err);
 	} else {

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

@@ -426,12 +426,13 @@ static void send_sdu_concurrently(void)
 static int change_mtu_on_channels(int num_channels, int new_mtu)
 {
 	struct bt_l2cap_chan *reconf_channels[ECRED_CHAN_MAX] = { NULL };
+	uint16_t mps = MIN(new_mtu + BT_L2CAP_SDU_HDR_SIZE, BT_L2CAP_RX_MTU);
 
 	for (int i = 0; i < num_channels; i++) {
 		reconf_channels[i] = &(&channels[i])->le.chan;
 	}
 
-	return bt_l2cap_ecred_chan_reconfigure(reconf_channels, new_mtu);
+	return bt_l2cap_ecred_chan_reconfigure(reconf_channels, new_mtu, mps);
 }
 
 static void test_peripheral_main(void)