[lvc-project] [PATCH BlueZ 1/2] audio: actually try to enable MTU auto-tuning

[Luiz Augusto von Dentz]

Hi Fedor,

On Tue, Jan 28, 2025 at 4:04 PM Fedor Pchelkin <pchelkin at ispras.ru> wrote:
>
> A "0" for the input MTU passed to the underlying socket is supposed to
> indicate that its value should be determined by the L2CAP layer.
> However, the current code treats a zero imtu just as if there is
> nothing to change.
>
> Introduce an additional flag to indicate that the zero imtu is
> explicitly requested by the caller for the purpose of auto-tuning.
> Otherwise, the similar behavior remains.
>
> Found by Linux Verification Center (linuxtesting.org).
>
> Fixes: ae5be371a9f5 ("avdtp: Enable MTU auto tunning")
> ---
>  btio/btio.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index 2d277e409..74a4003b6 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -66,6 +66,7 @@ struct set_opts {
>         uint16_t imtu;
>         uint16_t omtu;
>         int central;
> +       uint8_t auto_mtu;
>         uint8_t mode;
>         int flushable;
>         uint32_t priority;
> @@ -610,7 +611,7 @@ static uint8_t mode_l2mode(uint8_t mode)
>  }
>
>  static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
> -                                               uint8_t mode, GError **err)
> +                               uint8_t auto_mtu, uint8_t mode, GError **err)
>  {
>         struct l2cap_options l2o;
>         socklen_t len;
> @@ -622,7 +623,7 @@ static gboolean set_l2opts(int sock, uint16_t imtu, uint16_t omtu,
>                 return FALSE;
>         }
>
> -       if (imtu)
> +       if (imtu || auto_mtu)
>                 l2o.imtu = imtu;

We might need to do some more special handling for auto_mtu, so in
case it fail we retry with the default values instead.

>         if (omtu)
>                 l2o.omtu = omtu;
> @@ -666,17 +667,17 @@ static gboolean set_le_mode(int sock, uint8_t mode, GError **err)
>  }
>
>  static gboolean l2cap_set(int sock, uint8_t src_type, int sec_level,
> -                               uint16_t imtu, uint16_t omtu, uint8_t mode,
> -                               int central, int flushable, uint32_t priority,
> -                               GError **err)
> +                               uint16_t imtu, uint16_t omtu, uint8_t auto_mtu,
> +                               uint8_t mode, int central, int flushable,
> +                               uint32_t priority, GError **err)
>  {
> -       if (imtu || omtu || mode) {
> +       if (imtu || omtu || auto_mtu || mode) {
>                 gboolean ret = FALSE;
>
>                 if (src_type == BDADDR_BREDR)
> -                       ret = set_l2opts(sock, imtu, omtu, mode, err);
> +                       ret = set_l2opts(sock, imtu, omtu, auto_mtu, mode, err);

Perhaps here we do:

if (ret && auto_mtu)
  ret = set_l2opts(sock, imtu, omtu, false, mode, err);

Thoughts?

>                 else {
> -                       if (imtu)
> +                       if (imtu || auto_mtu)
>                                 ret = set_le_imtu(sock, imtu, err);
>
>                         if (ret && mode)
> @@ -986,6 +987,8 @@ static gboolean parse_set_opts(struct set_opts *opts, GError **err,
>                         opts->imtu = va_arg(args, int);
>                         if (!opts->mtu)
>                                 opts->mtu = opts->imtu;
> +                       if (!opts->imtu)
> +                               opts->auto_mtu = 1;
>                         break;
>                 case BT_IO_OPT_CENTRAL:
>                         opts->central = va_arg(args, gboolean);
> @@ -1890,7 +1893,7 @@ gboolean bt_io_set(GIOChannel *io, GError **err, BtIOOption opt1, ...)
>         switch (type) {
>         case BT_IO_L2CAP:
>                 return l2cap_set(sock, opts.src_type, opts.sec_level, opts.imtu,
> -                                       opts.omtu, opts.mode, opts.central,
> +                                       opts.omtu, opts.auto_mtu, opts.mode, opts.central,
>                                         opts.flushable, opts.priority, err);
>         case BT_IO_RFCOMM:
>                 return rfcomm_set(sock, opts.sec_level, opts.central, err);
> @@ -1941,7 +1944,7 @@ static GIOChannel *create_io(gboolean server, struct set_opts *opts,
>                                 server ? opts->psm : 0, opts->cid, err) < 0)
>                         goto failed;
>                 if (!l2cap_set(sock, opts->src_type, opts->sec_level,
> -                               opts->imtu, opts->omtu, opts->mode,
> +                               opts->imtu, opts->omtu, opts->auto_mtu, opts->mode,
>                                 opts->central, opts->flushable, opts->priority,
>                                 err))
>                         goto failed;
> --
> 2.39.5
>


-- 
Luiz Augusto von Dentz