Hi,
ti, 2024-05-28 kello 13:38 +0800, Ying Hsu kirjoitti:
> We just found Rx ACL data packets on the INTEL_STP2_AC7265 BT
> controller are using connection handle value >= 0x900 (e.g.
> 3585=0xe01):
> ```
> > ISO Data RX: Handle 3585 flags 0x02 dlen 16 #536 [hci0] 2024-05-28 00:41:23.779341
> ```
>
> To mitigate potential issues, we can limit the patch to verified
> models like AX211. What do you think?
How is this verified, is there a data sheet that says these are the
handle ranges? Will it be changed by later firmware updates
(unlikely?)?
>
> On Fri, May 24, 2024 at 12:50 PM Ying Hsu <yinghsu@xxxxxxxxxxxx> wrote:
> >
> > When HCI raw sockets are opened, the Bluetooth kernel module doesn't
> > track CIS/BIS connections. User-space applications have to identify
> > ISO data by maintaining connection information and look up the mapping
> > for each ACL data packet received. Besides, btsnoop log captured in
> > kernel couldn't tell ISO data from ACL data in this case.
> >
> > To avoid additional lookups, this patch introduces vendor-specific
> > packet classification for Intel BT controllers to distinguish
> > ISO data packets from ACL data packets.
> >
> > Signed-off-by: Ying Hsu <yinghsu@xxxxxxxxxxxx>
> > ---
> > Tested LE audio unicast recording on a ChromeOS device with Intel AX211
> >
> > Changes in v3:
> > - Move Intel's classify_pkt_type implementation from btusb.c to btintel.c.
> >
> > Changes in v2:
> > - Adds vendor-specific packet classificaton in hci_dev.
> > - Keeps reclassification in hci_recv_frame.
> >
> > drivers/bluetooth/btintel.c | 19 +++++++++++++++++++
> > drivers/bluetooth/btintel.h | 6 ++++++
> > drivers/bluetooth/btusb.c | 1 +
> > include/net/bluetooth/hci_core.h | 1 +
> > net/bluetooth/hci_core.c | 16 ++++++++++++++++
> > 5 files changed, 43 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index 27e03951e68b..bf1bd2b13c96 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -3187,6 +3187,25 @@ void btintel_secure_send_result(struct hci_dev *hdev,
> > }
> > EXPORT_SYMBOL_GPL(btintel_secure_send_result);
> >
> > +#define BTINTEL_ISODATA_HANDLE_BASE 0x900
> > +
> > +u8 btintel_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + /*
> > + * Distinguish ISO data packets form ACL data packets
> > + * based on their connection handle value range.
> > + */
> > + if (hci_skb_pkt_type(skb) == HCI_ACLDATA_PKT) {
> > + __u16 handle = __le16_to_cpu(hci_acl_hdr(skb)->handle);
> > +
> > + if (hci_handle(handle) >= BTINTEL_ISODATA_HANDLE_BASE)
> > + return HCI_ISODATA_PKT;
> > + }
> > +
> > + return hci_skb_pkt_type(skb);
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_classify_pkt_type);
> > +
> > MODULE_AUTHOR("Marcel Holtmann <marcel@xxxxxxxxxxxx>");
> > MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> > MODULE_VERSION(VERSION);
> > diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> > index 9dbad1a7c47c..4b77eb8d47a8 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -245,6 +245,7 @@ int btintel_bootloader_setup_tlv(struct hci_dev *hdev,
> > int btintel_shutdown_combined(struct hci_dev *hdev);
> > void btintel_hw_error(struct hci_dev *hdev, u8 code);
> > void btintel_print_fseq_info(struct hci_dev *hdev);
> > +u8 btintel_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb);
> > #else
> >
> > static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> > @@ -378,4 +379,9 @@ static inline void btintel_hw_error(struct hci_dev *hdev, u8 code)
> > static inline void btintel_print_fseq_info(struct hci_dev *hdev)
> > {
> > }
> > +
> > +static inline u8 btintel_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + return hci_skb_pkt_type(skb);
> > +}
> > #endif
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 79aefdb3324d..2ecc6d1140a5 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -4451,6 +4451,7 @@ static int btusb_probe(struct usb_interface *intf,
> > /* Transport specific configuration */
> > hdev->send = btusb_send_frame_intel;
> > hdev->cmd_timeout = btusb_intel_cmd_timeout;
> > + hdev->classify_pkt_type = btintel_classify_pkt_type;
> >
> > if (id->driver_info & BTUSB_INTEL_NO_WBS_SUPPORT)
> > btintel_set_flag(hdev, INTEL_ROM_LEGACY_NO_WBS_SUPPORT);
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index 9231396fe96f..7b7068a84ff7 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -649,6 +649,7 @@ struct hci_dev {
> > int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
> > struct bt_codec *codec, __u8 *vnd_len,
> > __u8 **vnd_data);
> > + u8 (*classify_pkt_type)(struct hci_dev *hdev, struct sk_buff *skb);
> > };
> >
> > #define HCI_PHY_HANDLE(handle) (handle & 0xff)
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index b3ee9ff17624..8b817a99cefd 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2941,15 +2941,31 @@ int hci_reset_dev(struct hci_dev *hdev)
> > }
> > EXPORT_SYMBOL(hci_reset_dev);
> >
> > +static u8 hci_dev_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
> > +{
> > + if (hdev->classify_pkt_type)
> > + return hdev->classify_pkt_type(hdev, skb);
> > +
> > + return hci_skb_pkt_type(skb);
> > +}
> > +
> > /* Receive frame from HCI drivers */
> > int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
> > {
> > + u8 dev_pkt_type;
> > +
> > if (!hdev || (!test_bit(HCI_UP, &hdev->flags)
> > && !test_bit(HCI_INIT, &hdev->flags))) {
> > kfree_skb(skb);
> > return -ENXIO;
> > }
> >
> > + /* Check if the driver agree with packet type classification */
> > + dev_pkt_type = hci_dev_classify_pkt_type(hdev, skb);
> > + if (hci_skb_pkt_type(skb) != dev_pkt_type) {
> > + hci_skb_pkt_type(skb) = dev_pkt_type;
> > + }
> > +
> > switch (hci_skb_pkt_type(skb)) {
> > case HCI_EVENT_PKT:
> > break;
> > --
> > 2.45.1.288.g0e0cd299f1-goog
> >
--
Pauli Virtanen
