2020-06-11 02:02:37

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v3 0/3] Support reading and setting default system parameters


This series adds support for reading and setting default system
parameters from userspace. In particular, combined with the userspace
changes, allows platforms to override default system parameters from a
main.conf file.

The series has been reviewed and tested on chromeos.


Changes in v3:
- Fixed sparse errors
-Fixing sparse errors

Changes in v2:
- Renamed the mgmt.h per Marcel's comments.
- Addressed Marcel's comments in the implementation.

Alain Michaud (3):
mgmt: read/set system parameter definitions
bluetooth:centralize default value initialization.
bluetooth: implement read/set default system parameters mgmt

include/net/bluetooth/hci_core.h | 18 +++
include/net/bluetooth/mgmt.h | 30 ++++
net/bluetooth/Makefile | 2 +-
net/bluetooth/hci_conn.c | 14 +-
net/bluetooth/hci_core.c | 14 +-
net/bluetooth/hci_request.c | 15 +-
net/bluetooth/mgmt.c | 6 +
net/bluetooth/mgmt_config.c | 253 +++++++++++++++++++++++++++++++
net/bluetooth/mgmt_config.h | 11 ++
9 files changed, 341 insertions(+), 22 deletions(-)
create mode 100644 net/bluetooth/mgmt_config.c
create mode 100644 net/bluetooth/mgmt_config.h

--
2.27.0.290.gba653c62da-goog


2020-06-11 02:02:37

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v3 1/3] mgmt: read/set system parameter definitions

This patch submits the corresponding kernel definitions to mgmt.h.
This is submitted before the implementation to avoid any conflicts in
values allocations.

Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
Reviewed-by: Yu Liu <[email protected]>

Signed-off-by: Alain Michaud <[email protected]>
---

Changes in v3:
- Fixed sparse errors

Changes in v2:
- Renamed the mgmt.h per Marcel's comments.
- Addressed Marcel's comments in the implementation.

include/net/bluetooth/mgmt.h | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 16e0d87bd8fa..09452d2ea6d3 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -702,6 +702,36 @@ struct mgmt_rp_set_exp_feature {
__le32 flags;
} __packed;

+#define MGMT_OP_READ_DEF_SYSTEM_CONFIG 0x004b
+
+struct mgmt_tlv {
+ __u16 type;
+ __u8 length;
+ __u8 value[];
+} __packed;
+
+struct mgmt_rp_read_default_system_config {
+ __u8 parameters[0]; /* mgmt_tlv */
+} __packed;
+
+#define MGMT_OP_SET_DEF_SYSTEM_CONFIG 0x004c
+
+struct mgmt_cp_set_default_system_config {
+ __u8 parameters[0]; /* mgmt_tlv */
+} __packed;
+
+#define MGMT_OP_READ_DEF_RUNTIME_CONFIG 0x004d
+
+struct mgmt_rp_read_default_runtime_config {
+ __u8 parameters[0]; /* mgmt_tlv */
+} __packed;
+
+#define MGMT_OP_SET_DEF_RUNTIME_CONFIG 0x004e
+
+struct mgmt_cp_set_default_runtime_config {
+ __u8 parameters[0]; /* mgmt_tlv */
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
--
2.27.0.290.gba653c62da-goog

2020-06-11 02:02:42

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v3 2/3] bluetooth:centralize default value initialization.

This patch centralized the initialization of default parameters. This
is required to allow clients to more easily customize the default
system parameters.

Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
Signed-off-by: Alain Michaud <[email protected]>
---

Changes in v3: None
Changes in v2: None

