New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 760792 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug

Blocked on:
issue 776423



Sign in to add a comment

Error registering advertisement

Project Member Reported by khorimoto@chromium.org, Aug 30 2017

Issue description

We're consistently seeing errors registering advertisements.

Example log:
Error registering advertisement. Device ID: "CAESR...cGg==", Service data: 0x89e221b9, Error code: 1

Error code 1 is ERROR_ADVERTISEMENT_ALREADY_EXISTS. I've found that this ALWAYS fails when a StartNotifySession() call is in progress but not yet complete (see Issue 758051). However, I also see this issue by itself (see Issue 760769).
 
Hi Kyle, Jeremy:

  I doubt that the registering advertisement failure was on the bluez side. Here below is what register_advertisement() in bluez does:

(1) Check if the arguments of the method call are valid.
(2) Check the owner and the object path of the dbus message. If both owner and path matches what currently exist in the advertiser manager, it reports error of "already exists".
(3) does more thing afterwards...

You could see that the "already exists" error was caught and returned in the very beginning of the dbus method before more things are processed.

Hi Kyle, Jeremy, could you please help check the chrome side
- if it retries to register the same advertisement under some timeout situation?  or
- if it registers an advertisement, it accidentally uses the same "object path"?

Thanks!
As a complement of C#1 which I think might be a bit helpful, if you want to registers a new advertisement, you need to use a new object path.

For example, if you register an adv for phone1, its object path could be something like "/org/bluez/magic/phone1".

if you register another adv for phone2, its object path could be something like "/org/bluez/magic/phone2".

If you use the same object path, it would be considered the same adv. Two situations are possible.
S1: if the adv for phone1 has been unregistered, that would be fine.
S2: if the adv for phone1 is still ongoing, that would be a problem. The adv for phone2 would be considered "already exists".


Cc: josephsih@chromium.org
Hi Kyle, as the first step, we may want to clarify where on earth the problem occurs -- the chrome os side or the chrome side. I put together a simple document (https://docs.google.com/document/d/1LS_0KYtoBcuZufrvyWQEXzrk1o6EYM4KGt5vqjXrr6c/edit#) to verify this.
Hi Sonny, I suspect that there exists some kind of race condition in registering multiple advertisements simultaneously which leads to the situation that the same object path might be reused multiple times. Would you please take a look at the document in C#3? Thanks!

Comment 5 by r...@chromium.org, Sep 12 2017

I don't get it. We generate the object path in the constructor of the advertisement, hence if two advertisements were being registered, even if on different threads, they are going to get their own object paths.

Using a GUID for object paths can theoretically produce the same object path but the chances of it are unlikely enough that they really couldn't be the cause for this issue. The only way to register another advertisement is to create another advertisement, hence I am not sure how we can end up in a state where we re-register the same advertisement. Maybe I am missing something?


We need someone to debug this to see exactly what is happening.

FYI, I sent this CL to Miao/Sonny last week, which makes reproducing this bug trivial: https://chromium-review.googlesource.com/c/chromium/src/+/649956. In my experience, the repro rate with this CL is very high.
Labels: ReleaseBlock-Stable
Status: Assigned (was: Untriaged)
Attached are logs for this failure.
prox_tether_logs.txt
117 KB View Download
bt_messages
5.7 MB View Download
ui.LATEST
8.7 KB Download
messages
6.9 MB View Download
dmesg.log
65.2 KB View Download
chrome
764 KB View Download
btsnoop.log
1.0 MB View Download
Thanks Kyle for the logs. From the logs

- the object paths sent from chrome were all unique. (sorry for my false alarms)

- the Intel 0xF3 firmware had "HCI Command Disallowed" errors when registering, unregistering, and registering advertisements again. 

Refer to b/64731753 (https://buganizer.corp.google.com/issues/64731753#comment63)  for more context. A tarball of test scripts to reproduce the errors was also attached in the buganizer issue.

Comment 9 by snanda@chromium.org, Sep 13 2017

Happens frequently on Intel & non-Intel devices. Miao to look at logs to see if it is a dupe. 
Cool. Please close if dupe.
Re C#9: this seems to be a race condition in kernel. Hence, the issue may not be related with controller firmware.
mcchou@ please comment when you get a chance.
Labels: -M-61 M-62
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 2 2017

Labels: merge-merged-release-R61-9765.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f951bd7cba9adc88e1f9462152c0fdc5cbf25f78

commit f951bd7cba9adc88e1f9462152c0fdc5cbf25f78
Author: Joseph Hwang <josephsih@chromium.org>
Date: Mon Oct 02 22:18:59 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/695947
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/f951bd7cba9adc88e1f9462152c0fdc5cbf25f78/net/bluetooth/mgmt.c

Project Member

Comment 15 by bugdroid1@chromium.org, Oct 2 2017

Labels: merge-merged-release-R62-9901.B-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/12e0368363053eec77299d86939765c7ccda400b

commit 12e0368363053eec77299d86939765c7ccda400b
Author: Joseph Hwang <josephsih@chromium.org>
Date: Mon Oct 02 22:22:12 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/695948
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Commit-Queue: Miao-chen Chou <mcchou@chromium.org>
Tested-by: Miao-chen Chou <mcchou@chromium.org>

[modify] https://crrev.com/12e0368363053eec77299d86939765c7ccda400b/net/bluetooth/mgmt.c

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/410544cf87d9ab10dd4f9d41736d229fe00ecf6e

commit 410544cf87d9ab10dd4f9d41736d229fe00ecf6e
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Oct 03 01:18:02 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/695949
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
Commit-Queue: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Trybot-Ready: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/410544cf87d9ab10dd4f9d41736d229fe00ecf6e/net/bluetooth/mgmt.c

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/2604cfac7d7ade74c0c6d408d823851ed301a3fb

commit 2604cfac7d7ade74c0c6d408d823851ed301a3fb
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Oct 03 01:18:14 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/696496
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
Commit-Queue: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Trybot-Ready: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/2604cfac7d7ade74c0c6d408d823851ed301a3fb/net/bluetooth/mgmt.c

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a99ba8d3e62114115144713be9df5e2b129e61d6

commit a99ba8d3e62114115144713be9df5e2b129e61d6
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Oct 03 01:21:24 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/695950
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
Commit-Queue: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Trybot-Ready: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/a99ba8d3e62114115144713be9df5e2b129e61d6/net/bluetooth/mgmt.c

Project Member

Comment 19 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c85eb265363ceeaf3a09a1f27efd14df023b12fd

commit c85eb265363ceeaf3a09a1f27efd14df023b12fd
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Oct 03 01:21:29 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/696664
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
Commit-Queue: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Trybot-Ready: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/c85eb265363ceeaf3a09a1f27efd14df023b12fd/net/bluetooth/mgmt.c

I left my Eve in a tether scan overnight (with Joseph's patches), and I still ended up seeing the infinite loop of advertising errors. Attached logs.
adv_fails_after_patches.zip
7.4 MB Download
Cc: khorimoto@chromium.org
Hi Kyle, the advertising errors were caused by "controller disabled" errors which Intel is trying to fix.

In ui.LATEST:
[1240:1240:1002/232605.860471:ERROR:device_event_log_impl.cc(156)] [23:26:05.860] Bluetooth: bluetooth_adapter_bluez.cc:1593 /org/bluez/hci0: Failed to stop discovery: org.bluez.Error.NotReady: Resource Not Ready

The Resource Not Ready means the controller was disabled.

And by checking the messages, we could see lots of the following messages:

$ grep "hci0 has been disabled" messages
2017-10-02T15:35:16.613110-07:00 INFO bluetoothd[1692]: adapter /org/bluez/hci0 has been disabled

...

2017-10-02T23:26:05.846658-07:00 INFO bluetoothd[1759]: adapter /org/bluez/hci0 has been disabled


Hence, we would expect Intel to fix the controller issue.
Re C#22: 

  The ui.LATEST log should be 

[1240:1240:1002/232601.779787:ERROR:bluetooth_advertisement_bluez.cc(52)] Error while registering advertisement. error_name = org.freedesktop.DBus.Error.UnknownMethod, error_message = Method "RegisterAdvertisement" with signature "oa{sv}" on interface "org.bluez.LEAdvertisingManager1" doesn't exist

which was caused by consecutive "controller disabled" errors.  Intel is setting up MT to reproduce and fix the issue. Thanks.
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 5 2017

Labels: merge-merged-chromeos-4.4
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f6e740788524e8e2b68b982020aa97e15cb7761b

commit f6e740788524e8e2b68b982020aa97e15cb7761b
Author: Joseph Hwang <josephsih@chromium.org>
Date: Thu Oct 05 12:16:52 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694947
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>

[modify] https://crrev.com/f6e740788524e8e2b68b982020aa97e15cb7761b/net/bluetooth/mgmt.c

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/dbe68f64ae7e75cb6f487b86f036edade5e02612

commit dbe68f64ae7e75cb6f487b86f036edade5e02612
Author: Joseph Hwang <josephsih@chromium.org>
Date: Thu Oct 05 12:16:53 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694948
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/dbe68f64ae7e75cb6f487b86f036edade5e02612/net/bluetooth/mgmt.c

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1

commit 0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1
Author: Joseph Hwang <josephsih@chromium.org>
Date: Thu Oct 05 12:16:54 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694949
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1/net/bluetooth/mgmt.c

Status: Fixed (was: Assigned)
Blockedon: 776423
Status: Assigned (was: Fixed)
Labels: -Pri-1 Pri-0
Upping priority as we build the R62 stable RC tomorrow, we need a fix in the next 24 hours.
Do we have anything left to merge here?
bhthompson@: Yes - the updates are on blocking issue 776423.
I think we have merged what we were going to in 62, if there is nothing else to merge here should this bug drop the releaseblock-stable label?
Labels: -M-62
Status: Fixed (was: Assigned)
Yes - thanks for your patience, bhthompson@!
Project Member

Comment 35 by bugdroid1@chromium.org, Nov 7 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a3ac1f603184e24dbf1c19d2a410c326d450a29e

commit a3ac1f603184e24dbf1c19d2a410c326d450a29e
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:25:50 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694947
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
(cherry picked from commit f6e740788524e8e2b68b982020aa97e15cb7761b)
Reviewed-on: https://chromium-review.googlesource.com/754173
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/a3ac1f603184e24dbf1c19d2a410c326d450a29e/net/bluetooth/mgmt.c

Project Member

Comment 36 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3037b11bb2c142e065553ace8fdbb4979b413fdc

commit 3037b11bb2c142e065553ace8fdbb4979b413fdc
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:25:51 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754173

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694948
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit dbe68f64ae7e75cb6f487b86f036edade5e02612)
Reviewed-on: https://chromium-review.googlesource.com/754174
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/3037b11bb2c142e065553ace8fdbb4979b413fdc/net/bluetooth/mgmt.c

Project Member

Comment 37 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a442d54929c5d1b20d7349486982597917f946e8

commit a442d54929c5d1b20d7349486982597917f946e8
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:25:53 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754174

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694949
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit 0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1)
Reviewed-on: https://chromium-review.googlesource.com/754175
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/a442d54929c5d1b20d7349486982597917f946e8/net/bluetooth/mgmt.c

Project Member

Comment 38 by bugdroid1@chromium.org, Nov 7 2017

Labels: merge-merged-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e39adf00d9c5d6cad76dd378515838692762b131

commit e39adf00d9c5d6cad76dd378515838692762b131
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:25:57 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694947
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
(cherry picked from commit f6e740788524e8e2b68b982020aa97e15cb7761b)
Reviewed-on: https://chromium-review.googlesource.com/754403
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/e39adf00d9c5d6cad76dd378515838692762b131/net/bluetooth/mgmt.c

Project Member

Comment 39 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/54e59001881675969240be06f9e62ca0b6ca9fa6

commit 54e59001881675969240be06f9e62ca0b6ca9fa6
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:25:59 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754403

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694948
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit dbe68f64ae7e75cb6f487b86f036edade5e02612)
Reviewed-on: https://chromium-review.googlesource.com/754404
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/54e59001881675969240be06f9e62ca0b6ca9fa6/net/bluetooth/mgmt.c

