public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously
@ 2025-06-27  7:05 Christian Eggers
  2025-06-27  7:35 ` [v3] " bluez.test.bot
  2025-06-27 10:09 ` [PATCH v3] " Christian Eggers
  0 siblings, 2 replies; 4+ messages in thread
From: Christian Eggers @ 2025-06-27  7:05 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	Jaganath Kanakkassery
  Cc: linux-bluetooth, netdev, linux-kernel, Christian Eggers, stable

Currently, for controllers with extended advertising, the advertising
data is set in the asynchronous response handler for extended
adverstising params. As most advertising settings are performed in a
synchronous context, the (asynchronous) setting of the advertising data
is done too late (after enabling the advertising).

Move setting of adverstising data from asynchronous response handler
into synchronous context to fix ordering of HCI commands.

Signed-off-by: Christian Eggers <ceggers@arri.de>
Fixes: a0fb3726ba55 ("Bluetooth: Use Set ext adv/scan rsp data if controller supports")
Cc: stable@vger.kernel.org
v2: https://lore.kernel.org/linux-bluetooth/20250626115209.17839-1-ceggers@arri.de/
---
v3: refactor: store adv_addr_type/tx_power within hci_set_ext_adv_params_sync()

v2: convert setting of adv data into synchronous context (rather than moving
more methods into asynchronous response handlers).
- hci_set_ext_adv_params_sync: new method
- hci_set_ext_adv_data_sync: move within source file (no changes)
- hci_set_adv_data_sync: dito
- hci_update_adv_data_sync: dito
- hci_cc_set_ext_adv_param: remove (performed synchronously now)

 net/bluetooth/hci_event.c |  36 -------
 net/bluetooth/hci_sync.c  | 207 ++++++++++++++++++++++++--------------
 2 files changed, 130 insertions(+), 113 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 66052d6aaa1d..4d5ace9d245d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2150,40 +2150,6 @@ static u8 hci_cc_set_adv_param(struct hci_dev *hdev, void *data,
 	return rp->status;
 }
 
-static u8 hci_cc_set_ext_adv_param(struct hci_dev *hdev, void *data,
-				   struct sk_buff *skb)
-{
-	struct hci_rp_le_set_ext_adv_params *rp = data;
-	struct hci_cp_le_set_ext_adv_params *cp;
-	struct adv_info *adv_instance;
-
-	bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
-
-	if (rp->status)
-		return rp->status;
-
-	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS);
-	if (!cp)
-		return rp->status;
-
-	hci_dev_lock(hdev);
-	hdev->adv_addr_type = cp->own_addr_type;
-	if (!cp->handle) {
-		/* Store in hdev for instance 0 */
-		hdev->adv_tx_power = rp->tx_power;
-	} else {
-		adv_instance = hci_find_adv_instance(hdev, cp->handle);
-		if (adv_instance)
-			adv_instance->tx_power = rp->tx_power;
-	}
-	/* Update adv data as tx power is known now */
-	hci_update_adv_data(hdev, cp->handle);
-
-	hci_dev_unlock(hdev);
-
-	return rp->status;
-}
-
 static u8 hci_cc_read_rssi(struct hci_dev *hdev, void *data,
 			   struct sk_buff *skb)
 {
@@ -4164,8 +4130,6 @@ static const struct hci_cc {
 	HCI_CC(HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
 	       hci_cc_le_read_num_adv_sets,
 	       sizeof(struct hci_rp_le_read_num_supported_adv_sets)),
-	HCI_CC(HCI_OP_LE_SET_EXT_ADV_PARAMS, hci_cc_set_ext_adv_param,
-	       sizeof(struct hci_rp_le_set_ext_adv_params)),
 	HCI_CC_STATUS(HCI_OP_LE_SET_EXT_ADV_ENABLE,
 		      hci_cc_le_set_ext_adv_enable),
 	HCI_CC_STATUS(HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 1f8806dfa556..563614b53485 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1205,9 +1205,126 @@ static int hci_set_adv_set_random_addr_sync(struct hci_dev *hdev, u8 instance,
 				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
 }
 
+static int
+hci_set_ext_adv_params_sync(struct hci_dev *hdev, struct adv_info *adv,
+			    const struct hci_cp_le_set_ext_adv_params *cp,
+			    struct hci_rp_le_set_ext_adv_params *rp)
+{
+	struct sk_buff *skb;
+
+	skb = __hci_cmd_sync(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(*cp),
+			     cp, HCI_CMD_TIMEOUT);
+
+	/* If command return a status event, skb will be set to -ENODATA */
+	if (skb == ERR_PTR(-ENODATA))
+		return 0;
+
+	if (IS_ERR(skb)) {
+		bt_dev_err(hdev, "Opcode 0x%4.4x failed: %ld",
+			   HCI_OP_LE_SET_EXT_ADV_PARAMS, PTR_ERR(skb));
+		return PTR_ERR(skb);
+	}
+
+	if (skb->len != sizeof(*rp)) {
+		bt_dev_err(hdev, "Invalid response length for "
+			   "HCI_OP_LE_SET_EXT_ADV_PARAMS: %u", skb->len);
+		kfree_skb(skb);
+		return -EIO;
+	}
+
+	memcpy(rp, skb->data, sizeof(*rp));
+	kfree_skb(skb);
+
+	if (!rp->status) {
+		hdev->adv_addr_type = cp->own_addr_type;
+		if (!cp->handle) {
+			/* Store in hdev for instance 0 */
+			hdev->adv_tx_power = rp->tx_power;
+		} else if (adv) {
+			adv->tx_power = rp->tx_power;
+		}
+	}
+
+	return rp->status;
+}
+
+static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
+{
+	DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
+		    HCI_MAX_EXT_AD_LENGTH);
+	u8 len;
+	struct adv_info *adv = NULL;
+	int err;
+
+	if (instance) {
+		adv = hci_find_adv_instance(hdev, instance);
+		if (!adv || !adv->adv_data_changed)
+			return 0;
+	}
+
+	len = eir_create_adv_data(hdev, instance, pdu->data,
+				  HCI_MAX_EXT_AD_LENGTH);
+
+	pdu->length = len;
+	pdu->handle = adv ? adv->handle : instance;
+	pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
+	pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
+
+	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
+				    struct_size(pdu, data, len), pdu,
+				    HCI_CMD_TIMEOUT);
+	if (err)
+		return err;
+
+	/* Update data if the command succeed */
+	if (adv) {
+		adv->adv_data_changed = false;
+	} else {
+		memcpy(hdev->adv_data, pdu->data, len);
+		hdev->adv_data_len = len;
+	}
+
+	return 0;
+}
+
+static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
+{
+	struct hci_cp_le_set_adv_data cp;
+	u8 len;
+
+	memset(&cp, 0, sizeof(cp));
+
+	len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
+
+	/* There's nothing to do if the data hasn't changed */
+	if (hdev->adv_data_len == len &&
+	    memcmp(cp.data, hdev->adv_data, len) == 0)
+		return 0;
+
+	memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
+	hdev->adv_data_len = len;
+
+	cp.length = len;
+
+	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
+				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+}
+
+int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
+{
+	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
+		return 0;
+
+	if (ext_adv_capable(hdev))
+		return hci_set_ext_adv_data_sync(hdev, instance);
+
+	return hci_set_adv_data_sync(hdev, instance);
+}
+
 int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
 {
 	struct hci_cp_le_set_ext_adv_params cp;
+	struct hci_rp_le_set_ext_adv_params rp;
 	bool connectable;
 	u32 flags;
 	bdaddr_t random_addr;
@@ -1316,8 +1433,12 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
 		cp.secondary_phy = HCI_ADV_PHY_1M;
 	}
 
-	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
-				    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+	err = hci_set_ext_adv_params_sync(hdev, adv, &cp, &rp);
+	if (err)
+		return err;
+
+	/* Update adv data as tx power is known now */
+	err = hci_set_ext_adv_data_sync(hdev, cp.handle);
 	if (err)
 		return err;
 
@@ -1822,79 +1943,6 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason)
 				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
 }
 
