diff mbox series

[v4,1/3] Bluetooth: aosp: surface AOSP quality report through mgmt

Message ID 20220215213519.v4.1.I2015b42d2d0a502334c9c3a2983438b89716d4f0@changeid (mailing list archive)
State Changes Requested
Headers show
Series [v4,1/3] Bluetooth: aosp: surface AOSP quality report through mgmt | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/subjectprefix success PASS
tedd_an/buildkernel success Build Kernel PASS
tedd_an/buildkernel32 success Build Kernel32 PASS
tedd_an/incremental_build success Pass
tedd_an/testrunnersetup success Test Runner Setup PASS
tedd_an/testrunnerl2cap-tester success Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerbnep-tester success Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnermgmt-tester success Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerrfcomm-tester success Total: 10, Passed: 10 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersco-tester success Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersmp-tester success Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunneruserchan-tester success Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0

Commit Message

Joseph Hwang Feb. 15, 2022, 1:35 p.m. UTC 
When receiving a HCI vendor event, the kernel checks if it is an
AOSP bluetooth quality report. If yes, the event is sent to bluez
user space through the mgmt socket.

Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-by: Archie Pusaka <apusaka@chromium.org>
---

(no changes since v3)

Changes in v3:
- Rebase to resolve the code conflict.
- Move aosp_quality_report_evt() from hci_event.c to aosp.c.
- A new patch (3/3) is added to enable the quality report feature.

Changes in v2:
- Scrap the two structures defined in aosp.c and use constants for
  size check.
- Do a basic size check about the quality report event. Do not pull
  data from the event in which the kernel has no interest.
- Define vendor event prefixes with which vendor events of distinct
  vendor specifications can be clearly differentiated.
- Use mgmt helpers to add the header and data to a mgmt skb.

 include/net/bluetooth/hci_core.h |  5 ++
 include/net/bluetooth/mgmt.h     |  7 +++
 net/bluetooth/aosp.c             | 27 ++++++++++
 net/bluetooth/aosp.h             | 13 +++++
 net/bluetooth/hci_event.c        | 84 +++++++++++++++++++++++++++++++-
 net/bluetooth/mgmt.c             | 20 ++++++++
 net/bluetooth/msft.c             | 14 ++++++
 net/bluetooth/msft.h             | 12 +++++
 8 files changed, 181 insertions(+), 1 deletion(-)

Comments

bluez.test.bot@gmail.com Feb. 15, 2022, 2:08 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=614537

---Test result---

Test Summary:
CheckPatch                    PASS      4.35 seconds
GitLint                       PASS      1.15 seconds
SubjectPrefix                 PASS      0.69 seconds
BuildKernel                   PASS      35.09 seconds
BuildKernel32                 PASS      30.56 seconds
Incremental Build with patchesPASS      82.91 seconds
TestRunner: Setup             PASS      556.96 seconds
TestRunner: l2cap-tester      PASS      14.92 seconds
TestRunner: bnep-tester       PASS      6.70 seconds
TestRunner: mgmt-tester       PASS      119.24 seconds
TestRunner: rfcomm-tester     PASS      8.50 seconds
TestRunner: sco-tester        PASS      8.70 seconds
TestRunner: smp-tester        PASS      8.49 seconds
TestRunner: userchan-tester   PASS      7.03 seconds



