[RFC 1/3] adapter-api: Add Experimental property

[RFC 1/3] adapter-api: Add Experimental property

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds Experimental property which indicates what experimental
features are currently enabled.
---
 doc/adapter-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
index 464434a81..13e904425 100644
--- a/doc/adapter-api.txt
+++ b/doc/adapter-api.txt
@@ -335,3 +335,8 @@ Properties	string Address [readonly]
 				"peripheral": Supports the peripheral role.
 				"central-peripheral": Supports both roles
 						      concurrently.
+
+		array{string} Experimental [readonly, optional]
+
+			List of 128-bit UUIDs that represents the experimental
+			features currently enabled.
-- 
2.31.1

[RFC 2/3] adapter: Implement Experimental property

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This implements Experimental property which indicates the list of UUIDs
that represents the experimental features currently enabled.
---
 src/adapter.c | 149 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 109 insertions(+), 40 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index ddd896751..076600645 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -102,6 +102,30 @@ static const struct mgmt_blocked_key_info blocked_keys[] = {
 		 0x22, 0x8e, 0x07, 0x56, 0xb4, 0xe8, 0x5f, 0x01}},
 };
 
+/* d4992530-b9ec-469f-ab01-6c481c47da1c */
+static const uint8_t debug_uuid[16] = {
+	0x1c, 0xda, 0x47, 0x1c, 0x48, 0x6c, 0x01, 0xab,
+	0x9f, 0x46, 0xec, 0xb9, 0x30, 0x25, 0x99, 0xd4,
+};
+
+/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
+static const uint8_t le_simult_central_peripheral_uuid[16] = {
+	0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
+	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
+};
+
+/* 330859bc-7506-492d-9370-9a6f0614037f */
+static const uint8_t quality_report_uuid[16] = {
+	0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
+	0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33,
+};
+
+/* 15c0a148-c273-11ea-b3de-0242ac130004 */
+static const uint8_t rpa_resolution_uuid[16] = {
+	0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
+	0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15,
+};
+
 static DBusConnection *dbus_conn = NULL;
 
 static uint32_t kernel_features = 0;
@@ -285,8 +309,7 @@ struct btd_adapter {
 
 	bool is_default;		/* true if adapter is default one */
 
-	bool le_simult_roles_supported;
-	bool quality_report_supported;
+	struct queue *exps;
 };
 
 typedef enum {
@@ -3250,7 +3273,8 @@ static gboolean property_get_roles(const GDBusPropertyTable *property,
 		dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &str);
 	}
 
-	if (adapter->le_simult_roles_supported) {
+	if (queue_find(adapter->exps, NULL,
+				le_simult_central_peripheral_uuid)) {
 		const char *str = "central-peripheral";
 		dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, &str);
 	}
@@ -3260,6 +3284,48 @@ static gboolean property_get_roles(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static void property_append_experimental(void *data, void *user_data)
+{
+	uint8_t *feature = data;
+	DBusMessageIter *iter = user_data;
+	uint128_t value;
+	bt_uuid_t uuid;
+	char str[MAX_LEN_UUID_STR + 1];
+	char *ptr;
+
+	bswap_128(feature, &value);
+	bt_uuid128_create(&uuid, value);
+	bt_uuid_to_string(&uuid, str, sizeof(str));
+
+	ptr = str;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
+}
+
+static gboolean property_get_experimental(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *user_data)
+{
+	struct btd_adapter *adapter = user_data;
+	DBusMessageIter entry;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_STRING_AS_STRING, &entry);
+
+	queue_foreach(adapter->exps, property_append_experimental, &entry);
+
+	dbus_message_iter_close_container(iter, &entry);
+
+	return TRUE;
+}
+
+static gboolean property_experimental_exits(const GDBusPropertyTable *property,
+								void *data)
+{
+	struct btd_adapter *adapter = data;
+
+	return !queue_isempty(adapter->exps);
+}
+
 static DBusMessage *remove_device(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -3619,6 +3685,8 @@ static const GDBusPropertyTable adapter_properties[] = {
 	{ "Modalias", "s", property_get_modalias, NULL,
 					property_exists_modalias },
 	{ "Roles", "as", property_get_roles },
+	{ "Experimental", "as", property_get_experimental, NULL,
+					property_experimental_exits },
 	{ }
 };
 
@@ -5526,6 +5594,7 @@ static void adapter_free(gpointer user_data)
 
 	g_queue_foreach(adapter->auths, free_service_auth, NULL);
 	g_queue_free(adapter->auths);
+	queue_destroy(adapter->exps, NULL);
 
 	/*
 	 * Unregister all handlers for this specific index since
@@ -6496,6 +6565,7 @@ static struct btd_adapter *btd_adapter_new(uint16_t index)
 	DBG("Pairable timeout: %u seconds", adapter->pairable_timeout);
 
 	adapter->auths = g_queue_new();
+	adapter->exps = queue_new();
 
 	return btd_adapter_ref(adapter);
 }
@@ -9394,38 +9464,22 @@ static bool set_blocked_keys(struct btd_adapter *adapter)
 	.func = _func, \
 }
 
-/* d4992530-b9ec-469f-ab01-6c481c47da1c */
-static const uint8_t debug_uuid[16] = {
-	0x1c, 0xda, 0x47, 0x1c, 0x48, 0x6c, 0x01, 0xab,
-	0x9f, 0x46, 0xec, 0xb9, 0x30, 0x25, 0x99, 0xd4,
-};
-
-/* 671b10b5-42c0-4696-9227-eb28d1b049d6 */
-static const uint8_t le_simult_central_peripheral_uuid[16] = {
-	0xd6, 0x49, 0xb0, 0xd1, 0x28, 0xeb, 0x27, 0x92,
-	0x96, 0x46, 0xc0, 0x42, 0xb5, 0x10, 0x1b, 0x67,
-};
-
-/* 330859bc-7506-492d-9370-9a6f0614037f */
-static const uint8_t quality_report_uuid[16] = {
-	0x7f, 0x03, 0x14, 0x06, 0x6f, 0x9a, 0x70, 0x93,
-	0x2d, 0x49, 0x06, 0x75, 0xbc, 0x59, 0x08, 0x33,
-};
-
-/* 15c0a148-c273-11ea-b3de-0242ac130004 */
-static const uint8_t rpa_resolution_uuid[16] = {
-	0x04, 0x00, 0x13, 0xac, 0x42, 0x02, 0xde, 0xb3,
-	0xea, 0x11, 0x73, 0xc2, 0x48, 0xa1, 0xc0, 0x15,
-};
-
 static void set_exp_debug_complete(uint8_t status, uint16_t len,
 					const void *param, void *user_data)
 {
-	if (status != 0)
+	struct btd_adapter *adapter = user_data;
+	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
+
+	if (status != 0) {
 		error("Set Experimental Debug failed with status 0x%02x (%s)",
 						status, mgmt_errstr(status));
-	else
-		DBG("Experimental Debug successfully set");
+		return;
+	}
+
+	DBG("Experimental Debug successfully set");
+
+	if (action)
+		queue_push_tail(adapter->exps, (void *)debug_uuid);
 }
 
 static void exp_debug_func(struct btd_adapter *adapter, uint32_t flags)
@@ -9434,8 +9488,11 @@ static void exp_debug_func(struct btd_adapter *adapter, uint32_t flags)
 	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
 
 	/* If already set don't attempt to set it again */
-	if (action == (flags & BIT(0)))
+	if (action == (flags & BIT(0))) {
+		if (action)
+			queue_push_tail(adapter->exps, (void *)debug_uuid);
 		return;
+	}
 
 	memset(&cp, 0, sizeof(cp));
 	memcpy(cp.uuid, debug_uuid, 16);
@@ -9452,25 +9509,33 @@ static void exp_debug_func(struct btd_adapter *adapter, uint32_t flags)
 static void le_simult_central_peripheral_func(struct btd_adapter *adapter,
 							uint32_t flags)
 {
-	adapter->le_simult_roles_supported = flags & 0x01;
+	if (flags & 0x01)
+		queue_push_tail(adapter->exps,
+				(void *)le_simult_central_peripheral_uuid);
 }
 
 static void quality_report_func(struct btd_adapter *adapter, uint32_t flags)
 {
-	adapter->quality_report_supported = le32_to_cpu(flags) & 0x01;
-
-	btd_info(adapter->dev_id, "quality_report_supported %d",
-			adapter->quality_report_supported);
+	if (flags & 0x01)
+		queue_push_tail(adapter->exps, (void *)quality_report_uuid);
 }
 
 static void set_rpa_resolution_complete(uint8_t status, uint16_t len,
 					const void *param, void *user_data)
 {
-	if (status != 0)
+	struct btd_adapter *adapter = user_data;
+	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
+
+	if (status != 0) {
 		error("Set RPA Resolution failed with status 0x%02x (%s)",
 						status, mgmt_errstr(status));
-	else
-		DBG("RPA Resolution successfully set");
+		return;
+	}
+
+	DBG("RPA Resolution successfully set");
+
+	if (action)
+		queue_push_tail(adapter->exps, (void *)rpa_resolution_uuid);
 }
 
 static void rpa_resolution_func(struct btd_adapter *adapter, uint32_t flags)
@@ -9479,8 +9544,12 @@ static void rpa_resolution_func(struct btd_adapter *adapter, uint32_t flags)
 	uint8_t action = btd_opts.experimental ? 0x01 : 0x00;
 
 	/* If already set don't attempt to set it again */
-	if (action == (flags & BIT(0)))
+	if (action == (flags & BIT(0))) {
+		if (action)
+			queue_push_tail(adapter->exps,
+						(void *)rpa_resolution_uuid);
 		return;
+	}
 
 	memset(&cp, 0, sizeof(cp));
 	memcpy(cp.uuid, rpa_resolution_uuid, 16);
-- 
2.31.1

[RFC 3/3] client: Add support for printing Experimental property

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Thid adds support to show command to print Experimental property:

[bluetooth]# show
Controller ...
	Experimental: BlueZ Experimental LL p.. (15c0a148-c273-11ea-b3de-0242ac130004)
---
 client/main.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/client/main.c b/client/main.c
index 506602bbd..80f40cf57 100644
--- a/client/main.c
+++ b/client/main.c
@@ -319,7 +319,7 @@ static void print_property(GDBusProxy *proxy, const char *name)
 	print_property_with_label(proxy, name, NULL);
 }
 
