2015-10-31 03:47:40

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: add virtual hci command

This patch adds virtual HCI command HCI_OP_VIRT_LE_SCAN_DISABLED.
It's called virtual, because it'll never be sent to the controller.
It can be added to asynchronous hci_request, and when time comes for its
execution, it can do some logic and decide either to transform itself
into real packet, or send nothing.

Logic for HCI_OP_VIRT_LE_SCAN_DISABLED is as follows: if at time of
execution scan is disabled, do nothing. If scan is enabled, disable it.
It is needed because scan state migth change between moment when we
schedule command and execution time. Later patches will use this
new command to fix some error scenarios.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
include/net/bluetooth/hci.h | 2 +
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 86 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 89 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 0205b80..4253312 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1961,6 +1961,8 @@ struct hci_ev_si_security {
__u8 incoming;
} __packed;

+#define HCI_OP_VIRT_LE_SCAN_DISABLE 0xFF0b
+
/* ---- HCI Packet structures ---- */
#define HCI_COMMAND_HDR_SIZE 3
#define HCI_EVENT_HDR_SIZE 2
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1878d0a..4fcca8f 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -322,6 +322,7 @@ struct hci_dev {
struct delayed_work service_cache;

struct delayed_work cmd_timer;
+ struct work_struct cmd_timer_cancel;

struct work_struct rx_work;
struct work_struct cmd_work;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 83a6aac..b012059 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2623,6 +2623,15 @@ static void hci_cmd_timeout(struct work_struct *work)
queue_work(hdev->workqueue, &hdev->cmd_work);
}

+static void hci_cmd_timeout_cancel(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev,
+ cmd_timer_cancel);
+
+ BT_DBG("%s cmd_timer canceled", hdev->name);
+ cancel_delayed_work(&hdev->cmd_timer);
+}
+
struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev,
bdaddr_t *bdaddr, u8 bdaddr_type)
{
@@ -3304,6 +3313,7 @@ struct hci_dev *hci_alloc_dev(void)
init_waitqueue_head(&hdev->req_wait_q);

INIT_DELAYED_WORK(&hdev->cmd_timer, hci_cmd_timeout);
+ INIT_WORK(&hdev->cmd_timer_cancel, hci_cmd_timeout_cancel);

hci_init_sysfs(hdev);
discovery_init(hdev);
@@ -3594,12 +3604,88 @@ int hci_unregister_cb(struct hci_cb *cb)
}
EXPORT_SYMBOL(hci_unregister_cb);

+static void hci_virtual_frame_skip(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ struct hci_command_hdr *hdr = (void *)skb->data;
+ hci_req_complete_t req_complete = NULL;
+ hci_req_complete_skb_t req_complete_skb = NULL;
+ int status = 0;
+ __u16 opcode;
+
+ BT_DBG("%s Skipping virtual packet 0x%4.4x", hdev->name,
+ bt_cb(skb)->opcode);
+
+ /* Do cleanup similar to that in hci_cmd_status_evt */
+ if (!test_bit(HCI_RESET, &hdev->flags))
+ atomic_set(&hdev->cmd_cnt, 1);
+
+ opcode = __le16_to_cpu(hdr->opcode);
+
+ /* Indicate request completion */
+ hci_req_cmd_complete(hdev, opcode, 0, &req_complete, &req_complete_skb);
+
+ if (atomic_read(&hdev->cmd_cnt) && !skb_queue_empty(&hdev->cmd_q))
+ queue_work(hdev->workqueue, &hdev->cmd_work);
+
+ /* Do cleanup similar to that in hci_event_packet */
+ if (req_complete)
+ req_complete(hdev, status, opcode);
+ else if (req_complete_skb)
+ req_complete_skb(hdev, status, opcode, skb);
+
+ kfree_skb(skb);
+
+ /* There will be no packet really send to controller, there will be no
+ * response that normally stops cmd_timer. cmd_timer was not scheduled
+ * yet, but will be in hci_cmd_work. Make sure it'll be cancelled right
+ * away.
+ */
+ schedule_work(&hdev->cmd_timer_cancel);
+}
+
+static bool hci_transform_virtual_frame(struct hci_dev *hdev,
+ struct sk_buff *skb)
+{
+ struct hci_command_hdr *hdr = (void *)skb->data;
+
+ BT_DBG("%s Transforming virual packet 0x%4.4x", hdev->name,
+ bt_cb(skb)->opcode);
+
+ switch (bt_cb(skb)->opcode) {
+ case HCI_OP_VIRT_LE_SCAN_DISABLE:
+ if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
+ /* Transform virtual command into real one. It already
+ * have proper size and data.
+ */
+ hdr->opcode = cpu_to_le16(HCI_OP_LE_SET_SCAN_ENABLE);
+ bt_cb(skb)->opcode = HCI_OP_LE_SET_SCAN_ENABLE;
+ } else {
+ hci_virtual_frame_skip(hdev, skb);
+ return false;
+ }
+ break;
+ default:
+ BT_ERR("Unknown virtual command 0x%4.4x", bt_cb(skb)->opcode);
+ return false;
+ }
+
+ return true;
+}
+
static void hci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
{
int err;

BT_DBG("%s type %d len %d", hdev->name, bt_cb(skb)->pkt_type, skb->len);

+ /* If it was virtual packet, transform it. If nothing is left after
+ * transformation finish here. If it changed into real packet, send it
+ */
+ if (bt_cb(skb)->pkt_type == HCI_COMMAND_PKT &&
+ (bt_cb(skb)->opcode & 0xff00) == 0xff00 &&
+ !hci_transform_virtual_frame(hdev, skb))
+ return;
+
/* Time stamp */
__net_timestamp(skb);

--
2.5.0



2015-10-31 03:47:41

by Jakub Pawlowski

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: fix connect race condition

Right now, when calling Device1.Connect() on two LE devices using RPA
at same time, second call to Connect() will fail every few times. It is
due to race condition inside kernel, that happens when adding devices to
whitelist. This patch fixes that.

Each time device is added to whitelist, __hci_update_background_scan will
be called. Scan state will be checked, and two commands will be scheduled,
but not executed yet: LE_SET_SCAN_PARAM and LE_SET_SCAN_ENABLE. Then
during execution time, if those commands were scheduled twice, first two
commands will succeed, but later two will fail, because LE_SET_SCAN_PARAM
must be executed when scan is disabled, and fist two commands left it
enabled.

In order to solve that, HCI_OP_VIRT_LE_SCAN_DISABLE must be used. It'll
check the scan state at the runtime, instead of schedule time, and will
stop the scan if necessary.

Signed-off-by: Jakub Pawlowski <[email protected]>
---
net/bluetooth/hci_request.c | 22 ++++++++++++++++------
net/bluetooth/mgmt.c | 1 -
2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 981f8a2..9a5c41c 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -150,6 +150,19 @@ void hci_req_add_le_scan_disable(struct hci_request *req)
hci_req_add(req, HCI_OP_LE_SET_SCAN_ENABLE, sizeof(cp), &cp);
}

+void hci_req_add_virt_le_scan_disable(struct hci_request *req)
+{
+ struct hci_cp_le_set_scan_enable cp;
+
+ memset(&cp, 0, sizeof(cp));
+ cp.enable = LE_SCAN_DISABLE;
+
+ /* Instead of sending real scan disable packet, send virtual one,
+ * that will check and stop scan at time of running command.
+ */
+ hci_req_add(req, HCI_OP_VIRT_LE_SCAN_DISABLE, sizeof(cp), &cp);
+}
+
static void add_to_white_list(struct hci_request *req,
struct hci_conn_params *params)
{
@@ -258,6 +271,9 @@ void hci_req_add_le_passive_scan(struct hci_request *req)
u8 own_addr_type;
u8 filter_policy;

+ /* HCI_OP_LE_SET_SCAN_PARAM must be run when scan is disabled. */
+ hci_req_add_virt_le_scan_disable(req);
+
/* Set require_privacy to false since no SCAN_REQ are send
* during passive scanning. Not using an non-resolvable address
* here is important so that peer devices using direct
@@ -531,12 +547,6 @@ void __hci_update_background_scan(struct hci_request *req)
if (hci_lookup_le_connect(hdev))
return;

- /* If controller is currently scanning, we stop it to ensure we
- * don't miss any advertising (due to duplicates filter).
- */
- if (hci_dev_test_flag(hdev, HCI_LE_SCAN))
- hci_req_add_le_scan_disable(req);
-
hci_req_add_le_passive_scan(req);

BT_DBG("%s starting background scanning", hdev->name);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7f22119..89efb31 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -5045,7 +5045,6 @@ static int set_scan_params(struct sock *sk, struct hci_dev *hdev,

hci_req_init(&req, hdev);

- hci_req_add_le_scan_disable(&req);
hci_req_add_le_passive_scan(&req);

hci_req_run(&req, NULL);
--
2.5.0