Bluetooth: (conn.c) ACL buffers can "steal" ISO TX contexts

[jori-nordic]

Reporting just so I don't forget about it.

Both iso_tx[] and conn_tx[] arrays end up in the free_tx fifo.
This fifo is then used indiscriminately by conn_tx_alloc() to allocate a TX context for a given buffer. That buffer can be for an ACL or ISO link.

This means an ACL link can starve an ISO link.

Env

Zephyr @

zephyr/subsys/bluetooth/host/conn.c

Lines 427 to 449 in 78150c8

	static struct bt_conn_tx *conn_tx_alloc(void)
	{
	/* The TX context always get freed in the system workqueue,
	* so if we're in the same workqueue but there are no immediate
	* contexts available, there's no chance we'll get one by waiting.
	*/
	if (k_current_get() == &k_sys_work_q.thread) {
	return k_fifo_get(&free_tx, K_NO_WAIT);
	}
	if (IS_ENABLED(CONFIG_BT_CONN_LOG_LEVEL_DBG)) {
	struct bt_conn_tx *tx = k_fifo_get(&free_tx, K_NO_WAIT);
	if (tx) {
	return tx;
	}
	LOG_WRN("Unable to get an immediate free conn_tx");
	}
	return k_fifo_get(&free_tx, K_FOREVER);
	}

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[Thalley]

I guess the solution here is to have individual pools for ACL and ISO?
What about BREDR? L2CAP?

[jori-nordic]

we already have the individual pools. I guess the solution is pretty simple, also have individual fifos, by adding a conn-type param to conn_tx_alloc(). I have not dived too deep on this one yet.

[Thalley]

we already have the individual pools. I guess the solution is pretty simple, also have individual fifos, by adding a conn-type param to conn_tx_alloc(). I have not dived too deep on this one yet.

Right, I'm sure that's also what I meant :P

But I still assume that the issue between ISO and ACL also exist for ACL and SCO/BR, although much less apparent?

[jori-nordic]

It's possible, yes. Although I'm not sure how to test SCO.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.