Add support for TX timestamping in ISO/SCO/L2CAP sockets.
These patches allow fixing / working around controller(?) issue where
two ISO streams in same group get desynchronized. It also gives user
applications the best latency information as available to kernel.
Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR
wakeup on TX timestamp arrival, which is mainly a nuisance in the use
case here. The alternative to this seems be to deal with the POLLERR
wakeups in BlueZ side, but this seems hard as it's always enabled in
poll() flags so not clear if anything else than polling at regular
intervals can be done there.
Pipewire side:
https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-ts-test2
With this change, https://github.com/bluez/bluez/issues/515 is more or
less fixed, and the sound server can figure out the total latency to
audio rendering (tx latency + transport latency + presentation delay).
For ISO, we can later use LE Read ISO TX Sync to provide hardware
timestamps, but this requires figuring out the sequence number
synchronization first.
v2:
- Rename *tx_comp* -> *tx*
- Add hci_send_conn_frame() and handle all link types
- Add SCO timestamping. Deal with no flow control -> no Num_Comp_* events
- Handle HCI_FLOW_CTL_MODE_BLOCK_BASED
- Add BT_NO_ERRQUEUE_POLL
Pauli Virtanen (5):
Bluetooth: add support for skb TX timestamping
Bluetooth: ISO: add TX timestamping
Bluetooth: L2CAP: add TX timestamping
Bluetooth: SCO: add TX timestamping
Bluetooth: add BT_NO_ERRQUEUE_POLL socket option
include/net/bluetooth/bluetooth.h | 10 ++-
include/net/bluetooth/hci_core.h | 12 ++++
include/net/bluetooth/l2cap.h | 3 +-
net/bluetooth/6lowpan.c | 2 +-
net/bluetooth/af_bluetooth.c | 72 ++++++++++++++++++-
net/bluetooth/hci_conn.c | 111 ++++++++++++++++++++++++++++++
net/bluetooth/hci_core.c | 19 +++--
net/bluetooth/hci_event.c | 11 ++-
net/bluetooth/iso.c | 32 ++++++---
net/bluetooth/l2cap_core.c | 11 ++-
net/bluetooth/l2cap_sock.c | 23 +++++--
net/bluetooth/sco.c | 27 ++++++--
net/bluetooth/smp.c | 2 +-
13 files changed, 303 insertions(+), 32 deletions(-)
--
2.44.0
Add BT_SCM_ERROR socket CMSG type.
Support TX timestamping in ISO sockets.
Support MSG_ERRQUEUE in ISO recvmsg.
Signed-off-by: Pauli Virtanen <[email protected]>
---
include/net/bluetooth/bluetooth.h | 1 +
net/bluetooth/iso.c | 24 ++++++++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 7ffa8c192c3f..9280e72093ee 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -155,6 +155,7 @@ struct bt_voice {
#define BT_PKT_STATUS 16
#define BT_SCM_PKT_STATUS 0x03
+#define BT_SCM_ERROR 0x04
#define BT_ISO_QOS 17
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 8af75d37b14c..a77ab9df7994 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -489,7 +489,8 @@ static struct bt_iso_qos *iso_sock_get_qos(struct sock *sk)
return &iso_pi(sk)->qos;
}
-static int iso_send_frame(struct sock *sk, struct sk_buff *skb)
+static int iso_send_frame(struct sock *sk, struct sk_buff *skb,
+ const struct sockcm_cookie *sockc)
{
struct iso_conn *conn = iso_pi(sk)->conn;
struct bt_iso_qos *qos = iso_sock_get_qos(sk);
@@ -509,10 +510,12 @@ static int iso_send_frame(struct sock *sk, struct sk_buff *skb)
hdr->slen = cpu_to_le16(hci_iso_data_len_pack(len,
HCI_ISO_STATUS_VALID));
- if (sk->sk_state == BT_CONNECTED)
+ if (sk->sk_state == BT_CONNECTED) {
+ hci_tx_timestamp(skb, sockc);
hci_send_iso(conn->hcon, skb);
- else
+ } else {
len = -ENOTCONN;
+ }
return len;
}
@@ -1266,6 +1269,7 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
{
struct sock *sk = sock->sk;
struct sk_buff *skb, **frag;
+ struct sockcm_cookie sockc;
size_t mtu;
int err;
@@ -1278,6 +1282,14 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (msg->msg_flags & MSG_OOB)
return -EOPNOTSUPP;
+ sockcm_init(&sockc, sk);
+
+ if (msg->msg_controllen) {
+ err = sock_cmsg_send(sk, msg, &sockc);
+ if (err)
+ return err;
+ }
+
lock_sock(sk);
if (sk->sk_state != BT_CONNECTED) {
@@ -1323,7 +1335,7 @@ static int iso_sock_sendmsg(struct socket *sock, struct msghdr *msg,
lock_sock(sk);
if (sk->sk_state == BT_CONNECTED)
- err = iso_send_frame(sk, skb);
+ err = iso_send_frame(sk, skb, &sockc);
else
err = -ENOTCONN;
@@ -1379,6 +1391,10 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
BT_DBG("sk %p", sk);
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return sock_recv_errqueue(sk, msg, len, SOL_BLUETOOTH,
+ BT_SCM_ERROR);
+
if (test_and_clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
lock_sock(sk);
switch (sk->sk_state) {
--
2.44.0
Support TX timestamping in L2CAP sockets.
Support MSG_ERRQUEUE recvmsg.
Signed-off-by: Pauli Virtanen <[email protected]>
---
include/net/bluetooth/l2cap.h | 3 ++-
net/bluetooth/6lowpan.c | 2 +-
net/bluetooth/l2cap_core.c | 11 ++++++++++-
net/bluetooth/l2cap_sock.c | 15 ++++++++++++++-
net/bluetooth/smp.c | 2 +-
5 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a4278aa618ab..3f4057ced971 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -941,7 +941,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason);
int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
bdaddr_t *dst, u8 dst_type, u16 timeout);
int l2cap_chan_reconfigure(struct l2cap_chan *chan, __u16 mtu);
-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len);
+int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
+ const struct sockcm_cookie *sockc);
void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
int l2cap_chan_check_security(struct l2cap_chan *chan, bool initiator);
void l2cap_chan_set_defaults(struct l2cap_chan *chan);
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 27520a8a486f..24635f9e758a 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -443,7 +443,7 @@ static int send_pkt(struct l2cap_chan *chan, struct sk_buff *skb,
memset(&msg, 0, sizeof(msg));
iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iv, 1, skb->len);
- err = l2cap_chan_send(chan, &msg, skb->len);
+ err = l2cap_chan_send(chan, &msg, skb->len, NULL);
if (err > 0) {
netdev->stats.tx_bytes += err;
netdev->stats.tx_packets++;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 467b242d8be0..cf3b8e9b7b3b 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2488,7 +2488,8 @@ static void l2cap_le_flowctl_send(struct l2cap_chan *chan)
skb_queue_len(&chan->tx_q));
}
-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
+int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
+ const struct sockcm_cookie *sockc)
{
struct sk_buff *skb;
int err;
@@ -2503,6 +2504,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
if (IS_ERR(skb))
return PTR_ERR(skb);
+ hci_tx_timestamp(skb, sockc);
+
l2cap_do_send(chan, skb);
return len;
}
@@ -2526,6 +2529,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
if (err)
return err;
+ hci_tx_timestamp(skb_peek(&seg_queue), sockc);
+
skb_queue_splice_tail_init(&seg_queue, &chan->tx_q);
l2cap_le_flowctl_send(chan);
@@ -2547,6 +2552,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
if (IS_ERR(skb))
return PTR_ERR(skb);
+ hci_tx_timestamp(skb, sockc);
+
l2cap_do_send(chan, skb);
err = len;
break;
@@ -2570,6 +2577,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
if (err)
break;
+ hci_tx_timestamp(skb_peek(&seg_queue), sockc);
+
if (chan->mode == L2CAP_MODE_ERTM)
l2cap_tx(chan, NULL, &seg_queue, L2CAP_EV_DATA_REQUEST);
else
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 4287aa6cc988..9a9f933a748b 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1115,6 +1115,7 @@ static int l2cap_sock_sendmsg(struct socket *sock, struct msghdr *msg,
{
struct sock *sk = sock->sk;
struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+ struct sockcm_cookie sockc;
int err;
BT_DBG("sock %p, sk %p", sock, sk);
@@ -1129,6 +1130,14 @@ static int l2cap_sock_sendmsg(struct socket *sock, struct msghdr *msg,
if (sk->sk_state != BT_CONNECTED)
return -ENOTCONN;
+ sockcm_init(&sockc, sk);
+
+ if (msg->msg_controllen) {
+ err = sock_cmsg_send(sk, msg, &sockc);
+ if (err)
+ return err;
+ }
+
lock_sock(sk);
err = bt_sock_wait_ready(sk, msg->msg_flags);
release_sock(sk);
@@ -1136,7 +1145,7 @@ static int l2cap_sock_sendmsg(struct socket *sock, struct msghdr *msg,
return err;
l2cap_chan_lock(chan);
- err = l2cap_chan_send(chan, msg, len);
+ err = l2cap_chan_send(chan, msg, len, &sockc);
l2cap_chan_unlock(chan);
return err;
@@ -1149,6 +1158,10 @@ static int l2cap_sock_recvmsg(struct socket *sock, struct msghdr *msg,
struct l2cap_pinfo *pi = l2cap_pi(sk);
int err;
+ if (unlikely(flags & MSG_ERRQUEUE))
+ return sock_recv_errqueue(sk, msg, len, SOL_BLUETOOTH,
+ BT_SCM_ERROR);
+
lock_sock(sk);
if (sk->sk_state == BT_CONNECT2 && test_bit(BT_SK_DEFER_SETUP,
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 1e7ea3a4b7ef..4612115ec09a 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -608,7 +608,7 @@ static void smp_send_cmd(struct l2cap_conn *conn, u8 code, u16 len, void *data)
iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, iv, 2, 1 + len);
- l2cap_chan_send(chan, &msg, 1 + len);
+ l2cap_chan_send(chan, &msg, 1 + len, NULL);
if (!chan->data)
return;
--
2.44.0
Support enabling TX timestamping for some skbs, and track them until
packet completion. Generate SCM_TSTAMP_SCHED when sending to driver,
and SCM_TSTAMP_SND at packet completion.
Make the default situation with no TX timestamping more efficient by
only counting packets in the queue when there is nothing to track. When
there is something to track, we need to make clones, since the driver
may modify sent skbs.
Signed-off-by: Pauli Virtanen <[email protected]>
---
include/net/bluetooth/hci_core.h | 12 ++++
net/bluetooth/hci_conn.c | 111 +++++++++++++++++++++++++++++++
net/bluetooth/hci_core.c | 19 ++++--
net/bluetooth/hci_event.c | 11 ++-
4 files changed, 146 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 56fb42df44a3..f89c0eee6912 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -267,6 +267,12 @@ struct adv_info {
struct delayed_work rpa_expired_cb;
};
+struct tx_queue {
+ struct sk_buff_head queue;
+ unsigned int extra;
+ unsigned int tracked;
+};
+
#define HCI_MAX_ADV_INSTANCES 5
#define HCI_DEFAULT_ADV_DURATION 2
@@ -763,6 +769,8 @@ struct hci_conn {
struct sk_buff_head data_q;
struct list_head chan_list;
+ struct tx_queue tx_q;
+
struct delayed_work disc_work;
struct delayed_work auto_accept_work;
struct delayed_work idle_work;
@@ -1546,6 +1554,10 @@ void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);
void hci_conn_failed(struct hci_conn *conn, u8 status);
u8 hci_conn_set_handle(struct hci_conn *conn, u16 handle);
+void hci_conn_tx_queue(struct hci_conn *conn, struct sk_buff *skb);
+void hci_conn_tx_dequeue(struct hci_conn *conn);
+void hci_tx_timestamp(struct sk_buff *skb, const struct sockcm_cookie *sockc);
+
/*
* hci_conn_get() and hci_conn_put() are used to control the life-time of an
* "hci_conn" object. They do not guarantee that the hci_conn object is running,
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 3ad74f76983b..ce94ffaf06d4 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -27,6 +27,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
+#include <linux/errqueue.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -973,6 +974,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
}
skb_queue_head_init(&conn->data_q);
+ skb_queue_head_init(&conn->tx_q.queue);
INIT_LIST_HEAD(&conn->chan_list);
INIT_LIST_HEAD(&conn->link_list);
@@ -1117,6 +1119,7 @@ void hci_conn_del(struct hci_conn *conn)
}
skb_queue_purge(&conn->data_q);
+ skb_queue_purge(&conn->tx_q.queue);
/* Remove the connection from the list and cleanup its remaining
* state. This is a separate function since for some cases like
@@ -2928,3 +2931,111 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
return hci_cmd_sync_queue_once(hdev, abort_conn_sync, conn, NULL);
}
+
+void hci_tx_timestamp(struct sk_buff *skb, const struct sockcm_cookie *sockc)
+{
+ /* This shall be called on a single skb of those generated by user
+ * sendmsg(), and only when the sendmsg() does not return error to
+ * user. This is required for keeping the tskey that increments here in
+ * sync with possible sendmsg() counting by user.
+ */
+
+ if (!skb || !sockc)
+ return;
+
+ skb_setup_tx_timestamp(skb, sockc->tsflags);
+}
+
+void hci_conn_tx_queue(struct hci_conn *conn, struct sk_buff *skb)
+{
+ struct tx_queue *comp = &conn->tx_q;
+ const unsigned int max_queue_len = 128;
+ bool track = false;
+ bool comp_events = false;
+
+ if (skb->sk) {
+ if (skb_shinfo(skb)->tx_flags & SKBTX_SCHED_TSTAMP)
+ __skb_tstamp_tx(skb, NULL, NULL, skb->sk,
+ SCM_TSTAMP_SCHED);
+
+ if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
+ track = true;
+ }
+
+ /* If nothing is tracked, just count extra skbs at the queue head */
+ if (!track && !comp->tracked) {
+ comp->extra++;
+ return;
+ }
+
+ /* Emit SND timestamp for the tracked skb later on Number of Completed
+ * Packets/Data Blocks event, if these events are generated. Otherwise,
+ * emit it now.
+ */
+ switch (conn->type) {
+ case ISO_LINK:
+ case ACL_LINK:
+ case LE_LINK:
+ comp_events = true;
+ break;
+ }
+
+ if (!comp_events) {
+ if (track)
+ skb_tstamp_tx(skb, NULL);
+ return;
+ }
+
+ /* Safeguard vs. controller not acking packets */
+ if (skb_queue_len(&comp->queue) >= max_queue_len)
+ goto count_only;
+
+ if (track) {
+ skb = skb_clone_sk(skb);
+ if (!skb)
+ goto count_only;
+
+ comp->tracked++;
+ } else {
+ skb = skb_clone(skb, GFP_KERNEL);
+ if (!skb)
+ goto count_only;
+ }
+
+ skb_queue_tail(&comp->queue, skb);
+ return;
+
+count_only:
+ /* Stop tracking skbs, and only count. This will not emit timestamps for
+ * the packets, but if we get here something is more seriously wrong.
+ */
+ comp->tracked = 0;
+ comp->extra += skb_queue_len(&comp->queue) + 1;
+ skb_queue_purge(&comp->queue);
+}
+
+void hci_conn_tx_dequeue(struct hci_conn *conn)
+{
+ struct tx_queue *comp = &conn->tx_q;
+ struct sk_buff *skb;
+
+ /* If there are tracked skbs, the counted extra go before dequeuing real
+ * skbs, to keep ordering. When nothing is tracked, the ordering doesn't
+ * matter so dequeue real skbs first to get rid of them ASAP.
+ */
+ if (comp->extra && (comp->tracked || skb_queue_empty(&comp->queue))) {
+ comp->extra--;
+ return;
+ }
+
+ skb = skb_dequeue(&comp->queue);
+ if (!skb)
+ return;
+
+ if (skb->sk) {
+ comp->tracked--;
+ skb_tstamp_tx(skb, NULL);
+ }
+
+ kfree_skb(skb);
+}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1690ae57a09d..b77a8b204f21 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3101,6 +3101,13 @@ static int hci_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
return 0;
}
+static int hci_send_conn_frame(struct hci_dev *hdev, struct hci_conn *conn,
+ struct sk_buff *skb)
+{
+ hci_conn_tx_queue(conn, skb);
+ return hci_send_frame(hdev, skb);
+}
+
/* Send HCI command */
int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen,
const void *param)
@@ -3657,7 +3664,7 @@ static void hci_sched_sco(struct hci_dev *hdev)
while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, "e))) {
while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
BT_DBG("skb %p len %d", skb, skb->len);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, conn, skb);
conn->sent++;
if (conn->sent == ~0)
@@ -3681,7 +3688,7 @@ static void hci_sched_esco(struct hci_dev *hdev)
"e))) {
while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
BT_DBG("skb %p len %d", skb, skb->len);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, conn, skb);
conn->sent++;
if (conn->sent == ~0)
@@ -3715,7 +3722,7 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
hci_conn_enter_active_mode(chan->conn,
bt_cb(skb)->force_active);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, chan->conn, skb);
hdev->acl_last_tx = jiffies;
hdev->acl_cnt--;
@@ -3771,7 +3778,7 @@ static void hci_sched_acl_blk(struct hci_dev *hdev)
hci_conn_enter_active_mode(chan->conn,
bt_cb(skb)->force_active);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, chan->conn, skb);
hdev->acl_last_tx = jiffies;
hdev->block_cnt -= blocks;
@@ -3837,7 +3844,7 @@ static void hci_sched_le(struct hci_dev *hdev)
skb = skb_dequeue(&chan->data_q);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, chan->conn, skb);
hdev->le_last_tx = jiffies;
cnt--;
@@ -3876,7 +3883,7 @@ static void hci_sched_iso(struct hci_dev *hdev)
while (*cnt && (conn = hci_low_sent(hdev, ISO_LINK, "e))) {
while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
BT_DBG("skb %p len %d", skb, skb->len);
- hci_send_frame(hdev, skb);
+ hci_send_conn_frame(hdev, conn, skb);
conn->sent++;
if (conn->sent == ~0)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 4ae224824012..fd18c2810eb7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4439,6 +4439,7 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data,
struct hci_comp_pkts_info *info = &ev->handles[i];
struct hci_conn *conn;
__u16 handle, count;
+ unsigned int i;
handle = __le16_to_cpu(info->handle);
count = __le16_to_cpu(info->count);
@@ -4449,6 +4450,9 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, void *data,
conn->sent -= count;
+ for (i = 0; i < count; ++i)
+ hci_conn_tx_dequeue(conn);
+
switch (conn->type) {
case ACL_LINK:
hdev->acl_cnt += count;
@@ -4543,10 +4547,12 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
for (i = 0; i < ev->num_hndl; i++) {
struct hci_comp_blocks_info *info = &ev->handles[i];
struct hci_conn *conn = NULL;
- __u16 handle, block_count;
+ __u16 handle, block_count, pkt_count;
+ unsigned int i;
handle = __le16_to_cpu(info->handle);
block_count = __le16_to_cpu(info->blocks);
+ pkt_count = __le16_to_cpu(info->pkts);
conn = __hci_conn_lookup_handle(hdev, handle);
if (!conn)
@@ -4554,6 +4560,9 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, void *data,
conn->sent -= block_count;
+ for (i = 0; i < pkt_count; ++i)
+ hci_conn_tx_dequeue(conn);
+
switch (conn->type) {
case ACL_LINK:
case AMP_LINK:
--
2.44.0
Add socket option to disable POLLERR on non-empty socket error queue.
Applications can use this for sleeping socket error POLLERR waits, also
when using TX timestamping. This is useful when multiple layers of the
stack are using the same socket.
Signed-off-by: Pauli Virtanen <[email protected]>
---
include/net/bluetooth/bluetooth.h | 9 +++-
net/bluetooth/af_bluetooth.c | 72 ++++++++++++++++++++++++++++++-
net/bluetooth/iso.c | 8 ++--
net/bluetooth/l2cap_sock.c | 8 ++--
net/bluetooth/sco.c | 8 ++--
5 files changed, 91 insertions(+), 14 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9280e72093ee..dcee5384d715 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -240,6 +240,8 @@ struct bt_codecs {
#define BT_ISO_BASE 20
+#define BT_NO_ERRQUEUE_POLL 21
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
@@ -387,7 +389,8 @@ struct bt_sock {
enum {
BT_SK_DEFER_SETUP,
BT_SK_SUSPEND,
- BT_SK_PKT_STATUS
+ BT_SK_PKT_STATUS,
+ BT_SK_NO_ERRQUEUE_POLL
};
struct bt_sock_list {
@@ -410,6 +413,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
size_t len, int flags);
__poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+ sockptr_t optval, unsigned int optlen);
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+ char __user *optval, int __user *optlen);
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 67604ccec2f4..997197aa7b48 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -500,6 +500,12 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
return 0;
}
+static bool bt_sock_error_queue_poll(struct sock *sk)
+{
+ return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
+ !skb_queue_empty_lockless(&sk->sk_error_queue);
+}
+
__poll_t bt_sock_poll(struct file *file, struct socket *sock,
poll_table *wait)
{
@@ -511,7 +517,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
if (sk->sk_state == BT_LISTEN)
return bt_accept_poll(sk);
- if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
+ if (sk->sk_err || bt_sock_error_queue_poll(sk))
mask |= EPOLLERR |
(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
@@ -582,6 +588,70 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(bt_sock_ioctl);
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+ sockptr_t optval, unsigned int optlen)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ u32 opt;
+
+ if (level != SOL_BLUETOOTH)
+ return -ENOPROTOOPT;
+
+ lock_sock(sk);
+
+ switch (optname) {
+ case BT_NO_ERRQUEUE_POLL:
+ if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opt)
+ set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ else
+ clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ break;
+
+ default:
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ release_sock(sk);
+ return err;
+}
+EXPORT_SYMBOL(bt_sock_setsockopt);
+
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+ char __user *optval, int __user *optlen)
+{
+ struct sock *sk = sock->sk;
+ int err = 0;
+ u32 opt;
+
+ if (level != SOL_BLUETOOTH)
+ return -ENOPROTOOPT;
+
+ lock_sock(sk);
+
+ switch (optname) {
+ case BT_NO_ERRQUEUE_POLL:
+ opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+ if (put_user(opt, (u32 __user *)optval))
+ err = -EFAULT;
+ break;
+
+ default:
+ err = -ENOPROTOOPT;
+ break;
+ }
+
+ release_sock(sk);
+ return err;
+}
+EXPORT_SYMBOL(bt_sock_getsockopt);
+
/* This function expects the sk lock to be held when called */
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
{
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index a77ab9df7994..6f5af549a8cc 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1596,8 +1596,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
@@ -1667,8 +1667,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 9a9f933a748b..06277ce1fd6b 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -697,8 +697,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
@@ -1102,8 +1102,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index b3c2af7c7d67..b8b1b314aed4 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_setsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
@@ -1211,8 +1211,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
break;
default:
- err = -ENOPROTOOPT;
- break;
+ release_sock(sk);
+ return bt_sock_getsockopt(sock, level, optname, optval, optlen);
}
release_sock(sk);
--
2.44.0
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=835394
---Test result---
Test Summary:
CheckPatch PASS 5.54 seconds
GitLint PASS 1.64 seconds
SubjectPrefix PASS 0.60 seconds
BuildKernel PASS 28.34 seconds
CheckAllWarning PASS 30.80 seconds
CheckSparse WARNING 37.07 seconds
CheckSmatch WARNING 98.95 seconds
BuildKernel32 PASS 27.15 seconds
TestRunnerSetup PASS 508.13 seconds
TestRunner_l2cap-tester PASS 18.01 seconds
TestRunner_iso-tester PASS 32.23 seconds
TestRunner_bnep-tester PASS 4.71 seconds
TestRunner_mgmt-tester FAIL 110.32 seconds
TestRunner_rfcomm-tester PASS 7.29 seconds
TestRunner_sco-tester PASS 14.87 seconds
TestRunner_ioctl-tester PASS 7.75 seconds
TestRunner_mesh-tester PASS 5.83 seconds
TestRunner_smp-tester PASS 6.84 seconds
TestRunner_userchan-tester PASS 4.86 seconds
IncrementalBuild PASS 89.80 seconds
Details
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structuresnet/bluetooth/af_bluetooth.c:223:25: warning: context imbalance in 'bt_accept_enqueue' - different lock contexts for basic blocknet/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structuresnet/bluetooth/sco.c: note: in included file:./include/net/bluetooth/hci_core.h:150:35: warning: array of flexible structures
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2
Failed Test Cases
LL Privacy - Start Discovery 2 (Disable RL) Failed 0.165 seconds
---
Regards,
Linux Bluetooth
Hi Pauli,
On Thu, Mar 14, 2024 at 2:23 PM Pauli Virtanen <[email protected]> wrote:
>
> Add socket option to disable POLLERR on non-empty socket error queue.
>
> Applications can use this for sleeping socket error POLLERR waits, also
> when using TX timestamping. This is useful when multiple layers of the
> stack are using the same socket.
So the idea here is that bluetoothd would set BT_NO_ERRQUEUE_POLL to
not be awakened by timestamps? I wonder why this works on per
fd/socket level while SO_TIMESTAMPING does apply to all sockets
though, or perhaps that is not really related to SO_TIMESTAMPING but
errqueue being the same to all cloned sockets? I'm a little hesitant
to start introducing new socket options if the whole point was to
avoid doing so with the use of SO_TIMESTAMPING.
> Signed-off-by: Pauli Virtanen <[email protected]>
> ---
> include/net/bluetooth/bluetooth.h | 9 +++-
> net/bluetooth/af_bluetooth.c | 72 ++++++++++++++++++++++++++++++-
> net/bluetooth/iso.c | 8 ++--
> net/bluetooth/l2cap_sock.c | 8 ++--
> net/bluetooth/sco.c | 8 ++--
> 5 files changed, 91 insertions(+), 14 deletions(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9280e72093ee..dcee5384d715 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -240,6 +240,8 @@ struct bt_codecs {
>
> #define BT_ISO_BASE 20
>
> +#define BT_NO_ERRQUEUE_POLL 21
> +
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> @@ -387,7 +389,8 @@ struct bt_sock {
> enum {
> BT_SK_DEFER_SETUP,
> BT_SK_SUSPEND,
> - BT_SK_PKT_STATUS
> + BT_SK_PKT_STATUS,
> + BT_SK_NO_ERRQUEUE_POLL
> };
>
> struct bt_sock_list {
> @@ -410,6 +413,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> size_t len, int flags);
> __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> + sockptr_t optval, unsigned int optlen);
> +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> + char __user *optval, int __user *optlen);
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
>
> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> index 67604ccec2f4..997197aa7b48 100644
> --- a/net/bluetooth/af_bluetooth.c
> +++ b/net/bluetooth/af_bluetooth.c
> @@ -500,6 +500,12 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
> return 0;
> }
>
> +static bool bt_sock_error_queue_poll(struct sock *sk)
> +{
> + return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
> + !skb_queue_empty_lockless(&sk->sk_error_queue);
> +}
> +
> __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> poll_table *wait)
> {
> @@ -511,7 +517,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> if (sk->sk_state == BT_LISTEN)
> return bt_accept_poll(sk);
>
> - if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
> + if (sk->sk_err || bt_sock_error_queue_poll(sk))
> mask |= EPOLLERR |
> (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
>
> @@ -582,6 +588,70 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> }
> EXPORT_SYMBOL(bt_sock_ioctl);
>
> +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> + sockptr_t optval, unsigned int optlen)
> +{
> + struct sock *sk = sock->sk;
> + int err = 0;
> + u32 opt;
> +
> + if (level != SOL_BLUETOOTH)
> + return -ENOPROTOOPT;
> +
> + lock_sock(sk);
> +
> + switch (optname) {
> + case BT_NO_ERRQUEUE_POLL:
> + if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
> + err = -EFAULT;
> + break;
> + }
> +
> + if (opt)
> + set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + else
> + clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + break;
> +
> + default:
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + release_sock(sk);
> + return err;
> +}
> +EXPORT_SYMBOL(bt_sock_setsockopt);
> +
> +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> + char __user *optval, int __user *optlen)
> +{
> + struct sock *sk = sock->sk;
> + int err = 0;
> + u32 opt;
> +
> + if (level != SOL_BLUETOOTH)
> + return -ENOPROTOOPT;
> +
> + lock_sock(sk);
> +
> + switch (optname) {
> + case BT_NO_ERRQUEUE_POLL:
> + opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> + if (put_user(opt, (u32 __user *)optval))
> + err = -EFAULT;
> + break;
> +
> + default:
> + err = -ENOPROTOOPT;
> + break;
> + }
> +
> + release_sock(sk);
> + return err;
> +}
> +EXPORT_SYMBOL(bt_sock_getsockopt);
> +
> /* This function expects the sk lock to be held when called */
> int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> {
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index a77ab9df7994..6f5af549a8cc 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -1596,8 +1596,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1667,8 +1667,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 9a9f933a748b..06277ce1fd6b 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -697,8 +697,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1102,8 +1102,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index b3c2af7c7d67..b8b1b314aed4 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> @@ -1211,8 +1211,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> break;
>
> default:
> - err = -ENOPROTOOPT;
> - break;
> + release_sock(sk);
> + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> }
>
> release_sock(sk);
> --
> 2.44.0
>
>
--
Luiz Augusto von Dentz
Hi Luiz,
to, 2024-03-14 kello 23:18 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Thu, Mar 14, 2024 at 2:23 PM Pauli Virtanen <[email protected]> wrote:
> >
> > Add socket option to disable POLLERR on non-empty socket error queue.
> >
> > Applications can use this for sleeping socket error POLLERR waits, also
> > when using TX timestamping. This is useful when multiple layers of the
> > stack are using the same socket.
>
> So the idea here is that bluetoothd would set BT_NO_ERRQUEUE_POLL to
> not be awakened by timestamps? I wonder why this works on per
> fd/socket level while SO_TIMESTAMPING does apply to all sockets
> though, or perhaps that is not really related to SO_TIMESTAMPING but
> errqueue being the same to all cloned sockets? I'm a little hesitant
> to start introducing new socket options if the whole point was to
> avoid doing so with the use of SO_TIMESTAMPING.
It works the same as SO_TIMESTAMPING, setting BT_NO_ERRQUEUE_POLL
applies to all fds referring to the socket, so it is not per fd either.
The idea is that Pipewire sets BT_NO_ERRQUEUE_POLL before enabling
SO_TIMESTAMPING. Then nobody gets POLLERR on errqueue messages.
Instead, it reads MSG_ERRQUEUE always when sending the next packet,
instead of using POLLERR. This works fine in practice and is tested.
I'd appreciate if we can agree on the whole-stack plan.
***
For sockets vs. fds, see net/socket.c:sockfd_lookup
dup() of fd creates new int fd pointing to the same struct file &
struct socket, but not duplicate struct socket/sock, see fs/file.c:1412
Passing fds via unix socket like in DBus seems the same, see
include/net/scm.h:scm_recv_one_fd & fs/file.c:receive_fd
Indeed, you can also see this in debug logs, e.g.
[53573.676885] iso_sock_getname:1103: sock 0000000018c143a2, sk 000000002aff8d2b (from BlueZ)
[53580.831753] iso_sock_getname:1103: sock 00000000419df212, sk 00000000526df659 (from BlueZ)
...
[53592.472038] iso_sock_sendmsg:1127: sock 0000000018c143a2, sk 000000002aff8d2b (from PW)
[53592.472062] iso_sock_sendmsg:1127: sock 00000000419df212, sk 00000000526df659 (from PW)
setsockopt(), errqueue, RX queue etc. are struct socket level stuff.
>
>
> > Signed-off-by: Pauli Virtanen <[email protected]>
> > ---
> > include/net/bluetooth/bluetooth.h | 9 +++-
> > net/bluetooth/af_bluetooth.c | 72 ++++++++++++++++++++++++++++++-
> > net/bluetooth/iso.c | 8 ++--
> > net/bluetooth/l2cap_sock.c | 8 ++--
> > net/bluetooth/sco.c | 8 ++--
> > 5 files changed, 91 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index 9280e72093ee..dcee5384d715 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -240,6 +240,8 @@ struct bt_codecs {
> >
> > #define BT_ISO_BASE 20
> >
> > +#define BT_NO_ERRQUEUE_POLL 21
> > +
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > __printf(1, 2)
> > @@ -387,7 +389,8 @@ struct bt_sock {
> > enum {
> > BT_SK_DEFER_SETUP,
> > BT_SK_SUSPEND,
> > - BT_SK_PKT_STATUS
> > + BT_SK_PKT_STATUS,
> > + BT_SK_NO_ERRQUEUE_POLL
> > };
> >
> > struct bt_sock_list {
> > @@ -410,6 +413,10 @@ int bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
> > size_t len, int flags);
> > __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
> > int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
> > +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> > + sockptr_t optval, unsigned int optlen);
> > +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> > + char __user *optval, int __user *optlen);
> > int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
> > int bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
> >
> > diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
> > index 67604ccec2f4..997197aa7b48 100644
> > --- a/net/bluetooth/af_bluetooth.c
> > +++ b/net/bluetooth/af_bluetooth.c
> > @@ -500,6 +500,12 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
> > return 0;
> > }
> >
> > +static bool bt_sock_error_queue_poll(struct sock *sk)
> > +{
> > + return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
> > + !skb_queue_empty_lockless(&sk->sk_error_queue);
> > +}
> > +
> > __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> > poll_table *wait)
> > {
> > @@ -511,7 +517,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
> > if (sk->sk_state == BT_LISTEN)
> > return bt_accept_poll(sk);
> >
> > - if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
> > + if (sk->sk_err || bt_sock_error_queue_poll(sk))
> > mask |= EPOLLERR |
> > (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
> >
> > @@ -582,6 +588,70 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> > }
> > EXPORT_SYMBOL(bt_sock_ioctl);
> >
> > +int bt_sock_setsockopt(struct socket *sock, int level, int optname,
> > + sockptr_t optval, unsigned int optlen)
> > +{
> > + struct sock *sk = sock->sk;
> > + int err = 0;
> > + u32 opt;
> > +
> > + if (level != SOL_BLUETOOTH)
> > + return -ENOPROTOOPT;
> > +
> > + lock_sock(sk);
> > +
> > + switch (optname) {
> > + case BT_NO_ERRQUEUE_POLL:
> > + if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
> > + err = -EFAULT;
> > + break;
> > + }
> > +
> > + if (opt)
> > + set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + else
> > + clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + break;
> > +
> > + default:
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + release_sock(sk);
> > + return err;
> > +}
> > +EXPORT_SYMBOL(bt_sock_setsockopt);
> > +
> > +int bt_sock_getsockopt(struct socket *sock, int level, int optname,
> > + char __user *optval, int __user *optlen)
> > +{
> > + struct sock *sk = sock->sk;
> > + int err = 0;
> > + u32 opt;
> > +
> > + if (level != SOL_BLUETOOTH)
> > + return -ENOPROTOOPT;
> > +
> > + lock_sock(sk);
> > +
> > + switch (optname) {
> > + case BT_NO_ERRQUEUE_POLL:
> > + opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
> > + if (put_user(opt, (u32 __user *)optval))
> > + err = -EFAULT;
> > + break;
> > +
> > + default:
> > + err = -ENOPROTOOPT;
> > + break;
> > + }
> > +
> > + release_sock(sk);
> > + return err;
> > +}
> > +EXPORT_SYMBOL(bt_sock_getsockopt);
> > +
> > /* This function expects the sk lock to be held when called */
> > int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
> > {
> > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> > index a77ab9df7994..6f5af549a8cc 100644
> > --- a/net/bluetooth/iso.c
> > +++ b/net/bluetooth/iso.c
> > @@ -1596,8 +1596,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1667,8 +1667,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index 9a9f933a748b..06277ce1fd6b 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -697,8 +697,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1102,8 +1102,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> > index b3c2af7c7d67..b8b1b314aed4 100644
> > --- a/net/bluetooth/sco.c
> > +++ b/net/bluetooth/sco.c
> > @@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_setsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > @@ -1211,8 +1211,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> > break;
> >
> > default:
> > - err = -ENOPROTOOPT;
> > - break;
> > + release_sock(sk);
> > + return bt_sock_getsockopt(sock, level, optname, optval, optlen);
> > }
> >
> > release_sock(sk);
> > --
> > 2.44.0
> >
> >
>
>
Hi Luiz,
to, 2024-03-14 kello 20:20 +0200, Pauli Virtanen kirjoitti:
> Add support for TX timestamping in ISO/SCO/L2CAP sockets.
>
> These patches allow fixing / working around controller(?) issue where
> two ISO streams in same group get desynchronized. It also gives user
> applications the best latency information as available to kernel.
>
> Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR
> wakeup on TX timestamp arrival, which is mainly a nuisance in the use
> case here. The alternative to this seems be to deal with the POLLERR
> wakeups in BlueZ side, but this seems hard as it's always enabled in
> poll() flags so not clear if anything else than polling at regular
> intervals can be done there.
Any suggestions what the plan here should be?
The suggestions so far:
1. Socket TX timestamping & deal with POLLERR in BlueZ
2. Socket TX timestamping & disable POLLERR via setsockopt
3. Some custom latency reporting mechanism
> Pipewire side:
> https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-ts-test2
>
> With this change, https://github.com/bluez/bluez/issues/515 is more or
> less fixed, and the sound server can figure out the total latency to
> audio rendering (tx latency + transport latency + presentation delay).
>
> For ISO, we can later use LE Read ISO TX Sync to provide hardware
> timestamps, but this requires figuring out the sequence number
> synchronization first.
>
> v2:
> - Rename *tx_comp* -> *tx*
> - Add hci_send_conn_frame() and handle all link types
> - Add SCO timestamping. Deal with no flow control -> no Num_Comp_* events
> - Handle HCI_FLOW_CTL_MODE_BLOCK_BASED
> - Add BT_NO_ERRQUEUE_POLL
>
> Pauli Virtanen (5):
> Bluetooth: add support for skb TX timestamping
> Bluetooth: ISO: add TX timestamping
> Bluetooth: L2CAP: add TX timestamping
> Bluetooth: SCO: add TX timestamping
> Bluetooth: add BT_NO_ERRQUEUE_POLL socket option
>
> include/net/bluetooth/bluetooth.h | 10 ++-
> include/net/bluetooth/hci_core.h | 12 ++++
> include/net/bluetooth/l2cap.h | 3 +-
> net/bluetooth/6lowpan.c | 2 +-
> net/bluetooth/af_bluetooth.c | 72 ++++++++++++++++++-
> net/bluetooth/hci_conn.c | 111 ++++++++++++++++++++++++++++++
> net/bluetooth/hci_core.c | 19 +++--
> net/bluetooth/hci_event.c | 11 ++-
> net/bluetooth/iso.c | 32 ++++++---
> net/bluetooth/l2cap_core.c | 11 ++-
> net/bluetooth/l2cap_sock.c | 23 +++++--
> net/bluetooth/sco.c | 27 ++++++--
> net/bluetooth/smp.c | 2 +-
> 13 files changed, 303 insertions(+), 32 deletions(-)
>
--
Pauli Virtanen
Hello:
This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Thu, 14 Mar 2024 20:20:16 +0200 you wrote:
> Add support for TX timestamping in ISO/SCO/L2CAP sockets.
>
> These patches allow fixing / working around controller(?) issue where
> two ISO streams in same group get desynchronized. It also gives user
> applications the best latency information as available to kernel.
>
> Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR
> wakeup on TX timestamp arrival, which is mainly a nuisance in the use
> case here. The alternative to this seems be to deal with the POLLERR
> wakeups in BlueZ side, but this seems hard as it's always enabled in
> poll() flags so not clear if anything else than polling at regular
> intervals can be done there.
>
> [...]
Here is the summary with links:
- [v2,1/5] Bluetooth: add support for skb TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/0368e609d090
- [v2,2/5] Bluetooth: ISO: add TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/3067d73e114f
- [v2,3/5] Bluetooth: L2CAP: add TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/e22f35ed23a7
- [v2,4/5] Bluetooth: SCO: add TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/b7adccd0e8b6
- [v2,5/5] Bluetooth: add BT_NO_ERRQUEUE_POLL socket option
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hi Pauli,
On Mon, Apr 1, 2024 at 4:00 PM <[email protected]> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluetooth-next.git (master)
> by Luiz Augusto von Dentz <[email protected]>:
>
> On Thu, 14 Mar 2024 20:20:16 +0200 you wrote:
> > Add support for TX timestamping in ISO/SCO/L2CAP sockets.
> >
> > These patches allow fixing / working around controller(?) issue where
> > two ISO streams in same group get desynchronized. It also gives user
> > applications the best latency information as available to kernel.
> >
> > Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR
> > wakeup on TX timestamp arrival, which is mainly a nuisance in the use
> > case here. The alternative to this seems be to deal with the POLLERR
> > wakeups in BlueZ side, but this seems hard as it's always enabled in
> > poll() flags so not clear if anything else than polling at regular
> > intervals can be done there.
> >
> > [...]
>
> Here is the summary with links:
> - [v2,1/5] Bluetooth: add support for skb TX timestamping
> https://git.kernel.org/bluetooth/bluetooth-next/c/0368e609d090
> - [v2,2/5] Bluetooth: ISO: add TX timestamping
> https://git.kernel.org/bluetooth/bluetooth-next/c/3067d73e114f
> - [v2,3/5] Bluetooth: L2CAP: add TX timestamping
> https://git.kernel.org/bluetooth/bluetooth-next/c/e22f35ed23a7
> - [v2,4/5] Bluetooth: SCO: add TX timestamping
> https://git.kernel.org/bluetooth/bluetooth-next/c/b7adccd0e8b6
> - [v2,5/5] Bluetooth: add BT_NO_ERRQUEUE_POLL socket option
> (no matching commit)
Ive left 5/5 out on purpose, let's have it behind an experimental flag
so we can later rework it if necessary, I know it is a little annoying
to have to do extra setup in order to test it but we are not supposed
to introduce something like this without some safeguards that we can
later rework if necessary.
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
>
--
Luiz Augusto von Dentz
Hi Luiz,
ma, 2024-04-01 kello 16:05 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Mon, Apr 1, 2024 at 4:00 PM <[email protected]> wrote:
> >
> > Hello:
> >
> > This series was applied to bluetooth/bluetooth-next.git (master)
> > by Luiz Augusto von Dentz <[email protected]>:
> >
> > On Thu, 14 Mar 2024 20:20:16 +0200 you wrote:
> > > Add support for TX timestamping in ISO/SCO/L2CAP sockets.
> > >
> > > These patches allow fixing / working around controller(?) issue where
> > > two ISO streams in same group get desynchronized. It also gives user
> > > applications the best latency information as available to kernel.
> > >
> > > Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR
> > > wakeup on TX timestamp arrival, which is mainly a nuisance in the use
> > > case here. The alternative to this seems be to deal with the POLLERR
> > > wakeups in BlueZ side, but this seems hard as it's always enabled in
> > > poll() flags so not clear if anything else than polling at regular
> > > intervals can be done there.
> > >
> > > [...]
> >
> > Here is the summary with links:
> > - [v2,1/5] Bluetooth: add support for skb TX timestamping
> > https://git.kernel.org/bluetooth/bluetooth-next/c/0368e609d090
> > - [v2,2/5] Bluetooth: ISO: add TX timestamping
> > https://git.kernel.org/bluetooth/bluetooth-next/c/3067d73e114f
> > - [v2,3/5] Bluetooth: L2CAP: add TX timestamping
> > https://git.kernel.org/bluetooth/bluetooth-next/c/e22f35ed23a7
> > - [v2,4/5] Bluetooth: SCO: add TX timestamping
> > https://git.kernel.org/bluetooth/bluetooth-next/c/b7adccd0e8b6
> > - [v2,5/5] Bluetooth: add BT_NO_ERRQUEUE_POLL socket option
> > (no matching commit)
>
> Ive left 5/5 out on purpose, let's have it behind an experimental flag
> so we can later rework it if necessary, I know it is a little annoying
> to have to do extra setup in order to test it but we are not supposed
> to introduce something like this without some safeguards that we can
> later rework if necessary.
Thanks, I'll add the experimental flag to 5/5 and resend.
--
Pauli Virtanen