BLE: Mesh: When transport send seg_msg to LPN

[LingaoM]

zephyr/subsys/bluetooth/mesh/transport.c

Lines 314 to 465 in 0746017

	static int send_seg(struct bt_mesh_net_tx *net_tx, struct net_buf_simple *sdu,
	const struct bt_mesh_send_cb *cb, void *cb_data)
	{
	u8_t seg_hdr, seg_o;
	u16_t seq_zero;
	struct seg_tx *tx;
	int i;
	BT_DBG("src 0x%04x dst 0x%04x app_idx 0x%04x aszmic %u sdu_len %u",
	net_tx->src, net_tx->ctx->addr, net_tx->ctx->app_idx,
	net_tx->aszmic, sdu->len);
	if (sdu->len < 1) {
	BT_ERR("Zero-length SDU not allowed");
	return -EINVAL;
	}
	if (sdu->len > BT_MESH_TX_SDU_MAX) {
	BT_ERR("Not enough segment buffers for length %u", sdu->len);
	return -EMSGSIZE;
	}
	for (tx = NULL, i = 0; i < ARRAY_SIZE(seg_tx); i++) {
	if (!seg_tx[i].nack_count) {
	tx = &seg_tx[i];
	break;
	}
	}
	if (!tx) {
	BT_ERR("No multi-segment message contexts available");
	return -EBUSY;
	}
	if (net_tx->ctx->app_idx == BT_MESH_KEY_DEV) {
	seg_hdr = SEG_HDR(0, 0);
	} else {
	seg_hdr = SEG_HDR(1, net_tx->aid);
	}
	seg_o = 0U;
	tx->dst = net_tx->ctx->addr;
	tx->seg_n = (sdu->len - 1) / 12U;
	tx->nack_count = tx->seg_n + 1;
	tx->seq_auth = SEQ_AUTH(BT_MESH_NET_IVI_TX, bt_mesh.seq);
	tx->sub = net_tx->sub;
	tx->new_key = net_tx->sub->kr_flag;
	tx->cb = cb;
	tx->cb_data = cb_data;
	if (net_tx->ctx->send_ttl == BT_MESH_TTL_DEFAULT) {
	tx->ttl = bt_mesh_default_ttl_get();
	} else {
	tx->ttl = net_tx->ctx->send_ttl;
	}
	seq_zero = tx->seq_auth & 0x1fff;
	BT_DBG("SeqZero 0x%04x", seq_zero);
	if (IS_ENABLED(CONFIG_BT_MESH_FRIEND) &&
	!bt_mesh_friend_queue_has_space(tx->sub->net_idx, net_tx->src,
	tx->dst, &tx->seq_auth,
	tx->seg_n + 1) &&
	BT_MESH_ADDR_IS_UNICAST(tx->dst)) {
	BT_ERR("Not enough space in Friend Queue for %u segments",
	tx->seg_n + 1);
	seg_tx_reset(tx);
	return -ENOBUFS;
	}
	for (seg_o = 0U; sdu->len; seg_o++) {
	struct net_buf *seg;
	u16_t len;
	int err;
	seg = bt_mesh_adv_create(BT_MESH_ADV_DATA, net_tx->xmit,
	BUF_TIMEOUT);
	if (!seg) {
	BT_ERR("Out of segment buffers");
	seg_tx_reset(tx);
	return -ENOBUFS;
	}
	BT_MESH_ADV(seg)->seg.attempts = SEG_RETRANSMIT_ATTEMPTS;
	net_buf_reserve(seg, BT_MESH_NET_HDR_LEN);
	net_buf_add_u8(seg, seg_hdr);
	net_buf_add_u8(seg, (net_tx->aszmic << 7) | seq_zero >> 6);
	net_buf_add_u8(seg, (((seq_zero & 0x3f) << 2) |
	(seg_o >> 3)));
	net_buf_add_u8(seg, ((seg_o & 0x07) << 5) | tx->seg_n);
	len = MIN(sdu->len, 12);
	net_buf_add_mem(seg, sdu->data, len);
	net_buf_simple_pull(sdu, len);
	if (IS_ENABLED(CONFIG_BT_MESH_FRIEND)) {
	enum bt_mesh_friend_pdu_type type;
	if (seg_o == tx->seg_n) {
	type = BT_MESH_FRIEND_PDU_COMPLETE;
	} else {
	type = BT_MESH_FRIEND_PDU_PARTIAL;
	}
	if (bt_mesh_friend_enqueue_tx(net_tx, type,
	&tx->seq_auth,
	tx->seg_n + 1,
	&seg->b) &&
	BT_MESH_ADDR_IS_UNICAST(net_tx->ctx->addr)) {
	/* PDUs for a specific Friend should only go
	* out through the Friend Queue.
	*/
	net_buf_unref(seg);
	continue;
	}
	}
	tx->seg[seg_o] = net_buf_ref(seg);
	BT_DBG("Sending %u/%u", seg_o, tx->seg_n);
	err = bt_mesh_net_send(net_tx, seg,
	seg_o ? &seg_sent_cb : &first_sent_cb,
	tx);
	if (err) {
	BT_ERR("Sending segment failed");
	seg_tx_reset(tx);
	return err;
	}
	}
	/* This can happen if segments only went into the Friend Queue */
	if (IS_ENABLED(CONFIG_BT_MESH_FRIEND) && !tx->seg[0]) {
	seg_tx_reset(tx);
	/* If there was a callback notify sending immediately since
	* there's no other way to track this (at least currently)
	* with the Friend Queue.
	*/
	send_cb_finalize(cb, cb_data);
	}
	if (IS_ENABLED(CONFIG_BT_MESH_LOW_POWER) &&
	bt_mesh_lpn_established()) {
	bt_mesh_lpn_poll();
	}
	return 0;
	}

        if (IS_ENABLED(CONFIG_BT_MESH_FRIEND) && 
 	    !bt_mesh_friend_queue_has_space(tx->sub->net_idx, net_tx->src, 
 					    tx->dst, &tx->seq_auth, 
 					    tx->seg_n + 1) && 
 	    BT_MESH_ADDR_IS_UNICAST(tx->dst)) { 
 		BT_ERR("Not enough space in Friend Queue for %u segments", 
 		       tx->seg_n + 1); 
 		seg_tx_reset(tx); 
 		return -ENOBUFS; 
 	} 

According to the code above, if I send a SEG message (greater than CONFIG_BT_MESH_FRIEND_QUEUE_SIZE) to a UNICAST Address (not an LPN address),will be cause an abnormality.

I think should add some functions to determine if it is sent to LPN, such as bt_mesh_friend_match

[jhedberg]

@menglingao1997 I don't see a bug here. Could be because of bad naming of the APIs, but if you look at the implementation of bt_mesh_friend_queue_has_space() you'll see that it'll return true if no LPN matches the given address, i.e. this branch will not be taken in that case.

[jhedberg]

@menglingao1997 here's the code I'm referring to:

zephyr/subsys/bluetooth/mesh/friend.c

Lines 1280 to 1285 in 0746017

	/* If there were no matched LPNs treat this as success, so the
	* transport layer can continue its work.
	*/
	if (!friend_match) {
	return true;
	}

I'll close this, but please reopen if you think something is still buggy or unclear.

[jhedberg]

Just wanted to add that one of the reasons I didn't go for a separate "match" call is that I'm trying to avoid iterating the LPN list too many times. We're already doing it twice for TX and three times for RX, which I'm not very happy about.

[LingaoM]

zephyr/subsys/bluetooth/mesh/friend.c

Lines 1256 to 1264 in 0746017

	bool bt_mesh_friend_queue_has_space(u16_t net_idx, u16_t src, u16_t dst,
	u64_t *seq_auth, u8_t seg_count)
	{
	bool someone_has_space = false, friend_match = false;
	int i;
	if (seg_count > CONFIG_BT_MESH_FRIEND_QUEUE_SIZE) {
	return false;
	}

if seg_count bigger than CONFIG_BT_MESH_FRIEND_QUEUE_SIZE, above function return false, this is my concerned.

[jhedberg]

if seg_count bigger than CONFIG_BT_MESH_FRIEND_QUEUE_SIZE, above function return false, this is my concerned.

@menglingao1997 that's a real issue, thanks for clarifying. I'm thinking we might be able solve this through Kconfig so that the Friend Queue size must be at least as big as the maximum number of segments we can receive.

[jhedberg]

FWIW, e.g. with the default settings this is a non-issue, since that's 3 RX segments max and 16 for the friend queue size.

I also realised it's not so trivial with Kconfig since the RX segment count is derived indirectly from BT_MESH_RX_SDU_MAX

[jhedberg]

@menglingao1997 I think I'll just fix it like this:

--- a/subsys/bluetooth/mesh/friend.c
+++ b/subsys/bluetooth/mesh/friend.c
@@ -1226,6 +1226,10 @@ static bool friend_queue_has_space(struct bt_mesh_friend *frnd, u16_t addr,
        u32_t total = 0;
        int i;
 
+       if (seg_count > CONFIG_BT_MESH_FRIEND_QUEUE_SIZE) {
+               return false;
+       }
+
        for (i = 0; i < ARRAY_SIZE(frnd->seg); i++) {
                struct bt_mesh_friend_seg *seg = &frnd->seg[i];
 
@@ -1259,10 +1263,6 @@ bool bt_mesh_friend_queue_has_space(u16_t net_idx, u16_t src, u16_t dst,
        bool someone_has_space = false, friend_match = false;
        int i;
 
-       if (seg_count > CONFIG_BT_MESH_FRIEND_QUEUE_SIZE) {
-               return false;
-       }
-
        for (i = 0; i < ARRAY_SIZE(bt_mesh.frnd); i++) {
                struct bt_mesh_friend *frnd = &bt_mesh.frnd[i];
 

Can I get a quick ack to proceed?

[LingaoM]

I think ok.