-static void print_uuid(const char *uuid)
+static void print_uuid(const char *label, const char *uuid)
 {
 	const char *text;
 
@@ -340,9 +340,10 @@ static void print_uuid(const char *uuid)
 			n = sizeof(str) - 1;
 		}
 
-		bt_shell_printf("\tUUID: %s%*c(%s)\n", str, 26 - n, ' ', uuid);
+		bt_shell_printf("\t%s: %s%*c(%s)\n", label, str, 26 - n, ' ',
+									uuid);
 	} else
-		bt_shell_printf("\tUUID: %*c(%s)\n", 26, ' ', uuid);
+		bt_shell_printf("\t%s: %*c(%s)\n", label, 26, ' ', uuid);
 }
 
 static void print_uuids(GDBusProxy *proxy)
@@ -359,7 +360,27 @@ static void print_uuids(GDBusProxy *proxy)
 
 		dbus_message_iter_get_basic(&value, &uuid);
 
-		print_uuid(uuid);
+		print_uuid("UUID", uuid);
+
+		dbus_message_iter_next(&value);
+	}
+}
+
+static void print_experimental(GDBusProxy *proxy)
+{
+	DBusMessageIter iter, value;
+
+	if (g_dbus_proxy_get_property(proxy, "Experimental", &iter) == FALSE)
+		return;
+
+	dbus_message_iter_recurse(&iter, &value);
+
+	while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
+		const char *uuid;
+
+		dbus_message_iter_get_basic(&value, &uuid);
+
+		print_uuid("Experimental", uuid);
 
 		dbus_message_iter_next(&value);
 	}
