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: <20230413183113.896669-1-luiz.dentz@gmail.com> <20230413183113.896669-2-luiz.dentz@gmail.com>
 <1ae133312ec9d702ebf18cbf3a2b106e8c27acd4.camel@iki.fi>
In-Reply-To: <1ae133312ec9d702ebf18cbf3a2b106e8c27acd4.camel@iki.fi>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 14 Apr 2023 13:13:00 -0700
Message-ID: <CABBYNZ+eDB7ajTM2ktkwSrQB-8RWrhixkChW9DKfV3aazX-RSw@mail.gmail.com>
Subject: Re: [PATCH v2 2/3] Bluetooth: hci_conn: Fix not matching by CIS ID
To:     Pauli Virtanen <pav@iki.fi>
Cc:     linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Pauli,

On Fri, Apr 14, 2023 at 12:53=E2=80=AFPM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> to, 2023-04-13 kello 11:31 -0700, Luiz Augusto von Dentz kirjoitti:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This fixes only matching CIS by address which prevents creating new hco=
n
> > if upper layer is requesting a specific CIS ID.
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> >  include/net/bluetooth/hci_core.h | 7 ++++++-
> >  net/bluetooth/hci_conn.c         | 2 +-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/h=
ci_core.h
> > index 4fe1e71cb9d8..6f5e8594ff2d 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -1180,7 +1180,8 @@ static inline struct hci_conn *hci_conn_hash_look=
up_le(struct hci_dev *hdev,
> >
> >  static inline struct hci_conn *hci_conn_hash_lookup_cis(struct hci_dev=
 *hdev,
> >                                                       bdaddr_t *ba,
> > -                                                     __u8 ba_type)
> > +                                                     __u8 ba_type,
> > +                                                     __u8 id)
> >  {
> >       struct hci_conn_hash *h =3D &hdev->conn_hash;
> >       struct hci_conn  *c;
> > @@ -1191,6 +1192,10 @@ static inline struct hci_conn *hci_conn_hash_loo=
kup_cis(struct hci_dev *hdev,
> >               if (c->type !=3D ISO_LINK)
> >                       continue;
> >
> > +             /* Match CIS ID if set */
> > +             if (id !=3D BT_ISO_QOS_CIS_UNSET && id !=3D c->iso_qos.uc=
ast.cis)
> > +                     continue;
> > +
>
> Should this also check the CIG ID?
>
> Core v5.3 4.E Sec. 5.3: "The CIS_ID has a separate number space for
> each CIG_ID.", and I didn't manage to find restriction that you
> couldn't have the same peripheral in multiple CIG.

That is a good point, it seems the BAP Audio Configurations are
limited to just one CIG but in theory there could be multiple if they
don't need to be synchronized, I will fix that to check both CIG and
CIS.

> >               if (ba_type =3D=3D c->dst_type && !bacmp(&c->dst, ba)) {
> >                       rcu_read_unlock();
> >                       return c;
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 204164ee5f9a..b9ecfc782be9 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -1842,7 +1842,7 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hde=
v, bdaddr_t *dst,
> >  {
> >       struct hci_conn *cis;
> >
> > -     cis =3D hci_conn_hash_lookup_cis(hdev, dst, dst_type);
> > +     cis =3D hci_conn_hash_lookup_cis(hdev, dst, dst_type, qos->ucast.=
cis);
> >       if (!cis) {
> >               cis =3D hci_conn_add(hdev, ISO_LINK, dst, HCI_ROLE_MASTER=
);
> >               if (!cis)
>
> --
> Pauli Virtanen


--=20
Luiz Augusto von Dentz