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

Hi Marcel,

On Fri, Jan 28, 2022 at 5:37 AM Marcel Holtmann [off-list ref] wrote:

Hi Luiz,

quoted

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(-)

diff --git a/src/adapter.c b/src/adapter.c
index 9772e843a..72e98ba0a 100644
--- a/src/adapter.c
+++ b/src/adapter.c

@@ -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.
-      */

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

Sure I will incorporate them back.

Regards

Marcel

-- 
Luiz Augusto von Dentz