[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7cee4e74-3a0c-4b7c-9984-696e646160f8@v0yd.nl>
Date: Mon, 8 Jan 2024 23:25:39 +0100
From: Jonas Dreßler <verdre@...d.nl>
To: Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>,
Johan Hedberg <johan.hedberg@...il.com>, asahi@...ts.linux.dev,
linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, verdre@...d.nl
Subject: Re: [PATCH v3 0/4] Disconnect devices before rfkilling adapter
Hi Luiz,
On 1/8/24 19:05, Luiz Augusto von Dentz wrote:
> Hi Jonas,
>
> On Sun, Jan 7, 2024 at 1:03 PM Jonas Dreßler <verdre@...d.nl> wrote:
>>
>> Apparently the firmware is supposed to power off the bluetooth card
>> properly, including disconnecting devices, when we use rfkill to block
>> bluetooth. This doesn't work on a lot of laptops though, leading to weird
>> issues after turning off bluetooth, like the connection timing out on the
>> peripherals which were connected, and bluetooth not connecting properly
>> when the adapter is turned on again after rfkilling.
>>
>> This series uses the rfkill hook in the bluetooth subsystem
>> to execute a few more shutdown commands and make sure that all
>> devices get disconnected before we close the HCI connection to the adapter.
>>
>> ---
>>
>> v1: https://lore.kernel.org/linux-bluetooth/20240102133311.6712-1-verdre@v0yd.nl/
>> v2: https://lore.kernel.org/linux-bluetooth/20240102181946.57288-1-verdre@v0yd.nl/
>> v3:
>> - Update commit message titles to reflect what's actually happening
>> (disconnecting devices, not sending a power-off command).
>> - Doing the shutdown sequence synchronously instead of async now.
>> - Move HCI_RFKILLED flag back again to be set before shutdown.
>> - Added a "fallback" hci_dev_do_close() to the error path because
>> hci_set_powered_sync() might bail-out early on error.
>>
>> Jonas Dreßler (4):
>> Bluetooth: Remove HCI_POWER_OFF_TIMEOUT
>> Bluetooth: mgmt: Remove leftover queuing of power_off work
>> Bluetooth: Add new state HCI_POWERING_DOWN
>> Bluetooth: Disconnect connected devices before rfkilling adapter
>>
>> include/net/bluetooth/hci.h | 2 +-
>> net/bluetooth/hci_core.c | 35 +++++++++++++++++++++++++++++++++--
>> net/bluetooth/hci_sync.c | 16 +++++++++++-----
>> net/bluetooth/mgmt.c | 30 ++++++++++++++----------------
>> 4 files changed, 59 insertions(+), 24 deletions(-)
>>
>> --
>> 2.43.0
>
> I will probably be applying this sortly, but let's try to add tests to
> mgmt-tester just to make sure we don't introduce regressions later,
> btw it seems there are a few suspend test that do connect, for
> example:
>
> Suspend - Success 5 (Pairing - Legacy) - waiting 1 seconds
> random: crng init done
> New connection with handle 0x002a
> Test condition complete, 1 left
> Suspend - Success 5 (Pairing - Legacy) - waiting done
> Set the system into Suspend via force_suspend
> New Controller Suspend event received
> Test condition complete, 0 left
>
Thanks for that hint, I've been starting to write a test and managed to
write to the rfkill file and it's blocking the device just fine, except
I've run into what might be a bug in the virtual HCI driver:
So the power down sequence is initiated on the rfkill as expected and
hci_set_powered_sync(false) is called. That then calls
hci_write_scan_enable_sync(), and this HCI command never gets a response
from the virtual HCI driver. Strangely, BT_HCI_CMD_WRITE_SCAN_ENABLE is
implemented in btdev.c and the callback does get executed (I checked), it
just doesn't send the command completed event:
< HCI Command: Write Scan Enable (0x03|0x001a) plen 1 #1588 [hci1] 12.294234
Scan enable: No Scans (0x00)
no response after...
Below is my current mgmt-tester patch adding the test:
diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index 7dfd1b0c7..2095b7203 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -12439,6 +12439,30 @@ static const struct generic_data suspend_resume_success_5 = {
.expect_alt_ev_len = sizeof(suspend_state_param_disconnect),
};
+static const uint8_t rfkill_hci_disconnect_device[] = {
+ 0x00, 0x00, 0x01, 0x01, 0xaa, 0x00, 0x00,
+ 0x05,
+};
+
+static const uint8_t rfkill_new_settings_ev[] = {
+ 0x92, 0x00, 0x00, 0x00,
+};
+
+
+static const struct generic_data rfkill_disconnect_devices = {
+ .setup_settings = settings_powered_connectable_bondable,
+ .pin = pair_device_pin,
+ .pin_len = sizeof(pair_device_pin),
+ .client_pin = pair_device_pin,
+ .client_pin_len = sizeof(pair_device_pin),
+ .expect_hci_command = BT_HCI_CMD_DISCONNECT,
+ .expect_hci_param = rfkill_hci_disconnect_device,
+ .expect_hci_len = sizeof(rfkill_hci_disconnect_device),
+ .expect_alt_ev = MGMT_EV_NEW_SETTINGS,
+ .expect_alt_ev_param = rfkill_new_settings_ev,
+ .expect_alt_ev_len = sizeof(rfkill_new_settings_ev),
+};
+
static void trigger_force_suspend(void *user_data)
{
struct test_data *data = tester_get_data();
@@ -12454,6 +12478,81 @@ static void trigger_force_suspend(void *user_data)
}
}
+enum rfkill_type {
+ RFKILL_TYPE_ALL = 0,
+ RFKILL_TYPE_WLAN,
+ RFKILL_TYPE_BLUETOOTH,
+ RFKILL_TYPE_UWB,
+ RFKILL_TYPE_WIMAX,
+ RFKILL_TYPE_WWAN,
+};
+
+enum rfkill_operation {
+ RFKILL_OP_ADD = 0,
+ RFKILL_OP_DEL,
+ RFKILL_OP_CHANGE,
+ RFKILL_OP_CHANGE_ALL,
+};
+
+struct rfkill_event {
+ uint32_t idx;
+ uint8_t type;
+ uint8_t op;
+ uint8_t soft;
+ uint8_t hard;
+};
+#define RFKILL_EVENT_SIZE_V1 8
+
+static void trigger_rfkill(void *user_data)
+{
+ int fd;
+ int latest_rfkill_idx;
+ struct rfkill_event write_event;
+ ssize_t l;
+
+ tester_print("Triggering rfkill block of hci device");
+
+ fd = open("/dev/rfkill", O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
+ if (fd < 0) {
+ tester_warn("Failed to open RFKILL control device");
+ return;
+ }
+
+ latest_rfkill_idx = -1;
+ while (1) {
+ struct rfkill_event event = { 0 };
+ ssize_t len;
+
+ len = read(fd, &event, sizeof(event));
+ if (len < RFKILL_EVENT_SIZE_V1)
+ break;
+
+ if (event.type == RFKILL_TYPE_BLUETOOTH)
+ latest_rfkill_idx = event.idx;
+ }
+
+ if (latest_rfkill_idx < 0) {
+ tester_warn("No rfkill device to block found");
+ return;
+ }
+
+ write_event.idx = latest_rfkill_idx;
+ write_event.op = RFKILL_OP_CHANGE;
+ write_event.soft = true;
+
+ l = write(fd, &write_event, sizeof write_event);
+
+ close(fd);
+
+ if (l < 0) {
+ tester_warn("Failed to execute rfkill op");
+ return;
+ }
+
+ if ((size_t)l < RFKILL_EVENT_SIZE_V1)
+ tester_warn("Failed to write to rfkill file");
+}
+
static void trigger_force_resume(void *user_data)
{
struct test_data *data = tester_get_data();
@@ -12475,6 +12574,12 @@ static void test_suspend_resume_success_5(const void *test_data)
tester_wait(1, trigger_force_suspend, NULL);
}
+static void test_disconnect_on_rfkill(const void *test_data)
+{
+ test_pairing_acceptor(test_data);
+ tester_wait(1, trigger_rfkill, NULL);
+}
+
static const struct generic_data suspend_resume_success_6 = {
.setup_settings = settings_powered_connectable_bondable_ssp,
.client_enable_ssp = true,
@@ -14534,6 +14639,15 @@ int main(int argc, char *argv[])
&suspend_resume_success_5, NULL,
test_suspend_resume_success_5);
+ /* Suspend/Resume
+ * Setup: Pair.
+ * Run: Enable suspend
+ * Expect: Receive the Suspend Event
+ */
+ test_bredrle("Rfkill - disconnect devices",
+ &rfkill_disconnect_devices, NULL,
+ test_disconnect_on_rfkill);
+
/* Suspend/Resume
* Setup: Pair.
* Run: Enable suspend
Powered by blists - more mailing lists