[nrf fromtree] Bluetooth: Mesh: Decouple RPL store timeout from storage mode

Solve two issues:

  * Other storage modes than CONFIG_BT_MESH_RPL_STORAGE_MODE_SETTINGS
    could not reuse the pending store scheduler used by the settings
    backend.
  * The store timing of SRPL, which is always stored through the
    Settings subsystem, would use CONFIG_BT_MESH_RPL_STORE_TIMEOUT if
    RPL was also stored in settings via
    CONFIG_BT_MESH_RPL_STORAGE_MODE_SETTINGS, but not if another storage
    mode was used for RPL.

Decouple the store scheduler from the Settings subsystem and the
chosen RPL storage mode, so the scheduler code and the separate
CONFIG_BT_MESH_RPL_STORE_TIMEOUT option are

a) Available regardless of storage mode, and always used for SRPL when
set and
b) Available without CONFIG_BT_SETTINGS, in case an RPL storage mode
does not need Settings but would like the scheduling.

Also change the BT_MESH_SETTINGS_RPL_RESET_PENDING to always store
immediately. This was in practice already the case whenever settings was
used, since this was only ever scheduled right before a second NO_WAIT
configuration was scheduled (like net key, IV, etc.). Now this behavior
is consistent both with and without BT_SETTINGS enabled.

Assisted-by: Copilot:claude-sonnet-5.5
Signed-off-by: Ludvig Jordet <ludvig.jordet@nordicsemi.no>

(cherry picked from commit 3f878df514ad9ebe63d73b36a0ea50fa557517d7)

Author: ludvigsj

doc/services/connectivity/bluetooth/api/mesh/core.rst

@@ -80,20 +80,20 @@ is defined by the :kconfig:option:`CONFIG_BT_MESH_STORE_TIMEOUT` option. Once
 storing of data is scheduled, it can not be rescheduled until the work item is
 processed. Exceptions are made in certain cases as described below.
 
-When IV index, Sequence Number or CDB configuration have to be stored, the work
-item is submitted to the workqueue without the delay. If the work item was
-previously scheduled, it will be rescheduled without the delay.
-
-The Replay Protection List uses the same work item to store RPL entries. If
-storing of RPL entries is requested and no other configuration is pending to be
-stored, the delay is set to :kconfig:option:`CONFIG_BT_MESH_RPL_STORE_TIMEOUT`.
-If other stack configuration has to be stored, the delay defined by
-the :kconfig:option:`CONFIG_BT_MESH_STORE_TIMEOUT` option is less than
-:kconfig:option:`CONFIG_BT_MESH_RPL_STORE_TIMEOUT`, and the work item was
-scheduled by the Replay Protection List, the work item will be rescheduled.
-
-When the work item is running, the stack will store all pending configuration,
-including the RPL entries.
+When IV index, Sequence Number, CDB configuration or a Replay Protection List
+reset have to be stored, the work item is submitted to the workqueue without
+the delay. If the work item was previously scheduled, it will be rescheduled
+without the delay.
+
+The Replay Protection List and the Solicitation Replay Protection List are
+stored by a separate work item, scheduled with the delay defined by
+:kconfig:option:`CONFIG_BT_MESH_RPL_STORE_TIMEOUT`. Pending stores of other
+kinds do not shorten this delay. If the option is set to ``-1``, the replay
+protection lists are stored together with the rest of the stack
+configuration.
+
+When a work item is running, the stack will store all pending configuration
+that the work item is responsible for.
 
 Work item execution context
 ===========================

subsys/bluetooth/mesh/CMakeLists.txt

@@ -26,7 +26,7 @@ zephyr_library_sources_ifdef(CONFIG_BT_MESH_ADV_LEGACY adv_legacy.c)
 
 zephyr_library_sources_ifdef(CONFIG_BT_MESH_ADV_EXT adv_ext.c)
 
-zephyr_library_sources_ifdef(CONFIG_BT_SETTINGS settings.c)
+zephyr_library_sources_ifdef(CONFIG_BT_MESH_PENDING_STORE settings.c)
 
 zephyr_library_sources_ifdef(CONFIG_BT_MESH_RPL_STORAGE_MODE_SETTINGS rpl.c)
 

subsys/bluetooth/mesh/Kconfig

@@ -1868,8 +1868,16 @@ config BT_MESH_LABEL_COUNT
 
 endmenu # Capabilities
 
