diff mbox series

gatt-client: when disconnected return default MTU for GattCharacteristic1.MTU

Message ID 20211127222132.14351-1-dev.git@javispedro.com (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series gatt-client: when disconnected return default MTU for GattCharacteristic1.MTU | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/setupell success Setup ELL PASS
tedd_an/buildprep success Build Prep PASS
tedd_an/build success Build Configuration PASS
tedd_an/makecheck success Make Check PASS
tedd_an/makedistcheck success Make Distcheck PASS
tedd_an/build_extell success Build External ELL PASS
tedd_an/build_extell_make success Build Make with External ELL PASS

Commit Message

Javier de San Pedro Nov. 27, 2021, 10:21 p.m. UTC 
From: "Javier de San Pedro" <dev.git@javispedro.com>

After the MTU dbus property patches in 5.62 we are seeing bluetoothd
terminate frequently with "Disconnected from D-Bus. Exiting." msgs.
Apparently this is because bluetoothd sent an invalid reply to a D-Bus
Property Get (for GattCharacteristic1's MTU).
Multiple issues in bluez Github.com project reported similar behavior;
at least #235 (see Fixes:), #219, and likely #238.

When the Characteristic1 object is still cached/alive, but the
underlying att connection is not (e.g. someone just called Disconnect),
the property getter (characteristic_get_mtu) right now returns false.
However, gdbus seems to ignore the return value and sends the empty reply
message anyway (rather than a dbus error?), and this seems to cause
the dbus connection to be terminated (due to the ill-formed reply?).
bluetoothd then aborts.

This patch makes the property value BT_ATT_DEFAULT_LE_MTU if the
underlying att object does not exist, rather than returning an invalid
message. This is consistent with the existing PropertyChanged signal
behavior (we will emit a PropertyChange only if a larger MTU is
exchanged), and fixes the issue on my machines.
An alternative could be to change gdbus behavior, but I'm not sure if we
are allowed to return an error here anyway without causing problems in
other dbus libraries/wrappers.

Signed-off-by: Javier de San Pedro <dev.git@javispedro.com>
Fixes: aaa0c4996ae9 ("gatt: Add implementation of GattCharacteristic1.MTU")
Fixes: https://github.com/bluez/bluez/issues/235
---
 src/gatt-client.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

bluez.test.bot@gmail.com Nov. 27, 2021, 11:50 p.m. UTC | #1 
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=586773

---Test result---

Test Summary:
CheckPatch                    PASS      1.34 seconds
GitLint                       PASS      0.93 seconds
Prep - Setup ELL              PASS      51.78 seconds
Build - Prep                  PASS      0.51 seconds
Build - Configure             PASS      9.44 seconds
Build - Make                  PASS      222.52 seconds
Make Check                    PASS      9.90 seconds
Make Distcheck                PASS      263.17 seconds
Build w/ext ELL - Configure   PASS      9.56 seconds
Build w/ext ELL - Make        PASS      209.87 seconds



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Nov. 29, 2021, 9:17 p.m. UTC | #2 
Hi Javier,

On Mon, Nov 29, 2021 at 1:14 PM <bluez.test.bot@gmail.com> wrote:
>
> 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=586773
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      1.34 seconds
> GitLint                       PASS      0.93 seconds
> Prep - Setup ELL              PASS      51.78 seconds
> Build - Prep                  PASS      0.51 seconds
> Build - Configure             PASS      9.44 seconds
> Build - Make                  PASS      222.52 seconds
> Make Check                    PASS      9.90 seconds
> Make Distcheck                PASS      263.17 seconds
> Build w/ext ELL - Configure   PASS      9.56 seconds
> Build w/ext ELL - Make        PASS      209.87 seconds
>
>
>
> ---
> Regards,
> Linux Bluetooth

Applied, thanks.
diff mbox series

Patch

diff --git a/src/gatt-client.c b/src/gatt-client.c
index de18bea9708e..6ee984db9410 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -883,10 +883,7 @@  static gboolean characteristic_get_mtu(const GDBusPropertyTable *property,
 	uint16_t mtu;
 
 	att = bt_gatt_client_get_att(gatt);
-	if (!att)
-		return FALSE;
-
-	mtu = bt_att_get_mtu(att);
+	mtu = att ? bt_att_get_mtu(att) : BT_ATT_DEFAULT_LE_MTU;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &mtu);