* [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support
2020-12-16 4:33 [PATCH v3 0/5] MSFT offloading support for advertisement monitor Archie Pusaka
@ 2020-12-16 4:33 ` Archie Pusaka
2020-12-16 5:14 ` MSFT offloading support for advertisement monitor bluez.test.bot
2020-12-21 9:05 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Marcel Holtmann
2020-12-16 4:33 ` [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor Archie Pusaka
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Archie Pusaka @ 2020-12-16 4:33 UTC (permalink / raw)
To: linux-bluetooth, Marcel Holtmann
Cc: CrosBT Upstreaming, Archie Pusaka, Manish Mandlik,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, linux-kernel, netdev
From: Archie Pusaka <apusaka@chromium.org>
MSFT needs rssi parameter for monitoring advertisement packet,
therefore we should supply them from mgmt. This adds a new opcode
to add advertisement monitor with rssi parameters.
Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---
Changes in v3:
* Flips the order of rssi and pattern_count on mgmt struct
Changes in v2:
* Add a new opcode instead of modifying an existing one
include/net/bluetooth/hci_core.h | 9 +++
include/net/bluetooth/mgmt.h | 16 ++++++
net/bluetooth/mgmt.c | 99 ++++++++++++++++++++++++--------
3 files changed, 101 insertions(+), 23 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 677a8c50b2ad..8b7cf3620938 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -250,8 +250,17 @@ struct adv_pattern {
__u8 value[HCI_MAX_AD_LENGTH];
};
+struct adv_rssi_thresholds {
+ __s8 low_threshold;
+ __s8 high_threshold;
+ __u16 low_threshold_timeout;
+ __u16 high_threshold_timeout;
+ __u8 sampling_period;
+};
+
struct adv_monitor {
struct list_head patterns;
+ struct adv_rssi_thresholds rssi;
bool active;
__u16 handle;
};
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index f9a6638e20b3..9917b911a4fb 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
__u8 instance;
} __packed;
+struct mgmt_adv_rssi_thresholds {
+ __s8 high_threshold;
+ __le16 high_threshold_timeout;
+ __s8 low_threshold;
+ __le16 low_threshold_timeout;
+ __u8 sampling_period;
+} __packed;
+
+#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
+struct mgmt_cp_add_adv_patterns_monitor_rssi {
+ struct mgmt_adv_rssi_thresholds rssi;
+ __u8 pattern_count;
+ struct mgmt_adv_pattern patterns[];
+} __packed;
+#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE 8
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index fa0f7a4a1d2f..cd574054aa39 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
MGMT_OP_REMOVE_ADV_MONITOR,
MGMT_OP_ADD_EXT_ADV_PARAMS,
MGMT_OP_ADD_EXT_ADV_DATA,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
};
static const u16 mgmt_events[] = {
@@ -4225,22 +4226,40 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
return err;
}
-static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
- void *data, u16 len)
+static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len, u16 op)
{
- struct mgmt_cp_add_adv_patterns_monitor *cp = data;
+ struct mgmt_cp_add_adv_patterns_monitor *cp = NULL;
+ struct mgmt_cp_add_adv_patterns_monitor_rssi *cp_rssi = NULL;
struct mgmt_rp_add_adv_patterns_monitor rp;
+ struct mgmt_adv_rssi_thresholds *rssi = NULL;
+ struct mgmt_adv_pattern *patterns = NULL;
struct adv_monitor *m = NULL;
struct adv_pattern *p = NULL;
unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
__u8 cp_ofst = 0, cp_len = 0;
int err, i;
+ u8 pattern_count;
+ u16 expected_len;
BT_DBG("request for %s", hdev->name);
- if (len <= sizeof(*cp) || cp->pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
+ cp_rssi = data;
+ pattern_count = cp_rssi->pattern_count;
+ rssi = &cp_rssi->rssi;
+ patterns = cp_rssi->patterns;
+ expected_len = sizeof(*cp_rssi) +
+ pattern_count * sizeof(*patterns);
+ } else {
+ cp = data;
+ pattern_count = cp->pattern_count;
+ patterns = cp->patterns;
+ expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
+ }
+
+ if (len != expected_len || pattern_count == 0) {
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
@@ -4254,21 +4273,40 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
INIT_LIST_HEAD(&m->patterns);
m->active = false;
- for (i = 0; i < cp->pattern_count; i++) {
+ if (rssi) {
+ m->rssi.low_threshold = rssi->low_threshold;
+ m->rssi.low_threshold_timeout =
+ __le16_to_cpu(rssi->low_threshold_timeout);
+ m->rssi.high_threshold = rssi->high_threshold;
+ m->rssi.high_threshold_timeout =
+ __le16_to_cpu(rssi->high_threshold_timeout);
+ m->rssi.sampling_period = rssi->sampling_period;
+ } else {
+ /* Default values. These numbers are the least constricting
+ * parameters for MSFT API to work, so it behaves as if there
+ * are no rssi parameter to consider. May need to be changed
+ * if other API are to be supported.
+ */
+ m->rssi.low_threshold = -127;
+ m->rssi.low_threshold_timeout = 60;
+ m->rssi.high_threshold = -127;
+ m->rssi.high_threshold_timeout = 0;
+ m->rssi.sampling_period = 0;
+ }
+
+ for (i = 0; i < pattern_count; i++) {
if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
- cp_ofst = cp->patterns[i].offset;
- cp_len = cp->patterns[i].length;
+ cp_ofst = patterns[i].offset;
+ cp_len = patterns[i].length;
if (cp_ofst >= HCI_MAX_AD_LENGTH ||
cp_len > HCI_MAX_AD_LENGTH ||
(cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
@@ -4279,18 +4317,17 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
goto failed;
}
- p->ad_type = cp->patterns[i].ad_type;
- p->offset = cp->patterns[i].offset;
- p->length = cp->patterns[i].length;
- memcpy(p->value, cp->patterns[i].value, p->length);
+ p->ad_type = patterns[i].ad_type;
+ p->offset = patterns[i].offset;
+ p->length = patterns[i].length;
+ memcpy(p->value, patterns[i].value, p->length);
INIT_LIST_HEAD(&p->list);
list_add(&p->list, &m->patterns);
}
- if (mp_cnt != cp->pattern_count) {
- err = mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ if (mp_cnt != pattern_count) {
+ err = mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_INVALID_PARAMS);
goto failed;
}
@@ -4302,8 +4339,7 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
err = hci_add_adv_monitor(hdev, m);
if (err) {
if (err == -ENOSPC) {
- mgmt_cmd_status(sk, hdev->id,
- MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ mgmt_cmd_status(sk, hdev->id, op,
MGMT_STATUS_NO_RESOURCES);
}
goto unlock;
@@ -4316,7 +4352,7 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
rp.monitor_handle = cpu_to_le16(m->handle);
- return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
+ return mgmt_cmd_complete(sk, hdev->id, op,
MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
unlock:
@@ -4327,6 +4363,20 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
return err;
}
+static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len)
+{
+ return __add_adv_patterns_monitor(sk, hdev, data, len,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR);
+}
+
+static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev,
+ void *data, u16 len)
+{
+ return __add_adv_patterns_monitor(sk, hdev, data, len,
+ MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI);
+}
+
static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len)
{
@@ -8234,6 +8284,9 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
HCI_MGMT_VAR_LEN },
{ add_ext_adv_data, MGMT_ADD_EXT_ADV_DATA_SIZE,
HCI_MGMT_VAR_LEN },
+ { add_adv_patterns_monitor_rssi,
+ MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE,
+ HCI_MGMT_VAR_LEN },
};
void mgmt_index_added(struct hci_dev *hdev)
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: MSFT offloading support for advertisement monitor
2020-12-16 4:33 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Archie Pusaka
@ 2020-12-16 5:14 ` bluez.test.bot
2020-12-21 9:05 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Marcel Holtmann
1 sibling, 0 replies; 13+ messages in thread
From: bluez.test.bot @ 2020-12-16 5:14 UTC (permalink / raw)
To: linux-bluetooth, apusaka
[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]
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=402657
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - FAIL
workflow: Add workflow files for ci
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
Bluetooth: advmon offload MSFT add rssi support
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
Bluetooth: advmon offload MSFT add monitor
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
Bluetooth: advmon offload MSFT remove monitor
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
Bluetooth: advmon offload MSFT handle controller reset
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
Bluetooth: advmon offload MSFT handle filter enablement
1: T1 Title exceeds max length (92>72): "Merge ba28ee5f173c8807a7660ca31bdb60c3b73d8e04 into 0760187a715ce4a520f59ef74e8d3fa9d7b924e9"
3: B6 Body message is missing
##############################
Test: CheckBuildK - PASS
##############################
Test: CheckTestRunner: Setup - PASS
##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6
##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: mgmt-tester - FAIL
Total: 416, Passed: 394 (94.7%), Failed: 8, Not Run: 14
##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43344 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3534 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 548209 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11655 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9890 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11801 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5432 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support
2020-12-16 4:33 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Archie Pusaka
2020-12-16 5:14 ` MSFT offloading support for advertisement monitor bluez.test.bot
@ 2020-12-21 9:05 ` Marcel Holtmann
1 sibling, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2020-12-21 9:05 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Manish Mandlik, Miao-chen Chou, Yun-Hao Chung, David S. Miller,
Jakub Kicinski, Johan Hedberg, Luiz Augusto von Dentz, LKML,
netdev
Hi Archie,
> MSFT needs rssi parameter for monitoring advertisement packet,
> therefore we should supply them from mgmt. This adds a new opcode
> to add advertisement monitor with rssi parameters.
>
> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
>
> ---
>
> Changes in v3:
> * Flips the order of rssi and pattern_count on mgmt struct
>
> Changes in v2:
> * Add a new opcode instead of modifying an existing one
>
> include/net/bluetooth/hci_core.h | 9 +++
> include/net/bluetooth/mgmt.h | 16 ++++++
> net/bluetooth/mgmt.c | 99 ++++++++++++++++++++++++--------
> 3 files changed, 101 insertions(+), 23 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 677a8c50b2ad..8b7cf3620938 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -250,8 +250,17 @@ struct adv_pattern {
> __u8 value[HCI_MAX_AD_LENGTH];
> };
>
> +struct adv_rssi_thresholds {
> + __s8 low_threshold;
> + __s8 high_threshold;
> + __u16 low_threshold_timeout;
> + __u16 high_threshold_timeout;
> + __u8 sampling_period;
> +};
> +
> struct adv_monitor {
> struct list_head patterns;
> + struct adv_rssi_thresholds rssi;
> bool active;
> __u16 handle;
> };
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index f9a6638e20b3..9917b911a4fb 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -821,6 +821,22 @@ struct mgmt_rp_add_ext_adv_data {
> __u8 instance;
> } __packed;
>
> +struct mgmt_adv_rssi_thresholds {
> + __s8 high_threshold;
> + __le16 high_threshold_timeout;
> + __s8 low_threshold;
> + __le16 low_threshold_timeout;
> + __u8 sampling_period;
> +} __packed;
> +
please indent this in a away that it stays readable.
__s8 a
__le16 b
..
> +#define MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI 0x0056
> +struct mgmt_cp_add_adv_patterns_monitor_rssi {
> + struct mgmt_adv_rssi_thresholds rssi;
> + __u8 pattern_count;
> + struct mgmt_adv_pattern patterns[];
> +} __packed;
> +#define MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE 8
> +
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index fa0f7a4a1d2f..cd574054aa39 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -124,6 +124,7 @@ static const u16 mgmt_commands[] = {
> MGMT_OP_REMOVE_ADV_MONITOR,
> MGMT_OP_ADD_EXT_ADV_PARAMS,
> MGMT_OP_ADD_EXT_ADV_DATA,
> + MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI,
> };
>
> static const u16 mgmt_events[] = {
> @@ -4225,22 +4226,40 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> -static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> - void *data, u16 len)
> +static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 len, u16 op)
> {
> - struct mgmt_cp_add_adv_patterns_monitor *cp = data;
> + struct mgmt_cp_add_adv_patterns_monitor *cp = NULL;
> + struct mgmt_cp_add_adv_patterns_monitor_rssi *cp_rssi = NULL;
> struct mgmt_rp_add_adv_patterns_monitor rp;
> + struct mgmt_adv_rssi_thresholds *rssi = NULL;
> + struct mgmt_adv_pattern *patterns = NULL;
> struct adv_monitor *m = NULL;
> struct adv_pattern *p = NULL;
> unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
> __u8 cp_ofst = 0, cp_len = 0;
> int err, i;
> + u8 pattern_count;
> + u16 expected_len;
>
> BT_DBG("request for %s", hdev->name);
>
> - if (len <= sizeof(*cp) || cp->pattern_count == 0) {
> - err = mgmt_cmd_status(sk, hdev->id,
> - MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
> + cp_rssi = data;
> + pattern_count = cp_rssi->pattern_count;
> + rssi = &cp_rssi->rssi;
> + patterns = cp_rssi->patterns;
> + expected_len = sizeof(*cp_rssi) +
> + pattern_count * sizeof(*patterns);
> + } else {
> + cp = data;
> + pattern_count = cp->pattern_count;
> + patterns = cp->patterns;
> + expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
> + }
> +
> + if (len != expected_len || pattern_count == 0) {
> + err = mgmt_cmd_status(sk, hdev->id, op,
> MGMT_STATUS_INVALID_PARAMS);
> goto failed;
> }
> @@ -4254,21 +4273,40 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> INIT_LIST_HEAD(&m->patterns);
> m->active = false;
>
> - for (i = 0; i < cp->pattern_count; i++) {
> + if (rssi) {
> + m->rssi.low_threshold = rssi->low_threshold;
> + m->rssi.low_threshold_timeout =
> + __le16_to_cpu(rssi->low_threshold_timeout);
> + m->rssi.high_threshold = rssi->high_threshold;
> + m->rssi.high_threshold_timeout =
> + __le16_to_cpu(rssi->high_threshold_timeout);
> + m->rssi.sampling_period = rssi->sampling_period;
> + } else {
> + /* Default values. These numbers are the least constricting
> + * parameters for MSFT API to work, so it behaves as if there
> + * are no rssi parameter to consider. May need to be changed
> + * if other API are to be supported.
> + */
> + m->rssi.low_threshold = -127;
> + m->rssi.low_threshold_timeout = 60;
> + m->rssi.high_threshold = -127;
> + m->rssi.high_threshold_timeout = 0;
> + m->rssi.sampling_period = 0;
> + }
> +
> + for (i = 0; i < pattern_count; i++) {
> if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
> - err = mgmt_cmd_status(sk, hdev->id,
> - MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + err = mgmt_cmd_status(sk, hdev->id, op,
> MGMT_STATUS_INVALID_PARAMS);
> goto failed;
> }
>
> - cp_ofst = cp->patterns[i].offset;
> - cp_len = cp->patterns[i].length;
> + cp_ofst = patterns[i].offset;
> + cp_len = patterns[i].length;
> if (cp_ofst >= HCI_MAX_AD_LENGTH ||
> cp_len > HCI_MAX_AD_LENGTH ||
> (cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
> - err = mgmt_cmd_status(sk, hdev->id,
> - MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + err = mgmt_cmd_status(sk, hdev->id, op,
> MGMT_STATUS_INVALID_PARAMS);
> goto failed;
> }
> @@ -4279,18 +4317,17 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> goto failed;
> }
>
> - p->ad_type = cp->patterns[i].ad_type;
> - p->offset = cp->patterns[i].offset;
> - p->length = cp->patterns[i].length;
> - memcpy(p->value, cp->patterns[i].value, p->length);
> + p->ad_type = patterns[i].ad_type;
> + p->offset = patterns[i].offset;
> + p->length = patterns[i].length;
> + memcpy(p->value, patterns[i].value, p->length);
>
> INIT_LIST_HEAD(&p->list);
> list_add(&p->list, &m->patterns);
> }
>
> - if (mp_cnt != cp->pattern_count) {
> - err = mgmt_cmd_status(sk, hdev->id,
> - MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + if (mp_cnt != pattern_count) {
> + err = mgmt_cmd_status(sk, hdev->id, op,
> MGMT_STATUS_INVALID_PARAMS);
> goto failed;
> }
> @@ -4302,8 +4339,7 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> err = hci_add_adv_monitor(hdev, m);
> if (err) {
> if (err == -ENOSPC) {
> - mgmt_cmd_status(sk, hdev->id,
> - MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + mgmt_cmd_status(sk, hdev->id, op,
> MGMT_STATUS_NO_RESOURCES);
> }
> goto unlock;
> @@ -4316,7 +4352,7 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
>
> rp.monitor_handle = cpu_to_le16(m->handle);
>
> - return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_ADD_ADV_PATTERNS_MONITOR,
> + return mgmt_cmd_complete(sk, hdev->id, op,
> MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
>
> unlock:
> @@ -4327,6 +4363,20 @@ static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 len)
> +{
> + return __add_adv_patterns_monitor(sk, hdev, data, len,
> + MGMT_OP_ADD_ADV_PATTERNS_MONITOR);
> +}
> +
> +static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev,
> + void *data, u16 len)
> +{
> + return __add_adv_patterns_monitor(sk, hdev, data, len,
> + MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI);
> +}
> +
Is this really the best way to handle this. I would put the parameter parsing into these functions and then call the common function.
> static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev,
> void *data, u16 len)
> {
> @@ -8234,6 +8284,9 @@ static const struct hci_mgmt_handler mgmt_handlers[] = {
> HCI_MGMT_VAR_LEN },
> { add_ext_adv_data, MGMT_ADD_EXT_ADV_DATA_SIZE,
> HCI_MGMT_VAR_LEN },
> + { add_adv_patterns_monitor_rssi,
> + MGMT_ADD_ADV_PATTERNS_MONITOR_RSSI_SIZE,
> + HCI_MGMT_VAR_LEN },
> };
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor
2020-12-16 4:33 [PATCH v3 0/5] MSFT offloading support for advertisement monitor Archie Pusaka
2020-12-16 4:33 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Archie Pusaka
@ 2020-12-16 4:33 ` Archie Pusaka
2020-12-21 9:09 ` Marcel Holtmann
2020-12-16 4:33 ` [PATCH v3 3/5] Bluetooth: advmon offload MSFT remove monitor Archie Pusaka
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Archie Pusaka @ 2020-12-16 4:33 UTC (permalink / raw)
To: linux-bluetooth, Marcel Holtmann
Cc: CrosBT Upstreaming, Archie Pusaka, Manish Mandlik,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, linux-kernel, netdev
From: Archie Pusaka <apusaka@chromium.org>
Enables advertising monitor offloading to the controller, if MSFT
extension is supported. The kernel won't adjust the monitor parameters
to match what the controller supports - that is the user space's
responsibility.
This patch only manages the addition of monitors. Monitor removal is
going to be handled by another patch.
Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---
(no changes since v2)
Changes in v2:
* Also implement the new MGMT opcode and merge the functionality with
the old one.
include/net/bluetooth/hci_core.h | 17 ++-
net/bluetooth/hci_core.c | 54 +++++++--
net/bluetooth/mgmt.c | 144 ++++++++++++++--------
net/bluetooth/msft.c | 201 ++++++++++++++++++++++++++++++-
net/bluetooth/msft.h | 12 ++
5 files changed, 367 insertions(+), 61 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8b7cf3620938..879d1e38ce96 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
struct adv_monitor {
struct list_head patterns;
struct adv_rssi_thresholds rssi;
- bool active;
__u16 handle;
+
+ enum {
+ ADV_MONITOR_STATE_NOT_REGISTERED,
+ ADV_MONITOR_STATE_REGISTERED,
+ ADV_MONITOR_STATE_OFFLOADED
+ } state;
};
#define HCI_MIN_ADV_MONITOR_HANDLE 1
-#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
+#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
#define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
+#define HCI_ADV_MONITOR_EXT_NONE 1
+#define HCI_ADV_MONITOR_EXT_MSFT 2
#define HCI_MAX_SHORT_NAME_LENGTH 10
@@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
void hci_free_adv_monitor(struct adv_monitor *monitor);
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err);
int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
@@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct hci_dev *hdev,
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d2c9a1c552f..fa13e35f775d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
kfree(monitor);
}
-/* This function requires the caller holds hdev->lock */
-int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
+int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+/* Assigns handle to a monitor, and if offloading is supported and power is on,
+ * also attempts to forward the request to the controller.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ int *err)
{
int min, max, handle;
- if (!monitor)
- return -EINVAL;
+ *err = 0;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
min = HCI_MIN_ADV_MONITOR_HANDLE;
max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
handle = idr_alloc(&hdev->adv_monitors_idr, monitor, min, max,
GFP_KERNEL);
- if (handle < 0)
- return handle;
+ if (handle < 0) {
+ *err = handle;
+ return false;
+ }
- hdev->adv_monitors_cnt++;
monitor->handle = handle;
- hci_update_background_scan(hdev);
+ if (!hdev_is_powered(hdev))
+ return false;
- return 0;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE:
+ hci_update_background_scan(hdev);
+ BT_DBG("%s add monitor status %d", hdev->name, *err);
+ /* Message was not forwarded to controller - not an error */
+ return false;
+ case HCI_ADV_MONITOR_EXT_MSFT:
+ *err = msft_add_monitor_pattern(hdev, monitor);
+ BT_DBG("%s add monitor msft status %d", hdev->name, *err);
+ break;
+ }
+
+ return (*err == 0);
}
static int free_adv_monitor(int id, void *ptr, void *data)
@@ -3134,6 +3162,14 @@ bool hci_is_adv_monitoring(struct hci_dev *hdev)
return !idr_is_empty(&hdev->adv_monitors_idr);
}
+int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev)
+{
+ if (msft_monitor_supported(hdev))
+ return HCI_ADV_MONITOR_EXT_MSFT;
+
+ return HCI_ADV_MONITOR_EXT_NONE;
+}
+
struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *bdaddr_list,
bdaddr_t *bdaddr, u8 type)
{
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index cd574054aa39..9bd9f6540664 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4185,6 +4185,7 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
int handle, err;
size_t rp_size = 0;
__u32 supported = 0;
+ __u32 enabled = 0;
__u16 num_handles = 0;
__u16 handles[HCI_MAX_ADV_MONITOR_NUM_HANDLES];
@@ -4192,12 +4193,11 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
hci_dev_lock(hdev);
- if (msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR)
+ if (msft_monitor_supported(hdev))
supported |= MGMT_ADV_MONITOR_FEATURE_MASK_OR_PATTERNS;
- idr_for_each_entry(&hdev->adv_monitors_idr, monitor, handle) {
+ idr_for_each_entry(&hdev->adv_monitors_idr, monitor, handle)
handles[num_handles++] = monitor->handle;
- }
hci_dev_unlock(hdev);
@@ -4206,11 +4206,11 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
if (!rp)
return -ENOMEM;
- /* Once controller-based monitoring is in place, the enabled_features
- * should reflect the use.
- */
+ /* All supported features are currently enabled */
+ enabled = supported;
+
rp->supported_features = cpu_to_le32(supported);
- rp->enabled_features = 0;
+ rp->enabled_features = cpu_to_le32(enabled);
rp->max_num_handles = cpu_to_le16(HCI_MAX_ADV_MONITOR_NUM_HANDLES);
rp->max_num_patterns = HCI_MAX_ADV_MONITOR_NUM_PATTERNS;
rp->num_handles = cpu_to_le16(num_handles);
@@ -4226,6 +4226,45 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
return err;
}
+int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ struct mgmt_rp_add_adv_patterns_monitor rp;
+ struct mgmt_pending_cmd *cmd;
+ struct adv_monitor *monitor;
+ int err = 0;
+
+ hci_dev_lock(hdev);
+
+ cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev);
+ if (!cmd) {
+ cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev);
+ if (!cmd)
+ goto done;
+ }
+
+ monitor = cmd->user_data;
+ rp.monitor_handle = cpu_to_le16(monitor->handle);
+
+ if (!status) {
+ mgmt_adv_monitor_added(cmd->sk, hdev, monitor->handle);
+ hdev->adv_monitors_cnt++;
+ if (monitor->state == ADV_MONITOR_STATE_NOT_REGISTERED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+ hci_update_background_scan(hdev);
+ }
+
+ err = mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
+ mgmt_status(status), &rp, sizeof(rp));
+ mgmt_pending_remove(cmd);
+
+done:
+ hci_dev_unlock(hdev);
+ bt_dev_dbg(hdev, "add monitor %d complete, status %d",
+ rp.monitor_handle, status);
+
+ return err;
+}
+
static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len, u16 op)
{
@@ -4236,14 +4275,25 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
struct mgmt_adv_pattern *patterns = NULL;
struct adv_monitor *m = NULL;
struct adv_pattern *p = NULL;
- unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
+ struct mgmt_pending_cmd *cmd;
__u8 cp_ofst = 0, cp_len = 0;
- int err, i;
+ int err, status, i;
+ bool pending;
u8 pattern_count;
u16 expected_len;
BT_DBG("request for %s", hdev->name);
+ hci_dev_lock(hdev);
+
+ if (pending_find(MGMT_OP_SET_LE, hdev) ||
+ pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) ||
+ pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev) ||
+ pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) {
+ status = MGMT_STATUS_BUSY;
+ goto unlock;
+ }
+
if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
cp_rssi = data;
pattern_count = cp_rssi->pattern_count;
@@ -4258,20 +4308,20 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
}
- if (len != expected_len || pattern_count == 0) {
- err = mgmt_cmd_status(sk, hdev->id, op,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
+ if (len != expected_len || pattern_count == 0 ||
+ pattern_count > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
+ status = MGMT_STATUS_INVALID_PARAMS;
+ goto unlock;
}
m = kmalloc(sizeof(*m), GFP_KERNEL);
if (!m) {
- err = -ENOMEM;
- goto failed;
+ status = MGMT_STATUS_NO_RESOURCES;
+ goto unlock;
}
INIT_LIST_HEAD(&m->patterns);
- m->active = false;
+ m->state = ADV_MONITOR_STATE_NOT_REGISTERED;
if (rssi) {
m->rssi.low_threshold = rssi->low_threshold;
@@ -4295,26 +4345,19 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
}
for (i = 0; i < pattern_count; i++) {
- if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
- err = mgmt_cmd_status(sk, hdev->id, op,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
- }
-
cp_ofst = patterns[i].offset;
cp_len = patterns[i].length;
if (cp_ofst >= HCI_MAX_AD_LENGTH ||
cp_len > HCI_MAX_AD_LENGTH ||
(cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
- err = mgmt_cmd_status(sk, hdev->id, op,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
+ status = MGMT_STATUS_INVALID_PARAMS;
+ goto unlock;
}
p = kmalloc(sizeof(*p), GFP_KERNEL);
if (!p) {
- err = -ENOMEM;
- goto failed;
+ status = MGMT_STATUS_NO_RESOURCES;
+ goto unlock;
}
p->ad_type = patterns[i].ad_type;
@@ -4326,41 +4369,46 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
list_add(&p->list, &m->patterns);
}
- if (mp_cnt != pattern_count) {
- err = mgmt_cmd_status(sk, hdev->id, op,
- MGMT_STATUS_INVALID_PARAMS);
- goto failed;
+ cmd = mgmt_pending_add(sk, op, hdev, data, len);
+ if (!cmd) {
+ status = MGMT_STATUS_NO_RESOURCES;
+ goto unlock;
}
- hci_dev_lock(hdev);
-
- prev_adv_monitors_cnt = hdev->adv_monitors_cnt;
-
- err = hci_add_adv_monitor(hdev, m);
+ pending = hci_add_adv_monitor(hdev, m, &err);
if (err) {
- if (err == -ENOSPC) {
- mgmt_cmd_status(sk, hdev->id, op,
- MGMT_STATUS_NO_RESOURCES);
- }
+ if (err == -ENOSPC || err == -ENOMEM)
+ status = MGMT_STATUS_NO_RESOURCES;
+ else if (err == -EINVAL)
+ status = MGMT_STATUS_INVALID_PARAMS;
+ else
+ status = MGMT_STATUS_FAILED;
+
+ mgmt_pending_remove(cmd);
goto unlock;
}
- if (hdev->adv_monitors_cnt > prev_adv_monitors_cnt)
+ if (!pending) {
+ mgmt_pending_remove(cmd);
+ rp.monitor_handle = cpu_to_le16(m->handle);
mgmt_adv_monitor_added(sk, hdev, m->handle);
+ m->state = ADV_MONITOR_STATE_REGISTERED;
+ hdev->adv_monitors_cnt++;
- hci_dev_unlock(hdev);
+ hci_dev_unlock(hdev);
+ return mgmt_cmd_complete(sk, hdev->id, op, MGMT_STATUS_SUCCESS,
+ &rp, sizeof(rp));
+ }
- rp.monitor_handle = cpu_to_le16(m->handle);
+ hci_dev_unlock(hdev);
- return mgmt_cmd_complete(sk, hdev->id, op,
- MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+ cmd->user_data = m;
+ return 0;
unlock:
hci_dev_unlock(hdev);
-
-failed:
hci_free_adv_monitor(m);
- return err;
+ return mgmt_cmd_status(sk, hdev->id, op, status);
}
static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 4b39534a14a1..e4b8fe71b9c3 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -5,9 +5,16 @@
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
+#include <net/bluetooth/mgmt.h>
+#include "hci_request.h"
+#include "mgmt_util.h"
#include "msft.h"
+#define MSFT_RSSI_THRESHOLD_VALUE_MIN -127
+#define MSFT_RSSI_THRESHOLD_VALUE_MAX 20
+#define MSFT_RSSI_LOW_TIMEOUT_MAX 0x3C
+
#define MSFT_OP_READ_SUPPORTED_FEATURES 0x00
struct msft_cp_read_supported_features {
__u8 sub_opcode;
@@ -21,12 +28,55 @@ struct msft_rp_read_supported_features {
__u8 evt_prefix[];
} __packed;
+#define MSFT_OP_LE_MONITOR_ADVERTISEMENT 0x03
+#define MSFT_MONITOR_ADVERTISEMENT_TYPE_PATTERN 0x01
+struct msft_le_monitor_advertisement_pattern {
+ __u8 length;
+ __u8 data_type;
+ __u8 start_byte;
+ __u8 pattern[0];
+};
+
+struct msft_le_monitor_advertisement_pattern_data {
+ __u8 count;
+ __u8 data[0];
+};
+
+struct msft_cp_le_monitor_advertisement {
+ __u8 sub_opcode;
+ __s8 rssi_high;
+ __s8 rssi_low;
+ __u8 rssi_low_interval;
+ __u8 rssi_sampling_period;
+ __u8 cond_type;
+ __u8 data[0];
+} __packed;
+
+struct msft_rp_le_monitor_advertisement {
+ __u8 status;
+ __u8 sub_opcode;
+ __u8 handle;
+} __packed;
+
+struct msft_monitor_advertisement_handle_data {
+ __u8 msft_handle;
+ __u16 mgmt_handle;
+ struct list_head list;
+};
+
struct msft_data {
__u64 features;
__u8 evt_prefix_len;
__u8 *evt_prefix;
+ struct list_head handle_map;
+ __u16 pending_add_handle;
};
+bool msft_monitor_supported(struct hci_dev *hdev)
+{
+ return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
+}
+
static bool read_supported_features(struct hci_dev *hdev,
struct msft_data *msft)
{
@@ -90,12 +140,14 @@ void msft_do_open(struct hci_dev *hdev)
return;
}
+ INIT_LIST_HEAD(&msft->handle_map);
hdev->msft_data = msft;
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
+ struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
if (!msft)
return;
@@ -104,6 +156,11 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
+ list_for_each_entry_safe(handle_data, tmp, &msft->handle_map, list) {
+ list_del(&handle_data->list);
+ kfree(handle_data);
+ }
+
kfree(msft->evt_prefix);
kfree(msft);
}
@@ -145,5 +202,147 @@ __u64 msft_get_features(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
- return msft ? msft->features : 0;
+ return msft ? msft->features : 0;
+}
+
+static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_rp_le_monitor_advertisement *rp;
+ struct adv_monitor *monitor;
+ struct msft_monitor_advertisement_handle_data *handle_data;
+ struct msft_data *msft = hdev->msft_data;
+
+ hci_dev_lock(hdev);
+
+ monitor = idr_find(&hdev->adv_monitors_idr, msft->pending_add_handle);
+ if (!monitor) {
+ bt_dev_err(hdev, "msft add advmon: monitor %d is not found!",
+ msft->pending_add_handle);
+ status = HCI_ERROR_UNSPECIFIED;
+ goto unlock;
+ }
+
+ if (status)
+ goto unlock;
+
+ rp = (struct msft_rp_le_monitor_advertisement *)skb->data;
+ if (skb->len < sizeof(*rp)) {
+ status = HCI_ERROR_UNSPECIFIED;
+ goto unlock;
+ }
+
+ handle_data = kmalloc(sizeof(*handle_data), GFP_KERNEL);
+ if (!handle_data) {
+ status = HCI_ERROR_UNSPECIFIED;
+ goto unlock;
+ }
+
+ handle_data->mgmt_handle = monitor->handle;
+ handle_data->msft_handle = rp->handle;
+ INIT_LIST_HEAD(&handle_data->list);
+ list_add(&handle_data->list, &msft->handle_map);
+
+ monitor->state = ADV_MONITOR_STATE_OFFLOADED;
+
+unlock:
+ if (status && monitor) {
+ idr_remove(&hdev->adv_monitors_idr, monitor->handle);
+ hci_free_adv_monitor(monitor);
+ }
+
+ hci_dev_unlock(hdev);
+
+ hci_add_adv_patterns_monitor_complete(hdev, status);
+}
+
+static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
+{
+ struct adv_rssi_thresholds *r = &monitor->rssi;
+
+ if (r->high_threshold < MSFT_RSSI_THRESHOLD_VALUE_MIN ||
+ r->high_threshold > MSFT_RSSI_THRESHOLD_VALUE_MAX ||
+ r->low_threshold < MSFT_RSSI_THRESHOLD_VALUE_MIN ||
+ r->low_threshold > MSFT_RSSI_THRESHOLD_VALUE_MAX)
+ return false;
+
+ /* High_threshold_timeout is not supported,
+ * once high_threshold is reached, events are immediately reported.
+ */
+ if (r->high_threshold_timeout != 0)
+ return false;
+
+ if (r->low_threshold_timeout > MSFT_RSSI_LOW_TIMEOUT_MAX)
+ return false;
+
+ /* Sampling period from 0x00 to 0xFF are all allowed */
+ return true;
+}
+
+static bool msft_monitor_pattern_valid(struct adv_monitor *monitor)
+{
+ return msft_monitor_rssi_valid(monitor);
+ /* No additional check needed for pattern-based monitor */
+}
+
+/* This function requires the caller holds hdev->lock */
+int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+{
+ struct msft_cp_le_monitor_advertisement *cp;
+ struct msft_le_monitor_advertisement_pattern_data *pattern_data;
+ struct msft_le_monitor_advertisement_pattern *pattern;
+ struct adv_pattern *entry;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ size_t total_size = sizeof(*cp) + sizeof(*pattern_data);
+ ptrdiff_t offset = 0;
+ u8 pattern_count = 0;
+ int err = 0;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ if (!msft_monitor_pattern_valid(monitor))
+ return -EINVAL;
+
+ list_for_each_entry(entry, &monitor->patterns, list) {
+ pattern_count++;
+ total_size += sizeof(*pattern) + entry->length;
+ }
+
+ cp = kmalloc(total_size, GFP_KERNEL);
+ if (!cp)
+ return -ENOMEM;
+
+ cp->sub_opcode = MSFT_OP_LE_MONITOR_ADVERTISEMENT;
+ cp->rssi_high = monitor->rssi.high_threshold;
+ cp->rssi_low = monitor->rssi.low_threshold;
+ cp->rssi_low_interval = (u8)monitor->rssi.low_threshold_timeout;
+ cp->rssi_sampling_period = monitor->rssi.sampling_period;
+
+ cp->cond_type = MSFT_MONITOR_ADVERTISEMENT_TYPE_PATTERN;
+
+ pattern_data = (void *)cp->data;
+ pattern_data->count = pattern_count;
+
+ list_for_each_entry(entry, &monitor->patterns, list) {
+ pattern = (void *)(pattern_data->data + offset);
+ /* the length also includes data_type and offset */
+ pattern->length = entry->length + 2;
+ pattern->data_type = entry->ad_type;
+ pattern->start_byte = entry->offset;
+ memcpy(pattern->pattern, entry->value, entry->length);
+ offset += sizeof(*pattern) + entry->length;
+ }
+
+ hci_req_init(&req, hdev);
+ hci_req_add(&req, hdev->msft_opcode, total_size, cp);
+ err = hci_req_run_skb(&req, msft_le_monitor_advertisement_cb);
+ kfree(cp);
+
+ if (!err)
+ msft->pending_add_handle = monitor->handle;
+
+ return err;
}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index e9c478e890b8..0ac9b15322b1 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -12,16 +12,28 @@
#if IS_ENABLED(CONFIG_BT_MSFTEXT)
+bool msft_monitor_supported(struct hci_dev *hdev);
void msft_do_open(struct hci_dev *hdev);
void msft_do_close(struct hci_dev *hdev);
void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb);
__u64 msft_get_features(struct hci_dev *hdev);
+int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor);
#else
+static inline bool msft_monitor_supported(struct hci_dev *hdev)
+{
+ return false;
+}
+
static inline void msft_do_open(struct hci_dev *hdev) {}
static inline void msft_do_close(struct hci_dev *hdev) {}
static inline void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb) {}
static inline __u64 msft_get_features(struct hci_dev *hdev) { return 0; }
+static inline int msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
+{
+ return -EOPNOTSUPP;
+}
#endif
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor
2020-12-16 4:33 ` [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor Archie Pusaka
@ 2020-12-21 9:09 ` Marcel Holtmann
0 siblings, 0 replies; 13+ messages in thread
From: Marcel Holtmann @ 2020-12-21 9:09 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Manish Mandlik, Miao-chen Chou, Yun-Hao Chung, David S. Miller,
Jakub Kicinski, Johan Hedberg, Luiz Augusto von Dentz,
linux-kernel, netdev
Hi Archie,
> Enables advertising monitor offloading to the controller, if MSFT
> extension is supported. The kernel won't adjust the monitor parameters
> to match what the controller supports - that is the user space's
> responsibility.
>
> This patch only manages the addition of monitors. Monitor removal is
> going to be handled by another patch.
>
> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> Reviewed-by: Manish Mandlik <mmandlik@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> * Also implement the new MGMT opcode and merge the functionality with
> the old one.
>
> include/net/bluetooth/hci_core.h | 17 ++-
> net/bluetooth/hci_core.c | 54 +++++++--
> net/bluetooth/mgmt.c | 144 ++++++++++++++--------
> net/bluetooth/msft.c | 201 ++++++++++++++++++++++++++++++-
> net/bluetooth/msft.h | 12 ++
> 5 files changed, 367 insertions(+), 61 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 8b7cf3620938..879d1e38ce96 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -261,13 +261,20 @@ struct adv_rssi_thresholds {
> struct adv_monitor {
> struct list_head patterns;
> struct adv_rssi_thresholds rssi;
> - bool active;
> __u16 handle;
> +
> + enum {
> + ADV_MONITOR_STATE_NOT_REGISTERED,
> + ADV_MONITOR_STATE_REGISTERED,
> + ADV_MONITOR_STATE_OFFLOADED
> + } state;
> };
>
> #define HCI_MIN_ADV_MONITOR_HANDLE 1
> -#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
> +#define HCI_MAX_ADV_MONITOR_NUM_HANDLES 32
> #define HCI_MAX_ADV_MONITOR_NUM_PATTERNS 16
> +#define HCI_ADV_MONITOR_EXT_NONE 1
> +#define HCI_ADV_MONITOR_EXT_MSFT 2
>
> #define HCI_MAX_SHORT_NAME_LENGTH 10
>
> @@ -1326,9 +1333,12 @@ void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
>
> void hci_adv_monitors_clear(struct hci_dev *hdev);
> void hci_free_adv_monitor(struct adv_monitor *monitor);
> -int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
> +int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
> +bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
> + int *err);
> int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
> bool hci_is_adv_monitoring(struct hci_dev *hdev);
> +int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
>
> void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
>
> @@ -1804,6 +1814,7 @@ void mgmt_advertising_added(struct sock *sk, struct hci_dev *hdev,
> void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
> u8 instance);
> int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
> +int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
>
> u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
> u16 to_multiplier);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9d2c9a1c552f..fa13e35f775d 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -3070,27 +3070,55 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
> kfree(monitor);
> }
>
> -/* This function requires the caller holds hdev->lock */
> -int hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
> +int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
> +{
> + return mgmt_add_adv_patterns_monitor_complete(hdev, status);
> +}
> +
> +/* Assigns handle to a monitor, and if offloading is supported and power is on,
> + * also attempts to forward the request to the controller.
> + * Returns true if request is forwarded (result is pending), false otherwise.
> + * This function requires the caller holds hdev->lock.
> + */
> +bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
> + int *err)
> {
> int min, max, handle;
>
> - if (!monitor)
> - return -EINVAL;
> + *err = 0;
> +
> + if (!monitor) {
> + *err = -EINVAL;
> + return false;
> + }
>
> min = HCI_MIN_ADV_MONITOR_HANDLE;
> max = HCI_MIN_ADV_MONITOR_HANDLE + HCI_MAX_ADV_MONITOR_NUM_HANDLES;
> handle = idr_alloc(&hdev->adv_monitors_idr, monitor, min, max,
> GFP_KERNEL);
> - if (handle < 0)
> - return handle;
> + if (handle < 0) {
> + *err = handle;
> + return false;
> + }
>
> - hdev->adv_monitors_cnt++;
> monitor->handle = handle;
>
> - hci_update_background_scan(hdev);
> + if (!hdev_is_powered(hdev))
> + return false;
>
> - return 0;
> + switch (hci_get_adv_monitor_offload_ext(hdev)) {
> + case HCI_ADV_MONITOR_EXT_NONE:
> + hci_update_background_scan(hdev);
> + BT_DBG("%s add monitor status %d", hdev->name, *err);
lets use bt_dev_dbg.
> + /* Message was not forwarded to controller - not an error */
> + return false;
> + case HCI_ADV_MONITOR_EXT_MSFT:
> + *err = msft_add_monitor_pattern(hdev, monitor);
> + BT_DBG("%s add monitor msft status %d", hdev->name, *err);
> + break;
> + }
> +
> + return (*err == 0);
> }
>
> static int free_adv_monitor(int id, void *ptr, void *data)
> @@ -3134,6 +3162,14 @@ bool hci_is_adv_monitoring(struct hci_dev *hdev)
> return !idr_is_empty(&hdev->adv_monitors_idr);
> }
>
> +int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev)
> +{
> + if (msft_monitor_supported(hdev))
> + return HCI_ADV_MONITOR_EXT_MSFT;
> +
> + return HCI_ADV_MONITOR_EXT_NONE;
> +}
> +
> struct bdaddr_list *hci_bdaddr_list_lookup(struct list_head *bdaddr_list,
> bdaddr_t *bdaddr, u8 type)
> {
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index cd574054aa39..9bd9f6540664 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4185,6 +4185,7 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
> int handle, err;
> size_t rp_size = 0;
> __u32 supported = 0;
> + __u32 enabled = 0;
> __u16 num_handles = 0;
> __u16 handles[HCI_MAX_ADV_MONITOR_NUM_HANDLES];
>
> @@ -4192,12 +4193,11 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
>
> hci_dev_lock(hdev);
>
> - if (msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR)
> + if (msft_monitor_supported(hdev))
> supported |= MGMT_ADV_MONITOR_FEATURE_MASK_OR_PATTERNS;
>
> - idr_for_each_entry(&hdev->adv_monitors_idr, monitor, handle) {
> + idr_for_each_entry(&hdev->adv_monitors_idr, monitor, handle)
> handles[num_handles++] = monitor->handle;
> - }
>
> hci_dev_unlock(hdev);
>
> @@ -4206,11 +4206,11 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
> if (!rp)
> return -ENOMEM;
>
> - /* Once controller-based monitoring is in place, the enabled_features
> - * should reflect the use.
> - */
> + /* All supported features are currently enabled */
> + enabled = supported;
> +
> rp->supported_features = cpu_to_le32(supported);
> - rp->enabled_features = 0;
> + rp->enabled_features = cpu_to_le32(enabled);
> rp->max_num_handles = cpu_to_le16(HCI_MAX_ADV_MONITOR_NUM_HANDLES);
> rp->max_num_patterns = HCI_MAX_ADV_MONITOR_NUM_PATTERNS;
> rp->num_handles = cpu_to_le16(num_handles);
> @@ -4226,6 +4226,45 @@ static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
> return err;
> }
>
> +int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
> +{
> + struct mgmt_rp_add_adv_patterns_monitor rp;
> + struct mgmt_pending_cmd *cmd;
> + struct adv_monitor *monitor;
> + int err = 0;
> +
> + hci_dev_lock(hdev);
> +
> + cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev);
> + if (!cmd) {
> + cmd = pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev);
> + if (!cmd)
> + goto done;
> + }
> +
> + monitor = cmd->user_data;
> + rp.monitor_handle = cpu_to_le16(monitor->handle);
> +
> + if (!status) {
> + mgmt_adv_monitor_added(cmd->sk, hdev, monitor->handle);
> + hdev->adv_monitors_cnt++;
> + if (monitor->state == ADV_MONITOR_STATE_NOT_REGISTERED)
> + monitor->state = ADV_MONITOR_STATE_REGISTERED;
> + hci_update_background_scan(hdev);
> + }
> +
> + err = mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
> + mgmt_status(status), &rp, sizeof(rp));
> + mgmt_pending_remove(cmd);
> +
> +done:
> + hci_dev_unlock(hdev);
> + bt_dev_dbg(hdev, "add monitor %d complete, status %d",
> + rp.monitor_handle, status);
> +
> + return err;
> +}
> +
> static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> void *data, u16 len, u16 op)
> {
> @@ -4236,14 +4275,25 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> struct mgmt_adv_pattern *patterns = NULL;
> struct adv_monitor *m = NULL;
> struct adv_pattern *p = NULL;
> - unsigned int mp_cnt = 0, prev_adv_monitors_cnt;
> + struct mgmt_pending_cmd *cmd;
> __u8 cp_ofst = 0, cp_len = 0;
> - int err, i;
> + int err, status, i;
> + bool pending;
> u8 pattern_count;
> u16 expected_len;
>
> BT_DBG("request for %s", hdev->name);
>
> + hci_dev_lock(hdev);
> +
> + if (pending_find(MGMT_OP_SET_LE, hdev) ||
> + pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) ||
> + pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev) ||
> + pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) {
> + status = MGMT_STATUS_BUSY;
> + goto unlock;
> + }
> +
> if (op == MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI) {
> cp_rssi = data;
> pattern_count = cp_rssi->pattern_count;
> @@ -4258,20 +4308,20 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> expected_len = sizeof(*cp) + pattern_count * sizeof(*patterns);
> }
>
> - if (len != expected_len || pattern_count == 0) {
> - err = mgmt_cmd_status(sk, hdev->id, op,
> - MGMT_STATUS_INVALID_PARAMS);
> - goto failed;
> + if (len != expected_len || pattern_count == 0 ||
> + pattern_count > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
> + status = MGMT_STATUS_INVALID_PARAMS;
> + goto unlock;
> }
>
> m = kmalloc(sizeof(*m), GFP_KERNEL);
> if (!m) {
> - err = -ENOMEM;
> - goto failed;
> + status = MGMT_STATUS_NO_RESOURCES;
> + goto unlock;
> }
>
> INIT_LIST_HEAD(&m->patterns);
> - m->active = false;
> + m->state = ADV_MONITOR_STATE_NOT_REGISTERED;
>
> if (rssi) {
> m->rssi.low_threshold = rssi->low_threshold;
> @@ -4295,26 +4345,19 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> }
>
> for (i = 0; i < pattern_count; i++) {
> - if (++mp_cnt > HCI_MAX_ADV_MONITOR_NUM_PATTERNS) {
> - err = mgmt_cmd_status(sk, hdev->id, op,
> - MGMT_STATUS_INVALID_PARAMS);
> - goto failed;
> - }
> -
> cp_ofst = patterns[i].offset;
> cp_len = patterns[i].length;
> if (cp_ofst >= HCI_MAX_AD_LENGTH ||
> cp_len > HCI_MAX_AD_LENGTH ||
> (cp_ofst + cp_len) > HCI_MAX_AD_LENGTH) {
> - err = mgmt_cmd_status(sk, hdev->id, op,
> - MGMT_STATUS_INVALID_PARAMS);
> - goto failed;
> + status = MGMT_STATUS_INVALID_PARAMS;
> + goto unlock;
> }
>
> p = kmalloc(sizeof(*p), GFP_KERNEL);
> if (!p) {
> - err = -ENOMEM;
> - goto failed;
> + status = MGMT_STATUS_NO_RESOURCES;
> + goto unlock;
> }
>
> p->ad_type = patterns[i].ad_type;
> @@ -4326,41 +4369,46 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> list_add(&p->list, &m->patterns);
> }
>
> - if (mp_cnt != pattern_count) {
> - err = mgmt_cmd_status(sk, hdev->id, op,
> - MGMT_STATUS_INVALID_PARAMS);
> - goto failed;
> + cmd = mgmt_pending_add(sk, op, hdev, data, len);
> + if (!cmd) {
> + status = MGMT_STATUS_NO_RESOURCES;
> + goto unlock;
> }
>
> - hci_dev_lock(hdev);
> -
> - prev_adv_monitors_cnt = hdev->adv_monitors_cnt;
> -
> - err = hci_add_adv_monitor(hdev, m);
> + pending = hci_add_adv_monitor(hdev, m, &err);
> if (err) {
> - if (err == -ENOSPC) {
> - mgmt_cmd_status(sk, hdev->id, op,
> - MGMT_STATUS_NO_RESOURCES);
> - }
> + if (err == -ENOSPC || err == -ENOMEM)
> + status = MGMT_STATUS_NO_RESOURCES;
> + else if (err == -EINVAL)
> + status = MGMT_STATUS_INVALID_PARAMS;
> + else
> + status = MGMT_STATUS_FAILED;
> +
> + mgmt_pending_remove(cmd);
> goto unlock;
> }
>
> - if (hdev->adv_monitors_cnt > prev_adv_monitors_cnt)
> + if (!pending) {
> + mgmt_pending_remove(cmd);
> + rp.monitor_handle = cpu_to_le16(m->handle);
> mgmt_adv_monitor_added(sk, hdev, m->handle);
> + m->state = ADV_MONITOR_STATE_REGISTERED;
> + hdev->adv_monitors_cnt++;
>
> - hci_dev_unlock(hdev);
> + hci_dev_unlock(hdev);
> + return mgmt_cmd_complete(sk, hdev->id, op, MGMT_STATUS_SUCCESS,
> + &rp, sizeof(rp));
> + }
>
> - rp.monitor_handle = cpu_to_le16(m->handle);
> + hci_dev_unlock(hdev);
>
> - return mgmt_cmd_complete(sk, hdev->id, op,
> - MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
> + cmd->user_data = m;
> + return 0;
>
> unlock:
> hci_dev_unlock(hdev);
> -
> -failed:
> hci_free_adv_monitor(m);
> - return err;
> + return mgmt_cmd_status(sk, hdev->id, op, status);
> }
>
> static int add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index 4b39534a14a1..e4b8fe71b9c3 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -5,9 +5,16 @@
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> +#include <net/bluetooth/mgmt.h>
>
> +#include "hci_request.h"
> +#include "mgmt_util.h"
> #include "msft.h"
>
> +#define MSFT_RSSI_THRESHOLD_VALUE_MIN -127
> +#define MSFT_RSSI_THRESHOLD_VALUE_MAX 20
> +#define MSFT_RSSI_LOW_TIMEOUT_MAX 0x3C
> +
> #define MSFT_OP_READ_SUPPORTED_FEATURES 0x00
> struct msft_cp_read_supported_features {
> __u8 sub_opcode;
> @@ -21,12 +28,55 @@ struct msft_rp_read_supported_features {
> __u8 evt_prefix[];
> } __packed;
>
> +#define MSFT_OP_LE_MONITOR_ADVERTISEMENT 0x03
> +#define MSFT_MONITOR_ADVERTISEMENT_TYPE_PATTERN 0x01
> +struct msft_le_monitor_advertisement_pattern {
> + __u8 length;
> + __u8 data_type;
> + __u8 start_byte;
> + __u8 pattern[0];
> +};
> +
> +struct msft_le_monitor_advertisement_pattern_data {
> + __u8 count;
> + __u8 data[0];
> +};
> +
> +struct msft_cp_le_monitor_advertisement {
> + __u8 sub_opcode;
> + __s8 rssi_high;
> + __s8 rssi_low;
> + __u8 rssi_low_interval;
> + __u8 rssi_sampling_period;
> + __u8 cond_type;
> + __u8 data[0];
> +} __packed;
> +
> +struct msft_rp_le_monitor_advertisement {
> + __u8 status;
> + __u8 sub_opcode;
> + __u8 handle;
> +} __packed;
> +
> +struct msft_monitor_advertisement_handle_data {
> + __u8 msft_handle;
> + __u16 mgmt_handle;
> + struct list_head list;
> +};
> +
> struct msft_data {
> __u64 features;
> __u8 evt_prefix_len;
> __u8 *evt_prefix;
> + struct list_head handle_map;
> + __u16 pending_add_handle;
> };
>
> +bool msft_monitor_supported(struct hci_dev *hdev)
> +{
> + return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
> +}
> +
> static bool read_supported_features(struct hci_dev *hdev,
> struct msft_data *msft)
> {
> @@ -90,12 +140,14 @@ void msft_do_open(struct hci_dev *hdev)
> return;
> }
>
> + INIT_LIST_HEAD(&msft->handle_map);
> hdev->msft_data = msft;
> }
>
> void msft_do_close(struct hci_dev *hdev)
> {
> struct msft_data *msft = hdev->msft_data;
> + struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
>
> if (!msft)
> return;
> @@ -104,6 +156,11 @@ void msft_do_close(struct hci_dev *hdev)
>
> hdev->msft_data = NULL;
>
> + list_for_each_entry_safe(handle_data, tmp, &msft->handle_map, list) {
> + list_del(&handle_data->list);
> + kfree(handle_data);
> + }
> +
> kfree(msft->evt_prefix);
> kfree(msft);
> }
> @@ -145,5 +202,147 @@ __u64 msft_get_features(struct hci_dev *hdev)
> {
> struct msft_data *msft = hdev->msft_data;
>
> - return msft ? msft->features : 0;
> + return msft ? msft->features : 0;
> +}
> +
> +static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,
> + u8 status, u16 opcode,
> + struct sk_buff *skb)
> +{
> + struct msft_rp_le_monitor_advertisement *rp;
> + struct adv_monitor *monitor;
> + struct msft_monitor_advertisement_handle_data *handle_data;
> + struct msft_data *msft = hdev->msft_data;
> +
> + hci_dev_lock(hdev);
> +
> + monitor = idr_find(&hdev->adv_monitors_idr, msft->pending_add_handle);
> + if (!monitor) {
> + bt_dev_err(hdev, "msft add advmon: monitor %d is not found!",
> + msft->pending_add_handle);
> + status = HCI_ERROR_UNSPECIFIED;
> + goto unlock;
> + }
> +
> + if (status)
> + goto unlock;
> +
> + rp = (struct msft_rp_le_monitor_advertisement *)skb->data;
> + if (skb->len < sizeof(*rp)) {
> + status = HCI_ERROR_UNSPECIFIED;
> + goto unlock;
> + }
> +
> + handle_data = kmalloc(sizeof(*handle_data), GFP_KERNEL);
> + if (!handle_data) {
> + status = HCI_ERROR_UNSPECIFIED;
> + goto unlock;
> + }
> +
> + handle_data->mgmt_handle = monitor->handle;
> + handle_data->msft_handle = rp->handle;
> + INIT_LIST_HEAD(&handle_data->list);
> + list_add(&handle_data->list, &msft->handle_map);
> +
> + monitor->state = ADV_MONITOR_STATE_OFFLOADED;
> +
> +unlock:
> + if (status && monitor) {
> + idr_remove(&hdev->adv_monitors_idr, monitor->handle);
> + hci_free_adv_monitor(monitor);
> + }
> +
> + hci_dev_unlock(hdev);
> +
> + hci_add_adv_patterns_monitor_complete(hdev, status);
> +}
> +
> +static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
> +{
> + struct adv_rssi_thresholds *r = &monitor->rssi;
> +
> + if (r->high_threshold < MSFT_RSSI_THRESHOLD_VALUE_MIN ||
> + r->high_threshold > MSFT_RSSI_THRESHOLD_VALUE_MAX ||
> + r->low_threshold < MSFT_RSSI_THRESHOLD_VALUE_MIN ||
> + r->low_threshold > MSFT_RSSI_THRESHOLD_VALUE_MAX)
> + return false;
> +
> + /* High_threshold_timeout is not supported,
> + * once high_threshold is reached, events are immediately reported.
> + */
> + if (r->high_threshold_timeout != 0)
> + return false;
> +
> + if (r->low_threshold_timeout > MSFT_RSSI_LOW_TIMEOUT_MAX)
> + return false;
> +
> + /* Sampling period from 0x00 to 0xFF are all allowed */
> + return true;
> +}
> +
> +static bool msft_monitor_pattern_valid(struct adv_monitor *monitor)
> +{
> + return msft_monitor_rssi_valid(monitor);
> + /* No additional check needed for pattern-based monitor */
> +}
> +
> +/* This function requires the caller holds hdev->lock */
> +int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
> +{
> + struct msft_cp_le_monitor_advertisement *cp;
> + struct msft_le_monitor_advertisement_pattern_data *pattern_data;
> + struct msft_le_monitor_advertisement_pattern *pattern;
> + struct adv_pattern *entry;
> + struct hci_request req;
> + struct msft_data *msft = hdev->msft_data;
> + size_t total_size = sizeof(*cp) + sizeof(*pattern_data);
> + ptrdiff_t offset = 0;
> + u8 pattern_count = 0;
> + int err = 0;
> +
> + if (!msft)
> + return -EOPNOTSUPP;
> +
> + if (!msft_monitor_pattern_valid(monitor))
> + return -EINVAL;
> +
> + list_for_each_entry(entry, &monitor->patterns, list) {
> + pattern_count++;
> + total_size += sizeof(*pattern) + entry->length;
> + }
> +
> + cp = kmalloc(total_size, GFP_KERNEL);
> + if (!cp)
> + return -ENOMEM;
> +
> + cp->sub_opcode = MSFT_OP_LE_MONITOR_ADVERTISEMENT;
> + cp->rssi_high = monitor->rssi.high_threshold;
> + cp->rssi_low = monitor->rssi.low_threshold;
> + cp->rssi_low_interval = (u8)monitor->rssi.low_threshold_timeout;
> + cp->rssi_sampling_period = monitor->rssi.sampling_period;
> +
> + cp->cond_type = MSFT_MONITOR_ADVERTISEMENT_TYPE_PATTERN;
> +
> + pattern_data = (void *)cp->data;
> + pattern_data->count = pattern_count;
> +
> + list_for_each_entry(entry, &monitor->patterns, list) {
> + pattern = (void *)(pattern_data->data + offset);
> + /* the length also includes data_type and offset */
> + pattern->length = entry->length + 2;
> + pattern->data_type = entry->ad_type;
> + pattern->start_byte = entry->offset;
> + memcpy(pattern->pattern, entry->value, entry->length);
> + offset += sizeof(*pattern) + entry->length;
> + }
> +
> + hci_req_init(&req, hdev);
> + hci_req_add(&req, hdev->msft_opcode, total_size, cp);
> + err = hci_req_run_skb(&req, msft_le_monitor_advertisement_cb);
> + kfree(cp);
> +
> + if (!err)
> + msft->pending_add_handle = monitor->handle;
> +
> + return err;
> }
> diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
> index e9c478e890b8..0ac9b15322b1 100644
> --- a/net/bluetooth/msft.h
> +++ b/net/bluetooth/msft.h
> @@ -12,16 +12,28 @@
>
> #if IS_ENABLED(CONFIG_BT_MSFTEXT)
>
> +bool msft_monitor_supported(struct hci_dev *hdev);
> void msft_do_open(struct hci_dev *hdev);
> void msft_do_close(struct hci_dev *hdev);
> void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb);
> __u64 msft_get_features(struct hci_dev *hdev);
> +int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor);
>
> #else
>
> +static inline bool msft_monitor_supported(struct hci_dev *hdev)
> +{
> + return false;
> +}
> +
> static inline void msft_do_open(struct hci_dev *hdev) {}
> static inline void msft_do_close(struct hci_dev *hdev) {}
> static inline void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb) {}
> static inline __u64 msft_get_features(struct hci_dev *hdev) { return 0; }
> +static inline int msft_add_monitor_pattern(struct hci_dev *hdev,
> + struct adv_monitor *monitor)
> +{
> + return -EOPNOTSUPP;
> +}
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 3/5] Bluetooth: advmon offload MSFT remove monitor
2020-12-16 4:33 [PATCH v3 0/5] MSFT offloading support for advertisement monitor Archie Pusaka
2020-12-16 4:33 ` [PATCH v3 1/5] Bluetooth: advmon offload MSFT add rssi support Archie Pusaka
2020-12-16 4:33 ` [PATCH v3 2/5] Bluetooth: advmon offload MSFT add monitor Archie Pusaka
@ 2020-12-16 4:33 ` Archie Pusaka
2020-12-16 4:33 ` [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset Archie Pusaka
2020-12-16 4:33 ` [PATCH v3 5/5] Bluetooth: advmon offload MSFT handle filter enablement Archie Pusaka
4 siblings, 0 replies; 13+ messages in thread
From: Archie Pusaka @ 2020-12-16 4:33 UTC (permalink / raw)
To: linux-bluetooth, Marcel Holtmann
Cc: CrosBT Upstreaming, Archie Pusaka, Miao-chen Chou, Yun-Hao Chung,
kernel test robot, Dan Carpenter, David S. Miller,
Jakub Kicinski, Johan Hedberg, Luiz Augusto von Dentz,
linux-kernel, netdev
From: Archie Pusaka <apusaka@chromium.org>
Implements the monitor removal functionality for advertising monitor
offloading to MSFT controllers. Supply handle = 0 to remove all
monitors.
Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Changes in v3:
* Fix return type of msft_remove_monitor
include/net/bluetooth/hci_core.h | 8 +-
net/bluetooth/hci_core.c | 119 +++++++++++++++++++++++------
net/bluetooth/mgmt.c | 110 +++++++++++++++++++++-----
net/bluetooth/msft.c | 127 ++++++++++++++++++++++++++++++-
net/bluetooth/msft.h | 9 +++
5 files changed, 323 insertions(+), 50 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 879d1e38ce96..29cfc6a2d689 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1332,11 +1332,13 @@ int hci_remove_adv_instance(struct hci_dev *hdev, u8 instance);
void hci_adv_instances_set_rpa_expired(struct hci_dev *hdev, bool rpa_expired);
void hci_adv_monitors_clear(struct hci_dev *hdev);
-void hci_free_adv_monitor(struct adv_monitor *monitor);
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor);
int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
int *err);
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle);
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err);
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err);
bool hci_is_adv_monitoring(struct hci_dev *hdev);
int hci_get_adv_monitor_offload_ext(struct hci_dev *hdev);
@@ -1813,8 +1815,10 @@ void mgmt_advertising_added(struct sock *sk, struct hci_dev *hdev,
u8 instance);
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
u8 instance);
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
int mgmt_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status);
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status);
u8 hci_le_conn_update(struct hci_conn *conn, u16 min, u16 max, u16 latency,
u16 to_multiplier);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fa13e35f775d..782b05ca0d0c 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3051,12 +3051,15 @@ void hci_adv_monitors_clear(struct hci_dev *hdev)
int handle;
idr_for_each_entry(&hdev->adv_monitors_idr, monitor, handle)
- hci_free_adv_monitor(monitor);
+ hci_free_adv_monitor(hdev, monitor);
idr_destroy(&hdev->adv_monitors_idr);
}
-void hci_free_adv_monitor(struct adv_monitor *monitor)
+/* Frees the monitor structure and do some bookkeepings.
+ * This function requires the caller holds hdev->lock.
+ */
+void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
{
struct adv_pattern *pattern;
struct adv_pattern *tmp;
@@ -3064,8 +3067,18 @@ void hci_free_adv_monitor(struct adv_monitor *monitor)
if (!monitor)
return;
- list_for_each_entry_safe(pattern, tmp, &monitor->patterns, list)
+ list_for_each_entry_safe(pattern, tmp, &monitor->patterns, list) {
+ list_del(&pattern->list);
kfree(pattern);
+ }
+
+ if (monitor->handle)
+ idr_remove(&hdev->adv_monitors_idr, monitor->handle);
+
+ if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
+ hdev->adv_monitors_cnt--;
+ mgmt_adv_monitor_removed(hdev, monitor->handle);
+ }
kfree(monitor);
}
@@ -3075,6 +3088,11 @@ int hci_add_adv_patterns_monitor_complete(struct hci_dev *hdev, u8 status)
return mgmt_add_adv_patterns_monitor_complete(hdev, status);
}
+int hci_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ return mgmt_remove_adv_monitor_complete(hdev, status);
+}
+
/* Assigns handle to a monitor, and if offloading is supported and power is on,
* also attempts to forward the request to the controller.
* Returns true if request is forwarded (result is pending), false otherwise.
@@ -3121,39 +3139,94 @@ bool hci_add_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
return (*err == 0);
}
-static int free_adv_monitor(int id, void *ptr, void *data)
+/* Attempts to tell the controller and free the monitor. If somehow the
+ * controller doesn't have a corresponding handle, remove anyway.
+ * Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+static bool hci_remove_adv_monitor(struct hci_dev *hdev,
+ struct adv_monitor *monitor,
+ u16 handle, int *err)
{
- struct hci_dev *hdev = data;
- struct adv_monitor *monitor = ptr;
+ *err = 0;
- idr_remove(&hdev->adv_monitors_idr, monitor->handle);
- hci_free_adv_monitor(monitor);
- hdev->adv_monitors_cnt--;
+ switch (hci_get_adv_monitor_offload_ext(hdev)) {
+ case HCI_ADV_MONITOR_EXT_NONE: /* also goes here when powered off */
+ goto free_monitor;
+ case HCI_ADV_MONITOR_EXT_MSFT:
+ *err = msft_remove_monitor(hdev, monitor, handle);
+ break;
+ }
- return 0;
+ /* In case no matching handle registered, just free the monitor */
+ if (*err == -ENOENT)
+ goto free_monitor;
+
+ return (*err == 0);
+
+free_monitor:
+ if (*err == -ENOENT)
+ bt_dev_warn(hdev, "Removing monitor with no matching handle %d",
+ monitor->handle);
+ hci_free_adv_monitor(hdev, monitor);
+
+ *err = 0;
+ return false;
}
-/* This function requires the caller holds hdev->lock */
-int hci_remove_adv_monitor(struct hci_dev *hdev, u16 handle)
+/* Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_remove_single_adv_monitor(struct hci_dev *hdev, u16 handle, int *err)
+{
+ struct adv_monitor *monitor = idr_find(&hdev->adv_monitors_idr, handle);
+ bool pending;
+
+ if (!monitor) {
+ *err = -EINVAL;
+ return false;
+ }
+
+ pending = hci_remove_adv_monitor(hdev, monitor, handle, err);
+ if (!*err && !pending)
+ hci_update_background_scan(hdev);
+
+ BT_DBG("%s remove monitor handle %d, status %d, %spending",
+ hdev->name, handle, *err, pending ? "" : "not ");
+
+ return pending;
+}
+
+/* Returns true if request is forwarded (result is pending), false otherwise.
+ * This function requires the caller holds hdev->lock.
+ */
+bool hci_remove_all_adv_monitor(struct hci_dev *hdev, int *err)
{
struct adv_monitor *monitor;
+ int idr_next_id = 0;
+ bool pending = false;
+ bool update = false;
+
+ *err = 0;
- if (handle) {
- monitor = idr_find(&hdev->adv_monitors_idr, handle);
+ while (!*err && !pending) {
+ monitor = idr_get_next(&hdev->adv_monitors_idr, &idr_next_id);
if (!monitor)
- return -ENOENT;
+ break;
- idr_remove(&hdev->adv_monitors_idr, monitor->handle);
- hci_free_adv_monitor(monitor);
- hdev->adv_monitors_cnt--;
- } else {
- /* Remove all monitors if handle is 0. */
- idr_for_each(&hdev->adv_monitors_idr, &free_adv_monitor, hdev);
+ pending = hci_remove_adv_monitor(hdev, monitor, 0, err);
+
+ if (!*err && !pending)
+ update = true;
}
- hci_update_background_scan(hdev);
+ if (update)
+ hci_update_background_scan(hdev);
- return 0;
+ BT_DBG("%s remove all monitors status %d, %spending",
+ hdev->name, *err, pending ? "" : "not ");
+
+ return pending;
}
/* This function requires the caller holds hdev->lock */
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 9bd9f6540664..35834517d338 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -4167,14 +4167,24 @@ static void mgmt_adv_monitor_added(struct sock *sk, struct hci_dev *hdev,
mgmt_event(MGMT_EV_ADV_MONITOR_ADDED, hdev, &ev, sizeof(ev), sk);
}
-static void mgmt_adv_monitor_removed(struct sock *sk, struct hci_dev *hdev,
- u16 handle)
+void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle)
{
- struct mgmt_ev_adv_monitor_added ev;
+ struct mgmt_ev_adv_monitor_removed ev;
+ struct mgmt_pending_cmd *cmd;
+ struct sock *sk_skip = NULL;
+ struct mgmt_cp_remove_adv_monitor *cp;
+
+ cmd = pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev);
+ if (cmd) {
+ cp = cmd->param;
+
+ if (cp->monitor_handle)
+ sk_skip = cmd->sk;
+ }
ev.monitor_handle = cpu_to_le16(handle);
- mgmt_event(MGMT_EV_ADV_MONITOR_REMOVED, hdev, &ev, sizeof(ev), sk);
+ mgmt_event(MGMT_EV_ADV_MONITOR_REMOVED, hdev, &ev, sizeof(ev), sk_skip);
}
static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
@@ -4406,8 +4416,8 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
return 0;
unlock:
+ hci_free_adv_monitor(hdev, m);
hci_dev_unlock(hdev);
- hci_free_adv_monitor(m);
return mgmt_cmd_status(sk, hdev->id, op, status);
}
@@ -4425,42 +4435,100 @@ static int add_adv_patterns_monitor_rssi(struct sock *sk, struct hci_dev *hdev,
MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI);
}
+int mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, u8 status)
+{
+ struct mgmt_rp_remove_adv_monitor rp;
+ struct mgmt_cp_remove_adv_monitor *cp;
+ struct mgmt_pending_cmd *cmd;
+ int err = 0;
+
+ hci_dev_lock(hdev);
+
+ cmd = pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev);
+ if (!cmd)
+ goto done;
+
+ cp = cmd->param;
+ rp.monitor_handle = cp->monitor_handle;
+
+ if (!status)
+ hci_update_background_scan(hdev);
+
+ err = mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
+ mgmt_status(status), &rp, sizeof(rp));
+ mgmt_pending_remove(cmd);
+
+done:
+ hci_dev_unlock(hdev);
+ bt_dev_dbg(hdev, "remove monitor %d complete, status %d",
+ rp.monitor_handle, status);
+
+ return err;
+}
+
static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev,
void *data, u16 len)
{
struct mgmt_cp_remove_adv_monitor *cp = data;
struct mgmt_rp_remove_adv_monitor rp;
- unsigned int prev_adv_monitors_cnt;
- u16 handle;
- int err;
+ struct mgmt_pending_cmd *cmd;
+ u16 handle = __le16_to_cpu(cp->monitor_handle);
+ int err, status;
+ bool pending;
BT_DBG("request for %s", hdev->name);
+ rp.monitor_handle = cp->monitor_handle;
hci_dev_lock(hdev);
- handle = __le16_to_cpu(cp->monitor_handle);
- prev_adv_monitors_cnt = hdev->adv_monitors_cnt;
+ if (pending_find(MGMT_OP_SET_LE, hdev) ||
+ pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev) ||
+ pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) ||
+ pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev)) {
+ status = MGMT_STATUS_BUSY;
+ goto unlock;
+ }
- err = hci_remove_adv_monitor(hdev, handle);
- if (err == -ENOENT) {
- err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADV_MONITOR,
- MGMT_STATUS_INVALID_INDEX);
+ cmd = mgmt_pending_add(sk, MGMT_OP_REMOVE_ADV_MONITOR, hdev, data, len);
+ if (!cmd) {
+ status = MGMT_STATUS_NO_RESOURCES;
goto unlock;
}
- if (hdev->adv_monitors_cnt < prev_adv_monitors_cnt)
- mgmt_adv_monitor_removed(sk, hdev, handle);
+ if (handle)
+ pending = hci_remove_single_adv_monitor(hdev, handle, &err);
+ else
+ pending = hci_remove_all_adv_monitor(hdev, &err);
- hci_dev_unlock(hdev);
+ if (err) {
+ mgmt_pending_remove(cmd);
- rp.monitor_handle = cp->monitor_handle;
+ if (err == -ENOENT)
+ status = MGMT_STATUS_INVALID_INDEX;
+ else
+ status = MGMT_STATUS_FAILED;
+
+ goto unlock;
+ }
+
+ /* monitor can be removed without forwarding request to controller */
+ if (!pending) {
+ mgmt_pending_remove(cmd);
+ hci_dev_unlock(hdev);
+
+ return mgmt_cmd_complete(sk, hdev->id,
+ MGMT_OP_REMOVE_ADV_MONITOR,
+ MGMT_STATUS_SUCCESS,
+ &rp, sizeof(rp));
+ }
- return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_REMOVE_ADV_MONITOR,
- MGMT_STATUS_SUCCESS, &rp, sizeof(rp));
+ hci_dev_unlock(hdev);
+ return 0;
unlock:
hci_dev_unlock(hdev);
- return err;
+ return mgmt_cmd_status(sk, hdev->id, MGMT_OP_REMOVE_ADV_MONITOR,
+ status);
}
static void read_local_oob_data_complete(struct hci_dev *hdev, u8 status,
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index e4b8fe71b9c3..f5aa0e3b1b9b 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -58,6 +58,17 @@ struct msft_rp_le_monitor_advertisement {
__u8 handle;
} __packed;
+#define MSFT_OP_LE_CANCEL_MONITOR_ADVERTISEMENT 0x04
+struct msft_cp_le_cancel_monitor_advertisement {
+ __u8 sub_opcode;
+ __u8 handle;
+} __packed;
+
+struct msft_rp_le_cancel_monitor_advertisement {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -70,6 +81,7 @@ struct msft_data {
__u8 *evt_prefix;
struct list_head handle_map;
__u16 pending_add_handle;
+ __u16 pending_remove_handle;
};
bool msft_monitor_supported(struct hci_dev *hdev)
@@ -205,6 +217,26 @@ __u64 msft_get_features(struct hci_dev *hdev)
return msft ? msft->features : 0;
}
+/* is_mgmt = true matches the handle exposed to userspace via mgmt.
+ * is_mgmt = false matches the handle used by the msft controller.
+ * This function requires the caller holds hdev->lock
+ */
+static struct msft_monitor_advertisement_handle_data *msft_find_handle_data
+ (struct hci_dev *hdev, u16 handle, bool is_mgmt)
+{
+ struct msft_monitor_advertisement_handle_data *entry;
+ struct msft_data *msft = hdev->msft_data;
+
+ list_for_each_entry(entry, &msft->handle_map, list) {
+ if (is_mgmt && entry->mgmt_handle == handle)
+ return entry;
+ if (!is_mgmt && entry->msft_handle == handle)
+ return entry;
+ }
+
+ return NULL;
+}
+
static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,
u8 status, u16 opcode,
struct sk_buff *skb)
@@ -247,16 +279,71 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,
monitor->state = ADV_MONITOR_STATE_OFFLOADED;
unlock:
- if (status && monitor) {
- idr_remove(&hdev->adv_monitors_idr, monitor->handle);
- hci_free_adv_monitor(monitor);
- }
+ if (status && monitor)
+ hci_free_adv_monitor(hdev, monitor);
hci_dev_unlock(hdev);
hci_add_adv_patterns_monitor_complete(hdev, status);
}
+static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_cancel_monitor_advertisement *cp;
+ struct msft_rp_le_cancel_monitor_advertisement *rp;
+ struct adv_monitor *monitor;
+ struct msft_monitor_advertisement_handle_data *handle_data;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+ bool pending;
+
+ if (status)
+ goto done;
+
+ rp = (struct msft_rp_le_cancel_monitor_advertisement *)skb->data;
+ if (skb->len < sizeof(*rp)) {
+ status = HCI_ERROR_UNSPECIFIED;
+ goto done;
+ }
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ handle_data = msft_find_handle_data(hdev, cp->handle, false);
+
+ if (handle_data) {
+ monitor = idr_find(&hdev->adv_monitors_idr,
+ handle_data->mgmt_handle);
+ if (monitor)
+ hci_free_adv_monitor(hdev, monitor);
+
+ list_del(&handle_data->list);
+ kfree(handle_data);
+ }
+
+ /* If remove all monitors is required, we need to continue the process
+ * here because the earlier it was paused when waiting for the
+ * response from controller.
+ */
+ if (msft->pending_remove_handle == 0) {
+ pending = hci_remove_all_adv_monitor(hdev, &err);
+ if (pending) {
+ hci_dev_unlock(hdev);
+ return;
+ }
+
+ if (err)
+ status = HCI_ERROR_UNSPECIFIED;
+ }
+
+ hci_dev_unlock(hdev);
+
+done:
+ hci_remove_adv_monitor_complete(hdev, status);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = &monitor->rssi;
@@ -346,3 +433,35 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
return err;
}
+
+/* This function requires the caller holds hdev->lock */
+int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ u16 handle)
+{
+ struct msft_cp_le_cancel_monitor_advertisement cp;
+ struct msft_monitor_advertisement_handle_data *handle_data;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err = 0;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ handle_data = msft_find_handle_data(hdev, monitor->handle, true);
+
+ /* If no matched handle, just remove without telling controller */
+ if (!handle_data)
+ return -ENOENT;
+
+ cp.sub_opcode = MSFT_OP_LE_CANCEL_MONITOR_ADVERTISEMENT;
+ cp.handle = handle_data->msft_handle;
+
+ hci_req_init(&req, hdev);
+ hci_req_add(&req, hdev->msft_opcode, sizeof(cp), &cp);
+ err = hci_req_run_skb(&req, msft_le_cancel_monitor_advertisement_cb);
+
+ if (!err)
+ msft->pending_remove_handle = handle;
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 0ac9b15322b1..6f126a1f1688 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -18,6 +18,8 @@ void msft_do_close(struct hci_dev *hdev);
void msft_vendor_evt(struct hci_dev *hdev, struct sk_buff *skb);
__u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor);
+int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
+ u16 handle);
#else
@@ -36,4 +38,11 @@ static inline int msft_add_monitor_pattern(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_remove_monitor(struct hci_dev *hdev,
+ struct adv_monitor *monitor,
+ u16 handle)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
2020-12-16 4:33 [PATCH v3 0/5] MSFT offloading support for advertisement monitor Archie Pusaka
` (2 preceding siblings ...)
2020-12-16 4:33 ` [PATCH v3 3/5] Bluetooth: advmon offload MSFT remove monitor Archie Pusaka
@ 2020-12-16 4:33 ` Archie Pusaka
2020-12-21 9:12 ` Marcel Holtmann
2020-12-16 4:33 ` [PATCH v3 5/5] Bluetooth: advmon offload MSFT handle filter enablement Archie Pusaka
4 siblings, 1 reply; 13+ messages in thread
From: Archie Pusaka @ 2020-12-16 4:33 UTC (permalink / raw)
To: linux-bluetooth, Marcel Holtmann
Cc: CrosBT Upstreaming, Archie Pusaka, Miao-chen Chou, Yun-Hao Chung,
David S. Miller, Jakub Kicinski, Johan Hedberg,
Luiz Augusto von Dentz, linux-kernel, netdev
From: Archie Pusaka <apusaka@chromium.org>
When the controller is powered off, the registered advertising monitor
is removed from the controller. This patch handles the re-registration
of those monitors when the power is on.
Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---
(no changes since v1)
net/bluetooth/msft.c | 79 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 74 insertions(+), 5 deletions(-)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index f5aa0e3b1b9b..7e33a85c3f1c 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -82,8 +82,15 @@ struct msft_data {
struct list_head handle_map;
__u16 pending_add_handle;
__u16 pending_remove_handle;
+
+ struct {
+ u8 reregistering:1;
+ } flags;
};
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor);
+
bool msft_monitor_supported(struct hci_dev *hdev)
{
return !!(msft_get_features(hdev) & MSFT_FEATURE_MASK_LE_ADV_MONITOR);
@@ -134,6 +141,35 @@ static bool read_supported_features(struct hci_dev *hdev,
return false;
}
+/* This function requires the caller holds hdev->lock */
+static void reregister_monitor_on_restart(struct hci_dev *hdev, int handle)
+{
+ struct adv_monitor *monitor;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ while (1) {
+ monitor = idr_get_next(&hdev->adv_monitors_idr, &handle);
+ if (!monitor) {
+ /* All monitors have been reregistered */
+ msft->flags.reregistering = false;
+ hci_update_background_scan(hdev);
+ return;
+ }
+
+ msft->pending_add_handle = (u16)handle;
+ err = __msft_add_monitor_pattern(hdev, monitor);
+
+ /* If success, we return and wait for monitor added callback */
+ if (!err)
+ return;
+
+ /* Otherwise remove the monitor and keep registering */
+ hci_free_adv_monitor(hdev, monitor);
+ handle++;
+ }
+}
+
void msft_do_open(struct hci_dev *hdev)
{
struct msft_data *msft;
@@ -154,12 +190,18 @@ void msft_do_open(struct hci_dev *hdev)
INIT_LIST_HEAD(&msft->handle_map);
hdev->msft_data = msft;
+
+ if (msft_monitor_supported(hdev)) {
+ msft->flags.reregistering = true;
+ reregister_monitor_on_restart(hdev, 0);
+ }
}
void msft_do_close(struct hci_dev *hdev)
{
struct msft_data *msft = hdev->msft_data;
struct msft_monitor_advertisement_handle_data *handle_data, *tmp;
+ struct adv_monitor *monitor;
if (!msft)
return;
@@ -169,6 +211,12 @@ void msft_do_close(struct hci_dev *hdev)
hdev->msft_data = NULL;
list_for_each_entry_safe(handle_data, tmp, &msft->handle_map, list) {
+ monitor = idr_find(&hdev->adv_monitors_idr,
+ handle_data->mgmt_handle);
+
+ if (monitor && monitor->state == ADV_MONITOR_STATE_OFFLOADED)
+ monitor->state = ADV_MONITOR_STATE_REGISTERED;
+
list_del(&handle_data->list);
kfree(handle_data);
}
@@ -282,9 +330,15 @@ static void msft_le_monitor_advertisement_cb(struct hci_dev *hdev,
if (status && monitor)
hci_free_adv_monitor(hdev, monitor);
+ /* If in restart/reregister sequence, keep registering. */
+ if (msft->flags.reregistering)
+ reregister_monitor_on_restart(hdev,
+ msft->pending_add_handle + 1);
+
hci_dev_unlock(hdev);
- hci_add_adv_patterns_monitor_complete(hdev, status);
+ if (!msft->flags.reregistering)
+ hci_add_adv_patterns_monitor_complete(hdev, status);
}
static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
@@ -374,7 +428,8 @@ static bool msft_monitor_pattern_valid(struct adv_monitor *monitor)
}
/* This function requires the caller holds hdev->lock */
-int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+static int __msft_add_monitor_pattern(struct hci_dev *hdev,
+ struct adv_monitor *monitor)
{
struct msft_cp_le_monitor_advertisement *cp;
struct msft_le_monitor_advertisement_pattern_data *pattern_data;
@@ -387,9 +442,6 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
u8 pattern_count = 0;
int err = 0;
- if (!msft)
- return -EOPNOTSUPP;
-
if (!msft_monitor_pattern_valid(monitor))
return -EINVAL;
@@ -434,6 +486,20 @@ int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
return err;
}
+/* This function requires the caller holds hdev->lock */
+int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor)
+{
+ struct msft_data *msft = hdev->msft_data;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ if (msft->flags.reregistering)
+ return -EBUSY;
+
+ return __msft_add_monitor_pattern(hdev, monitor);
+}
+
/* This function requires the caller holds hdev->lock */
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle)
@@ -447,6 +513,9 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
if (!msft)
return -EOPNOTSUPP;
+ if (msft->flags.reregistering)
+ return -EBUSY;
+
handle_data = msft_find_handle_data(hdev, monitor->handle, true);
/* If no matched handle, just remove without telling controller */
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
2020-12-16 4:33 ` [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset Archie Pusaka
@ 2020-12-21 9:12 ` Marcel Holtmann
2020-12-22 3:26 ` Archie Pusaka
0 siblings, 1 reply; 13+ messages in thread
From: Marcel Holtmann @ 2020-12-21 9:12 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, LKML, netdev
Hi Archie,
> When the controller is powered off, the registered advertising monitor
> is removed from the controller. This patch handles the re-registration
> of those monitors when the power is on.
>
> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
>
> ---
>
> (no changes since v1)
>
> net/bluetooth/msft.c | 79 +++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 74 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index f5aa0e3b1b9b..7e33a85c3f1c 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -82,8 +82,15 @@ struct msft_data {
> struct list_head handle_map;
> __u16 pending_add_handle;
> __u16 pending_remove_handle;
> +
> + struct {
> + u8 reregistering:1;
> + } flags;
> };
hmmm. Do you have bigger plans with this struct? I would just skip it.
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
2020-12-21 9:12 ` Marcel Holtmann
@ 2020-12-22 3:26 ` Archie Pusaka
2020-12-22 10:03 ` Marcel Holtmann
0 siblings, 1 reply; 13+ messages in thread
From: Archie Pusaka @ 2020-12-22 3:26 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, LKML,
open list:NETWORKING [GENERAL]
Hi Marcel,
On Mon, 21 Dec 2020 at 17:12, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Archie,
>
> > When the controller is powered off, the registered advertising monitor
> > is removed from the controller. This patch handles the re-registration
> > of those monitors when the power is on.
> >
> > Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> > Reviewed-by: Yun-Hao Chung <howardchung@google.com>
> >
> > ---
> >
> > (no changes since v1)
> >
> > net/bluetooth/msft.c | 79 +++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 74 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> > index f5aa0e3b1b9b..7e33a85c3f1c 100644
> > --- a/net/bluetooth/msft.c
> > +++ b/net/bluetooth/msft.c
> > @@ -82,8 +82,15 @@ struct msft_data {
> > struct list_head handle_map;
> > __u16 pending_add_handle;
> > __u16 pending_remove_handle;
> > +
> > + struct {
> > + u8 reregistering:1;
> > + } flags;
> > };
>
> hmmm. Do you have bigger plans with this struct? I would just skip it.
>
This struct is also used in patch 5/5 to store the "enabled" status of
the filter.
Suspend/resume would need to enable/disable the filter, but it is not
yet implemented in this patch series.
Thanks,
Archie
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
2020-12-22 3:26 ` Archie Pusaka
@ 2020-12-22 10:03 ` Marcel Holtmann
2020-12-22 10:27 ` Archie Pusaka
0 siblings, 1 reply; 13+ messages in thread
From: Marcel Holtmann @ 2020-12-22 10:03 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, LKML,
open list:NETWORKING [GENERAL]
Hi Archie,
>>> When the controller is powered off, the registered advertising monitor
>>> is removed from the controller. This patch handles the re-registration
>>> of those monitors when the power is on.
>>>
>>> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
>>> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
>>> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
>>>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>> net/bluetooth/msft.c | 79 +++++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 74 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
>>> index f5aa0e3b1b9b..7e33a85c3f1c 100644
>>> --- a/net/bluetooth/msft.c
>>> +++ b/net/bluetooth/msft.c
>>> @@ -82,8 +82,15 @@ struct msft_data {
>>> struct list_head handle_map;
>>> __u16 pending_add_handle;
>>> __u16 pending_remove_handle;
>>> +
>>> + struct {
>>> + u8 reregistering:1;
>>> + } flags;
>>> };
>>
>> hmmm. Do you have bigger plans with this struct? I would just skip it.
>>
> This struct is also used in patch 5/5 to store the "enabled" status of
> the filter.
> Suspend/resume would need to enable/disable the filter, but it is not
> yet implemented in this patch series.
just do it without the nested structs. I think you are overdoing it here.
Regards
Marcel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset
2020-12-22 10:03 ` Marcel Holtmann
@ 2020-12-22 10:27 ` Archie Pusaka
0 siblings, 0 replies; 13+ messages in thread
From: Archie Pusaka @ 2020-12-22 10:27 UTC (permalink / raw)
To: Marcel Holtmann
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka,
Miao-chen Chou, Yun-Hao Chung, David S. Miller, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, LKML,
open list:NETWORKING [GENERAL]
Hi Marcel,
I've sent a new v5 patch to address this issue.
Thanks,
Archie
On Tue, 22 Dec 2020 at 18:03, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Archie,
>
> >>> When the controller is powered off, the registered advertising monitor
> >>> is removed from the controller. This patch handles the re-registration
> >>> of those monitors when the power is on.
> >>>
> >>> Signed-off-by: Archie Pusaka <apusaka@chromium.org>
> >>> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> >>> Reviewed-by: Yun-Hao Chung <howardchung@google.com>
> >>>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>> net/bluetooth/msft.c | 79 +++++++++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 74 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> >>> index f5aa0e3b1b9b..7e33a85c3f1c 100644
> >>> --- a/net/bluetooth/msft.c
> >>> +++ b/net/bluetooth/msft.c
> >>> @@ -82,8 +82,15 @@ struct msft_data {
> >>> struct list_head handle_map;
> >>> __u16 pending_add_handle;
> >>> __u16 pending_remove_handle;
> >>> +
> >>> + struct {
> >>> + u8 reregistering:1;
> >>> + } flags;
> >>> };
> >>
> >> hmmm. Do you have bigger plans with this struct? I would just skip it.
> >>
> > This struct is also used in patch 5/5 to store the "enabled" status of
> > the filter.
> > Suspend/resume would need to enable/disable the filter, but it is not
> > yet implemented in this patch series.
>
> just do it without the nested structs. I think you are overdoing it here.
>
> Regards
>
> Marcel
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 5/5] Bluetooth: advmon offload MSFT handle filter enablement
2020-12-16 4:33 [PATCH v3 0/5] MSFT offloading support for advertisement monitor Archie Pusaka
` (3 preceding siblings ...)
2020-12-16 4:33 ` [PATCH v3 4/5] Bluetooth: advmon offload MSFT handle controller reset Archie Pusaka
@ 2020-12-16 4:33 ` Archie Pusaka
4 siblings, 0 replies; 13+ messages in thread
From: Archie Pusaka @ 2020-12-16 4:33 UTC (permalink / raw)
To: linux-bluetooth, Marcel Holtmann
Cc: CrosBT Upstreaming, Archie Pusaka, Miao-chen Chou, Yun-Hao Chung,
David S. Miller, Jakub Kicinski, Johan Hedberg,
Luiz Augusto von Dentz, linux-kernel, netdev
From: Archie Pusaka <apusaka@chromium.org>
Implements the feature to disable/enable the filter used for
advertising monitor on MSFT controller, effectively have the same
effect as "remove all monitors" and "add all previously removed
monitors".
This feature would be needed when suspending, where we would not want
to get packets from anything outside the allowlist. Note that the
integration with the suspending part is not included in this patch.
Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@google.com>
---
(no changes since v1)
net/bluetooth/msft.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
net/bluetooth/msft.h | 6 ++++
2 files changed, 73 insertions(+)
diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
index 7e33a85c3f1c..055cc5a260df 100644
--- a/net/bluetooth/msft.c
+++ b/net/bluetooth/msft.c
@@ -69,6 +69,17 @@ struct msft_rp_le_cancel_monitor_advertisement {
__u8 sub_opcode;
} __packed;
+#define MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE 0x05
+struct msft_cp_le_set_advertisement_filter_enable {
+ __u8 sub_opcode;
+ __u8 enable;
+} __packed;
+
+struct msft_rp_le_set_advertisement_filter_enable {
+ __u8 status;
+ __u8 sub_opcode;
+} __packed;
+
struct msft_monitor_advertisement_handle_data {
__u8 msft_handle;
__u16 mgmt_handle;
@@ -85,6 +96,7 @@ struct msft_data {
struct {
u8 reregistering:1;
+ u8 filter_enabled:1;
} flags;
};
@@ -193,6 +205,7 @@ void msft_do_open(struct hci_dev *hdev)
if (msft_monitor_supported(hdev)) {
msft->flags.reregistering = true;
+ msft_set_filter_enable(hdev, true);
reregister_monitor_on_restart(hdev, 0);
}
}
@@ -398,6 +411,40 @@ static void msft_le_cancel_monitor_advertisement_cb(struct hci_dev *hdev,
hci_remove_adv_monitor_complete(hdev, status);
}
+static void msft_le_set_advertisement_filter_enable_cb(struct hci_dev *hdev,
+ u8 status, u16 opcode,
+ struct sk_buff *skb)
+{
+ struct msft_cp_le_set_advertisement_filter_enable *cp;
+ struct msft_rp_le_set_advertisement_filter_enable *rp;
+ struct msft_data *msft = hdev->msft_data;
+
+ rp = (struct msft_rp_le_set_advertisement_filter_enable *)skb->data;
+ if (skb->len < sizeof(*rp))
+ return;
+
+ /* Error 0x0C would be returned if the filter enabled status is
+ * already set to whatever we were trying to set.
+ * Although the default state should be disabled, some controller set
+ * the initial value to enabled. Because there is no way to know the
+ * actual initial value before sending this command, here we also treat
+ * error 0x0C as success.
+ */
+ if (status != 0x00 && status != 0x0C)
+ return;
+
+ hci_dev_lock(hdev);
+
+ cp = hci_sent_cmd_data(hdev, hdev->msft_opcode);
+ msft->flags.filter_enabled = cp->enable;
+
+ if (status == 0x0C)
+ bt_dev_warn(hdev, "MSFT filter_enable is already %s",
+ cp->enable ? "on" : "off");
+
+ hci_dev_unlock(hdev);
+}
+
static bool msft_monitor_rssi_valid(struct adv_monitor *monitor)
{
struct adv_rssi_thresholds *r = &monitor->rssi;
@@ -534,3 +581,23 @@ int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
return err;
}
+
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ struct msft_cp_le_set_advertisement_filter_enable cp;
+ struct hci_request req;
+ struct msft_data *msft = hdev->msft_data;
+ int err;
+
+ if (!msft)
+ return -EOPNOTSUPP;
+
+ cp.sub_opcode = MSFT_OP_LE_SET_ADVERTISEMENT_FILTER_ENABLE;
+ cp.enable = enable;
+
+ hci_req_init(&req, hdev);
+ hci_req_add(&req, hdev->msft_opcode, sizeof(cp), &cp);
+ err = hci_req_run_skb(&req, msft_le_set_advertisement_filter_enable_cb);
+
+ return err;
+}
diff --git a/net/bluetooth/msft.h b/net/bluetooth/msft.h
index 6f126a1f1688..f8e4d3a6d641 100644
--- a/net/bluetooth/msft.h
+++ b/net/bluetooth/msft.h
@@ -20,6 +20,7 @@ __u64 msft_get_features(struct hci_dev *hdev);
int msft_add_monitor_pattern(struct hci_dev *hdev, struct adv_monitor *monitor);
int msft_remove_monitor(struct hci_dev *hdev, struct adv_monitor *monitor,
u16 handle);
+int msft_set_filter_enable(struct hci_dev *hdev, bool enable);
#else
@@ -45,4 +46,9 @@ static inline int msft_remove_monitor(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static inline int msft_set_filter_enable(struct hci_dev *hdev, bool enable)
+{
+ return -EOPNOTSUPP;
+}
+
#endif
--
2.29.2.684.gfbc64c5ab5-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread