Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1733-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240209164630.92208-1-andrei.istodorescu@nxp.com> <20240209164630.92208-4-andrei.istodorescu@nxp.com>
In-Reply-To: <20240209164630.92208-4-andrei.istodorescu@nxp.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Fri, 9 Feb 2024 17:26:17 -0500
Message-ID: <CABBYNZ+RtEWtsB3Hq2L_DrL7Pgk66fcMF-5q5sJ-Fb8ph3oBGQ@mail.gmail.com>
Subject: Re: [PATCH BlueZ v2 3/6] btio: Check for non-zero qos before setting
To: Andrei Istodorescu <andrei.istodorescu@nxp.com>
Cc: linux-bluetooth@vger.kernel.org, mihai-octavian.urzica@nxp.com, 
	silviu.barbulescu@nxp.com, vlad.pruteanu@nxp.com, iulia.tanasescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Andrei,

On Fri, Feb 9, 2024 at 11:46=E2=80=AFAM Andrei Istodorescu
<andrei.istodorescu@nxp.com> wrote:
>
> ---
>  btio/btio.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index d30cfcac73ee..5a07f91c7d61 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -5,7 +5,7 @@
>   *
>   *  Copyright (C) 2009-2010  Marcel Holtmann <marcel@holtmann.org>
>   *  Copyright (C) 2009-2010  Nokia Corporation
> - *  Copyright 2023 NXP
> + *  Copyright 2023-2024 NXP
>   *
>   *
>   */
> @@ -1981,7 +1981,9 @@ static GIOChannel *create_io(gboolean server, struc=
t set_opts *opts,
>                 if (!sco_set(sock, opts->mtu, opts->voice, err))
>                         goto failed;
>                 break;
> -       case BT_IO_ISO:
> +       case BT_IO_ISO: {
> +               uint8_t zeroes[sizeof(opts->qos)] =3D {0};
> +
>                 sock =3D socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_ISO=
);
>                 if (sock < 0) {
>                         ERROR_FAILED(err, "socket(SEQPACKET, ISO)", errno=
);
> @@ -1992,12 +1994,14 @@ static GIOChannel *create_io(gboolean server, str=
uct set_opts *opts,
>                                  &opts->dst, opts->dst_type, opts->bc_sid=
,
>                                  opts->bc_num_bis, opts->bc_bis, err) < 0=
)
>                         goto failed;
> -               if (!iso_set_qos(sock, &opts->qos, err))
> -                       goto failed;
> +               if (memcmp(&opts->qos, zeroes, sizeof(opts->qos)))
> +                       if (!iso_set_qos(sock, &opts->qos, err))
> +                               goto failed;
>                 if (opts->base.base_len)
>                         if (!iso_set_base(sock, &opts->base, err))
>                                 goto failed;
>                 break;
> +       }
>         case BT_IO_INVALID:
>         default:
>                 g_set_error(err, BT_IO_ERROR, EINVAL,
> --
> 2.40.1

This is probably not the right layer to do this since btio doesn't
know what can be consider valid for BT_IO_ISO, specially considering
there could be different QOS settings that are not for audio streaming
in the future, so Id suggest to move this on top of btio, perhaps in
bap plugin.


--=20
Luiz Augusto von Dentz