include/net/bluetooth/hci_core.h | 18 ++++++++++++++++++
net/bluetooth/hci_conn.c | 14 ++++----------
net/bluetooth/hci_core.c | 14 +++++++++++++-
net/bluetooth/hci_request.c | 15 +++++----------
4 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index cdd4f1db8670..0d5dbb6cb5a0 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -295,6 +295,14 @@ struct hci_dev {
__u8 le_scan_type;
__u16 le_scan_interval;
__u16 le_scan_window;
+ __u16 le_scan_int_suspend;
+ __u16 le_scan_window_suspend;
+ __u16 le_scan_int_discovery;
+ __u16 le_scan_window_discovery;
+ __u16 le_scan_int_adv_monitor;
+ __u16 le_scan_window_adv_monitor;
+ __u16 le_scan_int_connect;
+ __u16 le_scan_window_connect;
__u16 le_conn_min_interval;
__u16 le_conn_max_interval;
__u16 le_conn_latency;
@@ -323,6 +331,16 @@ struct hci_dev {
__u16 devid_product;
__u16 devid_version;

+ __u8 def_page_scan_type;
+ __u16 def_page_scan_int;
+ __u16 def_page_scan_window;
+ __u8 def_inq_scan_type;
+ __u16 def_inq_scan_int;
+ __u16 def_inq_scan_window;
+ __u16 def_br_lsto;
+ __u16 def_page_timeout;
+ __u16 def_multi_adv_rotation_duration;
+
__u16 pkt_type;
__u16 esco_type;
__u16 link_policy;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 307800fd18e6..9bdffc4e79b0 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -789,11 +789,8 @@ static void set_ext_conn_params(struct hci_conn *conn,

memset(p, 0, sizeof(*p));

- /* Set window to be the same value as the interval to
- * enable continuous scanning.
- */
- p->scan_interval = cpu_to_le16(hdev->le_scan_interval);
- p->scan_window = p->scan_interval;
+ p->scan_interval = cpu_to_le16(hdev->le_scan_int_connect);
+ p->scan_window = cpu_to_le16(hdev->le_scan_window_connect);
p->conn_interval_min = cpu_to_le16(conn->le_conn_min_interval);
p->conn_interval_max = cpu_to_le16(conn->le_conn_max_interval);
p->conn_latency = cpu_to_le16(conn->le_conn_latency);
@@ -875,11 +872,8 @@ static void hci_req_add_le_create_conn(struct hci_request *req,

memset(&cp, 0, sizeof(cp));

- /* Set window to be the same value as the interval to enable
- * continuous scanning.
- */
- cp.scan_interval = cpu_to_le16(hdev->le_scan_interval);
- cp.scan_window = cp.scan_interval;
+ cp.scan_interval = cpu_to_le16(hdev->le_scan_int_connect);
+ cp.scan_window = cpu_to_le16(hdev->le_scan_window_connect);

bacpy(&cp.peer_addr, &conn->dst);
cp.peer_addr_type = conn->dst_type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dbe2d79f233f..122e44a82995 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2982,7 +2982,7 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
adv_instance->remaining_time = timeout;

if (duration == 0)
- adv_instance->duration = HCI_DEFAULT_ADV_DURATION;
+ adv_instance->duration = hdev->def_multi_adv_rotation_duration;
else
adv_instance->duration = duration;

@@ -3397,6 +3397,12 @@ struct hci_dev *hci_alloc_dev(void)
hdev->le_adv_max_interval = 0x0800;
hdev->le_scan_interval = 0x0060;
hdev->le_scan_window = 0x0030;
+ hdev->le_scan_int_suspend = 0x0400;
+ hdev->le_scan_window_suspend = 0x0012;
+ hdev->le_scan_int_discovery = DISCOV_LE_SCAN_INT;
+ hdev->le_scan_window_discovery = DISCOV_LE_SCAN_WIN;
+ hdev->le_scan_int_connect = 0x0060;
+ hdev->le_scan_window_connect = 0x0060;
hdev->le_conn_min_interval = 0x0018;
hdev->le_conn_max_interval = 0x0028;
hdev->le_conn_latency = 0x0000;
@@ -3412,6 +3418,7 @@ struct hci_dev *hci_alloc_dev(void)
hdev->le_tx_def_phys = HCI_LE_SET_PHY_1M;
hdev->le_rx_def_phys = HCI_LE_SET_PHY_1M;
hdev->le_num_of_adv_sets = HCI_MAX_ADV_INSTANCES;
+ hdev->def_multi_adv_rotation_duration = HCI_DEFAULT_ADV_DURATION;

hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
@@ -3420,6 +3427,11 @@ struct hci_dev *hci_alloc_dev(void)
hdev->auth_payload_timeout = DEFAULT_AUTH_PAYLOAD_TIMEOUT;
hdev->min_enc_key_size = HCI_MIN_ENC_KEY_SIZE;

+ /* default 1.28 sec page scan */
+ hdev->def_page_scan_type = PAGE_SCAN_TYPE_STANDARD;
+ hdev->def_page_scan_int = 0x0800;
+ hdev->def_page_scan_window = 0x0012;
+
mutex_init(&hdev->lock);
mutex_init(&hdev->req_lock);

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 1acf5b8e0910..a7f572ad38ef 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -34,9 +34,6 @@
#define HCI_REQ_PEND 1
#define HCI_REQ_CANCELED 2

-#define LE_SUSPEND_SCAN_WINDOW 0x0012
-#define LE_SUSPEND_SCAN_INTERVAL 0x0400
-
void hci_req_init(struct hci_request *req, struct hci_dev *hdev)
{
skb_queue_head_init(&req->cmd_q);
@@ -366,13 +363,11 @@ void __hci_req_write_fast_connectable(struct hci_request *req, bool enable)
/* 160 msec page scan interval */
acp.interval = cpu_to_le16(0x0100);
} else {
- type = PAGE_SCAN_TYPE_STANDARD; /* default */
-
- /* default 1.28 sec page scan */
- acp.interval = cpu_to_le16(0x0800);
+ type = hdev->def_page_scan_type;
+ acp.interval = cpu_to_le16(hdev->def_page_scan_int);
}

- acp.window = cpu_to_le16(0x0012);
+ acp.window = cpu_to_le16(hdev->def_page_scan_window);

if (__cpu_to_le16(hdev->page_scan_interval) != acp.interval ||
__cpu_to_le16(hdev->page_scan_window) != acp.window)
@@ -927,8 +922,8 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
filter_policy |= 0x02;

if (hdev->suspended) {
- window = LE_SUSPEND_SCAN_WINDOW;
- interval = LE_SUSPEND_SCAN_INTERVAL;
+ window = hdev->le_scan_window_suspend;
+ interval = hdev->le_scan_int_suspend;
} else {
window = hdev->le_scan_window;
interval = hdev->le_scan_interval;
--
2.27.0.290.gba653c62da-goog

2020-06-11 02:02:43

by Alain Michaud

[permalink] [raw]
Subject: [PATCH v3 3/3] bluetooth: implement read/set default system parameters mgmt

This patch implements the read default system parameters and the set
default system parameters mgmt commands.

Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
Reported-by: kernel test robot <[email protected]>

Signed-off-by: Alain Michaud <[email protected]>
---

Changes in v3:
-Fixing sparse errors

Changes in v2: None

include/net/bluetooth/mgmt.h | 2 +-
net/bluetooth/Makefile | 2 +-
net/bluetooth/mgmt.c | 6 +
net/bluetooth/mgmt_config.c | 253 +++++++++++++++++++++++++++++++++++
net/bluetooth/mgmt_config.h | 11 ++
5 files changed, 272 insertions(+), 2 deletions(-)
create mode 100644 net/bluetooth/mgmt_config.c
create mode 100644 net/bluetooth/mgmt_config.h

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 09452d2ea6d3..39e849744f28 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -705,7 +705,7 @@ struct mgmt_rp_set_exp_feature {
#define MGMT_OP_READ_DEF_SYSTEM_CONFIG 0x004b

struct mgmt_tlv {
- __u16 type;
+ __le16 type;
__u8 length;
__u8 value[];
} __packed;
diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
index 41dd541a44a5..1c645fba8c49 100644
--- a/net/bluetooth/Makefile
+++ b/net/bluetooth/Makefile
@@ -14,7 +14,7 @@ bluetooth_6lowpan-y := 6lowpan.o

bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
- ecdh_helper.o hci_request.o mgmt_util.o
+ ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o

bluetooth-$(CONFIG_BT_BREDR) += sco.o
bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9e8a3cccc6ca..69cd4f756a0d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -36,6 +36,7 @@
#include "hci_request.h"
#include "smp.h"
#include "mgmt_util.h"
+#include "mgmt_config.h"

#define MGMT_VERSION 1
#define MGMT_REVISION 17
@@ -111,6 +112,8 @@ static const u16 mgmt_commands[] = {
MGMT_OP_READ_SECURITY_INFO,
MGMT_OP_READ_EXP_FEATURES_INFO,
MGMT_OP_SET_EXP_FEATURE,
+ MGMT_OP_READ_DEF_SYSTEM_CONFIG,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
};

static const u16 mgmt_events[] = {
@@ -162,6 +165,7 @@ static const u16 mgmt_untrusted_commands[] = {
MGMT_OP_READ_EXT_INFO,
MGMT_OP_READ_SECURITY_INFO,
MGMT_OP_READ_EXP_FEATURES_INFO,
+ MGMT_OP_READ_DEF_SYSTEM_CONFIG,
};

static const u16 mgmt_untrusted_events[] = {
@@ -7297,6 +7301,8 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ set_exp_feature, MGMT_SET_EXP_FEATURE_SIZE,
HCI_MGMT_VAR_LEN |
HCI_MGMT_HDEV_OPTIONAL },
+ { read_def_system_config, 0, HCI_MGMT_UNTRUSTED },
+ { set_def_system_config, 0, HCI_MGMT_VAR_LEN },
};

void mgmt_index_added(struct hci_dev *hdev)
diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
new file mode 100644
index 000000000000..f6dfbe93542c
--- /dev/null
+++ b/net/bluetooth/mgmt_config.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (C) 2020 Google Corporation
+ */
+
+#include <net/bluetooth/bluetooth.h>
+#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/mgmt.h>
+
+#include "mgmt_util.h"
+#include "mgmt_config.h"
+
+#define HDEV_PARAM_U16(_param_code_, _param_name_) \
+{ \
+ { cpu_to_le16(_param_code_), sizeof(__u16) }, \
+ { cpu_to_le16(hdev->_param_name_) } \
+}
+
+int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 data_len)
+{
+ struct {
+ struct mgmt_tlv entry;
+ union {
+ /* This is a simplification for now since all values
+ * are 16 bits. In the future, this code may need
+ * refactoring to account for variable length values
+ * and properly calculate the required buffer size.
+ */
+ __le16 value;
+ };
+ } __packed params[] = {
+ /* Please see mgmt-api.txt for documentation of these values */
+ HDEV_PARAM_U16(0x0000, def_page_scan_type),
+ HDEV_PARAM_U16(0x0001, def_page_scan_int),
+ HDEV_PARAM_U16(0x0002, def_page_scan_window),
+ HDEV_PARAM_U16(0x0003, def_inq_scan_type),
+ HDEV_PARAM_U16(0x0004, def_inq_scan_int),
+ HDEV_PARAM_U16(0x0005, def_inq_scan_window),
+ HDEV_PARAM_U16(0x0006, def_br_lsto),
+ HDEV_PARAM_U16(0x0007, def_page_timeout),
+ HDEV_PARAM_U16(0x0008, sniff_min_interval),
+ HDEV_PARAM_U16(0x0009, sniff_max_interval),
+ HDEV_PARAM_U16(0x000a, le_adv_min_interval),
+ HDEV_PARAM_U16(0x000b, le_adv_max_interval),
+ HDEV_PARAM_U16(0x000c, def_multi_adv_rotation_duration),
+ HDEV_PARAM_U16(0x000d, le_scan_interval),
+ HDEV_PARAM_U16(0x000e, le_scan_window),
+ HDEV_PARAM_U16(0x000f, le_scan_int_suspend),
+ HDEV_PARAM_U16(0x0010, le_scan_window_suspend),
+ HDEV_PARAM_U16(0x0011, le_scan_int_discovery),
+ HDEV_PARAM_U16(0x0012, le_scan_window_discovery),
+ HDEV_PARAM_U16(0x0013, le_scan_int_adv_monitor),
+ HDEV_PARAM_U16(0x0014, le_scan_window_adv_monitor),
+ HDEV_PARAM_U16(0x0015, le_scan_int_connect),
+ HDEV_PARAM_U16(0x0016, le_scan_window_connect),
+ HDEV_PARAM_U16(0x0017, le_conn_min_interval),
+ HDEV_PARAM_U16(0x0018, le_conn_max_interval),
+ HDEV_PARAM_U16(0x0019, le_conn_latency),
+ HDEV_PARAM_U16(0x001a, le_supv_timeout),
+ };
+ struct mgmt_rp_read_def_system_config *rp = (void *)params;
+
+ bt_dev_dbg(hdev, "sock %p", sk);
+
+ return mgmt_cmd_complete(sk, hdev->id,
+ MGMT_OP_READ_DEF_SYSTEM_CONFIG,
+ 0, rp, sizeof(params));
+}
+
+#define TO_TLV(x) ((struct mgmt_tlv *)(x))
+#define TLV_GET_LE16(tlv) le16_to_cpu(*((__le16 *)(TO_TLV(tlv)->value)))
+
+int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 data_len)
+{
+ u16 buffer_left = data_len;
+ u8 *buffer = data;
+
+ if (buffer_left < sizeof(struct mgmt_tlv)) {
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+ MGMT_STATUS_INVALID_PARAMS);
+ }
+
+ /* First pass to validate the tlv */
+ while (buffer_left >= sizeof(struct mgmt_tlv)) {
+ const u8 len = TO_TLV(buffer)->length;
+ const u16 exp_len = sizeof(struct mgmt_tlv) +
+ len;
+ const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
+
+ if (buffer_left < exp_len) {
+ bt_dev_warn(hdev, "invalid len left %d, exp >= %d",
+ buffer_left, exp_len);
+
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+ MGMT_STATUS_INVALID_PARAMS);
+ }
+
+ /* Please see mgmt-api.txt for documentation of these values */
+ switch (type) {
+ case 0x0000:
+ case 0x0001:
+ case 0x0002:
+ case 0x0003:
+ case 0x0004:
+ case 0x0005:
+ case 0x0006:
+ case 0x0007:
+ case 0x0008:
+ case 0x0009:
+ case 0x000a:
+ case 0x000b:
+ case 0x000c:
+ case 0x000d:
+ case 0x000e:
+ case 0x000f:
+ case 0x0010:
+ case 0x0011:
+ case 0x0012:
+ case 0x0013:
+ case 0x0014:
+ case 0x0015:
+ case 0x0016:
+ case 0x0017:
+ case 0x0018:
+ case 0x0019:
+ case 0x001a:
+ if (len != sizeof(u16)) {
+ bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
+ len, sizeof(u16), type);
+
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+ MGMT_STATUS_INVALID_PARAMS);
+ }
+ break;
+ default:
+ bt_dev_warn(hdev, "unsupported parameter %u", type);
+ break;
+ }
+
+ buffer_left -= exp_len;
+ buffer += exp_len;
+ }
+
+ buffer_left = data_len;
+ buffer = data;
+ while (buffer_left >= sizeof(struct mgmt_tlv)) {
+ const u8 len = TO_TLV(buffer)->length;
+ const u16 exp_len = sizeof(struct mgmt_tlv) +
+ len;
+ const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
+
+ switch (type) {
+ case 0x0000:
+ hdev->def_page_scan_type = TLV_GET_LE16(buffer);
+ break;
+ case 0x0001:
+ hdev->def_page_scan_int = TLV_GET_LE16(buffer);
+ break;
+ case 0x0002:
+ hdev->def_page_scan_window = TLV_GET_LE16(buffer);
+ break;
+ case 0x0003:
+ hdev->def_inq_scan_type = TLV_GET_LE16(buffer);
+ break;
+ case 0x0004:
+ hdev->def_inq_scan_int = TLV_GET_LE16(buffer);
+ break;
+ case 0x0005:
+ hdev->def_inq_scan_window = TLV_GET_LE16(buffer);
+ break;
+ case 0x0006:
+ hdev->def_br_lsto = TLV_GET_LE16(buffer);
+ break;
+ case 0x0007:
+ hdev->def_page_timeout = TLV_GET_LE16(buffer);
+ break;
+ case 0x0008:
+ hdev->sniff_min_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x0009:
+ hdev->sniff_max_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x000a:
+ hdev->le_adv_min_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x000b:
+ hdev->le_adv_max_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x000c:
+ hdev->def_multi_adv_rotation_duration =
+ TLV_GET_LE16(buffer);
+ break;
+ case 0x000d:
+ hdev->le_scan_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x000e:
+ hdev->le_scan_window = TLV_GET_LE16(buffer);
+ break;
+ case 0x000f:
+ hdev->le_scan_int_suspend = TLV_GET_LE16(buffer);
+ break;
+ case 0x0010:
+ hdev->le_scan_window_suspend = TLV_GET_LE16(buffer);
+ break;
+ case 0x0011:
+ hdev->le_scan_int_discovery = TLV_GET_LE16(buffer);
+ break;
+ case 0x00012:
+ hdev->le_scan_window_discovery = TLV_GET_LE16(buffer);
+ break;
+ case 0x00013:
+ hdev->le_scan_int_adv_monitor = TLV_GET_LE16(buffer);
+ break;
+ case 0x00014:
+ hdev->le_scan_window_adv_monitor = TLV_GET_LE16(buffer);
+ break;
+ case 0x00015:
+ hdev->le_scan_int_connect = TLV_GET_LE16(buffer);
+ break;
+ case 0x00016:
+ hdev->le_scan_window_connect = TLV_GET_LE16(buffer);
+ break;
+ case 0x00017:
+ hdev->le_conn_min_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x00018:
+ hdev->le_conn_max_interval = TLV_GET_LE16(buffer);
+ break;
+ case 0x00019:
+ hdev->le_conn_latency = TLV_GET_LE16(buffer);
+ break;
+ case 0x0001a:
+ hdev->le_supv_timeout = TLV_GET_LE16(buffer);
+ break;
+ default:
+ bt_dev_warn(hdev, "unsupported parameter %u", type);
+ break;
+ }
+
+ buffer_left -= exp_len;
+ buffer += exp_len;
+ }
+
+ return mgmt_cmd_status(sk, hdev->id,
+ MGMT_OP_SET_DEF_SYSTEM_CONFIG,
+ MGMT_STATUS_SUCCESS);
+}
diff --git a/net/bluetooth/mgmt_config.h b/net/bluetooth/mgmt_config.h
new file mode 100644
index 000000000000..51da6e63b1a0
--- /dev/null
+++ b/net/bluetooth/mgmt_config.h
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ * Copyright (C) 2020 Google Corporation
+ */
+
+int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 data_len);
+
+int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
+ u16 data_len);
--
2.27.0.290.gba653c62da-goog

