2023-03-30 06:10:03

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v5 1/2] vhci: Add support to trigger devcoredump and read the dump file

Add vhci support to trigger the hci devcoredump by writing to
force_devcoredump debugfs entry and read the generated devcoredump
file.

---

Changes in v5:
- Refactor vhci_read_devcd()

Changes in v4:
- Split into two patches - vhci patch and mgmt-tester patch

Changes in v3:
- Fix compiler warning for signed comparision in test_hci_devcd()

Changes in v2:
- Rename function names to *_devcd

emulator/vhci.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++-
emulator/vhci.h | 2 ++
2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/emulator/vhci.c b/emulator/vhci.c
index a12b11e0f..ecf1db3c7 100644
--- a/emulator/vhci.c
+++ b/emulator/vhci.c
@@ -22,6 +22,7 @@
#include <sys/uio.h>
#include <fcntl.h>
#include <unistd.h>
+#include <dirent.h>

#include "lib/bluetooth.h"
#include "lib/hci.h"
@@ -32,6 +33,7 @@
#include "vhci.h"

#define DEBUGFS_PATH "/sys/kernel/debug/bluetooth"
+#define DEVCORE_PATH "/sys/class/devcoredump"

struct vhci {
enum btdev_type type;
@@ -184,7 +186,7 @@ struct btdev *vhci_get_btdev(struct vhci *vhci)
return vhci->btdev;
}