+config BT_MESH_PENDING_STORE
+	bool
+	default y if BT_SETTINGS
+	help
+	  Scheduler for deferred storing of pending mesh data. Enabled with the
+	  settings subsystem, and selected by storage backends that schedule
+	  their stores through it without the settings subsystem.
+
 menu "Persistent storage"
-	visible if BT_SETTINGS
+	visible if BT_MESH_PENDING_STORE
 
 config BT_MESH_STORE_TIMEOUT
 	int "Delay (in seconds) before storing anything persistently"
@@ -1897,33 +1905,32 @@ config BT_MESH_SEQ_STORE_RATE
 	  off with a value that's guaranteed to be larger than the last
 	  one used before power off.
 
-if BT_MESH_RPL_STORAGE_MODE_SETTINGS && BT_SETTINGS
-
 config BT_MESH_RPL_STORE_TIMEOUT
-	int "Minimum interval after which unsaved RPL and SRPL entries are updated in the settings subsystem"
+	int "Minimum interval after which unsaved RPL and SRPL entries are stored"
+	depends on BT_MESH_PENDING_STORE
 	range -1 1000000
 	default 5
 	help
 	  This value defines time in seconds until unsaved RPL and SRPL entries
-	  are written to the persistent storage. Setting this value
+	  are written to the persistent storage. The SRPL is always stored
+	  through the settings subsystem, whereas the RPL is only affected if
+	  its storage mode schedules stores with this timeout. Setting this value
 	  to a large number may lead to security vulnerabilities if a node
 	  gets powered off before the timer is fired. When flash is used
 	  as the persistent storage setting this value to a low number
 	  may wear out flash sooner or later. However, if the RPL gets
 	  updated infrequently a value as low as 0 (write immediately)
 	  may make sense. Setting this value to -1 will disable this timer.
-	  In this case, a user is responsible to store pending RPL entries
-	  using @ref bt_mesh_rpl_pending_store. In the mean time, when
-	  IV Index is updated, the outdated RPL entries will still be
-	  stored by @ref BT_MESH_STORE_TIMEOUT. Finding the right balance
+	  With the settings storage mode, a user is then responsible to store
+	  pending RPL entries using @ref bt_mesh_rpl_pending_store. In the
+	  mean time, the replay list invalidated by an IV Index update or
+	  recovery is still stored immediately. Finding the right balance
 	  between this timeout and calling @ref bt_mesh_rpl_pending_store
 	  may reduce a risk of security vulnerability and flash wear out.
 	  Failure to store the RPL and becoming vulnerable after reboot
 	  will cause the device to not perform the replay protection
 	  required by the spec.
 
-endif # BT_MESH_RPL_STORAGE_MODE_SETTINGS && BT_SETTINGS
-
 config BT_MESH_SETTINGS_WORKQ
 	bool "Store the Bluetooth Mesh settings in a separate work queue"
 	default y

subsys/bluetooth/mesh/main.c

@@ -412,7 +412,7 @@ void bt_mesh_reset(void)
 		bt_mesh_sol_reset();
 	}
 
-	if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
+	if (IS_ENABLED(CONFIG_BT_MESH_PENDING_STORE)) {
 		bt_mesh_settings_store_pending();
 	}
 
