[PATCH RFC v2 0/3] Bluetooth: Add generic support for vendor HCI frames

[PATCH RFC v2 0/3] Bluetooth: Add generic support for vendor HCI frames

[Zijun Hu]

Hi Luiz
To address the below concerns you raised on previous
discussion.

- Not use safer skb helper like skb_pull_data()
- It will skip sending to the monitor, making debugging much harder

Looking forward to your further comments.

Previous discussion link:
https://lore.kernel.org/all/f9bbf92f-d617-468b-a5b4-2367a1d9a751@oss.qualcomm.com
https://lore.kernel.org/all/CABBYNZJpdeoZ16bObLRPhng2dfyNeK9ix9_-z_hMZhJ0QxTxGg@mail.gmail.com
https://lore.kernel.org/all/CABBYNZJ0zoVVVxuM48L=Km==gnrQuskrbjB6q_aNV0KEEY3+5w@mail.gmail.com

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Changes in v2:
- Implement vendor HCI frames using HCI_VENDOR_PKT (Option A)
- Add recv_vendor_pkt() hook for drivers to handle vendor frames
- Link to v1: https://patch.msgid.link/20260719-support_vendor_hci-v1-1-764523a4ca3d@oss.qualcomm.com

To: Marcel Holtmann <marcel@holtmann.org>
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Zijun Hu <zijun_hu@icloud.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Zijun Hu (3):
      Bluetooth: btmrvl_sdio: Do not free HCI_VENDOR_PKT frame by hci_recv_frame()
      Bluetooth: Add generic support for vendor HCI frames using HCI_VENDOR_PKT
      Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames

 drivers/bluetooth/btmrvl_sdio.c   |  2 +-
 include/net/bluetooth/bluetooth.h |  2 ++
 include/net/bluetooth/hci.h       |  1 +
 include/net/bluetooth/hci_core.h  |  7 ++++
 include/net/bluetooth/hci_mon.h   |  2 ++
 net/bluetooth/hci_core.c          | 71 +++++++++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c         |  5 +++
 net/bluetooth/hci_sock.c          | 53 ++++++++++++++++++++++++++---
 8 files changed, 137 insertions(+), 6 deletions(-)
---
base-commit: bd8bee79e1fa8db2d587733f8b6fd9597b04d6e3
change-id: 20260719-support_vendor_hci-1fbb4449ab8e
prerequisite-change-id: 20260719-fix_hci_rx_work-f03dd49d8ef8:v1
prerequisite-patch-id: cdf6a14522201fa54cbe21392b7445544c24a977

Best regards,
--  
Zijun Hu <zijun.hu@oss.qualcomm.com>

[PATCH RFC v2 1/3] Bluetooth: btmrvl_sdio: Do not free HCI_VENDOR_PKT frame by hci_recv_frame()

[Zijun Hu]

For a HCI_VENDOR_PKT frame, hci_recv_frame() does not accept it and
will kfree_skb() it directly.

But btmrvl_sdio_card_to_host() is still calling hci_recv_frame() for
the frame.

Fix by freeing it with kfree_skb() directly.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/bluetooth/btmrvl_sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 93932a0d8625..b91fc63bc9fe 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -799,7 +799,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 		skb_pull(skb, SDIO_HEADER_LEN);
 
 		if (btmrvl_process_event(priv, skb))
-			hci_recv_frame(hdev, skb);
+			kfree_skb(skb);
 
 		hdev->stat.byte_rx += buf_len;
 		break;

-- 
2.34.1

RE: Bluetooth: Add generic support for vendor HCI frames

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: net/bluetooth/hci_core.c:4057
error: net/bluetooth/hci_core.c: patch does not apply
hint: Use 'git am --show-current-patch' to see the failed patch

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

[PATCH RFC v2 2/3] Bluetooth: Add generic support for vendor HCI frames using HCI_VENDOR_PKT

[Zijun Hu]

The virtual HCI_VENDOR_PKT (0xff) has been defined but never used by
the BT core. Now, there is a requirement to implement it, as follows:

For Qualcomm multi-subsystem BT chips, the transport wire carries both
BT-HCI and PERI-HCI frames. PERI is a subsystem on the chip. Take the
upcoming QCC2072 support as an example:

    Packet type                 BT-HCI indicator   PERI-HCI indicator
    -----------------------------------------------------------------
    CMD (Host -> Controller)    0x01               0x31
    ACL Data (bidirectional)    0x02               0x32
    EVENT (Controller -> Host)  0x04               0x34

To generically support vendor HCI frames:

- Show them in btmon logs as they appear on the wire.
- Allow userspace to send/receive them via HCI_CHANNEL_USER, with a
  socket option to control RX, defaulting off to eliminate regression
  risk for existing applications.
- Add hdev->recv_vendor_pkt() to handle vendor frames in hci_rx_work()
  like BT frame handlers.
- Add hci_send_vendor_frame() API similar to existing __hci_cmd_send().

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 include/net/bluetooth/bluetooth.h |  2 ++
 include/net/bluetooth/hci.h       |  1 +
 include/net/bluetooth/hci_core.h  |  4 +++
 include/net/bluetooth/hci_mon.h   |  2 ++
 net/bluetooth/hci_core.c          | 38 ++++++++++++++++++++++++++++
 net/bluetooth/hci_sock.c          | 53 +++++++++++++++++++++++++++++++++++----
 6 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index b624da5026f5..d9870a43a2c4 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -256,6 +256,8 @@ struct bt_codecs {
 
 #define BT_SCM_PKT_SEQNUM	0x05
 
+#define BT_RCV_VENDOR_PKT	23
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 50f0eef71fb1..ac97550cbdba 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -398,6 +398,7 @@ enum {
 /* HCI socket flags */
 enum {
 	HCI_SOCK_TRUSTED,
+	HCI_SOCK_RCV_VENDOR_PKT,
 	HCI_MGMT_INDEX_EVENTS,
 	HCI_MGMT_UNCONF_INDEX_EVENTS,
 	HCI_MGMT_EXT_INDEX_EVENTS,
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e7133ff87fbf..428261591288 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -28,6 +28,7 @@
 #include <linux/rculist.h>
 #include <linux/spinlock.h>
 #include <linux/srcu.h>
+#include <linux/uio.h>
 
 #include <net/bluetooth/hci.h>
 #include <net/bluetooth/hci_drv.h>
@@ -645,6 +646,7 @@ struct hci_dev {
 	int (*setup)(struct hci_dev *hdev);
 	int (*shutdown)(struct hci_dev *hdev);
 	int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
+	void (*recv_vendor_pkt)(struct hci_dev *hdev, struct sk_buff *skb);
 	void (*notify)(struct hci_dev *hdev, unsigned int evt);
 	void (*hw_error)(struct hci_dev *hdev, u8 code);
 	int (*post_init)(struct hci_dev *hdev);
@@ -2327,6 +2329,8 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
 int hci_register_cb(struct hci_cb *hcb);
 int hci_unregister_cb(struct hci_cb *hcb);
 
+int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter);
+
 int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen,
 		   const void *param);
 
diff --git a/include/net/bluetooth/hci_mon.h b/include/net/bluetooth/hci_mon.h
index 4b2a0af4ed58..7710688c0d30 100644
--- a/include/net/bluetooth/hci_mon.h
+++ b/include/net/bluetooth/hci_mon.h
@@ -50,6 +50,8 @@ struct hci_mon_hdr {
 #define HCI_MON_ISO_RX_PKT	19
 #define HCI_MON_DRV_TX_PKT	20
 #define HCI_MON_DRV_RX_PKT	21
+#define HCI_MON_VENDOR_TX_PKT	22
+#define HCI_MON_VENDOR_RX_PKT	23
 
 struct hci_mon_new_index {
 	__u8		type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1e324c05af24..90807562e8ed 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2917,6 +2917,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
 		break;
 	case HCI_ISODATA_PKT:
 		break;
+	case HCI_VENDOR_PKT:
+		break;
 	case HCI_DRV_PKT:
 		break;
 	default:
@@ -3053,6 +3055,33 @@ static int hci_send_conn_frame(struct hci_dev *hdev, struct hci_conn *conn,
 	return hci_send_frame(hdev, skb);
 }
 
+int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter)
+{
+	struct sk_buff *skb;
+	size_t len;
+
+	if (WARN_ON(!iov_iter_is_kvec(iter)))
+		return -EINVAL;
+
+	len = iov_iter_count(iter);
+	if (!len)
+		return -EINVAL;
+
+	skb = bt_skb_alloc(len, GFP_KERNEL);
+	if (!skb)
+		return -ENOMEM;
+
+	if (!copy_from_iter_full(skb_put(skb, len), len, iter)) {
+		kfree_skb(skb);
+		return -EFAULT;
+	}
+
+	hci_skb_pkt_type(skb) = HCI_VENDOR_PKT;
+
+	return hci_send_frame(hdev, skb);
+}
+EXPORT_SYMBOL(hci_send_vendor_frame);
+
 /* Send HCI command */
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen,
 		 const void *param)
@@ -4057,6 +4086,15 @@ static void hci_rx_work(struct work_struct *work)
 			hci_isodata_packet(hdev, skb);
 			break;
 
+		case HCI_VENDOR_PKT:
+			BT_DBG("%s Vendor packet with type 0x%2.2x",
+			       hdev->name, hci_skb_pkt_type(skb));
+			if (hdev->recv_vendor_pkt)
+				hdev->recv_vendor_pkt(hdev, skb);
+			else
+				kfree_skb(skb);
+			break;
+
 		case HCI_DRV_PKT:
 			kfree_skb(skb);
 			break;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 070ca388f9ac..060c265e2c53 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -229,12 +229,16 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb)
 		} else if (hci_pi(sk)->channel == HCI_CHANNEL_USER) {
 			if (!bt_cb(skb)->incoming)
 				continue;
-			if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
-			    hci_skb_pkt_type(skb) != HCI_DRV_PKT)
+			if (hci_skb_pkt_type(skb) == HCI_VENDOR_PKT) {
+				if (!hci_sock_test_flag(sk, HCI_SOCK_RCV_VENDOR_PKT))
+					continue;
+			} else if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
+				   hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
+				   hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
+				   hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+				   hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
 				continue;
