Re: [PATCH RFC] Bluetooth: Add generic support for vendor HCI frames

[Luiz Augusto von Dentz]

Hi Zijun,

On Mon, Jul 20, 2026 at 1:14 AM Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx> wrote:
>
> 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

I would say this was fine, but you actuall need to reclassify these to
0xff and then use an extra byte for the vendor opcode. Btw, you said
you could define H4 headers like this? The spec defines either:

https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Core_v6.3/out/en/host-controller-interface/three-wire-uart-transport-layer.html#UUID-1cf959bb-57a0-e782-4324-a9bc4ee3f134_table-idm13359018144714

Or

https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Core_v6.3/out/en/host-controller-interface/uart-transport-layer.html#UUID-83881875-5fbb-c366-400f-055ba726f70e_table-idm13359015052542

We end up defining vendor 0xff because it is at the end of the range
which is probably safer.

> 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.

We shouldn't need to do this, we should just accept 0xff as the vendor
packet type and then use the following byte as the real opcode (0x31,
0x32, 0x34). Otherwise, if we start accepting vendor packet types
outside 0xff this will get crowded quickly and could potentially clash
with future specs.

> - Add hdev->recv_vendor() to handle vendor frames in hci_rx_work() like
>   BT frame handlers.
> - Add hci_send_vendor_frame() API similar to existing __hci_cmd_send().

Encoding/decoding of the frames should be transparent. I'm fine adding
code to the likes of btmon to decode vendor packets, we already have
something similar for Intel although that uses a vendor event not a
vendor packet (both use 0xff, causing the confusion). The user of the
user channel shall be able to read/write starting with 0xff then
decode/encode the next byte as the actual vendor opcode.

> Signed-off-by: Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx>
> ---
> Hi Luiz
> To address the below concerns you raised on previous
> discussion, and inspired from your suggestion to add hdev callback:
>
> - 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.
>
> Why hdev->is_vendor() instead of multiplexing vendor frames under a
> virtual HCI_VENDOR_PKT + vendor indicator byte ?
>
> 1) The virtual HCI_VENDOR_PKT isn't used by the core itself
>    currently, and may be removed later.
> 2) It makes full use of the indicator (hci_skb_pkt_type(skb)) space,
>    which is large enough to accommodate vendor frames without an
>    extra layer of nesting inside HCI_VENDOR_PKT.
> 3) It lets userspace and btmon logs see the exact same frame as it
>    appears on the wire.
>
> Previous discussion link:
> https://lore.kernel.org/all/CABBYNZJpdeoZ16bObLRPhng2dfyNeK9ix9_-z_hMZhJ0QxTxGg@xxxxxxxxxxxxxx
> https://lore.kernel.org/all/CABBYNZJ0zoVVVxuM48L=Km==gnrQuskrbjB6q_aNV0KEEY3+5w@xxxxxxxxxxxxxx
> ---
>  include/net/bluetooth/bluetooth.h |  2 +
>  include/net/bluetooth/hci.h       |  1 +
>  include/net/bluetooth/hci_core.h  | 13 +++++++
>  include/net/bluetooth/hci_mon.h   |  2 +
>  net/bluetooth/hci_core.c          | 22 +++++++++++
>  net/bluetooth/hci_sock.c          | 78 ++++++++++++++++++++++++++++++++++-----
>  6 files changed, 108 insertions(+), 10 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..20a455d90ef3 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -645,6 +645,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);
> +       bool (*is_vendor)(struct hci_dev *hdev, const struct sk_buff *skb);
> +       void (*recv_vendor)(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);
> @@ -660,6 +662,15 @@ struct hci_dev {
>         u8 (*classify_pkt_type)(struct hci_dev *hdev, struct sk_buff *skb);
>  };
>
> +static inline bool hci_is_vendor_frame(struct hci_dev *hdev,
> +                                      const struct sk_buff *skb)
> +{
> +       if (!hdev->is_vendor)
> +               return false;
> +
> +       return hdev->is_vendor(hdev, skb);
> +}
> +
>  #define hci_set_quirk(hdev, nr) set_bit((nr), (hdev)->quirk_flags)
>  #define hci_clear_quirk(hdev, nr) clear_bit((nr), (hdev)->quirk_flags)
>  #define hci_test_quirk(hdev, nr) test_bit((nr), (hdev)->quirk_flags)
> @@ -2327,6 +2338,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 sk_buff *skb);
> +
>  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..c12c180e8293 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2920,6 +2920,8 @@ int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb)
>         case HCI_DRV_PKT:
>                 break;
>         default:
> +               if (hci_is_vendor_frame(hdev, skb))
> +                       break;
>                 kfree_skb(skb);
>                 return -EINVAL;
>         }
> @@ -3053,6 +3055,18 @@ 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 sk_buff *skb)
> +{
> +       if (hci_is_vendor_frame(hdev, skb))
> +               return hci_send_frame(hdev, skb);
> +
> +       bt_dev_err(hdev, "invalid vendor frame with pkt_type 0x%2.2x",
> +                  hci_skb_pkt_type(skb));
> +       kfree_skb(skb);
> +       return -EINVAL;
> +}
> +EXPORT_SYMBOL(hci_send_vendor_frame);
> +
>  /* Send HCI command */
>  int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen,
>                  const void *param)
> @@ -4066,6 +4080,14 @@ static void hci_rx_work(struct work_struct *work)
>                         break;
>
>                 default:
> +                       if (hci_is_vendor_frame(hdev, skb)) {
> +                               BT_DBG("%s Vendor packet with type 0x%2.2x",
> +                                      hdev->name, hci_skb_pkt_type(skb));
> +                               if (hdev->recv_vendor) {
> +                                       hdev->recv_vendor(hdev, skb);
> +                                       break;
> +                               }
> +                       }
>                         kfree_skb(skb);
>                         break;
>                 }
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 070ca388f9ac..903498e6db3d 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_is_vendor_frame(hdev, skb)) {
> +                               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;
> @@ -359,6 +363,7 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>         struct sk_buff *skb_copy = NULL;
>         struct hci_mon_hdr *hdr;
>         __le16 opcode;
> +       bool is_vendor_frame = false;
>
>         if (!atomic_read(&monitor_promisc))
>                 return;
> @@ -400,7 +405,26 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>                 opcode = cpu_to_le16(HCI_MON_VENDOR_DIAG);
>                 break;
>         default:
> -               return;
> +               is_vendor_frame = hci_is_vendor_frame(hdev, skb);
> +               if (!is_vendor_frame)
> +                       return;
> +               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;
> +       }
> +
> +       if (is_vendor_frame) {
> +               skb_copy = __pskb_copy_fclone(skb, HCI_MON_HDR_SIZE + 1,
> +                                             GFP_ATOMIC, true);
> +               if (!skb_copy)
> +                       return;
> +
> +               *(u8 *)skb_push(skb_copy, 1) = hci_skb_pkt_type(skb);
> +               hdr = skb_push(skb_copy, HCI_MON_HDR_SIZE);
> +               hdr->len = cpu_to_le16(skb->len + 1);
> +               goto out_comm;
>         }
>
>         /* Create a private copy with headroom */
> @@ -408,14 +432,15 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
>         if (!skb_copy)
>                 return;
>
> -       hci_sock_copy_creds(skb->sk, skb_copy);
> -
>         /* Put header before the data */
>         hdr = skb_push(skb_copy, HCI_MON_HDR_SIZE);
> +       hdr->len = cpu_to_le16(skb->len);
> +
> +out_comm:
>         hdr->opcode = opcode;
>         hdr->index = cpu_to_le16(hdev->id);
> -       hdr->len = cpu_to_le16(skb->len);
>
> +       hci_sock_copy_creds(skb->sk, skb_copy);
>         hci_send_to_channel(HCI_CHANNEL_MONITOR, skb_copy,
>                             HCI_SOCK_TRUSTED, NULL);
>         kfree_skb(skb_copy);
> @@ -1868,7 +1893,8 @@ 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_DRV_PKT) {
> +                   hci_skb_pkt_type(skb) != HCI_DRV_PKT &&
> +                   !hci_is_vendor_frame(hdev, skb)) {
>                         err = -EINVAL;
>                         goto drop;
>                 }
> @@ -2017,6 +2043,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 +2077,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 +2176,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 +2198,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;
>
> ---
> base-commit: 7f02cdbb4cf17df8aa1977eea0cf490c472a706d
> change-id: 20260719-support_vendor_hci-1fbb4449ab8e
>
> Best regards,
> --
> Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx>
>


-- 
Luiz Augusto von Dentz

Attachment: image.png
Description: PNG image