Message ID | 20220412120945.28862-1-vasyl.vavrychuk@opensynergy.com |
---|---|
State | New |
Headers | show |
Series | [RFC] Bluetooth: core: Allow bind HCI socket user channel when HCI is UP. | 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=631489 ---Test result--- Test Summary: CheckPatch FAIL 1.67 seconds GitLint FAIL 1.02 seconds SubjectPrefix PASS 0.82 seconds BuildKernel PASS 30.11 seconds BuildKernel32 PASS 27.80 seconds Incremental Build with patchesPASS 39.08 seconds TestRunner: Setup PASS 471.62 seconds TestRunner: l2cap-tester PASS 16.22 seconds TestRunner: bnep-tester PASS 6.26 seconds TestRunner: mgmt-tester PASS 106.06 seconds TestRunner: rfcomm-tester PASS 8.22 seconds TestRunner: sco-tester PASS 7.99 seconds TestRunner: smp-tester PASS 7.98 seconds TestRunner: userchan-tester FAIL 6.66 seconds Details ############################## Test: CheckPatch - FAIL - 1.67 seconds Run checkpatch.pl script with rule in .checkpatch.conf [RFC] Bluetooth: core: Allow bind HCI socket user channel when HCI is UP.\WARNING:FROM_SIGN_OFF_MISMATCH: From:/Signed-off-by: email address mismatch: 'From: Vasyl Vavrychuk <vvavrychuk@gmail.com>' != 'Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com>' total: 0 errors, 1 warnings, 0 checks, 10 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/12810739.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ############################## Test: GitLint - FAIL - 1.02 seconds Run gitlint with rule in .gitlint [RFC] Bluetooth: core: Allow bind HCI socket user channel when HCI is UP. 1: T3 Title has trailing punctuation (.): "[RFC] Bluetooth: core: Allow bind HCI socket user channel when HCI is UP." ############################## Test: TestRunner: userchan-tester - FAIL - 6.66 seconds Run test-runner with userchan-tester Total: 4, Passed: 3 (75.0%), Failed: 1, Not Run: 0 Failed Test Cases User channel open - Failed Failed 0.082 seconds --- Regards, Linux Bluetooth
Hi Vasyl, > This is needed for user-space to ensure that HCI init scheduled from > hci_register_dev is completed. > > Function hci_register_dev queues power_on workqueue which will run > hci_power_on > hci_dev_do_open. Function hci_dev_do_open sets HCI_INIT > for some time. > > It is not allowed to bind to HCI socket user channel when HCI_INIT is > set. As result, bind might fail when user-space program is run early > enough during boot. > > Now, user-space program can first issue HCIDEVUP ioctl to ensure HCI > init scheduled at hci_register_dev was completed. > > Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> > --- > net/bluetooth/hci_sock.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c > index 33b3c0ffc339..c98de809f856 100644 > --- a/net/bluetooth/hci_sock.c > +++ b/net/bluetooth/hci_sock.c > @@ -1194,9 +1194,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, > > if (test_bit(HCI_INIT, &hdev->flags) || > hci_dev_test_flag(hdev, HCI_SETUP) || > - hci_dev_test_flag(hdev, HCI_CONFIG) || > - (!hci_dev_test_flag(hdev, HCI_AUTO_OFF) && > - test_bit(HCI_UP, &hdev->flags))) { > + hci_dev_test_flag(hdev, HCI_CONFIG)) { > err = -EBUSY; > hci_dev_put(hdev); > goto done; I am not following the reasoning here. It is true that the device has to run init before you can do something with it. From mgmt interface your device will only be announced when it is really ready. Regards Marcel
Hi, Marcel, On 4/22/2022 12:20 PM, Marcel Holtmann wrote: > Hi Vasyl, > >> This is needed for user-space to ensure that HCI init scheduled from >> hci_register_dev is completed. >> >> Function hci_register_dev queues power_on workqueue which will run >> hci_power_on > hci_dev_do_open. Function hci_dev_do_open sets HCI_INIT >> for some time. >> >> It is not allowed to bind to HCI socket user channel when HCI_INIT is >> set. As result, bind might fail when user-space program is run early >> enough during boot. >> >> Now, user-space program can first issue HCIDEVUP ioctl to ensure HCI >> init scheduled at hci_register_dev was completed. >> >> Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> >> --- >> net/bluetooth/hci_sock.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c >> index 33b3c0ffc339..c98de809f856 100644 >> --- a/net/bluetooth/hci_sock.c >> +++ b/net/bluetooth/hci_sock.c >> @@ -1194,9 +1194,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, >> >> if (test_bit(HCI_INIT, &hdev->flags) || >> hci_dev_test_flag(hdev, HCI_SETUP) || >> - hci_dev_test_flag(hdev, HCI_CONFIG) || >> - (!hci_dev_test_flag(hdev, HCI_AUTO_OFF) && >> - test_bit(HCI_UP, &hdev->flags))) { >> + hci_dev_test_flag(hdev, HCI_CONFIG)) { >> err = -EBUSY; >> hci_dev_put(hdev); >> goto done; > > I am not following the reasoning here. It is true that the device has to run init before you can do something with it. From mgmt interface your device will only be announced when it is really ready. Sorry, I am not familiar with mgmt interface. I obtain device using HCIGETDEVLIST. BTW. I have pushed related patch [1]. Comparing to this patch, [1] is less intrusive since it does not effect user-space semantics. Patch [1] allows to ensure that device is not in HCI_INIT state by running hciconfig hci0 down This will either wait for HCI_INIT complete and then powers HCI down, or cancels pending power_on. If we apply [1], we can still consider an optimization to allow binding during HCI_INIT since this optimization will allow me to ommit extra hciconfig hci0 down [1]: https://lore.kernel.org/linux-bluetooth/20220426081823.21557-1-vasyl.vavrychuk@opensynergy.com/T/#u Kind regards, Vasyl
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index 33b3c0ffc339..c98de809f856 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -1194,9 +1194,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr, if (test_bit(HCI_INIT, &hdev->flags) || hci_dev_test_flag(hdev, HCI_SETUP) || - hci_dev_test_flag(hdev, HCI_CONFIG) || - (!hci_dev_test_flag(hdev, HCI_AUTO_OFF) && - test_bit(HCI_UP, &hdev->flags))) { + hci_dev_test_flag(hdev, HCI_CONFIG)) { err = -EBUSY; hci_dev_put(hdev); goto done;
This is needed for user-space to ensure that HCI init scheduled from hci_register_dev is completed. Function hci_register_dev queues power_on workqueue which will run hci_power_on > hci_dev_do_open. Function hci_dev_do_open sets HCI_INIT for some time. It is not allowed to bind to HCI socket user channel when HCI_INIT is set. As result, bind might fail when user-space program is run early enough during boot. Now, user-space program can first issue HCIDEVUP ioctl to ensure HCI init scheduled at hci_register_dev was completed. Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com> --- net/bluetooth/hci_sock.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)