From 6dfc9ecc032e494a3a9c7ea8b3ae9832098a1851 Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Tue, 17 Jan 2023 10:59:35 +0100 Subject: [PATCH] Bluetooth: Mesh: Invalidate pending entries before calling settings API If, while storing app or net key, or changing cdb, we receive another mesh message that affects setting of the same key that the mesh currently stores, the new change won't be stored. The settings subsystem API has rescheduling point inside. The mesh settings are stored in the system workqueue and by default mesh messages are processed from BT RX thread which has higher priority than the system workqueue. When the case above happens, a new change will be written to the same cache entry. But this cache entry will be invalidated after leaving settings API, which in turns will invalidate the new change: - Receive Config AppKey Add message cfg_srv.c -> app_keys.c: bt_mesh_app_key_add() app_keys.c: update_app_key_settings() app_keys.c: bt_mesh_settings_store_schedule() - store_pending() in settings.c is scheduled settings.c -> app_keys.c: bt_mesh_app_key_pending_store() app_keys.c: store_app_key() called to store new app key app_keys.c: settings_save_one() calls flash driver and sleeps - Receive Config AppKey Delete message while in sleep, which wakes up BT RX thread before returning to the system workqueue cfg_srv.c -> app_keys.c: bt_mesh_app_key_del() app_keys.c: update_app_key_settings() app_keys.c: app_key_update_find() finds entry and returns it app_keys.c: update->clear = 1 app_keys.c: bt_mesh_settings_store_schedule() - returning back to bt_mesh_app_key_pending_store() from settings_save_one() app_keys.c: update->valid = 0 - the key won't be deleted next time store_pending() is scheduled. This change moves entry invalidation before calling settings API so that after returning from settings API, a new change won't be unintentionally invalidated. Signed-off-by: Pavel Vasilyev --- subsys/bluetooth/mesh/app_keys.c | 4 ++-- subsys/bluetooth/mesh/cdb.c | 18 ++++++++++-------- subsys/bluetooth/mesh/subnet.c | 4 ++-- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/subsys/bluetooth/mesh/app_keys.c b/subsys/bluetooth/mesh/app_keys.c index fd644be7c0a..cf5451fc837 100644 --- a/subsys/bluetooth/mesh/app_keys.c +++ b/subsys/bluetooth/mesh/app_keys.c @@ -665,12 +665,12 @@ void bt_mesh_app_key_pending_store(void) continue; } + update->valid = 0U; + if (update->clear) { clear_app_key(update->key_idx); } else { store_app_key(update->key_idx); } - - update->valid = 0U; } } diff --git a/subsys/bluetooth/mesh/cdb.c b/subsys/bluetooth/mesh/cdb.c index 89c5fe3d53b..58c9d70abc2 100644 --- a/subsys/bluetooth/mesh/cdb.c +++ b/subsys/bluetooth/mesh/cdb.c @@ -1023,27 +1023,29 @@ static void store_cdb_pending_nodes(void) for (i = 0; i < ARRAY_SIZE(cdb_node_updates); ++i) { struct node_update *update = &cdb_node_updates[i]; + uint16_t addr; if (update->addr == BT_MESH_ADDR_UNASSIGNED) { continue; } - LOG_DBG("addr: 0x%04x, clear: %d", update->addr, update->clear); + addr = update->addr; + update->addr = BT_MESH_ADDR_UNASSIGNED; + + LOG_DBG("addr: 0x%04x, clear: %d", addr, update->clear); if (update->clear) { - clear_cdb_node(update->addr); + clear_cdb_node(addr); } else { struct bt_mesh_cdb_node *node; - node = bt_mesh_cdb_node_get(update->addr); + node = bt_mesh_cdb_node_get(addr); if (node) { store_cdb_node(node); } else { - LOG_WRN("Node 0x%04x not found", update->addr); + LOG_WRN("Node 0x%04x not found", addr); } } - - update->addr = BT_MESH_ADDR_UNASSIGNED; } } @@ -1058,6 +1060,8 @@ static void store_cdb_pending_keys(void) continue; } + update->valid = 0U; + if (update->clear) { if (update->app_key) { clear_cdb_app_key(update->key_idx); @@ -1085,8 +1089,6 @@ static void store_cdb_pending_keys(void) } } } - - update->valid = 0U; } } diff --git a/subsys/bluetooth/mesh/subnet.c b/subsys/bluetooth/mesh/subnet.c index ebf0ee07510..f3b05ecbf82 100644 --- a/subsys/bluetooth/mesh/subnet.c +++ b/subsys/bluetooth/mesh/subnet.c @@ -831,12 +831,12 @@ void bt_mesh_subnet_pending_store(void) continue; } + update->valid = 0U; + if (update->clear) { clear_net_key(update->key_idx); } else { store_subnet(update->key_idx); } - - update->valid = 0U; } }