-static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
-{
-	DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
-		    HCI_MAX_EXT_AD_LENGTH);
-	u8 len;
-	struct adv_info *adv = NULL;
-	int err;
-
-	if (instance) {
-		adv = hci_find_adv_instance(hdev, instance);
-		if (!adv || !adv->adv_data_changed)
-			return 0;
-	}
-
-	len = eir_create_adv_data(hdev, instance, pdu->data,
-				  HCI_MAX_EXT_AD_LENGTH);
-
-	pdu->length = len;
-	pdu->handle = adv ? adv->handle : instance;
-	pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
-	pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
-
-	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
-				    struct_size(pdu, data, len), pdu,
-				    HCI_CMD_TIMEOUT);
-	if (err)
-		return err;
-
-	/* Update data if the command succeed */
-	if (adv) {
-		adv->adv_data_changed = false;
-	} else {
-		memcpy(hdev->adv_data, pdu->data, len);
-		hdev->adv_data_len = len;
-	}
-
-	return 0;
-}
-
-static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
-{
-	struct hci_cp_le_set_adv_data cp;
-	u8 len;
-
-	memset(&cp, 0, sizeof(cp));
-
-	len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
-
-	/* There's nothing to do if the data hasn't changed */
-	if (hdev->adv_data_len == len &&
-	    memcmp(cp.data, hdev->adv_data, len) == 0)
-		return 0;
-
-	memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
-	hdev->adv_data_len = len;
-
-	cp.length = len;
-
-	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
-				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
-}
-
-int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
-{
-	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
-		return 0;
-
-	if (ext_adv_capable(hdev))
-		return hci_set_ext_adv_data_sync(hdev, instance);
-
-	return hci_set_adv_data_sync(hdev, instance);
-}
-
 int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance,
 				   bool force)
 {
@@ -6269,6 +6317,7 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
 						struct hci_conn *conn)
 {
 	struct hci_cp_le_set_ext_adv_params cp;
+	struct hci_rp_le_set_ext_adv_params rp;
 	int err;
 	bdaddr_t random_addr;
 	u8 own_addr_type;
@@ -6310,8 +6359,12 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
 	if (err)
 		return err;
 
-	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
-				    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
+	err = hci_set_ext_adv_params_sync(hdev, NULL, &cp, &rp);
+	if (err)
+		return err;
+
+	/* Update adv data as tx power is known now */
+	err = hci_set_ext_adv_data_sync(hdev, cp.handle);
 	if (err)
 		return err;
 
-- 
2.44.1


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

* RE: [v3] Bluetooth: HCI: Set extended advertising data synchronously
  2025-06-27  7:05 [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously Christian Eggers
@ 2025-06-27  7:35 ` bluez.test.bot
  2025-06-27 10:09 ` [PATCH v3] " Christian Eggers
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2025-06-27  7:35 UTC (permalink / raw)
  To: linux-bluetooth, ceggers

[-- Attachment #1: Type: text/plain, Size: 2497 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=976499

---Test result---

Test Summary:
CheckPatch                    PENDING   0.54 seconds
GitLint                       PENDING   0.45 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      24.65 seconds
CheckAllWarning               PASS      27.06 seconds
CheckSparse                   WARNING   30.44 seconds
BuildKernel32                 PASS      24.15 seconds
TestRunnerSetup               PASS      478.45 seconds
TestRunner_l2cap-tester       PASS      25.31 seconds
TestRunner_iso-tester         PASS      38.53 seconds
TestRunner_bnep-tester        PASS      5.98 seconds
TestRunner_mgmt-tester        FAIL      136.89 seconds
TestRunner_rfcomm-tester      PASS      9.25 seconds
TestRunner_sco-tester         PASS      14.74 seconds
TestRunner_ioctl-tester       PASS      10.10 seconds
TestRunner_mesh-tester        FAIL      11.76 seconds
TestRunner_smp-tester         PASS      8.57 seconds
TestRunner_userchan-tester    PASS      6.21 seconds
IncrementalBuild              PENDING   0.94 seconds

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

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

##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 485 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
LL Privacy - Set Flags 4 (RL is full)                Failed       0.276 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.025 seconds
Mesh - Send cancel - 2                               Timed out    1.999 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


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

* Re: [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously
  2025-06-27  7:05 [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously Christian Eggers
  2025-06-27  7:35 ` [v3] " bluez.test.bot
@ 2025-06-27 10:09 ` Christian Eggers
  2025-06-27 15:25   ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 4+ messages in thread
From: Christian Eggers @ 2025-06-27 10:09 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	Jaganath Kanakkassery
  Cc: linux-bluetooth, netdev, linux-kernel, stable

Hi Luiz,

after changing my test setup (I now only use Mesh, no "normal" advertising
anymore), I get many of these errors:

bluetooth-meshd[276]: @ MGMT Command: Mesh Send (0x0059) plen 40                                                                   {0x0001} [hci0] 43.846388
        Address: 00:00:00:00:00:00 (OUI 00-00-00)
        Addr Type: 2
        Instant: 0x0000000000000000
        Delay: 0
        Count: 1
        Data Length: 21
        Data[21]: 142b003a8b6fe779bd4385a94fed0a9cf611880000
< HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25                                                            #479 [hci0] 43.846505
        Handle: 0x05
        Properties: 0x0010
          Use legacy advertising PDUs: ADV_NONCONN_IND
        Min advertising interval: 1280.000 msec (0x0800)
        Max advertising interval: 1280.000 msec (0x0800)
        Channel map: 37, 38, 39 (0x07)
        Own address type: Random (0x01)
        Peer address type: Public (0x00)
        Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
        Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
        TX power: Host has no preference (0x7f)
        Primary PHY: LE 1M (0x01)
        Secondary max skip: 0x00
        Secondary PHY: LE 1M (0x01)
        SID: 0x00
        Scan request notifications: Disabled (0x00)
> HCI Event: Command Complete (0x0e) plen 5                                                                                            #480 [hci0] 43.847480
      LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 2
--->    Status: Command Disallowed (0x0c)
        TX power (selected): 0 dbm (0x00)


From the btmon output it is obvious that advertising is not disabled before updating the parameters.

I added the following debug line in hci_setup_ext_adv_instance_sync():

	printk(KERN_ERR "instance = %u, adv = %p, adv->pending = %d, adv->enabled = %d\n",
	       instance, adv, adv ? adv->pending : -1, adv ? adv->enabled : -1);

From the debug output I see that adv->pending is still true (so advertising is not disabled
before setting the advertising params). After changing the check from 

	if (adv && !adv->pending) {

to

	if (adv && adv->enabled) {

it seems to do the job correctly. What do you think?


regards,
Christian


On Friday, 27 June 2025, 09:05:08 CEST, Christian Eggers wrote:
> Currently, for controllers with extended advertising, the advertising
> data is set in the asynchronous response handler for extended
> adverstising params. As most advertising settings are performed in a
> synchronous context, the (asynchronous) setting of the advertising data
> is done too late (after enabling the advertising).
> 
> Move setting of adverstising data from asynchronous response handler
> into synchronous context to fix ordering of HCI commands.
> 
> Signed-off-by: Christian Eggers <ceggers@arri.de>
> Fixes: a0fb3726ba55 ("Bluetooth: Use Set ext adv/scan rsp data if controller supports")
> Cc: stable@vger.kernel.org
> v2: https://lore.kernel.org/linux-bluetooth/20250626115209.17839-1-ceggers@arri.de/
> ---
> v3: refactor: store adv_addr_type/tx_power within hci_set_ext_adv_params_sync()
> 
> v2: convert setting of adv data into synchronous context (rather than moving
> more methods into asynchronous response handlers).
> - hci_set_ext_adv_params_sync: new method
> - hci_set_ext_adv_data_sync: move within source file (no changes)
> - hci_set_adv_data_sync: dito
> - hci_update_adv_data_sync: dito
> - hci_cc_set_ext_adv_param: remove (performed synchronously now)
> 
>  net/bluetooth/hci_event.c |  36 -------
>  net/bluetooth/hci_sync.c  | 207 ++++++++++++++++++++++++--------------
>  2 files changed, 130 insertions(+), 113 deletions(-)
> 
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 66052d6aaa1d..4d5ace9d245d 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2150,40 +2150,6 @@ static u8 hci_cc_set_adv_param(struct hci_dev *hdev, void *data,
>  	return rp->status;
>  }
>  
> -static u8 hci_cc_set_ext_adv_param(struct hci_dev *hdev, void *data,
> -				   struct sk_buff *skb)
> -{
> -	struct hci_rp_le_set_ext_adv_params *rp = data;
> -	struct hci_cp_le_set_ext_adv_params *cp;
> -	struct adv_info *adv_instance;
> -
> -	bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
> -
> -	if (rp->status)
> -		return rp->status;
> -
> -	cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS);
> -	if (!cp)
> -		return rp->status;
> -
> -	hci_dev_lock(hdev);
> -	hdev->adv_addr_type = cp->own_addr_type;
> -	if (!cp->handle) {
> -		/* Store in hdev for instance 0 */
> -		hdev->adv_tx_power = rp->tx_power;
> -	} else {
> -		adv_instance = hci_find_adv_instance(hdev, cp->handle);
> -		if (adv_instance)
> -			adv_instance->tx_power = rp->tx_power;
> -	}
> -	/* Update adv data as tx power is known now */
> -	hci_update_adv_data(hdev, cp->handle);
> -
> -	hci_dev_unlock(hdev);
> -
> -	return rp->status;
> -}
> -
>  static u8 hci_cc_read_rssi(struct hci_dev *hdev, void *data,
>  			   struct sk_buff *skb)
>  {
> @@ -4164,8 +4130,6 @@ static const struct hci_cc {
>  	HCI_CC(HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
>  	       hci_cc_le_read_num_adv_sets,
>  	       sizeof(struct hci_rp_le_read_num_supported_adv_sets)),
> -	HCI_CC(HCI_OP_LE_SET_EXT_ADV_PARAMS, hci_cc_set_ext_adv_param,
> -	       sizeof(struct hci_rp_le_set_ext_adv_params)),
>  	HCI_CC_STATUS(HCI_OP_LE_SET_EXT_ADV_ENABLE,
>  		      hci_cc_le_set_ext_adv_enable),
>  	HCI_CC_STATUS(HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 1f8806dfa556..563614b53485 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -1205,9 +1205,126 @@ static int hci_set_adv_set_random_addr_sync(struct hci_dev *hdev, u8 instance,
>  				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
>  }
>  
> +static int
> +hci_set_ext_adv_params_sync(struct hci_dev *hdev, struct adv_info *adv,
> +			    const struct hci_cp_le_set_ext_adv_params *cp,
> +			    struct hci_rp_le_set_ext_adv_params *rp)
> +{
> +	struct sk_buff *skb;
> +
> +	skb = __hci_cmd_sync(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(*cp),
> +			     cp, HCI_CMD_TIMEOUT);
> +
> +	/* If command return a status event, skb will be set to -ENODATA */
> +	if (skb == ERR_PTR(-ENODATA))
> +		return 0;
> +
> +	if (IS_ERR(skb)) {
> +		bt_dev_err(hdev, "Opcode 0x%4.4x failed: %ld",
> +			   HCI_OP_LE_SET_EXT_ADV_PARAMS, PTR_ERR(skb));
> +		return PTR_ERR(skb);
> +	}
> +
> +	if (skb->len != sizeof(*rp)) {
> +		bt_dev_err(hdev, "Invalid response length for "
> +			   "HCI_OP_LE_SET_EXT_ADV_PARAMS: %u", skb->len);
> +		kfree_skb(skb);
> +		return -EIO;
> +	}
> +
> +	memcpy(rp, skb->data, sizeof(*rp));
> +	kfree_skb(skb);
> +
> +	if (!rp->status) {
> +		hdev->adv_addr_type = cp->own_addr_type;
> +		if (!cp->handle) {
> +			/* Store in hdev for instance 0 */
> +			hdev->adv_tx_power = rp->tx_power;
> +		} else if (adv) {
> +			adv->tx_power = rp->tx_power;
> +		}
> +	}
> +
> +	return rp->status;
> +}
> +
> +static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
> +{
> +	DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
> +		    HCI_MAX_EXT_AD_LENGTH);
> +	u8 len;
> +	struct adv_info *adv = NULL;
> +	int err;
> +
> +	if (instance) {
> +		adv = hci_find_adv_instance(hdev, instance);
> +		if (!adv || !adv->adv_data_changed)
> +			return 0;
> +	}
> +
> +	len = eir_create_adv_data(hdev, instance, pdu->data,
> +				  HCI_MAX_EXT_AD_LENGTH);
> +
> +	pdu->length = len;
> +	pdu->handle = adv ? adv->handle : instance;
> +	pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
> +	pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
> +
> +	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
> +				    struct_size(pdu, data, len), pdu,
> +				    HCI_CMD_TIMEOUT);
> +	if (err)
> +		return err;
> +
> +	/* Update data if the command succeed */
> +	if (adv) {
> +		adv->adv_data_changed = false;
> +	} else {
> +		memcpy(hdev->adv_data, pdu->data, len);
> +		hdev->adv_data_len = len;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
> +{
> +	struct hci_cp_le_set_adv_data cp;
> +	u8 len;
> +
> +	memset(&cp, 0, sizeof(cp));
> +
> +	len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
> +
> +	/* There's nothing to do if the data hasn't changed */
> +	if (hdev->adv_data_len == len &&
> +	    memcmp(cp.data, hdev->adv_data, len) == 0)
> +		return 0;
> +
> +	memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
> +	hdev->adv_data_len = len;
> +
> +	cp.length = len;
> +
> +	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
> +				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> +}
> +
> +int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
> +{
> +	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
> +		return 0;
> +
> +	if (ext_adv_capable(hdev))
> +		return hci_set_ext_adv_data_sync(hdev, instance);
> +
> +	return hci_set_adv_data_sync(hdev, instance);
> +}
> +
>  int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
>  {
>  	struct hci_cp_le_set_ext_adv_params cp;
> +	struct hci_rp_le_set_ext_adv_params rp;
>  	bool connectable;
>  	u32 flags;
>  	bdaddr_t random_addr;
> @@ -1316,8 +1433,12 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
>  		cp.secondary_phy = HCI_ADV_PHY_1M;
>  	}
>  
> -	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
> -				    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> +	err = hci_set_ext_adv_params_sync(hdev, adv, &cp, &rp);
> +	if (err)
> +		return err;
> +
> +	/* Update adv data as tx power is known now */
> +	err = hci_set_ext_adv_data_sync(hdev, cp.handle);
>  	if (err)
>  		return err;
>  
> @@ -1822,79 +1943,6 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason)
>  				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
>  }
>  
> -static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
> -{
> -	DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
> -		    HCI_MAX_EXT_AD_LENGTH);
> -	u8 len;
> -	struct adv_info *adv = NULL;
> -	int err;
> -
> -	if (instance) {
> -		adv = hci_find_adv_instance(hdev, instance);
> -		if (!adv || !adv->adv_data_changed)
> -			return 0;
> -	}
> -
> -	len = eir_create_adv_data(hdev, instance, pdu->data,
> -				  HCI_MAX_EXT_AD_LENGTH);
> -
> -	pdu->length = len;
> -	pdu->handle = adv ? adv->handle : instance;
> -	pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
> -	pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
> -
> -	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
> -				    struct_size(pdu, data, len), pdu,
> -				    HCI_CMD_TIMEOUT);
> -	if (err)
> -		return err;
> -
> -	/* Update data if the command succeed */
> -	if (adv) {
> -		adv->adv_data_changed = false;
> -	} else {
> -		memcpy(hdev->adv_data, pdu->data, len);
> -		hdev->adv_data_len = len;
> -	}
> -
> -	return 0;
> -}
> -
> -static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
> -{
> -	struct hci_cp_le_set_adv_data cp;
> -	u8 len;
> -
> -	memset(&cp, 0, sizeof(cp));
> -
> -	len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
> -
> -	/* There's nothing to do if the data hasn't changed */
> -	if (hdev->adv_data_len == len &&
> -	    memcmp(cp.data, hdev->adv_data, len) == 0)
> -		return 0;
> -
> -	memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
> -	hdev->adv_data_len = len;
> -
> -	cp.length = len;
> -
> -	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
> -				     sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> -}
> -
> -int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
> -{
> -	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
> -		return 0;
> -
> -	if (ext_adv_capable(hdev))
> -		return hci_set_ext_adv_data_sync(hdev, instance);
> -
> -	return hci_set_adv_data_sync(hdev, instance);
> -}
> -
>  int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance,
>  				   bool force)
>  {
> @@ -6269,6 +6317,7 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
>  						struct hci_conn *conn)
>  {
>  	struct hci_cp_le_set_ext_adv_params cp;
> +	struct hci_rp_le_set_ext_adv_params rp;
>  	int err;
>  	bdaddr_t random_addr;
>  	u8 own_addr_type;
> @@ -6310,8 +6359,12 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
>  	if (err)
>  		return err;
>  
> -	err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
> -				    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> +	err = hci_set_ext_adv_params_sync(hdev, NULL, &cp, &rp);
> +	if (err)
> +		return err;
> +
> +	/* Update adv data as tx power is known now */
> +	err = hci_set_ext_adv_data_sync(hdev, cp.handle);
>  	if (err)
>  		return err;
>  
> 





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

* Re: [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously
  2025-06-27 10:09 ` [PATCH v3] " Christian Eggers
@ 2025-06-27 15:25   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2025-06-27 15:25 UTC (permalink / raw)
  To: Christian Eggers
  Cc: Marcel Holtmann, Johan Hedberg, Jaganath Kanakkassery,
	linux-bluetooth, netdev, linux-kernel, stable

Hi Christian,

On Fri, Jun 27, 2025 at 6:09 AM Christian Eggers <ceggers@arri.de> wrote:
>
> Hi Luiz,
>
> after changing my test setup (I now only use Mesh, no "normal" advertising
> anymore), I get many of these errors:
>
> bluetooth-meshd[276]: @ MGMT Command: Mesh Send (0x0059) plen 40                                                                   {0x0001} [hci0] 43.846388
>         Address: 00:00:00:00:00:00 (OUI 00-00-00)
>         Addr Type: 2
>         Instant: 0x0000000000000000
>         Delay: 0
>         Count: 1
>         Data Length: 21
>         Data[21]: 142b003a8b6fe779bd4385a94fed0a9cf611880000
> < HCI Command: LE Set Extended Advertising Parameters (0x08|0x0036) plen 25                                                            #479 [hci0] 43.846505
>         Handle: 0x05
>         Properties: 0x0010
>           Use legacy advertising PDUs: ADV_NONCONN_IND
>         Min advertising interval: 1280.000 msec (0x0800)
>         Max advertising interval: 1280.000 msec (0x0800)
>         Channel map: 37, 38, 39 (0x07)
>         Own address type: Random (0x01)
>         Peer address type: Public (0x00)
>         Peer address: 00:00:00:00:00:00 (OUI 00-00-00)
>         Filter policy: Allow Scan Request from Any, Allow Connect Request from Any (0x00)
>         TX power: Host has no preference (0x7f)
>         Primary PHY: LE 1M (0x01)
>         Secondary max skip: 0x00
>         Secondary PHY: LE 1M (0x01)
>         SID: 0x00
>         Scan request notifications: Disabled (0x00)
> > HCI Event: Command Complete (0x0e) plen 5                                                                                            #480 [hci0] 43.847480
>       LE Set Extended Advertising Parameters (0x08|0x0036) ncmd 2
> --->    Status: Command Disallowed (0x0c)
>         TX power (selected): 0 dbm (0x00)
>
>
> From the btmon output it is obvious that advertising is not disabled before updating the parameters.
>
> I added the following debug line in hci_setup_ext_adv_instance_sync():
>
>         printk(KERN_ERR "instance = %u, adv = %p, adv->pending = %d, adv->enabled = %d\n",
>                instance, adv, adv ? adv->pending : -1, adv ? adv->enabled : -1);
>
> From the debug output I see that adv->pending is still true (so advertising is not disabled
> before setting the advertising params). After changing the check from
>
>         if (adv && !adv->pending) {
>
> to
>
>         if (adv && adv->enabled) {
>
> it seems to do the job correctly. What do you think?

Yeah, that is indeed a bug, in fact we can just leave for
hci_disable_ext_adv_instance_sync to detect if the instance is
enabled:

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 2e0e532384c3..13ebd1a380fd 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1228,7 +1228,7 @@ int hci_setup_ext_adv_instance_sync(struct
hci_dev *hdev, u8 instance)
         * Command Disallowed error, so we must first disable the
         * instance if it is active.
         */
-       if (adv && !adv->pending) {
+       if (adv) {
                err = hci_disable_ext_adv_instance_sync(hdev, instance);
                if (err)
                        return err;


>
> regards,
> Christian
>
>
> On Friday, 27 June 2025, 09:05:08 CEST, Christian Eggers wrote:
> > Currently, for controllers with extended advertising, the advertising
> > data is set in the asynchronous response handler for extended
> > adverstising params. As most advertising settings are performed in a
> > synchronous context, the (asynchronous) setting of the advertising data
> > is done too late (after enabling the advertising).
> >
> > Move setting of adverstising data from asynchronous response handler
> > into synchronous context to fix ordering of HCI commands.
> >
> > Signed-off-by: Christian Eggers <ceggers@arri.de>
> > Fixes: a0fb3726ba55 ("Bluetooth: Use Set ext adv/scan rsp data if controller supports")
> > Cc: stable@vger.kernel.org
> > v2: https://lore.kernel.org/linux-bluetooth/20250626115209.17839-1-ceggers@arri.de/
> > ---
> > v3: refactor: store adv_addr_type/tx_power within hci_set_ext_adv_params_sync()
> >
> > v2: convert setting of adv data into synchronous context (rather than moving
> > more methods into asynchronous response handlers).
> > - hci_set_ext_adv_params_sync: new method
> > - hci_set_ext_adv_data_sync: move within source file (no changes)
> > - hci_set_adv_data_sync: dito
> > - hci_update_adv_data_sync: dito
> > - hci_cc_set_ext_adv_param: remove (performed synchronously now)
> >
> >  net/bluetooth/hci_event.c |  36 -------
> >  net/bluetooth/hci_sync.c  | 207 ++++++++++++++++++++++++--------------
> >  2 files changed, 130 insertions(+), 113 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> > index 66052d6aaa1d..4d5ace9d245d 100644
> > --- a/net/bluetooth/hci_event.c
> > +++ b/net/bluetooth/hci_event.c
> > @@ -2150,40 +2150,6 @@ static u8 hci_cc_set_adv_param(struct hci_dev *hdev, void *data,
> >       return rp->status;
> >  }
> >
> > -static u8 hci_cc_set_ext_adv_param(struct hci_dev *hdev, void *data,
> > -                                struct sk_buff *skb)
> > -{
> > -     struct hci_rp_le_set_ext_adv_params *rp = data;
> > -     struct hci_cp_le_set_ext_adv_params *cp;
> > -     struct adv_info *adv_instance;
> > -
> > -     bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
> > -
> > -     if (rp->status)
> > -             return rp->status;
> > -
> > -     cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS);
> > -     if (!cp)
> > -             return rp->status;
> > -
> > -     hci_dev_lock(hdev);
> > -     hdev->adv_addr_type = cp->own_addr_type;
> > -     if (!cp->handle) {
> > -             /* Store in hdev for instance 0 */
> > -             hdev->adv_tx_power = rp->tx_power;
> > -     } else {
> > -             adv_instance = hci_find_adv_instance(hdev, cp->handle);
> > -             if (adv_instance)
> > -                     adv_instance->tx_power = rp->tx_power;
> > -     }
> > -     /* Update adv data as tx power is known now */
> > -     hci_update_adv_data(hdev, cp->handle);
> > -
> > -     hci_dev_unlock(hdev);
> > -
> > -     return rp->status;
> > -}
> > -
> >  static u8 hci_cc_read_rssi(struct hci_dev *hdev, void *data,
> >                          struct sk_buff *skb)
> >  {
> > @@ -4164,8 +4130,6 @@ static const struct hci_cc {
> >       HCI_CC(HCI_OP_LE_READ_NUM_SUPPORTED_ADV_SETS,
> >              hci_cc_le_read_num_adv_sets,
> >              sizeof(struct hci_rp_le_read_num_supported_adv_sets)),
> > -     HCI_CC(HCI_OP_LE_SET_EXT_ADV_PARAMS, hci_cc_set_ext_adv_param,
> > -            sizeof(struct hci_rp_le_set_ext_adv_params)),
> >       HCI_CC_STATUS(HCI_OP_LE_SET_EXT_ADV_ENABLE,
> >                     hci_cc_le_set_ext_adv_enable),
> >       HCI_CC_STATUS(HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index 1f8806dfa556..563614b53485 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -1205,9 +1205,126 @@ static int hci_set_adv_set_random_addr_sync(struct hci_dev *hdev, u8 instance,
> >                                    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> >  }
> >
> > +static int
> > +hci_set_ext_adv_params_sync(struct hci_dev *hdev, struct adv_info *adv,
> > +                         const struct hci_cp_le_set_ext_adv_params *cp,
> > +                         struct hci_rp_le_set_ext_adv_params *rp)
> > +{
> > +     struct sk_buff *skb;
> > +
> > +     skb = __hci_cmd_sync(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS, sizeof(*cp),
> > +                          cp, HCI_CMD_TIMEOUT);
> > +
> > +     /* If command return a status event, skb will be set to -ENODATA */
> > +     if (skb == ERR_PTR(-ENODATA))
> > +             return 0;
> > +
> > +     if (IS_ERR(skb)) {
> > +             bt_dev_err(hdev, "Opcode 0x%4.4x failed: %ld",
> > +                        HCI_OP_LE_SET_EXT_ADV_PARAMS, PTR_ERR(skb));
> > +             return PTR_ERR(skb);
> > +     }
> > +
> > +     if (skb->len != sizeof(*rp)) {
> > +             bt_dev_err(hdev, "Invalid response length for "
> > +                        "HCI_OP_LE_SET_EXT_ADV_PARAMS: %u", skb->len);
> > +             kfree_skb(skb);
> > +             return -EIO;
> > +     }
> > +
> > +     memcpy(rp, skb->data, sizeof(*rp));
> > +     kfree_skb(skb);
> > +
> > +     if (!rp->status) {
> > +             hdev->adv_addr_type = cp->own_addr_type;
> > +             if (!cp->handle) {
> > +                     /* Store in hdev for instance 0 */
> > +                     hdev->adv_tx_power = rp->tx_power;
> > +             } else if (adv) {
> > +                     adv->tx_power = rp->tx_power;
> > +             }
> > +     }
> > +
> > +     return rp->status;
> > +}
> > +
> > +static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > +{
> > +     DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
> > +                 HCI_MAX_EXT_AD_LENGTH);
> > +     u8 len;
> > +     struct adv_info *adv = NULL;
> > +     int err;
> > +
> > +     if (instance) {
> > +             adv = hci_find_adv_instance(hdev, instance);
> > +             if (!adv || !adv->adv_data_changed)
> > +                     return 0;
> > +     }
> > +
> > +     len = eir_create_adv_data(hdev, instance, pdu->data,
> > +                               HCI_MAX_EXT_AD_LENGTH);
> > +
> > +     pdu->length = len;
> > +     pdu->handle = adv ? adv->handle : instance;
> > +     pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
> > +     pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
> > +
> > +     err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
> > +                                 struct_size(pdu, data, len), pdu,
> > +                                 HCI_CMD_TIMEOUT);
> > +     if (err)
> > +             return err;
> > +
> > +     /* Update data if the command succeed */
> > +     if (adv) {
> > +             adv->adv_data_changed = false;
> > +     } else {
> > +             memcpy(hdev->adv_data, pdu->data, len);
> > +             hdev->adv_data_len = len;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > +{
> > +     struct hci_cp_le_set_adv_data cp;
> > +     u8 len;
> > +
> > +     memset(&cp, 0, sizeof(cp));
> > +
> > +     len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
> > +
> > +     /* There's nothing to do if the data hasn't changed */
> > +     if (hdev->adv_data_len == len &&
> > +         memcmp(cp.data, hdev->adv_data, len) == 0)
> > +             return 0;
> > +
> > +     memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
> > +     hdev->adv_data_len = len;
> > +
> > +     cp.length = len;
> > +
> > +     return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
> > +                                  sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> > +}
> > +
> > +int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > +{
> > +     if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
> > +             return 0;
> > +
> > +     if (ext_adv_capable(hdev))
> > +             return hci_set_ext_adv_data_sync(hdev, instance);
> > +
> > +     return hci_set_adv_data_sync(hdev, instance);
> > +}
> > +
> >  int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
> >  {
> >       struct hci_cp_le_set_ext_adv_params cp;
> > +     struct hci_rp_le_set_ext_adv_params rp;
> >       bool connectable;
> >       u32 flags;
> >       bdaddr_t random_addr;
> > @@ -1316,8 +1433,12 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance)
> >               cp.secondary_phy = HCI_ADV_PHY_1M;
> >       }
> >
> > -     err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
> > -                                 sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> > +     err = hci_set_ext_adv_params_sync(hdev, adv, &cp, &rp);
> > +     if (err)
> > +             return err;
> > +
> > +     /* Update adv data as tx power is known now */
> > +     err = hci_set_ext_adv_data_sync(hdev, cp.handle);
> >       if (err)
> >               return err;
> >
> > @@ -1822,79 +1943,6 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason)
> >                                    sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> >  }
> >
> > -static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > -{
> > -     DEFINE_FLEX(struct hci_cp_le_set_ext_adv_data, pdu, data, length,
> > -                 HCI_MAX_EXT_AD_LENGTH);
> > -     u8 len;
> > -     struct adv_info *adv = NULL;
> > -     int err;
> > -
> > -     if (instance) {
> > -             adv = hci_find_adv_instance(hdev, instance);
> > -             if (!adv || !adv->adv_data_changed)
> > -                     return 0;
> > -     }
> > -
> > -     len = eir_create_adv_data(hdev, instance, pdu->data,
> > -                               HCI_MAX_EXT_AD_LENGTH);
> > -
> > -     pdu->length = len;
> > -     pdu->handle = adv ? adv->handle : instance;
> > -     pdu->operation = LE_SET_ADV_DATA_OP_COMPLETE;
> > -     pdu->frag_pref = LE_SET_ADV_DATA_NO_FRAG;
> > -
> > -     err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_DATA,
> > -                                 struct_size(pdu, data, len), pdu,
> > -                                 HCI_CMD_TIMEOUT);
> > -     if (err)
> > -             return err;
> > -
> > -     /* Update data if the command succeed */
> > -     if (adv) {
> > -             adv->adv_data_changed = false;
> > -     } else {
> > -             memcpy(hdev->adv_data, pdu->data, len);
> > -             hdev->adv_data_len = len;
> > -     }
> > -
> > -     return 0;
> > -}
> > -
> > -static int hci_set_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > -{
> > -     struct hci_cp_le_set_adv_data cp;
> > -     u8 len;
> > -
> > -     memset(&cp, 0, sizeof(cp));
> > -
> > -     len = eir_create_adv_data(hdev, instance, cp.data, sizeof(cp.data));
> > -
> > -     /* There's nothing to do if the data hasn't changed */
> > -     if (hdev->adv_data_len == len &&
> > -         memcmp(cp.data, hdev->adv_data, len) == 0)
> > -             return 0;
> > -
> > -     memcpy(hdev->adv_data, cp.data, sizeof(cp.data));
> > -     hdev->adv_data_len = len;
> > -
> > -     cp.length = len;
> > -
> > -     return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_DATA,
> > -                                  sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> > -}
> > -
> > -int hci_update_adv_data_sync(struct hci_dev *hdev, u8 instance)
> > -{
> > -     if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
> > -             return 0;
> > -
> > -     if (ext_adv_capable(hdev))
> > -             return hci_set_ext_adv_data_sync(hdev, instance);
> > -
> > -     return hci_set_adv_data_sync(hdev, instance);
> > -}
> > -
> >  int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance,
> >                                  bool force)
> >  {
> > @@ -6269,6 +6317,7 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
> >                                               struct hci_conn *conn)
> >  {
> >       struct hci_cp_le_set_ext_adv_params cp;
> > +     struct hci_rp_le_set_ext_adv_params rp;
> >       int err;
> >       bdaddr_t random_addr;
> >       u8 own_addr_type;
> > @@ -6310,8 +6359,12 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
> >       if (err)
> >               return err;
> >
> > -     err = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_EXT_ADV_PARAMS,
> > -                                 sizeof(cp), &cp, HCI_CMD_TIMEOUT);
> > +     err = hci_set_ext_adv_params_sync(hdev, NULL, &cp, &rp);
> > +     if (err)
> > +             return err;
> > +
> > +     /* Update adv data as tx power is known now */
> > +     err = hci_set_ext_adv_data_sync(hdev, cp.handle);
> >       if (err)
> >               return err;
> >
> >
>
>
>
>


-- 
Luiz Augusto von Dentz

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

end of thread, other threads:[~2025-06-27 15:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-27  7:05 [PATCH v3] Bluetooth: HCI: Set extended advertising data synchronously Christian Eggers
2025-06-27  7:35 ` [v3] " bluez.test.bot
2025-06-27 10:09 ` [PATCH v3] " Christian Eggers
2025-06-27 15:25   ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox