Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 16 Mar 2021 15:49:56 -0700
In-Reply-To: <20210316224957.3294962-1-danielwinkler@google.com>
Message-Id: <20210316154901.Bluez.v3.1.I45b896f4512038309cbeab7a01f51e503141edab@changeid>
Mime-Version: 1.0
References: <20210316224957.3294962-1-danielwinkler@google.com>
Subject: [Bluez PATCH v3 1/2] advertising: Create and use scannable adv param flag
From:   Daniel Winkler <danielwinkler@google.com>
To:     linux-bluetooth@vger.kernel.org, luiz.dentz@gmail.com
Cc:     chromeos-bluetooth-upstreaming@chromium.org,
        Daniel Winkler <danielwinkler@google.com>,
        Sonny Sasaka <sonnysasaka@chromium.org>,
        Miao-chen Chou <mcchou@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

In order for the advertising parameters hci request to indicate that an
advertising set uses a scannable PDU, we pass a scannable flag along
with the initial parameters MGMT request. This flag is populated based
on the existence of any scan response data requested by the client.

Without this patch, a broadcast advertisement with a scan response will
either be rejected by the controller, or will ignore the requested scan
response. The patch is tested by performing the above and confirming
that the scan response is retrievable from a peer as expected.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>

---

Changes in v3:
    - Use helpers to determine scannable rather than generating earlier

Changes in v2:
    - Check kernel supports flag before setting it

 lib/mgmt.h        |  1 +
 src/advertising.c | 24 +++++++++++++++++++++++-
 src/shared/ad.c   | 17 +++++++++++++++++
 src/shared/ad.h   |  2 ++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 76a03c9c2..7b1b9ab54 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -507,6 +507,7 @@ struct mgmt_rp_add_advertising {
 #define MGMT_ADV_PARAM_TIMEOUT		(1 << 13)
 #define MGMT_ADV_PARAM_INTERVALS	(1 << 14)
 #define MGMT_ADV_PARAM_TX_POWER		(1 << 15)
+#define MGMT_ADV_PARAM_SCAN_RSP		(1 << 16)
 
 #define MGMT_OP_REMOVE_ADVERTISING	0x003F
 struct mgmt_cp_remove_advertising {
diff --git a/src/advertising.c b/src/advertising.c
index d76e97a74..4ea0e60ee 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -793,6 +793,22 @@ static uint8_t *generate_scan_rsp(struct btd_adv_client *client,
 	return bt_ad_generate(client->scan, len);
 }
 
+static bool adv_client_has_scan_response(struct btd_adv_client *client,
+						uint32_t flags)
+{
+	/* The local name isn't added into the bt_ad structure until
+	 * generate_scan_rsp is called, so we must check these conditions as
+	 * well.
+	 */
+	if (!(flags & MGMT_ADV_FLAG_LOCAL_NAME) &&
+			!client->name &&
+			bt_ad_is_empty(client->scan)) {
+		return false;
+	}
+
+	return true;
+}
+
 static int get_adv_flags(struct btd_adv_client *client)
 {
 	uint32_t flags = 0;
@@ -917,7 +933,13 @@ static int refresh_extended_adv(struct btd_adv_client *client,
 		flags |= MGMT_ADV_PARAM_TX_POWER;
 	}
 
-	cp.flags = htobl(flags);
+	/* Indicate that this instance will be configured as scannable */
+	if (adv_client_has_scan_response(client, flags) &&
+		client->manager->supported_flags & MGMT_ADV_PARAM_SCAN_RSP) {
+		flags |= MGMT_ADV_PARAM_SCAN_RSP;
+	}
+
+	cp.flags = cpu_to_le32(flags);
 
 	mgmt_ret = mgmt_send(client->manager->mgmt, MGMT_OP_ADD_EXT_ADV_PARAMS,
 			client->manager->mgmt_index, sizeof(cp), &cp,
diff --git a/src/shared/ad.c b/src/shared/ad.c
index 23c8c34f4..d40d15331 100644
--- a/src/shared/ad.c
+++ b/src/shared/ad.c
@@ -552,6 +552,23 @@ uint8_t *bt_ad_generate(struct bt_ad *ad, size_t *length)
 	return adv_data;
 }
 
+bool bt_ad_is_empty(struct bt_ad *ad)
+{
+	/* If any of the bt_ad fields are non-empty or don't have the default
+	 * value, then bt_ad_generate will return a non-empty buffer
+	 */
+	if (!ad->name &&
+		ad->appearance == UINT16_MAX &&
+		queue_isempty(ad->service_uuids) &&
+		queue_isempty(ad->manufacturer_data) &&
+		queue_isempty(ad->solicit_uuids) &&
+		queue_isempty(ad->service_data) &&
+		queue_isempty(ad->data)) {
+		return true;
+	}
+	return false;
+}
+
 static bool queue_add_uuid(struct queue *queue, const bt_uuid_t *uuid)
 {
 	bt_uuid_t *new_uuid;
diff --git a/src/shared/ad.h b/src/shared/ad.h
index 13adcb406..84ef9dee9 100644
--- a/src/shared/ad.h
+++ b/src/shared/ad.h
@@ -105,6 +105,8 @@ void bt_ad_unref(struct bt_ad *ad);
 
 uint8_t *bt_ad_generate(struct bt_ad *ad, size_t *length);
 
+bool bt_ad_is_empty(struct bt_ad *ad);
+
 bool bt_ad_add_service_uuid(struct bt_ad *ad, const bt_uuid_t *uuid);
 
 bool bt_ad_remove_service_uuid(struct bt_ad *ad, bt_uuid_t *uuid);
-- 
2.31.0.rc2.261.g7f71774620-goog