-static int vhci_debugfs_write(struct vhci *vhci, char *option, void *data,
+static int vhci_debugfs_write(struct vhci *vhci, char *option, const void *data,
size_t len)
{
char path[64];
@@ -267,3 +269,60 @@ int vhci_set_force_static_address(struct vhci *vhci, bool enable)
return vhci_debugfs_write(vhci, "force_static_address", &val,
sizeof(val));
}
+
+int vhci_force_devcd(struct vhci *vhci, const void *data, size_t len)
+{
+ return vhci_debugfs_write(vhci, "force_devcoredump", data, len);
+}
+
+int vhci_read_devcd(struct vhci *vhci, void *buf, size_t size)
+{
+ DIR *dir;
+ struct dirent *entry;
+ char filename[PATH_MAX];
+ int fd;
+ int ret;
+
+ dir = opendir(DEVCORE_PATH);
+ if (dir == NULL)
+ return -errno;
+
+ while ((entry = readdir(dir)) != NULL) {
+ if (strstr(entry->d_name, "devcd"))
+ break;
+ }
+
+ if (entry == NULL) {
+ ret = -ENOENT;
+ goto close_dir;
+ }
+
+ sprintf(filename, DEVCORE_PATH "/%s/data", entry->d_name);
+ fd = open(filename, O_RDWR);
+ if (fd < 0) {
+ ret = -errno;
+ goto close_dir;
+ }
+
+ ret = read(fd, buf, size);
+ if (ret < 0) {
+ ret = -errno;
+ goto close_file;
+ }
+
+ /* Once the devcoredump is read, write anything to it to mark it for
+ * cleanup.
+ */
+ if (write(fd, "0", 1) < 0) {
+ ret = -errno;
+ goto close_file;
+ }
+
+close_file:
+ close(fd);
+
+close_dir:
+ closedir(dir);
+
+ return ret;
+}
diff --git a/emulator/vhci.h b/emulator/vhci.h
index 6da56cb58..68eae4c4a 100644
--- a/emulator/vhci.h
+++ b/emulator/vhci.h
@@ -29,3 +29,5 @@ int vhci_set_msft_opcode(struct vhci *vhci, uint16_t opcode);
int vhci_set_aosp_capable(struct vhci *vhci, bool enable);
int vhci_set_emu_opcode(struct vhci *vhci, uint16_t opcode);
int vhci_set_force_static_address(struct vhci *vhci, bool enable);
+int vhci_force_devcd(struct vhci *vhci, const void *data, size_t len);
+int vhci_read_devcd(struct vhci *vhci, void *buf, size_t size);
--
2.40.0.348.gf938b09366-goog


2023-03-30 06:14:11

by Manish Mandlik

[permalink] [raw]
Subject: [BlueZ PATCH v5 2/2] mgmt-tester: Add devcoredump tests

Add mgmt-tester tests for hci devcoredump. These testa trigger the
devcoredump with a test data and verifies the generated devcoredump
file for the test data and correct devcoredump header fields.

---

Changes in v5:
- Refactor test_hci_devcd()
- Add tests for devcoredump abort and timeout

Changes in v4:
- New patch in the series

tools/mgmt-tester.c | 173 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 173 insertions(+)

diff --git a/tools/mgmt-tester.c b/tools/mgmt-tester.c
index a56c38173..1a8cff999 100644
--- a/tools/mgmt-tester.c
+++ b/tools/mgmt-tester.c
@@ -290,6 +290,20 @@ static void index_removed_callback(uint16_t index, uint16_t length,
tester_post_teardown_complete();
}

+#define MAX_COREDUMP_LINE_LEN 40
+
+struct devcoredump_test_data {
+ enum devcoredump_state {
+ HCI_DEVCOREDUMP_IDLE,
+ HCI_DEVCOREDUMP_ACTIVE,
+ HCI_DEVCOREDUMP_DONE,
+ HCI_DEVCOREDUMP_ABORT,
+ HCI_DEVCOREDUMP_TIMEOUT,
+ } state;
+ unsigned int timeout;
+ char data[MAX_COREDUMP_LINE_LEN];
+};
+
struct hci_cmd_data {
uint16_t opcode;
uint8_t len;
@@ -362,6 +376,8 @@ struct generic_data {
bool set_adv;
const uint8_t *adv_data;
uint8_t adv_data_len;
+ const struct devcoredump_test_data *dump_data;
+ const char (*expect_dump_data)[MAX_COREDUMP_LINE_LEN];
};

static const uint8_t set_exp_feat_param_debug[] = {
@@ -12511,6 +12527,139 @@ static void test_suspend_resume_success_10(const void *test_data)
tester_wait(2, trigger_force_resume, NULL);
}

+#define MAX_COREDUMP_BUF_LEN 512
+
+static const struct devcoredump_test_data data_complete_dump = {
+ .state = HCI_DEVCOREDUMP_DONE,
+ .data = "test data",
+};
+
+static const char expected_complete_dump[][MAX_COREDUMP_LINE_LEN] = {
+ "Bluetooth devcoredump",
+ "State: 2",
+ "Controller Name: vhci_ctrl",
+ "Firmware Version: vhci_fw",
+ "Driver: vhci_drv",
+ "Vendor: vhci",
+ "--- Start dump ---",
+ "", /* end of header data */
+};
+
+static const struct generic_data dump_complete = {
+ .dump_data = &data_complete_dump,
+ .expect_dump_data = expected_complete_dump,
+};
+
+static const struct devcoredump_test_data data_abort_dump = {
+ .state = HCI_DEVCOREDUMP_ABORT,
+ .data = "test data",
+};
+
+static const char expected_abort_dump[][MAX_COREDUMP_LINE_LEN] = {
+ "Bluetooth devcoredump",
+ "State: 3",
+ "Controller Name: vhci_ctrl",
+ "Firmware Version: vhci_fw",
+ "Driver: vhci_drv",
+ "Vendor: vhci",
+ "--- Start dump ---",
+ "", /* end of header data */
+};
+
+static const struct generic_data dump_abort = {
+ .dump_data = &data_abort_dump,
+ .expect_dump_data = expected_abort_dump,
+};
+
+static const struct devcoredump_test_data data_timeout_dump = {
+ .state = HCI_DEVCOREDUMP_TIMEOUT,
+ .timeout = 1,
+ .data = "test data",
+};
+
+static const char expected_timeout_dump[][MAX_COREDUMP_LINE_LEN] = {
+ "Bluetooth devcoredump",
+ "State: 4",
+ "Controller Name: vhci_ctrl",
+ "Firmware Version: vhci_fw",
+ "Driver: vhci_drv",
+ "Vendor: vhci",
+ "--- Start dump ---",
+ "", /* end of header data */
+};
+
+static const struct generic_data dump_timeout = {
+ .dump_data = &data_timeout_dump,
+ .expect_dump_data = expected_timeout_dump,
+};
+
+static void verify_devcd(void *user_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ struct vhci *vhci = hciemu_get_vhci(data->hciemu);
+ char buf[MAX_COREDUMP_BUF_LEN] = {0};
+ char delim[] = "\n";
+ char *line;
+ char *saveptr;
+ int i = 0;
+
+ /* Read the generated devcoredump file */
+ if (vhci_read_devcd(vhci, buf, sizeof(buf)) <= 0) {
+ tester_warn("Unable to read devcoredump");
+ tester_test_failed();
+ return;
+ }
+
+ /* Verify if all devcoredump header fields are present */
+ line = strtok_r(buf, delim, &saveptr);
+ while (strlen(test->expect_dump_data[i])) {
+ if (!line || strcmp(line, test->expect_dump_data[i])) {
+ tester_warn("Incorrect coredump data: %s (expected %s)",
+ line, test->expect_dump_data[i]);
+ tester_test_failed();
+ return;
+ }
+
+ if (!strcmp(strtok(line, ":"), "State")) {
+ /* After updating the devcoredump state, the HCI
+ * devcoredump API adds a `\0` at the end. Skip it
+ * before reading the next line.
+ */
+ saveptr++;
+ }
+
+ line = strtok_r(NULL, delim, &saveptr);
+ i++;
+ }
+
+ /* Verify the devcoredump data */
+ if (!line || strcmp(line, test->dump_data->data)) {
+ tester_warn("Incorrect coredump data: %s (expected %s)", line,
+ test->dump_data->data);
+ tester_test_failed();
+ return;
+ }
+
+ tester_test_passed();
+}
+
+static void test_hci_devcd(const void *test_data)
+{
+ struct test_data *data = tester_get_data();
+ const struct generic_data *test = data->test_data;
+ struct vhci *vhci = hciemu_get_vhci(data->hciemu);
+
+ /* Triggers the devcoredump */
+ if (vhci_force_devcd(vhci, test->dump_data, sizeof(*test->dump_data))) {
+ tester_warn("Unable to set force_devcoredump");
+ tester_test_failed();
+ return;
+ }
+
+ tester_wait(test->dump_data->timeout + 1, verify_devcd, NULL);
+}
+
int main(int argc, char *argv[])
{
tester_init(&argc, &argv);
@@ -14651,5 +14800,29 @@ int main(int argc, char *argv[])
setup_ll_privacy_add_device,
test_command_generic);

+ /* HCI Devcoredump
+ * Setup : Power on
+ * Run: Trigger devcoredump via force_devcoredump
+ * Expect: Devcoredump is generated with correct data
+ */
+ test_bredrle("HCI Devcoredump - Dump Complete", &dump_complete, NULL,
+ test_hci_devcd);
+
+ /* HCI Devcoredump
+ * Setup : Power on
+ * Run: Trigger devcoredump via force_devcoredump
+ * Expect: Devcoredump is generated with correct data
+ */
+ test_bredrle("HCI Devcoredump - Dump Abort", &dump_abort, NULL,
+ test_hci_devcd);
+
+ /* HCI Devcoredump
+ * Setup : Power on
+ * Run: Trigger devcoredump via force_devcoredump
+ * Expect: Devcoredump is generated with correct data
+ */
+ test_bredrle_full("HCI Devcoredump - Dump Timeout", &dump_timeout, NULL,
+ test_hci_devcd, 3);
+
return tester_run();
}
--
2.40.0.348.gf938b09366-goog

2023-03-30 07:37:48

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,v5,1/2] vhci: Add support to trigger devcoredump and read the dump file

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