+			}
 		} else {
 			/* Don't send frame to other channel types */
 			continue;
@@ -390,6 +394,12 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
 		else
 			opcode = cpu_to_le16(HCI_MON_ISO_TX_PKT);
 		break;
+	case HCI_VENDOR_PKT:
+		if (bt_cb(skb)->incoming)
+			opcode = cpu_to_le16(HCI_MON_VENDOR_RX_PKT);
+		else
+			opcode = cpu_to_le16(HCI_MON_VENDOR_TX_PKT);
+		break;
 	case HCI_DRV_PKT:
 		if (bt_cb(skb)->incoming)
 			opcode = cpu_to_le16(HCI_MON_DRV_RX_PKT);
@@ -1868,6 +1878,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
 		    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_ISODATA_PKT &&
+		    hci_skb_pkt_type(skb) != HCI_VENDOR_PKT &&
 		    hci_skb_pkt_type(skb) != HCI_DRV_PKT) {
 			err = -EINVAL;
 			goto drop;
@@ -2017,6 +2028,7 @@ static int hci_sock_setsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	int err = 0;
+	int opt_int;
 	u16 opt;
 
 	BT_DBG("sk %p, opt %d", sk, optname);
@@ -2050,6 +2062,23 @@ static int hci_sock_setsockopt(struct socket *sock, int level, int optname,
 		hci_pi(sk)->mtu = opt;
 		break;
 
+	case BT_RCV_VENDOR_PKT:
+		if (hci_pi(sk)->channel != HCI_CHANNEL_USER) {
+			err = -ENOPROTOOPT;
+			break;
+		}
+
+		err = copy_safe_from_sockptr(&opt_int, sizeof(opt_int),
+					     optval, optlen);
+		if (err)
+			break;
+
+		if (opt_int)
+			hci_sock_set_flag(sk, HCI_SOCK_RCV_VENDOR_PKT);
+		else
+			hci_sock_clear_flag(sk, HCI_SOCK_RCV_VENDOR_PKT);
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;
@@ -2132,6 +2161,7 @@ static int hci_sock_getsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	int err = 0;
+	int opt_int;
 	u16 mtu;
 
 	BT_DBG("sk %p, opt %d", sk, optname);
@@ -2153,6 +2183,19 @@ static int hci_sock_getsockopt(struct socket *sock, int level, int optname,
 			err = -EFAULT;
 		break;
 
+	case BT_RCV_VENDOR_PKT:
+		if (hci_pi(sk)->channel != HCI_CHANNEL_USER) {
+			err = -ENOPROTOOPT;
+			break;
+		}
+
+		opt_int = hci_sock_test_flag(sk, HCI_SOCK_RCV_VENDOR_PKT) ?
+			  1 : 0;
+		if (copy_to_iter(&opt_int, sizeof(opt_int),
+				 &sopt->iter_out) != sizeof(opt_int))
+			err = -EFAULT;
+		break;
+
 	default:
 		err = -ENOPROTOOPT;
 		break;

-- 
2.34.1

[PATCH RFC v2 3/3] Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames

[Zijun Hu]

For unsolicited VSE and ACL frames with vendor reserved handles:
Many transport drivers pre-process them in the RX-path because the BT core
cannot recognize them, and cause:

- Impact performance since skb_clone() in IRQ-disabled context
- Difficult to debug as frames consumed without btmon logging

Fix by adding the hook to process them within stack's process
context, when they have been logged by btmon.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 include/net/bluetooth/hci_core.h |  3 +++
 net/bluetooth/hci_core.c         | 33 +++++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        |  5 +++++
 3 files changed, 41 insertions(+)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 428261591288..12f3720f6cc8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -646,6 +646,8 @@ struct hci_dev {
 	int (*setup)(struct hci_dev *hdev);
 	int (*shutdown)(struct hci_dev *hdev);
 	int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
+	/* Return true if @skb was consumed, false otherwise */
+	bool (*recv_bt_vendor)(struct hci_dev *hdev, struct sk_buff *skb);
 	void (*recv_vendor_pkt)(struct hci_dev *hdev, struct sk_buff *skb);
 	void (*notify)(struct hci_dev *hdev, unsigned int evt);
 	void (*hw_error)(struct hci_dev *hdev, u8 code);
@@ -2329,6 +2331,7 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
 int hci_register_cb(struct hci_cb *hcb);
 int hci_unregister_cb(struct hci_cb *hcb);
 
+bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb);
 int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter);
 
 int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen,
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 90807562e8ed..21ef748e3528 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3830,6 +3830,12 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 	__u16 handle, flags;
 	int err;
 
+	/* Header is valid when ACL frame arrives here */
+	if (hci_recv_bt_vendor(hdev, skb)) {
+		hdev->stat.acl_rx++;
+		return;
+	}
+
 	hdr = skb_pull_data(skb, sizeof(*hdr));
 	if (!hdr) {
 		bt_dev_err(hdev, "ACL packet too small");
@@ -3918,6 +3924,33 @@ static void hci_isodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
 			   handle, err);
 }
 
+bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb)
+{
+	struct hci_event_hdr *evt_hdr;
+	u8 evt, pkt_type;
+
+	if (!hdev->recv_bt_vendor)
+		return false;
+
+	pkt_type = hci_skb_pkt_type(skb);
+
+	/* Save the event code before calling into the driver, since @skb
+	 * may be freed once consumed.
+	 */
+	if (pkt_type == HCI_EVENT_PKT) {
+		evt_hdr = hci_event_hdr(skb);
+		evt = evt_hdr->evt;
+	}
+
+	if (!hdev->recv_bt_vendor(hdev, skb))
+		return false;
+
+	if (pkt_type == HCI_EVENT_PKT && evt != HCI_EV_VENDOR)
+		bt_dev_warn(hdev, "consumed unexpected BT event 0x%2.2x", evt);
+
+	return true;
+}
+
 static bool hci_req_is_complete(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index ea858391c789..94c02b66bdfa 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -7801,6 +7801,11 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
 		goto done;
 	}
 
+	if (hci_recv_bt_vendor(hdev, skb)) {
+		hdev->stat.evt_rx++;
+		return;
+	}
+
 	hci_dev_lock(hdev);
 	kfree_skb(hdev->recv_event);
 	hdev->recv_event = skb_clone(skb, GFP_KERNEL);

-- 
2.34.1

Re: [PATCH RFC v2 3/3] Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames

[Luiz Augusto von Dentz]

Hi Zijun,

On Wed, Jul 22, 2026 at 12:24 PM Zijun Hu <zijun.hu@oss.qualcomm.com> wrote:
>
> For unsolicited VSE and ACL frames with vendor reserved handles:
> Many transport drivers pre-process them in the RX-path because the BT core
> cannot recognize them, and cause:
>
> - Impact performance since skb_clone() in IRQ-disabled context
> - Difficult to debug as frames consumed without btmon logging
>
> Fix by adding the hook to process them within stack's process
> context, when they have been logged by btmon.
>
> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  include/net/bluetooth/hci_core.h |  3 +++
>  net/bluetooth/hci_core.c         | 33 +++++++++++++++++++++++++++++++++
>  net/bluetooth/hci_event.c        |  5 +++++
>  3 files changed, 41 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 428261591288..12f3720f6cc8 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -646,6 +646,8 @@ struct hci_dev {
>         int (*setup)(struct hci_dev *hdev);
>         int (*shutdown)(struct hci_dev *hdev);
>         int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
> +       /* Return true if @skb was consumed, false otherwise */
> +       bool (*recv_bt_vendor)(struct hci_dev *hdev, struct sk_buff *skb);

This should be probably called recv_vendor_ev or something like that.

>         void (*recv_vendor_pkt)(struct hci_dev *hdev, struct sk_buff *skb);
>         void (*notify)(struct hci_dev *hdev, unsigned int evt);
>         void (*hw_error)(struct hci_dev *hdev, u8 code);
> @@ -2329,6 +2331,7 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
>  int hci_register_cb(struct hci_cb *hcb);
>  int hci_unregister_cb(struct hci_cb *hcb);
>
> +bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb);
>  int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter);
>
>  int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen,
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 90807562e8ed..21ef748e3528 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3830,6 +3830,12 @@ static void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
>         __u16 handle, flags;
>         int err;
>
> +       /* Header is valid when ACL frame arrives here */
> +       if (hci_recv_bt_vendor(hdev, skb)) {
> +               hdev->stat.acl_rx++;
> +               return;
> +       }
> +
>         hdr = skb_pull_data(skb, sizeof(*hdr));
>         if (!hdr) {
>                 bt_dev_err(hdev, "ACL packet too small");
> @@ -3918,6 +3924,33 @@ static void hci_isodata_packet(struct hci_dev *hdev, struct sk_buff *skb)
>                            handle, err);
>  }
>
> +bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       struct hci_event_hdr *evt_hdr;
> +       u8 evt, pkt_type;
> +
> +       if (!hdev->recv_bt_vendor)
> +               return false;
> +
> +       pkt_type = hci_skb_pkt_type(skb);
> +
> +       /* Save the event code before calling into the driver, since @skb
> +        * may be freed once consumed.
> +        */
> +       if (pkt_type == HCI_EVENT_PKT) {
> +               evt_hdr = hci_event_hdr(skb);
> +               evt = evt_hdr->evt;
> +       }
> +
> +       if (!hdev->recv_bt_vendor(hdev, skb))
> +               return false;
> +
> +       if (pkt_type == HCI_EVENT_PKT && evt != HCI_EV_VENDOR)
> +               bt_dev_warn(hdev, "consumed unexpected BT event 0x%2.2x", evt);
> +
> +       return true;
> +}
> +
>  static bool hci_req_is_complete(struct hci_dev *hdev)
>  {
>         struct sk_buff *skb;
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index ea858391c789..94c02b66bdfa 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -7801,6 +7801,11 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>                 goto done;
>         }
>
> +       if (hci_recv_bt_vendor(hdev, skb)) {
> +               hdev->stat.evt_rx++;
> +               return;
> +       }

