linux-kernel - Re: [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABBYNZ+HzCDakR18naDA1dw-PwGn_F-r7yCK+EXUodmtKmawhg@mail.gmail.com>
Date: Mon, 30 Jun 2025 09:49:39 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Pavel Shpakovskiy <shpakovskiip@...il.com>
Cc: marcel@...tmann.org, johan.hedberg@...il.com, davem@...emloft.net, 
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, horms@...nel.org, 
	linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, kernel@...utedevices.com
Subject: Re: [PATCH v1] Bluetooth: L2CAP: Introduce minimum limit of
 rx_credits value

Hi Pavel,

On Mon, Jun 30, 2025 at 3:57 AM Pavel Shpakovskiy
<shpakovskiip@...il.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@...il.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