| Message ID | 20240108183938.468426-5-verdre@v0yd.nl |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: Improve retrying of connection attempts | expand |
On 1/8/24 19:39, Jonas Dreßler wrote: > With the last commit we moved to using the hci_sync queue for "Create > Connection" requests, removing the need for retrying the paging after > finished/failed "Create Connection" requests and after the end of > inquiries. > > hci_conn_check_pending() was used to trigger this retry, we can remove it > now. > > Note that we can also remove the special handling for COMMAND_DISALLOWED > errors in the completion handler of "Create Connection", because "Create > Connection" requests are now always serialized. > > This is somewhat reverting commit 4c67bc74f016 ("[Bluetooth] Support > concurrent connect requests"). > > With this, the BT_CONNECT2 state of ACL hci_conn objects should now be > back to meaning only one thing: That we received a connection request > from another device (see hci_conn_request_evt), but the actual connect > should be deferred. > --- > include/net/bluetooth/hci_core.h | 1 - > net/bluetooth/hci_conn.c | 16 ---------------- > net/bluetooth/hci_event.c | 21 ++++----------------- > 3 files changed, 4 insertions(+), 34 deletions(-) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index 2c30834c1..d7483958d 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -1330,7 +1330,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, > u8 role); > void hci_conn_del(struct hci_conn *conn); > void hci_conn_hash_flush(struct hci_dev *hdev); > -void hci_conn_check_pending(struct hci_dev *hdev); > > struct hci_chan *hci_chan_create(struct hci_conn *conn); > void hci_chan_del(struct hci_chan *chan); > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > index 541d55301..22033057b 100644 > --- a/net/bluetooth/hci_conn.c > +++ b/net/bluetooth/hci_conn.c > @@ -2534,22 +2534,6 @@ void hci_conn_hash_flush(struct hci_dev *hdev) > } > } > > -/* Check pending connect attempts */ > -void hci_conn_check_pending(struct hci_dev *hdev) > -{ > - struct hci_conn *conn; > - > - BT_DBG("hdev %s", hdev->name); > - > - hci_dev_lock(hdev); > - > - conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2); > - if (conn) > - hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync, conn, NULL); > - > - hci_dev_unlock(hdev); > -} > - > static u32 get_link_mode(struct hci_conn *conn) > { > u32 link_mode = 0; > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index e8b4a0126..91973d6d1 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -117,8 +117,6 @@ static u8 hci_cc_inquiry_cancel(struct hci_dev *hdev, void *data, > hci_discovery_set_state(hdev, DISCOVERY_STOPPED); > hci_dev_unlock(hdev); > > - hci_conn_check_pending(hdev); > - > return rp->status; > } > > @@ -149,8 +147,6 @@ static u8 hci_cc_exit_periodic_inq(struct hci_dev *hdev, void *data, > > hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ); > > - hci_conn_check_pending(hdev); > - > return rp->status; > } > > @@ -2296,10 +2292,8 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status) > { > bt_dev_dbg(hdev, "status 0x%2.2x", status); > > - if (status) { > - hci_conn_check_pending(hdev); > + if (status) > return; > - } > > set_bit(HCI_INQUIRY, &hdev->flags); > } > @@ -2323,12 +2317,9 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) > > if (status) { > if (conn && conn->state == BT_CONNECT) { > - if (status != HCI_ERROR_COMMAND_DISALLOWED || conn->attempt > 2) { > - conn->state = BT_CLOSED; > - hci_connect_cfm(conn, status); > - hci_conn_del(conn); > - } else > - conn->state = BT_CONNECT2; > + conn->state = BT_CLOSED; > + hci_connect_cfm(conn, status); > + hci_conn_del(conn); > } > } else { > if (!conn) { > @@ -3020,8 +3011,6 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, void *data, > > bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); > > - hci_conn_check_pending(hdev); > - > if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags)) > return; > > @@ -3247,8 +3236,6 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, > > unlock: > hci_dev_unlock(hdev); > - > - hci_conn_check_pending(hdev); > } > > static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr) Please take a special look at this one: I'm not sure if I'm breaking the functionality of deferred connecting using BT_CONNECT2 in hci_conn_request_evt() here, as I don't see anywhere where we check for this state and establish a connection later. It seems that this is how hci_conn_request_evt() was initially written though, hci_conn_check_pending() only got introduced later and seems unrelated. Thanks, Jonas
Hi Jonas, On Mon, Jan 8, 2024 at 2:29 PM Jonas Dreßler <verdre@v0yd.nl> wrote: > > Hi Luiz, > > On 1/8/24 20:14, Luiz Augusto von Dentz wrote: > > Hi Jonas, > > > > On Mon, Jan 8, 2024 at 1:55 PM Jonas Dreßler <verdre@v0yd.nl> wrote: > >> > >> On 1/8/24 19:44, Jonas Dreßler wrote: > >>> On 1/8/24 19:39, Jonas Dreßler wrote: > >>>> With the last commit we moved to using the hci_sync queue for "Create > >>>> Connection" requests, removing the need for retrying the paging after > >>>> finished/failed "Create Connection" requests and after the end of > >>>> inquiries. > >>>> > >>>> hci_conn_check_pending() was used to trigger this retry, we can remove it > >>>> now. > >>>> > >>>> Note that we can also remove the special handling for COMMAND_DISALLOWED > >>>> errors in the completion handler of "Create Connection", because "Create > >>>> Connection" requests are now always serialized. > >>>> > >>>> This is somewhat reverting commit 4c67bc74f016 ("[Bluetooth] Support > >>>> concurrent connect requests"). > >>>> > >>>> With this, the BT_CONNECT2 state of ACL hci_conn objects should now be > >>>> back to meaning only one thing: That we received a connection request > >>>> from another device (see hci_conn_request_evt), but the actual connect > >>>> should be deferred. > >>>> --- > >>>> include/net/bluetooth/hci_core.h | 1 - > >>>> net/bluetooth/hci_conn.c | 16 ---------------- > >>>> net/bluetooth/hci_event.c | 21 ++++----------------- > >>>> 3 files changed, 4 insertions(+), 34 deletions(-) > >>>> > >>>> diff --git a/include/net/bluetooth/hci_core.h > >>>> b/include/net/bluetooth/hci_core.h > >>>> index 2c30834c1..d7483958d 100644 > >>>> --- a/include/net/bluetooth/hci_core.h > >>>> +++ b/include/net/bluetooth/hci_core.h > >>>> @@ -1330,7 +1330,6 @@ struct hci_conn *hci_conn_add(struct hci_dev > >>>> *hdev, int type, bdaddr_t *dst, > >>>> u8 role); > >>>> void hci_conn_del(struct hci_conn *conn); > >>>> void hci_conn_hash_flush(struct hci_dev *hdev); > >>>> -void hci_conn_check_pending(struct hci_dev *hdev); > >>>> struct hci_chan *hci_chan_create(struct hci_conn *conn); > >>>> void hci_chan_del(struct hci_chan *chan); > >>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > >>>> index 541d55301..22033057b 100644 > >>>> --- a/net/bluetooth/hci_conn.c > >>>> +++ b/net/bluetooth/hci_conn.c > >>>> @@ -2534,22 +2534,6 @@ void hci_conn_hash_flush(struct hci_dev *hdev) > >>>> } > >>>> } > >>>> -/* Check pending connect attempts */ > >>>> -void hci_conn_check_pending(struct hci_dev *hdev) > >>>> -{ > >>>> - struct hci_conn *conn; > >>>> - > >>>> - BT_DBG("hdev %s", hdev->name); > >>>> - > >>>> - hci_dev_lock(hdev); > >>>> - > >>>> - conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2); > >>>> - if (conn) > >>>> - hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync, > >>>> conn, NULL); > >>>> - > >>>> - hci_dev_unlock(hdev); > >>>> -} > >>>> - > >>>> static u32 get_link_mode(struct hci_conn *conn) > >>>> { > >>>> u32 link_mode = 0; > >>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > >>>> index e8b4a0126..91973d6d1 100644 > >>>> --- a/net/bluetooth/hci_event.c > >>>> +++ b/net/bluetooth/hci_event.c > >>>> @@ -117,8 +117,6 @@ static u8 hci_cc_inquiry_cancel(struct hci_dev > >>>> *hdev, void *data, > >>>> hci_discovery_set_state(hdev, DISCOVERY_STOPPED); > >>>> hci_dev_unlock(hdev); > >>>> - hci_conn_check_pending(hdev); > >>>> - > >>>> return rp->status; > >>>> } > >>>> @@ -149,8 +147,6 @@ static u8 hci_cc_exit_periodic_inq(struct hci_dev > >>>> *hdev, void *data, > >>>> hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ); > >>>> - hci_conn_check_pending(hdev); > >>>> - > >>>> return rp->status; > >>>> } > >>>> @@ -2296,10 +2292,8 @@ static void hci_cs_inquiry(struct hci_dev > >>>> *hdev, __u8 status) > >>>> { > >>>> bt_dev_dbg(hdev, "status 0x%2.2x", status); > >>>> - if (status) { > >>>> - hci_conn_check_pending(hdev); > >>>> + if (status) > >>>> return; > >>>> - } > >>>> set_bit(HCI_INQUIRY, &hdev->flags); > >>>> } > >>>> @@ -2323,12 +2317,9 @@ static void hci_cs_create_conn(struct hci_dev > >>>> *hdev, __u8 status) > >>>> if (status) { > >>>> if (conn && conn->state == BT_CONNECT) { > >>>> - if (status != HCI_ERROR_COMMAND_DISALLOWED || > >>>> conn->attempt > 2) { > >>>> - conn->state = BT_CLOSED; > >>>> - hci_connect_cfm(conn, status); > >>>> - hci_conn_del(conn); > >>>> - } else > >>>> - conn->state = BT_CONNECT2; > >>>> + conn->state = BT_CLOSED; > >>>> + hci_connect_cfm(conn, status); > >>>> + hci_conn_del(conn); > >>>> } > >>>> } else { > >>>> if (!conn) { > >>>> @@ -3020,8 +3011,6 @@ static void hci_inquiry_complete_evt(struct > >>>> hci_dev *hdev, void *data, > >>>> bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); > >>>> - hci_conn_check_pending(hdev); > >>>> - > >>>> if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags)) > >>>> return; > >>>> @@ -3247,8 +3236,6 @@ static void hci_conn_complete_evt(struct hci_dev > >>>> *hdev, void *data, > >>>> unlock: > >>>> hci_dev_unlock(hdev); > >>>> - > >>>> - hci_conn_check_pending(hdev); > >>>> } > >>>> static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr) > >>> > >>> Please take a special look at this one: I'm not sure if I'm breaking the > >>> functionality of deferred connecting using BT_CONNECT2 in > >>> hci_conn_request_evt() here, as I don't see anywhere where we check for > >>> this state and establish a connection later. > >>> > >>> It seems that this is how hci_conn_request_evt() was initially written > >>> though, hci_conn_check_pending() only got introduced later and seems > >>> unrelated. > >> > >> Ahh nevermind... The check for BT_CONNECT2 on "Conn Complete event" got > >> introduced with 4c67bc74f01 ([Bluetooth] Support concurrent connect > >> requests). And later the deferred connection setup on "Conn Request > >> event" got introduced with 20714bfef8 ("Bluetooth: Implement deferred > >> sco socket setup"). > >> > >> I assume the latter commit was relying on the "Create Connection" > >> request "Conn Complete event" that got introduced with the former commit > >> then? That would imply that we use BT_CONNECT2 if there's already a > >> "Create Connection" going on when the "Conn Request event" happens, and > >> we must wait for that existing request to finish.. Is that how those > >> deferred connections are supposed to work? > > > > Well if you are not sure that works we better make sure we have tests > > that cover this, for LE I know for sure it works because we have the > > likes of iso-tester that do connect 2 peers simultaneously, but for > > classic I don't recall having any test that does multiple connections. > > The sequential "Create Connection" logic works, I tested that (of course > I'm happy to add tests if it's not too much work). > > What I'm unsure about is if and how incoming connection requests from > other devices with HCI_PROTO_DEFER flag are supposed to work and whether > they are meant to trigger a "Create Connection" from us? For incoming connections on Classic that should result in an accept/reject connection command, so it should cause another Create Connection if that is what you are afraid of. > > > >>> > >>> Thanks, > >>> Jonas > > > > > >
Hi Jonas, On Mon, Jan 8, 2024 at 3:26 PM Jonas Dreßler <verdre@v0yd.nl> wrote: > > Hi Luiz, > > On 1/8/24 20:41, Luiz Augusto von Dentz wrote: > > Hi Jonas, > > > > On Mon, Jan 8, 2024 at 2:29 PM Jonas Dreßler <verdre@v0yd.nl> wrote: > >> > >> Hi Luiz, > >> > >> On 1/8/24 20:14, Luiz Augusto von Dentz wrote: > >>> Hi Jonas, > >>> > >>> On Mon, Jan 8, 2024 at 1:55 PM Jonas Dreßler <verdre@v0yd.nl> wrote: > >>>> > >>>> On 1/8/24 19:44, Jonas Dreßler wrote: > >>>>> On 1/8/24 19:39, Jonas Dreßler wrote: > >>>>>> With the last commit we moved to using the hci_sync queue for "Create > >>>>>> Connection" requests, removing the need for retrying the paging after > >>>>>> finished/failed "Create Connection" requests and after the end of > >>>>>> inquiries. > >>>>>> > >>>>>> hci_conn_check_pending() was used to trigger this retry, we can remove it > >>>>>> now. > >>>>>> > >>>>>> Note that we can also remove the special handling for COMMAND_DISALLOWED > >>>>>> errors in the completion handler of "Create Connection", because "Create > >>>>>> Connection" requests are now always serialized. > >>>>>> > >>>>>> This is somewhat reverting commit 4c67bc74f016 ("[Bluetooth] Support > >>>>>> concurrent connect requests"). > >>>>>> > >>>>>> With this, the BT_CONNECT2 state of ACL hci_conn objects should now be > >>>>>> back to meaning only one thing: That we received a connection request > >>>>>> from another device (see hci_conn_request_evt), but the actual connect > >>>>>> should be deferred. > >>>>>> --- > >>>>>> include/net/bluetooth/hci_core.h | 1 - > >>>>>> net/bluetooth/hci_conn.c | 16 ---------------- > >>>>>> net/bluetooth/hci_event.c | 21 ++++----------------- > >>>>>> 3 files changed, 4 insertions(+), 34 deletions(-) > >>>>>> > >>>>>> diff --git a/include/net/bluetooth/hci_core.h > >>>>>> b/include/net/bluetooth/hci_core.h > >>>>>> index 2c30834c1..d7483958d 100644 > >>>>>> --- a/include/net/bluetooth/hci_core.h > >>>>>> +++ b/include/net/bluetooth/hci_core.h > >>>>>> @@ -1330,7 +1330,6 @@ struct hci_conn *hci_conn_add(struct hci_dev > >>>>>> *hdev, int type, bdaddr_t *dst, > >>>>>> u8 role); > >>>>>> void hci_conn_del(struct hci_conn *conn); > >>>>>> void hci_conn_hash_flush(struct hci_dev *hdev); > >>>>>> -void hci_conn_check_pending(struct hci_dev *hdev); > >>>>>> struct hci_chan *hci_chan_create(struct hci_conn *conn); > >>>>>> void hci_chan_del(struct hci_chan *chan); > >>>>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > >>>>>> index 541d55301..22033057b 100644 > >>>>>> --- a/net/bluetooth/hci_conn.c > >>>>>> +++ b/net/bluetooth/hci_conn.c > >>>>>> @@ -2534,22 +2534,6 @@ void hci_conn_hash_flush(struct hci_dev *hdev) > >>>>>> } > >>>>>> } > >>>>>> -/* Check pending connect attempts */ > >>>>>> -void hci_conn_check_pending(struct hci_dev *hdev) > >>>>>> -{ > >>>>>> - struct hci_conn *conn; > >>>>>> - > >>>>>> - BT_DBG("hdev %s", hdev->name); > >>>>>> - > >>>>>> - hci_dev_lock(hdev); > >>>>>> - > >>>>>> - conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2); > >>>>>> - if (conn) > >>>>>> - hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync, > >>>>>> conn, NULL); > >>>>>> - > >>>>>> - hci_dev_unlock(hdev); > >>>>>> -} > >>>>>> - > >>>>>> static u32 get_link_mode(struct hci_conn *conn) > >>>>>> { > >>>>>> u32 link_mode = 0; > >>>>>> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > >>>>>> index e8b4a0126..91973d6d1 100644 > >>>>>> --- a/net/bluetooth/hci_event.c > >>>>>> +++ b/net/bluetooth/hci_event.c > >>>>>> @@ -117,8 +117,6 @@ static u8 hci_cc_inquiry_cancel(struct hci_dev > >>>>>> *hdev, void *data, > >>>>>> hci_discovery_set_state(hdev, DISCOVERY_STOPPED); > >>>>>> hci_dev_unlock(hdev); > >>>>>> - hci_conn_check_pending(hdev); > >>>>>> - > >>>>>> return rp->status; > >>>>>> } > >>>>>> @@ -149,8 +147,6 @@ static u8 hci_cc_exit_periodic_inq(struct hci_dev > >>>>>> *hdev, void *data, > >>>>>> hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ); > >>>>>> - hci_conn_check_pending(hdev); > >>>>>> - > >>>>>> return rp->status; > >>>>>> } > >>>>>> @@ -2296,10 +2292,8 @@ static void hci_cs_inquiry(struct hci_dev > >>>>>> *hdev, __u8 status) > >>>>>> { > >>>>>> bt_dev_dbg(hdev, "status 0x%2.2x", status); > >>>>>> - if (status) { > >>>>>> - hci_conn_check_pending(hdev); > >>>>>> + if (status) > >>>>>> return; > >>>>>> - } > >>>>>> set_bit(HCI_INQUIRY, &hdev->flags); > >>>>>> } > >>>>>> @@ -2323,12 +2317,9 @@ static void hci_cs_create_conn(struct hci_dev > >>>>>> *hdev, __u8 status) > >>>>>> if (status) { > >>>>>> if (conn && conn->state == BT_CONNECT) { > >>>>>> - if (status != HCI_ERROR_COMMAND_DISALLOWED || > >>>>>> conn->attempt > 2) { > >>>>>> - conn->state = BT_CLOSED; > >>>>>> - hci_connect_cfm(conn, status); > >>>>>> - hci_conn_del(conn); > >>>>>> - } else > >>>>>> - conn->state = BT_CONNECT2; > >>>>>> + conn->state = BT_CLOSED; > >>>>>> + hci_connect_cfm(conn, status); > >>>>>> + hci_conn_del(conn); > >>>>>> } > >>>>>> } else { > >>>>>> if (!conn) { > >>>>>> @@ -3020,8 +3011,6 @@ static void hci_inquiry_complete_evt(struct > >>>>>> hci_dev *hdev, void *data, > >>>>>> bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); > >>>>>> - hci_conn_check_pending(hdev); > >>>>>> - > >>>>>> if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags)) > >>>>>> return; > >>>>>> @@ -3247,8 +3236,6 @@ static void hci_conn_complete_evt(struct hci_dev > >>>>>> *hdev, void *data, > >>>>>> unlock: > >>>>>> hci_dev_unlock(hdev); > >>>>>> - > >>>>>> - hci_conn_check_pending(hdev); > >>>>>> } > >>>>>> static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr) > >>>>> > >>>>> Please take a special look at this one: I'm not sure if I'm breaking the > >>>>> functionality of deferred connecting using BT_CONNECT2 in > >>>>> hci_conn_request_evt() here, as I don't see anywhere where we check for > >>>>> this state and establish a connection later. > >>>>> > >>>>> It seems that this is how hci_conn_request_evt() was initially written > >>>>> though, hci_conn_check_pending() only got introduced later and seems > >>>>> unrelated. > >>>> > >>>> Ahh nevermind... The check for BT_CONNECT2 on "Conn Complete event" got > >>>> introduced with 4c67bc74f01 ([Bluetooth] Support concurrent connect > >>>> requests). And later the deferred connection setup on "Conn Request > >>>> event" got introduced with 20714bfef8 ("Bluetooth: Implement deferred > >>>> sco socket setup"). > >>>> > >>>> I assume the latter commit was relying on the "Create Connection" > >>>> request "Conn Complete event" that got introduced with the former commit > >>>> then? That would imply that we use BT_CONNECT2 if there's already a > >>>> "Create Connection" going on when the "Conn Request event" happens, and > >>>> we must wait for that existing request to finish.. Is that how those > >>>> deferred connections are supposed to work? > >>> > >>> Well if you are not sure that works we better make sure we have tests > >>> that cover this, for LE I know for sure it works because we have the > >>> likes of iso-tester that do connect 2 peers simultaneously, but for > >>> classic I don't recall having any test that does multiple connections. > >> > >> The sequential "Create Connection" logic works, I tested that (of course > >> I'm happy to add tests if it's not too much work). > >> > >> What I'm unsure about is if and how incoming connection requests from > >> other devices with HCI_PROTO_DEFER flag are supposed to work and whether > >> they are meant to trigger a "Create Connection" from us? > > > > For incoming connections on Classic that should result in an > > accept/reject connection command, so it should cause another Create > > Connection if that is what you are afraid of. > > > > Hmm, do you mean it *shouldn't* cause another "Create Connection"? Yeah, sorry about that, it is Monday I should probably double check if what I wrote makes any sense before sending :D > I just checked in the spec: It sounds like once we send the "Accept > Connection Request" to the controller, the controller takes care of > establishing the connection by itself (no "Create Connection" > necessary), and will then later give us a "Connection Complete" event to > indicate that the connection is done. Yep, it will follow up with a Connection Complete. > If I'm reading all this correctly, that sounds like my commit is > correct, and we had a bug in this logic before by interpreting > BT_CONNECT2 in two different ways. > > >>> > >>>>> > >>>>> Thanks, > >>>>> Jonas > >>> > >>> > >>> > > > > > >
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 2c30834c1..d7483958d 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1330,7 +1330,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, u8 role); void hci_conn_del(struct hci_conn *conn); void hci_conn_hash_flush(struct hci_dev *hdev); -void hci_conn_check_pending(struct hci_dev *hdev); struct hci_chan *hci_chan_create(struct hci_conn *conn); void hci_chan_del(struct hci_chan *chan); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 541d55301..22033057b 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -2534,22 +2534,6 @@ void hci_conn_hash_flush(struct hci_dev *hdev) } } -/* Check pending connect attempts */ -void hci_conn_check_pending(struct hci_dev *hdev) -{ - struct hci_conn *conn; - - BT_DBG("hdev %s", hdev->name); - - hci_dev_lock(hdev); - - conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2); - if (conn) - hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync, conn, NULL); - - hci_dev_unlock(hdev); -} - static u32 get_link_mode(struct hci_conn *conn) { u32 link_mode = 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index e8b4a0126..91973d6d1 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -117,8 +117,6 @@ static u8 hci_cc_inquiry_cancel(struct hci_dev *hdev, void *data, hci_discovery_set_state(hdev, DISCOVERY_STOPPED); hci_dev_unlock(hdev); - hci_conn_check_pending(hdev); - return rp->status; } @@ -149,8 +147,6 @@ static u8 hci_cc_exit_periodic_inq(struct hci_dev *hdev, void *data, hci_dev_clear_flag(hdev, HCI_PERIODIC_INQ); - hci_conn_check_pending(hdev); - return rp->status; } @@ -2296,10 +2292,8 @@ static void hci_cs_inquiry(struct hci_dev *hdev, __u8 status) { bt_dev_dbg(hdev, "status 0x%2.2x", status); - if (status) { - hci_conn_check_pending(hdev); + if (status) return; - } set_bit(HCI_INQUIRY, &hdev->flags); } @@ -2323,12 +2317,9 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (status) { if (conn && conn->state == BT_CONNECT) { - if (status != HCI_ERROR_COMMAND_DISALLOWED || conn->attempt > 2) { - conn->state = BT_CLOSED; - hci_connect_cfm(conn, status); - hci_conn_del(conn); - } else - conn->state = BT_CONNECT2; + conn->state = BT_CLOSED; + hci_connect_cfm(conn, status); + hci_conn_del(conn); } } else { if (!conn) { @@ -3020,8 +3011,6 @@ static void hci_inquiry_complete_evt(struct hci_dev *hdev, void *data, bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); - hci_conn_check_pending(hdev); - if (!test_and_clear_bit(HCI_INQUIRY, &hdev->flags)) return; @@ -3247,8 +3236,6 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, unlock: hci_dev_unlock(hdev); - - hci_conn_check_pending(hdev); } static void hci_reject_conn(struct hci_dev *hdev, bdaddr_t *bdaddr)
With the last commit we moved to using the hci_sync queue for "Create Connection" requests, removing the need for retrying the paging after finished/failed "Create Connection" requests and after the end of inquiries. hci_conn_check_pending() was used to trigger this retry, we can remove it now. Note that we can also remove the special handling for COMMAND_DISALLOWED errors in the completion handler of "Create Connection", because "Create Connection" requests are now always serialized. This is somewhat reverting commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect requests"). With this, the BT_CONNECT2 state of ACL hci_conn objects should now be back to meaning only one thing: That we received a connection request from another device (see hci_conn_request_evt), but the actual connect should be deferred. --- include/net/bluetooth/hci_core.h | 1 - net/bluetooth/hci_conn.c | 16 ---------------- net/bluetooth/hci_event.c | 21 ++++----------------- 3 files changed, 4 insertions(+), 34 deletions(-)