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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> ---
>  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