Project Member

Comment 40 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/daa4c96f0f6e98d7b8386b7da12e93f3bd409267

commit daa4c96f0f6e98d7b8386b7da12e93f3bd409267
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:26:00 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754404

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694949
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit 0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1)
Reviewed-on: https://chromium-review.googlesource.com/754405
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/daa4c96f0f6e98d7b8386b7da12e93f3bd409267/net/bluetooth/mgmt.c

Project Member

Comment 41 by bugdroid1@chromium.org, Nov 7 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/79f1cb0cc9310183b58e147bdcc4766012c8e6fa

commit 79f1cb0cc9310183b58e147bdcc4766012c8e6fa
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:26:05 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to remove_advertising

If a conditional command is the last one in a request and is skipped,
bad thing occurs.

A work around is to append an additional HCI command in the tail of
the request command queue. This whatever HCI command triggers the
cleaning up work in the original hci event functions.

BUG= chromium:760792 
TEST=Run magic tether flow and verify that bluetoothd hears back from
kernel for the completion of remove_advertising request.

Change-Id: If22999fe1dc28d03ba73eeebd5f3c46398c477c3
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694947
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
(cherry picked from commit f6e740788524e8e2b68b982020aa97e15cb7761b)
Reviewed-on: https://chromium-review.googlesource.com/754225
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/79f1cb0cc9310183b58e147bdcc4766012c8e6fa/net/bluetooth/mgmt.c

