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

[Zijun Hu <zijun.hu@oss.qualcomm.com>]

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?

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

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().
-) 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);