This set of patches addresses the situation when an application
calls Leave() method 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()
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() method
doc/mesh-api.txt | 4 +++-
mesh/dbus.c | 14 +++++++----
mesh/dbus.h | 1 +
mesh/error.h | 1 +
mesh/manager.c | 11 ++++-----
mesh/mesh.c | 17 +++++++------
mesh/node.c | 62 +++++++++++++++++++++++++++++-------------------
mesh/node.h | 2 +-
8 files changed, 68 insertions(+), 44 deletions(-)
--
2.26.2
This adds a destroy callback as a function parameter to
dbus_send_with_timeout() to allow automatic release of user data
on either reply or timeout.
---
mesh/dbus.c | 11 ++++++++---
mesh/dbus.h | 1 +
mesh/mesh.c | 12 ++++++------
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/mesh/dbus.c b/mesh/dbus.c
index 83ae22c9f..ba2a28de5 100644
--- a/mesh/dbus.c
+++ b/mesh/dbus.c
@@ -41,6 +41,7 @@ struct send_info {
struct l_dbus *dbus;
struct l_timeout *timeout;
l_dbus_message_func_t cb;
+ l_dbus_destroy_func_t destroy_cb;
void *user_data;
uint32_t serial;
};
@@ -153,12 +154,14 @@ void dbus_append_dict_entry_basic(struct l_dbus_message_builder *builder,
l_dbus_message_builder_leave_dict(builder);
}
-static void send_reply(struct l_dbus_message *message, void *user_data)
+static void send_reply_cb(struct l_dbus_message *message, void *user_data)
{
struct send_info *info = user_data;
l_timeout_remove(info->timeout);
info->cb(message, info->user_data);
+ if (info->destroy_cb)
+ info->destroy_cb(info->user_data);
l_free(info);
}
@@ -167,12 +170,13 @@ static void send_timeout(struct l_timeout *timeout, void *user_data)
struct send_info *info = user_data;
l_dbus_cancel(info->dbus, info->serial);
- send_reply(NULL, info);
+ send_reply_cb(NULL, info);
}
void dbus_send_with_timeout(struct l_dbus *dbus, struct l_dbus_message *msg,
l_dbus_message_func_t cb,
void *user_data,
+ l_dbus_destroy_func_t destroy,
unsigned int seconds)
{
struct send_info *info = l_new(struct send_info, 1);
@@ -180,7 +184,8 @@ void dbus_send_with_timeout(struct l_dbus *dbus, struct l_dbus_message *msg,
info->dbus = dbus;
info->cb = cb;
info->user_data = user_data;
- info->serial = l_dbus_send_with_reply(dbus, msg, send_reply,
+ info->destroy_cb = destroy;
+ info->serial = l_dbus_send_with_reply(dbus, msg, send_reply_cb,
info, NULL);
info->timeout = l_timeout_create(seconds, send_timeout, info, NULL);
}
diff --git a/mesh/dbus.h b/mesh/dbus.h
index aafb85f6b..89d6b1d31 100644
--- a/mesh/dbus.h
+++ b/mesh/dbus.h
@@ -36,4 +36,5 @@ struct l_dbus_message *dbus_error(struct l_dbus_message *msg, int err,
void dbus_send_with_timeout(struct l_dbus *dbus, struct l_dbus_message *msg,
l_dbus_message_func_t cb,
void *user_data,
+ l_dbus_destroy_func_t destroy,
unsigned int seconds);
diff --git a/mesh/mesh.c b/mesh/mesh.c
index 24ea3afd6..a5935c216 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -439,12 +439,12 @@ static void send_join_failed(const char *owner, const char *path,
free_pending_join_call(true);
}
-static void prov_join_complete_reply_cb(struct l_dbus_message *message,
+static void prov_join_complete_reply_cb(struct l_dbus_message *msg,
void *user_data)
{
bool failed = false;
- if (!message || l_dbus_message_is_error(message))
+ if (!msg || l_dbus_message_is_error(msg))
failed = true;
if (!failed)
@@ -488,7 +488,7 @@ static bool prov_complete_cb(void *user_data, uint8_t status,
l_dbus_message_set_arguments(msg, "t", l_get_be64(token));
dbus_send_with_timeout(dbus, msg, prov_join_complete_reply_cb,
- NULL, DEFAULT_DBUS_TIMEOUT);
+ NULL, NULL, DEFAULT_DBUS_TIMEOUT);
return true;
}
@@ -666,12 +666,12 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
return l_dbus_message_new_method_return(msg);
}
-static void create_join_complete_reply_cb(struct l_dbus_message *message,
+static void create_join_complete_reply_cb(struct l_dbus_message *msg,
void *user_data)
{
struct mesh_node *node = user_data;
- if (!message || l_dbus_message_is_error(message)) {
+ if (!msg || l_dbus_message_is_error(msg)) {
node_remove(node);
return;
}
@@ -716,7 +716,7 @@ static void create_node_ready_cb(void *user_data, int status,
l_dbus_message_set_arguments(msg, "t", l_get_be64(token));
dbus_send_with_timeout(dbus, msg, create_join_complete_reply_cb,
- node, DEFAULT_DBUS_TIMEOUT);
+ node, NULL, DEFAULT_DBUS_TIMEOUT);
l_dbus_message_unref(pending_msg);
}
--
2.26.2
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
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
This introduces the following behavior change for Leave() method:
If Leave method is called for a node that is being processed as a result
of a Create, Import, Join or Attach method calls in progress, node removal
is not allowed and org.bluez.mesh.Error.Busy error is returned.
---
doc/mesh-api.txt | 2 ++
mesh/mesh.c | 5 ++++-
mesh/node.c | 19 +++++++++++++++++--
mesh/node.h | 2 +-
4 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index e85f0bf52..b2eadc357 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -126,6 +126,8 @@ Methods:
PossibleErrors:
org.bluez.mesh.Error.InvalidArguments
+ org.bluez.mesh.Error.NotFound
+ org.bluez.mesh.Error.Busy
void CreateNetwork(object app_root, array{byte}[16] uuid)
diff --git a/mesh/mesh.c b/mesh/mesh.c
index a5935c216..0a933ffed 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -655,13 +655,16 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
void *user_data)
{
uint64_t token;
+ int result;
l_debug("Leave");
if (!l_dbus_message_get_arguments(msg, "t", &token))
return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
- node_remove(node_find_by_token(token));
+ result = node_remove(node_find_by_token(token));
+ if (result != MESH_ERROR_NONE)
+ return dbus_error(msg, result, NULL);
return l_dbus_message_new_method_return(msg);
}
diff --git a/mesh/node.c b/mesh/node.c
index 7ec06437b..a4c60ccb1 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -88,6 +88,7 @@ struct mesh_node {
char *storage_dir;
uint32_t disc_watch;
uint32_t seq_number;
+ bool busy;
bool provisioner;
uint16_t primary;
struct node_composition comp;
@@ -344,16 +345,21 @@ static void free_node_resources(void *data)
* This function is called to free resources and remove the
* configuration files for the specified node.
*/
-void node_remove(struct mesh_node *node)
+int node_remove(struct mesh_node *node)
{
if (!node)
- return;
+ return MESH_ERROR_NOT_FOUND;
+
+ if (node->busy)
+ return MESH_ERROR_BUSY;
l_queue_remove(nodes, node);
mesh_config_destroy_nvm(node->cfg);
free_node_resources(node);
+
+ return MESH_ERROR_NONE;
}
static bool add_models_from_storage(struct mesh_node *node,
@@ -1352,6 +1358,8 @@ static bool add_local_node(struct mesh_node *node, uint16_t unicast, bool kr,
/* Initialize configuration server model */
cfgmod_server_init(node, PRIMARY_ELE_IDX);
+ node->busy = true;
+
return true;
}
@@ -1459,6 +1467,9 @@ 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 (req->type == REQUEST_TYPE_ATTACH)
+ req->attach->busy = false;
+
if (!msg || l_dbus_message_is_error(msg)) {
l_error("Failed to get app's dbus objects");
goto fail;
@@ -1674,6 +1685,8 @@ void node_attach(const char *app_root, const char *sender, uint64_t token,
req->attach = node;
req->type = REQUEST_TYPE_ATTACH;
+ node->busy = true;
+
send_managed_objects_request(sender, app_root, req);
}
@@ -2347,6 +2360,8 @@ void node_finalize_new_node(struct mesh_node *node, struct mesh_io *io)
free_node_dbus_resources(node);
mesh_agent_remove(node->agent);
+ node->busy = false;
+
/* Register callback for the node's io */
attach_io(node, io);
}
diff --git a/mesh/node.h b/mesh/node.h
index e26d410c8..c201670b8 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -30,7 +30,7 @@ typedef void (*node_ready_func_t) (void *user_data, int status,
typedef void (*node_join_ready_func_t) (struct mesh_node *node,
struct mesh_agent *agent);
-void node_remove(struct mesh_node *node);
+int node_remove(struct mesh_node *node);
void node_join(const char *app_root, const char *sender, const uint8_t *uuid,
node_join_ready_func_t cb);
uint8_t *node_uuid_get(struct mesh_node *node);
--
2.26.2
Please disregard this patch. Needs more thought. Patches 1-3 still
okay.
On Mon, 2020-06-08 at 20:59 -0700, Inga Stotland wrote:
> This introduces the following behavior change for Leave() method:
>
> If Leave method is called for a node that is being processed as a result
> of a Create, Import, Join or Attach method calls in progress, node removal
> is not allowed and org.bluez.mesh.Error.Busy error is returned.
> ---
> doc/mesh-api.txt | 2 ++
> mesh/mesh.c | 5 ++++-
> mesh/node.c | 19 +++++++++++++++++--
> mesh/node.h | 2 +-
> 4 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index e85f0bf52..b2eadc357 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -126,6 +126,8 @@ Methods:
>
> PossibleErrors:
> org.bluez.mesh.Error.InvalidArguments
> + org.bluez.mesh.Error.NotFound
> + org.bluez.mesh.Error.Busy
>
> void CreateNetwork(object app_root, array{byte}[16] uuid)
>
> diff --git a/mesh/mesh.c b/mesh/mesh.c
> index a5935c216..0a933ffed 100644
> --- a/mesh/mesh.c
> +++ b/mesh/mesh.c
> @@ -655,13 +655,16 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
> void *user_data)
> {
> uint64_t token;
> + int result;
>
> l_debug("Leave");
>
> if (!l_dbus_message_get_arguments(msg, "t", &token))
> return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>
> - node_remove(node_find_by_token(token));
> + result = node_remove(node_find_by_token(token));
> + if (result != MESH_ERROR_NONE)
> + return dbus_error(msg, result, NULL);
>
> return l_dbus_message_new_method_return(msg);
> }
> diff --git a/mesh/node.c b/mesh/node.c
> index 7ec06437b..a4c60ccb1 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -88,6 +88,7 @@ struct mesh_node {
> char *storage_dir;
> uint32_t disc_watch;
> uint32_t seq_number;
> + bool busy;
> bool provisioner;
> uint16_t primary;
> struct node_composition comp;
> @@ -344,16 +345,21 @@ static void free_node_resources(void *data)
> * This function is called to free resources and remove the
> * configuration files for the specified node.
> */
> -void node_remove(struct mesh_node *node)
> +int node_remove(struct mesh_node *node)
> {
> if (!node)
> - return;
> + return MESH_ERROR_NOT_FOUND;
> +
> + if (node->busy)
> + return MESH_ERROR_BUSY;
>
> l_queue_remove(nodes, node);
>
> mesh_config_destroy_nvm(node->cfg);
>
> free_node_resources(node);
> +
> + return MESH_ERROR_NONE;
> }
>
> static bool add_models_from_storage(struct mesh_node *node,
> @@ -1352,6 +1358,8 @@ static bool add_local_node(struct mesh_node *node, uint16_t unicast, bool kr,
> /* Initialize configuration server model */
> cfgmod_server_init(node, PRIMARY_ELE_IDX);
>
> + node->busy = true;
> +
> return true;
> }
>
> @@ -1459,6 +1467,9 @@ 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 (req->type == REQUEST_TYPE_ATTACH)
> + req->attach->busy = false;
> +
> if (!msg || l_dbus_message_is_error(msg)) {
> l_error("Failed to get app's dbus objects");
> goto fail;
> @@ -1674,6 +1685,8 @@ void node_attach(const char *app_root, const char *sender, uint64_t token,
> req->attach = node;
> req->type = REQUEST_TYPE_ATTACH;
>
> + node->busy = true;
> +
> send_managed_objects_request(sender, app_root, req);
> }
>
> @@ -2347,6 +2360,8 @@ void node_finalize_new_node(struct mesh_node *node, struct mesh_io *io)
> free_node_dbus_resources(node);
> mesh_agent_remove(node->agent);
>
> + node->busy = false;
> +
> /* Register callback for the node's io */
> attach_io(node, io);
> }
> diff --git a/mesh/node.h b/mesh/node.h
> index e26d410c8..c201670b8 100644
> --- a/mesh/node.h
> +++ b/mesh/node.h
> @@ -30,7 +30,7 @@ typedef void (*node_ready_func_t) (void *user_data, int status,
> typedef void (*node_join_ready_func_t) (struct mesh_node *node,
> struct mesh_agent *agent);
>
> -void node_remove(struct mesh_node *node);
> +int node_remove(struct mesh_node *node);
> void node_join(const char *app_root, const char *sender, const uint8_t *uuid,
> node_join_ready_func_t cb);
> uint8_t *node_uuid_get(struct mesh_node *node);