Hi Jonah,
On Fri, Jul 14, 2017 at 6:56 PM, Jonah Petri <jonah@xxxxxxxxx> wrote:
> It can be useful for programs using GATT to be aware of the maximum
> transmittable packet size, as negotiated by bluetoothd. This change
> exposes the negotiated MTU over D-Bus, if such a negotiation has
> occurred.
> ---
> client/main.c | 1 +
> doc/device-api.txt | 5 +++++
> src/device.c | 31 +++++++++++++++++++++++++++++++
> src/shared/gatt-server.c | 15 +++++++++++++++
> src/shared/gatt-server.h | 3 +++
> 5 files changed, 55 insertions(+)
>
> diff --git a/client/main.c b/client/main.c
> index 255cbd5..6435674 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -1372,6 +1372,7 @@ static void cmd_info(const char *arg)
> print_property(proxy, "ServiceData");
> print_property(proxy, "RSSI");
> print_property(proxy, "TxPower");
> + print_property(proxy, "NegotiatedMTU");
> }
>
> static void pair_reply(DBusMessage *message, void *user_data)
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 13b2881..908616b 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -234,3 +234,8 @@ Properties string Address [readonly]
> array{byte} AdvertisingFlags [readonly, experimental]
>
> The Advertising Data Flags of the remote device.
> +
> + int16 NegotiatedMTU [readonly, optional, experimental]
> +
> + The ATT MTU negotiated with the connected host.
> + Available only once MTU negotiation is complete.
Despite being odd that we start exposing transport specific details on
device interface, there maybe a chance this is useful if we can
properly detect BR/EDR MTU not only LE, though this means checking the
via bt_att API not bt_gatt_server. Obviously, this would have to be
exposed even before it is negotiated since LE start with 23 but on
BR/EDR this is negotiated via L2CAP signaling, and perhaps it should
not really be MTU that we expose but the actual payload without the
headers that bluetoothd takes care of adding.
> diff --git a/src/device.c b/src/device.c
> index 8693eb8..1b171ee 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -930,6 +930,35 @@ static gboolean dev_property_exists_tx_power(const GDBusPropertyTable *property,
> return TRUE;
> }
>
> +static gboolean dev_property_get_negotiated_mtu(
> + const GDBusPropertyTable * property,
> + DBusMessageIter *iter, void *data)
> +{
> + const struct btd_device *dev = data;
> + dbus_int16_t val = 0;
> + int err;
> +
> + err = bt_gatt_server_get_negotiated_mtu(dev->server, &val);
> + if (err != 0)
> + return FALSE;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_INT16, &val);
> +
> + return TRUE;
> +}
> +
> +static gboolean dev_property_exists_negotiated_mtu(
> + const GDBusPropertyTable * property,
> + void *data)
> +{
> + const struct btd_device *dev = data;
> + int err;
> +
> + err = bt_gatt_server_get_negotiated_mtu(dev->server, NULL);
> +
> + return err == 0;
> +}
> +
> static gboolean
> dev_property_get_svc_resolved(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *data)
> @@ -2561,6 +2590,8 @@ static const GDBusPropertyTable device_properties[] = {
> NULL, dev_property_service_data_exist },
> { "TxPower", "n", dev_property_get_tx_power, NULL,
> dev_property_exists_tx_power },
> + { "NegotiatedMTU", "n", dev_property_get_negotiated_mtu, NULL,
> + dev_property_exists_negotiated_mtu },
AttMTU or GattMTU would probably indicate better what this is for.
> { "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/gatt-server.c b/src/shared/gatt-server.c
> index 79e01c8..2727f09 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -89,6 +89,7 @@ struct bt_gatt_server {
> struct bt_att *att;
> int ref_count;
> uint16_t mtu;
> + bool mtu_negotiated;
>
> unsigned int mtu_id;
> unsigned int read_by_grp_type_id;
> @@ -1378,12 +1379,25 @@ static void exchange_mtu_cb(uint8_t opcode, const void *pdu,
>
> /* Set MTU to be the minimum */
> server->mtu = final_mtu;
> + server->mtu_negotiated = true;
We do have bt_att_*_mtu for this and the ready callback should
indicate when we are done with exchange, or perhaps we want a callback
for when the MTU is changed at bt_att in case we there is a race where
the application wants to write/read immediately, though there will
always be a small race since the attributes can be cached once
connected the application may write/read while exchange is pending.
> bt_att_set_mtu(server->att, final_mtu);
>
> util_debug(server->debug_callback, server->debug_data,
> "MTU exchange complete, with MTU: %u", final_mtu);
> }
>
> +int bt_gatt_server_get_negotiated_mtu(const struct bt_gatt_server *server,
> + int16_t *mtu)
> +{
> + if (!server || !server->mtu_negotiated)
> + return -1;
> +
> + if (mtu)
> + *mtu = server->mtu;
> +
> + return 0;
> +}
> +
> static bool gatt_server_register_att_handlers(struct bt_gatt_server *server)
> {
> /* Exchange MTU */
> @@ -1493,6 +1507,7 @@ struct bt_gatt_server *bt_gatt_server_new(struct gatt_db *db,
> server->db = gatt_db_ref(db);
> server->att = bt_att_ref(att);
> server->mtu = MAX(mtu, BT_ATT_DEFAULT_LE_MTU);
> + server->mtu_negotiated = false;
> server->max_prep_queue_len = DEFAULT_MAX_PREP_QUEUE_LEN;
> server->prep_queue = queue_new();
>
> 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 *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);
> --
> 2.10.0
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
