Hi Pauli,
On Mon, Oct 10, 2022 at 12:08 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Hi Luiz,
>
> su, 2022-10-09 kello 14:45 -0700, Luiz Augusto von Dentz kirjoitti:
> [clip]
> > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > > index 9777e7b109ee..78d8b8b7fd72 100644
> > > --- a/net/bluetooth/hci_conn.c
> > > +++ b/net/bluetooth/hci_conn.c
> > > @@ -1691,12 +1691,19 @@ struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst,
> > > {
> > > struct hci_conn *cis;
> > >
> > > + /* Convert from ISO socket address type to HCI address type */
> > > + if (dst_type == BDADDR_LE_PUBLIC)
> > > + dst_type = ADDR_LE_DEV_PUBLIC;
> > > + else
> > > + dst_type = ADDR_LE_DEV_RANDOM;
> > > +
> > > cis = hci_conn_hash_lookup_cis(hdev, dst, dst_type);
> > > if (!cis) {
> > > cis = hci_conn_add(hdev, ISO_LINK, dst, HCI_ROLE_MASTER);
> > > if (!cis)
> > > return ERR_PTR(-ENOMEM);
> > > cis->cleanup = cis_cleanup;
> > > + cis->dst_type = dst_type;
> > > }
> > >
> > > if (cis->state == BT_CONNECTED)
> > > @@ -2075,20 +2082,21 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst,
> > > {
> > > struct hci_conn *le;
> > > struct hci_conn *cis;
> > > + u8 hci_dst_type;
> > >
> > > /* Convert from ISO socket address type to HCI address type */
> > > if (dst_type == BDADDR_LE_PUBLIC)
> > > - dst_type = ADDR_LE_DEV_PUBLIC;
> > > + hci_dst_type = ADDR_LE_DEV_PUBLIC;
> > > else
> > > - dst_type = ADDR_LE_DEV_RANDOM;
> > > + hci_dst_type = ADDR_LE_DEV_RANDOM;
> >
> > Nice catch, though I think we should make sure these types are not
> > from hci_conn.c as the name suggest these should be dealing HCI
> > procedures so it doesn't make much sense to propagate types other than
> > HCI.
>
> Not sure I parse right: You want to move the conversions to the
> callsite in iso.c? Or change the variable name here? If the former,
> there are a few other instances of these in hci_conn.c.
Lets move to iso.c and make the conversion before the calls into hci_conn.c.
> > > if (hci_dev_test_flag(hdev, HCI_ADVERTISING))
> > > - le = hci_connect_le(hdev, dst, dst_type, false,
> > > + le = hci_connect_le(hdev, dst, hci_dst_type, false,
> > > BT_SECURITY_LOW,
> > > HCI_LE_CONN_TIMEOUT,
> > > HCI_ROLE_SLAVE);
> > > else
> > > - le = hci_connect_le_scan(hdev, dst, dst_type,
> > > + le = hci_connect_le_scan(hdev, dst, hci_dst_type,
> > > BT_SECURITY_LOW,
> > > HCI_LE_CONN_TIMEOUT,
> > > CONN_REASON_ISO_CONNECT);
> > > --
> > > 2.37.3
> > >
> >
> > While at it probably makes sense to introduce a test to iso-tester
> > that uses random address rather than always using public, that way we
> > can make sure we exercise this code with CI.
--
Luiz Augusto von Dentz
