[kernel PATCH v1] Bluetooth: hci_sync: Resume adv with no RPA when active scan

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

If the host is using RPA, the controller needs to generate RPA for
the advertising, so the advertising must remain paused during the
active scan.

If the host is not using RPA, the advertising can be resumed after
disabling the address resolution.

Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
Signed-off-by: Zhengping Jiang <redacted>
---

Changes in v1:
- Always pause advertising when active scan, but resume the advertising if the host is not using RPA

 net/bluetooth/hci_sync.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 117eedb6f709..edbf9faf7fa1 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c

@@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 	u8 filter_policy;
 	int err;
 
-	/* Pause advertising if resolving list can be used as controllers are
+	/* Pause advertising if resolving list can be used as controllers
 	 * cannot accept resolving list modifications while advertising.
 	 */
 	if (use_ll_privacy(hdev)) {

@@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
 	/* Pause advertising since active scanning disables address resolution
 	 * which advertising depend on in order to generate its RPAs.
 	 */
-	if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
+	if (use_ll_privacy(hdev)) {
 		err = hci_pause_advertising_sync(hdev);
 		if (err) {
 			bt_dev_err(hdev, "pause advertising failed: %d", err);

@@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
 		goto failed;
 	}
 
+	// Resume paused advertising if the host is not using RPA
+	if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
+		hci_resume_advertising_sync(hdev);
+
 	/* All active scans will be done with either a resolvable private
 	 * address (when privacy feature has been enabled) or non-resolvable
 	 * private address.

-- 
2.39.1.581.gbfd45094c4-goog

Hi Zhengping,

On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang [off-list ref] wrote:

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

It is probably a good idea to quote the spec saying:

7.8.44 LE Set Address Resolution Enable command

This command shall not be used when:
• Advertising (other than periodic advertising) is enabled,

I think it is better that we add something like
hci_pause_addr_resolution so we can make it check all the conditions,
such as pausing advertising and resuming if needed. Btw, we do seem to
have proper checks for these conditions on the emulator:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090

But perhaps there is no test which attempts to enable LL Privacy
without enabling Local Privacy, so it would be great if you could
update mgmt-tester adding a test that emulates such behavior.

-- 
Luiz Augusto von Dentz

Hi Luiz,

Thanks for the comment. I will submit a new patch to address that.

I notice in the spec, it is mentioned

Note: This command does not affect the generation of Resolvable Private Addresses.

How should I understand this note? Does it mean even if the address
resolution is disabled, the controller can still generate RPA for
advertising?  Does it mean the advertising can always be resumed
during active scan?

Thanks,
Zhengping

On Tue, Feb 14, 2023 at 4:09 PM Luiz Augusto von Dentz
[off-list ref] wrote:

Hi Zhengping,

On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang [off-list ref] wrote:

quoted

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

It is probably a good idea to quote the spec saying:

7.8.44 LE Set Address Resolution Enable command

This command shall not be used when:
• Advertising (other than periodic advertising) is enabled,

quoted

If the host is using RPA, the controller needs to generate RPA for
the advertising, so the advertising must remain paused during the
active scan.

If the host is not using RPA, the advertising can be resumed after
disabling the address resolution.

Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
Signed-off-by: Zhengping Jiang <redacted>
---

Changes in v1:
- Always pause advertising when active scan, but resume the advertising if the host is not using RPA

 net/bluetooth/hci_sync.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 117eedb6f709..edbf9faf7fa1 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c

@@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
        u8 filter_policy;
        int err;

-       /* Pause advertising if resolving list can be used as controllers are
+       /* Pause advertising if resolving list can be used as controllers
         * cannot accept resolving list modifications while advertising.
         */
        if (use_ll_privacy(hdev)) {

@@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
        /* Pause advertising since active scanning disables address resolution
         * which advertising depend on in order to generate its RPAs.
         */
-       if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
+       if (use_ll_privacy(hdev)) {
                err = hci_pause_advertising_sync(hdev);
                if (err) {
                        bt_dev_err(hdev, "pause advertising failed: %d", err);

@@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
                goto failed;
        }

+       // Resume paused advertising if the host is not using RPA
+       if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
+               hci_resume_advertising_sync(hdev);
+
        /* All active scans will be done with either a resolvable private
         * address (when privacy feature has been enabled) or non-resolvable
         * private address.
--

2.39.1.581.gbfd45094c4-goog

I think it is better that we add something like
hci_pause_addr_resolution so we can make it check all the conditions,
such as pausing advertising and resuming if needed. Btw, we do seem to
have proper checks for these conditions on the emulator:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090

But perhaps there is no test which attempts to enable LL Privacy
without enabling Local Privacy, so it would be great if you could
update mgmt-tester adding a test that emulates such behavior.

--
Luiz Augusto von Dentz

Hi Zhengping,

On Tue, Feb 14, 2023 at 4:28 PM Zhengping Jiang [off-list ref] wrote:

Hi Luiz,

Thanks for the comment. I will submit a new patch to address that.

I notice in the spec, it is mentioned

quoted

Note: This command does not affect the generation of Resolvable Private Addresses.

Where is that mentioned?

How should I understand this note? Does it mean even if the address
resolution is disabled, the controller can still generate RPA for
advertising?  Does it mean the advertising can always be resumed
during active scan?

I think it may be related to the fact that it only affects the addr
resolution of remote devices, that said if you are active scanning
that probably means the user wants to setup a new device thus why we
don't enable any filtering like accept list, etc, so it is not really
useful to keep address resolution active either way.

Thanks,
Zhengping

On Tue, Feb 14, 2023 at 4:09 PM Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

Hi Zhengping,

On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang [off-list ref] wrote:

quoted

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

It is probably a good idea to quote the spec saying:

7.8.44 LE Set Address Resolution Enable command

This command shall not be used when:
• Advertising (other than periodic advertising) is enabled,

quoted

If the host is using RPA, the controller needs to generate RPA for
the advertising, so the advertising must remain paused during the
active scan.

If the host is not using RPA, the advertising can be resumed after
disabling the address resolution.

Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
Signed-off-by: Zhengping Jiang <redacted>
---

Changes in v1:
- Always pause advertising when active scan, but resume the advertising if the host is not using RPA

 net/bluetooth/hci_sync.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 117eedb6f709..edbf9faf7fa1 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c

@@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
        u8 filter_policy;
        int err;

-       /* Pause advertising if resolving list can be used as controllers are
+       /* Pause advertising if resolving list can be used as controllers
         * cannot accept resolving list modifications while advertising.
         */
        if (use_ll_privacy(hdev)) {

@@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
        /* Pause advertising since active scanning disables address resolution
         * which advertising depend on in order to generate its RPAs.
         */
-       if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
+       if (use_ll_privacy(hdev)) {
                err = hci_pause_advertising_sync(hdev);
                if (err) {
                        bt_dev_err(hdev, "pause advertising failed: %d", err);

@@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
                goto failed;
        }

+       // Resume paused advertising if the host is not using RPA
+       if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
+               hci_resume_advertising_sync(hdev);
+
        /* All active scans will be done with either a resolvable private
         * address (when privacy feature has been enabled) or non-resolvable
         * private address.
--

2.39.1.581.gbfd45094c4-goog

I think it is better that we add something like
hci_pause_addr_resolution so we can make it check all the conditions,
such as pausing advertising and resuming if needed. Btw, we do seem to
have proper checks for these conditions on the emulator:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090

But perhaps there is no test which attempts to enable LL Privacy
without enabling Local Privacy, so it would be great if you could
update mgmt-tester adding a test that emulates such behavior.

--
Luiz Augusto von Dentz

-- 
Luiz Augusto von Dentz

Hi Luiz,

Where is that mentioned?

It is just below the command on "7.8.44 LE Set Address Resolution
Enable command".

On "4.7 RESOLVING LIST", there is another note:

Note: The Controller may generate Resolvable Private Addresses even when address resolution is disabled.

If this is the case, then the comment in the kernel
(hci_active_scan_sync) is not accurate.

/* Pause advertising since active scanning disables address resolution
* which advertising depend on in order to generate its RPAs.
*/

I think it may be related to the fact that it only affects the addr
resolution of remote devices, that said if you are active scanning
that probably means the user wants to setup a new device thus why we
don't enable any filtering like accept list, etc, so it is not really
useful to keep address resolution active either way.

That makes sense. When the local privacy is enabled, I assume the host
RPA will change
when advertising. I haven't tested that scenario, but if RPA
generation is not related to disable/enable
address resolution, why should the advertising be paused when active scan?

Thanks,
Zhengping

quoted

Thanks,
Zhengping

On Tue, Feb 14, 2023 at 4:09 PM Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

Hi Zhengping,

On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang [off-list ref] wrote:

quoted

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

It is probably a good idea to quote the spec saying:

7.8.44 LE Set Address Resolution Enable command

This command shall not be used when:
• Advertising (other than periodic advertising) is enabled,

quoted

If the host is using RPA, the controller needs to generate RPA for
the advertising, so the advertising must remain paused during the
active scan.

If the host is not using RPA, the advertising can be resumed after
disabling the address resolution.

Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
Signed-off-by: Zhengping Jiang <redacted>
---

Changes in v1:
- Always pause advertising when active scan, but resume the advertising if the host is not using RPA

 net/bluetooth/hci_sync.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 117eedb6f709..edbf9faf7fa1 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c

@@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
        u8 filter_policy;
        int err;

-       /* Pause advertising if resolving list can be used as controllers are
+       /* Pause advertising if resolving list can be used as controllers
         * cannot accept resolving list modifications while advertising.
         */
        if (use_ll_privacy(hdev)) {

@@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
        /* Pause advertising since active scanning disables address resolution
         * which advertising depend on in order to generate its RPAs.
         */
-       if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
+       if (use_ll_privacy(hdev)) {
                err = hci_pause_advertising_sync(hdev);
                if (err) {
                        bt_dev_err(hdev, "pause advertising failed: %d", err);

@@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
                goto failed;
        }

+       // Resume paused advertising if the host is not using RPA
+       if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
+               hci_resume_advertising_sync(hdev);
+
        /* All active scans will be done with either a resolvable private
         * address (when privacy feature has been enabled) or non-resolvable
         * private address.
--

2.39.1.581.gbfd45094c4-goog

I think it is better that we add something like
hci_pause_addr_resolution so we can make it check all the conditions,
such as pausing advertising and resuming if needed. Btw, we do seem to
have proper checks for these conditions on the emulator:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090

But perhaps there is no test which attempts to enable LL Privacy
without enabling Local Privacy, so it would be great if you could
update mgmt-tester adding a test that emulates such behavior.

--
Luiz Augusto von Dentz

--
Luiz Augusto von Dentz

Hi Zhengping,

On Tue, Feb 14, 2023 at 5:20 PM Zhengping Jiang [off-list ref] wrote:

Hi Luiz,

quoted

Where is that mentioned?

It is just below the command on "7.8.44 LE Set Address Resolution
Enable command".

On "4.7 RESOLVING LIST", there is another note:

quoted

Note: The Controller may generate Resolvable Private Addresses even when address resolution is disabled.

Note that the term used here is 'may' not 'shall' so I don't think we
can take for granted that the controller will keep generating a new
RPA while address resolution is disabled, in fact I think this is sort
of a bad idea since as part of the resolving list is the actual IRK
and the procedures to update those entries requires address resolution
to be stopped, in fact I think the spec is sort of responsible for
creating this confusion when it mixed up remote IRK with local IRK in
the same list.

If this is the case, then the comment in the kernel
(hci_active_scan_sync) is not accurate.

quoted

/* Pause advertising since active scanning disables address resolution
* which advertising depend on in order to generate its RPAs.
*/

quoted

I think it may be related to the fact that it only affects the addr
resolution of remote devices, that said if you are active scanning
that probably means the user wants to setup a new device thus why we
don't enable any filtering like accept list, etc, so it is not really
useful to keep address resolution active either way.

That makes sense. When the local privacy is enabled, I assume the host
RPA will change
when advertising. I haven't tested that scenario, but if RPA
generation is not related to disable/enable
address resolution, why should the advertising be paused when active scan?

Initially it was because we wanted to disable address resolution which
requires both scan and advertising to be disabled to begin with, then
there is the fact that there is no fine control over how the
controller would priority the scanning and advertising states, and
there is also the situation where a incoming connection may come in
when advertising is enabled then we have a connection competing with
scanning which creates even more problems, not to mention not all
controllers do support scanning and advertising simultaneously.

Thanks,
Zhengping

quoted

quoted

Thanks,
Zhengping

On Tue, Feb 14, 2023 at 4:09 PM Luiz Augusto von Dentz
[off-list ref] wrote:

quoted

Hi Zhengping,

On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang [off-list ref] wrote:

quoted

The address resolution should be disabled during the active scan,
so all the advertisements can reach the host. The advertising
has to be paused before disabling the address resolution,
because the advertising will prevent any changes to the resolving
list and the address resolution status. Skipping this will cause
the hci error and the discovery failure.

It is probably a good idea to quote the spec saying:

7.8.44 LE Set Address Resolution Enable command

This command shall not be used when:
• Advertising (other than periodic advertising) is enabled,

quoted

If the host is using RPA, the controller needs to generate RPA for
the advertising, so the advertising must remain paused during the
active scan.

If the host is not using RPA, the advertising can be resumed after
disabling the address resolution.

Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
Signed-off-by: Zhengping Jiang <redacted>
---

Changes in v1:
- Always pause advertising when active scan, but resume the advertising if the host is not using RPA

 net/bluetooth/hci_sync.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 117eedb6f709..edbf9faf7fa1 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c

@@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
        u8 filter_policy;
        int err;

-       /* Pause advertising if resolving list can be used as controllers are
+       /* Pause advertising if resolving list can be used as controllers
         * cannot accept resolving list modifications while advertising.
         */
        if (use_ll_privacy(hdev)) {

@@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
        /* Pause advertising since active scanning disables address resolution
         * which advertising depend on in order to generate its RPAs.
         */
-       if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
+       if (use_ll_privacy(hdev)) {
                err = hci_pause_advertising_sync(hdev);
                if (err) {
                        bt_dev_err(hdev, "pause advertising failed: %d", err);

@@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
                goto failed;
        }

+       // Resume paused advertising if the host is not using RPA
+       if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
+               hci_resume_advertising_sync(hdev);
+
        /* All active scans will be done with either a resolvable private
         * address (when privacy feature has been enabled) or non-resolvable
         * private address.
--

2.39.1.581.gbfd45094c4-goog

I think it is better that we add something like
hci_pause_addr_resolution so we can make it check all the conditions,
such as pausing advertising and resuming if needed. Btw, we do seem to
have proper checks for these conditions on the emulator:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090

But perhaps there is no test which attempts to enable LL Privacy
without enabling Local Privacy, so it would be great if you could
update mgmt-tester adding a test that emulates such behavior.

--
Luiz Augusto von Dentz

--
Luiz Augusto von Dentz

-- 
Luiz Augusto von Dentz