All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt
@ 2022-01-21 11:22 Joseph Hwang
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Joseph Hwang @ 2022-01-21 11:22 UTC (permalink / raw)
  To: linux-bluetooth, marcel, luiz.dentz, pali
  Cc: chromeos-bluetooth-upstreaming, josephsih, Joseph Hwang,
	Archie Pusaka, David S. Miller, Jakub Kicinski, Johan Hedberg,
	linux-kernel, netdev

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

 include/net/bluetooth/hci_core.h |  2 ++
 include/net/bluetooth/mgmt.h     |  7 ++++
 net/bluetooth/aosp.c             | 61 ++++++++++++++++++++++++++++++++
 net/bluetooth/aosp.h             | 12 +++++++
 net/bluetooth/hci_event.c        | 33 ++++++++++++++++-
 net/bluetooth/mgmt.c             | 22 ++++++++++++
 6 files changed, 136 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 21eadb113a31..727cb9c056b2 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1861,6 +1861,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, struct sk_buff *skb,
+			u8 quality_spec);
 
 u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
 		      u16 to_multiplier);
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 99266f7aebdc..6a0fcb3aef8a 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;
+	__u8 data_len;
+	__u8 data[0];
+} __packed;
diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
index 432ae3aac9e3..9e3551627ad5 100644
--- a/net/bluetooth/aosp.c
+++ b/net/bluetooth/aosp.c
@@ -199,3 +199,64 @@ int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
 	else
 		return disable_quality_report(hdev);
 }
+
+#define BLUETOOTH_QUALITY_REPORT_EV		0x58
+struct bqr_data {
+	__u8 quality_report_id;
+	__u8 packet_type;
+	__le16 conn_handle;
+	__u8 conn_role;
+	__s8 tx_power_level;
+	__s8 rssi;
+	__u8 snr;
+	__u8 unused_afh_channel_count;
+	__u8 afh_select_unideal_channel_count;
+	__le16 lsto;
+	__le32 conn_piconet_clock;
+	__le32 retransmission_count;
+	__le32 no_rx_count;
+	__le32 nak_count;
+	__le32 last_tx_ack_timestamp;
+	__le32 flow_off_count;
+	__le32 last_flow_on_timestamp;
+	__le32 buffer_overflow_bytes;
+	__le32 buffer_underflow_bytes;
+
+	/* Vendor Specific Parameter */
+	__u8 vsp[0];
+} __packed;
+
+struct aosp_hci_vs_data {
+	__u8 code;
+	__u8 data[0];
+} __packed;
+
+bool aosp_is_quality_report_evt(struct sk_buff *skb)
+{
+	struct aosp_hci_vs_data *ev;
+
+	if (skb->len < sizeof(struct aosp_hci_vs_data))
+		return false;
+
+	ev = (struct aosp_hci_vs_data *)skb->data;
+
+	return ev->code == BLUETOOTH_QUALITY_REPORT_EV;
+}
+
+bool aosp_pull_quality_report_data(struct sk_buff *skb)
+{
+	size_t bqr_data_len = sizeof(struct bqr_data);
+
+	skb_pull(skb, sizeof(struct aosp_hci_vs_data));
+
+	/* skb->len is allowed to be larger than bqr_data_len to have
+	 * the Vendor Specific Parameter (vsp) field.
+	 */
+	if (skb->len < bqr_data_len) {
+		BT_ERR("AOSP evt data len %d too short (%u expected)",
+		       skb->len, bqr_data_len);
+		return false;
+	}
+
+	return true;
+}
diff --git a/net/bluetooth/aosp.h b/net/bluetooth/aosp.h
index 2fd8886d51b2..49894a995647 100644
--- a/net/bluetooth/aosp.h
+++ b/net/bluetooth/aosp.h
@@ -10,6 +10,8 @@ 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_is_quality_report_evt(struct sk_buff *skb);
+bool aosp_pull_quality_report_data(struct sk_buff *skb);
 
 #else
 
@@ -26,4 +28,14 @@ static inline int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
 	return -EOPNOTSUPP;
 }
 
+static inline bool aosp_is_quality_report_evt(struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline bool aosp_pull_quality_report_data(struct sk_buff *skb)
+{
+	return false;
+}
+
 #endif
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 681c623aa380..bccb659a9454 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"
@@ -4225,6 +4226,36 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
 	queue_work(hdev->workqueue, &hdev->tx_work);
 }
 
+#define QUALITY_SPEC_NA			0x0
+#define QUALITY_SPEC_INTEL_TELEMETRY	0x1
+#define QUALITY_SPEC_AOSP_BQR		0x2
+
+static bool quality_report_evt(struct hci_dev *hdev,  void *data,
+			       struct sk_buff *skb)
+{
+	if (aosp_is_quality_report_evt(skb)) {
+		if (aosp_has_quality_report(hdev) &&
+		    aosp_pull_quality_report_data(skb))
+			mgmt_quality_report(hdev, skb, QUALITY_SPEC_AOSP_BQR);
+
+		return true;
+	}
+
+	return false;
+}
+
+static void hci_vendor_evt(struct hci_dev *hdev, void *data,
+			   struct sk_buff *skb)
+{
+	/* Every distinct vendor specification must have a well-defined
+	 * condition to determine if an event meets the specification.
+	 * The skb is consumed by a specification only if the event meets
+	 * the specification.
+	 */
+	if (!quality_report_evt(hdev, data, skb))
+		msft_vendor_evt(hdev, data, skb);
+}
+
 static void hci_mode_change_evt(struct hci_dev *hdev, void *data,
 				struct sk_buff *skb)
 {
@@ -6811,7 +6842,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(HCI_EV_VENDOR, msft_vendor_evt, 0),
+	HCI_EV(HCI_EV_VENDOR, hci_vendor_evt, 0),
 };
 
 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 08d6494f1b34..78687ae885be 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4389,6 +4389,28 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
 			       MGMT_STATUS_NOT_SUPPORTED);
 }
 
+int mgmt_quality_report(struct hci_dev *hdev, struct sk_buff *skb,
+			u8 quality_spec)
+{
+	struct mgmt_ev_quality_report *ev;
+	size_t ev_len;
+	int err;
+
+	/* The ev comes with a variable-length data field. */
+	ev_len = sizeof(*ev) + skb->len;
+	ev = kmalloc(ev_len, GFP_KERNEL);
+	if (!ev)
+		return -ENOMEM;
+
+	ev->quality_spec = quality_spec;
+	ev->data_len = skb->len;
+	memcpy(ev->data, skb->data, skb->len);
+	err = mgmt_event(MGMT_EV_QUALITY_REPORT, hdev, ev, ev_len, NULL);
+	kfree(ev);
+
+	return err;
+}
+
 static int get_device_flags(struct sock *sk, struct hci_dev *hdev, void *data,
 			    u16 data_len)
 {
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
  2022-01-21 11:22 [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt Joseph Hwang
@ 2022-01-21 11:22 ` Joseph Hwang
  2022-01-21 18:30     ` kernel test robot
                     ` (2 more replies)
  2022-01-21 11:57 ` [v1,1/2] Bluetooth: aosp: surface AOSP quality report " bluez.test.bot
  2022-01-21 20:16 ` [PATCH v1 1/2] " Marcel Holtmann
  2 siblings, 3 replies; 9+ messages in thread
From: Joseph Hwang @ 2022-01-21 11:22 UTC (permalink / raw)
  To: linux-bluetooth, marcel, luiz.dentz, pali
  Cc: chromeos-bluetooth-upstreaming, josephsih, Joseph Hwang,
	Archie Pusaka, David S. Miller, Jakub Kicinski, Johan Hedberg,
	linux-kernel, netdev

When receiving a HCI vendor event, the kernel checks if it is an
Intel telemetry event. 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>
---

 drivers/bluetooth/btintel.c      | 43 +++++++++++++++++++++++++++++++-
 drivers/bluetooth/btintel.h      | 12 +++++++++
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_event.c        | 12 ++++++---
 4 files changed, 65 insertions(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 1a4f8b227eac..d3b7a796cb91 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -2401,8 +2401,10 @@ static int btintel_setup_combined(struct hci_dev *hdev)
 	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
 	set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
 
-	/* Set up the quality report callback for Intel devices */
+	/* Set up the quality report callbacks for Intel devices */
 	hdev->set_quality_report = btintel_set_quality_report;
+	hdev->is_quality_report_evt = btintel_is_quality_report_evt;
+	hdev->pull_quality_report_data = btintel_pull_quality_report_data;
 
 	/* For Legacy device, check the HW platform value and size */
 	if (skb->len == sizeof(ver) && skb->data[1] == 0x37) {
@@ -2645,6 +2647,45 @@ void btintel_secure_send_result(struct hci_dev *hdev,
 }
 EXPORT_SYMBOL_GPL(btintel_secure_send_result);
 
+#define INTEL_PREFIX		0x8087
+#define TELEMETRY_CODE		0x03
+
+struct intel_prefix_evt_data {
+	__le16 vendor_prefix;
+	__u8 code;
+	__u8 data[0];   /* a number of struct intel_tlv subevents */
+} __packed;
+
+bool btintel_is_quality_report_evt(struct sk_buff *skb)
+{
+	struct intel_prefix_evt_data *ev;
+	u16 vendor_prefix;
+
+	if (skb->len < sizeof(struct intel_prefix_evt_data))
+		return false;
+
+	ev = (struct intel_prefix_evt_data *)skb->data;
+	vendor_prefix = __le16_to_cpu(ev->vendor_prefix);
+
+	return vendor_prefix == INTEL_PREFIX && ev->code == TELEMETRY_CODE;
+}
+EXPORT_SYMBOL_GPL(btintel_is_quality_report_evt);
+
+bool btintel_pull_quality_report_data(struct sk_buff *skb)
+{
+	skb_pull(skb, sizeof(struct intel_prefix_evt_data));
+
+	/* A telemetry event contains at least one intel_tlv subevent. */
+	if (skb->len < sizeof(struct intel_tlv)) {
+		BT_ERR("Telemetry event length %d too short (at least %u)",
+		       skb->len, sizeof(struct intel_tlv));
+		return false;
+	}
+
+	return true;
+}
+EXPORT_SYMBOL_GPL(btintel_pull_quality_report_data);
+
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
 MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index c9b24e9299e2..841aef3dbd4c 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -210,6 +210,8 @@ void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
 void btintel_secure_send_result(struct hci_dev *hdev,
 				const void *ptr, unsigned int len);
 int btintel_set_quality_report(struct hci_dev *hdev, bool enable);
+bool btintel_is_quality_report_evt(struct sk_buff *skb);
+bool btintel_pull_quality_report_data(struct sk_buff *skb);
 #else
 
 static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -305,4 +307,14 @@ static inline int btintel_set_quality_report(struct hci_dev *hdev, bool enable)
 {
 	return -ENODEV;
 }
+
+static inline bool btintel_is_quality_report_evt(struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
+{
+	return false;
+}
 #endif
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 727cb9c056b2..b74ba1585df9 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -632,6 +632,8 @@ struct hci_dev {
 	void (*cmd_timeout)(struct hci_dev *hdev);
 	bool (*wakeup)(struct hci_dev *hdev);
 	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
+	bool (*is_quality_report_evt)(struct sk_buff *skb);
+	bool (*pull_quality_report_data)(struct sk_buff *skb);
 	int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path);
 	int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
 				     struct bt_codec *codec, __u8 *vnd_len,
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index bccb659a9454..5f9cc7b942a1 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4237,11 +4237,17 @@ static bool quality_report_evt(struct hci_dev *hdev,  void *data,
 		if (aosp_has_quality_report(hdev) &&
 		    aosp_pull_quality_report_data(skb))
 			mgmt_quality_report(hdev, skb, QUALITY_SPEC_AOSP_BQR);
-
-		return true;
+	} else if (hdev->is_quality_report_evt &&
+		   hdev->is_quality_report_evt(skb)) {
+		if (hdev->set_quality_report &&
+		    hdev->pull_quality_report_data(skb))
+			mgmt_quality_report(hdev, skb,
+					    QUALITY_SPEC_INTEL_TELEMETRY);
+	} else {
+		return false;
 	}
 
-	return false;
+	return true;
 }
 
 static void hci_vendor_evt(struct hci_dev *hdev, void *data,
-- 
2.35.0.rc0.227.g00780c9af4-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [v1,1/2] Bluetooth: aosp: surface AOSP quality report through mgmt
  2022-01-21 11:22 [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt Joseph Hwang
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
@ 2022-01-21 11:57 ` bluez.test.bot
  2022-01-21 20:16 ` [PATCH v1 1/2] " Marcel Holtmann
  2 siblings, 0 replies; 9+ messages in thread
From: bluez.test.bot @ 2022-01-21 11:57 UTC (permalink / raw)
  To: linux-bluetooth, josephsih

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

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=607231

---Test result---

Test Summary:
CheckPatch                    PASS      4.77 seconds
GitLint                       PASS      1.99 seconds
SubjectPrefix                 PASS      1.69 seconds
BuildKernel                   PASS      36.40 seconds
BuildKernel32                 PASS      32.88 seconds
Incremental Build with patchesPASS      74.26 seconds
TestRunner: Setup             PASS      564.84 seconds
TestRunner: l2cap-tester      PASS      15.78 seconds
TestRunner: bnep-tester       PASS      7.40 seconds
TestRunner: mgmt-tester       PASS      126.11 seconds
TestRunner: rfcomm-tester     PASS      9.67 seconds
TestRunner: sco-tester        PASS      10.10 seconds
TestRunner: smp-tester        PASS      9.89 seconds
TestRunner: userchan-tester   PASS      8.40 seconds



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
@ 2022-01-21 18:30     ` kernel test robot
  2022-01-21 20:21   ` Marcel Holtmann
  2022-01-22  7:31     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-01-21 18:30 UTC (permalink / raw)
  To: Joseph Hwang, linux-bluetooth, marcel, luiz.dentz, pali
  Cc: kbuild-all, chromeos-bluetooth-upstreaming, josephsih,
	Joseph Hwang, Archie Pusaka, Jakub Kicinski, Johan Hedberg

Hi Joseph,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20220121]
[cannot apply to net-next/master net/master bluetooth/master v5.16]
[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/20220121-192436
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: ia64-randconfig-r035-20220121 (https://download.01.org/0day-ci/archive/20220122/202201220238.tZ1BfUpc-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9484eb7f87f7c8759b6fd7eec9d431c375b97432
        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/20220121-192436
        git checkout 9484eb7f87f7c8759b6fd7eec9d431c375b97432
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/bluetooth/

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

All warnings (new ones prefixed by >>):

   In file included from arch/ia64/include/asm/pgtable.h:153,
                    from include/linux/pgtable.h:6,
                    from arch/ia64/include/asm/uaccess.h:40,
                    from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/rcuwait.h:6,
                    from include/linux/percpu-rwsem.h:7,
                    from include/linux/fs.h:33,
                    from include/linux/poll.h:10,
                    from include/net/bluetooth/bluetooth.h:28,
                    from drivers/bluetooth/btintel.c:14:
   arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
   arch/ia64/include/asm/mmu_context.h:127:48: warning: variable 'old_rr4' set but not used [-Wunused-but-set-variable]
     127 |         unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
         |                                                ^~~~~~~
   In file included from drivers/bluetooth/btintel.c:14:
   drivers/bluetooth/btintel.c: In function 'btintel_pull_quality_report_data':
>> drivers/bluetooth/btintel.c:2680:24: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
    2680 |                 BT_ERR("Telemetry event length %d too short (at least %u)",
         |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    2681 |                        skb->len, sizeof(struct intel_tlv));
         |                                  ~~~~~~~~~~~~~~~~~~~~~~~~
         |                                  |
         |                                  long unsigned int
   include/net/bluetooth/bluetooth.h:199:40: note: in definition of macro 'BT_ERR'
     199 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   drivers/bluetooth/btintel.c:2680:72: note: format string is defined here
    2680 |                 BT_ERR("Telemetry event length %d too short (at least %u)",
         |                                                                       ~^
         |                                                                        |
         |                                                                        unsigned int
         |                                                                       %lu


vim +2680 drivers/bluetooth/btintel.c

  2673	
  2674	bool btintel_pull_quality_report_data(struct sk_buff *skb)
  2675	{
  2676		skb_pull(skb, sizeof(struct intel_prefix_evt_data));
  2677	
  2678		/* A telemetry event contains at least one intel_tlv subevent. */
  2679		if (skb->len < sizeof(struct intel_tlv)) {
> 2680			BT_ERR("Telemetry event length %d too short (at least %u)",
  2681			       skb->len, sizeof(struct intel_tlv));
  2682			return false;
  2683		}
  2684	
  2685		return true;
  2686	}
  2687	EXPORT_SYMBOL_GPL(btintel_pull_quality_report_data);
  2688	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
@ 2022-01-21 18:30     ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-01-21 18:30 UTC (permalink / raw)
  To: kbuild-all

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

Hi Joseph,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on next-20220121]
[cannot apply to net-next/master net/master bluetooth/master v5.16]
[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/20220121-192436
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: ia64-randconfig-r035-20220121 (https://download.01.org/0day-ci/archive/20220122/202201220238.tZ1BfUpc-lkp(a)intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9484eb7f87f7c8759b6fd7eec9d431c375b97432
        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/20220121-192436
        git checkout 9484eb7f87f7c8759b6fd7eec9d431c375b97432
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/bluetooth/

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

All warnings (new ones prefixed by >>):

   In file included from arch/ia64/include/asm/pgtable.h:153,
                    from include/linux/pgtable.h:6,
                    from arch/ia64/include/asm/uaccess.h:40,
                    from include/linux/uaccess.h:11,
                    from include/linux/sched/task.h:11,
                    from include/linux/sched/signal.h:9,
                    from include/linux/rcuwait.h:6,
                    from include/linux/percpu-rwsem.h:7,
                    from include/linux/fs.h:33,
                    from include/linux/poll.h:10,
                    from include/net/bluetooth/bluetooth.h:28,
                    from drivers/bluetooth/btintel.c:14:
   arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
   arch/ia64/include/asm/mmu_context.h:127:48: warning: variable 'old_rr4' set but not used [-Wunused-but-set-variable]
     127 |         unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
         |                                                ^~~~~~~
   In file included from drivers/bluetooth/btintel.c:14:
   drivers/bluetooth/btintel.c: In function 'btintel_pull_quality_report_data':
>> drivers/bluetooth/btintel.c:2680:24: warning: format '%u' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int' [-Wformat=]
    2680 |                 BT_ERR("Telemetry event length %d too short (at least %u)",
         |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    2681 |                        skb->len, sizeof(struct intel_tlv));
         |                                  ~~~~~~~~~~~~~~~~~~~~~~~~
         |                                  |
         |                                  long unsigned int
   include/net/bluetooth/bluetooth.h:199:40: note: in definition of macro 'BT_ERR'
     199 | #define BT_ERR(fmt, ...)        bt_err(fmt "\n", ##__VA_ARGS__)
         |                                        ^~~
   drivers/bluetooth/btintel.c:2680:72: note: format string is defined here
    2680 |                 BT_ERR("Telemetry event length %d too short (at least %u)",
         |                                                                       ~^
         |                                                                        |
         |                                                                        unsigned int
         |                                                                       %lu


vim +2680 drivers/bluetooth/btintel.c

  2673	
  2674	bool btintel_pull_quality_report_data(struct sk_buff *skb)
  2675	{
  2676		skb_pull(skb, sizeof(struct intel_prefix_evt_data));
  2677	
  2678		/* A telemetry event contains at least one intel_tlv subevent. */
  2679		if (skb->len < sizeof(struct intel_tlv)) {
> 2680			BT_ERR("Telemetry event length %d too short (at least %u)",
  2681			       skb->len, sizeof(struct intel_tlv));
  2682			return false;
  2683		}
  2684	
  2685		return true;
  2686	}
  2687	EXPORT_SYMBOL_GPL(btintel_pull_quality_report_data);
  2688	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt
  2022-01-21 11:22 [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt Joseph Hwang
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
  2022-01-21 11:57 ` [v1,1/2] Bluetooth: aosp: surface AOSP quality report " bluez.test.bot
@ 2022-01-21 20:16 ` Marcel Holtmann
  2 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2022-01-21 20:16 UTC (permalink / raw)
  To: Joseph Hwang
  Cc: linux-bluetooth, Luiz Augusto von Dentz, Pali Rohár,
	CrosBT Upstreaming, Joseph Hwang, Archie Pusaka, David S. Miller,
	Jakub Kicinski, Johan Hedberg, LKML, netdev

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>
> ---
> 
> include/net/bluetooth/hci_core.h |  2 ++
> include/net/bluetooth/mgmt.h     |  7 ++++
> net/bluetooth/aosp.c             | 61 ++++++++++++++++++++++++++++++++
> net/bluetooth/aosp.h             | 12 +++++++
> net/bluetooth/hci_event.c        | 33 ++++++++++++++++-
> net/bluetooth/mgmt.c             | 22 ++++++++++++
> 6 files changed, 136 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 21eadb113a31..727cb9c056b2 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -1861,6 +1861,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, struct sk_buff *skb,
> +			u8 quality_spec);
> 
> u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
> 		      u16 to_multiplier);
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 99266f7aebdc..6a0fcb3aef8a 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;
> +	__u8 data_len;
> +	__u8 data[0];
> +} __packed;
> diff --git a/net/bluetooth/aosp.c b/net/bluetooth/aosp.c
> index 432ae3aac9e3..9e3551627ad5 100644
> --- a/net/bluetooth/aosp.c
> +++ b/net/bluetooth/aosp.c
> @@ -199,3 +199,64 @@ int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> 	else
> 		return disable_quality_report(hdev);
> }
> +
> +#define BLUETOOTH_QUALITY_REPORT_EV		0x58
> +struct bqr_data {
> +	__u8 quality_report_id;
> +	__u8 packet_type;
> +	__le16 conn_handle;
> +	__u8 conn_role;
> +	__s8 tx_power_level;
> +	__s8 rssi;
> +	__u8 snr;
> +	__u8 unused_afh_channel_count;
> +	__u8 afh_select_unideal_channel_count;
> +	__le16 lsto;
> +	__le32 conn_piconet_clock;
> +	__le32 retransmission_count;
> +	__le32 no_rx_count;
> +	__le32 nak_count;
> +	__le32 last_tx_ack_timestamp;
> +	__le32 flow_off_count;
> +	__le32 last_flow_on_timestamp;
> +	__le32 buffer_overflow_bytes;
> +	__le32 buffer_underflow_bytes;
> +
> +	/* Vendor Specific Parameter */
> +	__u8 vsp[0];
> +} __packed;
> +
> +struct aosp_hci_vs_data {
> +	__u8 code;
> +	__u8 data[0];
> +} __packed;

unless you need these two for something, scrap them. You can define constants for the size check.

> +
> +bool aosp_is_quality_report_evt(struct sk_buff *skb)
> +{
> +	struct aosp_hci_vs_data *ev;
> +
> +	if (skb->len < sizeof(struct aosp_hci_vs_data))
> +		return false;
> +
> +	ev = (struct aosp_hci_vs_data *)skb->data;
> +
> +	return ev->code == BLUETOOTH_QUALITY_REPORT_EV;
> +}
> +
> +bool aosp_pull_quality_report_data(struct sk_buff *skb)
> +{
> +	size_t bqr_data_len = sizeof(struct bqr_data);
> +
> +	skb_pull(skb, sizeof(struct aosp_hci_vs_data));
> +
> +	/* skb->len is allowed to be larger than bqr_data_len to have
> +	 * the Vendor Specific Parameter (vsp) field.
> +	 */
> +	if (skb->len < bqr_data_len) {
> +		BT_ERR("AOSP evt data len %d too short (%u expected)",
> +		       skb->len, bqr_data_len);
> +		return false;
> +	}
> +
> +	return true;
> +}

This part I find a bit convoluted, just do a basic length check and then move on. The kernel has no interest in this data.

> diff --git a/net/bluetooth/aosp.h b/net/bluetooth/aosp.h
> index 2fd8886d51b2..49894a995647 100644
> --- a/net/bluetooth/aosp.h
> +++ b/net/bluetooth/aosp.h
> @@ -10,6 +10,8 @@ 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_is_quality_report_evt(struct sk_buff *skb);
> +bool aosp_pull_quality_report_data(struct sk_buff *skb);
> 
> #else
> 
> @@ -26,4 +28,14 @@ static inline int aosp_set_quality_report(struct hci_dev *hdev, bool enable)
> 	return -EOPNOTSUPP;
> }
> 
> +static inline bool aosp_is_quality_report_evt(struct sk_buff *skb)
> +{
> +	return false;
> +}
> +
> +static inline bool aosp_pull_quality_report_data(struct sk_buff *skb)
> +{
> +	return false;
> +}
> +
> #endif
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 681c623aa380..bccb659a9454 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"
> @@ -4225,6 +4226,36 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
> 	queue_work(hdev->workqueue, &hdev->tx_work);
> }
> 
> +#define QUALITY_SPEC_NA			0x0
> +#define QUALITY_SPEC_INTEL_TELEMETRY	0x1
> +#define QUALITY_SPEC_AOSP_BQR		0x2
> +
> +static bool quality_report_evt(struct hci_dev *hdev,  void *data,
> +			       struct sk_buff *skb)
> +{
> +	if (aosp_is_quality_report_evt(skb)) {
> +		if (aosp_has_quality_report(hdev) &&
> +		    aosp_pull_quality_report_data(skb))
> +			mgmt_quality_report(hdev, skb, QUALITY_SPEC_AOSP_BQR);
> +
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void hci_vendor_evt(struct hci_dev *hdev, void *data,
> +			   struct sk_buff *skb)
> +{
> +	/* Every distinct vendor specification must have a well-defined
> +	 * condition to determine if an event meets the specification.
> +	 * The skb is consumed by a specification only if the event meets
> +	 * the specification.
> +	 */
> +	if (!quality_report_evt(hdev, data, skb))
> +		msft_vendor_evt(hdev, data, skb);
> +}

No, not like this. This gets messy really quickly.

We should allow for defining vendor event prefixes here. That AOSP decided to convolute the space 0x54 and above in unfortunate, but that is what we have to deal with.

> +
> static void hci_mode_change_evt(struct hci_dev *hdev, void *data,
> 				struct sk_buff *skb)
> {
> @@ -6811,7 +6842,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(HCI_EV_VENDOR, msft_vendor_evt, 0),
> +	HCI_EV(HCI_EV_VENDOR, hci_vendor_evt, 0),
> };
> 
> 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 08d6494f1b34..78687ae885be 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4389,6 +4389,28 @@ static int set_exp_feature(struct sock *sk, struct hci_dev *hdev,
> 			       MGMT_STATUS_NOT_SUPPORTED);
> }
> 
> +int mgmt_quality_report(struct hci_dev *hdev, struct sk_buff *skb,
> +			u8 quality_spec)
> +{
> +	struct mgmt_ev_quality_report *ev;
> +	size_t ev_len;
> +	int err;
> +
> +	/* The ev comes with a variable-length data field. */
> +	ev_len = sizeof(*ev) + skb->len;
> +	ev = kmalloc(ev_len, GFP_KERNEL);
> +	if (!ev)
> +		return -ENOMEM;
> +
> +	ev->quality_spec = quality_spec;
> +	ev->data_len = skb->len;
> +	memcpy(ev->data, skb->data, skb->len);
> +	err = mgmt_event(MGMT_EV_QUALITY_REPORT, hdev, ev, ev_len, NULL);
> +	kfree(ev);
> +
> +	return err;
> +}
> +

Don’t we have mgmt helper functions that allow us to add headers to a mgmt skb. I think there is really no point in allocating memory via kmalloc.

Regards

Marcel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
  2022-01-21 18:30     ` kernel test robot
@ 2022-01-21 20:21   ` Marcel Holtmann
  2022-01-22  7:31     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: Marcel Holtmann @ 2022-01-21 20:21 UTC (permalink / raw)
  To: Joseph Hwang
  Cc: linux-bluetooth, Luiz Augusto von Dentz, pali,
	chromeos-bluetooth-upstreaming, josephsih, Archie Pusaka,
	David S. Miller, Jakub Kicinski, Johan Hedberg, linux-kernel,
	netdev

Hi Jospeh,

> When receiving a HCI vendor event, the kernel checks if it is an
> Intel telemetry event. 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>
> ---
> 
> drivers/bluetooth/btintel.c      | 43 +++++++++++++++++++++++++++++++-
> drivers/bluetooth/btintel.h      | 12 +++++++++
> include/net/bluetooth/hci_core.h |  2 ++
> net/bluetooth/hci_event.c        | 12 ++++++---
> 4 files changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 1a4f8b227eac..d3b7a796cb91 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -2401,8 +2401,10 @@ static int btintel_setup_combined(struct hci_dev *hdev)
> 	set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> 	set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> 
> -	/* Set up the quality report callback for Intel devices */
> +	/* Set up the quality report callbacks for Intel devices */
> 	hdev->set_quality_report = btintel_set_quality_report;
> +	hdev->is_quality_report_evt = btintel_is_quality_report_evt;
> +	hdev->pull_quality_report_data = btintel_pull_quality_report_data;

we are not doing this. This is all internal handling. Don’t bother the core hci_dev with it.

> 
> 	/* For Legacy device, check the HW platform value and size */
> 	if (skb->len == sizeof(ver) && skb->data[1] == 0x37) {
> @@ -2645,6 +2647,45 @@ void btintel_secure_send_result(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_secure_send_result);
> 
> +#define INTEL_PREFIX		0x8087
> +#define TELEMETRY_CODE		0x03
> +
> +struct intel_prefix_evt_data {
> +	__le16 vendor_prefix;
> +	__u8 code;
> +	__u8 data[0];   /* a number of struct intel_tlv subevents */
> +} __packed;
> +
> +bool btintel_is_quality_report_evt(struct sk_buff *skb)
> +{
> +	struct intel_prefix_evt_data *ev;
> +	u16 vendor_prefix;
> +
> +	if (skb->len < sizeof(struct intel_prefix_evt_data))
> +		return false;
> +
> +	ev = (struct intel_prefix_evt_data *)skb->data;
> +	vendor_prefix = __le16_to_cpu(ev->vendor_prefix);
> +
> +	return vendor_prefix == INTEL_PREFIX && ev->code == TELEMETRY_CODE;
> +}
> +EXPORT_SYMBOL_GPL(btintel_is_quality_report_evt);
> +
> +bool btintel_pull_quality_report_data(struct sk_buff *skb)
> +{
> +	skb_pull(skb, sizeof(struct intel_prefix_evt_data));
> +
> +	/* A telemetry event contains at least one intel_tlv subevent. */
> +	if (skb->len < sizeof(struct intel_tlv)) {
> +		BT_ERR("Telemetry event length %d too short (at least %u)",
> +		       skb->len, sizeof(struct intel_tlv));
> +		return false;
> +	}
> +
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(btintel_pull_quality_report_data);
> +
> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> MODULE_VERSION(VERSION);
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index c9b24e9299e2..841aef3dbd4c 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -210,6 +210,8 @@ void btintel_bootup(struct hci_dev *hdev, const void *ptr, unsigned int len);
> void btintel_secure_send_result(struct hci_dev *hdev,
> 				const void *ptr, unsigned int len);
> int btintel_set_quality_report(struct hci_dev *hdev, bool enable);
> +bool btintel_is_quality_report_evt(struct sk_buff *skb);
> +bool btintel_pull_quality_report_data(struct sk_buff *skb);
> #else
> 
> static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -305,4 +307,14 @@ static inline int btintel_set_quality_report(struct hci_dev *hdev, bool enable)
> {
> 	return -ENODEV;
> }
> +
> +static inline bool btintel_is_quality_report_evt(struct sk_buff *skb)
> +{
> +	return false;
> +}
> +
> +static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
> +{
> +	return false;
> +}
> #endif
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 727cb9c056b2..b74ba1585df9 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -632,6 +632,8 @@ struct hci_dev {
> 	void (*cmd_timeout)(struct hci_dev *hdev);
> 	bool (*wakeup)(struct hci_dev *hdev);
> 	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
> +	bool (*is_quality_report_evt)(struct sk_buff *skb);
> +	bool (*pull_quality_report_data)(struct sk_buff *skb);
> 	int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path);
> 	int (*get_codec_config_data)(struct hci_dev *hdev, __u8 type,
> 				     struct bt_codec *codec, __u8 *vnd_len,
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index bccb659a9454..5f9cc7b942a1 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -4237,11 +4237,17 @@ static bool quality_report_evt(struct hci_dev *hdev,  void *data,
> 		if (aosp_has_quality_report(hdev) &&
> 		    aosp_pull_quality_report_data(skb))
> 			mgmt_quality_report(hdev, skb, QUALITY_SPEC_AOSP_BQR);
> -
> -		return true;
> +	} else if (hdev->is_quality_report_evt &&
> +		   hdev->is_quality_report_evt(skb)) {
> +		if (hdev->set_quality_report &&
> +		    hdev->pull_quality_report_data(skb))
> +			mgmt_quality_report(hdev, skb,
> +					    QUALITY_SPEC_INTEL_TELEMETRY);
> +	} else {
> +		return false;
> 	}

No. You now have Intel internal details bleeding into the core.

Regards

Marcel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
  2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
@ 2022-01-22  7:31     ` kernel test robot
  2022-01-21 20:21   ` Marcel Holtmann
  2022-01-22  7:31     ` kernel test robot
  2 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-01-22  7:31 UTC (permalink / raw)
  To: Joseph Hwang, linux-bluetooth, marcel, luiz.dentz, pali
  Cc: kbuild-all, chromeos-bluetooth-upstreaming, josephsih,
	Joseph Hwang, Archie Pusaka, Jakub Kicinski, Johan Hedberg

Hi Joseph,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on next-20220121]
[cannot apply to net-next/master net/master bluetooth/master v5.16]
[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/20220121-192436
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220122/202201221555.iq4k2aDR-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/9484eb7f87f7c8759b6fd7eec9d431c375b97432
        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/20220121-192436
        git checkout 9484eb7f87f7c8759b6fd7eec9d431c375b97432
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

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

All errors (new ones prefixed by >>):

   In file included from drivers/bluetooth/hci_ldisc.c:34:
>> drivers/bluetooth/btintel.h:317:1: error: expected identifier or '(' before '{' token
     317 | {
         | ^
   drivers/bluetooth/btintel.h:316:20: warning: 'btintel_pull_quality_report_data' declared 'static' but never defined [-Wunused-function]
     316 | static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +317 drivers/bluetooth/btintel.h

   315	
   316	static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
 > 317	{

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events through mgmt
@ 2022-01-22  7:31     ` kernel test robot
  0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2022-01-22  7:31 UTC (permalink / raw)
  To: kbuild-all

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

Hi Joseph,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bluetooth-next/master]
[also build test ERROR on next-20220121]
[cannot apply to net-next/master net/master bluetooth/master v5.16]
[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/20220121-192436
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220122/202201221555.iq4k2aDR-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/9484eb7f87f7c8759b6fd7eec9d431c375b97432
        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/20220121-192436
        git checkout 9484eb7f87f7c8759b6fd7eec9d431c375b97432
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

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

All errors (new ones prefixed by >>):

   In file included from drivers/bluetooth/hci_ldisc.c:34:
>> drivers/bluetooth/btintel.h:317:1: error: expected identifier or '(' before '{' token
     317 | {
         | ^
   drivers/bluetooth/btintel.h:316:20: warning: 'btintel_pull_quality_report_data' declared 'static' but never defined [-Wunused-function]
     316 | static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +317 drivers/bluetooth/btintel.h

   315	
   316	static inline bool btintel_pull_quality_report_data(struct sk_buff *skb);
 > 317	{

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-22  7:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-21 11:22 [PATCH v1 1/2] Bluetooth: aosp: surface AOSP quality report through mgmt Joseph Hwang
2022-01-21 11:22 ` [PATCH v1 2/2] Bluetooth: btintel: surface Intel telemetry events " Joseph Hwang
2022-01-21 18:30   ` kernel test robot
2022-01-21 18:30     ` kernel test robot
2022-01-21 20:21   ` Marcel Holtmann
2022-01-22  7:31   ` kernel test robot
2022-01-22  7:31     ` kernel test robot
2022-01-21 11:57 ` [v1,1/2] Bluetooth: aosp: surface AOSP quality report " bluez.test.bot
2022-01-21 20:16 ` [PATCH v1 1/2] " Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.