---
Regards,
Linux Bluetooth
kernel test robot Feb. 15, 2022, 5:52 p.m. UTC | #2 
Hi Joseph,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on net-next/master next-20220215]
[cannot apply to net/master v5.17-rc4]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Joseph-Hwang/Bluetooth-aosp-surface-AOSP-quality-report-through-mgmt/20220215-213800
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: h8300-randconfig-s032-20220214 (https://download.01.org/0day-ci/archive/20220216/202202160117.jjnGwidL-lkp@intel.com/config)
compiler: h8300-linux-gcc (GCC) 11.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/8c2212761e41006d67f3fad819b5bde57bc17773
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Joseph-Hwang/Bluetooth-aosp-surface-AOSP-quality-report-through-mgmt/20220215-213800
        git checkout 8c2212761e41006d67f3fad819b5bde57bc17773
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=h8300 SHELL=/bin/bash net/bluetooth/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   net/bluetooth/hci_event.c:338:15: sparse: sparse: restricted __le16 degrades to integer
>> net/bluetooth/hci_event.c:4288:3: sparse: sparse: symbol 'evt_prefixes' was not declared. Should it be static?
   net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
   include/net/bluetooth/hci.h:2473:47: sparse: sparse: array of flexible structures
   include/net/bluetooth/hci.h:2559:43: sparse: sparse: array of flexible structures

vim +/evt_prefixes +4288 net/bluetooth/hci_event.c

  4275	
  4276	/* Every distinct vendor specification must have a well-defined vendor
  4277	 * event prefix to determine if a vendor event meets the specification.
  4278	 * If an event prefix is fixed, it should be delcared with FIXED_EVT_PREFIX.
  4279	 * Otherwise, DYNAMIC_EVT_PREFIX should be used for variable prefixes.
  4280	 */
  4281	struct vendor_event_prefix {
  4282		__u8 *prefix;
  4283		__u8 prefix_len;
  4284		void (*vendor_func)(struct hci_dev *hdev, void *data,
  4285				    struct sk_buff *skb);
  4286		__u8 *(*get_prefix)(struct hci_dev *hdev);
  4287		__u8 (*get_prefix_len)(struct hci_dev *hdev);
> 4288	} evt_prefixes[] = {
  4289		FIXED_EVT_PREFIX(AOSP_BQR_PREFIX, aosp_quality_report_evt),
  4290		DYNAMIC_EVT_PREFIX(get_msft_evt_prefix, get_msft_evt_prefix_len,
  4291				   msft_vendor_evt),
  4292	
  4293		/* end with a null entry */
  4294		{},
  4295	};
  4296	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Marcel Holtmann Feb. 17, 2022, 12:41 p.m. UTC | #3 
Hi Joseph,

> When receiving a HCI vendor event, the kernel checks if it is an
> AOSP bluetooth quality report. If yes, the event is sent to bluez
> user space through the mgmt socket.
> 
> Signed-off-by: Joseph Hwang <josephsih@chromium.org>
> Reviewed-by: Archie Pusaka <apusaka@chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Rebase to resolve the code conflict.
> - Move aosp_quality_report_evt() from hci_event.c to aosp.c.
> - A new patch (3/3) is added to enable the quality report feature.
> 
> Changes in v2:
> - Scrap the two structures defined in aosp.c and use constants for
>  size check.
> - Do a basic size check about the quality report event. Do not pull
>  data from the event in which the kernel has no interest.
> - Define vendor event prefixes with which vendor events of distinct
>  vendor specifications can be clearly differentiated.
> - Use mgmt helpers to add the header and data to a mgmt skb.

this unsolicited vendor event business is giving me a headache. I assume it would be a lot simpler, but it doesn’t look like this. I need to spent some rounds of thinking to give you good advice. Unfortunately since we also want to support Intel vendor specific pieces, this is getting super complicated.

> include/net/bluetooth/hci_core.h |  5 ++
> include/net/bluetooth/mgmt.h     |  7 +++
> net/bluetooth/aosp.c             | 27 ++++++++++
> net/bluetooth/aosp.h             | 13 +++++
> net/bluetooth/hci_event.c        | 84 +++++++++++++++++++++++++++++++-
> net/bluetooth/mgmt.c             | 20 ++++++++
> net/bluetooth/msft.c             | 14 ++++++
> net/bluetooth/msft.h             | 12 +++++
> 8 files changed, 181 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index f5caff1ddb29..ea83619ac4de 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1864,6 +1864,8 @@ int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
> int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
> void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
> 				  bdaddr_t *bdaddr, u8 addr_type);
> +int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
> +			u8 quality_spec);
> 
> u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
> 		      u16 to_multiplier);
> @@ -1882,4 +1884,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> 
> #define TRANSPORT_TYPE_MAX	0x04
> 
> +#define QUALITY_SPEC_AOSP_BQR		0x0
> +#define QUALITY_SPEC_INTEL_TELEMETRY	0x1
> +
> #endif /* __HCI_CORE_H */
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 3d26e6a3478b..83b602636262 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -1120,3 +1120,10 @@ struct mgmt_ev_adv_monitor_device_lost {
> 	__le16 monitor_handle;
> 	struct mgmt_addr_info addr;
> } __packed;
> +
> +#define MGMT_EV_QUALITY_REPORT			0x0031
> +struct mgmt_ev_quality_report {
> +	__u8	quality_spec;
> +	__u32	data_len;
> +	__u8	data[];
> +} __packed;
> diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
> index 432ae3aac9e3..4a336433180d 100644
> --- a/net/bluetooth/aosp.c
> +++ b/net/bluetooth/aosp.c
> @@ -199,3 +199,30 @@ int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> 	else
> 		return disable_quality_report(hdev);
> }
> +
> +#define BLUETOOTH_QUALITY_REPORT_EV		0x58
> +
> +/* The following LEN = 1-byte Sub-event code + 48-byte Sub-event Parameters */
> +#define BLUETOOTH_QUALITY_REPORT_LEN		49
> +
> +bool aosp_check_quality_report_len(struct sk_buff *skb)
> +{
> +	/* skb->len is allowed to be larger than BLUETOOTH_QUALITY_REPORT_LEN
> +	 * to accommodate an additional Vendor Specific Parameter (vsp) field.
> +	 */
> +	if (skb->len < BLUETOOTH_QUALITY_REPORT_LEN) {
> +		BT_ERR("AOSP evt data len %d too short (%u expected)",
> +		       skb->len, BLUETOOTH_QUALITY_REPORT_LEN);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> +			     struct sk_buff *skb)
> +{
> +	if (aosp_has_quality_report(hdev) && aosp_check_quality_report_len(skb))
> +		mgmt_quality_report(hdev, skb->data, skb->len,
> +				    QUALITY_SPEC_AOSP_BQR);
> +}
> diff --git a/net/bluetooth/aosp.h b/net/bluetooth/aosp.h
> index 2fd8886d51b2..b21751e012de 100644
> --- a/net/bluetooth/aosp.h
> +++ b/net/bluetooth/aosp.h
> @@ -10,6 +10,9 @@ void aosp_do_close(struct hci_dev *hdev);
> 
> bool aosp_has_quality_report(struct hci_dev *hdev);
> int aosp_set_quality_report(struct hci_dev *hdev, bool enable);
> +bool aosp_check_quality_report_len(struct sk_buff *skb);
> +void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> +			     struct sk_buff *skb);
> 
> #else
> 
> @@ -26,4 +29,14 @@ static inline int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> 	return -EOPNOTSUPP;
> }
> 
> +static inline bool aosp_check_quality_report_len(struct sk_buff *skb)
> +{
> +	return false;
> +}
> +
> +static inline void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> +					   struct sk_buff *skb)
> +{
> +}
> +
> #endif
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 63b925921c87..6468ea0f71bd 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -37,6 +37,7 @@
> #include "smp.h"
> #include "msft.h"
> #include "eir.h"
> +#include "aosp.h"
> 
> #define ZERO_KEY "\x00\x00\x00\x00\x00\x00\x00\x00" \
> 		 "\x00\x00\x00\x00\x00\x00\x00\x00"
> @@ -4241,6 +4242,87 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
> 	queue_work(hdev->workqueue, &hdev->tx_work);
> }
> 
> +/* Define the fixed vendor event prefixes below.
> + * Note: AOSP HCI Requirements use 0x54 and up as sub-event codes without
> + *       actually defining a vendor prefix. Refer to
> + *       https://source.android.com/devices/bluetooth/hci_requirements
> + *       Hence, the other vendor event prefixes should not use the same
> + *       space to avoid collision.
> + */
> +static unsigned char AOSP_BQR_PREFIX[] = { 0x58 };
> +
> +/* Some vendor prefixes are fixed values and lengths. */
> +#define FIXED_EVT_PREFIX(_prefix, _vendor_func)				\
> +{									\
> +	.prefix = _prefix,						\
> +	.prefix_len = sizeof(_prefix),					\
> +	.vendor_func = _vendor_func,					\
> +	.get_prefix = NULL,						\
> +	.get_prefix_len = NULL,						\
> +}
> +
> +/* Some vendor prefixes are only available at run time. The
> + * values and lengths are variable.
> + */
> +#define DYNAMIC_EVT_PREFIX(_prefix_func, _prefix_len_func, _vendor_func)\
> +{									\
> +	.prefix = NULL,							\
> +	.prefix_len = 0,						\
> +	.vendor_func = _vendor_func,					\
> +	.get_prefix = _prefix_func,					\
> +	.get_prefix_len = _prefix_len_func,				\
> +}
> +
> +/* Every distinct vendor specification must have a well-defined vendor
> + * event prefix to determine if a vendor event meets the specification.
> + * If an event prefix is fixed, it should be delcared with FIXED_EVT_PREFIX.
> + * Otherwise, DYNAMIC_EVT_PREFIX should be used for variable prefixes.
> + */
> +struct vendor_event_prefix {
> +	__u8 *prefix;
> +	__u8 prefix_len;
> +	void (*vendor_func)(struct hci_dev *hdev, void *data,
> +			    struct sk_buff *skb);
> +	__u8 *(*get_prefix)(struct hci_dev *hdev);
> +	__u8 (*get_prefix_len)(struct hci_dev *hdev);
> +} evt_prefixes[] = {
> +	FIXED_EVT_PREFIX(AOSP_BQR_PREFIX, aosp_quality_report_evt),
> +	DYNAMIC_EVT_PREFIX(get_msft_evt_prefix, get_msft_evt_prefix_len,
> +			   msft_vendor_evt),
> +
> +	/* end with a null entry */
> +	{},
> +};
> +
> +static void hci_vendor_evt(struct hci_dev *hdev, void *data,
> +			   struct sk_buff *skb)
> +{
> +	int i;
> +	__u8 *prefix;
> +	__u8 prefix_len;
> +
> +	for (i = 0; evt_prefixes[i].vendor_func; i++) {
> +		if (evt_prefixes[i].get_prefix)
> +			prefix = evt_prefixes[i].get_prefix(hdev);
> +		else
> +			prefix = evt_prefixes[i].prefix;
> +
> +		if (evt_prefixes[i].get_prefix_len)
> +			prefix_len = evt_prefixes[i].get_prefix_len(hdev);
> +		else
> +			prefix_len = evt_prefixes[i].prefix_len;
> +
> +		if (!prefix || prefix_len == 0)
> +			continue;
> +
> +		/* Compare the raw prefix data directly. */
> +		if (!memcmp(prefix, skb->data, prefix_len)) {
> +			evt_prefixes[i].vendor_func(hdev, data, skb);
> +			break;
> +		}
> +	}
> +}
> +
> static void hci_mode_change_evt(struct hci_dev *hdev, void *data,
> 				struct sk_buff *skb)
> {
> @@ -6844,7 +6926,7 @@ static const struct hci_ev {
> 	HCI_EV(HCI_EV_NUM_COMP_BLOCKS, hci_num_comp_blocks_evt,
> 	       sizeof(struct hci_ev_num_comp_blocks)),
> 	/* [0xff = HCI_EV_VENDOR] */
> -	HCI_EV_VL(HCI_EV_VENDOR, msft_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
> +	HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
> };

I was thinking along the lines like this:

	HCI_EV_VND(evt_prefix, evt_prefix_len, callback),

So that we in the end can do things like this:

	HCI_EV_VL({ 0x58 }, 1, aosp_quality_report_evt),


> 
> static void hci_event_func(struct hci_dev *hdev, u8 event, struct sk_buff *skb,
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 914e2f2d3586..5e48576041fb 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4389,6 +4389,26 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> 			       MGMT_STATUS_NOT_SUPPORTED);
> }
> 
> +int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
> +			u8 quality_spec)
> +{
> +	struct mgmt_ev_quality_report *ev;
> +	struct sk_buff *skb;
> +
> +	skb = mgmt_alloc_skb(hdev, MGMT_EV_QUALITY_REPORT,
> +			     sizeof(*ev) + data_len);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	ev = skb_put(skb, sizeof(*ev));
> +	ev->quality_spec = quality_spec;
> +	ev->data_len = data_len;
> +	skb_put_data(skb, data, data_len);
> +
> +	return mgmt_event_skb(skb, NULL);
> +}
> +EXPORT_SYMBOL(mgmt_quality_report);
> +