@@ -612,7 +612,7 @@ int bt_mesh_init(const struct bt_mesh_prov *prov,
 	bt_mesh_beacon_init();
 	bt_mesh_adv_init();
 
-	if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
+	if (IS_ENABLED(CONFIG_BT_MESH_PENDING_STORE)) {
 		bt_mesh_settings_init();
 	}
 

subsys/bluetooth/mesh/rpl.h

@@ -26,4 +26,10 @@ bool bt_mesh_rpl_check(struct bt_mesh_net_rx *rx, struct bt_mesh_rpl **match, bo
 void bt_mesh_rpl_clear(void);
 void bt_mesh_rpl_update(struct bt_mesh_rpl *rpl,
 			struct bt_mesh_net_rx *rx);
+/*
+ * A storage backend using the pending store scheduler schedules
+ * BT_MESH_SETTINGS_RPL_PENDING for entry updates and
+ * BT_MESH_SETTINGS_RPL_RESET_PENDING when an IV Index update or recovery
+ * invalidates the list. Both run this function, which must cancel both flags.
+ */
 void bt_mesh_rpl_pending_store_all_nodes(void);

subsys/bluetooth/mesh/settings.c

@@ -59,8 +59,7 @@ LOG_MODULE_REGISTER(bt_mesh_settings);
 /* When true, the replay lists are stored on a deadline of their own. Otherwise
  * they are stored together with the generic pending flags.
  */
-#define RPL_OWN_TIMEOUT (IS_ENABLED(CONFIG_BT_MESH_RPL_STORAGE_MODE_SETTINGS) &&                   \
-			 RPL_STORE_TIMEOUT >= 0)
+#define RPL_OWN_TIMEOUT (RPL_STORE_TIMEOUT >= 0)
 
 static struct k_work_q settings_work_q;
 static K_THREAD_STACK_DEFINE(settings_work_stack, SETTINGS_WORKQ_STACK_SIZE);
@@ -90,6 +89,7 @@ int bt_mesh_settings_set(settings_read_cb read_cb, void *cb_arg,
 	return 0;
 }
 
+#if defined(CONFIG_BT_SETTINGS)
 static int mesh_commit(void)
 {
 	if (!atomic_test_bit(bt_mesh.flags, BT_MESH_INIT)) {
@@ -123,12 +123,14 @@ static int mesh_commit(void)
 
 SETTINGS_STATIC_HANDLER_DEFINE_WITH_CPRIO(bt_mesh, "bt/mesh", NULL, NULL, mesh_commit, NULL,
 					  BT_SETTINGS_CPRIO_2);
+#endif /* CONFIG_BT_SETTINGS */
 
 /* Pending flags that use K_NO_WAIT as the storage timeout */
 #define NO_WAIT_PENDING_BITS (BIT(BT_MESH_SETTINGS_NET_PENDING) |           \
 			      BIT(BT_MESH_SETTINGS_IV_PENDING)  |           \
 			      BIT(BT_MESH_SETTINGS_SEQ_PENDING) |           \
-			      BIT(BT_MESH_SETTINGS_CDB_PENDING))
+			      BIT(BT_MESH_SETTINGS_CDB_PENDING) |           \
+			      BIT(BT_MESH_SETTINGS_RPL_RESET_PENDING))
 
 /* Pending flags that use CONFIG_BT_MESH_RPL_STORE_TIMEOUT. Both replay lists
  * share the deadline, and each is stored only if its own flag is set.
@@ -183,6 +185,9 @@ void bt_mesh_settings_store_cancel(enum bt_mesh_settings_flag flag)
 	atomic_clear_bit(pending_flags, flag);
 }
 
+/* Data other than the replay protection list is only stored through the settings subsystem. */
+#define SETTINGS_HANDLER(_func) (IS_ENABLED(CONFIG_BT_SETTINGS) ? (_func) : NULL)
+
 static void store_pending_flags(uint32_t mask)
 {
 	LOG_DBG("");
@@ -192,42 +197,52 @@ static void store_pending_flags(uint32_t mask)
 	} handlers[BT_MESH_SETTINGS_FLAG_COUNT] = {
 		[BT_MESH_SETTINGS_RPL_PENDING]      = { bt_mesh_rpl_pending_store_all_nodes },
 		[BT_MESH_SETTINGS_RPL_RESET_PENDING] = { bt_mesh_rpl_pending_store_all_nodes },
-		[BT_MESH_SETTINGS_NET_KEYS_PENDING] = { bt_mesh_subnet_pending_store },
-		[BT_MESH_SETTINGS_APP_KEYS_PENDING] = { bt_mesh_app_key_pending_store },
-		[BT_MESH_SETTINGS_NET_PENDING]	    = { bt_mesh_net_pending_net_store },
-		[BT_MESH_SETTINGS_IV_PENDING]	    = { bt_mesh_net_pending_iv_store },
-		[BT_MESH_SETTINGS_SEQ_PENDING]	    = { bt_mesh_net_pending_seq_store },
+		[BT_MESH_SETTINGS_NET_KEYS_PENDING] = {
+			SETTINGS_HANDLER(bt_mesh_subnet_pending_store) },
+		[BT_MESH_SETTINGS_APP_KEYS_PENDING] = {
+			SETTINGS_HANDLER(bt_mesh_app_key_pending_store) },
+		[BT_MESH_SETTINGS_NET_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_net_pending_net_store) },
+		[BT_MESH_SETTINGS_IV_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_net_pending_iv_store) },
+		[BT_MESH_SETTINGS_SEQ_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_net_pending_seq_store) },
 		[BT_MESH_SETTINGS_DEV_KEY_CAND_PENDING] = {
-			bt_mesh_net_pending_dev_key_cand_store },
-		[BT_MESH_SETTINGS_HB_PUB_PENDING]   = { bt_mesh_hb_pub_pending_store },
-		[BT_MESH_SETTINGS_CFG_PENDING]	    = { bt_mesh_cfg_pending_store },
-		[BT_MESH_SETTINGS_COMP_PENDING]	    = { bt_mesh_comp_data_pending_clear },
-		[BT_MESH_SETTINGS_MOD_PENDING]	    = { bt_mesh_model_pending_store },
-		[BT_MESH_SETTINGS_VA_PENDING]	    = { bt_mesh_va_pending_store },
+			SETTINGS_HANDLER(bt_mesh_net_pending_dev_key_cand_store) },
+		[BT_MESH_SETTINGS_HB_PUB_PENDING]   = {
+			SETTINGS_HANDLER(bt_mesh_hb_pub_pending_store) },
+		[BT_MESH_SETTINGS_CFG_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_cfg_pending_store) },
+		[BT_MESH_SETTINGS_COMP_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_comp_data_pending_clear) },
+		[BT_MESH_SETTINGS_MOD_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_model_pending_store) },
+		[BT_MESH_SETTINGS_VA_PENDING]	    = {
+			SETTINGS_HANDLER(bt_mesh_va_pending_store) },
 		[BT_MESH_SETTINGS_CDB_PENDING]	    = {
 			IS_ENABLED(CONFIG_BT_MESH_CDB) ?
-				bt_mesh_cdb_pending_store : NULL },
+				SETTINGS_HANDLER(bt_mesh_cdb_pending_store) : NULL },
 		[BT_MESH_SETTINGS_SRPL_PENDING]     = {
 			IS_ENABLED(CONFIG_BT_MESH_OD_PRIV_PROXY_SRV) ?
-				bt_mesh_srpl_pending_store : NULL },
+				SETTINGS_HANDLER(bt_mesh_srpl_pending_store) : NULL },
 		[BT_MESH_SETTINGS_SSEQ_PENDING]     = {
 			IS_ENABLED(CONFIG_BT_MESH_PROXY_SOLICITATION) ?
-				bt_mesh_sseq_pending_store : NULL },
+				SETTINGS_HANDLER(bt_mesh_sseq_pending_store) : NULL },
 		[BT_MESH_SETTINGS_BRG_PENDING]      = {
 			IS_ENABLED(CONFIG_BT_MESH_BRG_CFG_SRV) ?
-				bt_mesh_brg_cfg_pending_store : NULL },
+				SETTINGS_HANDLER(bt_mesh_brg_cfg_pending_store) : NULL },
 	};
 
 	for (int i = 0; i < ARRAY_SIZE(handlers); i++) {
-		if (handlers[i].handler == NULL) {
+		if ((mask & BIT(i)) == 0) {
 			continue;
 		}
 
-		if ((mask & BIT(i)) == 0) {
+		if (!atomic_test_and_clear_bit(pending_flags, i)) {
 			continue;
 		}
 
-		if (atomic_test_and_clear_bit(pending_flags, i)) {
+		if (handlers[i].handler != NULL) {
 			handlers[i].handler();
 		}
 	}

subsys/bluetooth/mesh/settings.h

@@ -8,8 +8,7 @@
 enum bt_mesh_settings_flag {
 	BT_MESH_SETTINGS_RPL_PENDING,
 	/* Replay list invalidated by an IV Index update, or IV index recovery.
-	 * Stored on CONFIG_BT_MESH_STORE_TIMEOUT rather than the replay list
-	 * timeout.
+	 * Stored immediately rather than on the replay list timeout.
 	 */
 	BT_MESH_SETTINGS_RPL_RESET_PENDING,
 	BT_MESH_SETTINGS_NET_KEYS_PENDING,

tests/bluetooth/mesh/settings/CMakeLists.txt

@@ -33,6 +33,9 @@ target_compile_options(app
   -DCONFIG_BT_MESH_OD_PRIV_PROXY_SRV=1
   -DCONFIG_BT_MESH_PROXY_SOLICITATION=1
   -DCONFIG_BT_MESH_BRG_CFG_SRV=1
-  -DCONFIG_BT_SETTINGS=1
   -DCONFIG_PSA_CRYPTO_PROVIDER_MBEDTLS
 )
+
+if(NOT TEST_WITHOUT_BT_SETTINGS)
+  target_compile_options(app PRIVATE -DCONFIG_BT_SETTINGS=1)
+endif()