shared/mgmt: Add request timeout handling

PATCH Superseded Luiz Augusto von Dentz 3 participants 5 messages Jan 28, 2022 Bluetooth drivers

lore

Share & tools

lore

Luiz Augusto von Dentz

author maintainer

2022-01-28 02:07 UTC #0 lore raw

PATCH v2 1/2 shared/mgmt: Add request timeout handling

From: Luiz Augusto von Dentz <[email protected]>

This adds request timeout handling when using mgmt_send_timeout and mgmt_reply_timeout, the timeout is applied to the request only when it is actually transmitted and not while queued.
---
v2: Remove default timeout so the likes of mgmt_send/mgmt_reply use 0 instead which disables the timeout handling.

 src/shared/mgmt.c | 85 ++++++++++++++++++++++++++++++++++++++++-------
 src/shared/mgmt.h | 10 ++++++
 2 files changed, 83 insertions(+), 12 deletions(-)

src/shared/mgmt.c +73 ~~-12~~

index 41457b430..95229c248 100644
@@ -25,6 +25,7 @@
 #include "src/shared/queue.h"
 #include "src/shared/util.h"
 #include "src/shared/mgmt.h"
+#include "src/shared/timeout.h"
 
 struct mgmt {
 	int ref_count;

@@ -49,6 +50,7 @@ struct mgmt {
 };
 
 struct mgmt_request {
+	struct mgmt *mgmt;
 	unsigned int id;
 	uint16_t opcode;
 	uint16_t index;

@@ -57,6 +59,8 @@ struct mgmt_request {
 	mgmt_request_func_t callback;
 	mgmt_destroy_func_t destroy;
 	void *user_data;
+	int timeout;
+	unsigned int timeout_id;
 };
 
 struct mgmt_notify {

@@ -81,6 +85,9 @@ static void destroy_request(void *data)
 	if (request->destroy)
 		request->destroy(request->user_data);
 
+	if (request->timeout_id)
+		timeout_remove(request->timeout_id);
+
 	free(request->buf);
 	free(request);
 }

@@ -150,6 +157,26 @@ static void write_watch_destroy(void *user_data)
 	mgmt->writer_active = false;
 }
 
+static bool request_timeout(void *data)
+{
+	struct mgmt_request *request = data;
+
+	if (!request)
+		return false;
+
+	request->timeout_id = 0;
+
+	queue_remove_if(request->mgmt->pending_list, NULL, request);
+
+	if (request->callback)
+		request->callback(MGMT_STATUS_TIMEOUT, 0, NULL,
+						request->user_data);
+
+	destroy_request(request);
+
+	return false;
+}
+
 static bool send_request(struct mgmt *mgmt, struct mgmt_request *request)
 {
 	struct iovec iov;

@@ -169,6 +196,12 @@ static bool send_request(struct mgmt *mgmt, struct mgmt_request *request)
 		return false;
 	}
 
+	if (request->timeout)
+		request->timeout_id = timeout_add_seconds(request->timeout,
+							request_timeout,
+							request,
+							NULL);
+
 	util_debug(mgmt->debug_callback, mgmt->debug_data,
 				"[0x%04x] command 0x%04x",
 				request->index, request->opcode);

@@ -566,7 +599,8 @@ bool mgmt_set_close_on_unref(struct mgmt *mgmt, bool do_close)
 static struct mgmt_request *create_request(struct mgmt *mgmt, uint16_t opcode,
 				uint16_t index, uint16_t length,
 				const void *param, mgmt_request_func_t callback,
-				void *user_data, mgmt_destroy_func_t destroy)
+				void *user_data, mgmt_destroy_func_t destroy,
+				int timeout)
 {
 	struct mgmt_request *request;
 	struct mgmt_hdr *hdr;

@@ -598,12 +632,18 @@ static struct mgmt_request *create_request(struct mgmt *mgmt, uint16_t opcode,
 	hdr->index = htobs(index);
 	hdr->len = htobs(length);
 
+	/* Use a weak reference so requests don't prevent mgmt_unref to
+	 * cancel requests and free in case of the last reference is dropped by
+	 * the user.
+	 */
+	request->mgmt = mgmt;
 	request->opcode = opcode;
 	request->index = index;
 
 	request->callback = callback;
 	request->destroy = destroy;
 	request->user_data = user_data;
+	request->timeout = timeout;
 
 	return request;
 }

@@ -735,10 +775,11 @@ unsigned int mgmt_send_tlv(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 	return ret;
 }
 
-unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
-				uint16_t length, const void *param,
-				mgmt_request_func_t callback,
-				void *user_data, mgmt_destroy_func_t destroy)
+unsigned int mgmt_send_timeout(struct mgmt *mgmt, uint16_t opcode,
+				uint16_t index, uint16_t length,
+				const void *param, mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy,
+				int timeout)
 {
 	struct mgmt_request *request;

@@ -746,7 +787,7 @@ unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 		return 0;
 
 	request = create_request(mgmt, opcode, index, length, param,
-					callback, user_data, destroy);
+					callback, user_data, destroy, timeout);
 	if (!request)
 		return 0;

