diff mbox series

[BlueZ,v2,1/2] audio: actually try to enable MTU auto-tuning

Message ID 20250129084950.41987-1-pchelkin@ispras.ru (mailing list archive)
State New
Headers show
Series [BlueZ,v2,1/2] audio: actually try to enable MTU auto-tuning | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Fedor Pchelkin Jan. 29, 2025, 8:49 a.m. UTC
A "0" for the input MTU passed to the underlying socket is supposed to
indicate that its value should be determined by the L2CAP layer.
However, the current code treats a zero imtu just as if there is
nothing to change.

Introduce an additional flag to indicate that the zero imtu is
explicitly requested by the caller for the purpose of auto-tuning.
Otherwise, the similar behavior remains.

Found by Linux Verification Center (linuxtesting.org).

Fixes: ae5be371a9f5 ("avdtp: Enable MTU auto tunning")
---
v1->v2: incorporate error handling in case the kernel still doesn't
support MTU auto-tuning

 btio/btio.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

Comments

bluez.test.bot@gmail.com Jan. 29, 2025, 10:09 a.m. UTC | #1
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=929013

---Test result---

Test Summary:
CheckPatch                    PENDING   0.20 seconds
GitLint                       PENDING   0.23 seconds
BuildEll                      PASS      20.68 seconds
BluezMake                     PASS      1518.75 seconds
MakeCheck                     PASS      13.65 seconds
MakeDistcheck                 PASS      161.86 seconds
CheckValgrind                 PASS      215.75 seconds
CheckSmatch                   PASS      272.39 seconds
bluezmakeextell               PASS      98.65 seconds
IncrementalBuild              PENDING   0.30 seconds
ScanBuild                     PASS      866.92 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth
patchwork-bot+bluetooth@kernel.org Jan. 30, 2025, 5:30 p.m. UTC | #2
Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 29 Jan 2025 11:49:49 +0300 you wrote:
> A "0" for the input MTU passed to the underlying socket is supposed to
> indicate that its value should be determined by the L2CAP layer.
> However, the current code treats a zero imtu just as if there is
> nothing to change.
> 
> Introduce an additional flag to indicate that the zero imtu is
> explicitly requested by the caller for the purpose of auto-tuning.
> Otherwise, the similar behavior remains.
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,1/2] audio: actually try to enable MTU auto-tuning
    (no matching commit)
  - [BlueZ,v2,2/2] a2dp: enable input MTU auto-tuning for the server
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=252a32ac0b3f

You are awesome, thank you!
Luiz Augusto von Dentz Jan. 30, 2025, 5:51 p.m. UTC | #3
Hi Fedor,

On Thu, Jan 30, 2025 at 12:30 PM <patchwork-bot+bluetooth@kernel.org> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluez.git (master)
> by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
>
> On Wed, 29 Jan 2025 11:49:49 +0300 you wrote:
> > A "0" for the input MTU passed to the underlying socket is supposed to
> > indicate that its value should be determined by the L2CAP layer.
> > However, the current code treats a zero imtu just as if there is
> > nothing to change.
> >
> > Introduce an additional flag to indicate that the zero imtu is
> > explicitly requested by the caller for the purpose of auto-tuning.
> > Otherwise, the similar behavior remains.
> >
> > [...]
>
> Here is the summary with links:
>   - [BlueZ,v2,1/2] audio: actually try to enable MTU auto-tuning
>     (no matching commit)

Ive done quite a few modifications to the above change, so instead of
using a auto_mtu it now checks if -1 for not set, Ive checked with a
couple headsets I have and it seem to work fine but perhaps you want
to check as well just to make sure I didn't screw something up.

>   - [BlueZ,v2,2/2] a2dp: enable input MTU auto-tuning for the server
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=252a32ac0b3f
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
>
diff mbox series

Patch

diff --git a/btio/btio.c b/btio/btio.c
index 2d277e409..7b38bc819 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -66,6 +66,7 @@  struct set_opts {
 	uint16_t imtu;
 	uint16_t omtu;
 	int central;
+	uint8_t auto_mtu;
 	uint8_t mode;
 	int flushable;
 	uint32_t priority;
@@ -610,7 +611,7 @@  static uint8_t mode_l2mode(uint8_t mode)
 }
 
 static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
-						uint8_t mode, GError **err)
+				uint8_t auto_mtu, uint8_t mode, GError **err)
 {
 	struct l2cap_options l2o;
 	socklen_t len;
@@ -622,7 +623,7 @@  static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
 		return FALSE;
 	}
 
-	if (imtu)
+	if (imtu || auto_mtu)
 		l2o.imtu = imtu;
 	if (omtu)
 		l2o.omtu = omtu;
@@ -666,17 +667,24 @@  static gboolean set_le_mode(int sock, uint8_t mode, GError **err)
 }
 
 static gboolean l2cap_set(int sock, uint8_t src_type, int sec_level,
-				uint16_t imtu, uint16_t omtu, uint8_t mode,
-				int central, int flushable, uint32_t priority,
-				GError **err)
+				uint16_t imtu, uint16_t omtu, uint8_t auto_mtu,
+				uint8_t mode, int central, int flushable,
+				uint32_t priority, GError **err)
 {
-	if (imtu || omtu || mode) {
+	if (imtu || omtu || auto_mtu || mode) {
 		gboolean ret = FALSE;
 
-		if (src_type == BDADDR_BREDR)
-			ret = set_l2opts(sock, imtu, omtu, mode, err);
-		else {
-			if (imtu)
+		if (src_type == BDADDR_BREDR) {
+			ret = set_l2opts(sock, imtu, omtu, auto_mtu, mode, err);
+
+			/* Back to default behavior in case the first call fails:
+			 * it may happen if the used kernel still doesn't support
+			 * auto-tuning the MTU 
+			 */
+			if (!ret && auto_mtu)
+				ret = set_l2opts(sock, imtu, omtu, 0, mode, err);
+		} else {
+			if (imtu || auto_mtu)
 				ret = set_le_imtu(sock, imtu, err);
 
 			if (ret && mode)
@@ -986,6 +994,8 @@  static gboolean parse_set_opts(struct set_opts *opts, GError **err,
 			opts->imtu = va_arg(args, int);
 			if (!opts->mtu)
 				opts->mtu = opts->imtu;
+			if (!opts->imtu)
+				opts->auto_mtu = 1;
 			break;
 		case BT_IO_OPT_CENTRAL:
 			opts->central = va_arg(args, gboolean);
@@ -1890,7 +1900,7 @@  gboolean bt_io_set(GIOChannel *io, GError **err, BtIOOption opt1, ...)
 	switch (type) {
 	case BT_IO_L2CAP:
 		return l2cap_set(sock, opts.src_type, opts.sec_level, opts.imtu,
-					opts.omtu, opts.mode, opts.central,
+					opts.omtu, opts.auto_mtu, opts.mode, opts.central,
 					opts.flushable, opts.priority, err);
 	case BT_IO_RFCOMM:
 		return rfcomm_set(sock, opts.sec_level, opts.central, err);
@@ -1941,7 +1951,7 @@  static GIOChannel *create_io(gboolean server, struct set_opts *opts,
 				server ? opts->psm : 0, opts->cid, err) < 0)
 			goto failed;
 		if (!l2cap_set(sock, opts->src_type, opts->sec_level,
-				opts->imtu, opts->omtu, opts->mode,
+				opts->imtu, opts->omtu, opts->auto_mtu, opts->mode,
 				opts->central, opts->flushable, opts->priority,
 				err))
 			goto failed;