2020-06-12 13:47:21

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] bluetooth:centralize default value initialization.

Hi Alain,

please use Bluetooth: as subject prefix.

> This patch centralized the initialization of default parameters. This
> is required to allow clients to more easily customize the default
> system parameters.
>
> Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> Signed-off-by: Alain Michaud <[email protected]>

Other way around please.

> ---
>
> Changes in v3: None
> Changes in v2: None
>
> include/net/bluetooth/hci_core.h | 18 ++++++++++++++++++
> net/bluetooth/hci_conn.c | 14 ++++----------
> net/bluetooth/hci_core.c | 14 +++++++++++++-
> net/bluetooth/hci_request.c | 15 +++++----------
> 4 files changed, 40 insertions(+), 21 deletions(-)

Patch has been applied to bluetooth-next tree.

Regards

Marcel

2020-06-12 13:47:21

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] bluetooth: implement read/set default system parameters mgmt

Hi Alain,

> This patch implements the read default system parameters and the set
> default system parameters mgmt commands.
>
> Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> Reported-by: kernel test robot <[email protected]>
>
> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> Changes in v3:
> -Fixing sparse errors
>
> Changes in v2: None
>
> include/net/bluetooth/mgmt.h | 2 +-
> net/bluetooth/Makefile | 2 +-
> net/bluetooth/mgmt.c | 6 +
> net/bluetooth/mgmt_config.c | 253 +++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt_config.h | 11 ++
> 5 files changed, 272 insertions(+), 2 deletions(-)
> create mode 100644 net/bluetooth/mgmt_config.c
> create mode 100644 net/bluetooth/mgmt_config.h
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 09452d2ea6d3..39e849744f28 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -705,7 +705,7 @@ struct mgmt_rp_set_exp_feature {
> #define MGMT_OP_READ_DEF_SYSTEM_CONFIG 0x004b
>
> struct mgmt_tlv {
> - __u16 type;
> + __le16 type;
> __u8 length;
> __u8 value[];
> } __packed;

I fixed this cleanly in the first patch.

> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index 41dd541a44a5..1c645fba8c49 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -14,7 +14,7 @@ bluetooth_6lowpan-y := 6lowpan.o
>
> bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
> hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
> - ecdh_helper.o hci_request.o mgmt_util.o
> + ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o
>
> bluetooth-$(CONFIG_BT_BREDR) += sco.o
> bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 9e8a3cccc6ca..69cd4f756a0d 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -36,6 +36,7 @@
> #include "hci_request.h"
> #include "smp.h"
> #include "mgmt_util.h"
> +#include "mgmt_config.h"
>
> #define MGMT_VERSION 1
> #define MGMT_REVISION 17
> @@ -111,6 +112,8 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_READ_SECURITY_INFO,
> MGMT_OP_READ_EXP_FEATURES_INFO,
> MGMT_OP_SET_EXP_FEATURE,
> + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> };
>
> static const u16 mgmt_events[] = {
> @@ -162,6 +165,7 @@ static const u16 mgmt_untrusted_commands[] = {
> MGMT_OP_READ_EXT_INFO,
> MGMT_OP_READ_SECURITY_INFO,
> MGMT_OP_READ_EXP_FEATURES_INFO,
> + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> };
>
> static const u16 mgmt_untrusted_events[] = {
> @@ -7297,6 +7301,8 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> { set_exp_feature, MGMT_SET_EXP_FEATURE_SIZE,
> HCI_MGMT_VAR_LEN |
> HCI_MGMT_HDEV_OPTIONAL },
> + { read_def_system_config, 0, HCI_MGMT_UNTRUSTED },
> + { set_def_system_config, 0, HCI_MGMT_VAR_LEN },
> };

I also added _SIZE constants for you.

>
> void mgmt_index_added(struct hci_dev *hdev)
> diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
> new file mode 100644
> index 000000000000..f6dfbe93542c
> --- /dev/null
> +++ b/net/bluetooth/mgmt_config.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2020 Google Corporation
> + */
> +
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/mgmt.h>
> +
> +#include "mgmt_util.h"
> +#include "mgmt_config.h"
> +
> +#define HDEV_PARAM_U16(_param_code_, _param_name_) \
> +{ \
> + { cpu_to_le16(_param_code_), sizeof(__u16) }, \
> + { cpu_to_le16(hdev->_param_name_) } \
> +}
> +
> +int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 data_len)
> +{
> + struct {
> + struct mgmt_tlv entry;
> + union {
> + /* This is a simplification for now since all values
> + * are 16 bits. In the future, this code may need
> + * refactoring to account for variable length values
> + * and properly calculate the required buffer size.
> + */
> + __le16 value;
> + };
> + } __packed params[] = {
> + /* Please see mgmt-api.txt for documentation of these values */
> + HDEV_PARAM_U16(0x0000, def_page_scan_type),
> + HDEV_PARAM_U16(0x0001, def_page_scan_int),
> + HDEV_PARAM_U16(0x0002, def_page_scan_window),
> + HDEV_PARAM_U16(0x0003, def_inq_scan_type),
> + HDEV_PARAM_U16(0x0004, def_inq_scan_int),
> + HDEV_PARAM_U16(0x0005, def_inq_scan_window),
> + HDEV_PARAM_U16(0x0006, def_br_lsto),
> + HDEV_PARAM_U16(0x0007, def_page_timeout),
> + HDEV_PARAM_U16(0x0008, sniff_min_interval),
> + HDEV_PARAM_U16(0x0009, sniff_max_interval),
> + HDEV_PARAM_U16(0x000a, le_adv_min_interval),
> + HDEV_PARAM_U16(0x000b, le_adv_max_interval),
> + HDEV_PARAM_U16(0x000c, def_multi_adv_rotation_duration),
> + HDEV_PARAM_U16(0x000d, le_scan_interval),
> + HDEV_PARAM_U16(0x000e, le_scan_window),
> + HDEV_PARAM_U16(0x000f, le_scan_int_suspend),
> + HDEV_PARAM_U16(0x0010, le_scan_window_suspend),
> + HDEV_PARAM_U16(0x0011, le_scan_int_discovery),
> + HDEV_PARAM_U16(0x0012, le_scan_window_discovery),
> + HDEV_PARAM_U16(0x0013, le_scan_int_adv_monitor),
> + HDEV_PARAM_U16(0x0014, le_scan_window_adv_monitor),
> + HDEV_PARAM_U16(0x0015, le_scan_int_connect),
> + HDEV_PARAM_U16(0x0016, le_scan_window_connect),
> + HDEV_PARAM_U16(0x0017, le_conn_min_interval),
> + HDEV_PARAM_U16(0x0018, le_conn_max_interval),
> + HDEV_PARAM_U16(0x0019, le_conn_latency),
> + HDEV_PARAM_U16(0x001a, le_supv_timeout),
> + };
> + struct mgmt_rp_read_def_system_config *rp = (void *)params;
> +
> + bt_dev_dbg(hdev, "sock %p", sk);
> +
> + return mgmt_cmd_complete(sk, hdev->id,
> + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> + 0, rp, sizeof(params));
> +}
> +
> +#define TO_TLV(x) ((struct mgmt_tlv *)(x))
> +#define TLV_GET_LE16(tlv) le16_to_cpu(*((__le16 *)(TO_TLV(tlv)->value)))
> +
> +int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 data_len)
> +{
> + u16 buffer_left = data_len;
> + u8 *buffer = data;
> +
> + if (buffer_left < sizeof(struct mgmt_tlv)) {
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> + MGMT_STATUS_INVALID_PARAMS);
> + }
> +
> + /* First pass to validate the tlv */
> + while (buffer_left >= sizeof(struct mgmt_tlv)) {
> + const u8 len = TO_TLV(buffer)->length;
> + const u16 exp_len = sizeof(struct mgmt_tlv) +
> + len;
> + const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
> +
> + if (buffer_left < exp_len) {
> + bt_dev_warn(hdev, "invalid len left %d, exp >= %d",
> + buffer_left, exp_len);
> +
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> + MGMT_STATUS_INVALID_PARAMS);
> + }
> +
> + /* Please see mgmt-api.txt for documentation of these values */
> + switch (type) {
> + case 0x0000:
> + case 0x0001:
> + case 0x0002:
> + case 0x0003:
> + case 0x0004:
> + case 0x0005:
> + case 0x0006:
> + case 0x0007:
> + case 0x0008:
> + case 0x0009:
> + case 0x000a:
> + case 0x000b:
> + case 0x000c:
> + case 0x000d:
> + case 0x000e:
> + case 0x000f:
> + case 0x0010:
> + case 0x0011:
> + case 0x0012:
> + case 0x0013:
> + case 0x0014:
> + case 0x0015:
> + case 0x0016:
> + case 0x0017:
> + case 0x0018:
> + case 0x0019:
> + case 0x001a:
> + if (len != sizeof(u16)) {
> + bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
> + len, sizeof(u16), type);
> +
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> + MGMT_STATUS_INVALID_PARAMS);
> + }
> + break;
> + default:
> + bt_dev_warn(hdev, "unsupported parameter %u", type);
> + break;
> + }
> +
> + buffer_left -= exp_len;
> + buffer += exp_len;
> + }
> +
> + buffer_left = data_len;
> + buffer = data;
> + while (buffer_left >= sizeof(struct mgmt_tlv)) {
> + const u8 len = TO_TLV(buffer)->length;
> + const u16 exp_len = sizeof(struct mgmt_tlv) +
> + len;
> + const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
> +
> + switch (type) {
> + case 0x0000:
> + hdev->def_page_scan_type = TLV_GET_LE16(buffer);
> + break;
> + case 0x0001:
> + hdev->def_page_scan_int = TLV_GET_LE16(buffer);
> + break;
> + case 0x0002:
> + hdev->def_page_scan_window = TLV_GET_LE16(buffer);
> + break;
> + case 0x0003:
> + hdev->def_inq_scan_type = TLV_GET_LE16(buffer);
> + break;
> + case 0x0004:
> + hdev->def_inq_scan_int = TLV_GET_LE16(buffer);
> + break;
> + case 0x0005:
> + hdev->def_inq_scan_window = TLV_GET_LE16(buffer);
> + break;
> + case 0x0006:
> + hdev->def_br_lsto = TLV_GET_LE16(buffer);
> + break;
> + case 0x0007:
> + hdev->def_page_timeout = TLV_GET_LE16(buffer);
> + break;
> + case 0x0008:
> + hdev->sniff_min_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x0009:
> + hdev->sniff_max_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x000a:
> + hdev->le_adv_min_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x000b:
> + hdev->le_adv_max_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x000c:
> + hdev->def_multi_adv_rotation_duration =
> + TLV_GET_LE16(buffer);
> + break;
> + case 0x000d:
> + hdev->le_scan_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x000e:
> + hdev->le_scan_window = TLV_GET_LE16(buffer);
> + break;
> + case 0x000f:
> + hdev->le_scan_int_suspend = TLV_GET_LE16(buffer);
> + break;
> + case 0x0010:
> + hdev->le_scan_window_suspend = TLV_GET_LE16(buffer);
> + break;
> + case 0x0011:
> + hdev->le_scan_int_discovery = TLV_GET_LE16(buffer);
> + break;
> + case 0x00012:
> + hdev->le_scan_window_discovery = TLV_GET_LE16(buffer);
> + break;
> + case 0x00013:
> + hdev->le_scan_int_adv_monitor = TLV_GET_LE16(buffer);
> + break;
> + case 0x00014:
> + hdev->le_scan_window_adv_monitor = TLV_GET_LE16(buffer);
> + break;
> + case 0x00015:
> + hdev->le_scan_int_connect = TLV_GET_LE16(buffer);
> + break;
> + case 0x00016:
> + hdev->le_scan_window_connect = TLV_GET_LE16(buffer);
> + break;
> + case 0x00017:
> + hdev->le_conn_min_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x00018:
> + hdev->le_conn_max_interval = TLV_GET_LE16(buffer);
> + break;
> + case 0x00019:
> + hdev->le_conn_latency = TLV_GET_LE16(buffer);
> + break;
> + case 0x0001a:
> + hdev->le_supv_timeout = TLV_GET_LE16(buffer);
> + break;
> + default:
> + bt_dev_warn(hdev, "unsupported parameter %u", type);
> + break;
> + }
> +
> + buffer_left -= exp_len;
> + buffer += exp_len;
> + }
> +
> + return mgmt_cmd_status(sk, hdev->id,
> + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> + MGMT_STATUS_SUCCESS);
> +}
> diff --git a/net/bluetooth/mgmt_config.h b/net/bluetooth/mgmt_config.h
> new file mode 100644
> index 000000000000..51da6e63b1a0
> --- /dev/null
> +++ b/net/bluetooth/mgmt_config.h
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2020 Google Corporation
> + */
> +
> +int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 data_len);
> +
> +int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> + u16 data_len);

