Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CABBYNZ+mRgkn_PiL5=4NuQQudrQZQvdYqVuab3m_S7pRvj9YeQ@mail.gmail.com>
 <20230518071507.4640-1-iulia.tanasescu@nxp.com>
In-Reply-To: <20230518071507.4640-1-iulia.tanasescu@nxp.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 18 May 2023 10:13:25 -0700
Message-ID: <CABBYNZ+M7yS0tEcuoyUBoPnfqa3SiJ3z_nvTU+vLwHpCmSU_Aw@mail.gmail.com>
Subject: Re: [PATCH 1/1] Bluetooth: Add support for creating multiple BISes
To:     Iulia Tanasescu <iulia.tanasescu@nxp.com>
Cc:     linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Iulia,

On Thu, May 18, 2023 at 12:15=E2=80=AFAM Iulia Tanasescu
<iulia.tanasescu@nxp.com> wrote:
>
> Hi Luiz,
>
> Thank you for the review, let me better explain the flow that I proposed,
> because I think I should have added a more detailed description to this
> patch.
>
> I added the num_bis field to the QoS structure so that the user can
> specify from the start the total number of connections that will be
> opened for the BIG, but one socket will always be connected to an
> unique BIS.
>
> So the user will first open a socket, set the QoS options with the
> num_bis parameter set to the number of BISes, and then the user will
> call connect on that socket. The BIG will be created with the specified
> number of BISes, but the socket will only be connected to one of them.
> The rest of the connection handles will be stored in the "bigs" queue
> that I added.
>
> Later on, the user might decide to open new sockets, for the rest of
> the BISes that are created and stored in the queue. In this case,
> the connect API on the socket will not issue the LE Create BIG command
> again, but it will extract a connection handle from the queue and the
> socket will be connected instantly.

Ok, that makes more sense now.

> As for the HCI_CONN_PER_ADV flag, I noticed that it was only checked
> in the "bis_cleanup" function, to decide whether the advertising set
> and the BIG should be terminated. I removed it because now I am only
> terminating the advertising set and the BIG if all of the BIS handles
> have been assigned and no other BISes are in the BT_CONNECTED state
> for that BIG, so I thought I might not need the flag anymore.

Hmm, I think Ive added it in case the BIG was not created yet but the
socket is terminated then we need to clean up the periodic advertising
set since we don't have a hci_conn yet.

> I think it's a better idea to use DEFER_SETUP for binding multiple
> BISes to a BIG, instead of using the num_bis QoS parameter, so that
> I can keep each socket completely separated from information about
> other connections, so I will update the implementation.

Yeah, I think adding BIS on-demand based on the number of sockets open
is more consistent to how we handle that in case of unicast.

> Regards,
> Iulia


--=20
Luiz Augusto von Dentz