2020-11-30 23:13:09

by Daniel Winkler

[permalink] [raw]
Subject: [PATCH v6 0/5] Bluetooth: Add new MGMT interface for advertising add

Hi Maintainers,

This patch series defines the new two-call MGMT interface for adding
new advertising instances. Similarly to the hci advertising commands, a
mgmt call to set parameters is expected to be first, followed by a mgmt
call to set advertising data/scan response. The members of the
parameters request are optional; the caller defines a "params" bitfield
in the structure that indicates which parameters were intentionally set,
and others are set to defaults.

The main feature here is the introduction of min/max parameters and tx
power that can be requested by the client. Min/max parameters will be
used both with and without extended advertising support, and tx power
will be used with extended advertising support. After a call to set
advertising parameters, the selected transmission power will be
propagated in the reponse to alert userspace to the actual power used.

Additionally, to inform userspace of the controller LE Tx power
capabilities for the client's benefit, this series also changes the
security info MGMT command to more flexibly contain other capabilities,
such as LE min and max tx power.

All changes have been tested on hatch (extended advertising) and kukui
(no extended advertising) chromebooks with manual testing verifying
correctness of parameters/data in btmon traces, and our automated test
suite of 25 single- and multi-advertising usage scenarios.

A separate patch series will add support in bluetoothd. Thanks in
advance for your feedback!

Daniel Winkler


Changes in v6:
- Only populate LE tx power range if controller reports it

Changes in v5:
- Ensure data/scan rsp length is returned for non-ext adv

Changes in v4:
- Add remaining data and scan response length to MGMT params response
- Moving optional params into 'flags' field of MGMT command
- Combine LE tx range into a single EIR field for MGMT capabilities cmd

Changes in v3:
- Adding selected tx power to adv params mgmt response, removing event
- Re-using security info MGMT command to carry controller capabilities

Changes in v2:
- Fixed sparse error in Capabilities MGMT command

Daniel Winkler (5):
Bluetooth: Add helper to set adv data
Bluetooth: Break add adv into two mgmt commands
Bluetooth: Use intervals and tx power from mgmt cmds
Bluetooth: Query LE tx power on startup
Bluetooth: Change MGMT security info CMD to be more generic

include/net/bluetooth/hci.h | 7 +
include/net/bluetooth/hci_core.h | 12 +-
include/net/bluetooth/mgmt.h | 49 +++-
net/bluetooth/hci_core.c | 47 +++-
net/bluetooth/hci_event.c | 19 ++
net/bluetooth/hci_request.c | 29 ++-
net/bluetooth/mgmt.c | 430 +++++++++++++++++++++++++++++--
7 files changed, 548 insertions(+), 45 deletions(-)

--
2.29.2.454.gaff20da3a2-goog


2020-11-30 23:13:41

by Daniel Winkler

[permalink] [raw]
Subject: [PATCH v6 3/5] Bluetooth: Use intervals and tx power from mgmt cmds

This patch takes the min/max intervals and tx power optionally provided
in mgmt interface, stores them in the advertisement struct, and uses
them when configuring the hci requests. While tx power is not used if
extended advertising is unavailable, software rotation will use the min
and max advertising intervals specified by the client.

This change is validated manually by ensuring the min/max intervals are
propagated to the controller on both hatch (extended advertising) and
kukui (no extended advertising) chromebooks, and that tx power is
propagated correctly on hatch. These tests are performed with multiple
advertisements simultaneously.

Reviewed-by: Sonny Sasaka <[email protected]>
Signed-off-by: Daniel Winkler <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

include/net/bluetooth/hci_core.h | 5 ++++-
net/bluetooth/hci_core.c | 8 +++++---
net/bluetooth/hci_request.c | 29 +++++++++++++++++++----------
net/bluetooth/mgmt.c | 8 ++++++--
4 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 48d144ae8b57d6..ab168f46b6d909 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -230,6 +230,8 @@ struct adv_info {
__u16 scan_rsp_len;
__u8 scan_rsp_data[HCI_MAX_AD_LENGTH];
__s8 tx_power;
+ __u32 min_interval;
+ __u32 max_interval;
bdaddr_t random_addr;
bool rpa_expired;
struct delayed_work rpa_expired_cb;
@@ -1292,7 +1294,8 @@ struct adv_info *hci_get_next_instance(struct hci_dev *hdev, u8 instance);
int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
u16 adv_data_len, u8 *adv_data,
u16 scan_rsp_len, u8 *scan_rsp_data,
- u16 timeout, u16 duration);
+ u16 timeout, u16 duration, s8 tx_power,
+ u32 min_interval, u32 max_interval);
int hci_set_adv_instance_data(struct hci_dev *hdev, u8 instance,
u16 adv_data_len, u8 *adv_data,
u16 scan_rsp_len, u8 *scan_rsp_data);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 35afb63514f38b..3397fc706e87a1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2951,7 +2951,8 @@ static void adv_instance_rpa_expired(struct work_struct *work)
int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
u16 adv_data_len, u8 *adv_data,
u16 scan_rsp_len, u8 *scan_rsp_data,
- u16 timeout, u16 duration)
+ u16 timeout, u16 duration, s8 tx_power,
+ u32 min_interval, u32 max_interval)
{
struct adv_info *adv_instance;

@@ -2979,6 +2980,9 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
adv_instance->flags = flags;
adv_instance->adv_data_len = adv_data_len;
adv_instance->scan_rsp_len = scan_rsp_len;
+ adv_instance->min_interval = min_interval;
+ adv_instance->max_interval = max_interval;
+ adv_instance->tx_power = tx_power;

if (adv_data_len)
memcpy(adv_instance->adv_data, adv_data, adv_data_len);
@@ -2995,8 +2999,6 @@ int hci_add_adv_instance(struct hci_dev *hdev, u8 instance, u32 flags,
else
adv_instance->duration = duration;

- adv_instance->tx_power = HCI_TX_POWER_INVALID;
-
INIT_DELAYED_WORK(&adv_instance->rpa_expired_cb,
adv_instance_rpa_expired);

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 048d4db9d4ea53..4e31f39df96838 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1443,6 +1443,7 @@ static bool is_advertising_allowed(struct hci_dev *hdev, bool connectable)
void __hci_req_enable_advertising(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;
+ struct adv_info *adv_instance;
struct hci_cp_le_set_adv_param cp;
u8 own_addr_type, enable = 0x01;
bool connectable;
@@ -1450,6 +1451,7 @@ void __hci_req_enable_advertising(struct hci_request *req)
u32 flags;

flags = get_adv_instance_flags(hdev, hdev->cur_adv_instance);
+ adv_instance = hci_find_adv_instance(hdev, hdev->cur_adv_instance);

/* If the "connectable" instance flag was not set, then choose between
* ADV_IND and ADV_NONCONN_IND based on the global connectable setting.
@@ -1481,11 +1483,16 @@ void __hci_req_enable_advertising(struct hci_request *req)

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

- if (connectable) {
- cp.type = LE_ADV_IND;
-
+ if (adv_instance) {
+ adv_min_interval = adv_instance->min_interval;
+ adv_max_interval = adv_instance->max_interval;
+ } else {
adv_min_interval = hdev->le_adv_min_interval;
adv_max_interval = hdev->le_adv_max_interval;
+ }
+
+ if (connectable) {
+ cp.type = LE_ADV_IND;
} else {
if (get_cur_adv_instance_scan_rsp_len(hdev))
cp.type = LE_ADV_SCAN_IND;
@@ -1496,9 +1503,6 @@ void __hci_req_enable_advertising(struct hci_request *req)
hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE)) {
adv_min_interval = DISCOV_LE_FAST_ADV_INT_MIN;
adv_max_interval = DISCOV_LE_FAST_ADV_INT_MAX;
- } else {
- adv_min_interval = hdev->le_adv_min_interval;
- adv_max_interval = hdev->le_adv_max_interval;
}
}

@@ -2021,9 +2025,15 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)

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

- /* In ext adv set param interval is 3 octets */
- hci_cpu_to_le24(hdev->le_adv_min_interval, cp.min_interval);
- hci_cpu_to_le24(hdev->le_adv_max_interval, cp.max_interval);
+ if (adv_instance) {
+ hci_cpu_to_le24(adv_instance->min_interval, cp.min_interval);
+ hci_cpu_to_le24(adv_instance->max_interval, cp.max_interval);
+ cp.tx_power = adv_instance->tx_power;
+ } else {
+ hci_cpu_to_le24(hdev->le_adv_min_interval, cp.min_interval);
+ hci_cpu_to_le24(hdev->le_adv_max_interval, cp.max_interval);
+ cp.tx_power = HCI_ADV_TX_POWER_NO_PREFERENCE;
+ }

secondary_adv = (flags & MGMT_ADV_FLAG_SEC_MASK);

@@ -2046,7 +2056,6 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)

cp.own_addr_type = own_addr_type;
cp.channel_map = hdev->le_adv_channel_map;
- cp.tx_power = 127;
cp.handle = instance;

if (flags & MGMT_ADV_FLAG_SEC_2M) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ec6b520be368be..668a62c8181eb1 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7533,7 +7533,10 @@ static int add_advertising(struct sock *sk, struct hci_dev *hdev,
cp->adv_data_len, cp->data,
cp->scan_rsp_len,
cp->data + cp->adv_data_len,
- timeout, duration);
+ timeout, duration,
+ HCI_ADV_TX_POWER_NO_PREFERENCE,
+ hdev->le_adv_min_interval,
+ hdev->le_adv_max_interval);
if (err < 0) {
err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_ADVERTISING,
MGMT_STATUS_FAILED);
@@ -7741,7 +7744,8 @@ static int add_ext_adv_params(struct sock *sk, struct hci_dev *hdev,

/* Create advertising instance with no advertising or response data */
err = hci_add_adv_instance(hdev, cp->instance, flags,
- 0, NULL, 0, NULL, timeout, duration);
+ 0, NULL, 0, NULL, timeout, duration,
+ tx_power, min_interval, max_interval);

if (err < 0) {
err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_ADD_EXT_ADV_PARAMS,
--
2.29.2.454.gaff20da3a2-goog

2020-11-30 23:13:43

by Daniel Winkler

[permalink] [raw]
Subject: [PATCH v6 5/5] Bluetooth: Change MGMT security info CMD to be more generic

For advertising, we wish to know the LE tx power capabilities of the
controller in userspace, so this patch edits the Security Info MGMT
command to be more generic, such that other various controller
capabilities can be included in the EIR data. This change also includes
the LE min and max tx power into this newly-named command.

The change was tested by manually verifying that the MGMT command
returns the tx power range as expected in userspace.

Reviewed-by: Sonny Sasaka <[email protected]>
Signed-off-by: Daniel Winkler <[email protected]>
---

Changes in v6:
- Only populate LE tx power range if controller reports it

Changes in v5: None
Changes in v4:
- Combine LE tx range into a single EIR field for MGMT capabilities cmd

Changes in v3:
- Re-using security info MGMT command to carry controller capabilities

Changes in v2:
- Fixed sparse error in Capabilities MGMT command

include/net/bluetooth/mgmt.h | 15 ++++++++-----
net/bluetooth/mgmt.c | 43 ++++++++++++++++++++++++------------
2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 2e18e4173e2fa5..f9a6638e20b3c6 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -686,11 +686,16 @@ struct mgmt_cp_set_blocked_keys {

#define MGMT_OP_SET_WIDEBAND_SPEECH 0x0047

-#define MGMT_OP_READ_SECURITY_INFO 0x0048
-#define MGMT_READ_SECURITY_INFO_SIZE 0
-struct mgmt_rp_read_security_info {
- __le16 sec_len;
- __u8 sec[];
+#define MGMT_CAP_SEC_FLAGS 0x01
+#define MGMT_CAP_MAX_ENC_KEY_SIZE 0x02
+#define MGMT_CAP_SMP_MAX_ENC_KEY_SIZE 0x03
+#define MGMT_CAP_LE_TX_PWR 0x04
+
+#define MGMT_OP_READ_CONTROLLER_CAP 0x0048
+#define MGMT_READ_CONTROLLER_CAP_SIZE 0
+struct mgmt_rp_read_controller_cap {
+ __le16 cap_len;
+ __u8 cap[0];
} __packed;

#define MGMT_OP_READ_EXP_FEATURES_INFO 0x0049
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 668a62c8181eb1..754489e4e0655c 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -110,7 +110,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_SET_APPEARANCE,
MGMT_OP_SET_BLOCKED_KEYS,
MGMT_OP_SET_WIDEBAND_SPEECH,
- MGMT_OP_READ_SECURITY_INFO,
+ MGMT_OP_READ_CONTROLLER_CAP,
MGMT_OP_READ_EXP_FEATURES_INFO,
MGMT_OP_SET_EXP_FEATURE,
MGMT_OP_READ_DEF_SYSTEM_CONFIG,
@@ -176,7 +176,7 @@ static const u16 mgmt_untrusted_commands[] = {
MGMT_OP_READ_CONFIG_INFO,
MGMT_OP_READ_EXT_INDEX_LIST,
MGMT_OP_READ_EXT_INFO,
- MGMT_OP_READ_SECURITY_INFO,
+ MGMT_OP_READ_CONTROLLER_CAP,
MGMT_OP_READ_EXP_FEATURES_INFO,
MGMT_OP_READ_DEF_SYSTEM_CONFIG,
MGMT_OP_READ_DEF_RUNTIME_CONFIG,
@@ -3710,13 +3710,14 @@ static int set_wideband_speech(struct sock *sk, struct hci_dev *hdev,
return err;
}

-static int read_security_info(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 data_len)
+static int read_controller_cap(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 data_len)
{
- char buf[16];
- struct mgmt_rp_read_security_info *rp = (void *)buf;
- u16 sec_len = 0;
+ char buf[20];
+ struct mgmt_rp_read_controller_cap *rp = (void *)buf;
+ u16 cap_len = 0;
u8 flags = 0;
+ u8 tx_power_range[2];

bt_dev_dbg(hdev, "sock %p", sk);

@@ -3740,23 +3741,37 @@ static int read_security_info(struct sock *sk, struct hci_dev *hdev,

flags |= 0x08; /* Encryption key size enforcement (LE) */

- sec_len = eir_append_data(rp->sec, sec_len, 0x01, &flags, 1);
+ cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_SEC_FLAGS,
+ &flags, 1);

/* When the Read Simple Pairing Options command is supported, then
* also max encryption key size information is provided.
*/
if (hdev->commands[41] & 0x08)
- sec_len = eir_append_le16(rp->sec, sec_len, 0x02,
+ cap_len = eir_append_le16(rp->cap, cap_len,
+ MGMT_CAP_MAX_ENC_KEY_SIZE,
hdev->max_enc_key_size);

- sec_len = eir_append_le16(rp->sec, sec_len, 0x03, SMP_MAX_ENC_KEY_SIZE);
+ cap_len = eir_append_le16(rp->cap, cap_len,
+ MGMT_CAP_SMP_MAX_ENC_KEY_SIZE,
+ SMP_MAX_ENC_KEY_SIZE);
+
+ /* Append the min/max LE tx power parameters if we were able to fetch
+ * it from the controller
+ */
+ if (hdev->commands[38] & 0x80) {
+ memcpy(&tx_power_range[0], &hdev->min_le_tx_power, 1);
+ memcpy(&tx_power_range[1], &hdev->max_le_tx_power, 1);
+ cap_len = eir_append_data(rp->cap, cap_len, MGMT_CAP_LE_TX_PWR,
+ tx_power_range, 2);
+ }

- rp->sec_len = cpu_to_le16(sec_len);
+ rp->cap_len = cpu_to_le16(cap_len);

hci_dev_unlock(hdev);

- return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_READ_SECURITY_INFO, 0,
- rp, sizeof(*rp) + sec_len);
+ return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_READ_CONTROLLER_CAP, 0,
+ rp, sizeof(*rp) + cap_len);
}

#ifdef CONFIG_BT_FEATURE_DEBUG
@@ -8193,7 +8208,7 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
{ set_blocked_keys, MGMT_OP_SET_BLOCKED_KEYS_SIZE,
HCI_MGMT_VAR_LEN },
{ set_wideband_speech, MGMT_SETTING_SIZE },
- { read_security_info, MGMT_READ_SECURITY_INFO_SIZE,
+ { read_controller_cap, MGMT_READ_CONTROLLER_CAP_SIZE,
HCI_MGMT_UNTRUSTED },
{ read_exp_features_info, MGMT_READ_EXP_FEATURES_INFO_SIZE,
HCI_MGMT_UNTRUSTED |
--
2.29.2.454.gaff20da3a2-goog

2020-11-30 23:14:42

by Daniel Winkler

[permalink] [raw]
Subject: [PATCH v6 4/5] Bluetooth: Query LE tx power on startup

Queries tx power via HCI_LE_Read_Transmit_Power command when the hci
device is initialized, and stores resulting min/max LE power in hdev
struct. If command isn't available (< BT5 support), min/max values
both default to HCI_TX_POWER_INVALID.

This patch is manually verified by ensuring BT5 devices correctly query
and receive controller tx power range.

Reviewed-by: Sonny Sasaka <[email protected]>
Signed-off-by: Daniel Winkler <[email protected]>
---

Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

include/net/bluetooth/hci.h | 7 +++++++
include/net/bluetooth/hci_core.h | 2 ++
net/bluetooth/hci_core.c | 8 ++++++++
net/bluetooth/hci_event.c | 18 ++++++++++++++++++
4 files changed, 35 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c8e67042a3b14c..c1504aa3d9cfd5 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1797,6 +1797,13 @@ struct hci_cp_le_set_adv_set_rand_addr {
bdaddr_t bdaddr;
} __packed;

+#define HCI_OP_LE_READ_TRANSMIT_POWER 0x204b
+struct hci_rp_le_read_transmit_power {
+ __u8 status;
+ __s8 min_le_tx_power;
+ __s8 max_le_tx_power;
+} __packed;
+
#define HCI_OP_LE_READ_BUFFER_SIZE_V2 0x2060
struct hci_rp_le_read_buffer_size_v2 {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ab168f46b6d909..9463039f85442c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -381,6 +381,8 @@ struct hci_dev {
__u16 def_page_timeout;
__u16 def_multi_adv_rotation_duration;
__u16 def_le_autoconnect_timeout;
+ __s8 min_le_tx_power;
+ __s8 max_le_tx_power;

__u16 pkt_type;
__u16 esco_type;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3397fc706e87a1..6f7d5ce965d7c8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -741,6 +741,12 @@ static int hci_init3_req(struct hci_request *req, unsigned long opt)
hci_req_add(req, HCI_OP_LE_READ_ADV_TX_POWER, 0, NULL);
}

+ if (hdev->commands[38] & 0x80) {
+ /* Read LE Min/Max Tx Power*/
+ hci_req_add(req, HCI_OP_LE_READ_TRANSMIT_POWER,
+ 0, NULL);
+ }
+
if (hdev->commands[26] & 0x40) {
/* Read LE White List Size */
hci_req_add(req, HCI_OP_LE_READ_WHITE_LIST_SIZE,
@@ -3656,6 +3662,8 @@ struct hci_dev *hci_alloc_dev(void)
hdev->le_num_of_adv_sets = HCI_MAX_ADV_INSTANCES;
hdev->def_multi_adv_rotation_duration = HCI_DEFAULT_ADV_DURATION;
hdev->def_le_autoconnect_timeout = HCI_LE_AUTOCONN_TIMEOUT;
+ hdev->min_le_tx_power = HCI_TX_POWER_INVALID;
+ hdev->max_le_tx_power = HCI_TX_POWER_INVALID;

hdev->rpa_timeout = HCI_DEFAULT_RPA_TIMEOUT;
hdev->discov_interleaved_timeout = DISCOV_INTERLEAVED_TIMEOUT;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index f193e73ef47c14..67668be3461e93 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1202,6 +1202,20 @@ static void hci_cc_le_set_adv_set_random_addr(struct hci_dev *hdev,
hci_dev_unlock(hdev);
}

+static void hci_cc_le_read_transmit_power(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_rp_le_read_transmit_power *rp = (void *)skb->data;
+
+ BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
+
+ if (rp->status)
+ return;
+
+ hdev->min_le_tx_power = rp->min_le_tx_power;
+ hdev->max_le_tx_power = rp->max_le_tx_power;
+}
+
static void hci_cc_le_set_adv_enable(struct hci_dev *hdev, struct sk_buff *skb)
{
__u8 *sent, status = *((__u8 *) skb->data);
@@ -3582,6 +3596,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cc_le_set_adv_set_random_addr(hdev, skb);
break;

+ case HCI_OP_LE_READ_TRANSMIT_POWER:
+ hci_cc_le_read_transmit_power(hdev, skb);
+ break;
+
default:
BT_DBG("%s opcode 0x%4.4x", hdev->name, *opcode);
break;
--
2.29.2.454.gaff20da3a2-goog

2020-12-03 12:49:55

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] Bluetooth: Add new MGMT interface for advertising add

Hi Daniel,

> This patch series defines the new two-call MGMT interface for adding
> new advertising instances. Similarly to the hci advertising commands, a
> mgmt call to set parameters is expected to be first, followed by a mgmt
> call to set advertising data/scan response. The members of the
> parameters request are optional; the caller defines a "params" bitfield
> in the structure that indicates which parameters were intentionally set,
> and others are set to defaults.
>
> The main feature here is the introduction of min/max parameters and tx
> power that can be requested by the client. Min/max parameters will be
> used both with and without extended advertising support, and tx power
> will be used with extended advertising support. After a call to set
> advertising parameters, the selected transmission power will be
> propagated in the reponse to alert userspace to the actual power used.
>
> Additionally, to inform userspace of the controller LE Tx power
> capabilities for the client's benefit, this series also changes the
> security info MGMT command to more flexibly contain other capabilities,
> such as LE min and max tx power.
>
> All changes have been tested on hatch (extended advertising) and kukui
> (no extended advertising) chromebooks with manual testing verifying
> correctness of parameters/data in btmon traces, and our automated test
> suite of 25 single- and multi-advertising usage scenarios.
>
> A separate patch series will add support in bluetoothd. Thanks in
> advance for your feedback!
>
> Daniel Winkler
>
>
> Changes in v6:
> - Only populate LE tx power range if controller reports it
>
> Changes in v5:
> - Ensure data/scan rsp length is returned for non-ext adv
>
> Changes in v4:
> - Add remaining data and scan response length to MGMT params response
> - Moving optional params into 'flags' field of MGMT command
> - Combine LE tx range into a single EIR field for MGMT capabilities cmd
>
> Changes in v3:
> - Adding selected tx power to adv params mgmt response, removing event
> - Re-using security info MGMT command to carry controller capabilities
>
> Changes in v2:
> - Fixed sparse error in Capabilities MGMT command
>
> Daniel Winkler (5):
> Bluetooth: Add helper to set adv data
> Bluetooth: Break add adv into two mgmt commands
> Bluetooth: Use intervals and tx power from mgmt cmds
> Bluetooth: Query LE tx power on startup
> Bluetooth: Change MGMT security info CMD to be more generic
>
> include/net/bluetooth/hci.h | 7 +
> include/net/bluetooth/hci_core.h | 12 +-
> include/net/bluetooth/mgmt.h | 49 +++-
> net/bluetooth/hci_core.c | 47 +++-
> net/bluetooth/hci_event.c | 19 ++
> net/bluetooth/hci_request.c | 29 ++-
> net/bluetooth/mgmt.c | 430 +++++++++++++++++++++++++++++--
> 7 files changed, 548 insertions(+), 45 deletions(-)

I am having problem with patch 3/5 which does not apply cleanly against bluetooth-next. Can you please fix it and re-send. Thanks.

Regards

Marcel

2020-12-03 20:17:52

by Daniel Winkler

[permalink] [raw]
Subject: Re: [PATCH v6 0/5] Bluetooth: Add new MGMT interface for advertising add

Hi Marcel,

I have just rebased onto bluetooth-next/master (commit
e2e03d27190561976f2631c36337299645f7c1a2 from Dec 3) - oddly, I didn't
have any conflicts in my rebase. Please let me know if the issue
continues.

Thanks!
Daniel

On Thu, Dec 3, 2020 at 4:47 AM Marcel Holtmann <[email protected]> wrote:
>
> Hi Daniel,
>
> > This patch series defines the new two-call MGMT interface for adding
> > new advertising instances. Similarly to the hci advertising commands, a
> > mgmt call to set parameters is expected to be first, followed by a mgmt
> > call to set advertising data/scan response. The members of the
> > parameters request are optional; the caller defines a "params" bitfield
> > in the structure that indicates which parameters were intentionally set,
> > and others are set to defaults.
> >
> > The main feature here is the introduction of min/max parameters and tx
> > power that can be requested by the client. Min/max parameters will be
> > used both with and without extended advertising support, and tx power
> > will be used with extended advertising support. After a call to set
> > advertising parameters, the selected transmission power will be
> > propagated in the reponse to alert userspace to the actual power used.
> >
> > Additionally, to inform userspace of the controller LE Tx power
> > capabilities for the client's benefit, this series also changes the
> > security info MGMT command to more flexibly contain other capabilities,
> > such as LE min and max tx power.
> >
> > All changes have been tested on hatch (extended advertising) and kukui
> > (no extended advertising) chromebooks with manual testing verifying
> > correctness of parameters/data in btmon traces, and our automated test
> > suite of 25 single- and multi-advertising usage scenarios.
> >
> > A separate patch series will add support in bluetoothd. Thanks in
> > advance for your feedback!
> >
> > Daniel Winkler
> >
> >
> > Changes in v6:
> > - Only populate LE tx power range if controller reports it
> >
> > Changes in v5:
> > - Ensure data/scan rsp length is returned for non-ext adv
> >
> > Changes in v4:
> > - Add remaining data and scan response length to MGMT params response
> > - Moving optional params into 'flags' field of MGMT command
> > - Combine LE tx range into a single EIR field for MGMT capabilities cmd
> >
> > Changes in v3:
> > - Adding selected tx power to adv params mgmt response, removing event
> > - Re-using security info MGMT command to carry controller capabilities
> >
> > Changes in v2:
> > - Fixed sparse error in Capabilities MGMT command
> >
> > Daniel Winkler (5):
> > Bluetooth: Add helper to set adv data
> > Bluetooth: Break add adv into two mgmt commands
> > Bluetooth: Use intervals and tx power from mgmt cmds
> > Bluetooth: Query LE tx power on startup
> > Bluetooth: Change MGMT security info CMD to be more generic
> >
> > include/net/bluetooth/hci.h | 7 +
> > include/net/bluetooth/hci_core.h | 12 +-
> > include/net/bluetooth/mgmt.h | 49 +++-
> > net/bluetooth/hci_core.c | 47 +++-
> > net/bluetooth/hci_event.c | 19 ++
> > net/bluetooth/hci_request.c | 29 ++-
> > net/bluetooth/mgmt.c | 430 +++++++++++++++++++++++++++++--
> > 7 files changed, 548 insertions(+), 45 deletions(-)
>
> I am having problem with patch 3/5 which does not apply cleanly against bluetooth-next. Can you please fix it and re-send. Thanks.
>
> Regards
>
> Marcel
>