@@ -766,6 +807,15 @@ unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 	return request->id;
 }
 
+unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
+				uint16_t length, const void *param,
+				mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy)
+{
+	return mgmt_send_timeout(mgmt, opcode, index, length, param, callback,
+					user_data, destroy, 0);
+}
+
 unsigned int mgmt_send_nowait(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,

@@ -777,7 +827,8 @@ unsigned int mgmt_send_nowait(struct mgmt *mgmt, uint16_t opcode, uint16_t index
 		return 0;
 
 	request = create_request(mgmt, opcode, index, length, param,
-					callback, user_data, destroy);
+					callback, user_data, destroy,
+					0);
 	if (!request)
 		return 0;

@@ -792,10 +843,11 @@ unsigned int mgmt_send_nowait(struct mgmt *mgmt, uint16_t opcode, uint16_t index
 	return request->id;
 }
 
-unsigned int mgmt_reply(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
-				uint16_t length, const void *param,
-				mgmt_request_func_t callback,
-				void *user_data, mgmt_destroy_func_t destroy)
+unsigned int mgmt_reply_timeout(struct mgmt *mgmt, uint16_t opcode,
+				uint16_t index, uint16_t length,
+				const void *param, mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy,
+				int timeout)
 {
 	struct mgmt_request *request;

@@ -803,7 +855,7 @@ unsigned int mgmt_reply(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 		return 0;
 
 	request = create_request(mgmt, opcode, index, length, param,
-					callback, user_data, destroy);
+					callback, user_data, destroy, timeout);
 	if (!request)
 		return 0;

@@ -823,6 +875,15 @@ unsigned int mgmt_reply(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 	return request->id;
 }
 
+unsigned int mgmt_reply(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
+				uint16_t length, const void *param,
+				mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy)
+{
+	return mgmt_reply_timeout(mgmt, opcode, index, length, param, callback,
+						user_data, destroy, 0);
+}
+
 bool mgmt_cancel(struct mgmt *mgmt, unsigned int id)
 {
 	struct mgmt_request *request;

src/shared/mgmt.h +10 -0

index 56add613d..b413cea78 100644
@@ -55,6 +55,11 @@ unsigned int mgmt_send(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,
 				void *user_data, mgmt_destroy_func_t destroy);
+unsigned int mgmt_send_timeout(struct mgmt *mgmt, uint16_t opcode,
+				uint16_t index, uint16_t length,
+				const void *param, mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy,
+				int timeout);
 unsigned int mgmt_send_nowait(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,

@@ -63,6 +68,11 @@ unsigned int mgmt_reply(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
 				uint16_t length, const void *param,
 				mgmt_request_func_t callback,
 				void *user_data, mgmt_destroy_func_t destroy);
+unsigned int mgmt_reply_timeout(struct mgmt *mgmt, uint16_t opcode,
+				uint16_t index, uint16_t length,
+				const void *param, mgmt_request_func_t callback,
+				void *user_data, mgmt_destroy_func_t destroy,
+				int timeout);
 bool mgmt_cancel(struct mgmt *mgmt, unsigned int id);
 bool mgmt_cancel_index(struct mgmt *mgmt, uint16_t index);
 bool mgmt_cancel_all(struct mgmt *mgmt);

Signature · 1 line

2.34.1

Luiz Augusto von Dentz

author maintainer

2022-01-28 02:07 UTC #1 lore raw

PATCH v2 2/2 adapter: Remove custom MGMT send/reply timeout

From: Luiz Augusto von Dentz <[email protected]>

This removes the custom MGMT send/reply timeout since bt_mgmt itself can handle them itself and it actually start the timer only when the command is actually sent to the kernel rather then when it is queued.

Fixes https://github.com/bluez/bluez/issues/275

---
 src/adapter.c | 162 ++++----------------------------------------------
 1 file changed, 10 insertions(+), 152 deletions(-)

src/adapter.c +10 ~~-152~~

index 9772e843a..72e98ba0a 100644
@@ -311,15 +311,6 @@ struct btd_adapter {
 
 	struct oob_handler *oob_handler;
 
-	unsigned int load_ltks_id;
-	unsigned int load_ltks_timeout;
-
-	unsigned int confirm_name_id;
-	unsigned int confirm_name_timeout;
-
-	unsigned int pair_device_id;
-	unsigned int pair_device_timeout;
-
 	unsigned int db_id;		/* Service event handler for GATT db */
 
 	bool is_default;		/* true if adapter is default one */

@@ -4134,21 +4125,6 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
 							adapter->dev_id);
 }
 
-static bool load_ltks_timeout(gpointer user_data)
-{
-	struct btd_adapter *adapter = user_data;
-
-	btd_error(adapter->dev_id, "Loading LTKs timed out for hci%u",
-							adapter->dev_id);
-
-	adapter->load_ltks_timeout = 0;
-
-	mgmt_cancel(adapter->mgmt, adapter->load_ltks_id);
-	adapter->load_ltks_id = 0;
-
-	return FALSE;
-}
-
 static void load_ltks_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {

@@ -4160,11 +4136,6 @@ static void load_ltks_complete(uint8_t status, uint16_t length,
 				adapter->dev_id, mgmt_errstr(status), status);
 	}
 
-	adapter->load_ltks_id = 0;
-
-	timeout_remove(adapter->load_ltks_timeout);
-	adapter->load_ltks_timeout = 0;
-
 	DBG("LTKs loaded for hci%u", adapter->dev_id);
 }

@@ -4237,27 +4208,13 @@ static void load_ltks(struct btd_adapter *adapter, GSList *keys)
 		}
 	}
 
