diff mbox series

[BlueZ,1/1] client/player: Rework transport select for encrypted streams

Message ID 20241220144458.27739-2-iulia.tanasescu@nxp.com (mailing list archive)
State New
Headers show
Series client/player: Rework transport select for encrypted streams | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/ScanBuild success Scan Build PASS

Commit Message

Iulia Tanasescu Dec. 20, 2024, 2:44 p.m. UTC
This fixes the transport select flow in bluetoothctl: If the user tries
to select multiple encrypted transports, the prompts for the Broadcast
Code overlap, causing the UI to be distorted:

[11-AE-0A-C1-F4-30]# transport.select
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd0
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd1
[] Enter brocast code[value/no]: Successfully linked transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd1
[] Enter brocast code[value/no]: Borne House
(null)Setting broadcast code succeeded
(null)[CHG] Transport /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd0
                     State: broadcasting
(null)Select successful
(null)

This commit updates the transport select command handler to first
link all transports before setting the Broadcast Code only on the
primary link (the Broadcast Code is common for all BISes in the BIG).
After the Broadcast Code is successfully set, each link is selected
one by one. The bluetoothctl log below shows the updated output:

client/bluetoothctl
[bluetooth]# endpoint.register 00001851-0000-1000-8000-00805f9b34fb 0x06
[/local/endpoint/ep0] Auto Accept (yes/no): y
[/local/endpoint/ep0] Max Transports (auto/value): a
[/local/endpoint/ep0] Locations: 1
[/local/endpoint/ep0] Supported Context (value): 1
[bluetooth]# Endpoint /local/endpoint/ep0 registered
[bluetooth]# scan on
[bluetooth]# [NEW] Device 11:16:BD:36:58:3F 11-16-BD-36-58-3F
[11-16-BD-36-58-3F]# [CHG] Device 11:16:BD:36:58:3F Connected: yes
[11-16-BD-36-58-3F]# [NEW] Transport
                     /org/bluez/hci0/dev_11_16_BD_36_58_3F/bis1/fd0
[11-16-BD-36-58-3F]# [NEW] Transport
                     /org/bluez/hci0/dev_11_16_BD_36_58_3F/bis2/fd1
[11-16-BD-36-58-3F]# transport.select
                     /org/bluez/hci0/dev_11_16_BD_36_58_3F/bis1/fd0
                     /org/bluez/hci0/dev_11_16_BD_36_58_3F/bis2/fd1
[11-AE-0A-C1-F4-30]# Successfully linked transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3
[] Enter brocast code[value/no]: Borne House
[11-AE-0A-C1-F4-30]# Setting broadcast code succeeded
[11-AE-0A-C1-F4-30]# [CHG] Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd2
                     State: broadcasting
[11-AE-0A-C1-F4-30]# Select successful
[11-AE-0A-C1-F4-30]# [CHG] Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3
                     State: broadcasting
[11-AE-0A-C1-F4-30]# Select successful
[11-AE-0A-C1-F4-30]# transport.acquire
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd2
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3
auto acquiring...
Transport /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd2 acquiring
auto acquiring...
Transport /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3 acquiring
[11-AE-0A-C1-F4-30]# Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd2
                     acquiring complete
[11-AE-0A-C1-F4-30]# Acquire successful: fd 11 MTU 40:0
[11-AE-0A-C1-F4-30]# [CHG] Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis1/fd2
                     State: active
[11-AE-0A-C1-F4-30]# Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3
                     acquiring complete
[11-AE-0A-C1-F4-30]# Acquire successful: fd 7 MTU 40:0
[11-AE-0A-C1-F4-30]# [CHG] Transport
                     /org/bluez/hci0/dev_11_AE_0A_C1_F4_30/bis2/fd3
                     State: active

The BIG Create Sync command is sent with the correct Broadcast Code,
for the 2 acquired BISes:

< HCI Command: LE Broadcast Isochronous Group Create Sync (0x08|0x006b)
        BIG Handle: 0x00
        BIG Sync Handle: 0x0000
        Encryption: Encrypted (0x01)
        Broadcast Code[16]: 426f726e6520486f7573650000000000
        Maximum Number Subevents: 0x00
        Timeout: 20000 ms (0x07d0)
        Number of BIS: 2
        BIS ID: 0x01
        BIS ID: 0x02
> HCI Event: Command Status (0x0f)
      LE Broadcast Isochronous Group Create Sync (0x08|0x006b) ncmd 1
        Status: Success (0x00)
---
 client/player.c | 159 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 127 insertions(+), 32 deletions(-)

Comments