@@ -984,6 +1005,7 @@ static void cmd_show(int argc, char *argv[])
 	print_property(adapter->proxy, "Modalias");
 	print_property(adapter->proxy, "Discovering");
 	print_property(adapter->proxy, "Roles");
+	print_experimental(adapter->proxy);
 
 	if (adapter->ad_proxy) {
 		bt_shell_printf("Advertising Features:\n");
@@ -1424,7 +1446,7 @@ static void cmd_scan_filter_uuids(int argc, char *argv[])
 		char **uuid;
 
 		for (uuid = filter.uuids; uuid && *uuid; uuid++)
-			print_uuid(*uuid);
+			print_uuid("UUID", *uuid);
 
 		return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 	}
-- 
2.31.1

RE: [RFC,1/3] adapter-api: Add Experimental property

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]

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=532387

---Test result---

Test Summary:
CheckPatch                    FAIL      0.94 seconds
GitLint                       FAIL      0.37 seconds
Prep - Setup ELL              PASS      47.58 seconds
Build - Prep                  PASS      0.11 seconds
Build - Configure             PASS      8.30 seconds
Build - Make                  PASS      206.51 seconds
Make Check                    PASS      9.41 seconds
Make Distcheck                PASS      243.43 seconds
Build w/ext ELL - Configure   PASS      8.36 seconds
Build w/ext ELL - Make        PASS      194.51 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
client: Add support for printing Experimental property
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10: 
	Experimental: BlueZ Experimental LL p.. (15c0a148-c273-11ea-b3de-0242ac130004)

- total: 0 errors, 1 warnings, 63 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.

"[PATCH] client: Add support for printing Experimental property" 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: GitLint - FAIL
Desc: Run gitlint with rule in .gitlint
Output:
client: Add support for printing Experimental property
7: B3 Line contains hard tab characters (\t): "	Experimental: BlueZ Experimental LL p.. (15c0a148-c273-11ea-b3de-0242ac130004)"


##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL

##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build

##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree

##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree

##############################
Test: Make Check - PASS
Desc: Run 'make check'

##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution

##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration

##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration



---
Regards,
Linux Bluetooth

Re: [RFC 1/3] adapter-api: Add Experimental property

[Luiz Augusto von Dentz]

Hi Marcel,

On Mon, Aug 16, 2021 at 6:02 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This adds Experimental property which indicates what experimental
> features are currently enabled.
> ---
>  doc/adapter-api.txt | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 464434a81..13e904425 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -335,3 +335,8 @@ Properties  string Address [readonly]
>                                 "peripheral": Supports the peripheral role.
>                                 "central-peripheral": Supports both roles
>                                                       concurrently.
> +
> +               array{string} Experimental [readonly, optional]
> +
> +                       List of 128-bit UUIDs that represents the experimental
> +                       features currently enabled.
> --
> 2.31.1
>

Any feedback on this, by design this would only appear when
experimental is enabled (either via main.conf or using -E), the
intention is to have a common way to expose to application a certain
experimental feature is enabled.

-- 
Luiz Augusto von Dentz

Re: [RFC 1/3] adapter-api: Add Experimental property

[Marcel Holtmann]

Hi Luiz,

> This adds Experimental property which indicates what experimental
> features are currently enabled.
> ---
> doc/adapter-api.txt | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> index 464434a81..13e904425 100644
> --- a/doc/adapter-api.txt
> +++ b/doc/adapter-api.txt
> @@ -335,3 +335,8 @@ Properties	string Address [readonly]
> 				"peripheral": Supports the peripheral role.
> 				"central-peripheral": Supports both roles
> 						      concurrently.
> +
> +		array{string} Experimental [readonly, optional]
> +
> +			List of 128-bit UUIDs that represents the experimental
> +			features currently enabled.

I wonder if this is the best name.

Do we care about just the enabled experimental features or the overall supported experimental features as well. And please keep in mind that we also have per-adapter vs global experimental features. So we should distinguish here as well.

We also need to document that this property is only available if bluetoothd is started with -E and otherwise this property is omitted.

My proposal would be to at least name it ExperimentalSupport or ExperimentalFeatures to give us a path to nicely extend it if needed.

Regards

Marcel

Re: [RFC 1/3] adapter-api: Add Experimental property

[Luiz Augusto von Dentz]

Hi Marcel,

On Thu, Aug 19, 2021 at 7:57 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > This adds Experimental property which indicates what experimental
> > features are currently enabled.
> > ---
> > doc/adapter-api.txt | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
> > index 464434a81..13e904425 100644
> > --- a/doc/adapter-api.txt
> > +++ b/doc/adapter-api.txt
> > @@ -335,3 +335,8 @@ Properties        string Address [readonly]
> >                               "peripheral": Supports the peripheral role.
> >                               "central-peripheral": Supports both roles
> >                                                     concurrently.
> > +
> > +             array{string} Experimental [readonly, optional]
> > +
> > +                     List of 128-bit UUIDs that represents the experimental
> > +                     features currently enabled.
>
> I wonder if this is the best name.
>
> Do we care about just the enabled experimental features or the overall supported experimental features as well. And please keep in mind that we also have per-adapter vs global experimental features. So we should distinguish here as well.

This is per-adapter and I guess the global one would could exposed on
all adapters since we don't have a global object, or perhaps you are
suggesting to use / for that?

> We also need to document that this property is only available if bluetoothd is started with -E and otherwise this property is omitted.

Will add it.

> My proposal would be to at least name it ExperimentalSupport or ExperimentalFeatures to give us a path to nicely extend it if needed.

Sure, I do wonder if we should make it writable as well, so
applications can enable/disable experimental features themselves? Or
perhaps that is going too far as to enable unstable code by
application, it would only work when -E is given thought which would
enable everything anyway but I was thinking on having -E optionally
take a list of UUIDs so one could enable just certain features
instead.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz

Re: [RFC 1/3] adapter-api: Add Experimental property

[Marcel Holtmann]

Hi Luiz,

>>> This adds Experimental property which indicates what experimental
>>> features are currently enabled.
>>> ---
>>> doc/adapter-api.txt | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt
>>> index 464434a81..13e904425 100644
>>> --- a/doc/adapter-api.txt
>>> +++ b/doc/adapter-api.txt
>>> @@ -335,3 +335,8 @@ Properties        string Address [readonly]
>>>                              "peripheral": Supports the peripheral role.
>>>                              "central-peripheral": Supports both roles
>>>                                                    concurrently.
>>> +
>>> +             array{string} Experimental [readonly, optional]
>>> +
>>> +                     List of 128-bit UUIDs that represents the experimental
>>> +                     features currently enabled.
>> 
>> I wonder if this is the best name.
>> 
>> Do we care about just the enabled experimental features or the overall supported experimental features as well. And please keep in mind that we also have per-adapter vs global experimental features. So we should distinguish here as well.
> 
> This is per-adapter and I guess the global one would could exposed on
> all adapters since we don't have a global object, or perhaps you are
> suggesting to use / for that?

if it is read-only, then yes, the global one could be exposed on all adapters.

>> We also need to document that this property is only available if bluetoothd is started with -E and otherwise this property is omitted.
> 
> Will add it.
> 
>> My proposal would be to at least name it ExperimentalSupport or ExperimentalFeatures to give us a path to nicely extend it if needed.
> 
> Sure, I do wonder if we should make it writable as well, so
> applications can enable/disable experimental features themselves? Or
> perhaps that is going too far as to enable unstable code by
> application, it would only work when -E is given thought which would
> enable everything anyway but I was thinking on having -E optionally
> take a list of UUIDs so one could enable just certain features
> instead.

Lets not make this writeable from applications. Some of them require power off etc. So that is going to far.

Regards

Marcel