public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value
@ 2025-06-30  7:56 Pavel Shpakovskiy
  2025-06-30  8:41 ` [v1] " bluez.test.bot
  2025-06-30 13:49 ` [PATCH v1] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 3+ messages in thread
From: Pavel Shpakovskiy @ 2025-06-30  7:56 UTC (permalink / raw)
  To: marcel, johan.hedberg, luiz.dentz, davem, edumazet, kuba, pabeni,
	horms
  Cc: linux-bluetooth, netdev, linux-kernel, kernel, Pavel Shpakovskiy

The commit 96cd8eaa131f 
("Bluetooth: L2CAP: Derive rx credits from MTU and MPS") 
removed the static rx_credits setup to improve BLE packet
communication for high MTU values. However, due to vendor-specific 
issues in the Bluetooth module firmware, using low MTU values 
(especially less than 256 bytes) results in dynamically calculated
rx_credits being too low, causing slow speeds and occasional BLE 
connection failures.

This change aims to improve BLE connection stability and speed 
for low MTU values. It is possible to tune minimum value 
of rx credits with debugfs handle.

Signed-off-by: Pavel Shpakovskiy <shpakovskiip@gmail.com>
---
 include/net/bluetooth/l2cap.h |  2 ++
 net/bluetooth/l2cap_core.c    | 17 +++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 4bb0eaedda180..8648d9324a654 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -437,6 +437,8 @@ struct l2cap_conn_param_update_rsp {
 #define L2CAP_CONN_PARAM_ACCEPTED	0x0000
 #define L2CAP_CONN_PARAM_REJECTED	0x0001
 
+#define L2CAP_LE_MIN_CREDITS		10
+
 struct l2cap_le_conn_req {
 	__le16     psm;
 	__le16     scid;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c88f69dde995e..392d7ba0f0737 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -50,6 +50,8 @@ static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN | L2CAP_FEAT_UCD;
 static LIST_HEAD(chan_list);
 static DEFINE_RWLOCK(chan_list_lock);
 
+static u16 le_min_credits = L2CAP_LE_MIN_CREDITS;
+
 static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
 				       u8 code, u8 ident, u16 dlen, void *data);
 static void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len,
@@ -547,8 +549,17 @@ static __u16 l2cap_le_rx_credits(struct l2cap_chan *chan)
 	/* If we don't know the available space in the receiver buffer, give
 	 * enough credits for a full packet.
 	 */
-	if (chan->rx_avail == -1)
-		return (chan->imtu / chan->mps) + 1;
+	if (chan->rx_avail == -1) {
+		u16 rx_credits = (chan->imtu / chan->mps) + 1;
+
+		if (rx_credits < le_min_credits) {
+			rx_credits = le_min_credits;
+			BT_DBG("chan %p: set rx_credits to minimum value: %u",
+			       chan, chan->rx_credits);
+		}
+
+		return rx_credits;
+	}
 
 	/* If we know how much space is available in the receive buffer, give
 	 * out as many credits as would fill the buffer.
@@ -7661,6 +7672,8 @@ int __init l2cap_init(void)
 	l2cap_debugfs = debugfs_create_file("l2cap", 0444, bt_debugfs,
 					    NULL, &l2cap_debugfs_fops);
 
+	debugfs_create_u16("l2cap_le_min_credits", 0644, bt_debugfs,
+			   &le_min_credits);
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value
  2025-06-30  7:56 [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value Pavel Shpakovskiy
@ 2025-06-30  8:41 ` bluez.test.bot
  2025-06-30 13:49 ` [PATCH v1] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2025-06-30  8:41 UTC (permalink / raw)
  To: linux-bluetooth, shpakovskiip

[-- Attachment #1: Type: text/plain, Size: 3024 bytes --]

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=977196

---Test result---

Test Summary:
CheckPatch                    PENDING   0.30 seconds
GitLint                       PENDING   0.24 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      24.68 seconds
CheckAllWarning               PASS      26.93 seconds
CheckSparse                   PASS      30.34 seconds
BuildKernel32                 PASS      24.09 seconds
TestRunnerSetup               PASS      471.10 seconds
TestRunner_l2cap-tester       FAIL      25.49 seconds
TestRunner_iso-tester         PASS      36.43 seconds
TestRunner_bnep-tester        PASS      5.94 seconds
TestRunner_mgmt-tester        FAIL      131.35 seconds
TestRunner_rfcomm-tester      PASS      9.33 seconds
TestRunner_sco-tester         PASS      14.79 seconds
TestRunner_ioctl-tester       PASS      10.06 seconds
TestRunner_mesh-tester        FAIL      9.37 seconds
TestRunner_smp-tester         PASS      8.50 seconds
TestRunner_userchan-tester    PASS      6.22 seconds
IncrementalBuild              PENDING   0.81 seconds

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

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

##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
Total: 68, Passed: 64 (94.1%), Failed: 4, Not Run: 0

Failed Test Cases
L2CAP LE Server - Success                            Failed       0.112 seconds
L2CAP Ext-Flowctl Server - Success                   Failed       0.132 seconds
L2CAP LE EATT Server - Success                       Failed       0.142 seconds
L2CAP LE EATT Server - Reject                        Failed       0.133 seconds
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4

Failed Test Cases
LL Privacy - Add Device 2 (2 Devices to AL)          Failed       0.189 seconds
LL Privacy - Set Flags 3 (2 Devices to RL)           Failed       0.201 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
BUG: KASAN: slab-use-after-free in run_timer_softirq+0x76b/0x7d0
WARNING: CPU: 0 PID: 66 at kernel/workqueue.c:2257 __queue_work+0x93e/0xba0
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Failed       0.144 seconds
Mesh - Send cancel - 2                               Timed out    2.014 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value
  2025-06-30  7:56 [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value Pavel Shpakovskiy
  2025-06-30  8:41 ` [v1] " bluez.test.bot
@ 2025-06-30 13:49 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2025-06-30 13:49 UTC (permalink / raw)
  To: Pavel Shpakovskiy
  Cc: marcel, johan.hedberg, davem, edumazet, kuba, pabeni, horms,
	linux-bluetooth, netdev, linux-kernel, kernel

Hi Pavel,

On Mon, Jun 30, 2025 at 3:57 AM Pavel Shpakovskiy
<shpakovskiip@gmail.com> wrote:
>
> The commit 96cd8eaa131f
> ("Bluetooth: L2CAP: Derive rx credits from MTU and MPS")
> removed the static rx_credits setup to improve BLE packet
> communication for high MTU values. However, due to vendor-specific
> issues in the Bluetooth module firmware, using low MTU values
> (especially less than 256 bytes) results in dynamically calculated
> rx_credits being too low, causing slow speeds and occasional BLE
> connection failures.

You will have to be more specific here, what is the use case and model
that doesn't work depending on the number of credits? If the idea is
to just disable flow control to allow the remote side to pipe more
data then the MTU that sort of defeats the purpose of using CoC, but
maybe the use case requires or the remote side is too slow to process
the updates of credits?

> This change aims to improve BLE connection stability and speed
> for low MTU values. It is possible to tune minimum value
> of rx credits with debugfs handle.
>
> Signed-off-by: Pavel Shpakovskiy <shpakovskiip@gmail.com>
> ---
>  include/net/bluetooth/l2cap.h |  2 ++
>  net/bluetooth/l2cap_core.c    | 17 +++++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 4bb0eaedda180..8648d9324a654 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -437,6 +437,8 @@ struct l2cap_conn_param_update_rsp {
>  #define L2CAP_CONN_PARAM_ACCEPTED      0x0000
>  #define L2CAP_CONN_PARAM_REJECTED      0x0001
>
> +#define L2CAP_LE_MIN_CREDITS           10
> +
>  struct l2cap_le_conn_req {
>         __le16     psm;
>         __le16     scid;
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index c88f69dde995e..392d7ba0f0737 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -50,6 +50,8 @@ static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN | L2CAP_FEAT_UCD;
>  static LIST_HEAD(chan_list);
>  static DEFINE_RWLOCK(chan_list_lock);
>
> +static u16 le_min_credits = L2CAP_LE_MIN_CREDITS;
> +
>  static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
>                                        u8 code, u8 ident, u16 dlen, void *data);
>  static void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len,
> @@ -547,8 +549,17 @@ static __u16 l2cap_le_rx_credits(struct l2cap_chan *chan)
>         /* If we don't know the available space in the receiver buffer, give
>          * enough credits for a full packet.
>          */
> -       if (chan->rx_avail == -1)
> -               return (chan->imtu / chan->mps) + 1;
> +       if (chan->rx_avail == -1) {
> +               u16 rx_credits = (chan->imtu / chan->mps) + 1;
> +
> +               if (rx_credits < le_min_credits) {
> +                       rx_credits = le_min_credits;
> +                       BT_DBG("chan %p: set rx_credits to minimum value: %u",
> +                              chan, chan->rx_credits);

This doesn't make much sense in my opinion, if we want to disable flow
control then we shall allow the remote to pipe as many packets without
waiting for more credits, note though rx_credits handling changes
after receiving the first packet then the credits are updated based on
the socket receiving buffer:

https://github.com/bluez/bluetooth-next/commit/ce60b9231b66710b6ee24042ded26efee120ecfc

So perhaps it is the socket receiving buffer that needs to be
adjusted, which is something the process has control over.

> +               }
> +
> +               return rx_credits;
> +       }
>
>         /* If we know how much space is available in the receive buffer, give
>          * out as many credits as would fill the buffer.
> @@ -7661,6 +7672,8 @@ int __init l2cap_init(void)
>         l2cap_debugfs = debugfs_create_file("l2cap", 0444, bt_debugfs,
>                                             NULL, &l2cap_debugfs_fops);
>
> +       debugfs_create_u16("l2cap_le_min_credits", 0644, bt_debugfs,
> +                          &le_min_credits);
>         return 0;
>  }
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-30 13:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-30  7:56 [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx_credits value Pavel Shpakovskiy
2025-06-30  8:41 ` [v1] " bluez.test.bot
2025-06-30 13:49 ` [PATCH v1] " Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox