| Message ID | 20210608184457.3069064-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: HCI: Fix Set Extended (Scan Response) Data | expand |
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=496525 ---Test result--- Test Summary: CheckPatch PASS 1.03 seconds GitLint PASS 0.10 seconds BuildKernel PASS 512.94 seconds TestRunner: Setup PASS 336.29 seconds TestRunner: l2cap-tester PASS 2.59 seconds TestRunner: bnep-tester PASS 1.87 seconds TestRunner: mgmt-tester FAIL 29.48 seconds TestRunner: rfcomm-tester PASS 2.03 seconds TestRunner: sco-tester PASS 1.99 seconds TestRunner: smp-tester PASS 2.06 seconds TestRunner: userchan-tester PASS 1.88 seconds Details ############################## Test: CheckPatch - PASS - 1.03 seconds Run checkpatch.pl script with rule in .checkpatch.conf ############################## Test: GitLint - PASS - 0.10 seconds Run gitlint with rule in .gitlint ############################## Test: BuildKernel - PASS - 512.94 seconds Build Kernel with minimal configuration supports Bluetooth ############################## Test: TestRunner: Setup - PASS - 336.29 seconds Setup environment for running Test Runner ############################## Test: TestRunner: l2cap-tester - PASS - 2.59 seconds Run test-runner with l2cap-tester Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: bnep-tester - PASS - 1.87 seconds Run test-runner with bnep-tester Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: mgmt-tester - FAIL - 29.48 seconds Run test-runner with mgmt-tester Total: 446, Passed: 415 (93.0%), Failed: 18, Not Run: 13 Failed Test Cases Add Ext Advertising - Success 1 (Powered, Add Adv Inst) Failed 0.015 seconds Add Ext Advertising - Success 2 (!Powered, Add Adv Inst) Failed 0.021 seconds Add Ext Advertising - Success 5 (Set Adv off override) Failed 0.024 seconds Add Ext Advertising - Success 6 (Scan Rsp Dta, Adv ok) Failed 0.019 seconds Add Ext Advertising - Success 7 (Scan Rsp Dta, Scan ok) Failed 0.016 seconds Add Ext Advertising - Success 9 (General Discov Flag) Failed 0.020 seconds Add Ext Advertising - Success 10 (Limited Discov Flag) Failed 0.017 seconds Add Ext Advertising - Success 11 (Managed Flags) Failed 0.023 seconds Add Ext Advertising - Success 12 (TX Power Flag) Failed 0.017 seconds Add Ext Advertising - Success 20 (Add Adv override) Failed 0.018 seconds Add Ext Advertising - Success (Name is null) Failed 0.015 seconds Add Ext Advertising - Success (Complete name) Failed 0.024 seconds Add Ext Advertising - Success (Shortened name) Failed 0.024 seconds Add Ext Advertising - Success (Short name) Failed 0.018 seconds Add Ext Advertising - Success (Name + data) Failed 0.023 seconds Add Ext Advertising - Success (Name+data+appear) Failed 0.025 seconds Ext Adv MGMT - AD Data (5.0) Success Failed 0.022 seconds Ext Adv MGMT - AD Scan Response (5.0) Success Failed 0.020 seconds ############################## Test: TestRunner: rfcomm-tester - PASS - 2.03 seconds Run test-runner with rfcomm-tester Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: sco-tester - PASS - 1.99 seconds Run test-runner with sco-tester Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: smp-tester - PASS - 2.06 seconds Run test-runner with smp-tester Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: userchan-tester - PASS - 1.88 seconds Run test-runner with userchan-tester Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 --- Regards, Linux Bluetooth
Hi Tedd, On Tue, Jun 8, 2021 at 12:42 PM <bluez.test.bot@gmail.com> wrote: > > 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=496525 > > ---Test result--- > > Test Summary: > CheckPatch PASS 1.03 seconds > GitLint PASS 0.10 seconds > BuildKernel PASS 512.94 seconds > TestRunner: Setup PASS 336.29 seconds > TestRunner: l2cap-tester PASS 2.59 seconds > TestRunner: bnep-tester PASS 1.87 seconds > TestRunner: mgmt-tester FAIL 29.48 seconds > TestRunner: rfcomm-tester PASS 2.03 seconds > TestRunner: sco-tester PASS 1.99 seconds > TestRunner: smp-tester PASS 2.06 seconds > TestRunner: userchan-tester PASS 1.88 seconds > > Details > ############################## > Test: CheckPatch - PASS - 1.03 seconds > Run checkpatch.pl script with rule in .checkpatch.conf > > > ############################## > Test: GitLint - PASS - 0.10 seconds > Run gitlint with rule in .gitlint > > > ############################## > Test: BuildKernel - PASS - 512.94 seconds > Build Kernel with minimal configuration supports Bluetooth > > > ############################## > Test: TestRunner: Setup - PASS - 336.29 seconds > Setup environment for running Test Runner > > > ############################## > Test: TestRunner: l2cap-tester - PASS - 2.59 seconds > Run test-runner with l2cap-tester > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: bnep-tester - PASS - 1.87 seconds > Run test-runner with bnep-tester > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: mgmt-tester - FAIL - 29.48 seconds > Run test-runner with mgmt-tester > Total: 446, Passed: 415 (93.0%), Failed: 18, Not Run: 13 > > Failed Test Cases > Add Ext Advertising - Success 1 (Powered, Add Adv Inst) Failed 0.015 seconds > Add Ext Advertising - Success 2 (!Powered, Add Adv Inst) Failed 0.021 seconds > Add Ext Advertising - Success 5 (Set Adv off override) Failed 0.024 seconds > Add Ext Advertising - Success 6 (Scan Rsp Dta, Adv ok) Failed 0.019 seconds > Add Ext Advertising - Success 7 (Scan Rsp Dta, Scan ok) Failed 0.016 seconds > Add Ext Advertising - Success 9 (General Discov Flag) Failed 0.020 seconds > Add Ext Advertising - Success 10 (Limited Discov Flag) Failed 0.017 seconds > Add Ext Advertising - Success 11 (Managed Flags) Failed 0.023 seconds > Add Ext Advertising - Success 12 (TX Power Flag) Failed 0.017 seconds > Add Ext Advertising - Success 20 (Add Adv override) Failed 0.018 seconds > Add Ext Advertising - Success (Name is null) Failed 0.015 seconds > Add Ext Advertising - Success (Complete name) Failed 0.024 seconds > Add Ext Advertising - Success (Shortened name) Failed 0.024 seconds > Add Ext Advertising - Success (Short name) Failed 0.018 seconds > Add Ext Advertising - Success (Name + data) Failed 0.023 seconds > Add Ext Advertising - Success (Name+data+appear) Failed 0.025 seconds > Ext Adv MGMT - AD Data (5.0) Success Failed 0.022 seconds > Ext Adv MGMT - AD Scan Response (5.0) Success Failed 0.020 seconds We will probably need to fix these once the patch is merged, it looks like they are always expecting 31 bytes of data but the extended version this is variable size. > > ############################## > Test: TestRunner: rfcomm-tester - PASS - 2.03 seconds > Run test-runner with rfcomm-tester > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: sco-tester - PASS - 1.99 seconds > Run test-runner with sco-tester > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: smp-tester - PASS - 2.06 seconds > Run test-runner with smp-tester > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: userchan-tester - PASS - 1.88 seconds > Run test-runner with userchan-tester > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > --- > Regards, > Linux Bluetooth >
Hi Luiz, On Tue, 2021-06-08 at 12:57 -0700, Luiz Augusto von Dentz wrote: > Hi Tedd, > > On Tue, Jun 8, 2021 at 12:42 PM <bluez.test.bot@gmail.com> wrote: > > 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=496525 > > > > ---Test result--- > > > > Test Summary: > > CheckPatch PASS 1.03 seconds > > GitLint PASS 0.10 seconds > > BuildKernel PASS 512.94 seconds > > TestRunner: Setup PASS 336.29 seconds > > TestRunner: l2cap-tester PASS 2.59 seconds > > TestRunner: bnep-tester PASS 1.87 seconds > > TestRunner: mgmt-tester FAIL 29.48 seconds > > TestRunner: rfcomm-tester PASS 2.03 seconds > > TestRunner: sco-tester PASS 1.99 seconds > > TestRunner: smp-tester PASS 2.06 seconds > > TestRunner: userchan-tester PASS 1.88 seconds > > > > Details > > ############################## > > Test: CheckPatch - PASS - 1.03 seconds > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > ############################## > > Test: GitLint - PASS - 0.10 seconds > > Run gitlint with rule in .gitlint > > > > > > ############################## > > Test: BuildKernel - PASS - 512.94 seconds > > Build Kernel with minimal configuration supports Bluetooth > > > > > > ############################## > > Test: TestRunner: Setup - PASS - 336.29 seconds > > Setup environment for running Test Runner > > > > > > ############################## > > Test: TestRunner: l2cap-tester - PASS - 2.59 seconds > > Run test-runner with l2cap-tester > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: bnep-tester - PASS - 1.87 seconds > > Run test-runner with bnep-tester > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: mgmt-tester - FAIL - 29.48 seconds > > Run test-runner with mgmt-tester > > Total: 446, Passed: 415 (93.0%), Failed: 18, Not Run: 13 > > > > Failed Test Cases > > Add Ext Advertising - Success 1 (Powered, Add Adv Inst) Failed 0.015 seconds > > Add Ext Advertising - Success 2 (!Powered, Add Adv Inst) Failed 0.021 seconds > > Add Ext Advertising - Success 5 (Set Adv off override) Failed 0.024 seconds > > Add Ext Advertising - Success 6 (Scan Rsp Dta, Adv ok) Failed 0.019 seconds > > Add Ext Advertising - Success 7 (Scan Rsp Dta, Scan ok) Failed 0.016 seconds > > Add Ext Advertising - Success 9 (General Discov Flag) Failed 0.020 seconds > > Add Ext Advertising - Success 10 (Limited Discov Flag) Failed 0.017 seconds > > Add Ext Advertising - Success 11 (Managed Flags) Failed 0.023 seconds > > Add Ext Advertising - Success 12 (TX Power Flag) Failed 0.017 seconds > > Add Ext Advertising - Success 20 (Add Adv override) Failed 0.018 seconds > > Add Ext Advertising - Success (Name is null) Failed 0.015 seconds > > Add Ext Advertising - Success (Complete name) Failed 0.024 seconds > > Add Ext Advertising - Success (Shortened name) Failed 0.024 seconds > > Add Ext Advertising - Success (Short name) Failed 0.018 seconds > > Add Ext Advertising - Success (Name + data) Failed 0.023 seconds > > Add Ext Advertising - Success (Name+data+appear) Failed 0.025 seconds > > Ext Adv MGMT - AD Data (5.0) Success Failed 0.022 seconds > > Ext Adv MGMT - AD Scan Response (5.0) Success Failed 0.020 seconds > > We will probably need to fix these once the patch is merged, it looks > like they are always expecting 31 bytes of data but the extended > version this is variable size. > I will check if I can fix it even before merging this patch. > > ############################## > > Test: TestRunner: rfcomm-tester - PASS - 2.03 seconds > > Run test-runner with rfcomm-tester > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: sco-tester - PASS - 1.99 seconds > > Run test-runner with sco-tester > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: smp-tester - PASS - 2.06 seconds > > Run test-runner with smp-tester > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: userchan-tester - PASS - 1.88 seconds > > Run test-runner with userchan-tester > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > --- > > Regards, > > Linux Bluetooth > > > > Regards, Tedd
Hi Luiz, > These command do have variable length and the length can go up to 251, > so this changes the struct to not use a fixed size and then when > creating the PDU only the actual length of the data send to the > controller. > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > include/net/bluetooth/hci.h | 6 ++-- > include/net/bluetooth/hci_core.h | 8 ++--- > net/bluetooth/hci_request.c | 51 ++++++++++++++++++-------------- > 3 files changed, 37 insertions(+), 28 deletions(-) if possible, please git blame and add Fixes: tag. Regards Marcel
Hi Marcel, On Tue, Jun 8, 2021 at 9:52 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Luiz, > > > These command do have variable length and the length can go up to 251, > > so this changes the struct to not use a fixed size and then when > > creating the PDU only the actual length of the data send to the > > controller. > > > > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > --- > > include/net/bluetooth/hci.h | 6 ++-- > > include/net/bluetooth/hci_core.h | 8 ++--- > > net/bluetooth/hci_request.c | 51 ++++++++++++++++++-------------- > > 3 files changed, 37 insertions(+), 28 deletions(-) > > if possible, please git blame and add Fixes: tag. > > Regards > > Marcel Will do.
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 479adbde6db4..cfd4e40594d1 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -1775,13 +1775,15 @@ struct hci_cp_ext_adv_set { __u8 max_events; } __packed; +#define HCI_MAX_EXT_AD_LENGTH 251 + #define HCI_OP_LE_SET_EXT_ADV_DATA 0x2037 struct hci_cp_le_set_ext_adv_data { __u8 handle; __u8 operation; __u8 frag_pref; __u8 length; - __u8 data[HCI_MAX_AD_LENGTH]; + __u8 data[]; } __packed; #define HCI_OP_LE_SET_EXT_SCAN_RSP_DATA 0x2038 @@ -1790,7 +1792,7 @@ struct hci_cp_le_set_ext_scan_rsp_data { __u8 operation; __u8 frag_pref; __u8 length; - __u8 data[HCI_MAX_AD_LENGTH]; + __u8 data[]; } __packed; #define LE_SET_ADV_DATA_OP_COMPLETE 0x03 diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 212f46806ce7..a53e94459ecd 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -228,9 +228,9 @@ struct adv_info { __u16 remaining_time; __u16 duration; __u16 adv_data_len; - __u8 adv_data[HCI_MAX_AD_LENGTH]; + __u8 adv_data[HCI_MAX_EXT_AD_LENGTH]; __u16 scan_rsp_len; - __u8 scan_rsp_data[HCI_MAX_AD_LENGTH]; + __u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH]; __s8 tx_power; __u32 min_interval; __u32 max_interval; @@ -551,9 +551,9 @@ struct hci_dev { DECLARE_BITMAP(dev_flags, __HCI_NUM_FLAGS); __s8 adv_tx_power; - __u8 adv_data[HCI_MAX_AD_LENGTH]; + __u8 adv_data[HCI_MAX_EXT_AD_LENGTH]; __u8 adv_data_len; - __u8 scan_rsp_data[HCI_MAX_AD_LENGTH]; + __u8 scan_rsp_data[HCI_MAX_EXT_AD_LENGTH]; __u8 scan_rsp_data_len; struct list_head adv_instances; diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c index f7a9d97f3e84..1d14adc023e9 100644 --- a/net/bluetooth/hci_request.c +++ b/net/bluetooth/hci_request.c @@ -1716,30 +1716,33 @@ void __hci_req_update_scan_rsp_data(struct hci_request *req, u8 instance) return; if (ext_adv_capable(hdev)) { - struct hci_cp_le_set_ext_scan_rsp_data cp; + struct { + struct hci_cp_le_set_ext_scan_rsp_data cp; + u8 data[HCI_MAX_EXT_AD_LENGTH]; + } pdu; - memset(&cp, 0, sizeof(cp)); + memset(&pdu, 0, sizeof(pdu)); if (instance) len = create_instance_scan_rsp_data(hdev, instance, - cp.data); + pdu.data); else - len = create_default_scan_rsp_data(hdev, cp.data); + len = create_default_scan_rsp_data(hdev, pdu.data); if (hdev->scan_rsp_data_len == len && - !memcmp(cp.data, hdev->scan_rsp_data, len)) + !memcmp(pdu.data, hdev->scan_rsp_data, len)) return; - memcpy(hdev->scan_rsp_data, cp.data, sizeof(cp.data)); + memcpy(hdev->scan_rsp_data, pdu.data, len); hdev->scan_rsp_data_len = len; - cp.handle = instance; - cp.length = len; - cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; - cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; + pdu.cp.handle = instance; + pdu.cp.length = len; + pdu.cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; + pdu.cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; - hci_req_add(req, HCI_OP_LE_SET_EXT_SCAN_RSP_DATA, sizeof(cp), - &cp); + hci_req_add(req, HCI_OP_LE_SET_EXT_SCAN_RSP_DATA, + sizeof(pdu.cp) + len, &pdu.cp); } else { struct hci_cp_le_set_scan_rsp_data cp; @@ -1862,26 +1865,30 @@ void __hci_req_update_adv_data(struct hci_request *req, u8 instance) return; if (ext_adv_capable(hdev)) { - struct hci_cp_le_set_ext_adv_data cp; + struct { + struct hci_cp_le_set_ext_adv_data cp; + u8 data[HCI_MAX_EXT_AD_LENGTH]; + } pdu; - memset(&cp, 0, sizeof(cp)); + memset(&pdu, 0, sizeof(pdu)); - len = create_instance_adv_data(hdev, instance, cp.data); + len = create_instance_adv_data(hdev, instance, pdu.data); /* There's nothing to do if the data hasn't changed */ if (hdev->adv_data_len == len && - memcmp(cp.data, hdev->adv_data, len) == 0) + memcmp(pdu.data, hdev->adv_data, len) == 0) return; - memcpy(hdev->adv_data, cp.data, sizeof(cp.data)); + memcpy(hdev->adv_data, pdu.data, len); hdev->adv_data_len = len; - cp.length = len; - cp.handle = instance; - cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; - cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; + pdu.cp.length = len; + pdu.cp.handle = instance; + pdu.cp.operation = LE_SET_ADV_DATA_OP_COMPLETE; + pdu.cp.frag_pref = LE_SET_ADV_DATA_NO_FRAG; - hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_DATA, sizeof(cp), &cp); + hci_req_add(req, HCI_OP_LE_SET_EXT_ADV_DATA, + sizeof(pdu.cp) + len, &pdu.cp); } else { struct hci_cp_le_set_adv_data cp;