Project Member

Comment 42 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a41cc7a2bbdb15302c49926a36c5f42b338a125a

commit a41cc7a2bbdb15302c49926a36c5f42b338a125a
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:26:07 2017

CHROMIUM: Bluetooth: mgmt: add mgmt_cmd_status in add_advertising

If an error occurs during request building in add_advertising(),
remember to send MGMT_STATUS_FAILED command status back to bluetoothd.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754225

Change-Id: I55dbc5deb11160bc2b356a1041ff2aa910ccdff1
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694948
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit dbe68f64ae7e75cb6f487b86f036edade5e02612)
Reviewed-on: https://chromium-review.googlesource.com/754226
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/a41cc7a2bbdb15302c49926a36c5f42b338a125a/net/bluetooth/mgmt.c

Project Member

Comment 43 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a6c01afc10ce567671d5b238ac6400284ea6869d

commit a6c01afc10ce567671d5b238ac6400284ea6869d
Author: Joseph Hwang <josephsih@chromium.org>
Date: Tue Nov 07 01:26:08 2017

CHROMIUM: Bluetooth: mgmt: add additional HCI cmd to add_advertising

When a process registers and unregisters advertisements consecutively,
it is likely that an advertisement with the same adv data and adv
response data is unregistered and then registered again immediately.
In the case of a short period with a single advertisement, it is
possible that the adv related data is still in the controller and
there is no need to update the controller with the new data. Hence,
there are no HCI commands built in the request. However, it was
originally treated as an error.

A simple work around again is to insert a whatever HCI command in the
request command queue to prevent the queue from being empty.

There is supposed to be a better way to handle this situation.

BUG= chromium:760792 
TEST=None
CQ-DEPEND=CL:754226

Change-Id: I17196f3b0ca0684e556307905a651ba48da11c31
Signed-off-by: Joseph Hwang <josephsih@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/694949
Commit-Ready: Shyh-In Hwang <josephsih@chromium.org>
Tested-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Shyh-In Hwang <josephsih@chromium.org>
Reviewed-by: Dmitry Grinberg <dmitrygr@google.com>
(cherry picked from commit 0adc9f1acae8463b08d3406a7d84b1e0b31cd7a1)
Reviewed-on: https://chromium-review.googlesource.com/754227
Commit-Ready: Dmitry Grinberg <dmitrygr@google.com>
Tested-by: Dmitry Grinberg <dmitrygr@google.com>

[modify] https://crrev.com/a6c01afc10ce567671d5b238ac6400284ea6869d/net/bluetooth/mgmt.c

Comment 44 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 45 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)
Status: Verified (was: Fixed)

Sign in to add a comment