| Message ID | 20240711192501.3699613-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9cc587947b6ac56a4c94dcc880b273bc72af22a8 |
| Headers | show |
| Series | [BlueZ,v1] device: Fix overwritting current_flags | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | warning | WARNING:TYPO_SPELLING: 'overwritting' may be misspelled - perhaps 'overwriting'? #78: Subject: [PATCH BlueZ v1] device: Fix overwritting current_flags ^^^^^^^^^^^^ /github/workspace/src/src/13731021.patch total: 0 errors, 1 warnings, 108 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13731021.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=870593
---Test result---
Test Summary:
CheckPatch FAIL 0.68 seconds
GitLint PASS 0.29 seconds
BuildEll PASS 25.33 seconds
BluezMake PASS 1684.87 seconds
MakeCheck PASS 15.57 seconds
MakeDistcheck PASS 181.07 seconds
CheckValgrind PASS 256.48 seconds
CheckSmatch PENDING 457.59 seconds
bluezmakeextell PASS 120.47 seconds
IncrementalBuild PENDING 60.39 seconds
ScanBuild PASS 1060.66 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v1] device: Fix overwritting current_flags
WARNING:TYPO_SPELLING: 'overwritting' may be misspelled - perhaps 'overwriting'?
#78:
Subject: [PATCH BlueZ v1] device: Fix overwritting current_flags
^^^^^^^^^^^^
/github/workspace/src/src/13731021.patch total: 0 errors, 1 warnings, 108 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13731021.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: CheckSmatch - PENDING
Desc: Run smatch tool with source
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
Hello: This patch was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Thu, 11 Jul 2024 15:25:01 -0400 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > MGMT Set Device Flags overwrites the current_flags so only the last > flags set this way would remain active which can be seem in the > following sequence when LL Privacy is enabled: > > @ MGMT Command: Set Device Flags (0x0050) plen 11 > LE Address: CF:AC:A6:79:3D:B9 (Static) > Current Flags: 0x00000001 > Remote Wakeup > @ MGMT Event: Command Complete (0x0001) plen 10 > Set Device Flags (0x0050) plen 7 > Status: Success (0x00) > LE Address: CF:AC:A6:79:3D:B9 (Static) > @ MGMT Command: Set Device Flags (0x0050) plen 11 > LE Address: CF:AC:A6:79:3D:B9 (Static) > Current Flags: 0x00000002 > Device Privacy Mode > @ MGMT Event: Command Complete (0x0001) plen 10 > Set Device Flags (0x0050) plen 7 > Status: Success (0x00) > LE Address: CF:AC:A6:79:3D:B9 (Static) > > [...] Here is the summary with links: - [BlueZ,v1] device: Fix overwritting current_flags https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9cc587947b6a You are awesome, thank you!
diff --git a/src/adapter.c b/src/adapter.c index bb49a1ecad23..85ddfc16568f 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -5569,6 +5569,7 @@ void adapter_accept_list_remove(struct btd_adapter *adapter, static void set_device_privacy_complete(uint8_t status, uint16_t length, const void *param, void *user_data) { + struct btd_device *dev = user_data; const struct mgmt_rp_set_device_flags *rp = param; if (status != MGMT_STATUS_SUCCESS) { @@ -5581,6 +5582,9 @@ static void set_device_privacy_complete(uint8_t status, uint16_t length, error("Too small Set Device Flags complete event: %d", length); return; } + + btd_device_flags_changed(dev, btd_device_get_supported_flags(dev), + btd_device_get_pending_flags(dev)); } static void add_device_complete(uint8_t status, uint16_t length, @@ -5626,7 +5630,7 @@ static void add_device_complete(uint8_t status, uint16_t length, adapter_set_device_flags(adapter, dev, flags | DEVICE_FLAG_DEVICE_PRIVACY, set_device_privacy_complete, - NULL); + dev); } } } @@ -5676,6 +5680,7 @@ void adapter_set_device_flags(struct btd_adapter *adapter, { struct mgmt_cp_set_device_flags cp; uint32_t supported = btd_device_get_supported_flags(device); + uint32_t pending = btd_device_get_pending_flags(device); const bdaddr_t *bdaddr; uint8_t bdaddr_type; @@ -5683,6 +5688,14 @@ void adapter_set_device_flags(struct btd_adapter *adapter, (supported | flags) != supported) return; + /* Check if changing flags are pending */ + if (flags == (flags & pending)) + return; + + /* Set Device Privacy Mode if it has not set the flag yet. */ + if (btd_opts.device_privacy && !(flags & DEVICE_FLAG_DEVICE_PRIVACY)) + flags |= DEVICE_FLAG_DEVICE_PRIVACY & supported & ~pending; + bdaddr = device_get_address(device); bdaddr_type = btd_device_get_bdaddr_type(device); @@ -5691,8 +5704,9 @@ void adapter_set_device_flags(struct btd_adapter *adapter, cp.addr.type = bdaddr_type; cp.current_flags = cpu_to_le32(flags); - mgmt_send(adapter->mgmt, MGMT_OP_SET_DEVICE_FLAGS, adapter->dev_id, - sizeof(cp), &cp, func, user_data, NULL); + if (mgmt_send(adapter->mgmt, MGMT_OP_SET_DEVICE_FLAGS, adapter->dev_id, + sizeof(cp), &cp, func, user_data, NULL)) + btd_device_set_pending_flags(device, flags); } static void device_flags_changed_callback(uint16_t index, uint16_t length, diff --git a/src/device.c b/src/device.c index 097b1fbba37d..a1dc0750ca41 100644 --- a/src/device.c +++ b/src/device.c @@ -214,6 +214,7 @@ struct btd_device { GDBusPendingPropertySet wake_id; uint32_t supported_flags; + uint32_t pending_flags; uint32_t current_flags; GSList *svc_callbacks; GSList *eir_uuids; @@ -1569,7 +1570,7 @@ static void set_wake_allowed_complete(uint8_t status, uint16_t length, return; } - device_set_wake_allowed_complete(dev); + btd_device_flags_changed(dev, dev->supported_flags, dev->pending_flags); } void device_set_wake_allowed(struct btd_device *device, bool wake_allowed, @@ -7243,6 +7244,22 @@ uint32_t btd_device_get_supported_flags(struct btd_device *dev) return dev->supported_flags; } +void btd_device_set_pending_flags(struct btd_device *dev, uint32_t flags) +{ + if (!dev) + return; + + dev->pending_flags = flags; +} + +uint32_t btd_device_get_pending_flags(struct btd_device *dev) +{ + if (!dev) + return 0; + + return dev->pending_flags; +} + /* This event is sent immediately after add device on all mgmt sockets. * Afterwards, it is only sent to mgmt sockets other than the one which called * set_device_flags. @@ -7255,6 +7272,7 @@ void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags, dev->supported_flags = supported_flags; dev->current_flags = current_flags; + dev->pending_flags = 0; if (!changed_flags) return; diff --git a/src/device.h b/src/device.h index 0794f92d0178..3742f6028040 100644 --- a/src/device.h +++ b/src/device.h @@ -191,6 +191,8 @@ int btd_device_connect_services(struct btd_device *dev, GSList *services); uint32_t btd_device_get_current_flags(struct btd_device *dev); uint32_t btd_device_get_supported_flags(struct btd_device *dev); +uint32_t btd_device_get_pending_flags(struct btd_device *dev); +void btd_device_set_pending_flags(struct btd_device *dev, uint32_t flags); void btd_device_flags_changed(struct btd_device *dev, uint32_t supported_flags, uint32_t current_flags);
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> MGMT Set Device Flags overwrites the current_flags so only the last flags set this way would remain active which can be seem in the following sequence when LL Privacy is enabled: @ MGMT Command: Set Device Flags (0x0050) plen 11 LE Address: CF:AC:A6:79:3D:B9 (Static) Current Flags: 0x00000001 Remote Wakeup @ MGMT Event: Command Complete (0x0001) plen 10 Set Device Flags (0x0050) plen 7 Status: Success (0x00) LE Address: CF:AC:A6:79:3D:B9 (Static) @ MGMT Command: Set Device Flags (0x0050) plen 11 LE Address: CF:AC:A6:79:3D:B9 (Static) Current Flags: 0x00000002 Device Privacy Mode @ MGMT Event: Command Complete (0x0001) plen 10 Set Device Flags (0x0050) plen 7 Status: Success (0x00) LE Address: CF:AC:A6:79:3D:B9 (Static) In order to do this properly the code needs to track the pending_flags being set and also call btd_device_flags_changed whenever a change is complete since that event is not generated when MGMT_OP_SET_DEVICE_FLAGS is sent by bluetoothd itself. --- src/adapter.c | 20 +++++++++++++++++--- src/device.c | 20 +++++++++++++++++++- src/device.h | 2 ++ 3 files changed, 38 insertions(+), 4 deletions(-)