I know what you want to do, but I can not let you call mgmt_ function from a driver. We need to make this cleaner and abstract so that the driver has a proper path to report it to hci_core and that decides then to send the report or not.

> static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
> 			    u16 data_len)
> {
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index 9a3d77d3ca86..3edf64baf479 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -731,6 +731,20 @@ static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
> 				 handle_data->mgmt_handle);
> }
> 
> +__u8 *get_msft_evt_prefix(struct hci_dev *hdev)
> +{
> +	struct msft_data *msft = hdev->msft_data;
> +
> +	return msft->evt_prefix;
> +}
> +
> +__u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
> +{
> +	struct msft_data *msft = hdev->msft_data;
> +
> +	return msft->evt_prefix_len;
> +}
> +

So I wonder if this should be moved directly into hci_dev under CONFIG_BT_MSFTEXT check. Luiz also needs to have a look at this. This is unfortunately getting a bit nasty now. We need to find a clean solution, otherwise the next vendor thing is blowing up in our face.

> void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
> {
> 	struct msft_data *msft = hdev->msft_data;
> diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
> index afcaf7d3b1cb..a354ebf61fed 100644
> --- a/net/bluetooth/msft.h
> +++ b/net/bluetooth/msft.h
> @@ -27,6 +27,8 @@ int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
> int msft_suspend_sync(struct hci_dev *hdev);
> int msft_resume_sync(struct hci_dev *hdev);
> bool msft_curve_validity(struct hci_dev *hdev);
> +__u8 *get_msft_evt_prefix(struct hci_dev *hdev);
> +__u8 get_msft_evt_prefix_len(struct hci_dev *hdev);
> 
> #else
> 
> @@ -77,4 +79,14 @@ static inline bool msft_curve_validity(struct hci_dev *hdev)
> 	return false;
> }
> 
> +static inline __u8 *get_msft_evt_prefix(struct hci_dev *hdev)
> +{
> +	return NULL;
> +}
> +
> +static inline __u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
> +{
> +	return 0;
> +}
> +
> #endif

Regards

Marcel
Joseph Hwang March 5, 2022, 7:42 a.m. UTC | #4 
Hi Marcel, thank you for reviewing the patches! I have some questions
and would like to confirm with you. Please read my replies in the
lines below. Thanks!

On Thu, Feb 17, 2022 at 8:41 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Joseph,
>
> > When receiving a HCI vendor event, the kernel checks if it is an
> > AOSP bluetooth quality report. If yes, the event is sent to bluez
> > user space through the mgmt socket.
> >
> > Signed-off-by: Joseph Hwang <josephsih@chromium.org>
> > Reviewed-by: Archie Pusaka <apusaka@chromium.org>
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> > - Rebase to resolve the code conflict.
> > - Move aosp_quality_report_evt() from hci_event.c to aosp.c.
> > - A new patch (3/3) is added to enable the quality report feature.
> >
> > Changes in v2:
> > - Scrap the two structures defined in aosp.c and use constants for
> >  size check.
> > - Do a basic size check about the quality report event. Do not pull
> >  data from the event in which the kernel has no interest.
> > - Define vendor event prefixes with which vendor events of distinct
> >  vendor specifications can be clearly differentiated.
> > - Use mgmt helpers to add the header and data to a mgmt skb.
>
> this unsolicited vendor event business is giving me a headache. I assume it would be a lot simpler, but it doesn’t look like this. I need to spent some rounds of thinking to give you good advice. Unfortunately since we also want to support Intel vendor specific pieces, this is getting super complicated.

You have mentioned using hdev->hci_recv_quality_report to send the
unsolicited events from the Intel driver in your comments in Patch
2/3. Just would like to confirm with you. Thanks.

>
> > include/net/bluetooth/hci_core.h |  5 ++
> > include/net/bluetooth/mgmt.h     |  7 +++
> > net/bluetooth/aosp.c             | 27 ++++++++++
> > net/bluetooth/aosp.h             | 13 +++++
> > net/bluetooth/hci_event.c        | 84 +++++++++++++++++++++++++++++++-
> > net/bluetooth/mgmt.c             | 20 ++++++++
> > net/bluetooth/msft.c             | 14 ++++++
> > net/bluetooth/msft.h             | 12 +++++
> > 8 files changed, 181 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index f5caff1ddb29..ea83619ac4de 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1864,6 +1864,8 @@ int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
> > int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
> > void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
> >                                 bdaddr_t *bdaddr, u8 addr_type);
> > +int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
> > +                     u8 quality_spec);
> >
> > u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
> >                     u16 to_multiplier);
> > @@ -1882,4 +1884,7 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> >
> > #define TRANSPORT_TYPE_MAX    0x04
> >
> > +#define QUALITY_SPEC_AOSP_BQR                0x0
> > +#define QUALITY_SPEC_INTEL_TELEMETRY 0x1
> > +
> > #endif /* __HCI_CORE_H */
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index 3d26e6a3478b..83b602636262 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -1120,3 +1120,10 @@ struct mgmt_ev_adv_monitor_device_lost {
> >       __le16 monitor_handle;
> >       struct mgmt_addr_info addr;
> > } __packed;
> > +
> > +#define MGMT_EV_QUALITY_REPORT                       0x0031
> > +struct mgmt_ev_quality_report {
> > +     __u8    quality_spec;
> > +     __u32   data_len;
> > +     __u8    data[];
> > +} __packed;
> > diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
> > index 432ae3aac9e3..4a336433180d 100644
> > --- a/net/bluetooth/aosp.c
> > +++ b/net/bluetooth/aosp.c
> > @@ -199,3 +199,30 @@ int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> >       else
> >               return disable_quality_report(hdev);
> > }
> > +
> > +#define BLUETOOTH_QUALITY_REPORT_EV          0x58
> > +
> > +/* The following LEN = 1-byte Sub-event code + 48-byte Sub-event Parameters */
> > +#define BLUETOOTH_QUALITY_REPORT_LEN         49
> > +
> > +bool aosp_check_quality_report_len(struct sk_buff *skb)
> > +{
> > +     /* skb->len is allowed to be larger than BLUETOOTH_QUALITY_REPORT_LEN
> > +      * to accommodate an additional Vendor Specific Parameter (vsp) field.
> > +      */
> > +     if (skb->len < BLUETOOTH_QUALITY_REPORT_LEN) {
> > +             BT_ERR("AOSP evt data len %d too short (%u expected)",
> > +                    skb->len, BLUETOOTH_QUALITY_REPORT_LEN);
> > +             return false;
> > +     }
> > +
> > +     return true;
> > +}
> > +
> > +void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> > +                          struct sk_buff *skb)
> > +{
> > +     if (aosp_has_quality_report(hdev) && aosp_check_quality_report_len(skb))
> > +             mgmt_quality_report(hdev, skb->data, skb->len,
> > +                                 QUALITY_SPEC_AOSP_BQR);
> > +}
> > diff --git a/net/bluetooth/aosp.h b/net/bluetooth/aosp.h
> > index 2fd8886d51b2..b21751e012de 100644
> > --- a/net/bluetooth/aosp.h
> > +++ b/net/bluetooth/aosp.h
> > @@ -10,6 +10,9 @@ void aosp_do_close(struct hci_dev *hdev);
> >
> > bool aosp_has_quality_report(struct hci_dev *hdev);
> > int aosp_set_quality_report(struct hci_dev *hdev, bool enable);
> > +bool aosp_check_quality_report_len(struct sk_buff *skb);
> > +void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> > +                          struct sk_buff *skb);
> >
> > #else
> >
> > @@ -26,4 +29,14 @@ static inline int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> >       return -EOPNOTSUPP;
> > }
> >
> > +static inline bool aosp_check_quality_report_len(struct sk_buff *skb)
> > +{
> > +     return false;
> > +}
> > +
> > +static inline void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
> > +                                        struct sk_buff *skb)
> > +{
> > +}
> > +
> > #endif
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 63b925921c87..6468ea0f71bd 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -37,6 +37,7 @@
> > #include "smp.h"
> > #include "msft.h"
> > #include "eir.h"
> > +#include "aosp.h"
> >
> > #define ZERO_KEY "\x00\x00\x00\x00\x00\x00\x00\x00" \
> >                "\x00\x00\x00\x00\x00\x00\x00\x00"
> > @@ -4241,6 +4242,87 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
> >       queue_work(hdev->workqueue, &hdev->tx_work);
> > }
> >
> > +/* Define the fixed vendor event prefixes below.
> > + * Note: AOSP HCI Requirements use 0x54 and up as sub-event codes without
> > + *       actually defining a vendor prefix. Refer to
> > + *       https://source.android.com/devices/bluetooth/hci_requirements
> > + *       Hence, the other vendor event prefixes should not use the same
> > + *       space to avoid collision.
> > + */
> > +static unsigned char AOSP_BQR_PREFIX[] = { 0x58 };
> > +
> > +/* Some vendor prefixes are fixed values and lengths. */
> > +#define FIXED_EVT_PREFIX(_prefix, _vendor_func)                              \
> > +{                                                                    \
> > +     .prefix = _prefix,                                              \
> > +     .prefix_len = sizeof(_prefix),                                  \
> > +     .vendor_func = _vendor_func,                                    \
> > +     .get_prefix = NULL,                                             \
> > +     .get_prefix_len = NULL,                                         \
> > +}
> > +
> > +/* Some vendor prefixes are only available at run time. The
> > + * values and lengths are variable.
> > + */
> > +#define DYNAMIC_EVT_PREFIX(_prefix_func, _prefix_len_func, _vendor_func)\
> > +{                                                                    \
> > +     .prefix = NULL,                                                 \
> > +     .prefix_len = 0,                                                \
> > +     .vendor_func = _vendor_func,                                    \
> > +     .get_prefix = _prefix_func,                                     \
> > +     .get_prefix_len = _prefix_len_func,                             \
> > +}
> > +
> > +/* Every distinct vendor specification must have a well-defined vendor
> > + * event prefix to determine if a vendor event meets the specification.
> > + * If an event prefix is fixed, it should be delcared with FIXED_EVT_PREFIX.
> > + * Otherwise, DYNAMIC_EVT_PREFIX should be used for variable prefixes.
> > + */
> > +struct vendor_event_prefix {
> > +     __u8 *prefix;
> > +     __u8 prefix_len;
> > +     void (*vendor_func)(struct hci_dev *hdev, void *data,
> > +                         struct sk_buff *skb);
> > +     __u8 *(*get_prefix)(struct hci_dev *hdev);
> > +     __u8 (*get_prefix_len)(struct hci_dev *hdev);
> > +} evt_prefixes[] = {
> > +     FIXED_EVT_PREFIX(AOSP_BQR_PREFIX, aosp_quality_report_evt),
> > +     DYNAMIC_EVT_PREFIX(get_msft_evt_prefix, get_msft_evt_prefix_len,
> > +                        msft_vendor_evt),
> > +
> > +     /* end with a null entry */
> > +     {},
> > +};
> > +
> > +static void hci_vendor_evt(struct hci_dev *hdev, void *data,
> > +                        struct sk_buff *skb)
> > +{
> > +     int i;
> > +     __u8 *prefix;
> > +     __u8 prefix_len;
> > +
> > +     for (i = 0; evt_prefixes[i].vendor_func; i++) {
> > +             if (evt_prefixes[i].get_prefix)
> > +                     prefix = evt_prefixes[i].get_prefix(hdev);
> > +             else
> > +                     prefix = evt_prefixes[i].prefix;
> > +
> > +             if (evt_prefixes[i].get_prefix_len)
> > +                     prefix_len = evt_prefixes[i].get_prefix_len(hdev);
> > +             else
> > +                     prefix_len = evt_prefixes[i].prefix_len;
> > +
> > +             if (!prefix || prefix_len == 0)
> > +                     continue;
> > +
> > +             /* Compare the raw prefix data directly. */
> > +             if (!memcmp(prefix, skb->data, prefix_len)) {
> > +                     evt_prefixes[i].vendor_func(hdev, data, skb);
> > +                     break;
> > +             }
> > +     }
> > +}
> > +
> > static void hci_mode_change_evt(struct hci_dev *hdev, void *data,
> >                               struct sk_buff *skb)
> > {
> > @@ -6844,7 +6926,7 @@ static const struct hci_ev {
> >       HCI_EV(HCI_EV_NUM_COMP_BLOCKS, hci_num_comp_blocks_evt,
> >              sizeof(struct hci_ev_num_comp_blocks)),
> >       /* [0xff = HCI_EV_VENDOR] */
> > -     HCI_EV_VL(HCI_EV_VENDOR, msft_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
> > +     HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
> > };
>
> I was thinking along the lines like this:
>
>         HCI_EV_VND(evt_prefix, evt_prefix_len, callback),
>
> So that we in the end can do things like this:
>
>         HCI_EV_VL({ 0x58 }, 1, aosp_quality_report_evt),
>

I wish I could use a macro as simple as HCI_EV_VND(evt_prefix,
evt_prefix_len, callback). However, I do not have a clean method to
handle the dynamic msft vendor prefix of which the value and length
are not known until runtime. Do you have any suggestions here?

>
>
> >
> > static void hci_event_func(struct hci_dev *hdev, u8 event, struct sk_buff *skb,
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 914e2f2d3586..5e48576041fb 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -4389,6 +4389,26 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> >                              MGMT_STATUS_NOT_SUPPORTED);
> > }
> >
> > +int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
> > +                     u8 quality_spec)
> > +{
> > +     struct mgmt_ev_quality_report *ev;
> > +     struct sk_buff *skb;
> > +
> > +     skb = mgmt_alloc_skb(hdev, MGMT_EV_QUALITY_REPORT,
> > +                          sizeof(*ev) + data_len);
> > +     if (!skb)
> > +             return -ENOMEM;
> > +
> > +     ev = skb_put(skb, sizeof(*ev));
> > +     ev->quality_spec = quality_spec;
> > +     ev->data_len = data_len;
> > +     skb_put_data(skb, data, data_len);
> > +
> > +     return mgmt_event_skb(skb, NULL);
> > +}
> > +EXPORT_SYMBOL(mgmt_quality_report);
> > +
>
> I know what you want to do, but I can not let you call mgmt_ function from a driver. We need to make this cleaner and abstract so that the driver has a proper path to report it to hci_core and that decides then to send the report or not.

Will set up hdev->hci_recv_quality_report as you mentioned in comments
in Patch 2/3.

>
>
> > static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
> >                           u16 data_len)
> > {
> > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > index 9a3d77d3ca86..3edf64baf479 100644
> > --- a/net/bluetooth/msft.c
> > +++ b/net/bluetooth/msft.c
> > @@ -731,6 +731,20 @@ static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
> >                                handle_data->mgmt_handle);
> > }
> >
> > +__u8 *get_msft_evt_prefix(struct hci_dev *hdev)
> > +{
> > +     struct msft_data *msft = hdev->msft_data;
> > +
> > +     return msft->evt_prefix;
> > +}
> > +
> > +__u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
> > +{
> > +     struct msft_data *msft = hdev->msft_data;
> > +
> > +     return msft->evt_prefix_len;
> > +}
> > +
>
> So I wonder if this should be moved directly into hci_dev under CONFIG_BT_MSFTEXT check. Luiz also needs to have a look at this. This is unfortunately getting a bit nasty now. We need to find a clean solution, otherwise the next vendor thing is blowing up in our face.

My understanding is that static inline hci_set_msft_opcode() under
CONFIG_BT_MSFTEXT is placed in hci_core.h because drivers need to call
it. Here, get_msft_evt_prefix() and get_msft_evt_prefix_len() are not
to be called by drivers. Do we still need to move them into
hci_core.h? Please let me know if I have any misunderstanding. Thanks.

>
>
> > void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
> > {
> >       struct msft_data *msft = hdev->msft_data;
> > diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
> > index afcaf7d3b1cb..a354ebf61fed 100644
> > --- a/net/bluetooth/msft.h
> > +++ b/net/bluetooth/msft.h
> > @@ -27,6 +27,8 @@ int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
> > int msft_suspend_sync(struct hci_dev *hdev);
> > int msft_resume_sync(struct hci_dev *hdev);
> > bool msft_curve_validity(struct hci_dev *hdev);
> > +__u8 *get_msft_evt_prefix(struct hci_dev *hdev);
> > +__u8 get_msft_evt_prefix_len(struct hci_dev *hdev);
> >
> > #else
> >
> > @@ -77,4 +79,14 @@ static inline bool msft_curve_validity(struct hci_dev *hdev)
> >       return false;
> > }
> >
> > +static inline __u8 *get_msft_evt_prefix(struct hci_dev *hdev)
> > +{
> > +     return NULL;
> > +}
> > +
> > +static inline __u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
> > +{
> > +     return 0;
> > +}
> > +
> > #endif
>
> Regards
>
> Marcel
>

Regards,
diff mbox series

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index f5caff1ddb29..ea83619ac4de 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1864,6 +1864,8 @@  int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
 int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
 void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
 				  bdaddr_t *bdaddr, u8 addr_type);
+int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
+			u8 quality_spec);
 
 u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
 		      u16 to_multiplier);
@@ -1882,4 +1884,7 @@  void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
 
 #define TRANSPORT_TYPE_MAX	0x04
 
+#define QUALITY_SPEC_AOSP_BQR		0x0
+#define QUALITY_SPEC_INTEL_TELEMETRY	0x1
+
 #endif /* __HCI_CORE_H */
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 3d26e6a3478b..83b602636262 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -1120,3 +1120,10 @@  struct mgmt_ev_adv_monitor_device_lost {
 	__le16 monitor_handle;
 	struct mgmt_addr_info addr;
 } __packed;
+
+#define MGMT_EV_QUALITY_REPORT			0x0031
+struct mgmt_ev_quality_report {
+	__u8	quality_spec;
+	__u32	data_len;
+	__u8	data[];
+} __packed;
diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
index 432ae3aac9e3..4a336433180d 100644
--- a/net/bluetooth/aosp.c
+++ b/net/bluetooth/aosp.c
@@ -199,3 +199,30 @@  int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
 	else
 		return disable_quality_report(hdev);
 }
+
+#define BLUETOOTH_QUALITY_REPORT_EV		0x58
+
+/* The following LEN = 1-byte Sub-event code + 48-byte Sub-event Parameters */
+#define BLUETOOTH_QUALITY_REPORT_LEN		49
+
+bool aosp_check_quality_report_len(struct sk_buff *skb)
+{
+	/* skb->len is allowed to be larger than BLUETOOTH_QUALITY_REPORT_LEN
+	 * to accommodate an additional Vendor Specific Parameter (vsp) field.
+	 */
+	if (skb->len < BLUETOOTH_QUALITY_REPORT_LEN) {
+		BT_ERR("AOSP evt data len %d too short (%u expected)",
+		       skb->len, BLUETOOTH_QUALITY_REPORT_LEN);
+		return false;
+	}
+
+	return true;
+}
+
+void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
+			     struct sk_buff *skb)
+{
+	if (aosp_has_quality_report(hdev) && aosp_check_quality_report_len(skb))
+		mgmt_quality_report(hdev, skb->data, skb->len,
+				    QUALITY_SPEC_AOSP_BQR);
+}
diff --git a/net/bluetooth/aosp.h b/net/bluetooth/aosp.h
index 2fd8886d51b2..b21751e012de 100644
--- a/net/bluetooth/aosp.h
+++ b/net/bluetooth/aosp.h
@@ -10,6 +10,9 @@  void aosp_do_close(struct hci_dev *hdev);
 
 bool aosp_has_quality_report(struct hci_dev *hdev);
 int aosp_set_quality_report(struct hci_dev *hdev, bool enable);
+bool aosp_check_quality_report_len(struct sk_buff *skb);
+void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
+			     struct sk_buff *skb);
 
 #else
 
@@ -26,4 +29,14 @@  static inline int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
 	return -EOPNOTSUPP;
 }
 
+static inline bool aosp_check_quality_report_len(struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline void aosp_quality_report_evt(struct hci_dev *hdev,  void *data,
+					   struct sk_buff *skb)
+{
+}
+
 #endif
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 63b925921c87..6468ea0f71bd 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -37,6 +37,7 @@ 
 #include "smp.h"
 #include "msft.h"
 #include "eir.h"
+#include "aosp.h"
 
 #define ZERO_KEY "\x00\x00\x00\x00\x00\x00\x00\x00" \
 		 "\x00\x00\x00\x00\x00\x00\x00\x00"
@@ -4241,6 +4242,87 @@  static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
 	queue_work(hdev->workqueue, &hdev->tx_work);
 }
 