This seems too early actually, I would have expected it to run after
hci_event_func or within it if ev->func is NULL, that said we
currently assume HCI_EV_VENDOR=msft_vendor_evt which is probably not
valid, or maybe it is which then screw the whole idea that 0xff is
only used with 1 vendor specific domain, now if we try to squeesh both
a vendor event and a msft event handler their opcodes shall not
collide otherwise the whole thing doesn't work.

Anyway, I had the impression you would be using a vendor packet not a
vendor event, or you use both?

>         hci_dev_lock(hdev);
>         kfree_skb(hdev->recv_event);
>         hdev->recv_event = skb_clone(skb, GFP_KERNEL);
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH RFC v2 3/3] Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames

[Luiz Augusto von Dentz]

Hi Zijun,

On Thu, Jul 23, 2026 at 7:18 AM Zijun Hu <zijun.hu@oss.qualcomm.com> wrote:
>
> On 7/23/2026 12:46 AM, Luiz Augusto von Dentz wrote:
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index 428261591288..12f3720f6cc8 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -646,6 +646,8 @@ struct hci_dev {
> >>         int (*setup)(struct hci_dev *hdev);
> >>         int (*shutdown)(struct hci_dev *hdev);
> >>         int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
> >> +       /* Return true if @skb was consumed, false otherwise */
> >> +       bool (*recv_bt_vendor)(struct hci_dev *hdev, struct sk_buff *skb);
> > This should be probably called recv_vendor_ev or something like that.
> >
>
> For BT, besides VSEs, some vendors also reserve ACL handles for special usages, e.g.:
>
> | Vendor | Coredump | Other                                   |
> |--------|----------|-----------------------------------------|
> | QCOM   | 0xEDD    | Enhanced Logging (0xEDC)                |
> | MTK    | 0xFC6F   | Firmware debug logging (0x05FF, 0x05FE) |
> | NXP    | 0xFFF    | —                                       |
>
> So this hook may need to cover both. Any suggestion for a good name?

Derr, connection traffic is not handled as events and you have the
likes of  hdev->classify_pkt_type(hdev, skb); to reclassify them to
HCI_DIAG_PKT. Don't tell me the firmware it generating Connection
Complete, NOCP, or any other event, for these because that is totally
against the spec since the handle wouldn't match any connection.

> >>         void (*recv_vendor_pkt)(struct hci_dev *hdev, struct sk_buff *skb);
> >>         void (*notify)(struct hci_dev *hdev, unsigned int evt);
> >>         void (*hw_error)(struct hci_dev *hdev, u8 code);
> >> @@ -2329,6 +2331,7 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
> >>  int hci_register_cb(struct hci_cb *hcb);
> >>  int hci_unregister_cb(struct hci_cb *hcb);
> >>
> >> +bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb);
> >>  int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter);
> >>
>
> [...]
>
> >>  static bool hci_req_is_complete(struct hci_dev *hdev)
> >>  {
> >>         struct sk_buff *skb;
> >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> >> index ea858391c789..94c02b66bdfa 100644
> >> --- a/net/bluetooth/hci_event.c
> >> +++ b/net/bluetooth/hci_event.c
> >> @@ -7801,6 +7801,11 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
> >>                 goto done;
> >>         }
> >>
> >> +       if (hci_recv_bt_vendor(hdev, skb)) {
> >> +               hdev->stat.evt_rx++;
> >> +               return;
> >> +       }
> > This seems too early actually, I would have expected it to run after
> > hci_event_func or within it if ev->func is NULL, that said we
>
> I don't think running it after hci_event_func() works, because the VSE has already been
> corrupted by then, as explained below:
>
> msft_vendor_evt() is the handler for all vendor events called by hci_event_func(), but many
> VSEs are not MSFT ones. Once the MSFT extension is enabled, it always calls skb_pull_data()
> and corrupts the non-MSFT VSEs.
>
> Besides, running it there is asymmetric with the ACL path: the event header is already pulled,
> but the ACL header isn't.
>
> For ev->func is NULL:
> it may mean we don't need to care about that event, so the current behavior of ignoring it
> is reasonable.
>
> > currently assume HCI_EV_VENDOR=msft_vendor_evt which is probably not
> > valid, or maybe it is which then screw the whole idea that 0xff is
> > only used with 1 vendor specific domain, now if we try to squeesh both
> > a vendor event and a msft event handler their opcodes shall not
> > collide otherwise the whole thing doesn't work.
> >
>
> let me explain more for recv_bt_vendor() for BT as below:
>
> Design idea:
> let the transport driver consume whatever frames it is interested in first; for
> the rest, don't care how the stack handles them — it doesn't matter even if they are
> corrupted.

