Re: pull request: bluetooth-next 2024-05-10

From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	davem@davemloft.net,  linux-bluetooth@vger.kernel.org,
	netdev@vger.kernel.org,  Pauli Virtanen <pav@iki.fi>
Subject: Re: pull request: bluetooth-next 2024-05-10
Date: Mon, 13 May 2024 18:09:31 -0400	[thread overview]
Message-ID: <CABBYNZKn5YBRjj+RT_TVDtjOBS6V_H7BQmFMufQj-cOTC=RXDA@mail.gmail.com> (raw)
In-Reply-To: <20240513142641.0d721b18@kernel.org>

Hi Jakub,

On Mon, May 13, 2024 at 5:26 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 10 May 2024 17:14:28 -0400 Luiz Augusto von Dentz wrote:
> > The following changes since commit f8beae078c82abde57fed4a5be0bbc3579b59ad0:
> >
> >   Merge tag 'gtp-24-05-07' of git://git.kernel.org/pub/scm/linux/kernel/git/pablo/gtp Pablo neira Ayuso says: (2024-05-10 13:59:27 +0100)
> >
> > are available in the Git repository at:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git tags/for-net-next-2024-05-10
> >
> > for you to fetch changes up to 75f819bdf9cafb0f1458e24c05d24eec17b2f597:
> >
> >   Bluetooth: btintel: Fix compiler warning for multi_v7_defconfig config (2024-05-10 17:04:15 -0400)
> >
> > ----------------------------------------------------------------
> > bluetooth-next pull request for net-next:
> >
> >  - Add support MediaTek MT7921S SDIO
> >  - Various fixes for -Wflex-array-member-not-at-end and -Wfamnae
> >  - Add USB HW IDs for MT7921/MT7922/MT7925
> >  - Add support for Intel BlazarI and Filmore Peak2 (BE201)
> >  - Add initial support for Intel PCIe driver
> >  - Remove HCI_AMP support
> >  - Add TX timestamping support
>
> There is one more warning in the Intel driver:
>
> drivers/bluetooth/btintel_pcie.c:673:33: warning: symbol 'causes_list'
> was not declared. Should it be static?

We have a fix for that but I was hoping to have it in before the merge
window and then have the fix merged later.

> It'd also be great to get an ACK from someone familiar with the socket
> time stamping (Willem?) I'm not sure there's sufficient detail in the
> commit message to explain the choices to:
>  - change the definition of SCHED / SEND to mean queued / completed,
>    while for Ethernet they mean queued to qdisc, queued to HW.

hmm I thought this was hardware specific, it obviously won't work
exactly as Ethernet since it is a completely different protocol stack,
or are you suggesting we need other definitions for things like TX
completed?

>    How does it compare to stamping in the driver in terms of accuracy?

@Pauli any input here?

>  - the "experimental" BT_POLL_ERRQUEUE, how does the user space look?

There are test cases in BlueZ:

https://github.com/bluez/bluez/commit/141f66411ca488e26bdd64e6f858ffa190395d23

>    What is the "upper layer"? What does it mean for kernel uAPI to be
>    "experimental"? When does the "upper layer" get to run and how does
>    it know that there are time stamps on the error queue?

The socketopt only gets enabled with use of MGMT Set Experimental
Feature Command:

https://github.com/bluez/bluez/blob/master/doc/mgmt-api.txt#L3205

Anyway you can see on the tests how we are using it.

> Would be great to get more info and/or second opinion, because it's not
> sufficiently "obviously right" to me to pull right away :(

Well I assumed sockopt starting with SO_ sort of means it applies that
all socket families, in fact SO_TIMESTAMP already seem to work without
these changes they just don't generate anything, so in a way we are
just implementing a missing feature.

-- 
Luiz Augusto von Dentz