All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: Allow scannable adv with extended MGMT APIs
@ 2021-03-03 19:15 Daniel Winkler
  2021-03-03 21:10 ` Marcel Holtmann
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Winkler @ 2021-03-03 19:15 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: chromeos-bluetooth-upstreaming, Daniel Winkler, Alain Michaud,
	Sonny Sasaka, Miao-chen Chou, David S. Miller, Jakub Kicinski,
	Johan Hedberg, Luiz Augusto von Dentz, Marcel Holtmann,
	linux-kernel, netdev

An issue was found, where if a bluetooth client requests a broadcast
advertisement with scan response data, it will not be properly
registered with the controller. This is because at the time that the
hci_cp_le_set_scan_param structure is created, the scan response will
not yet have been received since it comes in a second MGMT call. With
empty scan response, the request defaults to a non-scannable PDU type.
On some controllers, the subsequent scan response request will fail due
to incorrect PDU type, and others will succeed and not use the scan
response.

This fix allows the advertising parameters MGMT call to include a flag
to let the kernel know whether a scan response will be coming, so that
the correct PDU type is used in the first place. A bluetoothd change is
also incoming to take advantage of it.

To test this, I created a broadcast advertisement with scan response
data and registered it on the hatch chromebook. Without this change, the
request fails, and with it will succeed.

Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Signed-off-by: Daniel Winkler <danielwinkler@google.com>
---

 include/net/bluetooth/mgmt.h | 1 +
 net/bluetooth/hci_request.c  | 3 ++-
 net/bluetooth/mgmt.c         | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 839a2028009ea1..a7cffb06956517 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -578,6 +578,7 @@ struct mgmt_rp_add_advertising {
 #define MGMT_ADV_PARAM_TIMEOUT		BIT(13)
 #define MGMT_ADV_PARAM_INTERVALS	BIT(14)
 #define MGMT_ADV_PARAM_TX_POWER		BIT(15)
+#define MGMT_ADV_PARAM_SCAN_RSP		BIT(16)
 
 #define MGMT_ADV_FLAG_SEC_MASK	(MGMT_ADV_FLAG_SEC_1M | MGMT_ADV_FLAG_SEC_2M | \
 				 MGMT_ADV_FLAG_SEC_CODED)
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 75a42178c82d9b..d7ee11ef70d3e1 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -2180,7 +2180,8 @@ int __hci_req_setup_ext_adv_instance(struct hci_request *req, u8 instance)
 			cp.evt_properties = cpu_to_le16(LE_EXT_ADV_CONN_IND);
 		else
 			cp.evt_properties = cpu_to_le16(LE_LEGACY_ADV_IND);
-	} else if (adv_instance_is_scannable(hdev, instance)) {
+	} else if (adv_instance_is_scannable(hdev, instance) ||
+		   (flags & MGMT_ADV_PARAM_SCAN_RSP)) {
 		if (secondary_adv)
 			cp.evt_properties = cpu_to_le16(LE_EXT_ADV_SCAN_IND);
 		else
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 74971b4bd4570d..90334ac4a13589 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7432,6 +7432,7 @@ static u32 get_supported_adv_flags(struct hci_dev *hdev)
 	flags |= MGMT_ADV_PARAM_TIMEOUT;
 	flags |= MGMT_ADV_PARAM_INTERVALS;
 	flags |= MGMT_ADV_PARAM_TX_POWER;
+	flags |= MGMT_ADV_PARAM_SCAN_RSP;
 
 	/* In extended adv TX_POWER returned from Set Adv Param
 	 * will be always valid.
-- 
2.30.1.766.gb4fecdf3b7-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Bluetooth: Allow scannable adv with extended MGMT APIs
  2021-03-03 19:15 [PATCH] Bluetooth: Allow scannable adv with extended MGMT APIs Daniel Winkler
@ 2021-03-03 21:10 ` Marcel Holtmann
  0 siblings, 0 replies; 2+ messages in thread
From: Marcel Holtmann @ 2021-03-03 21:10 UTC (permalink / raw)
  To: Daniel Winkler
  Cc: Bluetooth Kernel Mailing List, CrosBT Upstreaming, Alain Michaud,
	Sonny Sasaka, Miao-chen Chou, David S. Miller, Jakub Kicinski,
	Johan Hedberg, Luiz Augusto von Dentz, LKML, netdev

Hi Daniel,

> An issue was found, where if a bluetooth client requests a broadcast
> advertisement with scan response data, it will not be properly
> registered with the controller. This is because at the time that the
> hci_cp_le_set_scan_param structure is created, the scan response will
> not yet have been received since it comes in a second MGMT call. With
> empty scan response, the request defaults to a non-scannable PDU type.
> On some controllers, the subsequent scan response request will fail due
> to incorrect PDU type, and others will succeed and not use the scan
> response.
> 
> This fix allows the advertising parameters MGMT call to include a flag
> to let the kernel know whether a scan response will be coming, so that
> the correct PDU type is used in the first place. A bluetoothd change is
> also incoming to take advantage of it.
> 
> To test this, I created a broadcast advertisement with scan response
> data and registered it on the hatch chromebook. Without this change, the
> request fails, and with it will succeed.
> 
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Signed-off-by: Daniel Winkler <danielwinkler@google.com>
> ---
> 
> include/net/bluetooth/mgmt.h | 1 +
> net/bluetooth/hci_request.c  | 3 ++-
> net/bluetooth/mgmt.c         | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)

patch has been applied to bluetooth-next tree.

Regards

Marcel


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-03-04  0:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-03 19:15 [PATCH] Bluetooth: Allow scannable adv with extended MGMT APIs Daniel Winkler
2021-03-03 21:10 ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.