michaelh/zephyr
1
0
Fork 0
Code Releases Activity

Bluetooth: host: Fix ordering of TX sent callbacks

Browse source 
Now that the TX callbacks happen from the system workqueue but fixed
channels get processed from the RX thread there's a risk that the
ordering of these gets messed up. This is particularly bad for ATT
when it's trying to enforce flow control.

To fix the issue store the completed TX packet information in a
per-connection list and process this list before processing any new
packets for the same connection. We still also schedule a workqueue
callback, which will simply do nothing for this list if bt_recv()
already took care of it.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
This commit is contained in:
Johan Hedberg 2019-11-28 14:11:57 +02:00 • committed by Johan Hedberg
commit 1e831befea
3 changed files with 71 additions and 47 deletions
Download patch file Download diff file Expand all files Collapse all files

103
subsys/bluetooth/host/conn.c
View file

@ -240,6 +240,60 @@ static int send_conn_le_param_update(struct bt_conn *conn,
return bt_l2cap_update_conn_param(conn, param);
}
static void tx_free(struct bt_conn_tx *tx)
{
tx->cb = NULL;
tx->user_data = NULL;
tx->pending_no_cb = 0U;
k_fifo_put(&free_tx, tx);
}
static void tx_notify(struct bt_conn *conn)
{
BT_DBG("conn %p", conn);
while (1) {
struct bt_conn_tx *tx;
unsigned int key;
bt_conn_tx_cb_t cb;
void *user_data;
key = irq_lock();
if (sys_slist_is_empty(&conn->tx_complete)) {
irq_unlock(key);
break;
}
tx = (void *)sys_slist_get_not_empty(&conn->tx_complete);
irq_unlock(key);
BT_DBG("tx %p cb %p user_data %p", tx, tx->cb, tx->user_data);
/* Copy over the params */
cb = tx->cb;
user_data = tx->user_data;
/* Free up TX notify since there may be user waiting */
tx_free(tx);
/* Run the callback, at this point it should be safe to
* allocate new buffers since the TX should have been
* unblocked by tx_free.
*/
cb(conn, user_data);
}
}
static void tx_complete_work(struct k_work *work)
{
struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn,
tx_complete_work);
BT_DBG("conn %p", conn);
tx_notify(conn);
}
static void conn_update_timeout(struct k_work *work)
{
struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn, update_work);
@ -304,45 +358,6 @@ static void conn_update_timeout(struct k_work *work)
atomic_set_bit(conn->flags, BT_CONN_SLAVE_PARAM_UPDATE);
}
static void tx_free(struct bt_conn_tx *tx)
{
if (tx->conn) {
bt_conn_unref(tx->conn);
tx->conn = NULL;
}
tx->cb = NULL;
tx->user_data = NULL;
tx->pending_no_cb = 0U;
k_fifo_put(&free_tx, tx);
}
static void tx_notify_cb(struct k_work *work)
{
struct bt_conn_tx *tx = CONTAINER_OF(work, struct bt_conn_tx, work);
struct bt_conn *conn;
bt_conn_tx_cb_t cb;
void *user_data;
BT_DBG("tx %p conn %p cb %p user_data %p", tx, tx->conn, tx->cb,
tx->user_data);
/* Copy over the params */
conn = bt_conn_ref(tx->conn);
cb = tx->cb;
user_data = tx->user_data;
/* Free up TX notify since there may be user waiting */
tx_free(tx);
/* Run the callback, at this point it should be safe to allocate new
* buffers since the TX should have been unblocked by tx_free.
*/
cb(conn, user_data);
bt_conn_unref(conn);
}
static struct bt_conn *conn_new(void)
{
struct bt_conn *conn = NULL;
@ -362,6 +377,8 @@ static struct bt_conn *conn_new(void)
(void)memset(conn, 0, sizeof(*conn));
k_delayed_work_init(&conn->update_work, conn_update_timeout);
k_work_init(&conn->tx_complete_work, tx_complete_work);
atomic_set(&conn->ref, 1);
return conn;
@ -1114,6 +1131,11 @@ void bt_conn_recv(struct bt_conn *conn, struct net_buf *buf, u8_t flags)
struct bt_l2cap_hdr *hdr;
u16_t len;
/* Make sure we notify any pending TX callbacks before processing
* new data for this connection.
*/
tx_notify(conn);
BT_DBG("handle %u len %u flags %02x", conn->handle, buf->len, flags);
/* Check packet boundary flags */
@ -1250,9 +1272,7 @@ int bt_conn_send_cb(struct bt_conn *conn, struct net_buf *buf,
tx->cb = cb;
tx->user_data = user_data;
tx->conn = bt_conn_ref(conn);
tx->pending_no_cb = 0U;
k_work_init(&tx->work, tx_notify_cb);
tx_data(buf)->tx = tx;
} else {
@ -1616,6 +1636,7 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state)
if (old_state == BT_CONN_CONNECTED ||
old_state == BT_CONN_DISCONNECT) {
process_unack_tx(conn);
tx_notify(conn);
/* Cancel Connection Update if it is pending */
if (conn->type == BT_CONN_TYPE_LE) {

12
subsys/bluetooth/host/conn_internal.h
View file

@ -80,11 +80,8 @@ struct bt_conn_sco {
typedef void (*bt_conn_tx_cb_t)(struct bt_conn *conn, void *user_data);
struct bt_conn_tx {
union {
sys_snode_t node;
struct k_work work;
};
struct bt_conn *conn;
sys_snode_t node;
bt_conn_tx_cb_t cb;
void *user_data;
@ -123,6 +120,11 @@ struct bt_conn {
*/
u32_t pending_no_cb;
/* Completed TX for which we need to call the callback */
sys_slist_t tx_complete;
struct k_work tx_complete_work;
/* Queue for outgoing ACL data */
struct k_fifo tx_queue;

3
subsys/bluetooth/host/hci_core.c
View file

@ -728,9 +728,10 @@ static void hci_num_completed_packets(struct net_buf *buf)
key = irq_lock();
conn->pending_no_cb = tx->pending_no_cb;
tx->pending_no_cb = 0U;
sys_slist_append(&conn->tx_complete, &tx->node);
irq_unlock(key);
k_work_submit(&tx->work);
k_work_submit(&conn->tx_complete_work);
k_sem_give(bt_conn_get_pkts(conn));
}