Re: [PATCH v2 3/3] Bluetooth: hci_conn: Make unacked packet handling more robust

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dear Luiz,


Am 12.08.25 um 22:29 schrieb Luiz Augusto von Dentz:

On Tue, Aug 12, 2025 at 4:03 PM Paul Menzel <pmenzel@xxxxxxxxxxxxx> wrote:

Am 12.08.25 um 21:53 schrieb Luiz Augusto von Dentz:
From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>

This attempts to make unacked packet handling more robust by detecting
if there are no connections left then restore all buffers of the
respective pool.

Did you find this in code review, or was a bug reported?

Internal testing has run into a problem where some packets are never
TX for some reason, we are still accessing if this is going to help
but it is not an actual fix and more of a way to improve the handling
of unacked packets.

Now I'm curious, why this sort of question seem to be recurring, we do
encourage as much details as possible, but I don't think we ever said
that it is required to disclose what sort of test or bug report it
came from, especially if they come from internal testing which cannot
be referenced anyway, so was there anything unclear with the commit
message?

Kind of:

1. “attempt” sounds to me like, that you tried to fix something, but are not actually sure.

2. In the end, the devices need to work, so knowing how to verify a fix – myself for example – is always helpful in my opinion, so I often ask for reproducers and tests.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
---
   net/bluetooth/hci_conn.c | 34 ++++++++++++++++++++++++++++------
   1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index e6cea51b3eab..4bd2e4cd477f 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1152,22 +1152,44 @@ void hci_conn_del(struct hci_conn *conn)
       disable_delayed_work_sync(&conn->auto_accept_work);
       disable_delayed_work_sync(&conn->idle_work);

-     /* Handle unnacked frames */
+     /* Handle unnacked frames:
+      *
+      * - In case there are no connection restore all buffers to the pool
+      * - Otherwise restore just the buffers considered in transit for the
+      *   hci_conn
+      */
       switch (conn->type) {
       case ACL_LINK:
-             hdev->acl_cnt += conn->sent;
+             if (!hci_conn_num(hdev, ACL_LINK))
+                     hdev->acl_cnt = hdev->acl_pkts;
+             else
+                     hdev->acl_cnt += conn->sent;
               break;
       case LE_LINK:
               cancel_delayed_work(&conn->le_conn_timeout);

-             if (hdev->le_pkts)
-                     hdev->le_cnt += conn->sent;
-             else
-                     hdev->acl_cnt += conn->sent;
+             if (hdev->le_pkts) {
+                     if (!hci_conn_num(hdev, LE_LINK))
+                             hdev->le_cnt = hdev->le_pkts;
+                     else
+                             hdev->le_cnt += conn->sent;
+             } else {
+                     if (!hci_conn_num(hdev, LE_LINK) &&
+                         !hci_conn_num(hdev, ACL_LINK))
+                             hdev->acl_cnt = hdev->acl_pkts;
+                     else
+                             hdev->acl_cnt += conn->sent;
+             }
               break;
       case CIS_LINK:
       case BIS_LINK:
       case PA_LINK:
+             if (!hci_conn_num(hdev, CIS_LINK) &&
+                 !hci_conn_num(hdev, BIS_LINK) &&
+                 !hci_conn_num(hdev, PA_LINK))
+                     hdev->iso_cnt = hdev->iso_pkts;
+             else
+                     hdev->iso_cnt += conn->sent;
               hdev->iso_cnt += conn->sent;
               break;
       }

Kind regards,

Paul




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux