It can be useful for programs using GATT to be aware of the maximum
transmittable attribute size, as negotiated by bluetoothd. This change
exposes the negotiated size over D-Bus, if such a negotiation has
occurred.
---
attrib/gattrib.c | 6 +++++-
doc/device-api.txt | 11 +++++++++++
src/device.c | 41 +++++++++++++++++++++++++++++++++++++++++
src/shared/att.c | 15 +++++++++++++++
src/shared/gatt-server.c | 1 +
src/shared/gatt-server.h | 3 +++
6 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 2e1e39a..f080b6b 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -468,7 +468,11 @@ gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
=20
attrib->buflen =3D mtu;
=20
- return bt_att_set_mtu(attrib->att, mtu);
+ if (bt_att_set_mtu(attrib->att, mtu)) {
+ bt_att_set_mtu_negotiated(attrib->att, true);
+ return true;
+ } else
+ return false;
}
=20
gboolean g_attrib_unregister(GAttrib *attrib, guint id)
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 13b2881..fc04011 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -234,3 +234,14 @@ Properties string Address [readonly]
array{byte} AdvertisingFlags [readonly, experimental]
=20
The Advertising Data Flags of the remote device.
+
+ int16 NegotiatedMaxAttributeSize [readonly,
+ optional,
+ experimental]
+
+ The maximum supported byte size of attributes, =
as
+ negotiated between this host and the remote =
device.
+ This attribute is only available once the =
negotiation
+ is complete. If the attribute is not available,
+ clients should assume the default maximum size =
for
+ the underlying technology, e.g. 20 bytes for =
BTLE.
diff --git a/src/device.c b/src/device.c
index 8693eb8..cd3983b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -92,6 +92,8 @@
#define GATT_INCLUDE_UUID_STR "2802"
#define GATT_CHARAC_UUID_STR "2803"
=20
+#define ATT_PACKET_HEADER_SIZE 3
+
static DBusConnection *dbus_conn =3D NULL;
static unsigned service_state_cb_id;
=20
@@ -930,6 +932,41 @@ static gboolean dev_property_exists_tx_power(const =
GDBusPropertyTable *property,
return TRUE;
}
=20
+static gboolean dev_property_get_negotiated_max_att_payload(
+ const GDBusPropertyTable * =
property,
+ DBusMessageIter *iter, void =
*data)
+{
+ const struct btd_device *dev =3D data;
+ dbus_int16_t val =3D 0;
+ int err;
+
+ /* The spec for this API requires a failure if
+ * MTU negotiation is incomplete.
+ */
+ if (!bt_att_is_mtu_negotiated(dev->att))
+ return FALSE;
+
+ val =3D bt_att_get_mtu(dev->att);
+ // subtract off the ATT header length to get the max payload
+ val -=3D ATT_PACKET_HEADER_SIZE;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16, &val);
+
+ return TRUE;
+}
+
+static gboolean dev_property_exists_negotiated_max_att_payload(
+ const GDBusPropertyTable * =
property,
+ void *data)
+{
+ const struct btd_device *dev =3D data;
+
+ /* The spec for this API requires the property
+ * be absent until MTU negotiation is incomplete.
+ */
+ return bt_att_is_mtu_negotiated(dev->att);
+}
+
static gboolean
dev_property_get_svc_resolved(const GDBusPropertyTable *property,
DBusMessageIter *iter, void =
*data)
@@ -2561,6 +2598,10 @@ static const GDBusPropertyTable =
device_properties[] =3D {
NULL, dev_property_service_data_exist },
{ "TxPower", "n", dev_property_get_tx_power, NULL,
dev_property_exists_tx_power },
+ { "NegotiatedMaxAttributeSize", "n",
+ =
dev_property_get_negotiated_max_att_payload,
+ NULL, =
dev_property_exists_negotiated_max_att_payload,
+ =
G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
{ "ServicesResolved", "b", dev_property_get_svc_resolved, NULL, =
NULL },
{ "AdvertisingFlags", "ay", dev_property_get_flags, NULL,
dev_property_flags_exist,
diff --git a/src/shared/att.c b/src/shared/att.c
index ca2d051..bf20a01 100644
--- a/src/shared/att.c
+++ b/src/shared/att.c
@@ -70,6 +70,7 @@ struct bt_att {
=20
uint8_t *buf;
uint16_t mtu;
+ bool mtu_negotiated;
=20
unsigned int next_send_id; /* IDs for "send" ops */
unsigned int next_reg_id; /* IDs for registered callbacks =
*/
@@ -1100,6 +1101,20 @@ bool bt_att_set_debug(struct bt_att *att, =
bt_att_debug_func_t callback,
return true;
}
=20
+bool bt_att_is_mtu_negotiated(struct bt_att *att)
+{
+ if (!att)
+ return false;
+
+ return att->mtu_negotiated;
+}
+
+void bt_att_set_mtu_negotiated(struct bt_att *att, bool negotiated)
+{
+ if (att && !att->mtu_negotiated)
+ att->mtu_negotiated =3D negotiated;
+}
+
uint16_t bt_att_get_mtu(struct bt_att *att)
{
if (!att)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 79e01c8..c76ca23 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1379,6 +1379,7 @@ static void exchange_mtu_cb(uint8_t opcode, const =
void *pdu,
/* Set MTU to be the minimum */
server->mtu =3D final_mtu;
bt_att_set_mtu(server->att, final_mtu);
+ bt_att_set_mtu_negotiated(server->att, true);
=20
util_debug(server->debug_callback, server->debug_data,
"MTU exchange complete, with MTU: %u", =
final_mtu);
diff --git a/src/shared/gatt-server.h b/src/shared/gatt-server.h
index 0e480e1..0b99da5 100644
--- a/src/shared/gatt-server.h
+++ b/src/shared/gatt-server.h
@@ -31,6 +31,9 @@ struct bt_gatt_server *bt_gatt_server_new(struct =
gatt_db *db,
struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server =
*server);
void bt_gatt_server_unref(struct bt_gatt_server *server);
=20
+int bt_gatt_server_get_negotiated_mtu(const struct bt_gatt_server =
*server,
+ int16_t *mtu);
+
typedef void (*bt_gatt_server_destroy_func_t)(void *user_data);
typedef void (*bt_gatt_server_debug_func_t)(const char *str, void =
*user_data);
typedef void (*bt_gatt_server_conf_func_t)(void *user_data);
--=20
2.10.0
> On Sep 7, 2017, at 7:18 AM, Luiz Augusto von Dentz =
<[email protected]> wrote:
>=20
>>>=20
>>> So lets fix that and on the plus side you might get even better
>>> transfer speeds since we can bypass D-Bus entirely.
Hi Luiz,
You are totally right, but we're talking past each other here.
I have to conform to an existing app-level protocol, which transfers via =
GATT attributes on LE. As Emil noted, there are *many* similar =
implementations out there. Changing the protocol to solve the problem =
just isn't an option.
I'm not debating the merits of CoC. It seems great! I wish I could use =
it. I can't. All I'm asking is to implement a simple property to get =
the BTLE ATT MTU, so I can talk with my devices.
Jonah
Hi Emil,
On Thu, Sep 7, 2017 at 1:47 PM, Emil Lenngren <[email protected]> wro=
te:
> Hi
>
> 2017-09-07 12:22 GMT+02:00 Luiz Augusto von Dentz <[email protected]>:
>> Hi Jonah,
>>
>> On Thu, Sep 7, 2017 at 3:09 AM, Jonah Petri <[email protected]> wrote:
>>> Hi Luiz,
>>>
>>>> On Sep 6, 2017, at 3:57 PM, Luiz Augusto von Dentz <[email protected]=
om> wrote:
>>>>
>>>>>>
>>>>>> Also, the AcquireWrite and AcquireNotify should fulfil most of the u=
se
>>>>>> cases where GATT is used just as transport to another protocol, is
>>>>>> that what you are intending to do?
>>>>>
>>>>> I'm merely conforming to an existing protocol, which requires me to o=
nly publish GATT attributes which are length <=3D MTU - 3. This is because=
we can't depend on Long Read and Long Write support from BTLE clients. In=
cases where a larger MTU has been negotiated, I need to take advantage of =
it, for optimal throughput.
>>>>
>>>> Im afraid I don't follow what you are saying about publishing GATT
>>>> attributes, the spec doesn't require Long procedures by clients but
>>>> that doesn't mean you should omit attributes where their values are
>>>> bigger than the MTU or anything like that. Im also not sure why
>>>> throughput would be relevant, GATT/ATT is not throughput oriented, it
>>>> is a request/response protocol with 30 seconds timeout and on top of
>>>> that we have yet another request/response IPC.
>>>
>>> The protocol I=E2=80=99m implementing is a fairly typical =E2=80=98uart=
-on-btle=E2=80=99 protocol, of the sort supported by many BT vendors, espec=
ially for embedded MCUs. Reads of GATT attributes above the MTU - 3 thresh=
old are truncated in the client=E2=80=99s bt stack, but the clients typical=
ly don=E2=80=99t realize it, so they get corrupted data. However, for clie=
nts which support a larger MTU, the transfer speeds can be much faster, as =
the MTU is larger. (yes, we=E2=80=99re still talking about a few kb/sec, b=
ut 4x faster is 4x faster!)
>>
>> I will have to repeat this over and over, there are better solutions
>> to this than GATT, what we are seeing here is perhaps the lack of
>> support in Android. After these many years of L2CAP CoC being around
>> it really beats me why no vendor took the task to just implement it
>> and contributed to Android instead of working around it via GATT/ATT
>> and then blaming the peripheral stack for the lack of serial over
>> GATT/ATT. Not only this would bring proper serial support but things
>> like IPSP, Object Transfer, and other profiles that do use L2CAP CoC.
>>
>>> Of course, you=E2=80=99re right that BTLE isn=E2=80=99t exactly the rig=
ht way to do this. You don=E2=80=99t need to convince me of this. But I d=
o have a need for getting the MTU, to implement the protocol without corrup=
ting the data. Most people implementing the server half of these protocols=
like this will have the same need.
>>>
>>> AcquireWrite and AcquireRead are not useful to me, as I=E2=80=99m imple=
menting a gatt server, and those interfaces are marked as =E2=80=98Client O=
nly=E2=80=99.
>>
>> So lets fix that and on the plus side you might get even better
>> transfer speeds since we can bypass D-Bus entirely.
>
>
> I fully agree that the GATT specification kind of sucks for
> throughput-applications since it is, as you say, only meant to be used
> for small data transfers. But we have to remember that there are A LOT
> of devices and protocols out there that use GATT to stream data
> instead of L2CAP CoC since that was previously the only way to send
> data, so this still have to be dealt with. And if one uses Write
> Without Response and Notifications, GATT is only 1 extra byte overhead
> compared to L2CAP CoC per packet anyway, so it's not _that_ bad if
> used optimally.
1 extra byte if you don't do any segmentation/reassembly which is
normally the first thing to add follow by some sort of flow control.
Also something that most people forget is that L2CAP is connection
oriented, we can create dedicated socket per application, something
not possible in GATT/ATT which is why locking was necessary for
AcquireWrite/AcquireNotify otherwise another application can start
sending/receiving which may corrupt the data.
--=20
Luiz Augusto von Dentz
Hi
2017-09-07 12:22 GMT+02:00 Luiz Augusto von Dentz <[email protected]>:
> Hi Jonah,
>
> On Thu, Sep 7, 2017 at 3:09 AM, Jonah Petri <[email protected]> wrote:
>> Hi Luiz,
>>
>>> On Sep 6, 2017, at 3:57 PM, Luiz Augusto von Dentz <[email protected]=
m> wrote:
>>>
>>>>>
>>>>> Also, the AcquireWrite and AcquireNotify should fulfil most of the us=
e
>>>>> cases where GATT is used just as transport to another protocol, is
>>>>> that what you are intending to do?
>>>>
>>>> I'm merely conforming to an existing protocol, which requires me to on=
ly publish GATT attributes which are length <=3D MTU - 3. This is because =
we can't depend on Long Read and Long Write support from BTLE clients. In =
cases where a larger MTU has been negotiated, I need to take advantage of i=
t, for optimal throughput.
>>>
>>> Im afraid I don't follow what you are saying about publishing GATT
>>> attributes, the spec doesn't require Long procedures by clients but
>>> that doesn't mean you should omit attributes where their values are
>>> bigger than the MTU or anything like that. Im also not sure why
>>> throughput would be relevant, GATT/ATT is not throughput oriented, it
>>> is a request/response protocol with 30 seconds timeout and on top of
>>> that we have yet another request/response IPC.
>>
>> The protocol I=E2=80=99m implementing is a fairly typical =E2=80=98uart-=
on-btle=E2=80=99 protocol, of the sort supported by many BT vendors, especi=
ally for embedded MCUs. Reads of GATT attributes above the MTU - 3 thresho=
ld are truncated in the client=E2=80=99s bt stack, but the clients typicall=
y don=E2=80=99t realize it, so they get corrupted data. However, for clien=
ts which support a larger MTU, the transfer speeds can be much faster, as t=
he MTU is larger. (yes, we=E2=80=99re still talking about a few kb/sec, bu=
t 4x faster is 4x faster!)
>
> I will have to repeat this over and over, there are better solutions
> to this than GATT, what we are seeing here is perhaps the lack of
> support in Android. After these many years of L2CAP CoC being around
> it really beats me why no vendor took the task to just implement it
> and contributed to Android instead of working around it via GATT/ATT
> and then blaming the peripheral stack for the lack of serial over
> GATT/ATT. Not only this would bring proper serial support but things
> like IPSP, Object Transfer, and other profiles that do use L2CAP CoC.
>
>> Of course, you=E2=80=99re right that BTLE isn=E2=80=99t exactly the righ=
t way to do this. You don=E2=80=99t need to convince me of this. But I do=
have a need for getting the MTU, to implement the protocol without corrupt=
ing the data. Most people implementing the server half of these protocols =
like this will have the same need.
>>
>> AcquireWrite and AcquireRead are not useful to me, as I=E2=80=99m implem=
enting a gatt server, and those interfaces are marked as =E2=80=98Client On=
ly=E2=80=99.
>
> So lets fix that and on the plus side you might get even better
> transfer speeds since we can bypass D-Bus entirely.
I fully agree that the GATT specification kind of sucks for
throughput-applications since it is, as you say, only meant to be used
for small data transfers. But we have to remember that there are A LOT
of devices and protocols out there that use GATT to stream data
instead of L2CAP CoC since that was previously the only way to send
data, so this still have to be dealt with. And if one uses Write
Without Response and Notifications, GATT is only 1 extra byte overhead
compared to L2CAP CoC per packet anyway, so it's not _that_ bad if
used optimally.
>
> --
> Luiz Augusto von Dentz
Hi Jonah,
On Thu, Sep 7, 2017 at 3:09 AM, Jonah Petri <[email protected]> wrote:
> Hi Luiz,
>
>> On Sep 6, 2017, at 3:57 PM, Luiz Augusto von Dentz <[email protected]=
> wrote:
>>
>>>>
>>>> Also, the AcquireWrite and AcquireNotify should fulfil most of the use
>>>> cases where GATT is used just as transport to another protocol, is
>>>> that what you are intending to do?
>>>
>>> I'm merely conforming to an existing protocol, which requires me to onl=
y publish GATT attributes which are length <=3D MTU - 3. This is because w=
e can't depend on Long Read and Long Write support from BTLE clients. In c=
ases where a larger MTU has been negotiated, I need to take advantage of it=
, for optimal throughput.
>>
>> Im afraid I don't follow what you are saying about publishing GATT
>> attributes, the spec doesn't require Long procedures by clients but
>> that doesn't mean you should omit attributes where their values are
>> bigger than the MTU or anything like that. Im also not sure why
>> throughput would be relevant, GATT/ATT is not throughput oriented, it
>> is a request/response protocol with 30 seconds timeout and on top of
>> that we have yet another request/response IPC.
>
> The protocol I=E2=80=99m implementing is a fairly typical =E2=80=98uart-o=
n-btle=E2=80=99 protocol, of the sort supported by many BT vendors, especia=
lly for embedded MCUs. Reads of GATT attributes above the MTU - 3 threshol=
d are truncated in the client=E2=80=99s bt stack, but the clients typically=
don=E2=80=99t realize it, so they get corrupted data. However, for client=
s which support a larger MTU, the transfer speeds can be much faster, as th=
e MTU is larger. (yes, we=E2=80=99re still talking about a few kb/sec, but=
4x faster is 4x faster!)
I will have to repeat this over and over, there are better solutions
to this than GATT, what we are seeing here is perhaps the lack of
support in Android. After these many years of L2CAP CoC being around
it really beats me why no vendor took the task to just implement it
and contributed to Android instead of working around it via GATT/ATT
and then blaming the peripheral stack for the lack of serial over
GATT/ATT. Not only this would bring proper serial support but things
like IPSP, Object Transfer, and other profiles that do use L2CAP CoC.
> Of course, you=E2=80=99re right that BTLE isn=E2=80=99t exactly the right=
way to do this. You don=E2=80=99t need to convince me of this. But I do =
have a need for getting the MTU, to implement the protocol without corrupti=
ng the data. Most people implementing the server half of these protocols l=
ike this will have the same need.
>
> AcquireWrite and AcquireRead are not useful to me, as I=E2=80=99m impleme=
nting a gatt server, and those interfaces are marked as =E2=80=98Client Onl=
y=E2=80=99.
So lets fix that and on the plus side you might get even better
transfer speeds since we can bypass D-Bus entirely.
--=20
Luiz Augusto von Dentz
Hi Luiz,
> On Sep 6, 2017, at 3:57 PM, Luiz Augusto von Dentz =
<[email protected]> wrote:
>=20
>>>=20
>>> Also, the AcquireWrite and AcquireNotify should fulfil most of the =
use
>>> cases where GATT is used just as transport to another protocol, is
>>> that what you are intending to do?
>>=20
>> I'm merely conforming to an existing protocol, which requires me to =
only publish GATT attributes which are length <=3D MTU - 3. This is =
because we can't depend on Long Read and Long Write support from BTLE =
clients. In cases where a larger MTU has been negotiated, I need to =
take advantage of it, for optimal throughput.
>=20
> Im afraid I don't follow what you are saying about publishing GATT
> attributes, the spec doesn't require Long procedures by clients but
> that doesn't mean you should omit attributes where their values are
> bigger than the MTU or anything like that. Im also not sure why
> throughput would be relevant, GATT/ATT is not throughput oriented, it
> is a request/response protocol with 30 seconds timeout and on top of
> that we have yet another request/response IPC.
The protocol I=E2=80=99m implementing is a fairly typical =
=E2=80=98uart-on-btle=E2=80=99 protocol, of the sort supported by many =
BT vendors, especially for embedded MCUs. Reads of GATT attributes =
above the MTU - 3 threshold are truncated in the client=E2=80=99s bt =
stack, but the clients typically don=E2=80=99t realize it, so they get =
corrupted data. However, for clients which support a larger MTU, the =
transfer speeds can be much faster, as the MTU is larger. (yes, we=E2=80=99=
re still talking about a few kb/sec, but 4x faster is 4x faster!) =20
Of course, you=E2=80=99re right that BTLE isn=E2=80=99t exactly the =
right way to do this. You don=E2=80=99t need to convince me of this. =
But I do have a need for getting the MTU, to implement the protocol =
without corrupting the data. Most people implementing the server half =
of these protocols like this will have the same need.
AcquireWrite and AcquireRead are not useful to me, as I=E2=80=99m =
implementing a gatt server, and those interfaces are marked as =E2=80=98Cl=
ient Only=E2=80=99.
Regards,
Jonah
Hi Jonah,
On Wed, Sep 6, 2017 at 6:44 PM, Jonah Petri <[email protected]> wrote:
> Hello Luiz,
>
> Thanks for looking at my proposal.
>
>> On Sep 6, 2017, at 10:14 AM, Luiz Augusto von Dentz <[email protected]=
m> wrote:
>>
>> Your implementation is not really LE specific, in fact it will cause
>> the clients to believe they connected over LE if the MTU appears,
>> besides with cross transport pairing and dual mode topology this has
>> to work either way or then we have to disable GATT completely on
>> BR/EDR.
>
> Of course, yes, my previous implementation is not appropriate for this ne=
w API. There's differences of opinion about what the API should be, and I'=
m proposing a revised API to address feedback from yourself and others. If=
there's consensus that my revised API is appropriate, I'll then propose an=
implementation.
>
>>
>> Also, the AcquireWrite and AcquireNotify should fulfil most of the use
>> cases where GATT is used just as transport to another protocol, is
>> that what you are intending to do?
>
> I'm merely conforming to an existing protocol, which requires me to only =
publish GATT attributes which are length <=3D MTU - 3. This is because we =
can't depend on Long Read and Long Write support from BTLE clients. In cas=
es where a larger MTU has been negotiated, I need to take advantage of it, =
for optimal throughput.
Im afraid I don't follow what you are saying about publishing GATT
attributes, the spec doesn't require Long procedures by clients but
that doesn't mean you should omit attributes where their values are
bigger than the MTU or anything like that. Im also not sure why
throughput would be relevant, GATT/ATT is not throughput oriented, it
is a request/response protocol with 30 seconds timeout and on top of
that we have yet another request/response IPC.
>
> I didn't design the protocol, but it's out there in the wild, and I need =
to support it.
>
>>
>> Having an API just for a very specific use case that in the future may
>> actually disappear with OSes adopting L2CAP CoC is not very nice.
>
> We already have several LE-specific APIs (e.g. LEAdvertisingManager), so =
I don't understand this objection. BTLE GATT will be with us essentially f=
orever. Also, I've proposed marking this dbus property as experimental, so=
I don't see the problem with trying it out. It addresses a real need.
>
> Best,
>
> Jonah
--=20
Luiz Augusto von Dentz
Hello Luiz,
Thanks for looking at my proposal.
> On Sep 6, 2017, at 10:14 AM, Luiz Augusto von Dentz =
<[email protected]> wrote:
>=20
> Your implementation is not really LE specific, in fact it will cause
> the clients to believe they connected over LE if the MTU appears,
> besides with cross transport pairing and dual mode topology this has
> to work either way or then we have to disable GATT completely on
> BR/EDR.
Of course, yes, my previous implementation is not appropriate for this =
new API. There's differences of opinion about what the API should be, =
and I'm proposing a revised API to address feedback from yourself and =
others. If there's consensus that my revised API is appropriate, I'll =
then propose an implementation.
>=20
> Also, the AcquireWrite and AcquireNotify should fulfil most of the use
> cases where GATT is used just as transport to another protocol, is
> that what you are intending to do?
I'm merely conforming to an existing protocol, which requires me to only =
publish GATT attributes which are length <=3D MTU - 3. This is because =
we can't depend on Long Read and Long Write support from BTLE clients. =
In cases where a larger MTU has been negotiated, I need to take =
advantage of it, for optimal throughput.
I didn't design the protocol, but it's out there in the wild, and I need =
to support it.
>=20
> Having an API just for a very specific use case that in the future may
> actually disappear with OSes adopting L2CAP CoC is not very nice.
We already have several LE-specific APIs (e.g. LEAdvertisingManager), so =
I don't understand this objection. BTLE GATT will be with us =
essentially forever. Also, I've proposed marking this dbus property as =
experimental, so I don't see the problem with trying it out. It =
addresses a real need.
Best,
Jonah=
Hi Jonah,
On Wed, Sep 6, 2017 at 4:55 PM, Jonah Petri <[email protected]> wrote:
> Hi Luiz and Emil,
>
> Thanks for your comments!
>
>> On Sep 6, 2017, at 9:01 AM, Luiz Augusto von Dentz <[email protected]=
> wrote:
>>
>>> I know there were some discussions regarding if it should be called
>>> max attribute size or mtu.
>>> Maybe I'm a bit late but in my opinion I think mtu is a better option.
>>>
>>> "The maximum supported byte size of attributes, as negotiated between
>>> this host and the remote device" is a bit misleading/wrong since the
>>> BLE spec allows up to 512 byte attribute values no matter of the MTU.
>>
>> Note that the MTU can be in fact even bigger than 512 bytes since it
>> can be used by things like discover, etc, which is not bound to a
>> single attribute, though for practical reasons BlueZ will always
>> request 512 bytes since buffer size is usually not a concern in
>> systems running Linux.
>>
>>> You can still perform a Long Write where the attribute value is larger
>>> than the MTU for example.
>>> Also, the max value size for transfer is different depending on
>>> method. Max value size for a notification/indication or normal write
>>> is ATT_MTU-3 while max value size for a normal read is ATT_MTU-1.
>>
>> Yep, we can't really use the same value for read/write since the
>> actual payload changes depending on the opcode.
>>
>>> With the current patch it's not clear (by only looking at the
>>> documentation) which value "max attribute size" really means. If it's
>>> called MTU it's on the other hand very clear what it is (like it is
>>> called in Android's implementation). Even if there is a difference
>>> between BD/EDR and LE MTU negotiation, the packet format for ATT and
>>> the meaning of MTU is still the same.
>>
>> The fundamental part missing here is that over BR/EDR the MTU can be
>> asymmetric meaning we need the MTU value for input and output to be
>> separated.
>
> Hm. BR/EDR isn't even the intended use case here, and (as you point out)=
complicates the API greatly. And in general, I agree with Emil that the M=
TU is really what many clients of this API are interested in.
>
> What if I reformulate the API as LE-specific, and instead of making the A=
PI about Maximum Attribute Size, I return to simply making the MTU availabl=
e to dbus clients. That's all I really want for my use case, and I'll make=
an effort to make the documentation sufficiently educational that develope=
rs should be able to do the right thing. What about this?
Your implementation is not really LE specific, in fact it will cause
the clients to believe they connected over LE if the MTU appears,
besides with cross transport pairing and dual mode topology this has
to work either way or then we have to disable GATT completely on
BR/EDR.
Also, the AcquireWrite and AcquireNotify should fulfil most of the use
cases where GATT is used just as transport to another protocol, is
that what you are intending to do?
Having an API just for a very specific use case that in the future may
actually disappear with OSes adopting L2CAP CoC is not very nice.
> + int16 LENegotiatedATTMTU [readonly,
> + optional,
> + experimental]
> +
> + The Maximum Transmission Unit of the ATT protocol=
,
> + as negotiated with this device by bluetoothd. Th=
is
> + property is only available for devices connected =
via
> + BTLE, and only after MTU negotiation is complete.
> + If this property is not available, clients may
> + only assume a MTU of 23 bytes, as required by the
> + Bluetooth specification.
> + The maximum GATT attribute data is MTU - 3, altho=
ugh
> + some devices may support Long Read and Long Write
> + commands, allowing for larger attribute data size=
s.
> +
>
> Best,
>
> Jonah
>
>
--=20
Luiz Augusto von Dentz
Hi Luiz and Emil,
Thanks for your comments!
> On Sep 6, 2017, at 9:01 AM, Luiz Augusto von Dentz =
<[email protected]> wrote:
>=20
>> I know there were some discussions regarding if it should be called
>> max attribute size or mtu.
>> Maybe I'm a bit late but in my opinion I think mtu is a better =
option.
>>=20
>> "The maximum supported byte size of attributes, as negotiated between
>> this host and the remote device" is a bit misleading/wrong since the
>> BLE spec allows up to 512 byte attribute values no matter of the MTU.
>=20
> Note that the MTU can be in fact even bigger than 512 bytes since it
> can be used by things like discover, etc, which is not bound to a
> single attribute, though for practical reasons BlueZ will always
> request 512 bytes since buffer size is usually not a concern in
> systems running Linux.
>=20
>> You can still perform a Long Write where the attribute value is =
larger
>> than the MTU for example.
>> Also, the max value size for transfer is different depending on
>> method. Max value size for a notification/indication or normal write
>> is ATT_MTU-3 while max value size for a normal read is ATT_MTU-1.
>=20
> Yep, we can't really use the same value for read/write since the
> actual payload changes depending on the opcode.
>=20
>> With the current patch it's not clear (by only looking at the
>> documentation) which value "max attribute size" really means. If it's
>> called MTU it's on the other hand very clear what it is (like it is
>> called in Android's implementation). Even if there is a difference
>> between BD/EDR and LE MTU negotiation, the packet format for ATT and
>> the meaning of MTU is still the same.
>=20
> The fundamental part missing here is that over BR/EDR the MTU can be
> asymmetric meaning we need the MTU value for input and output to be
> separated.
Hm. BR/EDR isn't even the intended use case here, and (as you point =
out) complicates the API greatly. And in general, I agree with Emil =
that the MTU is really what many clients of this API are interested in.
What if I reformulate the API as LE-specific, and instead of making the =
API about Maximum Attribute Size, I return to simply making the MTU =
available to dbus clients. That's all I really want for my use case, =
and I'll make an effort to make the documentation sufficiently =
educational that developers should be able to do the right thing. What =
about this?
+ int16 LENegotiatedATTMTU [readonly,
+ optional,
+ experimental]
+
+ The Maximum Transmission Unit of the ATT =
protocol,
+ as negotiated with this device by bluetoothd. =
This=20
+ property is only available for devices connected =
via=20
+ BTLE, and only after MTU negotiation is =
complete.
+ If this property is not available, clients may
+ only assume a MTU of 23 bytes, as required by =
the=20
+ Bluetooth specification.
+ The maximum GATT attribute data is MTU - 3, =
although
+ some devices may support Long Read and Long =
Write
+ commands, allowing for larger attribute data =
sizes.
+
Best,
Jonah
Hi Emil,
On Wed, Sep 6, 2017 at 3:17 PM, Emil Lenngren <[email protected]> wro=
te:
> Hi,
>
> 2017-09-06 13:20 GMT+02:00 Jonah Petri <[email protected]>:
>> [Resending this patch, as I didn=E2=80=99t see any replies in the past m=
onth or so=E2=80=A6]
>>
>> It can be useful for programs using GATT to be aware of the maximum
>> transmittable attribute size, as negotiated by bluetoothd. This change
>> exposes the negotiated size over D-Bus, if such a negotiation has
>> occurred.
>> ---
>> attrib/gattrib.c | 6 +++++-
>> doc/device-api.txt | 11 +++++++++++
>> src/device.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> src/shared/att.c | 15 +++++++++++++++
>> src/shared/gatt-server.c | 1 +
>> src/shared/gatt-server.h | 3 +++
>> 6 files changed, 76 insertions(+), 1 deletion(-)
>>
>> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
>> index 2e1e39a..f080b6b 100644
>> --- a/attrib/gattrib.c
>> +++ b/attrib/gattrib.c
>> @@ -468,7 +468,11 @@ gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
>>
>> attrib->buflen =3D mtu;
>>
>> - return bt_att_set_mtu(attrib->att, mtu);
>> + if (bt_att_set_mtu(attrib->att, mtu)) {
>> + bt_att_set_mtu_negotiated(attrib->att, true);
>> + return true;
>> + } else
>> + return false;
>> }
>>
>> gboolean g_attrib_unregister(GAttrib *attrib, guint id)
>> diff --git a/doc/device-api.txt b/doc/device-api.txt
>> index 13b2881..fc04011 100644
>> --- a/doc/device-api.txt
>> +++ b/doc/device-api.txt
>> @@ -234,3 +234,14 @@ Properties string Address [readonly]
>> array{byte} AdvertisingFlags [readonly, experimental]
>>
>> The Advertising Data Flags of the remote device.
>> +
>> + int16 NegotiatedMaxAttributeSize [readonly,
>> + optional,
>> + experimental]
>> +
>> + The maximum supported byte size of attributes, a=
s
>> + negotiated between this host and the remote devi=
ce.
>> + This attribute is only available once the negoti=
ation
>> + is complete. If the attribute is not available,
>> + clients should assume the default maximum size f=
or
>> + the underlying technology, e.g. 20 bytes for BTL=
E.
>> diff --git a/src/device.c b/src/device.c
>> index 8693eb8..cd3983b 100644
>> --- a/src/device.c
>> +++ b/src/device.c
>> @@ -92,6 +92,8 @@
>> #define GATT_INCLUDE_UUID_STR "2802"
>> #define GATT_CHARAC_UUID_STR "2803"
>>
>> +#define ATT_PACKET_HEADER_SIZE 3
>> +
>> static DBusConnection *dbus_conn =3D NULL;
>> static unsigned service_state_cb_id;
>>
>> @@ -930,6 +932,41 @@ static gboolean dev_property_exists_tx_power(const =
GDBusPropertyTable *property,
>> return TRUE;
>> }
>>
>> +static gboolean dev_property_get_negotiated_max_att_payload(
>> + const GDBusPropertyTable * prope=
rty,
>> + DBusMessageIter *iter, void *dat=
a)
>> +{
>> + const struct btd_device *dev =3D data;
>> + dbus_int16_t val =3D 0;
>> + int err;
>> +
>> + /* The spec for this API requires a failure if
>> + * MTU negotiation is incomplete.
>> + */
>> + if (!bt_att_is_mtu_negotiated(dev->att))
>> + return FALSE;
>> +
>> + val =3D bt_att_get_mtu(dev->att);
>> + // subtract off the ATT header length to get the max payload
>> + val -=3D ATT_PACKET_HEADER_SIZE;
>> +
>> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16, &val);
>> +
>> + return TRUE;
>> +}
>> +
>> +static gboolean dev_property_exists_negotiated_max_att_payload(
>> + const GDBusPropertyTable * prope=
rty,
>> + void *data)
>> +{
>> + const struct btd_device *dev =3D data;
>> +
>> + /* The spec for this API requires the property
>> + * be absent until MTU negotiation is incomplete.
>> + */
>> + return bt_att_is_mtu_negotiated(dev->att);
>> +}
>> +
>> static gboolean
>> dev_property_get_svc_resolved(const GDBusPropertyTable *property,
>> DBusMessageIter *iter, void *dat=
a)
>> @@ -2561,6 +2598,10 @@ static const GDBusPropertyTable device_properties=
[] =3D {
>> NULL, dev_property_service_data_exist },
>> { "TxPower", "n", dev_property_get_tx_power, NULL,
>> dev_property_exists_tx_power },
>> + { "NegotiatedMaxAttributeSize", "n",
>> + dev_property_get_negotiated_max_=
att_payload,
>> + NULL, dev_property_exists_negoti=
ated_max_att_payload,
>> + G_DBUS_PROPERTY_FLAG_EXPERIMENTA=
L },
>> { "ServicesResolved", "b", dev_property_get_svc_resolved, NULL, =
NULL },
>> { "AdvertisingFlags", "ay", dev_property_get_flags, NULL,
>> dev_property_flags_exist,
>> diff --git a/src/shared/att.c b/src/shared/att.c
>> index ca2d051..bf20a01 100644
>> --- a/src/shared/att.c
>> +++ b/src/shared/att.c
>> @@ -70,6 +70,7 @@ struct bt_att {
>>
>> uint8_t *buf;
>> uint16_t mtu;
>> + bool mtu_negotiated;
>>
>> unsigned int next_send_id; /* IDs for "send" ops */
>> unsigned int next_reg_id; /* IDs for registered callbacks =
*/
>> @@ -1100,6 +1101,20 @@ bool bt_att_set_debug(struct bt_att *att, bt_att_=
debug_func_t callback,
>> return true;
>> }
>>
>> +bool bt_att_is_mtu_negotiated(struct bt_att *att)
>> +{
>> + if (!att)
>> + return false;
>> +
>> + return att->mtu_negotiated;
>> +}
>> +
>> +void bt_att_set_mtu_negotiated(struct bt_att *att, bool negotiated)
>> +{
>> + if (att && !att->mtu_negotiated)
>> + att->mtu_negotiated =3D negotiated;
Over BR/EDR it shall be possible to change the MTU at any given time
as it doesn't use Exchange MTU procedure its rules don't apply to
L2CAP Config.
>> +}
>> +
>> uint16_t bt_att_get_mtu(struct bt_att *att)
>> {
>> if (!att)
>> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
>> index 79e01c8..c76ca23 100644
>> --- a/src/shared/gatt-server.c
>> +++ b/src/shared/gatt-server.c
>> @@ -1379,6 +1379,7 @@ static void exchange_mtu_cb(uint8_t opcode, const =
void *pdu,
>> /* Set MTU to be the minimum */
>> server->mtu =3D final_mtu;
>> bt_att_set_mtu(server->att, final_mtu);
>> + bt_att_set_mtu_negotiated(server->att, true);
This doesn't seem to even care about BR/EDR which doesn't use this callback=
.
>> util_debug(server->debug_callback, server->debug_data,
>> "MTU exchange complete, with MTU: %u", final_mtu=
);
>> diff --git a/src/shared/gatt-server.h b/src/shared/gatt-server.h
>> index 0e480e1..0b99da5 100644
>> --- a/src/shared/gatt-server.h
>> +++ b/src/shared/gatt-server.h
>> @@ -31,6 +31,9 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_=
db *db,
>> struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server)=
;
>> void bt_gatt_server_unref(struct bt_gatt_server *server);
>>
>> +int bt_gatt_server_get_negotiated_mtu(const struct bt_gatt_server *serv=
er,
>> + int16_t *mtu);
>> +
I think we better add a callback to att.h which gets called whenever
the MTU changes.
>> typedef void (*bt_gatt_server_destroy_func_t)(void *user_data);
>> typedef void (*bt_gatt_server_debug_func_t)(const char *str, void *user_=
data);
>> typedef void (*bt_gatt_server_conf_func_t)(void *user_data);
>> --
>> 2.10.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetoot=
h" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> I know there were some discussions regarding if it should be called
> max attribute size or mtu.
> Maybe I'm a bit late but in my opinion I think mtu is a better option.
>
> "The maximum supported byte size of attributes, as negotiated between
> this host and the remote device" is a bit misleading/wrong since the
> BLE spec allows up to 512 byte attribute values no matter of the MTU.
Note that the MTU can be in fact even bigger than 512 bytes since it
can be used by things like discover, etc, which is not bound to a
single attribute, though for practical reasons BlueZ will always
request 512 bytes since buffer size is usually not a concern in
systems running Linux.
> You can still perform a Long Write where the attribute value is larger
> than the MTU for example.
> Also, the max value size for transfer is different depending on
> method. Max value size for a notification/indication or normal write
> is ATT_MTU-3 while max value size for a normal read is ATT_MTU-1.
Yep, we can't really use the same value for read/write since the
actual payload changes depending on the opcode.
> With the current patch it's not clear (by only looking at the
> documentation) which value "max attribute size" really means. If it's
> called MTU it's on the other hand very clear what it is (like it is
> called in Android's implementation). Even if there is a difference
> between BD/EDR and LE MTU negotiation, the packet format for ATT and
> the meaning of MTU is still the same.
The fundamental part missing here is that over BR/EDR the MTU can be
asymmetric meaning we need the MTU value for input and output to be
separated.
>
> /Emil
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--=20
Luiz Augusto von Dentz
Hi,
2017-09-06 13:20 GMT+02:00 Jonah Petri <[email protected]>:
> [Resending this patch, as I didn=E2=80=99t see any replies in the past mo=
nth or so=E2=80=A6]
>
> It can be useful for programs using GATT to be aware of the maximum
> transmittable attribute size, as negotiated by bluetoothd. This change
> exposes the negotiated size over D-Bus, if such a negotiation has
> occurred.
> ---
> attrib/gattrib.c | 6 +++++-
> doc/device-api.txt | 11 +++++++++++
> src/device.c | 41 +++++++++++++++++++++++++++++++++++++++++
> src/shared/att.c | 15 +++++++++++++++
> src/shared/gatt-server.c | 1 +
> src/shared/gatt-server.h | 3 +++
> 6 files changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/attrib/gattrib.c b/attrib/gattrib.c
> index 2e1e39a..f080b6b 100644
> --- a/attrib/gattrib.c
> +++ b/attrib/gattrib.c
> @@ -468,7 +468,11 @@ gboolean g_attrib_set_mtu(GAttrib *attrib, int mtu)
>
> attrib->buflen =3D mtu;
>
> - return bt_att_set_mtu(attrib->att, mtu);
> + if (bt_att_set_mtu(attrib->att, mtu)) {
> + bt_att_set_mtu_negotiated(attrib->att, true);
> + return true;
> + } else
> + return false;
> }
>
> gboolean g_attrib_unregister(GAttrib *attrib, guint id)
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 13b2881..fc04011 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -234,3 +234,14 @@ Properties string Address [readonly]
> array{byte} AdvertisingFlags [readonly, experimental]
>
> The Advertising Data Flags of the remote device.
> +
> + int16 NegotiatedMaxAttributeSize [readonly,
> + optional,
> + experimental]
> +
> + The maximum supported byte size of attributes, as
> + negotiated between this host and the remote devic=
e.
> + This attribute is only available once the negotia=
tion
> + is complete. If the attribute is not available,
> + clients should assume the default maximum size fo=
r
> + the underlying technology, e.g. 20 bytes for BTLE=
.
> diff --git a/src/device.c b/src/device.c
> index 8693eb8..cd3983b 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -92,6 +92,8 @@
> #define GATT_INCLUDE_UUID_STR "2802"
> #define GATT_CHARAC_UUID_STR "2803"
>
> +#define ATT_PACKET_HEADER_SIZE 3
> +
> static DBusConnection *dbus_conn =3D NULL;
> static unsigned service_state_cb_id;
>
> @@ -930,6 +932,41 @@ static gboolean dev_property_exists_tx_power(const G=
DBusPropertyTable *property,
> return TRUE;
> }
>
> +static gboolean dev_property_get_negotiated_max_att_payload(
> + const GDBusPropertyTable * proper=
ty,
> + DBusMessageIter *iter, void *data=
)
> +{
> + const struct btd_device *dev =3D data;
> + dbus_int16_t val =3D 0;
> + int err;
> +
> + /* The spec for this API requires a failure if
> + * MTU negotiation is incomplete.
> + */
> + if (!bt_att_is_mtu_negotiated(dev->att))
> + return FALSE;
> +
> + val =3D bt_att_get_mtu(dev->att);
> + // subtract off the ATT header length to get the max payload
> + val -=3D ATT_PACKET_HEADER_SIZE;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16, &val);
> +
> + return TRUE;
> +}
> +
> +static gboolean dev_property_exists_negotiated_max_att_payload(
> + const GDBusPropertyTable * proper=
ty,
> + void *data)
> +{
> + const struct btd_device *dev =3D data;
> +
> + /* The spec for this API requires the property
> + * be absent until MTU negotiation is incomplete.
> + */
> + return bt_att_is_mtu_negotiated(dev->att);
> +}
> +
> static gboolean
> dev_property_get_svc_resolved(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *data=
)
> @@ -2561,6 +2598,10 @@ static const GDBusPropertyTable device_properties[=
] =3D {
> NULL, dev_property_service_data_exist },
> { "TxPower", "n", dev_property_get_tx_power, NULL,
> dev_property_exists_tx_power },
> + { "NegotiatedMaxAttributeSize", "n",
> + dev_property_get_negotiated_max_a=
tt_payload,
> + NULL, dev_property_exists_negotia=
ted_max_att_payload,
> + G_DBUS_PROPERTY_FLAG_EXPERIMENTAL=
},
> { "ServicesResolved", "b", dev_property_get_svc_resolved, NULL, N=
ULL },
> { "AdvertisingFlags", "ay", dev_property_get_flags, NULL,
> dev_property_flags_exist,
> diff --git a/src/shared/att.c b/src/shared/att.c
> index ca2d051..bf20a01 100644
> --- a/src/shared/att.c
> +++ b/src/shared/att.c
> @@ -70,6 +70,7 @@ struct bt_att {
>
> uint8_t *buf;
> uint16_t mtu;
> + bool mtu_negotiated;
>
> unsigned int next_send_id; /* IDs for "send" ops */
> unsigned int next_reg_id; /* IDs for registered callbacks *=
/
> @@ -1100,6 +1101,20 @@ bool bt_att_set_debug(struct bt_att *att, bt_att_d=
ebug_func_t callback,
> return true;
> }
>
> +bool bt_att_is_mtu_negotiated(struct bt_att *att)
> +{
> + if (!att)
> + return false;
> +
> + return att->mtu_negotiated;
> +}
> +
> +void bt_att_set_mtu_negotiated(struct bt_att *att, bool negotiated)
> +{
> + if (att && !att->mtu_negotiated)
> + att->mtu_negotiated =3D negotiated;
> +}
> +
> uint16_t bt_att_get_mtu(struct bt_att *att)
> {
> if (!att)
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 79e01c8..c76ca23 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -1379,6 +1379,7 @@ static void exchange_mtu_cb(uint8_t opcode, const v=
oid *pdu,
> /* Set MTU to be the minimum */
> server->mtu =3D final_mtu;
> bt_att_set_mtu(server->att, final_mtu);
> + bt_att_set_mtu_negotiated(server->att, true);
>
> util_debug(server->debug_callback, server->debug_data,
> "MTU exchange complete, with MTU: %u", final_mtu)=
;
> diff --git a/src/shared/gatt-server.h b/src/shared/gatt-server.h
> index 0e480e1..0b99da5 100644
> --- a/src/shared/gatt-server.h
> +++ b/src/shared/gatt-server.h
> @@ -31,6 +31,9 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_d=
b *db,
> struct bt_gatt_server *bt_gatt_server_ref(struct bt_gatt_server *server);
> void bt_gatt_server_unref(struct bt_gatt_server *server);
>
> +int bt_gatt_server_get_negotiated_mtu(const struct bt_gatt_server *serve=
r,
> + int16_t *mtu);
> +
> typedef void (*bt_gatt_server_destroy_func_t)(void *user_data);
> typedef void (*bt_gatt_server_debug_func_t)(const char *str, void *user_d=
ata);
> typedef void (*bt_gatt_server_conf_func_t)(void *user_data);
> --
> 2.10.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I know there were some discussions regarding if it should be called
max attribute size or mtu.
Maybe I'm a bit late but in my opinion I think mtu is a better option.
"The maximum supported byte size of attributes, as negotiated between
this host and the remote device" is a bit misleading/wrong since the
BLE spec allows up to 512 byte attribute values no matter of the MTU.
You can still perform a Long Write where the attribute value is larger
than the MTU for example.
Also, the max value size for transfer is different depending on
method. Max value size for a notification/indication or normal write
is ATT_MTU-3 while max value size for a normal read is ATT_MTU-1.
With the current patch it's not clear (by only looking at the
documentation) which value "max attribute size" really means. If it's
called MTU it's on the other hand very clear what it is (like it is
called in Android's implementation). Even if there is a difference
between BD/EDR and LE MTU negotiation, the packet format for ATT and
the meaning of MTU is still the same.
/Emil