2020-06-09 20:34:46

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ v2 0/4] Put safeguards around Leave & Attach calls

This set of patches addresses the situation when an application
calls Leave() or Attach() methods on a node that has another incomplete
method call on Network interface associated with it.

The simple solution is to return error in response to Leave() method
if an application still owes the daemon either a reply to GetManagedObjects
request in case of Attach() or a reply to JoinComplete() in case of
Join(), Create() or Import()

Similarly, a "Busy" error is returned in response to Attach() method
if an application still owes the daemon a reply to JoinComplete()
in case of pending Join(), Create() or Import()

Inga Stotland (4):
mesh: Make "Busy" and "InProgress" to be distinct errors
mesh: Add destroy callback to dbus_send_with_timeout()
mesh: Add timeout to a get managed objects call
mesh: Add "node is busy" check for Leave() & Attach()

doc/mesh-api.txt | 5 +++-
mesh/dbus.c | 14 ++++++++---
mesh/dbus.h | 1 +
mesh/error.h | 1 +
mesh/manager.c | 11 ++++-----
mesh/mesh.c | 22 +++++++++++------
mesh/node.c | 64 +++++++++++++++++++++++++++++++-----------------
mesh/node.h | 1 +
test/test-mesh | 2 --
9 files changed, 78 insertions(+), 43 deletions(-)

--
2.26.2


2020-06-09 20:37:36

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/4] mesh: Add timeout to a get managed objects call

Switch to using dbus_send_with_timeout when making a request
to get managed objects from an application.
---
mesh/node.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index d1d4da23d..7ec06437b 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1459,7 +1459,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
struct keyring_net_key net_key;
uint8_t dev_key[16];

- if (l_dbus_message_is_error(msg)) {
+ if (!msg || l_dbus_message_is_error(msg)) {
l_error("Failed to get app's dbus objects");
goto fail;
}
@@ -1616,8 +1616,7 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)

fail:
/* Handle failed requests */
- if (node)
- node_remove(node);
+ node_remove(node);

if (req->type == REQUEST_TYPE_JOIN)
req->join_ready_cb(NULL, NULL);
@@ -1628,6 +1627,20 @@ fail:
l_free(req->import);
}

+static void send_managed_objects_request(const char *destination,
+ const char *path,
+ struct managed_obj_request *req)
+{
+ struct l_dbus_message *msg;
+
+ msg = l_dbus_message_new_method_call(dbus_get_bus(), destination, path,
+ L_DBUS_INTERFACE_OBJECT_MANAGER,
+ "GetManagedObjects");
+ l_dbus_message_set_arguments(msg, "");
+ dbus_send_with_timeout(dbus_get_bus(), msg, get_managed_objects_cb,
+ req, l_free, DEFAULT_DBUS_TIMEOUT);
+}
+
/* Establish relationship between application and mesh node */
void node_attach(const char *app_root, const char *sender, uint64_t token,
node_ready_func_t cb, void *user_data)
@@ -1661,11 +1674,7 @@ void node_attach(const char *app_root, const char *sender, uint64_t token,
req->attach = node;
req->type = REQUEST_TYPE_ATTACH;

- l_dbus_method_call(dbus_get_bus(), sender, app_root,
- L_DBUS_INTERFACE_OBJECT_MANAGER,
- "GetManagedObjects", NULL,
- get_managed_objects_cb,
- req, l_free);
+ send_managed_objects_request(sender, app_root, req);
}

/* Create a temporary pre-provisioned node */
@@ -1681,11 +1690,7 @@ void node_join(const char *app_root, const char *sender, const uint8_t *uuid,
req->join_ready_cb = cb;
req->type = REQUEST_TYPE_JOIN;

- l_dbus_method_call(dbus_get_bus(), sender, app_root,
- L_DBUS_INTERFACE_OBJECT_MANAGER,
- "GetManagedObjects", NULL,
- get_managed_objects_cb,
- req, l_free);
+ send_managed_objects_request(sender, app_root, req);
}

void node_import(const char *app_root, const char *sender, const uint8_t *uuid,
@@ -1715,11 +1720,7 @@ void node_import(const char *app_root, const char *sender, const uint8_t *uuid,

req->type = REQUEST_TYPE_IMPORT;

- l_dbus_method_call(dbus_get_bus(), sender, app_root,
- L_DBUS_INTERFACE_OBJECT_MANAGER,
- "GetManagedObjects", NULL,
- get_managed_objects_cb,
- req, l_free);
+ send_managed_objects_request(sender, app_root, req);
}

void node_create(const char *app_root, const char *sender, const uint8_t *uuid,
@@ -1735,11 +1736,7 @@ void node_create(const char *app_root, const char *sender, const uint8_t *uuid,
req->pending_msg = user_data;
req->type = REQUEST_TYPE_CREATE;

- l_dbus_method_call(dbus_get_bus(), sender, app_root,
- L_DBUS_INTERFACE_OBJECT_MANAGER,
- "GetManagedObjects", NULL,
- get_managed_objects_cb,
- req, l_free);
+ send_managed_objects_request(sender, app_root, req);
}

static void build_element_config(void *a, void *b)
--
2.26.2

2020-06-09 20:37:36

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/4] mesh: Make "Busy" and "InProgress" to be distinct errors

This separates "Busy" and "InProgress" error codes:
MESH_ERROR_IN_PROGRESS maps to org.bluez.mesh.Error.InProgress
MESH_ERROR_BUSY maps to org.bluez.mesh.Error.Busy

Minor API change:
UpdateAppKey() returns "InProgress" error instead of "Busy"
---
doc/mesh-api.txt | 2 +-
mesh/dbus.c | 3 ++-
mesh/error.h | 1 +
mesh/manager.c | 11 +++++------
4 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 24cface22..e85f0bf52 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -654,7 +654,7 @@ Methods:
org.bluez.mesh.Error.Failed
org.bluez.mesh.Error.InvalidArguments
org.bluez.mesh.Error.DoesNotExist
- org.bluez.mesh.Error.Busy
+ org.bluez.mesh.Error.InProgress

void DeleteAppKey(uint16 app_index)

diff --git a/mesh/dbus.c b/mesh/dbus.c
index bf0f73bd9..83ae22c9f 100644
--- a/mesh/dbus.c
+++ b/mesh/dbus.c
@@ -56,7 +56,8 @@ static struct error_entry const error_table[] =
{ ERROR_INTERFACE ".NotAuthorized", "Permission denied"},
{ ERROR_INTERFACE ".NotFound", "Object not found"},
{ ERROR_INTERFACE ".InvalidArgs", "Invalid arguments"},
- { ERROR_INTERFACE ".InProgress", "Already in progress"},
+ { ERROR_INTERFACE ".InProgress", "Operation already in progress"},
+ { ERROR_INTERFACE ".Busy", "Busy"},
{ ERROR_INTERFACE ".AlreadyExists", "Already exists"},
{ ERROR_INTERFACE ".DoesNotExist", "Does not exist"},
{ ERROR_INTERFACE ".Canceled", "Operation canceled"},
diff --git a/mesh/error.h b/mesh/error.h
index f3e0f5476..2809915b0 100644
--- a/mesh/error.h
+++ b/mesh/error.h
@@ -27,6 +27,7 @@ enum mesh_error {
MESH_ERROR_NOT_AUTHORIZED,
MESH_ERROR_NOT_FOUND,
MESH_ERROR_INVALID_ARGS,
+ MESH_ERROR_IN_PROGRESS,
MESH_ERROR_BUSY,
MESH_ERROR_ALREADY_EXISTS,
MESH_ERROR_DOES_NOT_EXIST,
diff --git a/mesh/manager.c b/mesh/manager.c
index 2be471088..8ef681366 100644
--- a/mesh/manager.c
+++ b/mesh/manager.c
@@ -60,7 +60,7 @@ static void scan_cancel(struct l_timeout *timeout, void *user_data)
struct mesh_io *io;
struct mesh_net *net;

- l_debug("scan_cancel");
+ l_debug("");

if (scan_timeout)
l_timeout_remove(scan_timeout);
@@ -249,11 +249,10 @@ static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);

if (!l_dbus_message_iter_get_fixed_array(&iter_uuid, &uuid, &n)
- || n != 16) {
- l_debug("n = %u", n);
+ || n != 16)
return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
"Bad device UUID");
- }
+
/* Allow AddNode to cancel Scanning if from the same node */
if (scan_node) {
if (scan_node != node)
@@ -263,7 +262,6 @@ static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
}

/* Invoke Prov Initiator */
-
add_pending = l_new(struct add_data, 1);
memcpy(add_pending->uuid, uuid, 16);
add_pending->node = node;
@@ -554,7 +552,8 @@ static struct l_dbus_message *update_subnet_call(struct l_dbus *dbus,
}

/* All other phases mean KR already in progress over-the-air */
- return dbus_error(msg, MESH_ERROR_BUSY, "Key Refresh in progress");
+ return dbus_error(msg, MESH_ERROR_IN_PROGRESS,
+ "Key Refresh in progress");
}

static struct l_dbus_message *delete_subnet_call(struct l_dbus *dbus,
--
2.26.2