From: Luiz Augusto von Dentz <[email protected]>
This adds bt_crypto_sirk which attempts to generate a unique SIRK using
the following steps:
- Generate a hash (k) using the str as input
- Generate a hash (sirk) using vendor, product, version and source as input
- Encrypt sirk using k as LTK with sef function.
---
src/shared/crypto.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/shared/crypto.h | 3 +++
2 files changed, 43 insertions(+)
diff --git a/src/shared/crypto.c b/src/shared/crypto.c
index 4cb2ea857ea8..5449621b55ea 100644
--- a/src/shared/crypto.c
+++ b/src/shared/crypto.c
@@ -926,3 +926,43 @@ bool bt_crypto_sef(struct bt_crypto *crypto, const uint8_t k[16],
return true;
}
+
+/* Generates a SIRK from a string using the following steps:
+ * - Generate a hash (k) using the str as input
+ * - Generate a hash (sirk) using vendor, product, version and source as input
+ * - Encrypt sirk using k as LTK with sef function.
+ */
+bool bt_crypto_sirk(struct bt_crypto *crypto, const char *str, uint16_t vendor,
+ uint16_t product, uint16_t version, uint16_t source,
+ uint8_t sirk[16])
+{
+ struct iovec iov[4];
+ uint8_t k[16];
+ uint8_t sirk_plaintext[16];
+
+ if (!crypto)
+ return false;
+
+ iov[0].iov_base = (void *)str;
+ iov[0].iov_len = strlen(str);
+
+ /* Generate a k using the str as input */
+ if (!bt_crypto_gatt_hash(crypto, iov, 1, k))
+ return false;
+
+ iov[0].iov_base = &vendor;
+ iov[0].iov_len = sizeof(vendor);
+ iov[1].iov_base = &product;
+ iov[1].iov_len = sizeof(product);
+ iov[2].iov_base = &version;
+ iov[2].iov_len = sizeof(version);
+ iov[3].iov_base = &source;
+ iov[3].iov_len = sizeof(source);
+
+ /* Generate a sirk using vendor, product, version and source as input */
+ if (!bt_crypto_gatt_hash(crypto, iov, 4, sirk_plaintext))
+ return false;
+
+ /* Encrypt sirk using k as LTK with sef function */
+ return bt_crypto_sef(crypto, k, sirk_plaintext, sirk);
+}
diff --git a/src/shared/crypto.h b/src/shared/crypto.h
index fc1ba0c4feeb..d45308abf90a 100644
--- a/src/shared/crypto.h
+++ b/src/shared/crypto.h
@@ -57,3 +57,6 @@ bool bt_crypto_sef(struct bt_crypto *crypto, const uint8_t k[16],
const uint8_t sirk[16], uint8_t out[16]);
bool bt_crypto_sih(struct bt_crypto *crypto, const uint8_t k[16],
const uint8_t r[3], uint8_t hash[3]);
+bool bt_crypto_sirk(struct bt_crypto *crypto, const char *str, uint16_t vendor,
+ uint16_t product, uint16_t version, uint16_t source,
+ uint8_t sirk[16]);
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This adds BT_AD_CSIP_RSI advertising data type.
---
src/shared/ad.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/shared/ad.h b/src/shared/ad.h
index feb712f508cf..b100a6796109 100644
--- a/src/shared/ad.h
+++ b/src/shared/ad.h
@@ -57,6 +57,7 @@
#define BT_AD_MESH_PROV 0x29
#define BT_AD_MESH_DATA 0x2a
#define BT_AD_MESH_BEACON 0x2b
+#define BT_AD_CSIP_RSI 0x2e
#define BT_AD_3D_INFO_DATA 0x3d
#define BT_AD_MANUFACTURER_DATA 0xff
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This adds set-api.rst which documents DeviceSet interface.
---
doc/set-api.rst | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 doc/set-api.rst
diff --git a/doc/set-api.rst b/doc/set-api.rst
new file mode 100644
index 000000000000..c49be1ae0514
--- /dev/null
+++ b/doc/set-api.rst
@@ -0,0 +1,53 @@
+=====================================
+BlueZ D-Bus DeviceSet API description
+=====================================
+
+
+DeviceSet interface
+===================
+
+Service org.bluez
+Interface org.bluez.DeviceSet1
+Object path [variable prefix]/{hci0,hci1,...}/set_{sirk}
+
+Methods
+=======
+
+**void Connect() [experimental]**
+
+ Connects all **devices** members of the set, each member is
+ connected in sequence as they were added/loaded following the
+ same proceedure as described in **Device1.Connect**.
+
+ Possible errors: org.bluez.Error.NotReady
+ org.bluez.Error.Failed
+ org.bluez.Error.InProgress
+ org.bluez.Error.AlreadyConnected
+
+**void Disconnect() [experimental]**
+
+ Disconnects all **devices** members of the set, each member is
+ disconnected in sequence as they were connected following the
+ same proceedure as described in **Device1.Disconnect**.
+
+ Possible errors: org.bluez.Error.NotConnected
+
+Properties
+==========
+
+**object Adapter [readonly]**
+
+ The object path of the adapter the set belongs to.
+
+**bool AutoConnect [read-write, experimental]**
+
+ Indicates if the **devices** members of the set shall be automatically
+ connected once any of its members is connected.
+
+**array(object) Devices [ready-only, experimental]**
+
+ List of devices objects that are members of the set.
+
+**byte Size [read-only, experimental]**
+
+ Set members size.
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This adds Set property so clients are able to identify when a device
belongs to a set.
---
doc/device-api.txt | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 628accb8a572..e4a8d3c5af33 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -281,3 +281,13 @@ Properties string Address [readonly]
Example:
<Transport Discovery> <Organization Flags...>
0x26 0x01 0x01...
+
+ array{object, dict} Sets [readonly, experimental]
+
+ The object paths of the sets the device belongs to
+ followed by a dictionary which can contain the
+ following:
+
+ byte Rank:
+
+ Rank of the device in the Set.
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This adds the initial implementation of DeviceSet interface as
documented in doc/set-api.rst.
---
Makefile.am | 3 +-
src/adapter.c | 6 +-
src/device.c | 283 +++++++++++++++++++++++++++++++++++++-
src/device.h | 11 +-
src/set.c | 371 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/set.h | 20 +++
6 files changed, 683 insertions(+), 11 deletions(-)
create mode 100644 src/set.c
create mode 100644 src/set.h
diff --git a/Makefile.am b/Makefile.am
index 0e8cce249c7d..7b010c8159e9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -326,7 +326,8 @@ src_bluetoothd_SOURCES = $(builtin_sources) \
src/eir.h src/eir.c \
src/adv_monitor.h src/adv_monitor.c \
src/battery.h src/battery.c \
- src/settings.h src/settings.c
+ src/settings.h src/settings.c \
+ src/set.h src/set.c
src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
gdbus/libgdbus-internal.la \
src/libshared-glib.la \
diff --git a/src/adapter.c b/src/adapter.c
index 538310c1ddc6..ae0eb364bad5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4387,8 +4387,8 @@ static void load_ltks(struct btd_adapter *adapter, GSList *keys)
if (dev) {
device_set_paired(dev, info->bdaddr_type);
device_set_bonded(dev, info->bdaddr_type);
- device_set_ltk_enc_size(dev, info->enc_size);
- device_set_ltk_enc_size(dev, info->enc_size);
+ device_set_ltk(dev, info->val, info->central,
+ info->enc_size);
}
}
@@ -8657,7 +8657,7 @@ static void new_long_term_key_callback(uint16_t index, uint16_t length,
device_set_bonded(device, addr->type);
}
- device_set_ltk_enc_size(device, ev->key.enc_size);
+ device_set_ltk(device, ev->key.val, ev->key.central, ev->key.enc_size);
bonding_complete(adapter, &addr->bdaddr, addr->type, 0);
}
diff --git a/src/device.c b/src/device.c
index cb16d37c1ae1..df50ce7b4f6c 100644
--- a/src/device.c
+++ b/src/device.c
@@ -64,6 +64,7 @@
#include "storage.h"
#include "eir.h"
#include "settings.h"
+#include "set.h"
#define DISCONNECT_TIMER 2
#define DISCOVERY_TIMER 1
@@ -159,11 +160,25 @@ struct bearer_state {
time_t last_seen;
};
+struct ltk_info {
+ uint8_t key[16];
+ bool central;
+ uint8_t enc_size;
+};
+
struct csrk_info {
uint8_t key[16];
uint32_t counter;
};
+struct sirk_info {
+ struct btd_device_set *set;
+ uint8_t encrypted;
+ uint8_t key[16];
+ uint8_t size;
+ uint8_t rank;
+};
+
enum {
WAKE_FLAG_DEFAULT = 0,
WAKE_FLAG_ENABLED,
@@ -253,7 +268,8 @@ struct btd_device {
struct csrk_info *local_csrk;
struct csrk_info *remote_csrk;
- uint8_t ltk_enc_size;
+ struct ltk_info *ltk;
+ struct queue *sirks;
sdp_list_t *tmp_records;
@@ -386,6 +402,24 @@ static void store_csrk(struct csrk_info *csrk, GKeyFile *key_file,
g_key_file_set_integer(key_file, group, "Counter", csrk->counter);
}
+static void store_sirk(struct sirk_info *sirk, GKeyFile *key_file,
+ uint8_t index)
+{
+ char group[28];
+ char key[33];
+ int i;
+
+ sprintf(group, "SetIdentityResolvingKey#%u", index);
+
+ for (i = 0; i < 16; i++)
+ sprintf(key + (i * 2), "%2.2X", sirk->key[i]);
+
+ g_key_file_set_boolean(key_file, group, "Encrypted", sirk->encrypted);
+ g_key_file_set_string(key_file, group, "Key", key);
+ g_key_file_set_integer(key_file, group, "Size", sirk->size);
+ g_key_file_set_integer(key_file, group, "Rank", sirk->rank);
+}
+
static gboolean store_device_info_cb(gpointer user_data)
{
struct btd_device *device = user_data;
@@ -483,6 +517,18 @@ static gboolean store_device_info_cb(gpointer user_data)
if (device->remote_csrk)
store_csrk(device->remote_csrk, key_file, "RemoteSignatureKey");
+ if (!queue_isempty(device->sirks)) {
+ const struct queue_entry *entry;
+ int i;
+
+ for (entry = queue_get_entries(device->sirks), i = 0; entry;
+ entry = entry->next, i++) {
+ struct sirk_info *sirk = entry->data;
+
+ store_sirk(sirk, key_file, i);
+ }
+ }
+
str = g_key_file_to_data(key_file, &length, NULL);
if (!g_file_set_contents(filename, str, length, &gerr)) {
error("Unable set contents for %s: (%s)", filename,
@@ -804,8 +850,11 @@ static void device_free(gpointer user_data)
if (device->eir_uuids)
g_slist_free_full(device->eir_uuids, g_free);
+ queue_destroy(device->sirks, free);
+
g_free(device->local_csrk);
g_free(device->remote_csrk);
+ free(device->ltk);
g_free(device->path);
g_free(device->alias);
free(device->modalias);
@@ -1607,6 +1656,62 @@ static gboolean dev_property_wake_allowed_exist(
return device_get_wake_support(device);
}
+static void append_set(void *data, void *user_data)
+{
+ struct sirk_info *info = data;
+ const char *path = btd_set_get_path(info->set);
+ DBusMessageIter *iter = user_data;
+ DBusMessageIter entry, dict;
+
+ dbus_message_iter_open_container(iter, DBUS_TYPE_DICT_ENTRY, NULL,
+ &entry);
+
+ dbus_message_iter_append_basic(&entry, DBUS_TYPE_OBJECT_PATH, &path);
+
+ dbus_message_iter_open_container(&entry, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING
+ DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
+
+ g_dbus_dict_append_entry(&dict, "Rank", DBUS_TYPE_BYTE, &info->rank);
+
+ dbus_message_iter_close_container(&entry, &dict);
+ dbus_message_iter_close_container(iter, &entry);
+}
+
+static gboolean dev_property_get_set(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_device *device = data;
+ DBusMessageIter array;
+
+ dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_OBJECT_PATH_AS_STRING
+ DBUS_TYPE_ARRAY_AS_STRING
+ DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+ DBUS_TYPE_STRING_AS_STRING
+ DBUS_TYPE_VARIANT_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+ DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
+ &array);
+
+ queue_foreach(device->sirks, append_set, &array);
+
+ dbus_message_iter_close_container(iter, &array);
+
+ return TRUE;
+}
+
+static gboolean dev_property_set_exists(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct btd_device *device = data;
+
+ return !queue_isempty(device->sirks);
+}
+
static bool disconnect_all(gpointer user_data)
{
struct btd_device *device = user_data;
@@ -1792,10 +1897,97 @@ bool device_is_disconnecting(struct btd_device *device)
return device->disconn_timer > 0;
}
-void device_set_ltk_enc_size(struct btd_device *device, uint8_t enc_size)
+static void add_set(void *data, void *user_data)
{
- device->ltk_enc_size = enc_size;
- bt_att_set_enc_key_size(device->att, device->ltk_enc_size);
+ struct sirk_info *sirk = data;
+ struct btd_device *device = user_data;
+ struct btd_device_set *set;
+
+ if (!sirk->encrypted)
+ return;
+
+ set = btd_set_add_device(device, device->ltk->key, sirk->key,
+ sirk->size);
+ if (!set)
+ return;
+
+ if (sirk->set != set) {
+ sirk->set = set;
+ g_dbus_emit_property_changed(dbus_conn, device->path,
+ DEVICE_INTERFACE, "Sets");
+ }
+}
+
+void device_set_ltk(struct btd_device *device, const uint8_t val[16],
+ bool central, uint8_t enc_size)
+{
+ if (!device->ltk)
+ device->ltk = new0(struct ltk_info, 1);
+
+ memcpy(device->ltk->key, val, sizeof(device->ltk->key));
+ device->ltk->central = central;
+ device->ltk->enc_size = enc_size;
+ bt_att_set_enc_key_size(device->att, enc_size);
+
+ /* Check if there is any set/sirk that needs decryption */
+ queue_foreach(device->sirks, add_set, device);
+}
+
+static bool match_sirk(const void *data, const void *match_data)
+{
+ const struct sirk_info *sirk = data;
+ const uint8_t *key = match_data;
+
+ return !memcmp(sirk->key, key, sizeof(sirk->key));
+}
+
+static struct sirk_info *device_add_sirk_info(struct btd_device *device,
+ bool encrypted, uint8_t key[16],
+ uint8_t size, uint8_t rank)
+{
+ struct sirk_info *sirk;
+
+ sirk = queue_find(device->sirks, match_sirk, key);
+ if (sirk)
+ return sirk;
+
+ sirk = new0(struct sirk_info, 1);
+ sirk->encrypted = encrypted;
+ memcpy(sirk->key, key, sizeof(sirk->key));
+ sirk->size = size;
+ sirk->rank = rank;
+
+ queue_push_tail(device->sirks, sirk);
+ store_device_info(device);
+
+ return sirk;
+}
+
+bool btd_device_add_set(struct btd_device *device, bool encrypted,
+ uint8_t key[16], uint8_t size, uint8_t rank)
+{
+ struct btd_device_set *set;
+ struct sirk_info *sirk;
+
+ if (encrypted && !device->ltk)
+ return false;
+
+ sirk = device_add_sirk_info(device, encrypted, key, size, rank);
+ if (!sirk)
+ return false;
+
+ set = btd_set_add_device(device, encrypted ? device->ltk->key : NULL,
+ key, size);
+ if (!set)
+ return false;
+
+ if (sirk->set != set) {
+ sirk->set = set;
+ g_dbus_emit_property_changed(dbus_conn, device->path,
+ DEVICE_INTERFACE, "Sets");
+ }
+
+ return true;
}
static void device_set_auto_connect(struct btd_device *device, gboolean enable)
@@ -2996,6 +3188,8 @@ static const GDBusPropertyTable device_properties[] = {
{ "WakeAllowed", "b", dev_property_get_wake_allowed,
dev_property_set_wake_allowed,
dev_property_wake_allowed_exist },
+ { "Sets", "a{oa{sv}}", dev_property_get_set, NULL,
+ dev_property_set_exists },
{ }
};
@@ -3290,6 +3484,63 @@ fail:
return NULL;
}
+static struct sirk_info *load_sirk(GKeyFile *key_file, uint8_t index)
+{
+ char group[28];
+ struct sirk_info *sirk;
+ char *str;
+ int i;
+
+ sprintf(group, "SetIdentityResolvingKey#%u", index);
+
+ str = g_key_file_get_string(key_file, group, "Key", NULL);
+ if (!str)
+ return NULL;
+
+ sirk = g_new0(struct sirk_info, 1);
+
+ for (i = 0; i < 16; i++) {
+ if (sscanf(str + (i * 2), "%2hhx", &sirk->key[i]) != 1)
+ goto fail;
+ }
+
+
+ sirk->encrypted = g_key_file_get_boolean(key_file, group, "Encrypted",
+ NULL);
+ sirk->size = g_key_file_get_integer(key_file, group, "Size", NULL);
+ sirk->rank = g_key_file_get_integer(key_file, group, "Rank", NULL);
+ g_free(str);
+
+ return sirk;
+
+fail:
+ g_free(str);
+ g_free(sirk);
+ return NULL;
+}
+
+static void load_sirks(struct btd_device *device, GKeyFile *key_file)
+{
+ struct sirk_info *sirk;
+ uint8_t i;
+
+ for (i = 0; i < UINT8_MAX; i++) {
+ sirk = load_sirk(key_file, i);
+ if (!sirk)
+ break;
+
+ queue_push_tail(device->sirks, sirk);
+
+ /* Only add DeviceSet object if sirk does need
+ * decryption otherwise it has to wait for the LTK in
+ * order to decrypt.
+ */
+ if (!sirk->encrypted)
+ btd_set_add_device(device, NULL, sirk->key,
+ sirk->size);
+ }
+}
+
static void load_services(struct btd_device *device, char **uuids)
{
char **uuid;
@@ -3430,6 +3681,8 @@ static void load_info(struct btd_device *device, const char *local,
device->local_csrk = load_csrk(key_file, "LocalSignatureKey");
device->remote_csrk = load_csrk(key_file, "RemoteSignatureKey");
+
+ load_sirks(device, key_file);
}
g_strfreev(techno);
@@ -3945,6 +4198,7 @@ static struct btd_device *device_new(struct btd_adapter *adapter,
}
device->adapter = adapter;
+ device->sirks = queue_new();
device->temporary = true;
device->db_id = gatt_db_register(device->db, gatt_service_added,
@@ -5207,7 +5461,9 @@ static void gatt_server_init(struct btd_device *device,
return;
}
- bt_att_set_enc_key_size(device->att, device->ltk_enc_size);
+ if (device->ltk)
+ bt_att_set_enc_key_size(device->att, device->ltk->enc_size);
+
bt_gatt_server_set_debug(device->server, gatt_debug, NULL, NULL);
btd_gatt_database_server_connected(database, device->server);
@@ -6760,6 +7016,14 @@ struct btd_device *btd_device_ref(struct btd_device *device)
return device;
}
+static void remove_sirk_info(void *data, void *user_data)
+{
+ struct sirk_info *info = data;
+ struct btd_device *device = user_data;
+
+ btd_set_remove_device(info->set, device);
+}
+
void btd_device_unref(struct btd_device *device)
{
if (__sync_sub_and_fetch(&device->ref_count, 1))
@@ -6770,6 +7034,9 @@ void btd_device_unref(struct btd_device *device)
return;
}
+ if (!queue_isempty(device->sirks))
+ queue_foreach(device->sirks, remove_sirk_info, device);
+
DBG("Freeing device %s", device->path);
g_dbus_unregister_interface(dbus_conn, device->path, DEVICE_INTERFACE);
@@ -6928,3 +7195,9 @@ int8_t btd_device_get_volume(struct btd_device *device)
{
return device->volume;
}
+
+void btd_device_foreach_ad(struct btd_device *dev, bt_ad_func_t func,
+ void *data)
+{
+ bt_ad_foreach_data(dev->ad, func, data);
+}
diff --git a/src/device.h b/src/device.h
index 9e81fda9e948..96347ff229cc 100644
--- a/src/device.h
+++ b/src/device.h
@@ -128,8 +128,10 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type,
bool *remove);
void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
bool device_is_disconnecting(struct btd_device *device);
-void device_set_ltk_enc_size(struct btd_device *device, uint8_t enc_size);
-
+void device_set_ltk(struct btd_device *device, const uint8_t val[16],
+ bool central, uint8_t enc_size);
+bool btd_device_add_set(struct btd_device *device, bool encrypted,
+ uint8_t sirk[16], uint8_t size, uint8_t rank);
void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
uint16_t value);
void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
@@ -188,3 +190,8 @@ void btd_device_cleanup(void);
void btd_device_set_volume(struct btd_device *dev, int8_t volume);
int8_t btd_device_get_volume(struct btd_device *dev);
+
+typedef void (*bt_device_ad_func_t)(void *data, void *user_data);
+
+void btd_device_foreach_ad(struct btd_device *dev, bt_device_ad_func_t func,
+ void *data);
diff --git a/src/set.c b/src/set.c
new file mode 100644
index 000000000000..5836a06b887a
--- /dev/null
+++ b/src/set.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2023 Intel Corporation
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <dbus/dbus.h>
+
+#include "gdbus/gdbus.h"
+#include "src/shared/util.h"
+#include "src/shared/queue.h"
+#include "src/shared/ad.h"
+#include "src/shared/crypto.h"
+
+#include "log.h"
+#include "error.h"
+#include "adapter.h"
+#include "device.h"
+#include "dbus-common.h"
+#include "set.h"
+
+static struct queue *set_list;
+
+struct btd_device_set {
+ struct btd_adapter *adapter;
+ char *path;
+ uint8_t sirk[16];
+ uint8_t size;
+ bool auto_connect;
+ struct queue *devices;
+ struct btd_device *device;
+};
+
+static DBusMessage *set_disconnect(DBusConnection *conn, DBusMessage *msg,
+ void *user_data)
+{
+ /* TODO */
+ return NULL;
+}
+
+static DBusMessage *set_connect(DBusConnection *conn, DBusMessage *msg,
+ void *user_data)
+{
+ /* TODO */
+ return NULL;
+}
+
+static const GDBusMethodTable set_methods[] = {
+ { GDBUS_EXPERIMENTAL_ASYNC_METHOD("Disconnect", NULL, NULL,
+ set_disconnect) },
+ { GDBUS_EXPERIMENTAL_ASYNC_METHOD("Connect", NULL, NULL,
+ set_connect) },
+ {}
+};
+
+static gboolean get_adapter(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_device_set *set = data;
+ const char *path = adapter_get_path(set->adapter);
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
+
+ return TRUE;
+}
+
+static gboolean get_auto_connect(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_device_set *set = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN,
+ &set->auto_connect);
+
+ return TRUE;
+}
+
+static void set_auto_connect(const GDBusPropertyTable *property,
+ DBusMessageIter *iter,
+ GDBusPendingPropertySet id, void *data)
+{
+ struct btd_device_set *set = data;
+ dbus_bool_t b;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_BOOLEAN) {
+ g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
+ return;
+ }
+
+ dbus_message_iter_get_basic(iter, &b);
+
+ set->auto_connect = b ? true : false;
+
+ g_dbus_pending_property_success(id);
+}
+
+static void append_device(void *data, void *user_data)
+{
+ struct btd_device *device = data;
+ const char *path = device_get_path(device);
+ DBusMessageIter *entry = user_data;
+
+ dbus_message_iter_append_basic(entry, DBUS_TYPE_OBJECT_PATH, &path);
+}
+
+static gboolean get_devices(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_device_set *set = data;
+ DBusMessageIter entry;
+
+ dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+ DBUS_TYPE_OBJECT_PATH_AS_STRING,
+ &entry);
+
+ queue_foreach(set->devices, append_device, &entry);
+
+ dbus_message_iter_close_container(iter, &entry);
+
+ return TRUE;
+}
+
+static gboolean get_size(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct btd_device_set *set = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &set->size);
+
+ return TRUE;
+}
+
+static const GDBusPropertyTable set_properties[] = {
+ { "Adapter", "o", get_adapter, NULL, NULL,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "AutoConnect", "b", get_auto_connect, set_auto_connect, NULL,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Devices", "ao", get_devices, NULL, NULL,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ { "Size", "y", get_size, NULL, NULL,
+ G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+ {}
+};
+
+static void set_free(void *data)
+{
+ struct btd_device_set *set = data;
+
+ queue_destroy(set->devices, NULL);
+ g_free(set->path);
+ free(set);
+}
+
+static struct btd_device_set *set_new(struct btd_device *device,
+ uint8_t sirk[16], uint8_t size)
+{
+ struct btd_device_set *set;
+
+ set = new0(struct btd_device_set, 1);
+ set->adapter = device_get_adapter(device);
+ memcpy(set->sirk, sirk, sizeof(set->sirk));
+ set->size = size;
+ set->auto_connect = true;
+ set->devices = queue_new();
+ queue_push_tail(set->devices, device);
+ set->path = g_strdup_printf("%s/set_%02x%02x%02x%02x%02x%02x%02x%02x"
+ "%02x%02x%02x%02x%02x%02x%02x%02x",
+ adapter_get_path(set->adapter),
+ sirk[15], sirk[14], sirk[13], sirk[12],
+ sirk[11], sirk[10], sirk[9], sirk[8],
+ sirk[7], sirk[6], sirk[5], sirk[4],
+ sirk[3], sirk[2], sirk[1], sirk[0]);
+
+ DBG("Creating set %s", set->path);
+
+ if (g_dbus_register_interface(btd_get_dbus_connection(),
+ set->path, BTD_DEVICE_SET_INTERFACE,
+ set_methods, NULL,
+ set_properties, set,
+ set_free) == FALSE) {
+ error("Unable to register set interface");
+ set_free(set);
+ return NULL;
+ }
+
+ return set;
+}
+
+static struct btd_device_set *set_find(struct btd_device *device,
+ uint8_t sirk[16])
+{
+ struct btd_adapter *adapter = device_get_adapter(device);
+ const struct queue_entry *entry;
+
+ for (entry = queue_get_entries(set_list); entry; entry = entry->next) {
+ struct btd_device_set *set = entry->data;
+
+ if (set->adapter != adapter)
+ continue;
+
+ if (!memcmp(set->sirk, sirk, sizeof(set->sirk)))
+ return set;
+ }
+
+ return NULL;
+}
+
+static void set_connect_next(struct btd_device_set *set)
+{
+ const struct queue_entry *entry;
+
+ for (entry = queue_get_entries(set->devices); entry;
+ entry = entry->next) {
+ struct btd_device *device = entry->data;
+
+ /* Only connect one at time(?) */
+ if (!device_connect_le(device))
+ return;
+ }
+}
+
+static void set_add(struct btd_device_set *set, struct btd_device *device)
+{
+ /* Check if device is already part of the set then skip to connect */
+ if (queue_find(set->devices, NULL, device))
+ goto done;
+
+ DBG("set %s device %s", set->path, device_get_path(device));
+
+ queue_push_tail(set->devices, device);
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), set->path,
+ BTD_DEVICE_SET_INTERFACE, "Devices");
+
+done:
+ /* Check if set is marked to auto-connect */
+ if (btd_device_is_connected(device) && set->auto_connect)
+ set_connect_next(set);
+}
+
+static void foreach_rsi(void *data, void *user_data)
+{
+ struct bt_ad_data *ad = data;
+ struct btd_device_set *set = user_data;
+ struct bt_crypto *crypto;
+ uint8_t res[3];
+
+ if (ad->type != BT_AD_CSIP_RSI || ad->len < 6)
+ return;
+
+ crypto = bt_crypto_new();
+ if (!crypto)
+ return;
+
+ if (!bt_crypto_sih(crypto, set->sirk, ad->data + 3, res)) {
+ bt_crypto_unref(crypto);
+ return;
+ }
+
+ bt_crypto_unref(crypto);
+
+ if (!memcmp(ad->data, res, sizeof(res))) {
+ btd_device_set_temporary(set->device, false);
+ device_connect_le(set->device);
+ }
+}
+
+static void foreach_device(struct btd_device *device, void *data)
+{
+ struct btd_device_set *set = data;
+
+ /* Check if device is already part of the set then skip */
+ if (queue_find(set->devices, NULL, device))
+ return;
+
+ set->device = device;
+
+ btd_device_foreach_ad(device, foreach_rsi, set);
+}
+
+struct btd_device_set *btd_set_add_device(struct btd_device *device,
+ uint8_t *key, uint8_t sirk[16],
+ uint8_t size)
+{
+ struct btd_device_set *set;
+
+ /* In case key has been set it means SIRK is encrypted */
+ if (key) {
+ struct bt_crypto *crypto = bt_crypto_new();
+
+ if (!crypto)
+ return NULL;
+
+ /* sef and sdf are symmetric */
+ bt_crypto_sef(crypto, key, sirk, sirk);
+
+ bt_crypto_unref(crypto);
+ }
+
+ /* Check if DeviceSet already exists */
+ set = set_find(device, sirk);
+ if (set) {
+ set_add(set, device);
+ return set;
+ }
+
+ set = set_new(device, sirk, size);
+ if (!set)
+ return NULL;
+
+ if (!set_list)
+ set_list = queue_new();
+
+ queue_push_tail(set_list, set);
+
+ /* Attempt to add devices which have matching RSI */
+ btd_adapter_for_each_device(device_get_adapter(device), foreach_device,
+ set);
+
+ return set;
+}
+
+bool btd_set_remove_device(struct btd_device_set *set,
+ struct btd_device *device)
+{
+ if (!set || !device)
+ return false;
+
+ if (!queue_remove_if(set->devices, NULL, device))
+ return false;
+
+ if (!queue_isempty(set->devices)) {
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ set->path,
+ BTD_DEVICE_SET_INTERFACE,
+ "Devices");
+ return true;
+ }
+
+ if (!queue_remove(set_list, set))
+ return false;
+
+ /* Unregister if there are no devices left in the set */
+ g_dbus_unregister_interface(btd_get_dbus_connection(), set->path,
+ BTD_DEVICE_SET_INTERFACE);
+
+ return true;
+}
+
+const char *btd_set_get_path(struct btd_device_set *set)
+{
+ return set->path;
+}
diff --git a/src/set.h b/src/set.h
new file mode 100644
index 000000000000..67177e8c7946
--- /dev/null
+++ b/src/set.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2023 Intel Corporation
+ *
+ *
+ */
+
+#define BTD_DEVICE_SET_INTERFACE "org.bluez.DeviceSet1"
+
+struct btd_device_set;
+
+struct btd_device_set *btd_set_add_device(struct btd_device *device,
+ uint8_t *ltk, uint8_t sirk[16],
+ uint8_t size);
+bool btd_set_remove_device(struct btd_device_set *set,
+ struct btd_device *device);
+const char *btd_set_get_path(struct btd_device_set *set);
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This checks if device is advertising an RSI and if so disregards if it
is not discoverable since other members can be.
---
src/adapter.c | 4 ++--
src/eir.c | 3 +++
src/eir.h | 2 ++
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index ae0eb364bad5..7947160a6c5c 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -7122,7 +7122,7 @@ void btd_adapter_update_found_device(struct btd_adapter *adapter,
dev = btd_adapter_find_device(adapter, bdaddr, bdaddr_type);
if (!dev) {
- if (!discoverable && !monitoring) {
+ if (!discoverable && !monitoring && !eir_data.rsi) {
eir_data_free(&eir_data);
return;
}
@@ -7169,7 +7169,7 @@ void btd_adapter_update_found_device(struct btd_adapter *adapter,
/* If there is no matched Adv monitors, don't continue if not
* discoverable or if active discovery filter don't match.
*/
- if (!monitoring && (!discoverable ||
+ if (!eir_data.rsi && !monitoring && (!discoverable ||
(adapter->filtered_discovery && !is_filter_match(
adapter->discovery_list, &eir_data, rssi)))) {
eir_data_free(&eir_data);
diff --git a/src/eir.c b/src/eir.c
index 2f9ee036ffd5..52152c0d7f52 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -236,6 +236,9 @@ static void eir_parse_data(struct eir_data *eir, uint8_t type,
memcpy(ad->data, data, len);
eir->data_list = g_slist_append(eir->data_list, ad);
+
+ if (type == EIR_CSIP_RSI)
+ eir->rsi = true;
}
void eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
diff --git a/src/eir.h b/src/eir.h
index 6154e23ec266..a4bf5fbd33f3 100644
--- a/src/eir.h
+++ b/src/eir.h
@@ -37,6 +37,7 @@
#define EIR_SVC_DATA32 0x20 /* LE: Service data, 32-bit UUID */
#define EIR_SVC_DATA128 0x21 /* LE: Service data, 128-bit UUID */
#define EIR_TRANSPORT_DISCOVERY 0x26 /* Transport Discovery Service */
+#define EIR_CSIP_RSI 0x2e /* Resolvable Set Identifier */
#define EIR_MANUFACTURER_DATA 0xFF /* Manufacturer Specific Data */
/* Flags Descriptions */
@@ -76,6 +77,7 @@ struct eir_data {
uint32_t class;
uint16_t appearance;
bool name_complete;
+ bool rsi;
int8_t tx_power;
uint8_t *hash;
uint8_t *randomizer;
--
2.39.2
From: Sathish Narasimman <[email protected]>
This introduces option to configure main.conf that can be used to
configure co-ordinated set identification profile.
---
src/btd.h | 9 ++++
src/main.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/main.conf | 25 ++++++++++
3 files changed, 158 insertions(+)
diff --git a/src/btd.h b/src/btd.h
index 42cffcde43ca..70051c71c7c1 100644
--- a/src/btd.h
+++ b/src/btd.h
@@ -92,6 +92,13 @@ struct btd_defaults {
struct btd_le_defaults le;
};
+struct btd_csis {
+ bool encrypt;
+ uint8_t sirk[16];
+ uint8_t size;
+ uint8_t rank;
+};
+
struct btd_avdtp_opts {
uint8_t session_mode;
uint8_t stream_mode;
@@ -142,6 +149,8 @@ struct btd_opts {
enum jw_repairing_t jw_repairing;
struct btd_advmon_opts advmon;
+
+ struct btd_csis csis;
};
extern struct btd_opts btd_opts;
diff --git a/src/main.c b/src/main.c
index 99d9c508ff91..2a4d9be05d7a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -43,6 +43,7 @@
#include "shared/mainloop.h"
#include "shared/timeout.h"
#include "shared/queue.h"
+#include "shared/crypto.h"
#include "lib/uuid.h"
#include "shared/util.h"
#include "btd.h"
@@ -60,6 +61,9 @@
#define DEFAULT_TEMPORARY_TIMEOUT 30 /* 30 seconds */
#define DEFAULT_NAME_REQUEST_RETRY_DELAY 300 /* 5 minutes */
+/*CSIP Profile - Server */
+#define DEFAULT_SIRK "761FAE703ED681F0C50B34155B6434FB"
+
#define SHUTDOWN_GRACE_SECONDS 10
struct btd_opts btd_opts;
@@ -146,6 +150,13 @@ static const char *gatt_options[] = {
NULL
};
+static const char *csip_options[] = {
+ "SIRK",
+ "Size",
+ "Rank",
+ NULL
+};
+
static const char *avdtp_options[] = {
"SessionMode",
"StreamMode",
@@ -166,11 +177,55 @@ static const struct group_table {
{ "LE", le_options },
{ "Policy", policy_options },
{ "GATT", gatt_options },
+ { "CSIP", csip_options },
{ "AVDTP", avdtp_options },
{ "AdvMon", advmon_options },
{ }
};
+#ifndef MIN
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+#endif
+
+static int8_t check_sirk_alpha_numeric(char *str)
+{
+ int8_t val = 0;
+ char *s = str;
+
+ if (strlen(s) != 32) /* 32 Bytes of Alpha numeric string */
+ return 0;
+
+ for ( ; *s; s++) {
+ if (((*s >= '0') & (*s <= '9'))
+ || ((*s >= 'a') && (*s <= 'z'))
+ || ((*s >= 'A') && (*s <= 'Z'))) {
+ val = 1;
+ } else {
+ val = 0;
+ break;
+ }
+ }
+
+ return val;
+}
+
+static size_t hex2bin(const char *hexstr, uint8_t *buf, size_t buflen)
+{
+ size_t i, len;
+
+ if (!hexstr)
+ return 0;
+
+ len = MIN((strlen(hexstr) / 2), buflen);
+ memset(buf, 0, len);
+
+ for (i = 0; i < len; i++) {
+ if (sscanf(hexstr + (i * 2), "%02hhX", &buf[i]) != 1)
+ continue;
+ }
+
+ return len;
+}
GKeyFile *btd_get_main_conf(void)
{
@@ -652,6 +707,27 @@ static void btd_parse_kernel_experimental(char **list)
}
}
+static bool gen_sirk(const char *str)
+{
+ struct bt_crypto *crypto;
+ int ret;
+
+ crypto = bt_crypto_new();
+ if (!crypto) {
+ error("Failed to open crypto");
+ return false;
+ }
+
+ ret = bt_crypto_sirk(crypto, str, btd_opts.did_vendor,
+ btd_opts.did_product, btd_opts.did_version,
+ btd_opts.did_source, btd_opts.csis.sirk);
+ if (!ret)
+ error("Failed to generate SIRK");
+
+ bt_crypto_unref(crypto);
+ return ret;
+}
+
static void parse_config(GKeyFile *config)
{
GError *err = NULL;
@@ -939,6 +1015,54 @@ static void parse_config(GKeyFile *config)
btd_opts.gatt_channels = val;
}
+ str = g_key_file_get_string(config, "CSIP", "SIRK", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ DBG("CSIS SIRK: %s", str);
+
+ if (strlen(str) == 32 && check_sirk_alpha_numeric(str)) {
+ hex2bin(str, btd_opts.csis.sirk,
+ sizeof(btd_opts.csis.sirk));
+ } else if (!gen_sirk(str))
+ DBG("Unable to generate SIRK from string");
+
+ g_free(str);
+ }
+
+ boolean = g_key_file_get_boolean(config, "CSIP", "SIRK", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ DBG("CSIS Encryption: %s", boolean ? "true" : "false");
+
+ btd_opts.csis.encrypt = boolean;
+ }
+
+ val = g_key_file_get_integer(config, "CSIP", "Size", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ val = MIN(val, 0xFF);
+ val = MAX(val, 0);
+ DBG("CSIS Size: %u", val);
+ btd_opts.csis.size = val;
+ }
+
+ val = g_key_file_get_integer(config, "CSIP", "Rank", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ val = MIN(val, 0xFF);
+ val = MAX(val, 0);
+ DBG("CSIS Rank: %u", val);
+ btd_opts.csis.rank = val;
+ }
+
str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err);
if (err) {
DBG("%s", err->message);
diff --git a/src/main.conf b/src/main.conf
index f187c9aaa482..11172c9dd7c6 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -258,6 +258,31 @@
# Default to 3
#Channels = 3
+[CSIS]
+# SIRK - Set Identification Resolution Key which is common for all the
+# sets. They SIRK key is used to identify its sets. This can be any
+# 128 bit value or a string value (e.g. product name) which is then hashed.
+# Possible Values:
+# 16 byte hexadecimal value: 861FAE703ED681F0C50B34155B6434FB
+# String value: "My Product Name"
+# Defaults to none
+#SIRK =
+
+# SIRK Encryption
+# Possible values:
+# yes: Encrypt SIRK when read
+# no: Do not encrypt SIRK when read. (plaintext)
+# Defaults to yes
+#Encryption = yes
+
+# Total no of sets belongs to this Profile
+# Defaults to 0
+#Size = 0
+
+# Rank for the device
+# Defaults to 0
+#Rank = 0
+
[AVDTP]
# AVDTP L2CAP Signalling Channel Mode.
# Possible values:
--
2.39.2
From: Sathish Narasimman <[email protected]>
This adds initial code for Coordinated Set Identification Profile.
---
Makefile.am | 1 +
src/shared/csip.c | 866 ++++++++++++++++++++++++++++++++++++++++++++++
src/shared/csip.h | 67 ++++
3 files changed, 934 insertions(+)
create mode 100644 src/shared/csip.c
create mode 100644 src/shared/csip.h
diff --git a/Makefile.am b/Makefile.am
index 7b010c8159e9..7ded3ba75138 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -233,6 +233,7 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
src/shared/bap.h src/shared/bap.c src/shared/ascs.h \
src/shared/mcs.h src/shared/mcp.h src/shared/mcp.c \
src/shared/vcp.c src/shared/vcp.h \
+ src/shared/csip.c src/shared/csip.h \
src/shared/lc3.h src/shared/tty.h
if READLINE
diff --git a/src/shared/csip.c b/src/shared/csip.c
new file mode 100644
index 000000000000..ff2047a4ade0
--- /dev/null
+++ b/src/shared/csip.c
@@ -0,0 +1,866 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2022 Intel Corporation. All rights reserved.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <inttypes.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include "lib/bluetooth.h"
+#include "lib/uuid.h"
+
+#include "src/shared/queue.h"
+#include "src/shared/util.h"
+#include "src/shared/timeout.h"
+#include "src/shared/att.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-server.h"
+#include "src/shared/gatt-client.h"
+#include "src/shared/crypto.h"
+#include "src/shared/csip.h"
+
+#define DBG(_csip, fmt, arg...) \
+ csip_debug(_csip, "%s:%s() " fmt, __FILE__, __func__, ## arg)
+
+/* SIRK is now hardcoded in the code. This can be moved
+ * to a configuration file. Since the code is to validate
+ * the CSIP use case of set member
+ */
+#define SIRK "761FAE703ED681F0C50B34155B6434FB"
+#define CSIS_SIZE 0x02
+#define CSIS_LOCK 0x01
+#define CSIS_RANK 0x01
+#define CSIS_PLAINTEXT 0x01
+#define CSIS_ENC 0x02
+
+struct bt_csip_db {
+ struct gatt_db *db;
+ struct bt_csis *csis;
+};
+
+struct csis_sirk {
+ uint8_t type;
+ uint8_t val[16];
+} __packed;
+
+struct bt_csis {
+ struct bt_csip_db *cdb;
+ struct csis_sirk *sirk_val;
+ uint8_t size_val;
+ uint8_t lock_val;
+ uint8_t rank_val;
+ struct gatt_db_attribute *service;
+ struct gatt_db_attribute *sirk;
+ struct gatt_db_attribute *size;
+ struct gatt_db_attribute *lock;
+ struct gatt_db_attribute *lock_ccc;
+ struct gatt_db_attribute *rank;
+};
+
+struct bt_csip_cb {
+ unsigned int id;
+ bt_csip_func_t attached;
+ bt_csip_func_t detached;
+ void *user_data;
+};
+
+struct bt_csip_ready {
+ unsigned int id;
+ bt_csip_ready_func_t func;
+ bt_csip_destroy_func_t destroy;
+ void *data;
+};
+
+struct bt_csip {
+ int ref_count;
+ struct bt_csip_db *ldb;
+ struct bt_csip_db *rdb;
+ struct bt_gatt_client *client;
+ struct bt_att *att;
+
+ unsigned int idle_id;
+ struct queue *ready_cbs;
+
+ bt_csip_debug_func_t debug_func;
+ bt_csip_destroy_func_t debug_destroy;
+ void *debug_data;
+
+ bt_csip_ltk_func_t ltk_func;
+ void *ltk_data;
+
+ bt_csip_sirk_func_t sirk_func;
+ void *sirk_data;
+
+ void *user_data;
+};
+
+static struct queue *csip_db;
+static struct queue *csip_cbs;
+static struct queue *sessions;
+
+static void csip_detached(void *data, void *user_data)
+{
+ struct bt_csip_cb *cb = data;
+ struct bt_csip *csip = user_data;
+
+ cb->detached(csip, cb->user_data);
+}
+
+void bt_csip_detach(struct bt_csip *csip)
+{
+ if (!queue_remove(sessions, csip))
+ return;
+
+ bt_gatt_client_unref(csip->client);
+ csip->client = NULL;
+
+ queue_foreach(csip_cbs, csip_detached, csip);
+}
+
+static void csip_db_free(void *data)
+{
+ struct bt_csip_db *cdb = data;
+
+ if (!cdb)
+ return;
+
+ gatt_db_unref(cdb->db);
+
+ free(cdb->csis);
+ free(cdb);
+}
+
+static void csip_ready_free(void *data)
+{
+ struct bt_csip_ready *ready = data;
+
+ if (ready->destroy)
+ ready->destroy(ready->data);
+
+ free(ready);
+}
+
+static void csip_free(void *data)
+{
+ struct bt_csip *csip = data;
+
+ bt_csip_detach(csip);
+
+ csip_db_free(csip->rdb);
+
+ queue_destroy(csip->ready_cbs, csip_ready_free);
+
+ free(csip);
+}
+
+struct bt_att *bt_csip_get_att(struct bt_csip *csip)
+{
+ if (!csip)
+ return NULL;
+
+ if (csip->att)
+ return csip->att;
+
+ return bt_gatt_client_get_att(csip->client);
+}
+
+struct bt_csip *bt_csip_ref(struct bt_csip *csip)
+{
+ if (!csip)
+ return NULL;
+
+ __sync_fetch_and_add(&csip->ref_count, 1);
+
+ return csip;
+}
+
+static struct bt_csip *bt_csip_ref_safe(struct bt_csip *csip)
+{
+ if (!csip || !csip->ref_count)
+ return NULL;
+
+ return bt_csip_ref(csip);
+}
+
+void bt_csip_unref(struct bt_csip *csip)
+{
+ if (!csip)
+ return;
+
+ if (__sync_sub_and_fetch(&csip->ref_count, 1))
+ return;
+
+ csip_free(csip);
+}
+
+static void csip_debug(struct bt_csip *csip, const char *format, ...)
+{
+ va_list ap;
+
+ if (!csip || !format || !csip->debug_func)
+ return;
+
+ va_start(ap, format);
+ util_debug_va(csip->debug_func, csip->debug_data, format, ap);
+ va_end(ap);
+}
+
+static bool csip_match_att(const void *data, const void *match_data)
+{
+ const struct bt_csip *csip = data;
+ const struct bt_att *att = match_data;
+
+ return bt_csip_get_att((void *)csip) == att;
+}
+
+static bool csis_sirk_enc(struct bt_csis *csis, struct bt_att *att,
+ struct csis_sirk *sirk)
+{
+ struct bt_csip *csip;
+ uint8_t k[16];
+ struct bt_crypto *crypto;
+ bool ret;
+
+ csip = queue_find(sessions, csip_match_att, att);
+ if (!csip)
+ return false;
+
+ if (!csip->ltk_func(csip, k, csip->ltk_data)) {
+ DBG(csip, "Unable to read sef key");
+ return false;
+ }
+
+ crypto = bt_crypto_new();
+ if (!crypto) {
+ DBG(csip, "Failed to open crypto");
+ return false;
+ }
+
+ ret = bt_crypto_sef(crypto, k, sirk->val, sirk->val);
+ if (!ret)
+ DBG(csip, "Failed to encrypt SIRK using sef");
+
+ bt_crypto_unref(crypto);
+
+ return ret;
+}
+
+static void csis_sirk_read(struct gatt_db_attribute *attrib,
+ unsigned int id, uint16_t offset,
+ uint8_t opcode, struct bt_att *att,
+ void *user_data)
+{
+ struct bt_csis *csis = user_data;
+ struct csis_sirk sirk;
+ struct iovec iov;
+
+ memcpy(&sirk, csis->sirk_val, sizeof(sirk));
+
+ if (sirk.type == BT_CSIP_SIRK_ENCRYPT &&
+ !csis_sirk_enc(csis, att, &sirk)) {
+ gatt_db_attribute_read_result(attrib, id, BT_ATT_ERROR_UNLIKELY,
+ NULL, 0);
+ return;
+ }
+
+ iov.iov_base = &sirk;
+ iov.iov_len = sizeof(sirk);
+
+ gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base,
+ iov.iov_len);
+}
+
+static void csis_size_read(struct gatt_db_attribute *attrib,
+ unsigned int id, uint16_t offset,
+ uint8_t opcode, struct bt_att *att,
+ void *user_data)
+{
+ struct bt_csis *csis = user_data;
+ struct iovec iov;
+
+ iov.iov_base = &csis->size;
+ iov.iov_len = sizeof(csis->size);
+
+ gatt_db_attribute_read_result(attrib, id, 0, iov.iov_base,
+ iov.iov_len);
+}
+
+static void csis_lock_read_cb(struct gatt_db_attribute *attrib,
+ unsigned int id, uint16_t offset,
+ uint8_t opcode, struct bt_att *att,
+ void *user_data)
+{
+ uint8_t value = CSIS_LOCK;
+
+ gatt_db_attribute_read_result(attrib, id, 0, &value, sizeof(value));
+}
+
+static void csis_lock_write_cb(struct gatt_db_attribute *attrib,
+ unsigned int id, uint16_t offset,
+ const uint8_t *value, size_t len,
+ uint8_t opcode, struct bt_att *att,
+ void *user_data)
+{
+ gatt_db_attribute_write_result(attrib, id, 0);
+}
+
+static void csis_rank_read_cb(struct gatt_db_attribute *attrib,
+ unsigned int id, uint16_t offset,
+ uint8_t opcode, struct bt_att *att,
+ void *user_data)
+{
+ uint8_t value = CSIS_RANK;
+
+ gatt_db_attribute_read_result(attrib, id, 0, &value, sizeof(value));
+}
+
+static struct bt_csis *csis_new(struct gatt_db *db)
+{
+ struct bt_csis *csis;
+
+ if (!db)
+ return NULL;
+
+ csis = new0(struct bt_csis, 1);
+
+ return csis;
+}
+
+static struct bt_csip_db *csip_db_new(struct gatt_db *db)
+{
+ struct bt_csip_db *cdb;
+
+ if (!db)
+ return NULL;
+
+ cdb = new0(struct bt_csip_db, 1);
+ cdb->db = gatt_db_ref(db);
+
+ if (!csip_db)
+ csip_db = queue_new();
+
+ cdb->csis = csis_new(db);
+ cdb->csis->cdb = cdb;
+
+ queue_push_tail(csip_db, cdb);
+
+ return cdb;
+}
+
+bool bt_csip_set_user_data(struct bt_csip *csip, void *user_data)
+{
+ if (!csip)
+ return false;
+
+ csip->user_data = user_data;
+
+ return true;
+}
+
+static bool csip_db_match(const void *data, const void *match_data)
+{
+ const struct bt_csip_db *cdb = data;
+ const struct gatt_db *db = match_data;
+
+ return (cdb->db == db);
+}
+
+static struct bt_csip_db *csip_get_db(struct gatt_db *db)
+{
+ struct bt_csip_db *cdb;
+
+ cdb = queue_find(csip_db, csip_db_match, db);
+ if (cdb)
+ return cdb;
+
+ return csip_db_new(db);
+}
+
+void bt_csip_add_db(struct gatt_db *db)
+{
+ csip_db_new(db);
+}
+
+bool bt_csip_set_debug(struct bt_csip *csip, bt_csip_debug_func_t func,
+ void *user_data, bt_csip_destroy_func_t destroy)
+{
+ if (!csip)
+ return false;
+
+ if (csip->debug_destroy)
+ csip->debug_destroy(csip->debug_data);
+
+ csip->debug_func = func;
+ csip->debug_destroy = destroy;
+ csip->debug_data = user_data;
+
+ return true;
+}
+
+unsigned int bt_csip_register(bt_csip_func_t attached, bt_csip_func_t detached,
+ void *user_data)
+{
+ struct bt_csip_cb *cb;
+ static unsigned int id;
+
+ if (!attached && !detached)
+ return 0;
+
+ if (!csip_cbs)
+ csip_cbs = queue_new();
+
+ cb = new0(struct bt_csip_cb, 1);
+ cb->id = ++id ? id : ++id;
+ cb->attached = attached;
+ cb->detached = detached;
+ cb->user_data = user_data;
+
+ queue_push_tail(csip_cbs, cb);
+
+ return cb->id;
+}
+
+static bool match_id(const void *data, const void *match_data)
+{
+ const struct bt_csip_cb *cb = data;
+ unsigned int id = PTR_TO_UINT(match_data);
+
+ return (cb->id == id);
+}
+
+bool bt_csip_unregister(unsigned int id)
+{
+ struct bt_csip_cb *cb;
+
+ cb = queue_remove_if(csip_cbs, match_id, UINT_TO_PTR(id));
+ if (!cb)
+ return false;
+
+ free(cb);
+
+ return true;
+}
+
+struct bt_csip *bt_csip_new(struct gatt_db *ldb, struct gatt_db *rdb)
+{
+ struct bt_csip *csip;
+ struct bt_csip_db *db;
+
+ if (!ldb)
+ return NULL;
+
+ db = csip_get_db(ldb);
+ if (!db)
+ return NULL;
+
+ csip = new0(struct bt_csip, 1);
+ csip->ldb = db;
+ csip->ready_cbs = queue_new();
+
+ if (!rdb)
+ goto done;
+
+ db = new0(struct bt_csip_db, 1);
+ db->db = gatt_db_ref(rdb);
+
+ csip->rdb = db;
+
+done:
+ bt_csip_ref(csip);
+
+ return csip;
+}
+
+static struct bt_csis *csip_get_csis(struct bt_csip *csip)
+{
+ if (!csip)
+ return NULL;
+
+ if (csip->rdb->csis)
+ return csip->rdb->csis;
+
+ csip->rdb->csis = new0(struct bt_csis, 1);
+ csip->rdb->csis->cdb = csip->rdb;
+
+ return csip->rdb->csis;
+}
+
+static void read_sirk(bool success, uint8_t att_ecode, const uint8_t *value,
+ uint16_t length, void *user_data)
+{
+ struct bt_csip *csip = user_data;
+ struct bt_csis *csis;
+ struct csis_sirk *sirk;
+ struct iovec iov = {
+ .iov_base = (void *)value,
+ .iov_len = length
+ };
+
+ if (!success) {
+ DBG(csip, "Unable to read SIRK: error 0x%02x", att_ecode);
+ return;
+ }
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return;
+
+ sirk = util_iov_pull_mem(&iov, sizeof(*sirk));
+ if (!sirk) {
+ DBG(csip, "Invalid size for SIRK: len %u", length);
+ return;
+ }
+
+ if (!csis->sirk_val)
+ csis->sirk_val = new0(struct csis_sirk, 1);
+
+ memcpy(csis->sirk_val, sirk, sizeof(*sirk));
+}
+
+static void read_size(bool success, uint8_t att_ecode, const uint8_t *value,
+ uint16_t length, void *user_data)
+{
+ struct bt_csip *csip = user_data;
+ struct bt_csis *csis;
+
+ if (!success) {
+ DBG(csip, "Unable to read Size: error 0x%02x", att_ecode);
+ return;
+ }
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return;
+
+ csis->size_val = *value;
+}
+
+static void read_rank(bool success, uint8_t att_ecode, const uint8_t *value,
+ uint16_t length, void *user_data)
+{
+ struct bt_csip *csip = user_data;
+ struct bt_csis *csis;
+
+ if (!success) {
+ DBG(csip, "Unable to read Rank: error 0x%02x", att_ecode);
+ return;
+ }
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return;
+
+ csis->rank_val = *value;
+}
+
+static void csip_notify_ready(struct bt_csip *csip)
+{
+ const struct queue_entry *entry;
+
+ if (!bt_csip_ref_safe(csip))
+ return;
+
+ for (entry = queue_get_entries(csip->ready_cbs); entry;
+ entry = entry->next) {
+ struct bt_csip_ready *ready = entry->data;
+
+ ready->func(csip, ready->data);
+ }
+
+ bt_csip_unref(csip);
+}
+
+static void foreach_csis_char(struct gatt_db_attribute *attr, void *user_data)
+{
+ struct bt_csip *csip = user_data;
+ uint16_t value_handle;
+ bt_uuid_t uuid, uuid_sirk, uuid_size, uuid_rank;
+ struct bt_csis *csis;
+
+ if (!gatt_db_attribute_get_char_data(attr, NULL, &value_handle,
+ NULL, NULL, &uuid))
+ return;
+
+ bt_uuid16_create(&uuid_sirk, CS_SIRK);
+ bt_uuid16_create(&uuid_size, CS_SIZE);
+ bt_uuid16_create(&uuid_rank, CS_RANK);
+
+ if (!bt_uuid_cmp(&uuid, &uuid_sirk)) {
+ DBG(csip, "SIRK found: handle 0x%04x", value_handle);
+
+ csis = csip_get_csis(csip);
+ if (!csis || csis->sirk)
+ return;
+
+ csis->sirk = attr;
+
+ bt_gatt_client_read_value(csip->client, value_handle, read_sirk,
+ csip, NULL);
+
+ return;
+ }
+
+ if (!bt_uuid_cmp(&uuid, &uuid_size)) {
+ DBG(csip, "Size found: handle 0x%04x", value_handle);
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return;
+
+ csis->size = attr;
+
+ bt_gatt_client_read_value(csip->client, value_handle, read_size,
+ csip, NULL);
+ }
+
+ if (!bt_uuid_cmp(&uuid, &uuid_rank)) {
+ DBG(csip, "Rank found: handle 0x%04x", value_handle);
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return;
+
+ csis->rank = attr;
+
+ bt_gatt_client_read_value(csip->client, value_handle, read_rank,
+ csip, NULL);
+ }
+}
+static void foreach_csis_service(struct gatt_db_attribute *attr,
+ void *user_data)
+{
+ struct bt_csip *csip = user_data;
+ struct bt_csis *csis = csip_get_csis(csip);
+
+ csis->service = attr;
+
+ gatt_db_service_set_claimed(attr, true);
+
+ gatt_db_service_foreach_char(attr, foreach_csis_char, csip);
+}
+
+static void csip_idle(void *data)
+{
+ struct bt_csip *csip = data;
+
+ csip->idle_id = 0;
+
+ csip_notify_ready(csip);
+}
+
+bool bt_csip_attach(struct bt_csip *csip, struct bt_gatt_client *client)
+{
+ bt_uuid_t uuid;
+
+ if (!sessions)
+ sessions = queue_new();
+
+ queue_push_tail(sessions, csip);
+
+ if (!client)
+ return true;
+
+ if (csip->client)
+ return false;
+
+ csip->client = bt_gatt_client_clone(client);
+ if (!csip->client)
+ return false;
+
+ csip->idle_id = bt_gatt_client_idle_register(csip->client, csip_idle,
+ csip, NULL);
+
+ bt_uuid16_create(&uuid, CSIS_UUID);
+ gatt_db_foreach_service(csip->rdb->db, &uuid, foreach_csis_service,
+ csip);
+
+ return true;
+}
+
+static struct csis_sirk *sirk_new(struct bt_csis *csis, struct gatt_db *db,
+ uint8_t type, uint8_t k[16],
+ uint8_t size, uint8_t rank)
+{
+ struct csis_sirk *sirk;
+ bt_uuid_t uuid;
+ struct gatt_db_attribute *cas;
+
+ if (!csis)
+ return NULL;
+
+ if (csis->sirk)
+ sirk = csis->sirk_val;
+ else
+ sirk = new0(struct csis_sirk, 1);
+
+ sirk->type = type;
+ memcpy(sirk->val, k, sizeof(sirk->val));
+ csis->sirk_val = sirk;
+ csis->size_val = size;
+ csis->lock_val = 1;
+ csis->rank_val = rank;
+
+ /* Check if service already active as that means the attributes have
+ * already been registered.
+ */
+ if (gatt_db_service_get_active(csis->service))
+ return sirk;
+
+ /* Populate DB with CSIS attributes */
+ bt_uuid16_create(&uuid, CSIS_UUID);
+ csis->service = gatt_db_add_service(db, &uuid, true, 10);
+
+ bt_uuid16_create(&uuid, CS_SIRK);
+ csis->sirk = gatt_db_service_add_characteristic(csis->service,
+ &uuid,
+ BT_ATT_PERM_READ,
+ BT_GATT_CHRC_PROP_READ,
+ csis_sirk_read, NULL,
+ csis);
+
+ bt_uuid16_create(&uuid, CS_SIZE);
+ csis->size = gatt_db_service_add_characteristic(csis->service,
+ &uuid,
+ BT_ATT_PERM_READ,
+ BT_GATT_CHRC_PROP_READ,
+ csis_size_read, NULL,
+ csis);
+
+ /* Lock */
+ bt_uuid16_create(&uuid, CS_LOCK);
+ csis->lock = gatt_db_service_add_characteristic(csis->service, &uuid,
+ BT_ATT_PERM_READ,
+ BT_GATT_CHRC_PROP_READ |
+ BT_GATT_CHRC_PROP_WRITE |
+ BT_GATT_CHRC_PROP_NOTIFY,
+ csis_lock_read_cb,
+ csis_lock_write_cb,
+ csis);
+
+ csis->lock_ccc = gatt_db_service_add_ccc(csis->service,
+ BT_ATT_PERM_READ | BT_ATT_PERM_WRITE);
+
+ /* Rank */
+ bt_uuid16_create(&uuid, CS_RANK);
+ csis->rank = gatt_db_service_add_characteristic(csis->service, &uuid,
+ BT_ATT_PERM_READ,
+ BT_GATT_CHRC_PROP_READ,
+ csis_rank_read_cb,
+ NULL, csis);
+
+ /* Add the CAS service */
+ bt_uuid16_create(&uuid, 0x1853);
+ cas = gatt_db_add_service(db, &uuid, true, 2);
+ gatt_db_service_add_included(cas, csis->service);
+ gatt_db_service_set_active(cas, true);
+ gatt_db_service_add_included(cas, csis->service);
+
+ gatt_db_service_set_active(csis->service, true);
+
+ return sirk;
+}
+
+bool bt_csip_set_sirk(struct bt_csip *csip, bool encrypt,
+ uint8_t k[16], uint8_t size, uint8_t rank,
+ bt_csip_ltk_func_t func, void *user_data)
+{
+ uint8_t zero[16] = {};
+ uint8_t type;
+
+ if (!csip || !csip->ldb || !memcmp(k, zero, sizeof(zero)))
+ return false;
+
+ type = encrypt ? BT_CSIP_SIRK_ENCRYPT : BT_CSIP_SIRK_CLEARTEXT;
+
+ /* In case of encrypted type requires sef key function */
+ if (type == BT_CSIP_SIRK_ENCRYPT && !func)
+ return false;
+
+ if (!sirk_new(csip->ldb->csis, csip->ldb->db, type, k, size, rank))
+ return false;
+
+ csip->ltk_func = func;
+ csip->ltk_data = user_data;
+
+ return true;
+}
+
+bool bt_csip_get_sirk(struct bt_csip *csip, uint8_t *type,
+ uint8_t k[16], uint8_t *size, uint8_t *rank)
+{
+ struct bt_csis *csis;
+
+ if (!csip)
+ return false;
+
+ csis = csip_get_csis(csip);
+ if (!csis)
+ return false;
+
+ if (type)
+ *type = csis->sirk_val->type;
+
+ memcpy(k, csis->sirk_val->val, sizeof(csis->sirk_val->val));
+
+ if (size)
+ *size = csis->size_val;
+
+ if (rank)
+ *rank = csis->rank_val;
+
+ return true;
+}
+
+unsigned int bt_csip_ready_register(struct bt_csip *csip,
+ bt_csip_ready_func_t func, void *user_data,
+ bt_csip_destroy_func_t destroy)
+{
+ struct bt_csip_ready *ready;
+ static unsigned int id;
+
+ if (!csip)
+ return 0;
+
+ ready = new0(struct bt_csip_ready, 1);
+ ready->id = ++id ? id : ++id;
+ ready->func = func;
+ ready->destroy = destroy;
+ ready->data = user_data;
+
+ queue_push_tail(csip->ready_cbs, ready);
+
+ return ready->id;
+}
+
+static bool match_ready_id(const void *data, const void *match_data)
+{
+ const struct bt_csip_ready *ready = data;
+ unsigned int id = PTR_TO_UINT(match_data);
+
+ return (ready->id == id);
+}
+
+bool bt_csip_ready_unregister(struct bt_csip *csip, unsigned int id)
+{
+ struct bt_csip_ready *ready;
+
+ ready = queue_remove_if(csip->ready_cbs, match_ready_id,
+ UINT_TO_PTR(id));
+ if (!ready)
+ return false;
+
+ csip_ready_free(ready);
+
+ return true;
+}
diff --git a/src/shared/csip.h b/src/shared/csip.h
new file mode 100644
index 000000000000..0f8acb1cae82
--- /dev/null
+++ b/src/shared/csip.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2022 Intel Corporation. All rights reserved.
+ *
+ */
+
+#include <stdbool.h>
+#include <inttypes.h>
+
+#include "src/shared/io.h"
+
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+struct bt_csip;
+
+enum {
+ BT_CSIP_SIRK_ENCRYPT = 0x00,
+ BT_CSIP_SIRK_CLEARTEXT = 0x01
+};
+
+typedef void (*bt_csip_ready_func_t)(struct bt_csip *csip, void *user_data);
+typedef void (*bt_csip_destroy_func_t)(void *user_data);
+typedef void (*bt_csip_debug_func_t)(const char *str, void *user_data);
+typedef void (*bt_csip_func_t)(struct bt_csip *csip, void *user_data);
+typedef bool (*bt_csip_ltk_func_t)(struct bt_csip *csip, uint8_t k[16],
+ void *user_data);
+typedef bool (*bt_csip_sirk_func_t)(struct bt_csip *csip, uint8_t type,
+ uint8_t k[16], uint8_t size, uint8_t rank,
+ void *user_data);
+
+struct bt_csip *bt_csip_ref(struct bt_csip *csip);
+void bt_csip_unref(struct bt_csip *csip);
+
+void bt_csip_add_db(struct gatt_db *db);
+
+bool bt_csip_attach(struct bt_csip *csip, struct bt_gatt_client *client);
+void bt_csip_detach(struct bt_csip *csip);
+
+bool bt_csip_set_debug(struct bt_csip *csip, bt_csip_debug_func_t func,
+ void *user_data, bt_csip_destroy_func_t destroy);
+
+struct bt_att *bt_csip_get_att(struct bt_csip *csip);
+
+bool bt_csip_set_user_data(struct bt_csip *csip, void *user_data);
+
+/* Session related function */
+unsigned int bt_csip_register(bt_csip_func_t added, bt_csip_func_t removed,
+ void *user_data);
+bool bt_csip_unregister(unsigned int id);
+struct bt_csip *bt_csip_new(struct gatt_db *ldb, struct gatt_db *rdb);
+
+bool bt_csip_set_sirk(struct bt_csip *csip, bool encrypt,
+ uint8_t k[16], uint8_t size, uint8_t rank,
+ bt_csip_ltk_func_t func, void *user_data);
+
+bool bt_csip_get_sirk(struct bt_csip *csip, uint8_t *type,
+ uint8_t k[16], uint8_t *size, uint8_t *rank);
+
+unsigned int bt_csip_ready_register(struct bt_csip *csip,
+ bt_csip_ready_func_t func, void *user_data,
+ bt_csip_destroy_func_t destroy);
+bool bt_csip_ready_unregister(struct bt_csip *csip, unsigned int id);
--
2.39.2
From: Sathish Narasimman <[email protected]>
This adds initial code for csip plugin which handles Coordinated
set identification Profile and Coordinated Set Identification
Service.
---
Makefile.plugins | 5 +
configure.ac | 4 +
profiles/audio/csip.c | 363 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 372 insertions(+)
create mode 100644 profiles/audio/csip.c
diff --git a/Makefile.plugins b/Makefile.plugins
index 20cac384ef44..0f119e8714b7 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -131,3 +131,8 @@ if VCP
builtin_modules += vcp
builtin_sources += profiles/audio/vcp.c
endif
+
+if CSIP
+builtin_modules += csip
+builtin_sources += profiles/audio/csip.c
+endif
diff --git a/configure.ac b/configure.ac
index 515cdf1461eb..6f890110f554 100644
--- a/configure.ac
+++ b/configure.ac
@@ -207,6 +207,10 @@ AC_ARG_ENABLE(vcp, AS_HELP_STRING([--disable-vcp],
[disable VCP profile]), [enable_vcp=${enableval}])
AM_CONDITIONAL(VCP, test "${enable_vcp}" != "no")
+AC_ARG_ENABLE(csip, AS_HELP_STRING([--disable-csip],
+ [disable CSIP profile]), [enable_csip=${enableval}])
+AM_CONDITIONAL(CSIP, test "${enable_csip}" != "no")
+
AC_ARG_ENABLE(tools, AS_HELP_STRING([--disable-tools],
[disable Bluetooth tools]), [enable_tools=${enableval}])
AM_CONDITIONAL(TOOLS, test "${enable_tools}" != "no")
diff --git a/profiles/audio/csip.c b/profiles/audio/csip.c
new file mode 100644
index 000000000000..c273c02b8e76
--- /dev/null
+++ b/profiles/audio/csip.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2022 Intel Corporation. All rights reserved.
+ *
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+
+#include <ctype.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include "gdbus/gdbus.h"
+
+#include "lib/bluetooth.h"
+#include "lib/hci.h"
+#include "lib/sdp.h"
+#include "lib/uuid.h"
+
+#include "src/dbus-common.h"
+#include "src/shared/util.h"
+#include "src/shared/att.h"
+#include "src/shared/queue.h"
+#include "src/shared/gatt-db.h"
+#include "src/shared/gatt-client.h"
+#include "src/shared/gatt-server.h"
+#include "src/shared/csip.h"
+
+#include "btio/btio.h"
+#include "src/plugin.h"
+#include "src/adapter.h"
+#include "src/gatt-database.h"
+#include "src/device.h"
+#include "src/profile.h"
+#include "src/service.h"
+#include "src/log.h"
+#include "src/error.h"
+#include "src/btd.h"
+
+#define CSIS_UUID_STR "00001846-0000-1000-8000-00805f9b34fb"
+
+struct csip_data {
+ struct btd_device *device;
+ struct btd_service *service;
+ struct bt_csip *csip;
+ unsigned int ready_id;
+};
+
+static struct queue *sessions;
+
+static void csip_debug(const char *str, void *user_data)
+{
+ DBG_IDX(0xffff, "%s", str);
+}
+
+static struct csip_data *csip_data_new(struct btd_device *device)
+{
+ struct csip_data *data;
+
+ data = new0(struct csip_data, 1);
+ data->device = device;
+
+ return data;
+}
+
+static bool csip_ltk_read(struct bt_csip *csip, uint8_t k[16], void *user_data)
+{
+ /* TODO: Retrieve LTK using device object */
+ return false;
+}
+
+static void csip_data_add(struct csip_data *data)
+{
+ DBG("data %p", data);
+
+ if (queue_find(sessions, NULL, data)) {
+ error("data %p already added", data);
+ return;
+ }
+
+ bt_csip_set_debug(data->csip, csip_debug, NULL, NULL);
+
+ bt_csip_set_sirk(data->csip, btd_opts.csis.encrypt, btd_opts.csis.sirk,
+ btd_opts.csis.size, btd_opts.csis.rank,
+ csip_ltk_read, data);
+
+ if (!sessions)
+ sessions = queue_new();
+
+ queue_push_tail(sessions, data);
+
+ if (data->service)
+ btd_service_set_user_data(data->service, data);
+}
+
+static int csip_disconnect(struct btd_service *service)
+{
+ struct csip_data *data = btd_service_get_user_data(service);
+
+ bt_csip_detach(data->csip);
+
+ btd_service_disconnecting_complete(service, 0);
+
+ return 0;
+}
+
+static bool match_data(const void *data, const void *match_data)
+{
+ const struct csip_data *vdata = data;
+ const struct bt_csip *csip = match_data;
+
+ return vdata->csip == csip;
+}
+
+static void csip_data_free(struct csip_data *data)
+{
+ if (data->service) {
+ btd_service_set_user_data(data->service, NULL);
+ bt_csip_set_user_data(data->csip, NULL);
+ }
+
+ bt_csip_ready_unregister(data->csip, data->ready_id);
+ bt_csip_unref(data->csip);
+ free(data);
+}
+
+static void csip_data_remove(struct csip_data *data)
+{
+ DBG("data %p", data);
+
+ if (!queue_remove(sessions, data))
+ return;
+
+ csip_data_free(data);
+
+ if (queue_isempty(sessions)) {
+ queue_destroy(sessions, NULL);
+ sessions = NULL;
+ }
+}
+
+static void csip_detached(struct bt_csip *csip, void *user_data)
+{
+ struct csip_data *data;
+
+ DBG("%p", csip);
+
+ data = queue_find(sessions, match_data, csip);
+ if (!data) {
+ error("Unable to find csip session");
+ return;
+ }
+
+ /* If there is a service it means there is CSIS thus we can keep
+ * instance allocated.
+ */
+ if (data->service)
+ return;
+
+ csip_data_remove(data);
+}
+
+static void csip_attached(struct bt_csip *csip, void *user_data)
+{
+ struct csip_data *data;
+ struct bt_att *att;
+ struct btd_device *device;
+
+ DBG("%p", csip);
+
+ data = queue_find(sessions, match_data, csip);
+ if (data)
+ return;
+
+ att = bt_csip_get_att(csip);
+ if (!att)
+ return;
+
+ device = btd_adapter_find_device_by_fd(bt_att_get_fd(att));
+ if (!device) {
+ error("Unable to find device");
+ return;
+ }
+
+ data = csip_data_new(device);
+ data->csip = csip;
+
+ csip_data_add(data);
+
+}
+
+static int csip_server_probe(struct btd_profile *p,
+ struct btd_adapter *adapter)
+{
+ struct btd_gatt_database *database = btd_adapter_get_database(adapter);
+
+ DBG("CSIP path %s", adapter_get_path(adapter));
+
+ bt_csip_add_db(btd_gatt_database_get_db(database));
+
+ return 0;
+}
+
+static void csip_server_remove(struct btd_profile *p,
+ struct btd_adapter *adapter)
+{
+ DBG("CSIP remove Adapter");
+}
+
+static int csip_accept(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct bt_gatt_client *client = btd_device_get_gatt_client(device);
+ struct csip_data *data = btd_service_get_user_data(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ if (!data) {
+ error("CSIP service not handled by profile");
+ return -EINVAL;
+ }
+
+ if (!bt_csip_attach(data->csip, client)) {
+ error("CSIP unable to attach");
+ return -EINVAL;
+ }
+
+ btd_service_connecting_complete(service, 0);
+
+ return 0;
+}
+
+static void csip_ready(struct bt_csip *csip, void *user_data)
+{
+ struct btd_service *service = user_data;
+ struct btd_device *device = btd_service_get_device(service);
+ uint8_t type, size, rank;
+ uint8_t k[16];
+
+ DBG("csip %p", csip);
+
+ if (!bt_csip_get_sirk(csip, &type, k, &size, &rank)) {
+ error("Unable to read SIRK");
+ return;
+ }
+
+ btd_device_add_set(device, type == BT_CSIP_SIRK_ENCRYPT ? true : false,
+ k, size, rank);
+}
+
+static int csip_probe(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct btd_adapter *adapter = device_get_adapter(device);
+ struct btd_gatt_database *database = btd_adapter_get_database(adapter);
+ struct csip_data *data = btd_service_get_user_data(service);
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ /* Ignore, if we were probed for this device already */
+ if (data) {
+ error("Profile probed twice for the same device!");
+ return -EINVAL;
+ }
+
+ data = csip_data_new(device);
+ data->service = service;
+
+ data->csip = bt_csip_new(btd_gatt_database_get_db(database),
+ btd_device_get_gatt_db(device));
+ if (!data->csip) {
+ error("Unable to create CSIP instance");
+ free(data);
+ return -EINVAL;
+ }
+
+ csip_data_add(data);
+
+ data->ready_id = bt_csip_ready_register(data->csip, csip_ready, service,
+ NULL);
+
+ bt_csip_set_user_data(data->csip, service);
+
+ return 0;
+}
+
+static void csip_remove(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct csip_data *data;
+ char addr[18];
+
+ ba2str(device_get_address(device), addr);
+ DBG("%s", addr);
+
+ data = btd_service_get_user_data(service);
+ if (!data) {
+ error("CSIP service not handled by profile");
+ return;
+ }
+
+ csip_data_remove(data);
+}
+
+static struct btd_profile csip_profile = {
+ .name = "csip",
+ .priority = BTD_PROFILE_PRIORITY_MEDIUM,
+ .remote_uuid = CSIS_UUID_STR,
+
+ .device_probe = csip_probe,
+ .device_remove = csip_remove,
+
+ .accept = csip_accept,
+ .disconnect = csip_disconnect,
+
+ .adapter_probe = csip_server_probe,
+ .adapter_remove = csip_server_remove,
+};
+
+static unsigned int csip_id;
+
+static int csip_init(void)
+{
+ if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL)) {
+ warn("D-Bus experimental not enabled");
+ return -ENOTSUP;
+ }
+
+ btd_profile_register(&csip_profile);
+ csip_id = bt_csip_register(csip_attached, csip_detached, NULL);
+
+ return 0;
+}
+
+static void csip_exit(void)
+{
+ if (g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL) {
+ btd_profile_unregister(&csip_profile);
+ bt_csip_unregister(csip_id);
+ }
+}
+
+BLUETOOTH_PLUGIN_DEFINE(csip, VERSION, BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+ csip_init, csip_exit)
--
2.39.2
From: Sathish Narasimman <[email protected]>
The patch helps to generate Resolvable set identifier adv data.
which can be used as ADV data during advertisement.
It will be used to identify the device as part of setmember for
Coordinated set identification profile.
Example:
$<path to advtest/>advtest -i "761FAE703ED681F0C50B34155B6434FB"
SIRK: 761FAE703ED681F0C50B34155B6434FB
RSI: 0x71 0xcb 0xbc 0x7e 0x01 0x84
Random: bccb71
Hash: 84017e
---
tools/advtest.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/tools/advtest.c b/tools/advtest.c
index de036e783325..9ef69ed5124a 100644
--- a/tools/advtest.c
+++ b/tools/advtest.c
@@ -13,6 +13,13 @@
#include <config.h>
#endif
+#include <stdlib.h>
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+
#include <getopt.h>
#include "lib/bluetooth.h"
@@ -32,6 +39,9 @@
"\xe1\x23\x99\xc1\xca\x9a\xc3\x31"
#define SCAN_IRK "\xfa\x73\x09\x11\x3f\x03\x37\x0f" \
"\xf4\xf9\x93\x1e\xf9\xa3\x63\xa6"
+#ifndef MIN
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+#endif
static struct mgmt *mgmt;
static uint16_t index1 = MGMT_INDEX_NONE;
@@ -43,13 +53,73 @@ static struct bt_hci *scan_dev;
static void print_rpa(const uint8_t addr[6])
{
- printf(" Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
+ printf(" RSI:\t0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x\n",
addr[5], addr[4], addr[3],
addr[2], addr[1], addr[0]);
printf(" Random: %02x%02x%02x\n", addr[3], addr[4], addr[5]);
printf(" Hash: %02x%02x%02x\n", addr[0], addr[1], addr[2]);
}
+static size_t hex2bin(const char *hexstr, uint8_t *buf, size_t buflen)
+{
+ size_t i, len;
+
+ len = MIN((strlen(hexstr) / 2), buflen);
+ memset(buf, 0, len);
+
+ for (i = 0; i < len; i++)
+ if (sscanf(hexstr + (i * 2), "%02hhX", &buf[i]) != 1)
+ continue;
+
+
+ return len;
+}
+
+static bool get_random_bytes(void *buf, size_t num_bytes)
+{
+ ssize_t len;
+ int fd;
+
+ fd = open("/dev/urandom", O_RDONLY);
+ if (fd < 0)
+ return false;
+
+ len = read(fd, buf, num_bytes);
+
+ close(fd);
+
+ if (len < 0)
+ return false;
+
+ return true;
+}
+
+static void generate_rsi(char *val)
+{
+ uint8_t sirk[16], hash[3];
+ uint8_t rsi[6] = {0};
+
+ hex2bin(val, sirk, sizeof(sirk));
+
+ get_random_bytes(&rsi[3], 3);
+
+ rsi[5] &= 0x3f; /* Clear 2 msb */
+ rsi[5] |= 0x40; /* Set 2nd msb */
+
+ crypto = bt_crypto_new();
+ if (!crypto) {
+ fprintf(stderr, "Failed to open crypto interface\n");
+ mainloop_exit_failure();
+ return;
+ }
+
+ bt_crypto_ah(crypto, sirk, rsi + 3, hash);
+ memcpy(rsi, hash, 3);
+
+ print_rpa(rsi);
+}
+
+
static void scan_le_adv_report(const void *data, uint8_t size,
void *user_data)
{
@@ -351,9 +421,11 @@ static void usage(void)
printf("\tadvtest [options]\n");
printf("options:\n"
"\t-h, --help Show help options\n");
+ printf(" \t-i <128bit SIRK>, Generate RSI ADV Data\n");
}
static const struct option main_options[] = {
+ { "hash", no_argument, NULL, 'i' },
{ "version", no_argument, NULL, 'v' },
{ "help", no_argument, NULL, 'h' },
{ }
@@ -366,11 +438,15 @@ int main(int argc ,char *argv[])
for (;;) {
int opt;
- opt = getopt_long(argc, argv, "vh", main_options, NULL);
+ opt = getopt_long(argc, argv, "i:vh", main_options, NULL);
if (opt < 0)
break;
switch (opt) {
+ case 'i':
+ printf("SIRK: %s\n", optarg);
+ generate_rsi(optarg);
+ return EXIT_SUCCESS;
case 'v':
printf("%s\n", VERSION);
return EXIT_SUCCESS;
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
This prints devices not discoverable in grey so the user are able to
distict when for example set members are actually visible.
---
client/main.c | 79 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 55 insertions(+), 24 deletions(-)
diff --git a/client/main.c b/client/main.c
index e4a39896b2c6..79895015d6a6 100644
--- a/client/main.c
+++ b/client/main.c
@@ -24,6 +24,7 @@
#include "src/shared/shell.h"
#include "src/shared/util.h"
+#include "src/shared/ad.h"
#include "gdbus/gdbus.h"
#include "print.h"
#include "agent.h"
@@ -143,10 +144,32 @@ static void print_adapter(GDBusProxy *proxy, const char *description)
}
+#define DISTANCE_VAL_INVALID 0x7FFF
+
+static struct set_discovery_filter_args {
+ char *transport;
+ char *pattern;
+ dbus_uint16_t rssi;
+ dbus_int16_t pathloss;
+ char **uuids;
+ size_t uuids_len;
+ dbus_bool_t duplicate;
+ dbus_bool_t discoverable;
+ bool set;
+ bool active;
+ unsigned int timeout;
+} filter = {
+ .rssi = DISTANCE_VAL_INVALID,
+ .pathloss = DISTANCE_VAL_INVALID,
+ .set = true,
+};
+
static void print_device(GDBusProxy *proxy, const char *description)
{
DBusMessageIter iter;
const char *address, *name;
+ uint8_t *flags;
+ int flags_len = 0;
if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
return;
@@ -158,11 +181,39 @@ static void print_device(GDBusProxy *proxy, const char *description)
else
name = "<unknown>";
+ if (g_dbus_proxy_get_property(proxy, "AdvertisingFlags", &iter)) {
+ DBusMessageIter array;
+
+ dbus_message_iter_recurse(&iter, &array);
+ dbus_message_iter_get_fixed_array(&array, &flags, &flags_len);
+ }
+
+ if (!flags_len)
+ goto done;
+
+ if (!(flags[0] & (BT_AD_FLAG_LIMITED | BT_AD_FLAG_GENERAL))) {
+ /* Only print hidden/non-discoverable if filter.discoverable is
+ * not set.
+ */
+ if (filter.discoverable)
+ return;
+
+ bt_shell_printf("%s%s%s" COLOR_BOLDGRAY "Device %s %s"
+ COLOR_OFF "\n",
+ description ? "[" : "",
+ description ? : "",
+ description ? "] " : "",
+ address, name);
+
+ return;
+ }
+
+done:
bt_shell_printf("%s%s%sDevice %s %s\n",
- description ? "[" : "",
- description ? : "",
- description ? "] " : "",
- address, name);
+ description ? "[" : "",
+ description ? : "",
+ description ? "] " : "",
+ address, name);
}
static void print_uuid(const char *label, const char *uuid)
@@ -1133,26 +1184,6 @@ static void cmd_default_agent(int argc, char *argv[])
agent_default(dbus_conn, agent_manager);
}
-#define DISTANCE_VAL_INVALID 0x7FFF
-
-static struct set_discovery_filter_args {
- char *transport;
- char *pattern;
- dbus_uint16_t rssi;
- dbus_int16_t pathloss;
- char **uuids;
- size_t uuids_len;
- dbus_bool_t duplicate;
- dbus_bool_t discoverable;
- bool set;
- bool active;
- unsigned int timeout;
-} filter = {
- .rssi = DISTANCE_VAL_INVALID,
- .pathloss = DISTANCE_VAL_INVALID,
- .set = true,
-};
-
static void start_discovery_reply(DBusMessage *message, void *user_data)
{
dbus_bool_t enable = GPOINTER_TO_UINT(user_data);
--
2.39.2
From: Luiz Augusto von Dentz <[email protected]>
---
client/main.c | 113 +++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 103 insertions(+), 10 deletions(-)
diff --git a/client/main.c b/client/main.c
index e5cf1e203ed1..e4a39896b2c6 100644
--- a/client/main.c
+++ b/client/main.c
@@ -51,6 +51,7 @@ struct adapter {
GDBusProxy *ad_proxy;
GDBusProxy *adv_monitor_proxy;
GList *devices;
+ GList *sets;
};
static struct adapter *default_ctrl;
@@ -232,7 +233,7 @@ static void print_experimental(GDBusProxy *proxy)
}
}
-static gboolean device_is_child(GDBusProxy *device, GDBusProxy *parent)
+static gboolean proxy_is_child(GDBusProxy *device, GDBusProxy *parent)
{
DBusMessageIter iter;
const char *adapter, *path;
@@ -269,14 +270,14 @@ static gboolean service_is_child(GDBusProxy *service)
"org.bluez.Device1") != NULL;
}
-static struct adapter *find_parent(GDBusProxy *device)
+static struct adapter *find_parent(GDBusProxy *proxy)
{
GList *list;
for (list = g_list_first(ctrl_list); list; list = g_list_next(list)) {
struct adapter *adapter = list->data;
- if (device_is_child(device, adapter->proxy) == TRUE)
+ if (proxy_is_child(proxy, adapter->proxy) == TRUE)
return adapter;
}
return NULL;
@@ -399,6 +400,27 @@ static void admon_manager_added(GDBusProxy *proxy)
adv_monitor_register_app(dbus_conn);
}
+static void print_set(GDBusProxy *proxy, const char *description)
+{
+ bt_shell_printf("%s%s%sDeviceSet %s\n",
+ description ? "[" : "",
+ description ? : "",
+ description ? "] " : "",
+ g_dbus_proxy_get_path(proxy));
+}
+
+static void set_added(GDBusProxy *proxy)
+{
+ struct adapter *adapter = find_parent(proxy);
+
+ if (!adapter)
+ return;
+
+ adapter->sets = g_list_append(adapter->sets, proxy);
+ print_set(proxy, COLORED_NEW);
+ bt_shell_set_env(g_dbus_proxy_get_path(proxy), proxy);
+}
+
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
const char *interface;
@@ -434,6 +456,8 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
} else if (!strcmp(interface,
"org.bluez.AdvertisementMonitorManager1")) {
admon_manager_added(proxy);
+ } else if (!strcmp(interface, "org.bluez.DeviceSet1")) {
+ set_added(proxy);
}
}
@@ -484,6 +508,7 @@ static void adapter_removed(GDBusProxy *proxy)
ctrl_list = g_list_remove_link(ctrl_list, ll);
g_list_free(adapter->devices);
+ g_list_free(adapter->sets);
g_free(adapter);
g_list_free(ll);
return;
@@ -491,6 +516,19 @@ static void adapter_removed(GDBusProxy *proxy)
}
}
+static void set_removed(GDBusProxy *proxy)
+{
+ struct adapter *adapter = find_parent(proxy);
+
+ if (!adapter)
+ return;
+
+ adapter->sets = g_list_remove(adapter->sets, proxy);
+
+ print_set(proxy, COLORED_DEL);
+ bt_shell_set_env(g_dbus_proxy_get_path(proxy), NULL);
+}
+
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
const char *interface;
@@ -531,6 +569,8 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
} else if (!strcmp(interface,
"org.bluez.AdvertisementMonitorManager1")) {
adv_monitor_remove_manager(dbus_conn);
+ } else if (!strcmp(interface, "org.bluez.DeviceSet1")) {
+ set_removed(proxy);
}
}
@@ -557,7 +597,7 @@ static void property_changed(GDBusProxy *proxy, const char *name,
interface = g_dbus_proxy_get_interface(proxy);
if (!strcmp(interface, "org.bluez.Device1")) {
- if (default_ctrl && device_is_child(proxy,
+ if (default_ctrl && proxy_is_child(proxy,
default_ctrl->proxy) == TRUE) {
DBusMessageIter addr_iter;
char *str;
@@ -1559,6 +1599,39 @@ static struct GDBusProxy *find_device(int argc, char *argv[])
return proxy;
}
+static struct GDBusProxy *find_set(int argc, char *argv[])
+{
+ GDBusProxy *proxy;
+
+ if (check_default_ctrl() == FALSE)
+ return NULL;
+
+ proxy = find_proxies_by_path(default_ctrl->sets, argv[1]);
+ if (!proxy) {
+ bt_shell_printf("DeviceSet %s not available\n", argv[1]);
+ return NULL;
+ }
+
+ return proxy;
+}
+
+static void cmd_set_info(int argc, char *argv[])
+{
+ GDBusProxy *proxy;
+
+ proxy = find_set(argc, argv);
+ if (!proxy)
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+
+ bt_shell_printf("DeviceSet %s\n", g_dbus_proxy_get_path(proxy));
+
+ print_property(proxy, "AutoConnect");
+ print_property(proxy, "Devices");
+ print_property(proxy, "Size");
+
+ return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+}
+
static void cmd_info(int argc, char *argv[])
{
GDBusProxy *proxy;
@@ -1568,7 +1641,7 @@ static void cmd_info(int argc, char *argv[])
proxy = find_device(argc, argv);
if (!proxy)
- return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ return cmd_set_info(argc, argv);
if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
return bt_shell_noninteractive_quit(EXIT_FAILURE);
@@ -1605,6 +1678,7 @@ static void cmd_info(int argc, char *argv[])
print_property(proxy, "TxPower");
print_property(proxy, "AdvertisingFlags");
print_property(proxy, "AdvertisingData");
+ print_property(proxy, "Sets");
battery_proxy = find_proxies_by_path(battery_proxies,
g_dbus_proxy_get_path(proxy));
@@ -2298,11 +2372,13 @@ static char *generic_generator(const char *text, int state,
index++;
- if (g_dbus_proxy_get_property(proxy, property, &iter) == FALSE)
+ if (!property)
+ str = g_dbus_proxy_get_path(proxy);
+ else if (g_dbus_proxy_get_property(proxy, property, &iter))
+ dbus_message_iter_get_basic(&iter, &str);
+ else
continue;
- dbus_message_iter_get_basic(&iter, &str);
-
if (!strncasecmp(str, text, len))
return strdup(str);
}
@@ -2348,6 +2424,23 @@ static char *dev_generator(const char *text, int state)
default_ctrl ? default_ctrl->devices : NULL, "Address");
}
+static char *set_generator(const char *text, int state)
+{
+ return generic_generator(text, state,
+ default_ctrl ? default_ctrl->sets : NULL, NULL);
+}
+
+static char *dev_set_generator(const char *text, int state)
+{
+ char *str;
+
+ str = dev_generator(text, state);
+ if (str)
+ return str;
+
+ return set_generator(text, state);
+}
+
static char *attribute_generator(const char *text, int state)
{
return gatt_attribute_generator(text, state);
@@ -2965,8 +3058,8 @@ static const struct bt_shell_menu main_menu = {
{ "set-alias", "<alias>", cmd_set_alias, "Set device alias" },
{ "scan", "<on/off/bredr/le>", cmd_scan,
"Scan for devices", scan_generator },
- { "info", "[dev]", cmd_info, "Device information",
- dev_generator },
+ { "info", "[dev/set]", cmd_info, "Device/Set information",
+ dev_set_generator },
{ "pair", "[dev]", cmd_pair, "Pair with device",
dev_generator },
{ "cancel-pairing", "[dev]", cmd_cancel_pairing,
--
2.39.2
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=727653
---Test result---
Test Summary:
CheckPatch FAIL 8.44 seconds
GitLint PASS 3.89 seconds
BuildEll PASS 32.44 seconds
BluezMake PASS 970.60 seconds
MakeCheck PASS 11.92 seconds
MakeDistcheck PASS 160.99 seconds
CheckValgrind PASS 263.34 seconds
CheckSmatch WARNING 363.40 seconds
bluezmakeextell PASS 111.16 seconds
IncrementalBuild PASS 9602.60 seconds
ScanBuild PASS 1222.18 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#84:
- Generate a hash (sirk) using vendor, product, version and source as input
/github/workspace/src/src/13164836.patch total: 0 errors, 1 warnings, 49 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13164836.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[RFC,v2,07/12] main.conf: Add CSIP profile configurable options
WARNING:STATIC_CONST_CHAR_ARRAY: static const char * array should probably be static const char * const
#145: FILE: src/main.c:153:
+static const char *csip_options[] = {
/github/workspace/src/src/13164842.patch total: 0 errors, 1 warnings, 217 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13164842.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
#998: FILE: src/shared/csip.h:16:
+#define __packed __attribute__((packed))
/github/workspace/src/src/13164843.patch total: 0 errors, 1 warnings, 940 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13164843.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/crypto.c:271:21: warning: Variable length array is used.src/shared/crypto.c:272:23: warning: Variable length array is used.src/shared/crypto.c:271:21: warning: Variable length array is used.src/shared/crypto.c:272:23: warning: Variable length array is used.src/shared/crypto.c:271:21: warning: Variable length array is used.src/shared/crypto.c:272:23: warning: Variable length array is used.
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> the following steps:
>
> - Generate a hash (k) using the str as input
> - Generate a hash (sirk) using vendor, product, version and source as input
> - Encrypt sirk using k as LTK with sef function.
>
> [...]
Here is the summary with links:
- [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
- [RFC,v2,02/12] shared/ad: Add RSI data type
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
- [RFC,v2,03/12] doc: Add set-api
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
- [RFC,v2,04/12] device-api: Add Set property
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
- [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
(no matching commit)
- [RFC,v2,06/12] core: Check if device has RSI
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
- [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
(no matching commit)
- [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
- [RFC,v2,09/12] profiles: Add initial code for csip plugin
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
- [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
- [RFC,v2,11/12] client: Add support for DeviceSet proxy
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
- [RFC,v2,12/12] client: Use AdvertisingFlags when available
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hi Pauli, Frederic,
On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluez.git (master)
> by Luiz Augusto von Dentz <[email protected]>:
>
> On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > the following steps:
> >
> > - Generate a hash (k) using the str as input
> > - Generate a hash (sirk) using vendor, product, version and source as input
> > - Encrypt sirk using k as LTK with sef function.
> >
> > [...]
>
> Here is the summary with links:
> - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> - [RFC,v2,02/12] shared/ad: Add RSI data type
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> - [RFC,v2,03/12] doc: Add set-api
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> - [RFC,v2,04/12] device-api: Add Set property
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> (no matching commit)
> - [RFC,v2,06/12] core: Check if device has RSI
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> (no matching commit)
> - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
Let me know if you guys are happy with this interface to detect
Coordinated Sets, it still experimental so we can experiment with it
until we think it is stable, regarding the implementation of the
transports one major difference here is that we will need to encode
left and right separately, not sure how hard it is to do that in
pipewire?
>
--
Luiz Augusto von Dentz
Hi,
su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli, Frederic,
>
> On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> >
> > Hello:
> >
> > This series was applied to bluetooth/bluez.git (master)
> > by Luiz Augusto von Dentz <[email protected]>:
> >
> > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > > From: Luiz Augusto von Dentz <[email protected]>
> > >
> > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > > the following steps:
> > >
> > > - Generate a hash (k) using the str as input
> > > - Generate a hash (sirk) using vendor, product, version and source as input
> > > - Encrypt sirk using k as LTK with sef function.
> > >
> > > [...]
> >
> > Here is the summary with links:
> > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> > - [RFC,v2,02/12] shared/ad: Add RSI data type
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> > - [RFC,v2,03/12] doc: Add set-api
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> > - [RFC,v2,04/12] device-api: Add Set property
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> > (no matching commit)
> > - [RFC,v2,06/12] core: Check if device has RSI
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> > (no matching commit)
> > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> >
> > You are awesome, thank you!
> > --
> > Deet-doot-dot, I am a bot.
> > https://korg.docs.kernel.org/patchwork/pwbot.html
>
> Let me know if you guys are happy with this interface to detect
> Coordinated Sets, it still experimental so we can experiment with it
> until we think it is stable, regarding the implementation of the
> transports one major difference here is that we will need to encode
> left and right separately, not sure how hard it is to do that in
> pipewire?
As far as the device set DBus interface is concerned, it seems to work
fine for me currently (in wip implementation for PW [0]). Don't right
now see something that would need to be added/changed in it.
Channel splitting/merging is generally easy in PW. How the playback
synchronization is going to work on socket level may determine a bit at
what level in PW it is convenient to do though.
---
Laundry list for PW related to this:
* How to do TX syncronization properly with the ISO sockets needs still
some thinking. I have some wip patches [2] that add the timestamps and
other socket API that provide timing information to allow
synchronization to the Number of Completed packets events.
Corresponding Pipewire implementation [3] rate matches to keep the time
difference between those events and our audio reference time fixed at
e.g. 25ms (2 packets in controller). Not really clear yet if this is a
right thing to do to help the controller send packets at the right
time.
Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
values in Command Complete in btmon for running CIS, so that command
doesn't seem to help here.
* BlueZ doesn't seem to pass on the PAC audio location it reads via
read_sink/source_pac_loc, probably very easy to fix.
* The CIS in a CIG cannot be started one by one, or connected to same
destination. The kernel appears to wait until all CIS sockets in same
CIG go to connect state before proceeding to create CIS. The spec does
not seem to require this (I have some pre-rfc patches to make it more
flexible [1].)
* PW currently does transport acquires synchronously and fails because
of that with multiple CIS, but it probably should do them async.
[0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
[1] https://github.com/pv/linux/commits/iso-fix-multicis
[2] https://github.com/pv/linux/commits/iso-timestamp
[3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
--
Pauli Virtanen
Hi Pauli,
On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
>
> Hi,
>
> su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli, Frederic,
> >
> > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> > >
> > > Hello:
> > >
> > > This series was applied to bluetooth/bluez.git (master)
> > > by Luiz Augusto von Dentz <[email protected]>:
> > >
> > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > > > From: Luiz Augusto von Dentz <[email protected]>
> > > >
> > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > > > the following steps:
> > > >
> > > > - Generate a hash (k) using the str as input
> > > > - Generate a hash (sirk) using vendor, product, version and source as input
> > > > - Encrypt sirk using k as LTK with sef function.
> > > >
> > > > [...]
> > >
> > > Here is the summary with links:
> > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> > > - [RFC,v2,02/12] shared/ad: Add RSI data type
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> > > - [RFC,v2,03/12] doc: Add set-api
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> > > - [RFC,v2,04/12] device-api: Add Set property
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> > > (no matching commit)
> > > - [RFC,v2,06/12] core: Check if device has RSI
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> > > (no matching commit)
> > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> > >
> > > You are awesome, thank you!
> > > --
> > > Deet-doot-dot, I am a bot.
> > > https://korg.docs.kernel.org/patchwork/pwbot.html
> >
> > Let me know if you guys are happy with this interface to detect
> > Coordinated Sets, it still experimental so we can experiment with it
> > until we think it is stable, regarding the implementation of the
> > transports one major difference here is that we will need to encode
> > left and right separately, not sure how hard it is to do that in
> > pipewire?
>
> As far as the device set DBus interface is concerned, it seems to work
> fine for me currently (in wip implementation for PW [0]). Don't right
> now see something that would need to be added/changed in it.
>
> Channel splitting/merging is generally easy in PW. How the playback
> synchronization is going to work on socket level may determine a bit at
> what level in PW it is convenient to do though.
>
>
> ---
>
> Laundry list for PW related to this:
>
> * How to do TX syncronization properly with the ISO sockets needs still
> some thinking. I have some wip patches [2] that add the timestamps and
> other socket API that provide timing information to allow
> synchronization to the Number of Completed packets events.
> Corresponding Pipewire implementation [3] rate matches to keep the time
> difference between those events and our audio reference time fixed at
> e.g. 25ms (2 packets in controller). Not really clear yet if this is a
> right thing to do to help the controller send packets at the right
> time.
I have to check with our controller folks, I do recall someone saying
that perhaps we should use framed instead of unframed so the
controller can better keep up with timings, but it is not yet clear
why.
> Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
> values in Command Complete in btmon for running CIS, so that command
> doesn't seem to help here.
Yeah, I don't think it is implemented yet.
> * BlueZ doesn't seem to pass on the PAC audio location it reads via
> read_sink/source_pac_loc, probably very easy to fix.
Will take a look, afaik we fixed something like this not long ago but
perhaps you are talking about something different.
> * The CIS in a CIG cannot be started one by one, or connected to same
> destination. The kernel appears to wait until all CIS sockets in same
> CIG go to connect state before proceeding to create CIS. The spec does
> not seem to require this (I have some pre-rfc patches to make it more
> flexible [1].)
It used to be like that but I actually have to fix it because the
controller don't accept multiple CreateCIS in parallel:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
And it would actually create a relatively big window if we queue and
wait for CIS established, because then the controller may set up its
scheduling without taking into account the second CIS which will then
fail when it comes to its setup, so I think it is better to program
them together to avoid having only one side working.
Btw, take a look at how it was done with bluetoothctl>
transport.acquire <transport_left> <transport_right>, we have been
able to use it to acquire both left/right earbuds and then send
pre-encoded files.
> * PW currently does transport acquires synchronously and fails because
> of that with multiple CIS, but it probably should do them async.
>
>
> [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
> [1] https://github.com/pv/linux/commits/iso-fix-multicis
> [2] https://github.com/pv/linux/commits/iso-timestamp
> [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
>
> --
> Pauli Virtanen
--
Luiz Augusto von Dentz
14. maaliskuuta 2023 2.18.21 GMT+02:00 Luiz Augusto von Dentz <[email protected]> kirjoitti:
>Hi Pauli,
>
>On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
>>
>> Hi,
>>
>> su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
>> > Hi Pauli, Frederic,
>> >
>> > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
>> > >
>> > > Hello:
>> > >
>> > > This series was applied to bluetooth/bluez.git (master)
>> > > by Luiz Augusto von Dentz <[email protected]>:
>> > >
>> > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
>> > > > From: Luiz Augusto von Dentz <[email protected]>
>> > > >
>> > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
>> > > > the following steps:
>> > > >
>> > > > - Generate a hash (k) using the str as input
>> > > > - Generate a hash (sirk) using vendor, product, version and source as input
>> > > > - Encrypt sirk using k as LTK with sef function.
>> > > >
>> > > > [...]
>> > >
>> > > Here is the summary with links:
>> > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
>> > > - [RFC,v2,02/12] shared/ad: Add RSI data type
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
>> > > - [RFC,v2,03/12] doc: Add set-api
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
>> > > - [RFC,v2,04/12] device-api: Add Set property
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
>> > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
>> > > (no matching commit)
>> > > - [RFC,v2,06/12] core: Check if device has RSI
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
>> > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
>> > > (no matching commit)
>> > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
>> > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
>> > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
>> > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
>> > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
>> > >
>> > > You are awesome, thank you!
>> > > --
>> > > Deet-doot-dot, I am a bot.
>> > > https://korg.docs.kernel.org/patchwork/pwbot.html
>> >
>> > Let me know if you guys are happy with this interface to detect
>> > Coordinated Sets, it still experimental so we can experiment with it
>> > until we think it is stable, regarding the implementation of the
>> > transports one major difference here is that we will need to encode
>> > left and right separately, not sure how hard it is to do that in
>> > pipewire?
>>
>> As far as the device set DBus interface is concerned, it seems to work
>> fine for me currently (in wip implementation for PW [0]). Don't right
>> now see something that would need to be added/changed in it.
>>
>> Channel splitting/merging is generally easy in PW. How the playback
>> synchronization is going to work on socket level may determine a bit at
>> what level in PW it is convenient to do though.
>>
>>
>> ---
>>
>> Laundry list for PW related to this:
>>
>> * How to do TX syncronization properly with the ISO sockets needs still
>> some thinking. I have some wip patches [2] that add the timestamps and
>> other socket API that provide timing information to allow
>> synchronization to the Number of Completed packets events.
>> Corresponding Pipewire implementation [3] rate matches to keep the time
>> difference between those events and our audio reference time fixed at
>> e.g. 25ms (2 packets in controller). Not really clear yet if this is a
>> right thing to do to help the controller send packets at the right
>> time.
>
>I have to check with our controller folks, I do recall someone saying
>that perhaps we should use framed instead of unframed so the
>controller can better keep up with timings, but it is not yet clear
>why.
>
>> Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
>> values in Command Complete in btmon for running CIS, so that command
>> doesn't seem to help here.
>
>Yeah, I don't think it is implemented yet.
>
>> * BlueZ doesn't seem to pass on the PAC audio location it reads via
>> read_sink/source_pac_loc, probably very easy to fix.
>
>Will take a look, afaik we fixed something like this not long ago but
>perhaps you are talking about something different.
>
>> * The CIS in a CIG cannot be started one by one, or connected to same
>> destination. The kernel appears to wait until all CIS sockets in same
>> CIG go to connect state before proceeding to create CIS. The spec does
>> not seem to require this (I have some pre-rfc patches to make it more
>> flexible [1].)
>
>It used to be like that but I actually have to fix it because the
>controller don't accept multiple CreateCIS in parallel:
>
>https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
>
>And it would actually create a relatively big window if we queue and
>wait for CIS established, because then the controller may set up its
>scheduling without taking into account the second CIS which will then
>fail when it comes to its setup, so I think it is better to program
>them together to avoid having only one side working.
Hmm, I made it queue and wait for the previous Create CIS to fully complete before emitting the next one, and that did seem to also work (also with TWS playback to the Samsung device).
However did not extensively test, so even if allowed by spec risks running to controller issues?
The problem here is that the second CIS is not necessarily going to come, as it may be unrelated device put to same CIG since controller doesn't support multiple CIG.
In principle sound server can acquire always all CIS, but then we should expose also the cig/cis properties on transports so they know which ones are needed.
It could also make sense for BlueZ do it, when any cis in cig is started.
>Btw, take a look at how it was done with bluetoothctl>
>transport.acquire <transport_left> <transport_right>, we have been
>able to use it to acquire both left/right earbuds and then send
>pre-encoded files.
>
>> * PW currently does transport acquires synchronously and fails because
>> of that with multiple CIS, but it probably should do them async.
>>
>>
>> [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
>> [1] https://github.com/pv/linux/commits/iso-fix-multicis
>> [2] https://github.com/pv/linux/commits/iso-timestamp
>> [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
>>
>> --
>> Pauli Virtanen
>
>
>
Hi,
Hi Pauli,
On Mon, Mar 13, 2023 at 6:05 PM Pauli Virtanen <[email protected]> wrote:
>
> 14. maaliskuuta 2023 2.18.21 GMT+02:00 Luiz Augusto von Dentz <[email protected]> kirjoitti:
> >Hi Pauli,
> >
> >On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> >> > Hi Pauli, Frederic,
> >> >
> >> > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> >> > >
> >> > > Hello:
> >> > >
> >> > > This series was applied to bluetooth/bluez.git (master)
> >> > > by Luiz Augusto von Dentz <[email protected]>:
> >> > >
> >> > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> >> > > > From: Luiz Augusto von Dentz <[email protected]>
> >> > > >
> >> > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> >> > > > the following steps:
> >> > > >
> >> > > > - Generate a hash (k) using the str as input
> >> > > > - Generate a hash (sirk) using vendor, product, version and source as input
> >> > > > - Encrypt sirk using k as LTK with sef function.
> >> > > >
> >> > > > [...]
> >> > >
> >> > > Here is the summary with links:
> >> > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> >> > > - [RFC,v2,02/12] shared/ad: Add RSI data type
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> >> > > - [RFC,v2,03/12] doc: Add set-api
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> >> > > - [RFC,v2,04/12] device-api: Add Set property
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> >> > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> >> > > (no matching commit)
> >> > > - [RFC,v2,06/12] core: Check if device has RSI
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> >> > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> >> > > (no matching commit)
> >> > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> >> > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> >> > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> >> > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> >> > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> >> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> >> > >
> >> > > You are awesome, thank you!
> >> > > --
> >> > > Deet-doot-dot, I am a bot.
> >> > > https://korg.docs.kernel.org/patchwork/pwbot.html
> >> >
> >> > Let me know if you guys are happy with this interface to detect
> >> > Coordinated Sets, it still experimental so we can experiment with it
> >> > until we think it is stable, regarding the implementation of the
> >> > transports one major difference here is that we will need to encode
> >> > left and right separately, not sure how hard it is to do that in
> >> > pipewire?
> >>
> >> As far as the device set DBus interface is concerned, it seems to work
> >> fine for me currently (in wip implementation for PW [0]). Don't right
> >> now see something that would need to be added/changed in it.
> >>
> >> Channel splitting/merging is generally easy in PW. How the playback
> >> synchronization is going to work on socket level may determine a bit at
> >> what level in PW it is convenient to do though.
> >>
> >>
> >> ---
> >>
> >> Laundry list for PW related to this:
> >>
> >> * How to do TX syncronization properly with the ISO sockets needs still
> >> some thinking. I have some wip patches [2] that add the timestamps and
> >> other socket API that provide timing information to allow
> >> synchronization to the Number of Completed packets events.
> >> Corresponding Pipewire implementation [3] rate matches to keep the time
> >> difference between those events and our audio reference time fixed at
> >> e.g. 25ms (2 packets in controller). Not really clear yet if this is a
> >> right thing to do to help the controller send packets at the right
> >> time.
> >
> >I have to check with our controller folks, I do recall someone saying
> >that perhaps we should use framed instead of unframed so the
> >controller can better keep up with timings, but it is not yet clear
> >why.
> >
> >> Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
> >> values in Command Complete in btmon for running CIS, so that command
> >> doesn't seem to help here.
> >
> >Yeah, I don't think it is implemented yet.
> >
> >> * BlueZ doesn't seem to pass on the PAC audio location it reads via
> >> read_sink/source_pac_loc, probably very easy to fix.
> >
> >Will take a look, afaik we fixed something like this not long ago but
> >perhaps you are talking about something different.
> >
> >> * The CIS in a CIG cannot be started one by one, or connected to same
> >> destination. The kernel appears to wait until all CIS sockets in same
> >> CIG go to connect state before proceeding to create CIS. The spec does
> >> not seem to require this (I have some pre-rfc patches to make it more
> >> flexible [1].)
> >
> >It used to be like that but I actually have to fix it because the
> >controller don't accept multiple CreateCIS in parallel:
> >
> >https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
> >
> >And it would actually create a relatively big window if we queue and
> >wait for CIS established, because then the controller may set up its
> >scheduling without taking into account the second CIS which will then
> >fail when it comes to its setup, so I think it is better to program
> >them together to avoid having only one side working.
>
> Hmm, I made it queue and wait for the previous Create CIS to fully complete before emitting the next one, and that did seem to also work (also with TWS playback to the Samsung device).
> However did not extensively test, so even if allowed by spec risks running to controller issues?
>
> The problem here is that the second CIS is not necessarily going to come, as it may be unrelated device put to same CIG since controller doesn't support multiple CIG.
>
> In principle sound server can acquire always all CIS, but then we should expose also the cig/cis properties on transports so they know which ones are needed.
>
> It could also make sense for BlueZ do it, when any cis in cig is started.
>
> >Btw, take a look at how it was done with bluetoothctl>
> >transport.acquire <transport_left> <transport_right>, we have been
> >able to use it to acquire both left/right earbuds and then send
> >pre-encoded files.
> >
> >> * PW currently does transport acquires synchronously and fails because
> >> of that with multiple CIS, but it probably should do them async.
> >>
> >>
> >> [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
Did you make any progress on these changes above? Looks like the pull
request is still open, I wonder if you hit some blocker?
> >> [1] https://github.com/pv/linux/commits/iso-fix-multicis
> >> [2] https://github.com/pv/linux/commits/iso-timestamp
> >> [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
> >>
> >> --
> >> Pauli Virtanen
> >
> >
> >
>
> Hi,
--
Luiz Augusto von Dentz
Hi Luiz,
ke, 2023-04-05 kello 17:16 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Mon, Mar 13, 2023 at 6:05 PM Pauli Virtanen <[email protected]> wrote:
> >
> > 14. maaliskuuta 2023 2.18.21 GMT+02:00 Luiz Augusto von Dentz <[email protected]> kirjoitti:
> > > Hi Pauli,
> > >
> > > On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
> > > >
> > > > Hi,
> > > >
> > > > su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> > > > > Hi Pauli, Frederic,
> > > > >
> > > > > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> > > > > >
> > > > > > Hello:
> > > > > >
> > > > > > This series was applied to bluetooth/bluez.git (master)
> > > > > > by Luiz Augusto von Dentz <[email protected]>:
> > > > > >
> > > > > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > > > > > > From: Luiz Augusto von Dentz <[email protected]>
> > > > > > >
> > > > > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > > > > > > the following steps:
> > > > > > >
> > > > > > > - Generate a hash (k) using the str as input
> > > > > > > - Generate a hash (sirk) using vendor, product, version and source as input
> > > > > > > - Encrypt sirk using k as LTK with sef function.
> > > > > > >
> > > > > > > [...]
> > > > > >
> > > > > > Here is the summary with links:
> > > > > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> > > > > > - [RFC,v2,02/12] shared/ad: Add RSI data type
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> > > > > > - [RFC,v2,03/12] doc: Add set-api
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> > > > > > - [RFC,v2,04/12] device-api: Add Set property
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> > > > > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> > > > > > (no matching commit)
> > > > > > - [RFC,v2,06/12] core: Check if device has RSI
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> > > > > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> > > > > > (no matching commit)
> > > > > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> > > > > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> > > > > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> > > > > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> > > > > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> > > > > >
> > > > > > You are awesome, thank you!
> > > > > > --
> > > > > > Deet-doot-dot, I am a bot.
> > > > > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > > > >
> > > > > Let me know if you guys are happy with this interface to detect
> > > > > Coordinated Sets, it still experimental so we can experiment with it
> > > > > until we think it is stable, regarding the implementation of the
> > > > > transports one major difference here is that we will need to encode
> > > > > left and right separately, not sure how hard it is to do that in
> > > > > pipewire?
> > > >
> > > > As far as the device set DBus interface is concerned, it seems to work
> > > > fine for me currently (in wip implementation for PW [0]). Don't right
> > > > now see something that would need to be added/changed in it.
> > > >
> > > > Channel splitting/merging is generally easy in PW. How the playback
> > > > synchronization is going to work on socket level may determine a bit at
> > > > what level in PW it is convenient to do though.
> > > >
> > > >
> > > > ---
> > > >
> > > > Laundry list for PW related to this:
> > > >
> > > > * How to do TX syncronization properly with the ISO sockets needs still
> > > > some thinking. I have some wip patches [2] that add the timestamps and
> > > > other socket API that provide timing information to allow
> > > > synchronization to the Number of Completed packets events.
> > > > Corresponding Pipewire implementation [3] rate matches to keep the time
> > > > difference between those events and our audio reference time fixed at
> > > > e.g. 25ms (2 packets in controller). Not really clear yet if this is a
> > > > right thing to do to help the controller send packets at the right
> > > > time.
> > >
> > > I have to check with our controller folks, I do recall someone saying
> > > that perhaps we should use framed instead of unframed so the
> > > controller can better keep up with timings, but it is not yet clear
> > > why.
> > >
> > > > Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
> > > > values in Command Complete in btmon for running CIS, so that command
> > > > doesn't seem to help here.
> > >
> > > Yeah, I don't think it is implemented yet.
> > >
> > > > * BlueZ doesn't seem to pass on the PAC audio location it reads via
> > > > read_sink/source_pac_loc, probably very easy to fix.
> > >
> > > Will take a look, afaik we fixed something like this not long ago but
> > > perhaps you are talking about something different.
> > >
> > > > * The CIS in a CIG cannot be started one by one, or connected to same
> > > > destination. The kernel appears to wait until all CIS sockets in same
> > > > CIG go to connect state before proceeding to create CIS. The spec does
> > > > not seem to require this (I have some pre-rfc patches to make it more
> > > > flexible [1].)
> > >
> > > It used to be like that but I actually have to fix it because the
> > > controller don't accept multiple CreateCIS in parallel:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
> > >
> > > And it would actually create a relatively big window if we queue and
> > > wait for CIS established, because then the controller may set up its
> > > scheduling without taking into account the second CIS which will then
> > > fail when it comes to its setup, so I think it is better to program
> > > them together to avoid having only one side working.
> >
> > Hmm, I made it queue and wait for the previous Create CIS to fully complete before emitting the next one, and that did seem to also work (also with TWS playback to the Samsung device).
> > However did not extensively test, so even if allowed by spec risks running to controller issues?
> >
> > The problem here is that the second CIS is not necessarily going to come, as it may be unrelated device put to same CIG since controller doesn't support multiple CIG.
> >
> > In principle sound server can acquire always all CIS, but then we should expose also the cig/cis properties on transports so they know which ones are needed.
> >
> > It could also make sense for BlueZ do it, when any cis in cig is started.
> >
> > > Btw, take a look at how it was done with bluetoothctl>
> > > transport.acquire <transport_left> <transport_right>, we have been
> > > able to use it to acquire both left/right earbuds and then send
> > > pre-encoded files.
> > >
> > > > * PW currently does transport acquires synchronously and fails because
> > > > of that with multiple CIS, but it probably should do them async.
> > > >
> > > >
> > > > [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
>
> Did you make any progress on these changes above? Looks like the pull
> request is still open, I wonder if you hit some blocker?
The device set part is fine now.
However, I've been first looking at sorting out TWS playback
synchronization, and ran to some issues there.
Current laundry list:
(1) The playback to the left/right earbuds can rarely desynchronize
spontaneously during playback, even though we send packets to both ISO
socket fds at the same time on ISO interval schedule.
Possibly, it goes to a state where the two CIS are off by one packet
due to some queue on controller side. Desynchronization by 10ms can be
heard easily in TWS earbuds as it transforms click at stereo center to
separate clicks on left and right as brain interprets it.
It usually resynchronizes if we halt sending packets to both CIS for ~
50+ ms and then continue, and sometimes it resynchronizes
spontaneously.
I've been looking at the conn->sent values at packet completion event
time, but it is not clear it's possible to distinguish the
desynchronized state from synchronized one, as it appears in both you
can have either 0 or 1 and occasionally 2 packets not yet acked by the
controller.
(2) With the Samsung device, Intel AX210 fails LE Create CIS for the
CIG with both earbuds with current kernel (btmon logs below). It works
if only one device is connected. As a workaround, I'm currently using a
patch [1] that changes it to do LE Create CIS for the CIG one by one.
Since that works, could be some controller issue.
(3) The playback from Intel AX210 -> Samsung device apparently has some
issue with the radio link/etc, as playback sometimes has crackling and
distortion. I thought earlier this was some TX sync issue on sender
side, but keeping 1+ packets queued in controller all the time did not
solve it. Since also RX appears to miss packets, it's maybe not related
to that after all.
(4) The Samsung device has some issue in that disabling source ASEs
does not complete. Workaround is to never release transports, but would
need to take a look again at this to be sure if it's only device issue.
(5) Sometimes when connecting one earbud, the other earbud fails to
connect. Haven't yet looked into this.
Because of (2) and (3), I'm not sure now if (1) is general issue or
only for this device combination. However, how things are done right
now, there doesn't seem to be a way for the user to detect CIS
desynchronization or recover from it.
Some things I tried: setting matching sequence numbers to the HCI ISO
packets for the two CIS, but doesn't seem to do anything (maybe
controller ignores them?). Setting packet timestamps does something,
but appears to require synchronizing to the controller clock to produce
any audio, which cannot be done since the controller does not have
working LE Read ISO TX Sync.
I also thought that maybe kernel hci_sched_iso could cause it, but at
least changing it to send packets to controller in sequence number
(provided by PW) order didn't seem to change anything.
So right now at least this TWS device usually works OK, but not yet
robustly enough to be non-experimental feature.
The Intel AX210 problem:
[ 1627.217064] usb 1-5: new full-speed USB device number 16 using xhci_hcd
[ 1627.370401] usb 1-5: New USB device found, idVendor=8087, idProduct=0032, bcdDevice= 0.00
[ 1627.370408] usb 1-5: New USB device strings: Mfr=0, Product=0, SerialNumber=0
[ 1627.374671] Bluetooth: hci0: Device revision is 0
[ 1627.374676] Bluetooth: hci0: Secure boot is enabled
[ 1627.374678] Bluetooth: hci0: OTP lock is enabled
[ 1627.374680] Bluetooth: hci0: API lock is enabled
[ 1627.374681] Bluetooth: hci0: Debug lock is disabled
[ 1627.374683] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
[ 1627.374685] Bluetooth: hci0: Bootloader timestamp 2019.40 buildtype 1 build 38
[ 1627.410381] Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
[ 1627.410446] Bluetooth: hci0: Boot Address: 0x100800
[ 1627.410447] Bluetooth: hci0: Firmware Version: 107-51.22
[ 1629.568723] Bluetooth: hci0: Waiting for firmware download to complete
[ 1629.569770] Bluetooth: hci0: Firmware loaded in 2108786 usecs
[ 1629.570011] Bluetooth: hci0: Waiting for device to boot
[ 1629.595622] Bluetooth: hci0: Device booted in 25187 usecs
[ 1629.595647] Bluetooth: hci0: Malformed MSFT vendor event: 0x02
[ 1629.595923] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0041-0041.ddc
[ 1629.597711] Bluetooth: hci0: Applying Intel DDC parameters completed
[ 1629.600707] Bluetooth: hci0: Firmware timestamp 2022.51 buildtype 1 build 56683
[ 1629.677307] Bluetooth: MGMT ver 1.22
< HCI Command: LE Set Connect.. (0x08|0x0062) plen 33 #16903 [hci0] 802.544515
CIG ID: 0x00
Central to Peripheral SDU Interval: 10000 us (0x002710)
Peripheral to Central SDU Interval: 10000 us (0x002710)
SCA: 201 - 500 ppm (0x00)
Packing: Sequential (0x00)
Framing: Unframed (0x00)
Central to Peripheral Maximum Latency: 20 ms (0x0014)
Peripheral to Central Maximum Latency: 20 ms (0x0014)
Number of CIS: 2
CIS ID: 0x00
Central to Peripheral Maximum SDU Size: 120
Peripheral to Central Maximum SDU Size: 120
Central to Peripheral PHY: LE 2M (0x02)
Peripheral to Central PHY: LE 2M (0x02)
Central to Peripheral Retransmission attempts: 0x05
Peripheral to Central Retransmission attempts: 0x05
CIS ID: 0x01
Central to Peripheral Maximum SDU Size: 120
Peripheral to Central Maximum SDU Size: 120
Central to Peripheral PHY: LE 2M (0x02)
Peripheral to Central PHY: LE 2M (0x02)
Central to Peripheral Retransmission attempts: 0x05
Peripheral to Central Retransmission attempts: 0x05
> HCI Event: Command Complete (0x0e) plen 10 #16904 [hci0] 802.547134
LE Set Connected Isochronous Group Parameters (0x08|0x0062) ncmd 1
Status: Success (0x00)
CIG ID: 0x00
Number of Handles: 2
Connection Handle #0: 2560
Connection Handle #1: 2561
...
< HCI Command: LE Create Conne.. (0x08|0x0064) plen 9 #16928 [hci0] 833.626566
Number of CIS: 2
CIS Handle: 2560
ACL Handle: 3585
CIS Handle: 2561
ACL Handle: 3586
> HCI Event: Command Status (0x0f) plen 4 #16929 [hci0] 833.628188
LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
Status: Success (0x00)
> HCI Event: LE Meta Event (0x3e) plen 29 #16930 [hci0] 833.670453
LE Connected Isochronous Stream Established (0x19)
Status: Unspecified Error (0x1f)
Connection Handle: 2561
CIG Synchronization Delay: 89856 us (0x015f00)
CIS Synchronization Delay: 4718843 us (0x4800fb)
Central to Peripheral Latency: 1 us (0x000001)
Peripheral to Central Latency: 0 us (0x000000)
Central to Peripheral PHY: Reserved (0x00)
Peripheral to Central PHY: Reserved (0x00)
Number of Subevents: 0
Central to Peripheral Burst Number: 0
Peripheral to Central Burst Number: 0
Central to Peripheral Flush Timeout: 0
Peripheral to Central Flush Timeout: 0
Central to Peripheral MTU: 1536
Peripheral to Central MTU: 0
ISO Interval: 10752
= bluetoothd: profiles/audio/bap.c:iso_connect_cb() connect to 2.. 833.670831
--
Pauli Virtanen
> > [1] https://github.com/pv/linux/commits/iso-fix-multicis
> > [2] https://github.com/pv/linux/commits/iso-timestamp
> > [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
Hi Pauli,
On Thu, Apr 6, 2023 at 11:08 AM Pauli Virtanen <[email protected]> wrote:
>
> Hi Luiz,
>
> ke, 2023-04-05 kello 17:16 -0700, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Mon, Mar 13, 2023 at 6:05 PM Pauli Virtanen <[email protected]> wrote:
> > >
> > > 14. maaliskuuta 2023 2.18.21 GMT+02:00 Luiz Augusto von Dentz <[email protected]> kirjoitti:
> > > > Hi Pauli,
> > > >
> > > > On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> > > > > > Hi Pauli, Frederic,
> > > > > >
> > > > > > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> > > > > > >
> > > > > > > Hello:
> > > > > > >
> > > > > > > This series was applied to bluetooth/bluez.git (master)
> > > > > > > by Luiz Augusto von Dentz <[email protected]>:
> > > > > > >
> > > > > > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > > > > > > > From: Luiz Augusto von Dentz <[email protected]>
> > > > > > > >
> > > > > > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > > > > > > > the following steps:
> > > > > > > >
> > > > > > > > - Generate a hash (k) using the str as input
> > > > > > > > - Generate a hash (sirk) using vendor, product, version and source as input
> > > > > > > > - Encrypt sirk using k as LTK with sef function.
> > > > > > > >
> > > > > > > > [...]
> > > > > > >
> > > > > > > Here is the summary with links:
> > > > > > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> > > > > > > - [RFC,v2,02/12] shared/ad: Add RSI data type
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> > > > > > > - [RFC,v2,03/12] doc: Add set-api
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> > > > > > > - [RFC,v2,04/12] device-api: Add Set property
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> > > > > > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> > > > > > > (no matching commit)
> > > > > > > - [RFC,v2,06/12] core: Check if device has RSI
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> > > > > > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> > > > > > > (no matching commit)
> > > > > > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> > > > > > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> > > > > > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> > > > > > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> > > > > > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> > > > > > >
> > > > > > > You are awesome, thank you!
> > > > > > > --
> > > > > > > Deet-doot-dot, I am a bot.
> > > > > > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > > > > >
> > > > > > Let me know if you guys are happy with this interface to detect
> > > > > > Coordinated Sets, it still experimental so we can experiment with it
> > > > > > until we think it is stable, regarding the implementation of the
> > > > > > transports one major difference here is that we will need to encode
> > > > > > left and right separately, not sure how hard it is to do that in
> > > > > > pipewire?
> > > > >
> > > > > As far as the device set DBus interface is concerned, it seems to work
> > > > > fine for me currently (in wip implementation for PW [0]). Don't right
> > > > > now see something that would need to be added/changed in it.
> > > > >
> > > > > Channel splitting/merging is generally easy in PW. How the playback
> > > > > synchronization is going to work on socket level may determine a bit at
> > > > > what level in PW it is convenient to do though.
> > > > >
> > > > >
> > > > > ---
> > > > >
> > > > > Laundry list for PW related to this:
> > > > >
> > > > > * How to do TX syncronization properly with the ISO sockets needs still
> > > > > some thinking. I have some wip patches [2] that add the timestamps and
> > > > > other socket API that provide timing information to allow
> > > > > synchronization to the Number of Completed packets events.
> > > > > Corresponding Pipewire implementation [3] rate matches to keep the time
> > > > > difference between those events and our audio reference time fixed at
> > > > > e.g. 25ms (2 packets in controller). Not really clear yet if this is a
> > > > > right thing to do to help the controller send packets at the right
> > > > > time.
> > > >
> > > > I have to check with our controller folks, I do recall someone saying
> > > > that perhaps we should use framed instead of unframed so the
> > > > controller can better keep up with timings, but it is not yet clear
> > > > why.
> > > >
> > > > > Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
> > > > > values in Command Complete in btmon for running CIS, so that command
> > > > > doesn't seem to help here.
> > > >
> > > > Yeah, I don't think it is implemented yet.
> > > >
> > > > > * BlueZ doesn't seem to pass on the PAC audio location it reads via
> > > > > read_sink/source_pac_loc, probably very easy to fix.
> > > >
> > > > Will take a look, afaik we fixed something like this not long ago but
> > > > perhaps you are talking about something different.
> > > >
> > > > > * The CIS in a CIG cannot be started one by one, or connected to same
> > > > > destination. The kernel appears to wait until all CIS sockets in same
> > > > > CIG go to connect state before proceeding to create CIS. The spec does
> > > > > not seem to require this (I have some pre-rfc patches to make it more
> > > > > flexible [1].)
> > > >
> > > > It used to be like that but I actually have to fix it because the
> > > > controller don't accept multiple CreateCIS in parallel:
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
> > > >
> > > > And it would actually create a relatively big window if we queue and
> > > > wait for CIS established, because then the controller may set up its
> > > > scheduling without taking into account the second CIS which will then
> > > > fail when it comes to its setup, so I think it is better to program
> > > > them together to avoid having only one side working.
> > >
> > > Hmm, I made it queue and wait for the previous Create CIS to fully complete before emitting the next one, and that did seem to also work (also with TWS playback to the Samsung device).
> > > However did not extensively test, so even if allowed by spec risks running to controller issues?
> > >
> > > The problem here is that the second CIS is not necessarily going to come, as it may be unrelated device put to same CIG since controller doesn't support multiple CIG.
> > >
> > > In principle sound server can acquire always all CIS, but then we should expose also the cig/cis properties on transports so they know which ones are needed.
> > >
> > > It could also make sense for BlueZ do it, when any cis in cig is started.
> > >
> > > > Btw, take a look at how it was done with bluetoothctl>
> > > > transport.acquire <transport_left> <transport_right>, we have been
> > > > able to use it to acquire both left/right earbuds and then send
> > > > pre-encoded files.
> > > >
> > > > > * PW currently does transport acquires synchronously and fails because
> > > > > of that with multiple CIS, but it probably should do them async.
> > > > >
> > > > >
> > > > > [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
> >
> > Did you make any progress on these changes above? Looks like the pull
> > request is still open, I wonder if you hit some blocker?
>
> The device set part is fine now.
>
> However, I've been first looking at sorting out TWS playback
> synchronization, and ran to some issues there.
>
>
> Current laundry list:
>
> (1) The playback to the left/right earbuds can rarely desynchronize
> spontaneously during playback, even though we send packets to both ISO
> socket fds at the same time on ISO interval schedule.
>
> Possibly, it goes to a state where the two CIS are off by one packet
> due to some queue on controller side. Desynchronization by 10ms can be
> heard easily in TWS earbuds as it transforms click at stereo center to
> separate clicks on left and right as brain interprets it.
>
> It usually resynchronizes if we halt sending packets to both CIS for ~
> 50+ ms and then continue, and sometimes it resynchronizes
> spontaneously.
>
> I've been looking at the conn->sent values at packet completion event
> time, but it is not clear it's possible to distinguish the
> desynchronized state from synchronized one, as it appears in both you
> can have either 0 or 1 and occasionally 2 packets not yet acked by the
> controller.
Interesting, what is the presentation delay configured, in theory that
should be used to synchronize the streams, we may need a way to
measure this latency and attempt to compensate for it somehow if
transfer speed variates.
>
> (2) With the Samsung device, Intel AX210 fails LE Create CIS for the
> CIG with both earbuds with current kernel (btmon logs below). It works
> if only one device is connected. As a workaround, I'm currently using a
> patch [1] that changes it to do LE Create CIS for the CIG one by one.
> Since that works, could be some controller issue.
I believe we fixed that, are you running with the latest firmware and
bluetooth-next? On the kernel side you may need the following:
https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=70eff70d453bdb529d8233f59e32c1ffedac7685
>
> (3) The playback from Intel AX210 -> Samsung device apparently has some
> issue with the radio link/etc, as playback sometimes has crackling and
> distortion. I thought earlier this was some TX sync issue on sender
> side, but keeping 1+ packets queued in controller all the time did not
> solve it. Since also RX appears to miss packets, it's maybe not related
> to that after all.
This might be hard to figure out without air traces, will need to
reproduce it internally.
>
> (4) The Samsung device has some issue in that disabling source ASEs
> does not complete. Workaround is to never release transports, but would
> need to take a look again at this to be sure if it's only device issue.
Make sure you have btmon to collect the traces when this happens.
>
> (5) Sometimes when connecting one earbud, the other earbud fails to
> connect. Haven't yet looked into this.
Ive seen that, this may actually be something like:
https://github.com/bluez/bluez/issues/340#issuecomment-1494877679
This is less pronounced when we use the auto-connect list, since that
attempts to connect whenever it sees the device advertising so it will
continue to attempt multiple times, with the latest bluez tree this is
now the default behavior:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=ca07d198f9c7d289e95091c30ed15bff2106a7db
>
> Because of (2) and (3), I'm not sure now if (1) is general issue or
> only for this device combination. However, how things are done right
> now, there doesn't seem to be a way for the user to detect CIS
> desynchronization or recover from it.
>
> Some things I tried: setting matching sequence numbers to the HCI ISO
> packets for the two CIS, but doesn't seem to do anything (maybe
> controller ignores them?). Setting packet timestamps does something,
> but appears to require synchronizing to the controller clock to produce
> any audio, which cannot be done since the controller does not have
> working LE Read ISO TX Sync.
>
> I also thought that maybe kernel hci_sched_iso could cause it, but at
> least changing it to send packets to controller in sequence number
> (provided by PW) order didn't seem to change anything.
>
> So right now at least this TWS device usually works OK, but not yet
> robustly enough to be non-experimental feature.
Yeah, I think there will be some time needed to mature the stacks and
firmware to match what we currently have in Classic, but it is great
to have this sort of feedback from the audio subsystem.
>
> The Intel AX210 problem:
>
> [ 1627.217064] usb 1-5: new full-speed USB device number 16 using xhci_hcd
> [ 1627.370401] usb 1-5: New USB device found, idVendor=8087, idProduct=0032, bcdDevice= 0.00
> [ 1627.370408] usb 1-5: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> [ 1627.374671] Bluetooth: hci0: Device revision is 0
> [ 1627.374676] Bluetooth: hci0: Secure boot is enabled
> [ 1627.374678] Bluetooth: hci0: OTP lock is enabled
> [ 1627.374680] Bluetooth: hci0: API lock is enabled
> [ 1627.374681] Bluetooth: hci0: Debug lock is disabled
> [ 1627.374683] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> [ 1627.374685] Bluetooth: hci0: Bootloader timestamp 2019.40 buildtype 1 build 38
> [ 1627.410381] Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
> [ 1627.410446] Bluetooth: hci0: Boot Address: 0x100800
> [ 1627.410447] Bluetooth: hci0: Firmware Version: 107-51.22
> [ 1629.568723] Bluetooth: hci0: Waiting for firmware download to complete
> [ 1629.569770] Bluetooth: hci0: Firmware loaded in 2108786 usecs
> [ 1629.570011] Bluetooth: hci0: Waiting for device to boot
> [ 1629.595622] Bluetooth: hci0: Device booted in 25187 usecs
> [ 1629.595647] Bluetooth: hci0: Malformed MSFT vendor event: 0x02
> [ 1629.595923] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0041-0041.ddc
> [ 1629.597711] Bluetooth: hci0: Applying Intel DDC parameters completed
> [ 1629.600707] Bluetooth: hci0: Firmware timestamp 2022.51 buildtype 1 build 56683
> [ 1629.677307] Bluetooth: MGMT ver 1.22
>
> < HCI Command: LE Set Connect.. (0x08|0x0062) plen 33 #16903 [hci0] 802.544515
> CIG ID: 0x00
> Central to Peripheral SDU Interval: 10000 us (0x002710)
> Peripheral to Central SDU Interval: 10000 us (0x002710)
> SCA: 201 - 500 ppm (0x00)
> Packing: Sequential (0x00)
> Framing: Unframed (0x00)
> Central to Peripheral Maximum Latency: 20 ms (0x0014)
> Peripheral to Central Maximum Latency: 20 ms (0x0014)
> Number of CIS: 2
> CIS ID: 0x00
> Central to Peripheral Maximum SDU Size: 120
> Peripheral to Central Maximum SDU Size: 120
> Central to Peripheral PHY: LE 2M (0x02)
> Peripheral to Central PHY: LE 2M (0x02)
> Central to Peripheral Retransmission attempts: 0x05
> Peripheral to Central Retransmission attempts: 0x05
> CIS ID: 0x01
> Central to Peripheral Maximum SDU Size: 120
> Peripheral to Central Maximum SDU Size: 120
> Central to Peripheral PHY: LE 2M (0x02)
> Peripheral to Central PHY: LE 2M (0x02)
> Central to Peripheral Retransmission attempts: 0x05
> Peripheral to Central Retransmission attempts: 0x05
> > HCI Event: Command Complete (0x0e) plen 10 #16904 [hci0] 802.547134
> LE Set Connected Isochronous Group Parameters (0x08|0x0062) ncmd 1
> Status: Success (0x00)
> CIG ID: 0x00
> Number of Handles: 2
> Connection Handle #0: 2560
> Connection Handle #1: 2561
> ...
> < HCI Command: LE Create Conne.. (0x08|0x0064) plen 9 #16928 [hci0] 833.626566
> Number of CIS: 2
> CIS Handle: 2560
> ACL Handle: 3585
> CIS Handle: 2561
> ACL Handle: 3586
> > HCI Event: Command Status (0x0f) plen 4 #16929 [hci0] 833.628188
> LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
> Status: Success (0x00)
> > HCI Event: LE Meta Event (0x3e) plen 29 #16930 [hci0] 833.670453
> LE Connected Isochronous Stream Established (0x19)
> Status: Unspecified Error (0x1f)
> Connection Handle: 2561
> CIG Synchronization Delay: 89856 us (0x015f00)
> CIS Synchronization Delay: 4718843 us (0x4800fb)
> Central to Peripheral Latency: 1 us (0x000001)
> Peripheral to Central Latency: 0 us (0x000000)
> Central to Peripheral PHY: Reserved (0x00)
> Peripheral to Central PHY: Reserved (0x00)
> Number of Subevents: 0
> Central to Peripheral Burst Number: 0
> Peripheral to Central Burst Number: 0
> Central to Peripheral Flush Timeout: 0
> Peripheral to Central Flush Timeout: 0
> Central to Peripheral MTU: 1536
> Peripheral to Central MTU: 0
> ISO Interval: 10752
> = bluetoothd: profiles/audio/bap.c:iso_connect_cb() connect to 2.. 833.670831
>
> --
> Pauli Virtanen
>
>
>
> > > [1] https://github.com/pv/linux/commits/iso-fix-multicis
> > > [2] https://github.com/pv/linux/commits/iso-timestamp
> > > [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
>
https://gitlab.freedesktop.org/pvir/pipewire/-/commit/667ab3c693e0425568ac405a6a311754ed798653
In the isotest/bluetoothctl we actually use the latency/interval to
determine how many packets we have to send at each ISO interval:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/isotest.c#n704
--
Luiz Augusto von Dentz
Hi Pauli,
On Thu, Apr 6, 2023 at 1:14 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Pauli,
>
> On Thu, Apr 6, 2023 at 11:08 AM Pauli Virtanen <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > ke, 2023-04-05 kello 17:16 -0700, Luiz Augusto von Dentz kirjoitti:
> > > Hi Pauli,
> > >
> > > On Mon, Mar 13, 2023 at 6:05 PM Pauli Virtanen <[email protected]> wrote:
> > > >
> > > > 14. maaliskuuta 2023 2.18.21 GMT+02:00 Luiz Augusto von Dentz <[email protected]> kirjoitti:
> > > > > Hi Pauli,
> > > > >
> > > > > On Mon, Mar 13, 2023 at 4:30 PM Pauli Virtanen <[email protected]> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > su, 2023-03-12 kello 22:36 -0700, Luiz Augusto von Dentz kirjoitti:
> > > > > > > Hi Pauli, Frederic,
> > > > > > >
> > > > > > > On Fri, Mar 10, 2023 at 4:40 PM <[email protected]> wrote:
> > > > > > > >
> > > > > > > > Hello:
> > > > > > > >
> > > > > > > > This series was applied to bluetooth/bluez.git (master)
> > > > > > > > by Luiz Augusto von Dentz <[email protected]>:
> > > > > > > >
> > > > > > > > On Tue, 7 Mar 2023 14:24:11 -0800 you wrote:
> > > > > > > > > From: Luiz Augusto von Dentz <[email protected]>
> > > > > > > > >
> > > > > > > > > This adds bt_crypto_sirk which attempts to generate a unique SIRK using
> > > > > > > > > the following steps:
> > > > > > > > >
> > > > > > > > > - Generate a hash (k) using the str as input
> > > > > > > > > - Generate a hash (sirk) using vendor, product, version and source as input
> > > > > > > > > - Encrypt sirk using k as LTK with sef function.
> > > > > > > > >
> > > > > > > > > [...]
> > > > > > > >
> > > > > > > > Here is the summary with links:
> > > > > > > > - [RFC,v2,01/12] shared/crypto: Add bt_crypto_sirk
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1dd94cc7f81
> > > > > > > > - [RFC,v2,02/12] shared/ad: Add RSI data type
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c2e99aefd337
> > > > > > > > - [RFC,v2,03/12] doc: Add set-api
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6477522e92e3
> > > > > > > > - [RFC,v2,04/12] device-api: Add Set property
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5bac4cddb191
> > > > > > > > - [RFC,v2,05/12] core: Add initial implementation of DeviceSet interface
> > > > > > > > (no matching commit)
> > > > > > > > - [RFC,v2,06/12] core: Check if device has RSI
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f95ffcc8b1fe
> > > > > > > > - [RFC,v2,07/12] main.conf: Add CSIP profile configurable options
> > > > > > > > (no matching commit)
> > > > > > > > - [RFC,v2,08/12] shared/csip: Add initial code for handling CSIP
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d297a03b7a61
> > > > > > > > - [RFC,v2,09/12] profiles: Add initial code for csip plugin
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9e1eb0a62b3f
> > > > > > > > - [RFC,v2,10/12] tools: Add support to generate RSI using SIRK
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c446a64d461b
> > > > > > > > - [RFC,v2,11/12] client: Add support for DeviceSet proxy
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=373bafc34ce6
> > > > > > > > - [RFC,v2,12/12] client: Use AdvertisingFlags when available
> > > > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=815f779aa8e4
> > > > > > > >
> > > > > > > > You are awesome, thank you!
> > > > > > > > --
> > > > > > > > Deet-doot-dot, I am a bot.
> > > > > > > > https://korg.docs.kernel.org/patchwork/pwbot.html
> > > > > > >
> > > > > > > Let me know if you guys are happy with this interface to detect
> > > > > > > Coordinated Sets, it still experimental so we can experiment with it
> > > > > > > until we think it is stable, regarding the implementation of the
> > > > > > > transports one major difference here is that we will need to encode
> > > > > > > left and right separately, not sure how hard it is to do that in
> > > > > > > pipewire?
> > > > > >
> > > > > > As far as the device set DBus interface is concerned, it seems to work
> > > > > > fine for me currently (in wip implementation for PW [0]). Don't right
> > > > > > now see something that would need to be added/changed in it.
> > > > > >
> > > > > > Channel splitting/merging is generally easy in PW. How the playback
> > > > > > synchronization is going to work on socket level may determine a bit at
> > > > > > what level in PW it is convenient to do though.
> > > > > >
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Laundry list for PW related to this:
> > > > > >
> > > > > > * How to do TX syncronization properly with the ISO sockets needs still
> > > > > > some thinking. I have some wip patches [2] that add the timestamps and
> > > > > > other socket API that provide timing information to allow
> > > > > > synchronization to the Number of Completed packets events.
> > > > > > Corresponding Pipewire implementation [3] rate matches to keep the time
> > > > > > difference between those events and our audio reference time fixed at
> > > > > > e.g. 25ms (2 packets in controller). Not really clear yet if this is a
> > > > > > right thing to do to help the controller send packets at the right
> > > > > > time.
> > > > >
> > > > > I have to check with our controller folks, I do recall someone saying
> > > > > that perhaps we should use framed instead of unframed so the
> > > > > controller can better keep up with timings, but it is not yet clear
> > > > > why.
> > > > >
> > > > > > Here I see LE Read ISO TX Sync with Intel AX210 returning only zero
> > > > > > values in Command Complete in btmon for running CIS, so that command
> > > > > > doesn't seem to help here.
> > > > >
> > > > > Yeah, I don't think it is implemented yet.
> > > > >
> > > > > > * BlueZ doesn't seem to pass on the PAC audio location it reads via
> > > > > > read_sink/source_pac_loc, probably very easy to fix.
> > > > >
> > > > > Will take a look, afaik we fixed something like this not long ago but
> > > > > perhaps you are talking about something different.
> > > > >
> > > > > > * The CIS in a CIG cannot be started one by one, or connected to same
> > > > > > destination. The kernel appears to wait until all CIS sockets in same
> > > > > > CIG go to connect state before proceeding to create CIS. The spec does
> > > > > > not seem to require this (I have some pre-rfc patches to make it more
> > > > > > flexible [1].)
> > > > >
> > > > > It used to be like that but I actually have to fix it because the
> > > > > controller don't accept multiple CreateCIS in parallel:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/tree/net/bluetooth/hci_conn.c#n1907
> > > > >
> > > > > And it would actually create a relatively big window if we queue and
> > > > > wait for CIS established, because then the controller may set up its
> > > > > scheduling without taking into account the second CIS which will then
> > > > > fail when it comes to its setup, so I think it is better to program
> > > > > them together to avoid having only one side working.
> > > >
> > > > Hmm, I made it queue and wait for the previous Create CIS to fully complete before emitting the next one, and that did seem to also work (also with TWS playback to the Samsung device).
> > > > However did not extensively test, so even if allowed by spec risks running to controller issues?
> > > >
> > > > The problem here is that the second CIS is not necessarily going to come, as it may be unrelated device put to same CIG since controller doesn't support multiple CIG.
> > > >
> > > > In principle sound server can acquire always all CIS, but then we should expose also the cig/cis properties on transports so they know which ones are needed.
> > > >
> > > > It could also make sense for BlueZ do it, when any cis in cig is started.
> > > >
> > > > > Btw, take a look at how it was done with bluetoothctl>
> > > > > transport.acquire <transport_left> <transport_right>, we have been
> > > > > able to use it to acquire both left/right earbuds and then send
> > > > > pre-encoded files.
> > > > >
> > > > > > * PW currently does transport acquires synchronously and fails because
> > > > > > of that with multiple CIS, but it probably should do them async.
> > > > > >
> > > > > >
> > > > > > [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
> > >
> > > Did you make any progress on these changes above? Looks like the pull
> > > request is still open, I wonder if you hit some blocker?
> >
> > The device set part is fine now.
> >
> > However, I've been first looking at sorting out TWS playback
> > synchronization, and ran to some issues there.
> >
> >
> > Current laundry list:
> >
> > (1) The playback to the left/right earbuds can rarely desynchronize
> > spontaneously during playback, even though we send packets to both ISO
> > socket fds at the same time on ISO interval schedule.
> >
> > Possibly, it goes to a state where the two CIS are off by one packet
> > due to some queue on controller side. Desynchronization by 10ms can be
> > heard easily in TWS earbuds as it transforms click at stereo center to
> > separate clicks on left and right as brain interprets it.
> >
> > It usually resynchronizes if we halt sending packets to both CIS for ~
> > 50+ ms and then continue, and sometimes it resynchronizes
> > spontaneously.
> >
> > I've been looking at the conn->sent values at packet completion event
> > time, but it is not clear it's possible to distinguish the
> > desynchronized state from synchronized one, as it appears in both you
> > can have either 0 or 1 and occasionally 2 packets not yet acked by the
> > controller.
>
> Interesting, what is the presentation delay configured, in theory that
> should be used to synchronize the streams, we may need a way to
> measure this latency and attempt to compensate for it somehow if
> transfer speed variates.
>
> >
> > (2) With the Samsung device, Intel AX210 fails LE Create CIS for the
> > CIG with both earbuds with current kernel (btmon logs below). It works
> > if only one device is connected. As a workaround, I'm currently using a
> > patch [1] that changes it to do LE Create CIS for the CIG one by one.
> > Since that works, could be some controller issue.
>
> I believe we fixed that, are you running with the latest firmware and
> bluetooth-next? On the kernel side you may need the following:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=70eff70d453bdb529d8233f59e32c1ffedac7685
>
> >
> > (3) The playback from Intel AX210 -> Samsung device apparently has some
> > issue with the radio link/etc, as playback sometimes has crackling and
> > distortion. I thought earlier this was some TX sync issue on sender
> > side, but keeping 1+ packets queued in controller all the time did not
> > solve it. Since also RX appears to miss packets, it's maybe not related
> > to that after all.
>
> This might be hard to figure out without air traces, will need to
> reproduce it internally.
>
> >
> > (4) The Samsung device has some issue in that disabling source ASEs
> > does not complete. Workaround is to never release transports, but would
> > need to take a look again at this to be sure if it's only device issue.
>
> Make sure you have btmon to collect the traces when this happens.
>
> >
> > (5) Sometimes when connecting one earbud, the other earbud fails to
> > connect. Haven't yet looked into this.
>
> Ive seen that, this may actually be something like:
>
> https://github.com/bluez/bluez/issues/340#issuecomment-1494877679
>
> This is less pronounced when we use the auto-connect list, since that
> attempts to connect whenever it sees the device advertising so it will
> continue to attempt multiple times, with the latest bluez tree this is
> now the default behavior:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=ca07d198f9c7d289e95091c30ed15bff2106a7db
>
> >
> > Because of (2) and (3), I'm not sure now if (1) is general issue or
> > only for this device combination. However, how things are done right
> > now, there doesn't seem to be a way for the user to detect CIS
> > desynchronization or recover from it.
> >
> > Some things I tried: setting matching sequence numbers to the HCI ISO
> > packets for the two CIS, but doesn't seem to do anything (maybe
> > controller ignores them?). Setting packet timestamps does something,
> > but appears to require synchronizing to the controller clock to produce
> > any audio, which cannot be done since the controller does not have
> > working LE Read ISO TX Sync.
> >
> > I also thought that maybe kernel hci_sched_iso could cause it, but at
> > least changing it to send packets to controller in sequence number
> > (provided by PW) order didn't seem to change anything.
> >
> > So right now at least this TWS device usually works OK, but not yet
> > robustly enough to be non-experimental feature.
>
> Yeah, I think there will be some time needed to mature the stacks and
> firmware to match what we currently have in Classic, but it is great
> to have this sort of feedback from the audio subsystem.
>
> >
> > The Intel AX210 problem:
> >
> > [ 1627.217064] usb 1-5: new full-speed USB device number 16 using xhci_hcd
> > [ 1627.370401] usb 1-5: New USB device found, idVendor=8087, idProduct=0032, bcdDevice= 0.00
> > [ 1627.370408] usb 1-5: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> > [ 1627.374671] Bluetooth: hci0: Device revision is 0
> > [ 1627.374676] Bluetooth: hci0: Secure boot is enabled
> > [ 1627.374678] Bluetooth: hci0: OTP lock is enabled
> > [ 1627.374680] Bluetooth: hci0: API lock is enabled
> > [ 1627.374681] Bluetooth: hci0: Debug lock is disabled
> > [ 1627.374683] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> > [ 1627.374685] Bluetooth: hci0: Bootloader timestamp 2019.40 buildtype 1 build 38
> > [ 1627.410381] Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
> > [ 1627.410446] Bluetooth: hci0: Boot Address: 0x100800
> > [ 1627.410447] Bluetooth: hci0: Firmware Version: 107-51.22
> > [ 1629.568723] Bluetooth: hci0: Waiting for firmware download to complete
> > [ 1629.569770] Bluetooth: hci0: Firmware loaded in 2108786 usecs
> > [ 1629.570011] Bluetooth: hci0: Waiting for device to boot
> > [ 1629.595622] Bluetooth: hci0: Device booted in 25187 usecs
> > [ 1629.595647] Bluetooth: hci0: Malformed MSFT vendor event: 0x02
> > [ 1629.595923] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0041-0041.ddc
> > [ 1629.597711] Bluetooth: hci0: Applying Intel DDC parameters completed
> > [ 1629.600707] Bluetooth: hci0: Firmware timestamp 2022.51 buildtype 1 build 56683
> > [ 1629.677307] Bluetooth: MGMT ver 1.22
> >
> > < HCI Command: LE Set Connect.. (0x08|0x0062) plen 33 #16903 [hci0] 802.544515
> > CIG ID: 0x00
> > Central to Peripheral SDU Interval: 10000 us (0x002710)
> > Peripheral to Central SDU Interval: 10000 us (0x002710)
> > SCA: 201 - 500 ppm (0x00)
> > Packing: Sequential (0x00)
> > Framing: Unframed (0x00)
> > Central to Peripheral Maximum Latency: 20 ms (0x0014)
> > Peripheral to Central Maximum Latency: 20 ms (0x0014)
> > Number of CIS: 2
> > CIS ID: 0x00
> > Central to Peripheral Maximum SDU Size: 120
> > Peripheral to Central Maximum SDU Size: 120
> > Central to Peripheral PHY: LE 2M (0x02)
> > Peripheral to Central PHY: LE 2M (0x02)
> > Central to Peripheral Retransmission attempts: 0x05
> > Peripheral to Central Retransmission attempts: 0x05
> > CIS ID: 0x01
> > Central to Peripheral Maximum SDU Size: 120
> > Peripheral to Central Maximum SDU Size: 120
> > Central to Peripheral PHY: LE 2M (0x02)
> > Peripheral to Central PHY: LE 2M (0x02)
> > Central to Peripheral Retransmission attempts: 0x05
> > Peripheral to Central Retransmission attempts: 0x05
> > > HCI Event: Command Complete (0x0e) plen 10 #16904 [hci0] 802.547134
> > LE Set Connected Isochronous Group Parameters (0x08|0x0062) ncmd 1
> > Status: Success (0x00)
> > CIG ID: 0x00
> > Number of Handles: 2
> > Connection Handle #0: 2560
> > Connection Handle #1: 2561
> > ...
> > < HCI Command: LE Create Conne.. (0x08|0x0064) plen 9 #16928 [hci0] 833.626566
> > Number of CIS: 2
> > CIS Handle: 2560
> > ACL Handle: 3585
> > CIS Handle: 2561
> > ACL Handle: 3586
> > > HCI Event: Command Status (0x0f) plen 4 #16929 [hci0] 833.628188
> > LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
> > Status: Success (0x00)
> > > HCI Event: LE Meta Event (0x3e) plen 29 #16930 [hci0] 833.670453
> > LE Connected Isochronous Stream Established (0x19)
> > Status: Unspecified Error (0x1f)
> > Connection Handle: 2561
> > CIG Synchronization Delay: 89856 us (0x015f00)
> > CIS Synchronization Delay: 4718843 us (0x4800fb)
> > Central to Peripheral Latency: 1 us (0x000001)
> > Peripheral to Central Latency: 0 us (0x000000)
> > Central to Peripheral PHY: Reserved (0x00)
> > Peripheral to Central PHY: Reserved (0x00)
> > Number of Subevents: 0
> > Central to Peripheral Burst Number: 0
> > Peripheral to Central Burst Number: 0
> > Central to Peripheral Flush Timeout: 0
> > Peripheral to Central Flush Timeout: 0
> > Central to Peripheral MTU: 1536
> > Peripheral to Central MTU: 0
> > ISO Interval: 10752
> > = bluetoothd: profiles/audio/bap.c:iso_connect_cb() connect to 2.. 833.670831
> >
> > --
> > Pauli Virtanen
> >
> >
> >
> > > > [1] https://github.com/pv/linux/commits/iso-fix-multicis
> > > > [2] https://github.com/pv/linux/commits/iso-timestamp
> > > > [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
> >
>
> https://gitlab.freedesktop.org/pvir/pipewire/-/commit/667ab3c693e0425568ac405a6a311754ed798653
>
> In the isotest/bluetoothctl we actually use the latency/interval to
> determine how many packets we have to send at each ISO interval:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/isotest.c#n704
Btw, not sure you if are following the list but I'm working on adding
support for handling multiple CIS to the same peer:
https://patchwork.kernel.org/project/bluetooth/list/?series=739574
That should allow you to set a different CIS ID if you want to use
different sockets for input and output.
Id also would like to discuss how we do some test automation using
pipewire endpoints in the future, we probably want to enable it via
test-runner but we probably need to enable it loading pipewire daemons
etc under the vm that test-runner creates, Frederic started adding
some support but it doesn't look like it loads pipewire so Im not
really sure how it supposed to be loaded:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n1108
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n277
> --
> Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
Hi Luiz,
> Hi Pauli,
>
> On Thu, Apr 6, 2023 at 11:08 AM Pauli Virtanen <[email protected]> wrote:
> > ke, 2023-04-05 kello 17:16 -0700, Luiz Augusto von Dentz kirjoitti:
[clip]
> > > > [0] https://gitlab.freedesktop.org/pipewire/pipewire/-/merge_requests/1564
> > >
> > > Did you make any progress on these changes above? Looks like the pull
> > > request is still open, I wonder if you hit some blocker?
> >
> > The device set part is fine now.
> >
> > However, I've been first looking at sorting out TWS playback
> > synchronization, and ran to some issues there.
> >
> >
> > Current laundry list:
> >
> > (1) The playback to the left/right earbuds can rarely desynchronize
> > spontaneously during playback, even though we send packets to both ISO
> > socket fds at the same time on ISO interval schedule.
> >
> > Possibly, it goes to a state where the two CIS are off by one packet
> > due to some queue on controller side. Desynchronization by 10ms can be
> > heard easily in TWS earbuds as it transforms click at stereo center to
> > separate clicks on left and right as brain interprets it.
> >
> > It usually resynchronizes if we halt sending packets to both CIS for ~
> > 50+ ms and then continue, and sometimes it resynchronizes
> > spontaneously.
> >
> > I've been looking at the conn->sent values at packet completion event
> > time, but it is not clear it's possible to distinguish the
> > desynchronized state from synchronized one, as it appears in both you
> > can have either 0 or 1 and occasionally 2 packets not yet acked by the
> > controller.
>
> Interesting, what is the presentation delay configured, in theory that
> should be used to synchronize the streams, we may need a way to
> measure this latency and attempt to compensate for it somehow if
> transfer speed variates.
The presentation delay is configured as 40ms, the device also reports
max=min=40ms limits for the delay.
The CIG configuration is
< HCI Command: LE Set Connected.. (0x08|0x0062) plen 33 #686 [hci1]
CIG ID: 0x00
Central to Peripheral SDU Interval: 10000 us (0x002710)
Peripheral to Central SDU Interval: 10000 us (0x002710)
SCA: 201 - 500 ppm (0x00)
Packing: Sequential (0x00)
Framing: Unframed (0x00)
Central to Peripheral Maximum Latency: 100 ms (0x0064)
Peripheral to Central Maximum Latency: 100 ms (0x0064)
Number of CIS: 2
CIS ID: 0x00
Central to Peripheral Maximum SDU Size: 120
Peripheral to Central Maximum SDU Size: 120
Central to Peripheral PHY: LE 2M (0x02)
Peripheral to Central PHY: LE 2M (0x02)
Central to Peripheral Retransmission attempts: 0x05
Peripheral to Central Retransmission attempts: 0x05
CIS ID: 0x01
Central to Peripheral Maximum SDU Size: 120
Peripheral to Central Maximum SDU Size: 120
Central to Peripheral PHY: LE 2M (0x02)
Peripheral to Central PHY: LE 2M (0x02)
Central to Peripheral Retransmission attempts: 0x05
Peripheral to Central Retransmission attempts: 0x05
Sometimes the desynchronization appears immediately after connect, or
randomly during playback. It can also be part of the time be triggered
by walking away from the transmitter, or covering the earpieces by hands
until it starts to stutter. This last one is the most reliable reproducer,
but still not high success rate so a bit hard to debug.
In one case this persisted also over device disconnect + reconnect.
From the ISO socket user side, nothing unusual seems to happen, all
writes succeed so we're not dropping data. In one case I checked where
the playback was desynchronized after start, we were writing to both
sockets every 10ms with max +-0.2ms jitter.
Not sure how to debug this further right now. Maybe should check with
different devices.
> > (2) With the Samsung device, Intel AX210 fails LE Create CIS for the
> > CIG with both earbuds with current kernel (btmon logs below). It works
> > if only one device is connected. As a workaround, I'm currently using a
> > patch [1] that changes it to do LE Create CIS for the CIG one by one.
> > Since that works, could be some controller issue.
>
> I believe we fixed that, are you running with the latest firmware and
> bluetooth-next? On the kernel side you may need the following:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git/commit/?id=70eff70d453bdb529d8233f59e32c1ffedac7685
I missed that patch, seems to work fine with it.
> > (3) The playback from Intel AX210 -> Samsung device apparently has some
> > issue with the radio link/etc, as playback sometimes has crackling and
> > distortion. I thought earlier this was some TX sync issue on sender
> > side, but keeping 1+ packets queued in controller all the time did not
> > solve it. Since also RX appears to miss packets, it's maybe not related
> > to that after all.
>
> This might be hard to figure out without air traces, will need to
> reproduce it internally.
This might be partly due to bad QoS.
good:
< HCI Command: LE Set Connected.. (0x08|0x0062) plen 33 #686 [hci1]
...
Central to Peripheral Maximum Latency: 100 ms (0x0064)
Peripheral to Central Maximum Latency: 100 ms (0x0064)
bad:
< HCI Command: LE Set Connected.. (0x08|0x0062) plen 33 #862 [hci1]
...
Central to Peripheral Maximum Latency: 20 ms (0x0014)
Peripheral to Central Maximum Latency: 20 ms (0x0014)
In the "good" case there is sometimes crackling, but in the "bad" case
it's continuous.
However, also in the "good" case it seems we are not receiving all RX
packets we should get. The average RX rate in btmon seems to be 183
packets/s for two devices instead of 200/s. (Suspiciously close to
200*44100/48000...)
The "bad" case occurred because BlueZ does not pass the Latency parameter (or
any other QoS parameters) to SelectProperties if it has not connected to the
device before, so we ended up configuring the QoS with low-latency value from
BAP Table 5.2, which apparently the devices couldn't handle. (I switched this
now to use the "high-reliability" values from the table as fallback, but it's a
workaround.)
IIUC the max latency is supposed to be configured as
1. Target_Latency -> Config Codec -> Max_Transport_Latency
2. select good Max_Transport_Latency -> Config QoS
Currently SelectProperties is supposed to select QoS parameters, but it
is called before Config Codec, so Max_Transport_Latency, preferred
Presentation_Delay, etc. are not known at that time if bluez didn't
cache them somewhere.
I see there are some TODOs in BlueZ sources on this, so looks like known issue.
> > (4) The Samsung device has some issue in that disabling source ASEs
> > does not complete. Workaround is to never release transports, but would
> > need to take a look again at this to be sure if it's only device issue.
>
> Make sure you have btmon to collect the traces when this happens.
I'll try to come back to this.
> > (5) Sometimes when connecting one earbud, the other earbud fails to
> > connect. Haven't yet looked into this.
>
> Ive seen that, this may actually be something like:
>
> https://github.com/bluez/bluez/issues/340#issuecomment-1494877679
>
> This is less pronounced when we use the auto-connect list, since that
> attempts to connect whenever it sees the device advertising so it will
> continue to attempt multiple times, with the latest bluez tree this is
> now the default behavior:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=ca07d198f9c7d289e95091c30ed15bff2106a7db
>
> >
> > Because of (2) and (3), I'm not sure now if (1) is general issue or
> > only for this device combination. However, how things are done right
> > now, there doesn't seem to be a way for the user to detect CIS
> > desynchronization or recover from it.
> >
> > Some things I tried: setting matching sequence numbers to the HCI ISO
> > packets for the two CIS, but doesn't seem to do anything (maybe
> > controller ignores them?). Setting packet timestamps does something,
> > but appears to require synchronizing to the controller clock to produce
> > any audio, which cannot be done since the controller does not have
> > working LE Read ISO TX Sync.
> >
> > I also thought that maybe kernel hci_sched_iso could cause it, but at
> > least changing it to send packets to controller in sequence number
> > (provided by PW) order didn't seem to change anything.
> >
> > So right now at least this TWS device usually works OK, but not yet
> > robustly enough to be non-experimental feature.
>
> Yeah, I think there will be some time needed to mature the stacks and
> firmware to match what we currently have in Classic, but it is great
> to have this sort of feedback from the audio subsystem.
>
> >
> > The Intel AX210 problem:
> >
> > [ 1627.217064] usb 1-5: new full-speed USB device number 16 using xhci_hcd
> > [ 1627.370401] usb 1-5: New USB device found, idVendor=8087, idProduct=0032, bcdDevice= 0.00
> > [ 1627.370408] usb 1-5: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> > [ 1627.374671] Bluetooth: hci0: Device revision is 0
> > [ 1627.374676] Bluetooth: hci0: Secure boot is enabled
> > [ 1627.374678] Bluetooth: hci0: OTP lock is enabled
> > [ 1627.374680] Bluetooth: hci0: API lock is enabled
> > [ 1627.374681] Bluetooth: hci0: Debug lock is disabled
> > [ 1627.374683] Bluetooth: hci0: Minimum firmware build 1 week 10 2014
> > [ 1627.374685] Bluetooth: hci0: Bootloader timestamp 2019.40 buildtype 1 build 38
> > [ 1627.410381] Bluetooth: hci0: Found device firmware: intel/ibt-0041-0041.sfi
> > [ 1627.410446] Bluetooth: hci0: Boot Address: 0x100800
> > [ 1627.410447] Bluetooth: hci0: Firmware Version: 107-51.22
> > [ 1629.568723] Bluetooth: hci0: Waiting for firmware download to complete
> > [ 1629.569770] Bluetooth: hci0: Firmware loaded in 2108786 usecs
> > [ 1629.570011] Bluetooth: hci0: Waiting for device to boot
> > [ 1629.595622] Bluetooth: hci0: Device booted in 25187 usecs
> > [ 1629.595647] Bluetooth: hci0: Malformed MSFT vendor event: 0x02
> > [ 1629.595923] Bluetooth: hci0: Found Intel DDC parameters: intel/ibt-0041-0041.ddc
> > [ 1629.597711] Bluetooth: hci0: Applying Intel DDC parameters completed
> > [ 1629.600707] Bluetooth: hci0: Firmware timestamp 2022.51 buildtype 1 build 56683
> > [ 1629.677307] Bluetooth: MGMT ver 1.22
> >
> > < HCI Command: LE Set Connect.. (0x08|0x0062) plen 33 #16903 [hci0] 802.544515
> > CIG ID: 0x00
> > Central to Peripheral SDU Interval: 10000 us (0x002710)
> > Peripheral to Central SDU Interval: 10000 us (0x002710)
> > SCA: 201 - 500 ppm (0x00)
> > Packing: Sequential (0x00)
> > Framing: Unframed (0x00)
> > Central to Peripheral Maximum Latency: 20 ms (0x0014)
> > Peripheral to Central Maximum Latency: 20 ms (0x0014)
> > Number of CIS: 2
> > CIS ID: 0x00
> > Central to Peripheral Maximum SDU Size: 120
> > Peripheral to Central Maximum SDU Size: 120
> > Central to Peripheral PHY: LE 2M (0x02)
> > Peripheral to Central PHY: LE 2M (0x02)
> > Central to Peripheral Retransmission attempts: 0x05
> > Peripheral to Central Retransmission attempts: 0x05
> > CIS ID: 0x01
> > Central to Peripheral Maximum SDU Size: 120
> > Peripheral to Central Maximum SDU Size: 120
> > Central to Peripheral PHY: LE 2M (0x02)
> > Peripheral to Central PHY: LE 2M (0x02)
> > Central to Peripheral Retransmission attempts: 0x05
> > Peripheral to Central Retransmission attempts: 0x05
> > > HCI Event: Command Complete (0x0e) plen 10 #16904 [hci0] 802.547134
> > LE Set Connected Isochronous Group Parameters (0x08|0x0062) ncmd 1
> > Status: Success (0x00)
> > CIG ID: 0x00
> > Number of Handles: 2
> > Connection Handle #0: 2560
> > Connection Handle #1: 2561
> > ...
> > < HCI Command: LE Create Conne.. (0x08|0x0064) plen 9 #16928 [hci0] 833.626566
> > Number of CIS: 2
> > CIS Handle: 2560
> > ACL Handle: 3585
> > CIS Handle: 2561
> > ACL Handle: 3586
> > > HCI Event: Command Status (0x0f) plen 4 #16929 [hci0] 833.628188
> > LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
> > Status: Success (0x00)
> > > HCI Event: LE Meta Event (0x3e) plen 29 #16930 [hci0] 833.670453
> > LE Connected Isochronous Stream Established (0x19)
> > Status: Unspecified Error (0x1f)
> > Connection Handle: 2561
> > CIG Synchronization Delay: 89856 us (0x015f00)
> > CIS Synchronization Delay: 4718843 us (0x4800fb)
> > Central to Peripheral Latency: 1 us (0x000001)
> > Peripheral to Central Latency: 0 us (0x000000)
> > Central to Peripheral PHY: Reserved (0x00)
> > Peripheral to Central PHY: Reserved (0x00)
> > Number of Subevents: 0
> > Central to Peripheral Burst Number: 0
> > Peripheral to Central Burst Number: 0
> > Central to Peripheral Flush Timeout: 0
> > Peripheral to Central Flush Timeout: 0
> > Central to Peripheral MTU: 1536
> > Peripheral to Central MTU: 0
> > ISO Interval: 10752
> > = bluetoothd: profiles/audio/bap.c:iso_connect_cb() connect to 2.. 833.670831
> >
> > --
> > Pauli Virtanen
> >
> >
> >
> > > > [1] https://github.com/pv/linux/commits/iso-fix-multicis
> > > > [2] https://github.com/pv/linux/commits/iso-timestamp
> > > > [3] https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-timestamp-test
> >
>
> https://gitlab.freedesktop.org/pvir/pipewire/-/commit/667ab3c693e0425568ac405a6a311754ed798653
>
> In the isotest/bluetoothctl we actually use the latency/interval to
> determine how many packets we have to send at each ISO interval:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/isotest.c#n704
In realtime context it seems what isotest.c does (or intends to --- does
it actually skip the first sleep to have extra packets queued in
controller?) probably is equivalent to pushing num extra packets of
silence to the socket when playback starts.
Looking at Core v5.3, Part G, Fig 3.2, it seems to me this increases
latency by the number of extra packets pushed, on top of the transport
latency. If it is so, I think we'd like to use as few as possible or
none.
OTOH, if the statement is that we should write num=latency/interval packets
every num intervals, I don't quite understand why, and it seems also
this should increase latency because we need to gather all that audio
data before we can send it to the socket?
--
Pauli Virtanen
Hi Luiz,
to, 2023-04-13 kello 11:48 -0700, Luiz Augusto von Dentz kirjoitti:
[clip]
> Btw, not sure you if are following the list but I'm working on adding
> support for handling multiple CIS to the same peer:
>
> https://patchwork.kernel.org/project/bluetooth/list/?series=739574
>
> That should allow you to set a different CIS ID if you want to use
> different sockets for input and output.
Great, I saw the patchset but didn't yet have time to take a proper look.
> Id also would like to discuss how we do some test automation using
> pipewire endpoints in the future, we probably want to enable it via
> test-runner but we probably need to enable it loading pipewire daemons
> etc under the vm that test-runner creates, Frederic started adding
> some support but it doesn't look like it loads pipewire so Im not
> really sure how it supposed to be loaded:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n1108
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n277
Yes, there'd be two daemons to start in the VM, pipewire and wireplumber,
and running with default config should then result to the endpoints being
created.
The interaction is probably simplest via the command-line tools.
In principle something more clever and better controlled is possible,
but I'd need to think a bit more what's best here.
Output from `pw-dump -m` can be polled and parsed for determining
when daemons are ready, what bluetooth sinks/devices appeared after
connect, etc. `pw-cat` can be used for playback and recording.
Some tests probably can be written along these lines, but without
trying now I don't know yet how well the above would work.
A separate question is how the virtual BT device that is going to
interact with the endpoints is going to be implemented. Hand-coded
data sent via emulator bthost?
--
Pauli Virtanen
Hi Pauli,
On Thu, Apr 13, 2023 at 2:14 PM Pauli Virtanen <[email protected]> wrote:
>
> Hi Luiz,
>
> to, 2023-04-13 kello 11:48 -0700, Luiz Augusto von Dentz kirjoitti:
> [clip]
> > Btw, not sure you if are following the list but I'm working on adding
> > support for handling multiple CIS to the same peer:
> >
> > https://patchwork.kernel.org/project/bluetooth/list/?series=739574
> >
> > That should allow you to set a different CIS ID if you want to use
> > different sockets for input and output.
>
> Great, I saw the patchset but didn't yet have time to take a proper look.
>
> > Id also would like to discuss how we do some test automation using
> > pipewire endpoints in the future, we probably want to enable it via
> > test-runner but we probably need to enable it loading pipewire daemons
> > etc under the vm that test-runner creates, Frederic started adding
> > some support but it doesn't look like it loads pipewire so Im not
> > really sure how it supposed to be loaded:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n1108
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n277
>
> Yes, there'd be two daemons to start in the VM, pipewire and wireplumber,
> and running with default config should then result to the endpoints being
> created.
>
> The interaction is probably simplest via the command-line tools.
> In principle something more clever and better controlled is possible,
> but I'd need to think a bit more what's best here.
>
> Output from `pw-dump -m` can be polled and parsed for determining
> when daemons are ready, what bluetooth sinks/devices appeared after
> connect, etc. `pw-cat` can be used for playback and recording.
Yep, in terms of tests the ideal solution would be simulate BAP
qualification tests:
https://www.bluetooth.org/docman/handlers/DownloadDoc.ashx?doc_id=524716
Ive started to write them as unit tests:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/unit/test-bap.c
It is somewhat laborious to write all the PDUs manually like that but
if we managed to write all the tests we could perhaps enable it
working with the full stack rather than limiting it bt_bap instance,
but perhaps it is overkill since we could instead automate
qualification tests via auto-pts.
> Some tests probably can be written along these lines, but without
> trying now I don't know yet how well the above would work.
>
> A separate question is how the virtual BT device that is going to
> interact with the endpoints is going to be implemented. Hand-coded
> data sent via emulator bthost?
We can do both a hand-coded tests with bthost or hook a second
instance of btdev to BlueZ so we test BlueZ vs BlueZ, well in theory
we could even bring another stack as well like zephyr into the
picture, anyway what we need to know is how to setup the environment
for pipewire and wireplumber, note that we don't have the so called
user session under test-runner environment, so I wonder if we need
wireplumber?
>
> --
> Pauli Virtanen
>
--
Luiz Augusto von Dentz
pe, 2023-04-14 kello 14:56 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Thu, Apr 13, 2023 at 2:14 PM Pauli Virtanen <[email protected]> wrote:
> >
> > Hi Luiz,
> >
> > to, 2023-04-13 kello 11:48 -0700, Luiz Augusto von Dentz kirjoitti:
> > [clip]
> > > Btw, not sure you if are following the list but I'm working on adding
> > > support for handling multiple CIS to the same peer:
> > >
> > > https://patchwork.kernel.org/project/bluetooth/list/?series=739574
> > >
> > > That should allow you to set a different CIS ID if you want to use
> > > different sockets for input and output.
> >
> > Great, I saw the patchset but didn't yet have time to take a proper look.
> >
> > > Id also would like to discuss how we do some test automation using
> > > pipewire endpoints in the future, we probably want to enable it via
> > > test-runner but we probably need to enable it loading pipewire daemons
> > > etc under the vm that test-runner creates, Frederic started adding
> > > some support but it doesn't look like it loads pipewire so Im not
> > > really sure how it supposed to be loaded:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n1108
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/tools/test-runner.c#n277
> >
> > Yes, there'd be two daemons to start in the VM, pipewire and wireplumber,
> > and running with default config should then result to the endpoints being
> > created.
> >
> > The interaction is probably simplest via the command-line tools.
> > In principle something more clever and better controlled is possible,
> > but I'd need to think a bit more what's best here.
> >
> > Output from `pw-dump -m` can be polled and parsed for determining
> > when daemons are ready, what bluetooth sinks/devices appeared after
> > connect, etc. `pw-cat` can be used for playback and recording.
>
> Yep, in terms of tests the ideal solution would be simulate BAP
> qualification tests:
>
> https://www.bluetooth.org/docman/handlers/DownloadDoc.ashx?doc_id=524716
>
> Ive started to write them as unit tests:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/unit/test-bap.c
>
> It is somewhat laborious to write all the PDUs manually like that but
> if we managed to write all the tests we could perhaps enable it
> working with the full stack rather than limiting it bt_bap instance,
> but perhaps it is overkill since we could instead automate
> qualification tests via auto-pts.
> >
> > Some tests probably can be written along these lines, but without
> > trying now I don't know yet how well the above would work.
> >
> > A separate question is how the virtual BT device that is going to
> > interact with the endpoints is going to be implemented. Hand-coded
> > data sent via emulator bthost?
>
> We can do both a hand-coded tests with bthost or hook a second
> instance of btdev to BlueZ so we test BlueZ vs BlueZ, well in theory
> we could even bring another stack as well like zephyr into the
> picture, anyway what we need to know is how to setup the environment
> for pipewire and wireplumber, note that we don't have the so called
> user session under test-runner environment, so I wonder if we need
> wireplumber?
I sent an example patch to the list that adds an option to launch PW
with tools/test-runner so that you get the endpoints registered. You
have to disable a few things for it to run in plain environment (ALSA
device reservations and flatpak portal want DBus user session, logind
integration wants logind).
The "session manager" is some JACK terminology, not really related to
user sessions, in PW it is what coordinates audio device discovery, and
connects clients to devices, so you have to run one.
--
Pauli Virtanen