bluez.test.bot@gmail.com Dec. 20, 2024, 4:14 p.m. UTC | #1
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=919923

---Test result---

Test Summary:
CheckPatch                    PENDING   0.25 seconds
GitLint                       PENDING   0.23 seconds
BuildEll                      PASS      21.54 seconds
BluezMake                     PASS      1690.61 seconds
MakeCheck                     PASS      12.90 seconds
MakeDistcheck                 PASS      170.49 seconds
CheckValgrind                 PASS      225.99 seconds
CheckSmatch                   PASS      291.03 seconds
bluezmakeextell               PASS      103.98 seconds
IncrementalBuild              PENDING   0.27 seconds
ScanBuild                     PASS      910.26 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/client/player.c b/client/player.c
index f93c9d908..464a9cc14 100644
--- a/client/player.c
+++ b/client/player.c
@@ -115,6 +115,8 @@  struct endpoint {
 	uint8_t iso_group;
 	uint8_t iso_stream;
 	struct queue *acquiring;
+	struct queue *links;
+	struct queue *selecting;
 	struct queue *transports;
 	DBusMessage *msg;
 	struct preset *preset;
@@ -148,6 +150,9 @@  struct transport {
 	int num;
 };
 
+static void transport_set_links(struct endpoint *ep, GDBusProxy *proxy);
+static void transport_select(GDBusProxy *proxy);
+
 static void endpoint_unregister(void *data)
 {
 	struct endpoint *ep = data;
@@ -2918,6 +2923,8 @@  static void endpoint_free(void *data)
 		free(ep->preset);
 
 	queue_destroy(ep->acquiring, NULL);
+	queue_destroy(ep->links, NULL);
+	queue_destroy(ep->selecting, NULL);
 	queue_destroy(ep->transports, free);
 
 	g_free(ep->path);
@@ -4887,6 +4894,7 @@  static void acquire_reply(DBusMessage *message, void *user_data)
 static void select_reply(DBusMessage *message, void *user_data)
 {
 	DBusError error;
+	struct endpoint *ep = user_data;
 
 	dbus_error_init(&error);
 
@@ -4898,7 +4906,13 @@  static void select_reply(DBusMessage *message, void *user_data)
 
 	bt_shell_printf("Select successful\n");
 
-	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+	if (queue_isempty(ep->selecting)) {
+		/* All links have been selected */
+		queue_destroy(ep->selecting, NULL);
+		ep->selecting = NULL;
+
+		return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+	}
 }
 
 static void unselect_reply(DBusMessage *message, void *user_data)
@@ -5170,9 +5184,7 @@  static void set_bcode_cb(const DBusError *error, void *user_data)
 
 	bt_shell_printf("Setting broadcast code succeeded\n");
 
-	if (!g_dbus_proxy_method_call(proxy, "Select", NULL,
-				select_reply, proxy, NULL))
-		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	transport_select(proxy);
 }
 
 static void set_bcode(const char *input, void *user_data)
@@ -5197,15 +5209,35 @@  static void set_bcode(const char *input, void *user_data)
 	g_free(bcode);
 }
 
-static void transport_select(void *data, void *user_data)
+static void transport_select(GDBusProxy *proxy)
+{
+	struct endpoint *ep;
+	GDBusProxy *link;
+
+	ep = find_ep_by_transport(g_dbus_proxy_get_path(proxy));
+	if (!ep)
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+
+	if (!g_dbus_proxy_method_call(proxy, "Select", NULL,
+					select_reply, ep, NULL)) {
+		bt_shell_printf("Failed select transport\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	/* Select next link */
+	link = queue_pop_head(ep->selecting);
+	if (link)
+		transport_select(link);
+}
+
+static void transport_set_bcode(GDBusProxy *proxy)
 {
-	GDBusProxy *proxy = data;
 	DBusMessageIter iter, array, entry, value;
 	unsigned char encryption;
 	const char *key;
 
 	if (g_dbus_proxy_get_property(proxy, "QoS", &iter) == FALSE)
-		return;
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
 
 	dbus_message_iter_recurse(&iter, &array);
 
@@ -5229,11 +5261,10 @@  static void transport_select(void *data, void *user_data)
 		dbus_message_iter_next(&array);
 	}
 
-	if (!g_dbus_proxy_method_call(proxy, "Select", NULL,
-					select_reply, proxy, NULL)) {
-		bt_shell_printf("Failed select transport\n");
-		return;
-	}
+	/* Go straight to selecting transport, if Broadcast Code
+	 * is not required.
+	 */
+	transport_select(proxy);
 }
 
 static void transport_unselect(GDBusProxy *proxy, bool prompt)
@@ -5247,7 +5278,23 @@  static void transport_unselect(GDBusProxy *proxy, bool prompt)
 
 static void set_links_cb(const DBusError *error, void *user_data)
 {
-	GDBusProxy *link = user_data;
+	GDBusProxy *proxy = user_data;
+	const char *path = g_dbus_proxy_get_path(proxy);
+	struct endpoint *ep;
+	GDBusProxy *link;
+
+	ep = find_ep_by_transport(path);
+	if (!ep) {
+		bt_shell_printf("Local endpoint not found\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	link = queue_pop_head(ep->links);
+
+	if (queue_isempty(ep->links)) {
+		queue_destroy(ep->links, NULL);
+		ep->links = NULL;
+	}
 
 	if (dbus_error_is_set(error)) {
 		bt_shell_printf("Failed to set link %s: %s\n",
@@ -5258,13 +5305,60 @@  static void set_links_cb(const DBusError *error, void *user_data)
 
 	bt_shell_printf("Successfully linked transport %s\n",
 						g_dbus_proxy_get_path(link));
+
+	if (!ep->selecting)
+		ep->selecting = queue_new();
+
+	/* Enqueue link to mark that it is ready to be selected */
+	queue_push_tail(ep->selecting, link);
+
+	/* Continue setting the remanining links */
+	transport_set_links(ep, proxy);
+}
+
+static void transport_set_links(struct endpoint *ep, GDBusProxy *proxy)
+{
+	GDBusProxy *link;
+	const char *path;
+
+	link = queue_peek_head(ep->links);
+	if (link) {
+		path = g_dbus_proxy_get_path(link);
+
+		if (g_dbus_proxy_set_property_array(proxy, "Links",
+					DBUS_TYPE_OBJECT_PATH,
+					&path, 1, set_links_cb,
+					proxy, NULL) == FALSE) {
+			bt_shell_printf("Linking transport %s failed\n", path);
+			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
+
+		return;
+	}
+
+	/* If all links have been set, check is transport requires the
+	 * user to provide a Broadcast Code.
+	 */
+	transport_set_bcode(proxy);
+}
+
+static void endpoint_set_links(struct endpoint *ep)
+{
+	GDBusProxy *proxy = queue_pop_head(ep->links);
+
+	if (!proxy) {
+		bt_shell_printf("No transport to set links for\n");
+		return bt_shell_noninteractive_quit(EXIT_FAILURE);
+	}
+
+	transport_set_links(ep, proxy);
 }
 
 static void cmd_select_transport(int argc, char *argv[])
 {
-	GDBusProxy *proxy = NULL, *link;
+	GDBusProxy *link = NULL;
 	struct queue *links = queue_new();
-	const char *path;
+	struct endpoint *ep;
 	int i;
 
 	for (i = 1; i < argc; i++) {
@@ -5272,35 +5366,36 @@  static void cmd_select_transport(int argc, char *argv[])
 					BLUEZ_MEDIA_TRANSPORT_INTERFACE);
 		if (!link) {
 			bt_shell_printf("Transport %s not found\n", argv[i]);
-			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+			goto fail;
 		}
 
 		if (find_transport(link)) {
 			bt_shell_printf("Transport %s already acquired\n",
 					argv[i]);
-			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+			goto fail;
 		}
 
+		/* Enqueue all links */
 		queue_push_tail(links, link);
+	}
 
-		if (!proxy) {
-			proxy = link;
-			continue;
-		}
+	/* Get reference to local endpoint */
+	ep = find_ep_by_transport(g_dbus_proxy_get_path(link));
+	if (!ep) {
+		bt_shell_printf("Local endpoint not found\n");
+		goto fail;
+	}
 
-		path = g_dbus_proxy_get_path(link);
+	ep->links = links;
 
-		if (g_dbus_proxy_set_property_array(proxy, "Links",
-					DBUS_TYPE_OBJECT_PATH,
-					&path, 1, set_links_cb,
-					link, NULL) == FALSE) {
-			bt_shell_printf("Linking transport %s failed\n",
-								argv[i]);
-			return bt_shell_noninteractive_quit(EXIT_FAILURE);
-		}
-	}
+	/* Link streams before selecting one by one */
+	endpoint_set_links(ep);
+
+	return;
 
-	queue_foreach(links, transport_select, NULL);
+fail:
+	queue_destroy(links, NULL);
+	return bt_shell_noninteractive_quit(EXIT_FAILURE);
 }
 
 static void cmd_unselect_transport(int argc, char *argv[])