Message ID | 20210815201611.v9.1.I41aec59e65ffd3226d368dabeb084af13cc133c8@changeid |
---|---|
State | Accepted |
Commit | 75ca2db346a1239d7b31a25bcdb5a1df7cc6d175 |
Headers | show |
Series | [v9,1/5] Bluetooth: btusb: disable Intel link statistics telemetry events | expand |
Hi Joseph, On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > ---Test result--- > > Test Summary: > CheckPatch PASS 2.92 seconds > GitLint PASS 0.61 seconds > BuildKernel PASS 598.41 seconds > TestRunner: Setup PASS 394.12 seconds > TestRunner: l2cap-tester PASS 2.89 seconds > TestRunner: bnep-tester PASS 2.07 seconds > TestRunner: mgmt-tester FAIL 30.94 seconds > TestRunner: rfcomm-tester PASS 2.31 seconds > TestRunner: sco-tester PASS 2.20 seconds > TestRunner: smp-tester FAIL 2.27 seconds > TestRunner: userchan-tester PASS 2.09 seconds > > Details > ############################## > Test: CheckPatch - PASS - 2.92 seconds > Run checkpatch.pl script with rule in .checkpatch.conf > > > ############################## > Test: GitLint - PASS - 0.61 seconds > Run gitlint with rule in .gitlint > > > ############################## > Test: BuildKernel - PASS - 598.41 seconds > Build Kernel with minimal configuration supports Bluetooth > > > ############################## > Test: TestRunner: Setup - PASS - 394.12 seconds > Setup environment for running Test Runner > > > ############################## > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > Run test-runner with l2cap-tester > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > Run test-runner with bnep-tester > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > Run test-runner with mgmt-tester > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > Failed Test Cases > Read Exp Feature - Success Failed 0.012 seconds Looks like there is a regression on mgmt-tester: Read Exp Feature - Success - run Sending Read Experimental Features Information (0x0049) Test condition added, total 1 Read Experimental Features Information (0x0049): Success (0x00) Invalid cmd response parameter size Read Exp Feature - Success - test failed > > ############################## > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > Run test-runner with rfcomm-tester > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: sco-tester - PASS - 2.20 seconds > Run test-runner with sco-tester > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > Run test-runner with smp-tester > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > Failed Test Cases > SMP Client - SC Request 2 Failed 0.024 seconds > > ############################## > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > Run test-runner with userchan-tester > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > --- > Regards, > Linux Bluetooth >
(Resent this email to contain only plaintext.) Hi Luiz: It seems that mgmt-tester currently uses a fixed feature count. Every time a new exp feature is added, the mgmt-tester would be broken. By checking the kernel, it seems that there are currently 2 or 3 exp features, i.e., debug uuid, simultaneous central peripheral uuid, and LL privacy uuid. Note that the debug exp feature is guarded by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is configured and made on your test setup. If we fix the mgmt-tester in bluez to have 3 or 4 (which one?) features before merging the kernel changes here that adds a new quality exp feature, it would not match the existing kernel which has only 2 or 3 features. Do you have any preference about how to fix the mgmt-tester? My suggestion is to remove the checking of the feature count from the mgmt-tester. The feature count changes over time. It is possible to implement a customized exp_feat_check function which can be more flexible. If a uuid is found in the MGMT response, its associated flags are checked against. The data currently used in the mgmt-tester: static const uint8_t read_exp_feat_param_success[] = { 0x02, 0x00, /* Feature Count */ 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */ 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */ 0xb5, 0x10, 0x1b, 0x67, 0x00, 0x00, 0x00, 0x00, /* Flags */ 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */ 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15, 0x02, 0x00, 0x00, 0x00, /* Flags */ }; Please let me know what you think. Thanks and regards, Joseph On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Joseph, > > On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > > > ---Test result--- > > > > Test Summary: > > CheckPatch PASS 2.92 seconds > > GitLint PASS 0.61 seconds > > BuildKernel PASS 598.41 seconds > > TestRunner: Setup PASS 394.12 seconds > > TestRunner: l2cap-tester PASS 2.89 seconds > > TestRunner: bnep-tester PASS 2.07 seconds > > TestRunner: mgmt-tester FAIL 30.94 seconds > > TestRunner: rfcomm-tester PASS 2.31 seconds > > TestRunner: sco-tester PASS 2.20 seconds > > TestRunner: smp-tester FAIL 2.27 seconds > > TestRunner: userchan-tester PASS 2.09 seconds > > > > Details > > ############################## > > Test: CheckPatch - PASS - 2.92 seconds > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > ############################## > > Test: GitLint - PASS - 0.61 seconds > > Run gitlint with rule in .gitlint > > > > > > ############################## > > Test: BuildKernel - PASS - 598.41 seconds > > Build Kernel with minimal configuration supports Bluetooth > > > > > > ############################## > > Test: TestRunner: Setup - PASS - 394.12 seconds > > Setup environment for running Test Runner > > > > > > ############################## > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > > Run test-runner with l2cap-tester > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > > Run test-runner with bnep-tester > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > > Run test-runner with mgmt-tester > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > > > Failed Test Cases > > Read Exp Feature - Success Failed 0.012 seconds > > Looks like there is a regression on mgmt-tester: > > Read Exp Feature - Success - run > Sending Read Experimental Features Information (0x0049) > Test condition added, total 1 > Read Experimental Features Information (0x0049): Success (0x00) > Invalid cmd response parameter size > Read Exp Feature - Success - test failed > > > > > ############################## > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > > Run test-runner with rfcomm-tester > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: sco-tester - PASS - 2.20 seconds > > Run test-runner with sco-tester > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > ############################## > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > > Run test-runner with smp-tester > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > > > Failed Test Cases > > SMP Client - SC Request 2 Failed 0.024 seconds > > > > ############################## > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > > Run test-runner with userchan-tester > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > --- > > Regards, > > Linux Bluetooth > > > > > -- > Luiz Augusto von Dentz -- Joseph Shyh-In Hwang Email: josephsih@google.com
Hi Luiz: I am wondering if it is possible to merge these kernel patches before fixing the mgmt-tester? The mgmt-tester failed due to its checking against the fixed exp feature count and the corresponding exp UUIDs and flags. A more flexible tester may be required so that the tester would not be broken whenever a new exp feature is added. Thanks and regards, Joseph On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote: > > (Resent this email to contain only plaintext.) > > Hi Luiz: > > It seems that mgmt-tester currently uses a fixed feature count. > Every time a new exp feature is added, the mgmt-tester would be > broken. By checking the kernel, it seems that there are currently 2 or > 3 exp features, i.e., debug uuid, simultaneous central peripheral > uuid, and LL privacy uuid. Note that the debug exp feature is guarded > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is > configured and made on your test setup. > > If we fix the mgmt-tester in bluez to have 3 or 4 (which one?) > features before merging the kernel changes here that adds a new > quality exp feature, it would not match the existing kernel which has > only 2 or 3 features. > > Do you have any preference about how to fix the mgmt-tester? > > My suggestion is to remove the checking of the feature count from > the mgmt-tester. The feature count changes over time. It is possible > to implement a customized exp_feat_check function which can be more > flexible. If a uuid is found in the MGMT response, its associated > flags are checked against. > > The data currently used in the mgmt-tester: > > static const uint8_t read_exp_feat_param_success[] = { > 0x02, 0x00, /* Feature Count */ > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */ > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */ > 0xb5, 0x10, 0x1b, 0x67, > 0x00, 0x00, 0x00, 0x00, /* Flags */ > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */ > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2, > 0x48, 0xa1, 0xc0, 0x15, > 0x02, 0x00, 0x00, 0x00, /* Flags */ > }; > > > Please let me know what you think. > > Thanks and regards, > Joseph > > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Joseph, > > > > On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > > > > > ---Test result--- > > > > > > Test Summary: > > > CheckPatch PASS 2.92 seconds > > > GitLint PASS 0.61 seconds > > > BuildKernel PASS 598.41 seconds > > > TestRunner: Setup PASS 394.12 seconds > > > TestRunner: l2cap-tester PASS 2.89 seconds > > > TestRunner: bnep-tester PASS 2.07 seconds > > > TestRunner: mgmt-tester FAIL 30.94 seconds > > > TestRunner: rfcomm-tester PASS 2.31 seconds > > > TestRunner: sco-tester PASS 2.20 seconds > > > TestRunner: smp-tester FAIL 2.27 seconds > > > TestRunner: userchan-tester PASS 2.09 seconds > > > > > > Details > > > ############################## > > > Test: CheckPatch - PASS - 2.92 seconds > > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > > > > ############################## > > > Test: GitLint - PASS - 0.61 seconds > > > Run gitlint with rule in .gitlint > > > > > > > > > ############################## > > > Test: BuildKernel - PASS - 598.41 seconds > > > Build Kernel with minimal configuration supports Bluetooth > > > > > > > > > ############################## > > > Test: TestRunner: Setup - PASS - 394.12 seconds > > > Setup environment for running Test Runner > > > > > > > > > ############################## > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > > > Run test-runner with l2cap-tester > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > > > ############################## > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > > > Run test-runner with bnep-tester > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > > > ############################## > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > > > Run test-runner with mgmt-tester > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > > > > > Failed Test Cases > > > Read Exp Feature - Success Failed 0.012 seconds > > > > Looks like there is a regression on mgmt-tester: > > > > Read Exp Feature - Success - run > > Sending Read Experimental Features Information (0x0049) > > Test condition added, total 1 > > Read Experimental Features Information (0x0049): Success (0x00) > > Invalid cmd response parameter size > > Read Exp Feature - Success - test failed > > > > > > > > ############################## > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > > > Run test-runner with rfcomm-tester > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > > > ############################## > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds > > > Run test-runner with sco-tester > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > > > ############################## > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > > > Run test-runner with smp-tester > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > > > > > Failed Test Cases > > > SMP Client - SC Request 2 Failed 0.024 seconds > > > > > > ############################## > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > > > Run test-runner with userchan-tester > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > --- > > > Regards, > > > Linux Bluetooth > > > > > > > > > -- > > Luiz Augusto von Dentz > > > > -- > > Joseph Shyh-In Hwang > Email: josephsih@google.com -- Joseph Shyh-In Hwang Email: josephsih@google.com
Hi Joseph, On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote: > > Hi Luiz: > > I am wondering if it is possible to merge these kernel patches > before fixing the mgmt-tester? > > The mgmt-tester failed due to its checking against the fixed exp > feature count and the corresponding exp UUIDs and flags. A more > flexible tester may be required so that the tester would not be broken > whenever a new exp feature is added. I would prefer to have the mgmt-tester changes applied as well so we don't have false positives for other patches causing mgmt-tester to fail, in fact we include CI is useful to validate this changes so we can check the feature is correctly exposed to userspace and UUID is properly formatted, etc, it shouldn't be a big change to mgmt-tester thought if you just include the new UUID later on we can think about how we could make it more extensible so it doesn't break when a new feature is added. > On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote: > > > > (Resent this email to contain only plaintext.) > > > > Hi Luiz: > > > > It seems that mgmt-tester currently uses a fixed feature count. > > Every time a new exp feature is added, the mgmt-tester would be > > broken. By checking the kernel, it seems that there are currently 2 or > > 3 exp features, i.e., debug uuid, simultaneous central peripheral > > uuid, and LL privacy uuid. Note that the debug exp feature is guarded > > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is > > configured and made on your test setup. > > > > If we fix the mgmt-tester in bluez to have 3 or 4 (which one?) > > features before merging the kernel changes here that adds a new > > quality exp feature, it would not match the existing kernel which has > > only 2 or 3 features. > > > > Do you have any preference about how to fix the mgmt-tester? > > > > My suggestion is to remove the checking of the feature count from > > the mgmt-tester. The feature count changes over time. It is possible > > to implement a customized exp_feat_check function which can be more > > flexible. If a uuid is found in the MGMT response, its associated > > flags are checked against. > > > > The data currently used in the mgmt-tester: > > > > static const uint8_t read_exp_feat_param_success[] = { > > 0x02, 0x00, /* Feature Count */ > > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */ > > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */ > > 0xb5, 0x10, 0x1b, 0x67, > > 0x00, 0x00, 0x00, 0x00, /* Flags */ > > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */ > > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2, > > 0x48, 0xa1, 0xc0, 0x15, > > 0x02, 0x00, 0x00, 0x00, /* Flags */ > > }; > > > > > > Please let me know what you think. > > > > Thanks and regards, > > Joseph > > > > > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > Hi Joseph, > > > > > > On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > > > > > > > ---Test result--- > > > > > > > > Test Summary: > > > > CheckPatch PASS 2.92 seconds > > > > GitLint PASS 0.61 seconds > > > > BuildKernel PASS 598.41 seconds > > > > TestRunner: Setup PASS 394.12 seconds > > > > TestRunner: l2cap-tester PASS 2.89 seconds > > > > TestRunner: bnep-tester PASS 2.07 seconds > > > > TestRunner: mgmt-tester FAIL 30.94 seconds > > > > TestRunner: rfcomm-tester PASS 2.31 seconds > > > > TestRunner: sco-tester PASS 2.20 seconds > > > > TestRunner: smp-tester FAIL 2.27 seconds > > > > TestRunner: userchan-tester PASS 2.09 seconds > > > > > > > > Details > > > > ############################## > > > > Test: CheckPatch - PASS - 2.92 seconds > > > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > > > > > > > ############################## > > > > Test: GitLint - PASS - 0.61 seconds > > > > Run gitlint with rule in .gitlint > > > > > > > > > > > > ############################## > > > > Test: BuildKernel - PASS - 598.41 seconds > > > > Build Kernel with minimal configuration supports Bluetooth > > > > > > > > > > > > ############################## > > > > Test: TestRunner: Setup - PASS - 394.12 seconds > > > > Setup environment for running Test Runner > > > > > > > > > > > > ############################## > > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > > > > Run test-runner with l2cap-tester > > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > ############################## > > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > > > > Run test-runner with bnep-tester > > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > ############################## > > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > > > > Run test-runner with mgmt-tester > > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > > > > > > > Failed Test Cases > > > > Read Exp Feature - Success Failed 0.012 seconds > > > > > > Looks like there is a regression on mgmt-tester: > > > > > > Read Exp Feature - Success - run > > > Sending Read Experimental Features Information (0x0049) > > > Test condition added, total 1 > > > Read Experimental Features Information (0x0049): Success (0x00) > > > Invalid cmd response parameter size > > > Read Exp Feature - Success - test failed > > > > > > > > > > > ############################## > > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > > > > Run test-runner with rfcomm-tester > > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > ############################## > > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds > > > > Run test-runner with sco-tester > > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > ############################## > > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > > > > Run test-runner with smp-tester > > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > > > > > > > Failed Test Cases > > > > SMP Client - SC Request 2 Failed 0.024 seconds > > > > > > > > ############################## > > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > > > > Run test-runner with userchan-tester > > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > > > > > --- > > > > Regards, > > > > Linux Bluetooth > > > > > > > > > > > > > -- > > > Luiz Augusto von Dentz > > > > > > > > -- > > > > Joseph Shyh-In Hwang > > Email: josephsih@google.com > > > > -- > > Joseph Shyh-In Hwang > Email: josephsih@google.com -- Luiz Augusto von Dentz
Hi Luiz: The mgmt-tester change to add the UUID of the new exp feature is just sent for review: https://patchwork.kernel.org/project/bluetooth/patch/20210820131751.BlueZ.v1.1.I165b6fc2b20d80c8d18946434005f0269d92f489@changeid/ The tester checks the exp features UUIDs and FLAGs. As the FLAG values depend on how the test is set up. I set the flags to be all 0x00 which passed the mgmt-tester in my local test setup. If that is not the case upstream, please let me know. Thanks and regards, Joseph On Fri, Aug 20, 2021 at 2:04 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Joseph, > > On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote: > > > > Hi Luiz: > > > > I am wondering if it is possible to merge these kernel patches > > before fixing the mgmt-tester? > > > > The mgmt-tester failed due to its checking against the fixed exp > > feature count and the corresponding exp UUIDs and flags. A more > > flexible tester may be required so that the tester would not be broken > > whenever a new exp feature is added. > > I would prefer to have the mgmt-tester changes applied as well so we > don't have false positives for other patches causing mgmt-tester to > fail, in fact we include CI is useful to validate this changes so we > can check the feature is correctly exposed to userspace and UUID is > properly formatted, etc, it shouldn't be a big change to mgmt-tester > thought if you just include the new UUID later on we can think about > how we could make it more extensible so it doesn't break when a new > feature is added. > > > On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote: > > > > > > (Resent this email to contain only plaintext.) > > > > > > Hi Luiz: > > > > > > It seems that mgmt-tester currently uses a fixed feature count. > > > Every time a new exp feature is added, the mgmt-tester would be > > > broken. By checking the kernel, it seems that there are currently 2 or > > > 3 exp features, i.e., debug uuid, simultaneous central peripheral > > > uuid, and LL privacy uuid. Note that the debug exp feature is guarded > > > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is > > > configured and made on your test setup. > > > > > > If we fix the mgmt-tester in bluez to have 3 or 4 (which one?) > > > features before merging the kernel changes here that adds a new > > > quality exp feature, it would not match the existing kernel which has > > > only 2 or 3 features. > > > > > > Do you have any preference about how to fix the mgmt-tester? > > > > > > My suggestion is to remove the checking of the feature count from > > > the mgmt-tester. The feature count changes over time. It is possible > > > to implement a customized exp_feat_check function which can be more > > > flexible. If a uuid is found in the MGMT response, its associated > > > flags are checked against. > > > > > > The data currently used in the mgmt-tester: > > > > > > static const uint8_t read_exp_feat_param_success[] = { > > > 0x02, 0x00, /* Feature Count */ > > > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */ > > > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */ > > > 0xb5, 0x10, 0x1b, 0x67, > > > 0x00, 0x00, 0x00, 0x00, /* Flags */ > > > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */ > > > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2, > > > 0x48, 0xa1, 0xc0, 0x15, > > > 0x02, 0x00, 0x00, 0x00, /* Flags */ > > > }; > > > > > > > > > Please let me know what you think. > > > > > > Thanks and regards, > > > Joseph > > > > > > > > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > Hi Joseph, > > > > > > > > On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > > > > > > > > > ---Test result--- > > > > > > > > > > Test Summary: > > > > > CheckPatch PASS 2.92 seconds > > > > > GitLint PASS 0.61 seconds > > > > > BuildKernel PASS 598.41 seconds > > > > > TestRunner: Setup PASS 394.12 seconds > > > > > TestRunner: l2cap-tester PASS 2.89 seconds > > > > > TestRunner: bnep-tester PASS 2.07 seconds > > > > > TestRunner: mgmt-tester FAIL 30.94 seconds > > > > > TestRunner: rfcomm-tester PASS 2.31 seconds > > > > > TestRunner: sco-tester PASS 2.20 seconds > > > > > TestRunner: smp-tester FAIL 2.27 seconds > > > > > TestRunner: userchan-tester PASS 2.09 seconds > > > > > > > > > > Details > > > > > ############################## > > > > > Test: CheckPatch - PASS - 2.92 seconds > > > > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > > > > > > > > > > ############################## > > > > > Test: GitLint - PASS - 0.61 seconds > > > > > Run gitlint with rule in .gitlint > > > > > > > > > > > > > > > ############################## > > > > > Test: BuildKernel - PASS - 598.41 seconds > > > > > Build Kernel with minimal configuration supports Bluetooth > > > > > > > > > > > > > > > ############################## > > > > > Test: TestRunner: Setup - PASS - 394.12 seconds > > > > > Setup environment for running Test Runner > > > > > > > > > > > > > > > ############################## > > > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > > > > > Run test-runner with l2cap-tester > > > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > ############################## > > > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > > > > > Run test-runner with bnep-tester > > > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > ############################## > > > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > > > > > Run test-runner with mgmt-tester > > > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > > > > > > > > > Failed Test Cases > > > > > Read Exp Feature - Success Failed 0.012 seconds > > > > > > > > Looks like there is a regression on mgmt-tester: > > > > > > > > Read Exp Feature - Success - run > > > > Sending Read Experimental Features Information (0x0049) > > > > Test condition added, total 1 > > > > Read Experimental Features Information (0x0049): Success (0x00) > > > > Invalid cmd response parameter size > > > > Read Exp Feature - Success - test failed > > > > > > > > > > > > > > ############################## > > > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > > > > > Run test-runner with rfcomm-tester > > > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > ############################## > > > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds > > > > > Run test-runner with sco-tester > > > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > ############################## > > > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > > > > > Run test-runner with smp-tester > > > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > > > > > > > > > Failed Test Cases > > > > > SMP Client - SC Request 2 Failed 0.024 seconds > > > > > > > > > > ############################## > > > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > > > > > Run test-runner with userchan-tester > > > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > > > > > > > > > --- > > > > > Regards, > > > > > Linux Bluetooth > > > > > > > > > > > > > > > > > -- > > > > Luiz Augusto von Dentz > > > > > > > > > > > > -- > > > > > > Joseph Shyh-In Hwang > > > Email: josephsih@google.com > > > > > > > > -- > > > > Joseph Shyh-In Hwang > > Email: josephsih@google.com > > > > -- > Luiz Augusto von Dentz -- Joseph Shyh-In Hwang Email: josephsih@google.com
(Resent in plaintext mode) Hi Luiz: I have fixed the mgmt-tester on the bluez side. The "Read Exp Feature - Success" test passed on my local test setup. Please let me know if there are any issues. Thanks and regards, Joseph On Fri, Aug 20, 2021 at 1:27 PM Joseph Hwang <josephsih@google.com> wrote: > > Hi Luiz: > > The mgmt-tester change to add the UUID of the new exp feature is just > sent for review: > > https://patchwork.kernel.org/project/bluetooth/patch/20210820131751.BlueZ.v1.1.I165b6fc2b20d80c8d18946434005f0269d92f489@changeid/ > > The tester checks the exp features UUIDs and FLAGs. As the FLAG > values depend on how the test is set up. I set the flags to be all > 0x00 which passed the mgmt-tester in my local test setup. If that is > not the case upstream, please let me know. > > Thanks and regards, > Joseph > > On Fri, Aug 20, 2021 at 2:04 AM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Joseph, > > > > On Tue, Aug 17, 2021 at 10:49 PM Joseph Hwang <josephsih@google.com> wrote: > > > > > > Hi Luiz: > > > > > > I am wondering if it is possible to merge these kernel patches > > > before fixing the mgmt-tester? > > > > > > The mgmt-tester failed due to its checking against the fixed exp > > > feature count and the corresponding exp UUIDs and flags. A more > > > flexible tester may be required so that the tester would not be broken > > > whenever a new exp feature is added. > > > > I would prefer to have the mgmt-tester changes applied as well so we > > don't have false positives for other patches causing mgmt-tester to > > fail, in fact we include CI is useful to validate this changes so we > > can check the feature is correctly exposed to userspace and UUID is > > properly formatted, etc, it shouldn't be a big change to mgmt-tester > > thought if you just include the new UUID later on we can think about > > how we could make it more extensible so it doesn't break when a new > > feature is added. > > > > > On Tue, Aug 17, 2021 at 5:54 PM Joseph Hwang <josephsih@google.com> wrote: > > > > > > > > (Resent this email to contain only plaintext.) > > > > > > > > Hi Luiz: > > > > > > > > It seems that mgmt-tester currently uses a fixed feature count. > > > > Every time a new exp feature is added, the mgmt-tester would be > > > > broken. By checking the kernel, it seems that there are currently 2 or > > > > 3 exp features, i.e., debug uuid, simultaneous central peripheral > > > > uuid, and LL privacy uuid. Note that the debug exp feature is guarded > > > > by CONFIG_BT_FEATURE_DEBUG. So I am not sure how the kernel is > > > > configured and made on your test setup. > > > > > > > > If we fix the mgmt-tester in bluez to have 3 or 4 (which one?) > > > > features before merging the kernel changes here that adds a new > > > > quality exp feature, it would not match the existing kernel which has > > > > only 2 or 3 features. > > > > > > > > Do you have any preference about how to fix the mgmt-tester? > > > > > > > > My suggestion is to remove the checking of the feature count from > > > > the mgmt-tester. The feature count changes over time. It is possible > > > > to implement a customized exp_feat_check function which can be more > > > > flexible. If a uuid is found in the MGMT response, its associated > > > > flags are checked against. > > > > > > > > The data currently used in the mgmt-tester: > > > > > > > > static const uint8_t read_exp_feat_param_success[] = { > > > > 0x02, 0x00, /* Feature Count */ > > > > 0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, /* UUID - Simultaneous */ > > > > 0x27, 0x92, 0x96, 0x46, 0xc0, 0x42, /* Central Peripheral */ > > > > 0xb5, 0x10, 0x1b, 0x67, > > > > 0x00, 0x00, 0x00, 0x00, /* Flags */ > > > > 0x04, 0x00, 0x13, 0xac, 0x42, 0x02, /* UUID - LL Privacy */ > > > > 0xde, 0xb3, 0xea, 0x11, 0x73, 0xc2, > > > > 0x48, 0xa1, 0xc0, 0x15, > > > > 0x02, 0x00, 0x00, 0x00, /* Flags */ > > > > }; > > > > > > > > > > > > Please let me know what you think. > > > > > > > > Thanks and regards, > > > > Joseph > > > > > > > > > > > > On Tue, Aug 17, 2021 at 1:49 AM Luiz Augusto von Dentz > > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > > > Hi Joseph, > > > > > > > > > > On Sun, Aug 15, 2021 at 6:25 AM <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=531683 > > > > > > > > > > > > ---Test result--- > > > > > > > > > > > > Test Summary: > > > > > > CheckPatch PASS 2.92 seconds > > > > > > GitLint PASS 0.61 seconds > > > > > > BuildKernel PASS 598.41 seconds > > > > > > TestRunner: Setup PASS 394.12 seconds > > > > > > TestRunner: l2cap-tester PASS 2.89 seconds > > > > > > TestRunner: bnep-tester PASS 2.07 seconds > > > > > > TestRunner: mgmt-tester FAIL 30.94 seconds > > > > > > TestRunner: rfcomm-tester PASS 2.31 seconds > > > > > > TestRunner: sco-tester PASS 2.20 seconds > > > > > > TestRunner: smp-tester FAIL 2.27 seconds > > > > > > TestRunner: userchan-tester PASS 2.09 seconds > > > > > > > > > > > > Details > > > > > > ############################## > > > > > > Test: CheckPatch - PASS - 2.92 seconds > > > > > > Run checkpatch.pl script with rule in .checkpatch.conf > > > > > > > > > > > > > > > > > > ############################## > > > > > > Test: GitLint - PASS - 0.61 seconds > > > > > > Run gitlint with rule in .gitlint > > > > > > > > > > > > > > > > > > ############################## > > > > > > Test: BuildKernel - PASS - 598.41 seconds > > > > > > Build Kernel with minimal configuration supports Bluetooth > > > > > > > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: Setup - PASS - 394.12 seconds > > > > > > Setup environment for running Test Runner > > > > > > > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: l2cap-tester - PASS - 2.89 seconds > > > > > > Run test-runner with l2cap-tester > > > > > > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: bnep-tester - PASS - 2.07 seconds > > > > > > Run test-runner with bnep-tester > > > > > > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: mgmt-tester - FAIL - 30.94 seconds > > > > > > Run test-runner with mgmt-tester > > > > > > Total: 448, Passed: 444 (99.1%), Failed: 1, Not Run: 3 > > > > > > > > > > > > Failed Test Cases > > > > > > Read Exp Feature - Success Failed 0.012 seconds > > > > > > > > > > Looks like there is a regression on mgmt-tester: > > > > > > > > > > Read Exp Feature - Success - run > > > > > Sending Read Experimental Features Information (0x0049) > > > > > Test condition added, total 1 > > > > > Read Experimental Features Information (0x0049): Success (0x00) > > > > > Invalid cmd response parameter size > > > > > Read Exp Feature - Success - test failed > > > > > > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: rfcomm-tester - PASS - 2.31 seconds > > > > > > Run test-runner with rfcomm-tester > > > > > > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: sco-tester - PASS - 2.20 seconds > > > > > > Run test-runner with sco-tester > > > > > > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: smp-tester - FAIL - 2.27 seconds > > > > > > Run test-runner with smp-tester > > > > > > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > > > > > > > > > > > Failed Test Cases > > > > > > SMP Client - SC Request 2 Failed 0.024 seconds > > > > > > > > > > > > ############################## > > > > > > Test: TestRunner: userchan-tester - PASS - 2.09 seconds > > > > > > Run test-runner with userchan-tester > > > > > > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > Regards, > > > > > > Linux Bluetooth > > > > > > > > > > > > > > > > > > > > > -- > > > > > Luiz Augusto von Dentz > > > > > > > > > > > > > > > > -- > > > > > > > > Joseph Shyh-In Hwang > > > > Email: josephsih@google.com > > > > > > > > > > > > -- > > > > > > Joseph Shyh-In Hwang > > > Email: josephsih@google.com > > > > > > > > -- > > Luiz Augusto von Dentz > > > > -- > > Joseph Shyh-In Hwang > Email: josephsih@google.com -- Joseph Shyh-In Hwang Email: josephsih@google.com
Hi Joseph, On Wed, Aug 25, 2021 at 5:02 PM Joseph Hwang <josephsih@google.com> wrote: > > (Resent in plaintext mode) > > Hi Luiz: > > I have fixed the mgmt-tester on the bluez side. The "Read Exp > Feature - Success" test passed on my local test setup. > > Please let me know if there are any issues. Applied, thanks.
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index f1705b46fc88..0fe093fa5158 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c @@ -1893,7 +1893,6 @@ static int btintel_bootloader_setup(struct hci_dev *hdev, u32 boot_param; char ddcname[64]; int err; - struct intel_debug_features features; BT_DBG("%s", hdev->name); @@ -1934,15 +1933,6 @@ static int btintel_bootloader_setup(struct hci_dev *hdev, btintel_load_ddc_config(hdev, ddcname); } - /* Read the Intel supported features and if new exception formats - * supported, need to load the additional DDC config to enable. - */ - err = btintel_read_debug_features(hdev, &features); - if (!err) { - /* Set DDC mask for available debug features */ - btintel_set_debug_features(hdev, &features); - } - /* Read the Intel version information after loading the FW */ err = btintel_read_version(hdev, &new_ver); if (err) @@ -2089,7 +2079,6 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, u32 boot_param; char ddcname[64]; int err; - struct intel_debug_features features; struct intel_version_tlv new_ver; bt_dev_dbg(hdev, ""); @@ -2125,15 +2114,6 @@ static int btintel_bootloader_setup_tlv(struct hci_dev *hdev, */ btintel_load_ddc_config(hdev, ddcname); - /* Read the Intel supported features and if new exception formats - * supported, need to load the additional DDC config to enable. - */ - err = btintel_read_debug_features(hdev, &features); - if (!err) { - /* Set DDC mask for available debug features */ - btintel_set_debug_features(hdev, &features); - } - /* Read the Intel version information after loading the FW */ err = btintel_read_version_tlv(hdev, &new_ver); if (err)