-	adapter->load_ltks_id = mgmt_send(adapter->mgmt,
-					MGMT_OP_LOAD_LONG_TERM_KEYS,
-					adapter->dev_id, cp_size, cp,
-					load_ltks_complete, adapter, NULL);
-
-	g_free(cp);
-
-	if (adapter->load_ltks_id == 0) {
+	if (!mgmt_send_timeout(adapter->mgmt, MGMT_OP_LOAD_LONG_TERM_KEYS,
+			adapter->dev_id, cp_size, cp, load_ltks_complete,
+			adapter, NULL, 2))
 		btd_error(adapter->dev_id, "Failed to load LTKs for hci%u",
 							adapter->dev_id);
-		return;
-	}
 
-	/*
-	 * This timeout handling is needed since the kernel is stupid
-	 * and forgets to send a command complete response. However in
-	 * case of failures it does send a command status.
-	 */
-	adapter->load_ltks_timeout = timeout_add_seconds(2,
-						load_ltks_timeout, adapter,
-						NULL);
+	g_free(cp);
 }
 
 static void load_irks_complete(uint8_t status, uint16_t length,

@@ -5610,15 +5567,6 @@ static void adapter_free(gpointer user_data)
 		adapter->passive_scan_timeout = 0;
 	}
 
-	if (adapter->load_ltks_timeout > 0)
-		timeout_remove(adapter->load_ltks_timeout);
-
-	if (adapter->confirm_name_timeout > 0)
-		timeout_remove(adapter->confirm_name_timeout);
-
-	if (adapter->pair_device_timeout > 0)
-		timeout_remove(adapter->pair_device_timeout);
-
 	if (adapter->auth_idle_id)
 		g_source_remove(adapter->auth_idle_id);

@@ -6746,21 +6694,6 @@ const bdaddr_t *btd_adapter_get_address(struct btd_adapter *adapter)
 	return &adapter->bdaddr;
 }
 
-static bool confirm_name_timeout(gpointer user_data)
-{
-	struct btd_adapter *adapter = user_data;
-
-	btd_error(adapter->dev_id, "Confirm name timed out for hci%u",
-							adapter->dev_id);
-
-	adapter->confirm_name_timeout = 0;
-
-	mgmt_cancel(adapter->mgmt, adapter->confirm_name_id);
-	adapter->confirm_name_id = 0;
-
-	return FALSE;
-}
-
 static void confirm_name_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {

@@ -6770,13 +6703,9 @@ static void confirm_name_complete(uint8_t status, uint16_t length,
 		btd_error(adapter->dev_id,
 				"Failed to confirm name for hci%u: %s (0x%02x)",
 				adapter->dev_id, mgmt_errstr(status), status);
+		return;
 	}
 
-	adapter->confirm_name_id = 0;
-
-	timeout_remove(adapter->confirm_name_timeout);
-	adapter->confirm_name_timeout = 0;
-
 	DBG("Confirm name complete for hci%u", adapter->dev_id);
 }

