Hi Paul,
On Tue, Aug 12, 2025 at 4:03 PM Paul Menzel <pmenzel@xxxxxxxxxxxxx> wrote:
>
> Dear Luiz,
>
>
> Thank you for your patch.
>
> 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?
> > 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
--
Luiz Augusto von Dentz
