* Re: [PATCH BlueZ] mesh: Fix io inititalization sequence
[not found] <20191114235210.3312-1-inga.stotland@intel.com>
@ 2019-11-15 23:23 ` Gix, Brian
2019-11-16 12:05 ` Aurelien Jarno
0 siblings, 1 reply; 2+ messages in thread
From: Gix, Brian @ 2019-11-15 23:23 UTC (permalink / raw)
To: Stotland, Inga; +Cc: linux-bluetooth@vger.kernel.org
Applied, Thanks
On Thu, 2019-11-14 at 15:52 -0800, Inga Stotland wrote:
> This introduces a chain of callbacks to indicate whether mesh io
> is initialized and mesh network is ready to use.
>
> This fixes the reported situation when the receive callbacks
> were setup before the HCI was fully initialized. In other words,
> BT_HCI_CMD_LE_SET_SCAN_PARAMETERS was called before BT_HCI_CMD_RESET
> and, as the result, the callback issueing BT_HCI_CMD_LE_SET_SCAN_ENABLE
> command was not called.
> ---
> mesh/main.c | 42 ++++++++++++++++++++++++------------
> mesh/mesh-io-api.h | 3 ++-
> mesh/mesh-io-generic.c | 48 +++++++++++++++++++++++++++++++-----------
> mesh/mesh-io.c | 5 +++--
> mesh/mesh-io.h | 6 +++++-
> mesh/mesh.c | 33 ++++++++++++++++++++++++-----
> mesh/mesh.h | 5 ++++-
> 7 files changed, 107 insertions(+), 35 deletions(-)
>
> diff --git a/mesh/main.c b/mesh/main.c
> index 6ea210c48..3c41acb75 100644
> --- a/mesh/main.c
> +++ b/mesh/main.c
> @@ -38,6 +38,9 @@
> #include "mesh/dbus.h"
> #include "mesh/mesh-io.h"
>
> +static const char *config_dir;
> +static int ctlr_index = MGMT_INDEX_NONE;
> +
> static const struct option main_options[] = {
> { "index", required_argument, NULL, 'i' },
> { "config", optional_argument, NULL, 'c' },
> @@ -69,16 +72,38 @@ static void do_debug(const char *str, void *user_data)
> l_info("%s%s", prefix, str);
> }
>
> +static void mesh_ready_callback(void *user_data, bool success)
> +{
> + struct l_dbus *dbus = user_data;
> +
> + if (!success) {
> + l_error("Failed to start mesh");
> + l_main_quit();
> + return;
> + }
> +
> + if (!dbus_init(dbus)) {
> + l_error("Failed to initialize mesh D-Bus resources");
> + l_main_quit();
> + }
> +}
> +
> static void request_name_callback(struct l_dbus *dbus, bool success,
> bool queued, void *user_data)
> {
> l_info("Request name %s",
> success ? "success": "failed");
>
> - if (success)
> - dbus_init(dbus);
> - else
> + if (!success) {
> l_main_quit();
> + return;
> + }
> +
> + if (!mesh_init(config_dir, MESH_IO_TYPE_GENERIC, &ctlr_index,
> + mesh_ready_callback, dbus)) {
> + l_error("Failed to initialize mesh");
> + l_main_quit();
> + }
> }
>
> static void ready_callback(void *user_data)
> @@ -88,7 +113,6 @@ static void ready_callback(void *user_data)
> l_info("D-Bus ready");
> l_dbus_name_acquire(dbus, BLUEZ_MESH_NAME, false, false, false,
> request_name_callback, NULL);
> -
> }
>
> static void disconnect_callback(void *user_data)
> @@ -114,8 +138,6 @@ int main(int argc, char *argv[])
> bool detached = true;
> bool dbus_debug = false;
> struct l_dbus *dbus = NULL;
> - const char *config_dir = NULL;
> - int index = MGMT_INDEX_NONE;
>
> if (!l_main_init())
> return -1;
> @@ -148,7 +170,7 @@ int main(int argc, char *argv[])
> goto done;
> }
>
> - index = atoi(str);
> + ctlr_index = atoi(str);
>
> break;
> case 'n':
> @@ -175,12 +197,6 @@ int main(int argc, char *argv[])
> }
>
>
> - if (!mesh_init(config_dir, MESH_IO_TYPE_GENERIC, &index)) {
> - l_error("Failed to initialize mesh");
> - status = EXIT_FAILURE;
> - goto done;
> - }
> -
> if (!detached)
> umask(0077);
>
> diff --git a/mesh/mesh-io-api.h b/mesh/mesh-io-api.h
> index 4cdf1f80a..75b881800 100644
> --- a/mesh/mesh-io-api.h
> +++ b/mesh/mesh-io-api.h
> @@ -19,7 +19,8 @@
>
> struct mesh_io_private;
>
> -typedef bool (*mesh_io_init_t)(struct mesh_io *io, void *opts);
> +typedef bool (*mesh_io_init_t)(struct mesh_io *io, void *opts,
> + mesh_io_ready_func_t cb, void *user_data);
> typedef bool (*mesh_io_destroy_t)(struct mesh_io *io);
> typedef bool (*mesh_io_caps_t)(struct mesh_io *io, struct mesh_io_caps *caps);
> typedef bool (*mesh_io_send_t)(struct mesh_io *io,
> diff --git a/mesh/mesh-io-generic.c b/mesh/mesh-io-generic.c
> index 42bf64a0b..b42fb4f1d 100644
> --- a/mesh/mesh-io-generic.c
> +++ b/mesh/mesh-io-generic.c
> @@ -37,14 +37,16 @@
> #include "mesh/mesh-io-generic.h"
>
> struct mesh_io_private {
> - uint16_t index;
> struct bt_hci *hci;
> + void *user_data;
> + mesh_io_ready_func_t ready_callback;
> struct l_timeout *tx_timeout;
> struct l_queue *rx_regs;
> struct l_queue *tx_pkts;
> + struct tx_pkt *tx;
> uint8_t filters[4];
> bool sending;
> - struct tx_pkt *tx;
> + uint16_t index;
> uint16_t interval;
> };
>
> @@ -283,22 +285,29 @@ static void configure_hci(struct mesh_io_private *io)
> sizeof(cmd), hci_generic_callback, NULL, NULL);
> }
>
> -static bool hci_init(struct mesh_io *io)
> +static void hci_init(void *user_data)
> {
> + struct mesh_io *io = user_data;
> + bool result = true;
> +
> io->pvt->hci = bt_hci_new_user_channel(io->pvt->index);
> if (!io->pvt->hci) {
> l_error("Failed to start mesh io (hci %u): %s", io->pvt->index,
> strerror(errno));
> - return false;
> + result = false;
> }
>
> - configure_hci(io->pvt);
> + if (result) {
> + configure_hci(io->pvt);
>
> - bt_hci_register(io->pvt->hci, BT_HCI_EVT_LE_META_EVENT,
> + bt_hci_register(io->pvt->hci, BT_HCI_EVT_LE_META_EVENT,
> event_callback, io, NULL);
>
> - l_debug("Started mesh on hci %u", io->pvt->index);
> - return true;
> + l_debug("Started mesh on hci %u", io->pvt->index);
> + }
> +
> + if (io->pvt->ready_callback)
> + io->pvt->ready_callback(io->pvt->user_data, result);
> }
>
> static void read_info(int index, void *user_data)
> @@ -315,7 +324,8 @@ static void read_info(int index, void *user_data)
> hci_init(io);
> }
>
> -static bool dev_init(struct mesh_io *io, void *opts)
> +static bool dev_init(struct mesh_io *io, void *opts,
> + mesh_io_ready_func_t cb, void *user_data)
> {
> if (!io || io->pvt)
> return false;
> @@ -326,10 +336,15 @@ static bool dev_init(struct mesh_io *io, void *opts)
> io->pvt->rx_regs = l_queue_new();
> io->pvt->tx_pkts = l_queue_new();
>
> + io->pvt->ready_callback = cb;
> + io->pvt->user_data = user_data;
> +
> if (io->pvt->index == MGMT_INDEX_NONE)
> return mesh_mgmt_list(read_info, io);
> - else
> - return hci_init(io);
> +
> + l_idle_oneshot(hci_init, io, NULL);
> +
> + return true;
> }
>
> static bool dev_destroy(struct mesh_io *io)
> @@ -696,6 +711,15 @@ static bool find_by_filter_id(const void *a, const void *b)
> return rx_reg->filter_id == filter_id;
> }
>
> +static void scan_enable_rsp(const void *buf, uint8_t size,
> + void *user_data)
> +{
> + uint8_t status = *((uint8_t *) buf);
> +
> + if (status)
> + l_error("LE Scan enable failed (0x%02x)", status);
> +}
> +
> static void set_recv_scan_enable(const void *buf, uint8_t size,
> void *user_data)
> {
> @@ -705,7 +729,7 @@ static void set_recv_scan_enable(const void *buf, uint8_t size,
> cmd.enable = 0x01; /* Enable scanning */
> cmd.filter_dup = 0x00; /* Report duplicates */
> bt_hci_send(pvt->hci, BT_HCI_CMD_LE_SET_SCAN_ENABLE,
> - &cmd, sizeof(cmd), NULL, NULL, NULL);
> + &cmd, sizeof(cmd), scan_enable_rsp, pvt, NULL);
> }
>
> static bool recv_register(struct mesh_io *io, uint8_t filter_id,
> diff --git a/mesh/mesh-io.c b/mesh/mesh-io.c
> index 94a92e885..95a99b6a5 100644
> --- a/mesh/mesh-io.c
> +++ b/mesh/mesh-io.c
> @@ -52,7 +52,8 @@ static bool match_by_type(const void *a, const void *b)
> return io->type == type;
> }
>
> -struct mesh_io *mesh_io_new(enum mesh_io_type type, void *opts)
> +struct mesh_io *mesh_io_new(enum mesh_io_type type, void *opts,
> + mesh_io_ready_func_t cb, void *user_data)
> {
> const struct mesh_io_api *api = NULL;
> struct mesh_io *io;
> @@ -78,7 +79,7 @@ struct mesh_io *mesh_io_new(enum mesh_io_type type, void *opts)
> io->type = type;
>
> io->api = api;
> - if (!api->init(io, opts))
> + if (!api->init(io, opts, cb, user_data))
> goto fail;
>
> if (!io_list)
> diff --git a/mesh/mesh-io.h b/mesh/mesh-io.h
> index 1c10779aa..45ff00a3c 100644
> --- a/mesh/mesh-io.h
> +++ b/mesh/mesh-io.h
> @@ -81,7 +81,11 @@ typedef void (*mesh_io_recv_func_t)(void *user_data,
> typedef void (*mesh_io_status_func_t)(void *user_data, int status,
> uint8_t filter_id);
>
> -struct mesh_io *mesh_io_new(enum mesh_io_type type, void *opts);
> +typedef void (*mesh_io_ready_func_t)(void *user_data, bool result);
> +
> +
> +struct mesh_io *mesh_io_new(enum mesh_io_type type, void *opts,
> + mesh_io_ready_func_t cb, void *user_data);
> void mesh_io_destroy(struct mesh_io *io);
>
> bool mesh_io_get_caps(struct mesh_io *io, struct mesh_io_caps *caps);
> diff --git a/mesh/mesh.c b/mesh/mesh.c
> index 9b2b2073b..55204da56 100644
> --- a/mesh/mesh.c
> +++ b/mesh/mesh.c
> @@ -70,6 +70,11 @@ struct join_data{
> uint8_t *uuid;
> };
>
> +struct mesh_init_request {
> + mesh_ready_func_t cb;
> + void *user_data;
> +};
> +
> static struct bt_mesh mesh;
>
> /* We allow only one outstanding Join request */
> @@ -138,9 +143,23 @@ void mesh_unreg_prov_rx(prov_rx_cb_t cb)
> mesh_io_deregister_recv_cb(mesh.io, MESH_IO_FILTER_PROV);
> }
>
> -bool mesh_init(const char *config_dir, enum mesh_io_type type, void *opts)
> +static void io_ready_callback(void *user_data, bool result)
> +{
> + struct mesh_init_request *req = user_data;
> +
> + if (result)
> + node_attach_io_all(mesh.io);
> +
> + req->cb(req->user_data, result);
> +
> + l_free(req);
> +}
> +
> +bool mesh_init(const char *config_dir, enum mesh_io_type type, void *opts,
> + mesh_ready_func_t cb, void *user_data)
> {
> struct mesh_io_caps caps;
> + struct mesh_init_request *req;
>
> if (mesh.io)
> return true;
> @@ -159,16 +178,20 @@ bool mesh_init(const char *config_dir, enum mesh_io_type type, void *opts)
> if (!node_load_from_storage(storage_dir))
> return false;
>
> - mesh.io = mesh_io_new(type, opts);
> - if (!mesh.io)
> + req = l_new(struct mesh_init_request, 1);
> + req->cb = cb;
> + req->user_data = user_data;
> +
> + mesh.io = mesh_io_new(type, opts, io_ready_callback, req);
> + if (!mesh.io) {
> + l_free(req);
> return false;
> + }
>
> l_debug("io %p", mesh.io);
> mesh_io_get_caps(mesh.io, &caps);
> mesh.max_filters = caps.max_num_filters;
>
> - node_attach_io_all(mesh.io);
> -
> return true;
> }
>
> diff --git a/mesh/mesh.h b/mesh/mesh.h
> index e0a3e1b96..c72632b15 100644
> --- a/mesh/mesh.h
> +++ b/mesh/mesh.h
> @@ -30,9 +30,12 @@
>
> enum mesh_io_type;
>
> +typedef void (*mesh_ready_func_t)(void *user_data, bool success);
> typedef void (*prov_rx_cb_t)(void *user_data, const uint8_t *data,
> uint16_t len);
> -bool mesh_init(const char *in_config_name, enum mesh_io_type type, void *opts);
> +
> +bool mesh_init(const char *in_config_name, enum mesh_io_type type, void *opts,
> + mesh_ready_func_t cb, void *user_data);
> void mesh_cleanup(void);
> bool mesh_dbus_init(struct l_dbus *dbus);
>
^ permalink raw reply [flat|nested] 2+ messages in thread