@@ -6790,49 +6719,16 @@ static void confirm_name(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 	DBG("hci%d bdaddr %s name_known %u", adapter->dev_id, addr,
 								name_known);
 
-	/*
-	 * If the kernel does not answer the confirm name command with
-	 * a command complete or command status in time, this might
-	 * race against another device found event that also requires
-	 * to confirm the name. If there is a pending command, just
-	 * cancel it to be safe here.
-	 */
-	if (adapter->confirm_name_id > 0) {
-		btd_warn(adapter->dev_id,
-				"Found pending confirm name for hci%u",
-							adapter->dev_id);
-		mgmt_cancel(adapter->mgmt, adapter->confirm_name_id);
-	}
-
-	if (adapter->confirm_name_timeout > 0) {
-		timeout_remove(adapter->confirm_name_timeout);
-		adapter->confirm_name_timeout = 0;
-	}
-
 	memset(&cp, 0, sizeof(cp));
 	bacpy(&cp.addr.bdaddr, bdaddr);
 	cp.addr.type = bdaddr_type;
 	cp.name_known = name_known;
 
-	adapter->confirm_name_id = mgmt_reply(adapter->mgmt,
-					MGMT_OP_CONFIRM_NAME,
-					adapter->dev_id, sizeof(cp), &cp,
-					confirm_name_complete, adapter, NULL);
-
-	if (adapter->confirm_name_id == 0) {
+	if (!mgmt_reply_timeout(adapter->mgmt, MGMT_OP_CONFIRM_NAME,
+				adapter->dev_id, sizeof(cp), &cp,
+				confirm_name_complete, adapter, NULL, 2))
 		btd_error(adapter->dev_id, "Failed to confirm name for hci%u",
 							adapter->dev_id);
-		return;
-	}
-
-	/*
-	 * This timeout handling is needed since the kernel is stupid
-	 * and forgets to send a command complete response. However in
-	 * case of failures it does send a command status.
-	 */
-	adapter->confirm_name_timeout = timeout_add_seconds(2,
-						confirm_name_timeout, adapter,
-						NULL);
 }
 
 static void adapter_msd_notify(struct btd_adapter *adapter,

@@ -8106,21 +8002,6 @@ static void free_pair_device_data(void *user_data)
 	g_free(data);
 }
 
-static bool pair_device_timeout(gpointer user_data)
-{
-	struct pair_device_data *data = user_data;
-	struct btd_adapter *adapter = data->adapter;
-
-	btd_error(adapter->dev_id, "Pair device timed out for hci%u",
-							adapter->dev_id);
-
-	adapter->pair_device_timeout = 0;
-
-	adapter_cancel_bonding(adapter, &data->bdaddr, data->addr_type);
-
-	return FALSE;
-}
-
 static void pair_device_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {

@@ -8130,13 +8011,6 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 
 	DBG("%s (0x%02x)", mgmt_errstr(status), status);
 
-	adapter->pair_device_id = 0;
-
-	if (adapter->pair_device_timeout > 0) {
-		timeout_remove(adapter->pair_device_timeout);
-		adapter->pair_device_timeout = 0;
-	}
-
 	/* Workaround for a kernel bug
 	 *
 	 * Broken kernels may reply to device pairing command with command

@@ -8164,12 +8038,6 @@ static void pair_device_complete(uint8_t status, uint16_t length,
 int adapter_create_bonding(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 					uint8_t addr_type, uint8_t io_cap)
 {
-	if (adapter->pair_device_id > 0) {
-		btd_error(adapter->dev_id,
-			"Unable pair since another pairing is in progress");
-		return -EBUSY;
-	}
-
 	suspend_discovery(adapter);
 
 	return adapter_bonding_attempt(adapter, bdaddr, addr_type, io_cap);

@@ -8201,10 +8069,10 @@ int adapter_bonding_attempt(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 	bacpy(&data->bdaddr, bdaddr);
 	data->addr_type = addr_type;
 
-	id = mgmt_send(adapter->mgmt, MGMT_OP_PAIR_DEVICE,
+	id = mgmt_send_timeout(adapter->mgmt, MGMT_OP_PAIR_DEVICE,
 				adapter->dev_id, sizeof(cp), &cp,
 				pair_device_complete, data,
-				free_pair_device_data);
+				free_pair_device_data, BONDING_TIMEOUT);
 
 	if (id == 0) {
 		btd_error(adapter->dev_id, "Failed to pair %s for hci%u",

@@ -8213,16 +8081,6 @@ int adapter_bonding_attempt(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 		return -EIO;
 	}
 
-	adapter->pair_device_id = id;
-
-	/* Due to a bug in the kernel it is possible that a LE pairing
-	 * request never times out. Therefore, add a timer to clean up
-	 * if no response arrives
-	 */
-	adapter->pair_device_timeout = timeout_add_seconds(BONDING_TIMEOUT,
-						pair_device_timeout, data,
-						NULL);
-
 	return 0;
 }

Signature · 1 line

2.34.1

Marcel Holtmann

maintainer

2022-01-28 13:37 UTC #2 lore raw

Re: [PATCH v2 2/2] adapter: Remove custom MGMT send/reply timeout

Hi Luiz,

Quoted 78 lines

Fixes https://github.com/bluez/bluez/issues/275

---
src/adapter.c | 162 ++++----------------------------------------------

1 file changed, 10 insertions(+), 152 deletions(-)

src/adapter.c +3 ~~-37~~

index 9772e843a..72e98ba0a 100644
@@ -311,15 +311,6 @@ struct btd_adapter {
 
 	struct oob_handler *oob_handler;
 
-	unsigned int load_ltks_id;
-	unsigned int load_ltks_timeout;
-
-	unsigned int confirm_name_id;
-	unsigned int confirm_name_timeout;
-
-	unsigned int pair_device_id;
-	unsigned int pair_device_timeout;
-
 	unsigned int db_id;		/* Service event handler for GATT db */
 
 	bool is_default;		/* true if adapter is default one */

@@ -4134,21 +4125,6 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
 							adapter->dev_id);
 }
 
-static bool load_ltks_timeout(gpointer user_data)
-{
-	struct btd_adapter *adapter = user_data;
-
-	btd_error(adapter->dev_id, "Loading LTKs timed out for hci%u",
-							adapter->dev_id);
-
-	adapter->load_ltks_timeout = 0;
-
-	mgmt_cancel(adapter->mgmt, adapter->load_ltks_id);
-	adapter->load_ltks_id = 0;
-
-	return FALSE;
-}
-
 static void load_ltks_complete(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {

@@ -4160,11 +4136,6 @@ static void load_ltks_complete(uint8_t status, uint16_t length,
 				adapter->dev_id, mgmt_errstr(status), status);
 	}
 
-	adapter->load_ltks_id = 0;
-
-	timeout_remove(adapter->load_ltks_timeout);
-	adapter->load_ltks_timeout = 0;
-
 	DBG("LTKs loaded for hci%u", adapter->dev_id);
 }

@@ -4237,27 +4208,13 @@ static void load_ltks(struct btd_adapter *adapter, GSList *keys)
 		}
 	}
 
-	adapter->load_ltks_id = mgmt_send(adapter->mgmt,
-					MGMT_OP_LOAD_LONG_TERM_KEYS,
-					adapter->dev_id, cp_size, cp,
-					load_ltks_complete, adapter, NULL);
-
-	g_free(cp);
-
-	if (adapter->load_ltks_id == 0) {
+	if (!mgmt_send_timeout(adapter->mgmt, MGMT_OP_LOAD_LONG_TERM_KEYS,
+			adapter->dev_id, cp_size, cp, load_ltks_complete,
+			adapter, NULL, 2))

src/adapter.c static void load_ltks(struct btd_adapter *adapter, GSList *keys) · line 4257

Quoted 10 lines

src/adapter.c +0 -7

 		btd_error(adapter->dev_id, "Failed to load LTKs for hci%u",
 							adapter->dev_id);
-		return;
-	}
 
-	/*
-	 * This timeout handling is needed since the kernel is stupid
-	 * and forgets to send a command complete response. However in
-	 * case of failures it does send a command status.
-	 */

please don’t loose these comments. They are important because of the kernel bugs we had.

Regards

Marcel

Luiz Augusto von Dentz

author maintainer

2022-01-28 20:28 UTC #3 lore raw