+/* Define the fixed vendor event prefixes below.
+ * Note: AOSP HCI Requirements use 0x54 and up as sub-event codes without
+ *       actually defining a vendor prefix. Refer to
+ *       https://source.android.com/devices/bluetooth/hci_requirements
+ *       Hence, the other vendor event prefixes should not use the same
+ *       space to avoid collision.
+ */
+static unsigned char AOSP_BQR_PREFIX[] = { 0x58 };
+
+/* Some vendor prefixes are fixed values and lengths. */
+#define FIXED_EVT_PREFIX(_prefix, _vendor_func)				\
+{									\
+	.prefix = _prefix,						\
+	.prefix_len = sizeof(_prefix),					\
+	.vendor_func = _vendor_func,					\
+	.get_prefix = NULL,						\
+	.get_prefix_len = NULL,						\
+}
+
+/* Some vendor prefixes are only available at run time. The
+ * values and lengths are variable.
+ */
+#define DYNAMIC_EVT_PREFIX(_prefix_func, _prefix_len_func, _vendor_func)\
+{									\
+	.prefix = NULL,							\
+	.prefix_len = 0,						\
+	.vendor_func = _vendor_func,					\
+	.get_prefix = _prefix_func,					\
+	.get_prefix_len = _prefix_len_func,				\
+}
+
+/* Every distinct vendor specification must have a well-defined vendor
+ * event prefix to determine if a vendor event meets the specification.
+ * If an event prefix is fixed, it should be delcared with FIXED_EVT_PREFIX.
+ * Otherwise, DYNAMIC_EVT_PREFIX should be used for variable prefixes.
+ */
+struct vendor_event_prefix {
+	__u8 *prefix;
+	__u8 prefix_len;
+	void (*vendor_func)(struct hci_dev *hdev, void *data,
+			    struct sk_buff *skb);
+	__u8 *(*get_prefix)(struct hci_dev *hdev);
+	__u8 (*get_prefix_len)(struct hci_dev *hdev);
+} evt_prefixes[] = {
+	FIXED_EVT_PREFIX(AOSP_BQR_PREFIX, aosp_quality_report_evt),
+	DYNAMIC_EVT_PREFIX(get_msft_evt_prefix, get_msft_evt_prefix_len,
+			   msft_vendor_evt),
+
+	/* end with a null entry */
+	{},
+};
+
+static void hci_vendor_evt(struct hci_dev *hdev, void *data,
+			   struct sk_buff *skb)
+{
+	int i;
+	__u8 *prefix;
+	__u8 prefix_len;
+
+	for (i = 0; evt_prefixes[i].vendor_func; i++) {
+		if (evt_prefixes[i].get_prefix)
+			prefix = evt_prefixes[i].get_prefix(hdev);
+		else
+			prefix = evt_prefixes[i].prefix;
+
+		if (evt_prefixes[i].get_prefix_len)
+			prefix_len = evt_prefixes[i].get_prefix_len(hdev);
+		else
+			prefix_len = evt_prefixes[i].prefix_len;
+
+		if (!prefix || prefix_len == 0)
+			continue;
+
+		/* Compare the raw prefix data directly. */
+		if (!memcmp(prefix, skb->data, prefix_len)) {
+			evt_prefixes[i].vendor_func(hdev, data, skb);
+			break;
+		}
+	}
+}
+
 static void hci_mode_change_evt(struct hci_dev *hdev, void *data,
 				struct sk_buff *skb)
 {
@@ -6844,7 +6926,7 @@  static const struct hci_ev {
 	HCI_EV(HCI_EV_NUM_COMP_BLOCKS, hci_num_comp_blocks_evt,
 	       sizeof(struct hci_ev_num_comp_blocks)),
 	/* [0xff = HCI_EV_VENDOR] */
-	HCI_EV_VL(HCI_EV_VENDOR, msft_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
+	HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
 };
 
 static void hci_event_func(struct hci_dev *hdev, u8 event, struct sk_buff *skb,
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 914e2f2d3586..5e48576041fb 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4389,6 +4389,26 @@  static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
 			       MGMT_STATUS_NOT_SUPPORTED);
 }
 
+int mgmt_quality_report(struct hci_dev *hdev, void *data, u32 data_len,
+			u8 quality_spec)
+{
+	struct mgmt_ev_quality_report *ev;
+	struct sk_buff *skb;
+
+	skb = mgmt_alloc_skb(hdev, MGMT_EV_QUALITY_REPORT,
+			     sizeof(*ev) + data_len);
+	if (!skb)
+		return -ENOMEM;
+
+	ev = skb_put(skb, sizeof(*ev));
+	ev->quality_spec = quality_spec;
+	ev->data_len = data_len;
+	skb_put_data(skb, data, data_len);
+
+	return mgmt_event_skb(skb, NULL);
+}
+EXPORT_SYMBOL(mgmt_quality_report);
+
 static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
 			    u16 data_len)
 {
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 9a3d77d3ca86..3edf64baf479 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -731,6 +731,20 @@  static void msft_monitor_device_evt(struct hci_dev *hdev, struct sk_buff *skb)
 				 handle_data->mgmt_handle);
 }
 
+__u8 *get_msft_evt_prefix(struct hci_dev *hdev)
+{
+	struct msft_data *msft = hdev->msft_data;
+
+	return msft->evt_prefix;
+}
+
+__u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
+{
+	struct msft_data *msft = hdev->msft_data;
+
+	return msft->evt_prefix_len;
+}
+
 void msft_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
 {
 	struct msft_data *msft = hdev->msft_data;
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index afcaf7d3b1cb..a354ebf61fed 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -27,6 +27,8 @@  int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
 int msft_suspend_sync(struct hci_dev *hdev);
 int msft_resume_sync(struct hci_dev *hdev);
 bool msft_curve_validity(struct hci_dev *hdev);
+__u8 *get_msft_evt_prefix(struct hci_dev *hdev);
+__u8 get_msft_evt_prefix_len(struct hci_dev *hdev);
 
 #else
 
@@ -77,4 +79,14 @@  static inline bool msft_curve_validity(struct hci_dev *hdev)
 	return false;
 }
 
+static inline __u8 *get_msft_evt_prefix(struct hci_dev *hdev)
+{
+	return NULL;
+}
+
+static inline __u8 get_msft_evt_prefix_len(struct hci_dev *hdev)
+{
+	return 0;
+}
+
 #endif