Bluetooth: Mesh: Rework publication timer

Periodic publication would previously build and send the first
publication inside the bt_mesh_model_pub() function, before cancelling
and rescheduling the next publication. The timer handler would only
handle retransmissions, and would abandon the rest of the publication
event if one of the packets failed to send.

This design has three issues:
- If the initial timer cancel fails, the publication would interfer with
  the periodic publication management, which might skip an event or
  send too many packets.
- If any of the messages fail to publish, the full publication event
  would be abandoned. This is not predictable or expected from the API.
- bt_mesh_model_pub() required 384 bytes of stack to build the message,
  which has to be factored into all calling threads.

This patch moves all transmission into the publication timer by
replacing k_work_cancel with a single k_work_reschedule(K_NO_WAIT). It
also changes the error recovery behavior to attempt to finish the full
publication event even if some of the transmissions fail.

Split out from #33782.

Signed-off-by: Trond Einar Snekvik <[email protected]>

Author: trond-snekvik

include/bluetooth/mesh/access.h

@@ -340,17 +340,17 @@ struct bt_mesh_model_pub {
 	/** The model the context belongs to. Initialized by the stack. */
 	struct bt_mesh_model *mod;
 
-	uint16_t addr;         /**< Publish Address. */
-	uint16_t key:12,       /**< Publish AppKey Index. */
-	      cred:1,       /**< Friendship Credentials Flag. */
-	      send_rel:1;   /**< Force reliable sending (segment acks) */
+	uint16_t addr;          /**< Publish Address. */
+	uint16_t key:12,        /**< Publish AppKey Index. */
+		 cred:1,        /**< Friendship Credentials Flag. */
+		 send_rel:1,    /**< Force reliable sending (segment acks) */
+		 fast_period:1; /**< Use FastPeriodDivisor */
 
 	uint8_t  ttl;          /**< Publish Time to Live. */
 	uint8_t  retransmit;   /**< Retransmit Count & Interval Steps. */
 	uint8_t  period;       /**< Publish Period. */
 	uint8_t  period_div:4, /**< Divisor for the Period. */
-	      fast_period:1,/**< Use FastPeriodDivisor */
-	      count:3;      /**< Retransmissions left. */
+		 count:4;      /**< Transmissions left. */
 
 	uint32_t period_start; /**< Start of the current period. */
 
@@ -381,7 +381,7 @@ struct bt_mesh_model_pub {
 	int (*update)(struct bt_mesh_model *mod);
 
 	/** Publish Period Timer. Only for stack-internal use. */
-	struct k_delayed_work timer;
+	struct k_work_delayable timer;
 };
 
 /** @def BT_MESH_MODEL_PUB_DEFINE

subsys/bluetooth/mesh/access.c

@@ -150,7 +150,10 @@ static void publish_sent(int err, void *user_data)
 
 	if (delay) {
 		BT_DBG("Publishing next time in %dms", delay);
-		k_delayed_work_submit(&mod->pub->timer, K_MSEC(delay));
+		/* Using schedule() in case the application has already called
+		 * bt_mesh_publish, and a publication is pending.
+		 */
+		k_work_schedule(&mod->pub->timer, K_MSEC(delay));
 	}
 }
 
@@ -161,6 +164,7 @@ static void publish_start(uint16_t duration, int err, void *user_data)
 
 	if (err) {
 		BT_ERR("Failed to publish: err %d", err);
+		publish_sent(err, user_data);
 		return;
 	}
 
@@ -175,7 +179,7 @@ static const struct bt_mesh_send_cb pub_sent_cb = {
 	.end = publish_sent,
 };
 
-static int publish_retransmit(struct bt_mesh_model *mod)
+static int publish_transmit(struct bt_mesh_model *mod)
 {
 	NET_BUF_SIMPLE_DEFINE(sdu, BT_MESH_TX_SDU_MAX);
 	struct bt_mesh_model_pub *pub = mod->pub;
@@ -192,67 +196,68 @@ static int publish_retransmit(struct bt_mesh_model *mod)
 
 	net_buf_simple_add_mem(&sdu, pub->msg->data, pub->msg->len);
 
-	pub->count--;
-
 	return bt_mesh_trans_send(&tx, &sdu, &pub_sent_cb, mod);
 }
 
-static void publish_retransmit_end(int err, struct bt_mesh_model_pub *pub)
+static int pub_period_start(struct bt_mesh_model_pub *pub)
 {
-	/* Cancel all retransmits for this publish attempt */
-	pub->count = 0U;
-	/* Make sure the publish timer gets reset */
-	publish_sent(err, pub->mod);
+	int err;
+
+	pub->count = BT_MESH_PUB_TRANSMIT_COUNT(pub->retransmit);
+
+	if (!pub->update) {
+		return 0;
+	}
+
+	err = pub->update(pub->mod);
+	if (err) {
+		/* Skip this publish attempt. */
+		BT_DBG("Update failed, skipping publish (err: %d)", err);
+		pub->count = 0;
+		pub->period_start = k_uptime_get_32();
+		publish_sent(err, pub);
+		return err;
+	}
+
+	return 0;
 }
 
 static void mod_publish(struct k_work *work)
 {
-	struct bt_mesh_model_pub *pub = CONTAINER_OF(work,
+	struct k_work_delayable *dwork = k_work_delayable_from_work(work);
+	struct bt_mesh_model_pub *pub = CONTAINER_OF(dwork,
 						     struct bt_mesh_model_pub,
-						     timer.work);
-	int32_t period_ms;
+						     timer);
 	int err;
 
-	BT_DBG("");
+	if (pub->addr == BT_MESH_ADDR_UNASSIGNED ||
+	    atomic_test_bit(bt_mesh.flags, BT_MESH_SUSPENDED)) {
+		/* Publication is no longer active, but the cancellation of the
+		 * delayed work failed. Abandon recurring timer.
+		 */
+		return;
+	}
 
-	period_ms = bt_mesh_model_pub_period_get(pub->mod);
-	BT_DBG("period %u ms", period_ms);
+	BT_DBG("");
 
 	if (pub->count) {
-		err = publish_retransmit(pub->mod);
+		pub->count--;
+	} else {
+		/* First publication in this period */
+		err = pub_period_start(pub);
 		if (err) {
-			BT_ERR("Failed to retransmit (err %d)", err);
-
-			pub->count = 0U;
-
-			/* Continue with normal publication */
-			if (period_ms) {
-				k_delayed_work_submit(&pub->timer,
-						      K_MSEC(period_ms));
-			}
+			return;
 		}
-
-		return;
 	}
 
-	if (!period_ms) {
-		return;
-	}
-
-	__ASSERT_NO_MSG(pub->update != NULL);
-
-	err = pub->update(pub->mod);
+	err = publish_transmit(pub->mod);
 	if (err) {
-		/* Cancel this publish attempt. */
-		BT_DBG("Update failed, skipping publish (err: %d)", err);
-		pub->period_start = k_uptime_get_32();
-		publish_retransmit_end(err, pub);
-		return;
-	}
+		BT_ERR("Failed to publish (err %d)", err);
+		if (pub->count == BT_MESH_PUB_TRANSMIT_COUNT(pub->retransmit)) {
+			pub->period_start = k_uptime_get_32();
+		}
 
-	err = bt_mesh_model_publish(pub->mod);
-	if (err) {
-		BT_ERR("Publishing failed (err %d)", err);
+		publish_sent(err, pub->mod);
 	}
 }
 
@@ -301,7 +306,7 @@ static void mod_init(struct bt_mesh_model *mod, struct bt_mesh_elem *elem,
 
 	if (mod->pub) {
 		mod->pub->mod = mod;
-		k_delayed_work_init(&mod->pub->timer, mod_publish);
+		k_work_init_delayable(&mod->pub->timer, mod_publish);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(mod->keys); i++) {
@@ -630,102 +635,67 @@ void bt_mesh_model_recv(struct bt_mesh_net_rx *rx, struct net_buf_simple *buf)
 	}
 }
 
-static int model_send(struct bt_mesh_model *model,
-		      struct bt_mesh_net_tx *tx, bool implicit_bind,
-		      struct net_buf_simple *msg,
-		      const struct bt_mesh_send_cb *cb, void *cb_data)
+int bt_mesh_model_send(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx,
+		       struct net_buf_simple *msg,
+		       const struct bt_mesh_send_cb *cb, void *cb_data)
 {
-	BT_DBG("net_idx 0x%04x app_idx 0x%04x dst 0x%04x", tx->ctx->net_idx,
-	       tx->ctx->app_idx, tx->ctx->addr);
+	struct bt_mesh_net_tx tx = {
+		.ctx = ctx,
+		.src = bt_mesh_model_elem(model)->addr,
+	};
+
+	BT_DBG("net_idx 0x%04x app_idx 0x%04x dst 0x%04x", tx.ctx->net_idx,
+	       tx.ctx->app_idx, tx.ctx->addr);
 	BT_DBG("len %u: %s", msg->len, bt_hex(msg->data, msg->len));
 
 	if (!bt_mesh_is_provisioned()) {
 		BT_ERR("Local node is not yet provisioned");
 		return -EAGAIN;
 	}
 
-	if (net_buf_simple_tailroom(msg) < 4) {
-		BT_ERR("Not enough tailroom for TransMIC");
+	if (!model_has_key(model, tx.ctx->app_idx)) {
+		BT_ERR("Model not bound to AppKey 0x%04x", tx.ctx->app_idx);
 		return -EINVAL;
 	}
 
-	if (msg->len > BT_MESH_TX_SDU_MAX - 4) {
-		BT_ERR("Too big message");
-		return -EMSGSIZE;
-	}
-
-	if (!implicit_bind && !model_has_key(model, tx->ctx->app_idx)) {
-		BT_ERR("Model not bound to AppKey 0x%04x", tx->ctx->app_idx);
-		return -EINVAL;
-	}
-
-	return bt_mesh_trans_send(tx, msg, cb, cb_data);
-}
-
-int bt_mesh_model_send(struct bt_mesh_model *model,
-		       struct bt_mesh_msg_ctx *ctx,
-		       struct net_buf_simple *msg,
-		       const struct bt_mesh_send_cb *cb, void *cb_data)
-{
-	struct bt_mesh_net_tx tx = {
-		.ctx = ctx,
-		.src = bt_mesh_model_elem(model)->addr,
-	};
-
-	return model_send(model, &tx, false, msg, cb, cb_data);
+	return bt_mesh_trans_send(&tx, msg, cb, cb_data);
 }
 
 int bt_mesh_model_publish(struct bt_mesh_model *model)
 {
-	NET_BUF_SIMPLE_DEFINE(sdu, BT_MESH_TX_SDU_MAX);
 	struct bt_mesh_model_pub *pub = model->pub;
 
 	if (!pub) {
 		return -ENOTSUP;
 	}
 
-	struct bt_mesh_msg_ctx ctx = {
-		.addr = pub->addr,
-		.send_ttl = pub->ttl,
-		.send_rel = pub->send_rel,
-		.app_idx = pub->key,
-	};
-	struct bt_mesh_net_tx tx = {
-		.ctx = &ctx,
-		.src = bt_mesh_model_elem(model)->addr,
-	};
-	int err;
-
 	BT_DBG("");
 
 	if (pub->addr == BT_MESH_ADDR_UNASSIGNED) {
 		return -EADDRNOTAVAIL;
 	}
 
-	if (pub->msg->len + 4 > BT_MESH_TX_SDU_MAX) {
+	if (!pub->msg || !pub->msg->len) {
+		BT_ERR("No publication message");
+		return -EINVAL;
+	}
+
+	if (pub->msg->len + BT_MESH_MIC_SHORT > BT_MESH_TX_SDU_MAX) {
 		BT_ERR("Message does not fit maximum SDU size");
 		return -EMSGSIZE;
 	}
 
 	if (pub->count) {
 		BT_WARN("Clearing publish retransmit timer");
-		k_delayed_work_cancel(&pub->timer);
 	}
 
-	net_buf_simple_add_mem(&sdu, pub->msg->data, pub->msg->len);
-
-	tx.friend_cred = pub->cred;
-
-	pub->count = BT_MESH_PUB_TRANSMIT_COUNT(pub->retransmit);
+	/* Account for initial transmission */
+	pub->count = BT_MESH_PUB_TRANSMIT_COUNT(pub->retransmit) + 1;
 
 	BT_DBG("Publish Retransmit Count %u Interval %ums", pub->count,
 	       BT_MESH_PUB_TRANSMIT_INT(pub->retransmit));
 
-	err = model_send(model, &tx, true, &sdu, &pub_sent_cb, model);
-	if (err) {
-		publish_retransmit_end(err, pub);
-		return err;
-	}
+	k_work_reschedule(&pub->timer, K_NO_WAIT);
 
 	return 0;
 }
@@ -1208,7 +1178,7 @@ static void commit_mod(struct bt_mesh_model *mod, struct bt_mesh_elem *elem,
 
 		if (ms > 0) {
 			BT_DBG("Starting publish timer (period %u ms)", ms);
-			k_delayed_work_submit(&mod->pub->timer, K_MSEC(ms));
+			k_work_schedule(&mod->pub->timer, K_MSEC(ms));
 		}
 	}
 

subsys/bluetooth/mesh/cfg_srv.c

@@ -201,7 +201,10 @@ static uint8_t _mod_pub_set(struct bt_mesh_model *model, uint16_t pub_addr,
 		model->pub->count = 0U;
 
 		if (model->pub->update) {
-			k_delayed_work_cancel(&model->pub->timer);
+			/* If this fails, the timer will check pub->addr and
+			 * exit without transmitting.
+			 */
+			(void)k_work_cancel_delayable(&model->pub->timer);
 		}
 
 		if (IS_ENABLED(CONFIG_BT_SETTINGS) && store) {
@@ -239,10 +242,13 @@ static uint8_t _mod_pub_set(struct bt_mesh_model *model, uint16_t pub_addr,
 		BT_DBG("period %u ms", period_ms);
 
 		if (period_ms > 0) {
-			k_delayed_work_submit(&model->pub->timer,
-					      K_MSEC(period_ms));
+			k_work_reschedule(&model->pub->timer,
+					  K_MSEC(period_ms));
 		} else {
-			k_delayed_work_cancel(&model->pub->timer);
+			/* If this fails, publication will stop after the
+			 * ongoing set of retransmits.
+			 */
+			(void)k_work_cancel_delayable(&model->pub->timer);
 		}
 	}
 

subsys/bluetooth/mesh/main.c

@@ -227,7 +227,10 @@ static void model_suspend(struct bt_mesh_model *mod, struct bt_mesh_elem *elem,
 {
 	if (mod->pub && mod->pub->update) {
 		mod->pub->count = 0U;
-		k_delayed_work_cancel(&mod->pub->timer);
+		/* If this fails, the work handler will check the suspend call
+		 * and exit without transmitting.
+		 */
+		(void)k_work_cancel_delayable(&mod->pub->timer);
 	}
 }
 
@@ -268,8 +271,8 @@ static void model_resume(struct bt_mesh_model *mod, struct bt_mesh_elem *elem,
 		int32_t period_ms = bt_mesh_model_pub_period_get(mod);
 
 		if (period_ms) {
-			k_delayed_work_submit(&mod->pub->timer,
-					      K_MSEC(period_ms));
+			k_work_reschedule(&mod->pub->timer,
+					  K_MSEC(period_ms));
 		}
 	}
 }