Other than that, patch has been applied to bluetooth-next tree.

Regards

Marcel

2020-06-12 13:47:23

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] mgmt: read/set system parameter definitions

Hi Alain,

please remember to put Bluetooth: as prefix in the subject. I fixed this up for you.

> This patch submits the corresponding kernel definitions to mgmt.h.
> This is submitted before the implementation to avoid any conflicts in
> values allocations.
>
> Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> Reviewed-by: Yu Liu <[email protected]>

Please put these behind your Signed-off-by line. I changed this for you.

> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> Changes in v3:
> - Fixed sparse errors
>
> Changes in v2:
> - Renamed the mgmt.h per Marcel's comments.
> - Addressed Marcel's comments in the implementation.
>
> include/net/bluetooth/mgmt.h | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 16e0d87bd8fa..09452d2ea6d3 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -702,6 +702,36 @@ struct mgmt_rp_set_exp_feature {
> __le32 flags;
> } __packed;
>
> +#define MGMT_OP_READ_DEF_SYSTEM_CONFIG 0x004b
> +
> +struct mgmt_tlv {
> + __u16 type;

I changed this to __le16 since we developed mgmt as little-endian. I missed this before and fixed this up as well.

> + __u8 length;
> + __u8 value[];
> +} __packed;
> +
> +struct mgmt_rp_read_default_system_config {
> + __u8 parameters[0]; /* mgmt_tlv */

I changed this to params[] to follow recent global style changes. And hmmm, that seems to be not possible with no other values.

./include/net/bluetooth/mgmt.h:714:7: error: flexible array member in a struct with no named members
714 | __u8 params[]; /* mgmt_tlv */
| ^~~~~~

Maybe we need to leave these structs out since they only contain TLVs anyway.

> +} __packed;
> +
> +#define MGMT_OP_SET_DEF_SYSTEM_CONFIG 0x004c
> +
> +struct mgmt_cp_set_default_system_config {
> + __u8 parameters[0]; /* mgmt_tlv */
> +} __packed;
> +
> +#define MGMT_OP_READ_DEF_RUNTIME_CONFIG 0x004d
> +
> +struct mgmt_rp_read_default_runtime_config {
> + __u8 parameters[0]; /* mgmt_tlv */
> +} __packed;
> +
> +#define MGMT_OP_SET_DEF_RUNTIME_CONFIG 0x004e
> +
> +struct mgmt_cp_set_default_runtime_config {
> + __u8 parameters[0]; /* mgmt_tlv */
> +} __packed;
> +
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;

Other than that patch has been applied to bluetooth-next tree.

Regards

Marcel

2020-06-12 13:48:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] Support reading and setting default system parameters

Hi Alain,

> This series adds support for reading and setting default system
> parameters from userspace. In particular, combined with the userspace
> changes, allows platforms to override default system parameters from a
> main.conf file.
>
> The series has been reviewed and tested on chromeos.
>
>
> Changes in v3:
> - Fixed sparse errors
> -Fixing sparse errors
>
> Changes in v2:
> - Renamed the mgmt.h per Marcel's comments.
> - Addressed Marcel's comments in the implementation.
>
> Alain Michaud (3):
> mgmt: read/set system parameter definitions
> bluetooth:centralize default value initialization.
> bluetooth: implement read/set default system parameters mgmt
>
> include/net/bluetooth/hci_core.h | 18 +++
> include/net/bluetooth/mgmt.h | 30 ++++
> net/bluetooth/Makefile | 2 +-
> net/bluetooth/hci_conn.c | 14 +-
> net/bluetooth/hci_core.c | 14 +-
> net/bluetooth/hci_request.c | 15 +-
> net/bluetooth/mgmt.c | 6 +
> net/bluetooth/mgmt_config.c | 253 +++++++++++++++++++++++++++++++
> net/bluetooth/mgmt_config.h | 11 ++
> 9 files changed, 341 insertions(+), 22 deletions(-)
> create mode 100644 net/bluetooth/mgmt_config.c
> create mode 100644 net/bluetooth/mgmt_config.h

all 3 patch have been applied with modifications to bluetooth-next tree.

Regards

Marcel

2020-06-12 19:42:29

by Alain Michaud

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] bluetooth: implement read/set default system parameters mgmt

Hi Marcel,

On Fri, Jun 12, 2020 at 9:46 AM Marcel Holtmann <[email protected]> wrote:
>
> Hi Alain,
>
> > This patch implements the read default system parameters and the set
> > default system parameters mgmt commands.
> >
> > Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> >
> > Signed-off-by: Alain Michaud <[email protected]>
> > ---
> >
> > Changes in v3:
> > -Fixing sparse errors
> >
> > Changes in v2: None
> >
> > include/net/bluetooth/mgmt.h | 2 +-
> > net/bluetooth/Makefile | 2 +-
> > net/bluetooth/mgmt.c | 6 +
> > net/bluetooth/mgmt_config.c | 253 +++++++++++++++++++++++++++++++++++
> > net/bluetooth/mgmt_config.h | 11 ++
> > 5 files changed, 272 insertions(+), 2 deletions(-)
> > create mode 100644 net/bluetooth/mgmt_config.c
> > create mode 100644 net/bluetooth/mgmt_config.h
> >
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index 09452d2ea6d3..39e849744f28 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -705,7 +705,7 @@ struct mgmt_rp_set_exp_feature {
> > #define MGMT_OP_READ_DEF_SYSTEM_CONFIG 0x004b
> >
> > struct mgmt_tlv {
> > - __u16 type;
> > + __le16 type;
> > __u8 length;
> > __u8 value[];
> > } __packed;
>
> I fixed this cleanly in the first patch.
>
> > diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> > index 41dd541a44a5..1c645fba8c49 100644
> > --- a/net/bluetooth/Makefile
> > +++ b/net/bluetooth/Makefile
> > @@ -14,7 +14,7 @@ bluetooth_6lowpan-y := 6lowpan.o
> >
> > bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
> > hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \
> > - ecdh_helper.o hci_request.o mgmt_util.o
> > + ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o
> >
> > bluetooth-$(CONFIG_BT_BREDR) += sco.o
> > bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 9e8a3cccc6ca..69cd4f756a0d 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -36,6 +36,7 @@
> > #include "hci_request.h"
> > #include "smp.h"
> > #include "mgmt_util.h"
> > +#include "mgmt_config.h"
> >
> > #define MGMT_VERSION 1
> > #define MGMT_REVISION 17
> > @@ -111,6 +112,8 @@ static const u16 mgmt_commands[] = {
> > MGMT_OP_READ_SECURITY_INFO,
> > MGMT_OP_READ_EXP_FEATURES_INFO,
> > MGMT_OP_SET_EXP_FEATURE,
> > + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> > + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> > };
> >
> > static const u16 mgmt_events[] = {
> > @@ -162,6 +165,7 @@ static const u16 mgmt_untrusted_commands[] = {
> > MGMT_OP_READ_EXT_INFO,
> > MGMT_OP_READ_SECURITY_INFO,
> > MGMT_OP_READ_EXP_FEATURES_INFO,
> > + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> > };
> >
> > static const u16 mgmt_untrusted_events[] = {
> > @@ -7297,6 +7301,8 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> > { set_exp_feature, MGMT_SET_EXP_FEATURE_SIZE,
> > HCI_MGMT_VAR_LEN |
> > HCI_MGMT_HDEV_OPTIONAL },
> > + { read_def_system_config, 0, HCI_MGMT_UNTRUSTED },
> > + { set_def_system_config, 0, HCI_MGMT_VAR_LEN },
> > };
>
> I also added _SIZE constants for you.
Looks like your fix may have a typo... Patch submitted to address the typo.

>
> >
> > void mgmt_index_added(struct hci_dev *hdev)
> > diff --git a/net/bluetooth/mgmt_config.c b/net/bluetooth/mgmt_config.c
> > new file mode 100644
> > index 000000000000..f6dfbe93542c
> > --- /dev/null
> > +++ b/net/bluetooth/mgmt_config.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +/*
> > + * Copyright (C) 2020 Google Corporation
> > + */
> > +
> > +#include <net/bluetooth/bluetooth.h>
> > +#include <net/bluetooth/hci_core.h>
> > +#include <net/bluetooth/mgmt.h>
> > +
> > +#include "mgmt_util.h"
> > +#include "mgmt_config.h"
> > +
> > +#define HDEV_PARAM_U16(_param_code_, _param_name_) \
> > +{ \
> > + { cpu_to_le16(_param_code_), sizeof(__u16) }, \
> > + { cpu_to_le16(hdev->_param_name_) } \
> > +}
> > +
> > +int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> > + u16 data_len)
> > +{
> > + struct {
> > + struct mgmt_tlv entry;
> > + union {
> > + /* This is a simplification for now since all values
> > + * are 16 bits. In the future, this code may need
> > + * refactoring to account for variable length values
> > + * and properly calculate the required buffer size.
> > + */
> > + __le16 value;
> > + };
> > + } __packed params[] = {
> > + /* Please see mgmt-api.txt for documentation of these values */
> > + HDEV_PARAM_U16(0x0000, def_page_scan_type),
> > + HDEV_PARAM_U16(0x0001, def_page_scan_int),
> > + HDEV_PARAM_U16(0x0002, def_page_scan_window),
> > + HDEV_PARAM_U16(0x0003, def_inq_scan_type),
> > + HDEV_PARAM_U16(0x0004, def_inq_scan_int),
> > + HDEV_PARAM_U16(0x0005, def_inq_scan_window),
> > + HDEV_PARAM_U16(0x0006, def_br_lsto),
> > + HDEV_PARAM_U16(0x0007, def_page_timeout),
> > + HDEV_PARAM_U16(0x0008, sniff_min_interval),
> > + HDEV_PARAM_U16(0x0009, sniff_max_interval),
> > + HDEV_PARAM_U16(0x000a, le_adv_min_interval),
> > + HDEV_PARAM_U16(0x000b, le_adv_max_interval),
> > + HDEV_PARAM_U16(0x000c, def_multi_adv_rotation_duration),
> > + HDEV_PARAM_U16(0x000d, le_scan_interval),
> > + HDEV_PARAM_U16(0x000e, le_scan_window),
> > + HDEV_PARAM_U16(0x000f, le_scan_int_suspend),
> > + HDEV_PARAM_U16(0x0010, le_scan_window_suspend),
> > + HDEV_PARAM_U16(0x0011, le_scan_int_discovery),
> > + HDEV_PARAM_U16(0x0012, le_scan_window_discovery),
> > + HDEV_PARAM_U16(0x0013, le_scan_int_adv_monitor),
> > + HDEV_PARAM_U16(0x0014, le_scan_window_adv_monitor),
> > + HDEV_PARAM_U16(0x0015, le_scan_int_connect),
> > + HDEV_PARAM_U16(0x0016, le_scan_window_connect),
> > + HDEV_PARAM_U16(0x0017, le_conn_min_interval),
> > + HDEV_PARAM_U16(0x0018, le_conn_max_interval),
> > + HDEV_PARAM_U16(0x0019, le_conn_latency),
> > + HDEV_PARAM_U16(0x001a, le_supv_timeout),
> > + };
> > + struct mgmt_rp_read_def_system_config *rp = (void *)params;
> > +
> > + bt_dev_dbg(hdev, "sock %p", sk);
> > +
> > + return mgmt_cmd_complete(sk, hdev->id,
> > + MGMT_OP_READ_DEF_SYSTEM_CONFIG,
> > + 0, rp, sizeof(params));
> > +}
> > +
> > +#define TO_TLV(x) ((struct mgmt_tlv *)(x))
> > +#define TLV_GET_LE16(tlv) le16_to_cpu(*((__le16 *)(TO_TLV(tlv)->value)))
> > +
> > +int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> > + u16 data_len)
> > +{
> > + u16 buffer_left = data_len;
> > + u8 *buffer = data;
> > +
> > + if (buffer_left < sizeof(struct mgmt_tlv)) {
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> > + MGMT_STATUS_INVALID_PARAMS);
> > + }
> > +
> > + /* First pass to validate the tlv */
> > + while (buffer_left >= sizeof(struct mgmt_tlv)) {
> > + const u8 len = TO_TLV(buffer)->length;
> > + const u16 exp_len = sizeof(struct mgmt_tlv) +
> > + len;
> > + const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
> > +
> > + if (buffer_left < exp_len) {
> > + bt_dev_warn(hdev, "invalid len left %d, exp >= %d",
> > + buffer_left, exp_len);
> > +
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> > + MGMT_STATUS_INVALID_PARAMS);
> > + }
> > +
> > + /* Please see mgmt-api.txt for documentation of these values */
> > + switch (type) {
> > + case 0x0000:
> > + case 0x0001:
> > + case 0x0002:
> > + case 0x0003:
> > + case 0x0004:
> > + case 0x0005:
> > + case 0x0006:
> > + case 0x0007:
> > + case 0x0008:
> > + case 0x0009:
> > + case 0x000a:
> > + case 0x000b:
> > + case 0x000c:
> > + case 0x000d:
> > + case 0x000e:
> > + case 0x000f:
> > + case 0x0010:
> > + case 0x0011:
> > + case 0x0012:
> > + case 0x0013:
> > + case 0x0014:
> > + case 0x0015:
> > + case 0x0016:
> > + case 0x0017:
> > + case 0x0018:
> > + case 0x0019:
> > + case 0x001a:
> > + if (len != sizeof(u16)) {
> > + bt_dev_warn(hdev, "invalid length %d, exp %zu for type %d",
> > + len, sizeof(u16), type);
> > +
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> > + MGMT_STATUS_INVALID_PARAMS);
> > + }
> > + break;
> > + default:
> > + bt_dev_warn(hdev, "unsupported parameter %u", type);
> > + break;
> > + }
> > +
> > + buffer_left -= exp_len;
> > + buffer += exp_len;
> > + }
> > +
> > + buffer_left = data_len;
> > + buffer = data;
> > + while (buffer_left >= sizeof(struct mgmt_tlv)) {
> > + const u8 len = TO_TLV(buffer)->length;
> > + const u16 exp_len = sizeof(struct mgmt_tlv) +
> > + len;
> > + const u16 type = le16_to_cpu(TO_TLV(buffer)->type);
> > +
> > + switch (type) {
> > + case 0x0000:
> > + hdev->def_page_scan_type = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0001:
> > + hdev->def_page_scan_int = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0002:
> > + hdev->def_page_scan_window = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0003:
> > + hdev->def_inq_scan_type = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0004:
> > + hdev->def_inq_scan_int = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0005:
> > + hdev->def_inq_scan_window = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0006:
> > + hdev->def_br_lsto = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0007:
> > + hdev->def_page_timeout = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0008:
> > + hdev->sniff_min_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0009:
> > + hdev->sniff_max_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000a:
> > + hdev->le_adv_min_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000b:
> > + hdev->le_adv_max_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000c:
> > + hdev->def_multi_adv_rotation_duration =
> > + TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000d:
> > + hdev->le_scan_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000e:
> > + hdev->le_scan_window = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x000f:
> > + hdev->le_scan_int_suspend = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0010:
> > + hdev->le_scan_window_suspend = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0011:
> > + hdev->le_scan_int_discovery = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00012:
> > + hdev->le_scan_window_discovery = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00013:
> > + hdev->le_scan_int_adv_monitor = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00014:
> > + hdev->le_scan_window_adv_monitor = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00015:
> > + hdev->le_scan_int_connect = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00016:
> > + hdev->le_scan_window_connect = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00017:
> > + hdev->le_conn_min_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00018:
> > + hdev->le_conn_max_interval = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x00019:
> > + hdev->le_conn_latency = TLV_GET_LE16(buffer);
> > + break;
> > + case 0x0001a:
> > + hdev->le_supv_timeout = TLV_GET_LE16(buffer);
> > + break;
> > + default:
> > + bt_dev_warn(hdev, "unsupported parameter %u", type);
> > + break;
> > + }
> > +
> > + buffer_left -= exp_len;
> > + buffer += exp_len;
> > + }
> > +
> > + return mgmt_cmd_status(sk, hdev->id,
> > + MGMT_OP_SET_DEF_SYSTEM_CONFIG,
> > + MGMT_STATUS_SUCCESS);
> > +}
> > diff --git a/net/bluetooth/mgmt_config.h b/net/bluetooth/mgmt_config.h
> > new file mode 100644
> > index 000000000000..51da6e63b1a0
> > --- /dev/null
> > +++ b/net/bluetooth/mgmt_config.h
> > @@ -0,0 +1,11 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +/*
> > + * Copyright (C) 2020 Google Corporation
> > + */
> > +
> > +int read_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> > + u16 data_len);
> > +
> > +int set_def_system_config(struct sock *sk, struct hci_dev *hdev, void *data,
> > + u16 data_len);
>
> Other than that, patch has been applied to bluetooth-next tree.
>
> Regards
>
> Marcel
>