Well I already told you that by doing preposing we may expose driver
to the same type of bugs that existed in core in the past, so we maybe
reintroducing parsing errors and since this requires to preserve the
skb it will not be able to use skb_data_pulll (which internally checks
skb->len).

> Motivation:
> For the VSEs and vendor-reserved-handle ACLs that transport drivers are interested in, move
> their handling from the transport driver's RX path (IRQ-disabled context) to the stack's
> process context — i.e. avoid pre-processing them.
>
> Advantages:
> -) Avoid skb_clone() in IRQ-disabled context of RX-path to improve performance
>    take INTEL diagnostic events as an example, btintel_diagnostics() skb_clone() them
> -) Avoid re-processing VSEs that the driver has already handled.
>    Intel diagnostic events still arrive at hci_event_packet() in hci_rx_work(), even though
>    they have already been handled by btintel_diagnostics().

What does btintel have to do with this? Are you saying there are bugs
in it and you are trying to report them here?

> -) Log them via btmon to help debugging
>     Several existing transport drivers consume these frames directly without btmon logging
>
> I'd welcome your thoughts — especially on the hook name (point 1) and whether the
> placement/design here is acceptable. Happy to rework it based on your guidance.
>
> > Anyway, I had the impression you would be using a vendor packet not a
> > vendor event, or you use both?
> >
>
> My PERI-HCI requirement is already addressed by recv_vendor_pkt() in [PATCH 2/3]
>
> This hook, recv_bt_vendor(), addresses concerns raised during the BT-HCI discussion. The same
> problem exists in many transport drivers, so it tries to address them for BT along the way
>
> I'd like to use it in the upcoming QCC2072 support as well, once these concerns are resolved.
>
> >>         hci_dev_lock(hdev);
> >>         kfree_skb(hdev->recv_event);
> >>         hdev->recv_event = skb_clone(skb, GFP_KERNEL);
>


