Re: [PATCH v2] Bluetooth: Use hci_conn data to handle failed LE Connection Complete

[Marcel Holtmann <marcel@holtmann.org>]

Hi Andrzej,

> >>       hci_dev_lock(hdev);
> >>
> >> +     if (ev->status) {
> >> +             conn = hci_conn_hash_lookup_state(hdev, LE_LINK, BT_CONNECT);
> >> +             if (!conn)
> >> +                     goto unlock;
> >> +
> >> +             mgmt_connect_failed(hdev, &conn->dst, conn->type,
> >> +                                 conn->dst_type, ev->status);
> >> +             hci_proto_connect_cfm(conn, ev->status);
> >> +             conn->state = BT_CLOSED;
> >> +             hci_conn_del(conn);
> >> +             goto unlock;
> >> +     }
> >> +
> >>       conn = hci_conn_hash_lookup_ba(hdev, LE_LINK, &ev->bdaddr);
> >>       if (!conn) {
> >>               conn = hci_conn_add(hdev, LE_LINK, &ev->bdaddr);
> >
> > this change is wrong. We are now treating every single adapter as being
> > broken. That is not acceptable.
> 
> Why do you think these adapters are broken? As I explained in cover
> letter for v1, spec does not require peer address to be provided in
> Connection Complete which is reasonable since we can only have one
> pending connection request. Also as Claudio and Andre noticed such
> behaviour could be to simplify whitelist implementation - in case of
> connection request using whitelist it does not make sense to include
> specific peer address in event.

what has whitelist behavior to do with this event in the failure case?

> > We should only add a tweak if the BD_ADDR parameter is BDADDR_ANY and
> > not as a general rule. In addition if we do this, we need to print a
> > warning to dmesg to make this known.
> 
> Perhaps we can just add warning in case BD_ADDR is not BDADDR_ANY and
> we cannot find hci_conn for it - in such case most probably something
> went wrong.

What are the adapters from Broadcom, CSR, TI and ST are returning in a
failure case? Are they all omitting the BD_ADDR value?

Regards

Marcel