Dear Ying,
Thank you for your patch. Some minor comments.
Could you please make the summary/title a statement by using a verb (in
imperative mood).
Am 22.05.24 um 13:11 schrieb Ying Hsu:
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 catpured in
captured
kernel would couldn't tell ISO data from ACL data in this case.
Excessive *would*?
By differentiating ISO data from ACL data earlier in btusb, we can
eliminate unnecessary lookups and improve btsnoop log clarity.
This patch enables vendor-specific differentiation between ISO and
ACL data packets.
Make it clear, that this is only done for Intel devices?
Signed-off-by: Ying Hsu <yinghsu@xxxxxxxxxxxx>
---
Tested LE unicast recording on a ChromeOS device with Intel AX211
I’d add that to the commit message, and also name the exact device name
you used.
drivers/bluetooth/btusb.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 79aefdb3324d..543961b6c671 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -889,6 +889,8 @@ struct btusb_data {
int (*recv_acl)(struct hci_dev *hdev, struct sk_buff *skb);
int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
+ int (*is_iso_data_pkt)(struct sk_buff *skb);
+
int (*setup_on_usb)(struct hci_dev *hdev);
int oob_wake_irq; /* irq for out-of-band wake-on-bt */
@@ -1229,6 +1231,8 @@ static int btusb_recv_bulk(struct btusb_data *data, void *buffer, int count)
if (!hci_skb_expect(skb)) {
/* Complete frame */
+ if (data->is_iso_data_pkt && data->is_iso_data_pkt(skb))
+ hci_skb_pkt_type(skb) = HCI_ISODATA_PKT;
btusb_recv_acl(data, skb);
skb = NULL;
}
@@ -2539,6 +2543,13 @@ static int btusb_recv_bulk_intel(struct btusb_data *data, void *buffer,
return btusb_recv_bulk(data, buffer, count);
}
+static int btusb_is_iso_data_pkt_intel(struct sk_buff *skb)
Why not return `bool`?
+{
+ __u16 handle = (__le16_to_cpu(hci_acl_hdr(skb)->handle) & 0xfff);
+
+ return (handle >= 0x900);
Define a macro?
+}
+
static int btusb_send_frame_intel(struct hci_dev *hdev, struct sk_buff *skb)
{
struct urb *urb;
@@ -4361,6 +4372,9 @@ static int btusb_probe(struct usb_interface *intf,
/* Override the rx handlers */
data->recv_event = btintel_recv_event;
data->recv_bulk = btusb_recv_bulk_intel;
+
+ /* Override for ISO data packet*/
Please add a space before */.
+ data->is_iso_data_pkt = btusb_is_iso_data_pkt_intel;
} else if (id->driver_info & BTUSB_REALTEK) {
/* Allocate extra space for Realtek device */
priv_size += sizeof(struct btrealtek_data);