---Test result---

Test Summary:
CheckPatch PASS 1.21 seconds
GitLint PASS 0.70 seconds
BuildEll PASS 27.03 seconds
BluezMake PASS 875.35 seconds
MakeCheck PASS 11.64 seconds
MakeDistcheck PASS 150.61 seconds
CheckValgrind PASS 247.98 seconds
CheckSmatch PASS 333.21 seconds
bluezmakeextell PASS 98.98 seconds
IncrementalBuild PASS 1423.83 seconds
ScanBuild PASS 1017.98 seconds



---
Regards,
Linux Bluetooth

2023-03-30 18:16:39

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [BlueZ PATCH v5 1/2] vhci: Add support to trigger devcoredump and read the dump file

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Wed, 29 Mar 2023 23:08:02 -0700 you wrote:
> Add vhci support to trigger the hci devcoredump by writing to
> force_devcoredump debugfs entry and read the generated devcoredump
> file.
>
> ---
>
> Changes in v5:
> - Refactor vhci_read_devcd()
>
> [...]

Here is the summary with links:
- [BlueZ,v5,1/2] vhci: Add support to trigger devcoredump and read the dump file
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8bd2f2961774
- [BlueZ,v5,2/2] mgmt-tester: Add devcoredump tests
(no matching commit)

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html


2023-03-30 18:19:35

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BlueZ PATCH v5 1/2] vhci: Add support to trigger devcoredump and read the dump file

Hi Manish,

On Thu, Mar 30, 2023 at 11:10 AM <[email protected]> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluez.git (master)
> by Luiz Augusto von Dentz <[email protected]>:
>
> On Wed, 29 Mar 2023 23:08:02 -0700 you wrote:
> > Add vhci support to trigger the hci devcoredump by writing to
> > force_devcoredump debugfs entry and read the generated devcoredump
> > file.
> >
> > ---
> >
> > Changes in v5:
> > - Refactor vhci_read_devcd()
> >
> > [...]
>
> Here is the summary with links:
> - [BlueZ,v5,1/2] vhci: Add support to trigger devcoredump and read the dump file
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8bd2f2961774
> - [BlueZ,v5,2/2] mgmt-tester: Add devcoredump tests
> (no matching commit)

Note that I did a small change to convert from tester_test_failed to
tester_test_abort that is why the no matching commit is shown above.

> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>


--
Luiz Augusto von Dentz