Re: [PATCH v2 2/2] adapter: Remove custom MGMT send/reply timeout

Hi Marcel,

Quoted 86 lines · Marcel Holtmann

Hi Luiz,

Quoted 83 lines

Fixes https://github.com/bluez/bluez/issues/275

---
src/adapter.c | 162 ++++----------------------------------------------

1 file changed, 10 insertions(+), 152 deletions(-)

src/adapter.c +3 ~~-39~~

index 9772e843a..72e98ba0a 100644
@@ -311,15 +311,6 @@ struct btd_adapter {
 
      struct oob_handler *oob_handler;
 
-     unsigned int load_ltks_id;
-     unsigned int load_ltks_timeout;
-
-     unsigned int confirm_name_id;
-     unsigned int confirm_name_timeout;
-
-     unsigned int pair_device_id;
-     unsigned int pair_device_timeout;
-
      unsigned int db_id;             /* Service event handler for GATT db */
 
      bool is_default;                /* true if adapter is default one */

@@ -4134,21 +4125,6 @@ static void load_link_keys(struct btd_adapter *adapter, GSList *keys,
                                                      adapter->dev_id);
 }
 
-static bool load_ltks_timeout(gpointer user_data)
-{
-     struct btd_adapter *adapter = user_data;
-
-     btd_error(adapter->dev_id, "Loading LTKs timed out for hci%u",
-                                                     adapter->dev_id);
-
-     adapter->load_ltks_timeout = 0;
-
-     mgmt_cancel(adapter->mgmt, adapter->load_ltks_id);
-     adapter->load_ltks_id = 0;
-
-     return FALSE;
-}
-
 static void load_ltks_complete(uint8_t status, uint16_t length,
                                      const void *param, void *user_data)
 {

@@ -4160,11 +4136,6 @@ static void load_ltks_complete(uint8_t status, uint16_t length,
                              adapter->dev_id, mgmt_errstr(status), status);
      }
 
-     adapter->load_ltks_id = 0;
-
-     timeout_remove(adapter->load_ltks_timeout);
-     adapter->load_ltks_timeout = 0;
-
      DBG("LTKs loaded for hci%u", adapter->dev_id);
 }

@@ -4237,27 +4208,13 @@ static void load_ltks(struct btd_adapter *adapter, GSList *keys)
              }
      }
 
-     adapter->load_ltks_id = mgmt_send(adapter->mgmt,
-                                     MGMT_OP_LOAD_LONG_TERM_KEYS,
-                                     adapter->dev_id, cp_size, cp,
-                                     load_ltks_complete, adapter, NULL);
-
-     g_free(cp);
-
-     if (adapter->load_ltks_id == 0) {
+     if (!mgmt_send_timeout(adapter->mgmt, MGMT_OP_LOAD_LONG_TERM_KEYS,
+                     adapter->dev_id, cp_size, cp, load_ltks_complete,
+                     adapter, NULL, 2))
              btd_error(adapter->dev_id, "Failed to load LTKs for hci%u",
                                                      adapter->dev_id);
-             return;
-     }

Quoted 7 lines

src/adapter.c static void load_ltks(struct btd_adapter *adapter, GSList *keys) · line 4257
Quoted 5 lines
src/adapter.c +0 -5
-     /*
-      * This timeout handling is needed since the kernel is stupid
-      * and forgets to send a command complete response. However in
-      * case of failures it does send a command status.
-      */
please don’t loose these comments. They are important because of the kernel bugs we had.

Sure I will incorporate them back.

Quoted 4 lines

Regards
Marcel

Signature · 1 line

Luiz Augusto von Dentz

[email protected]

2022-01-28 04:09 UTC #4 lore raw

In reply to Luiz Augusto von Dentz

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

---Test result---

Test Summary:

CheckPatch                    PASS      3.07 seconds
GitLint                       PASS      2.06 seconds
Prep - Setup ELL              PASS      49.45 seconds
Build - Prep                  PASS      0.80 seconds
Build - Configure             PASS      9.73 seconds
Build - Make                  PASS      1439.59 seconds
Make Check                    PASS      12.59 seconds
Make Check w/Valgrind         PASS      510.88 seconds
Make Distcheck                PASS      265.99 seconds
Build w/ext ELL - Configure   PASS      9.90 seconds
Build w/ext ELL - Make        PASS      1417.04 seconds
Incremental Build with patchesPASS      2901.91 seconds

---
Regards,
Linux Bluetooth