-- 
Luiz Augusto von Dentz

Re: [PATCH RFC v2 3/3] Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames

[Zijun Hu]

On 7/23/2026 9:56 PM, Luiz Augusto von Dentz wrote:
>> | Vendor | Coredump | Other                                   |
>> |--------|----------|-----------------------------------------|
>> | QCOM   | 0xEDD    | Enhanced Logging (0xEDC)                |
>> | MTK    | 0xFC6F   | Firmware debug logging (0x05FF, 0x05FE) |
>> | NXP    | 0xFFF    | —                                       |
>>
>> So this hook may need to cover both. Any suggestion for a good name?
> Derr, connection traffic is not handled as events and you have the
> likes of  hdev->classify_pkt_type(hdev, skb); to reclassify them to
> HCI_DIAG_PKT. Don't tell me the firmware it generating Connection
> Complete, NOCP, or any other event, for these because that is totally
> against the spec since the handle wouldn't match any connection.
> 

okay, got your point.

so, what about changing recv_vendor_ev() to recv_ev_vendor() to map HCI_EV_VENDOR ?


>>>>         void (*recv_vendor_pkt)(struct hci_dev *hdev, struct sk_buff *skb);
>>>>         void (*notify)(struct hci_dev *hdev, unsigned int evt);
>>>>         void (*hw_error)(struct hci_dev *hdev, u8 code);
>>>> @@ -2329,6 +2331,7 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency,
>>>>  int hci_register_cb(struct hci_cb *hcb);
>>>>  int hci_unregister_cb(struct hci_cb *hcb);
>>>>
>>>> +bool hci_recv_bt_vendor(struct hci_dev *hdev, struct sk_buff *skb);
>>>>  int hci_send_vendor_frame(struct hci_dev *hdev, struct iov_iter *iter);
>>>>
>> [...]
>>
>>>>  static bool hci_req_is_complete(struct hci_dev *hdev)
>>>>  {
>>>>         struct sk_buff *skb;
>>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
>>>> index ea858391c789..94c02b66bdfa 100644
>>>> --- a/net/bluetooth/hci_event.c
>>>> +++ b/net/bluetooth/hci_event.c
>>>> @@ -7801,6 +7801,11 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
>>>>                 goto done;
>>>>         }
>>>>
>>>> +       if (hci_recv_bt_vendor(hdev, skb)) {
>>>> +               hdev->stat.evt_rx++;
>>>> +               return;
>>>> +       }
>>> This seems too early actually, I would have expected it to run after
>>> hci_event_func or within it if ev->func is NULL, that said we
>> I don't think running it after hci_event_func() works, because the VSE has already been
>> corrupted by then, as explained below:
>>
>> msft_vendor_evt() is the handler for all vendor events called by hci_event_func(), but many
>> VSEs are not MSFT ones. Once the MSFT extension is enabled, it always calls skb_pull_data()
>> and corrupts the non-MSFT VSEs.
>>
>> Besides, running it there is asymmetric with the ACL path: the event header is already pulled,
>> but the ACL header isn't.
>>
>> For ev->func is NULL:
>> it may mean we don't need to care about that event, so the current behavior of ignoring it
>> is reasonable.
>>
>>> currently assume HCI_EV_VENDOR=msft_vendor_evt which is probably not
>>> valid, or maybe it is which then screw the whole idea that 0xff is
>>> only used with 1 vendor specific domain, now if we try to squeesh both
>>> a vendor event and a msft event handler their opcodes shall not
>>> collide otherwise the whole thing doesn't work.
>>>
>> let me explain more for recv_bt_vendor() for BT as below:
>>
>> Design idea:
>> let the transport driver consume whatever frames it is interested in first; for
>> the rest, don't care how the stack handles them — it doesn't matter even if they are
>> corrupted.
> Well I already told you that by doing preposing we may expose driver
> to the same type of bugs that existed in core in the past, so we maybe
> reintroducing parsing errors and since this requires to preserve the
> skb it will not be able to use skb_data_pulll (which internally checks
> skb->len).
> 


Sorry, my description was misleading.
The callback registered by the transport driver is meant to avoid pre-processing:
the stack invokes it for the driver's VSEs of interest, and the callback then uses skb_pull_data()
to handle the event just as the stack would.
>> Motivation:
>> For the VSEs and vendor-reserved-handle ACLs that transport drivers are interested in, move
>> their handling from the transport driver's RX path (IRQ-disabled context) to the stack's
>> process context — i.e. avoid pre-processing them.
>>
>> Advantages:
>> -) Avoid skb_clone() in IRQ-disabled context of RX-path to improve performance
>>    take INTEL diagnostic events as an example, btintel_diagnostics() skb_clone() them
>> -) Avoid re-processing VSEs that the driver has already handled.
>>    Intel diagnostic events still arrive at hci_event_packet() in hci_rx_work(), even though
>>    they have already been handled by btintel_diagnostics().
> What does btintel have to do with this? Are you saying there are bugs
> in it and you are trying to report them here?


No, I'm not reporting bugs in btintel. Almost all transport drivers handle these vendor frames the same way

show what this callback may improve, using Intel as an example since it came up earlier as a reference.


Any further comments ?

Re: [PATCH RFC v2 0/3] Bluetooth: Add generic support for vendor HCI frames

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 22 Jul 2026 09:24:19 -0700 you wrote:
> Hi Luiz
> To address the below concerns you raised on previous
> discussion.
> 
> - Not use safer skb helper like skb_pull_data()
> - It will skip sending to the monitor, making debugging much harder
> 
> [...]

Here is the summary with links:
  - [RFC,v2,1/3] Bluetooth: btmrvl_sdio: Do not free HCI_VENDOR_PKT frame by hci_recv_frame()
    https://git.kernel.org/bluetooth/bluetooth-next/c/dcf47a799e75
  - [RFC,v2,2/3] Bluetooth: Add generic support for vendor HCI frames using HCI_VENDOR_PKT
    (no matching commit)
  - [RFC,v2,3/3] Bluetooth: Add hdev->recv_bt_vendor() to handle BT vendor frames
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html