Hi Johan, On 05/11/13 05:09, Johan Hedberg wrote:
Hi Andre, On Mon, Nov 04, 2013, Andre Guedes wrote:According to b644ba336 (patch that introduced HCI_CONN_MGMT_CONNECTED flag), the HCI_CONN_MGMT_CONNECTED flag tracks when mgmt has been notified about the connection. That being said, there is no point in checking this flag in hci_ disconn_complete_evt() since neither mgmt_disconnect_failed() nor mgmt_device_disconnected() depend on it. Below follows more details: * mgmt_disconnect_failed() removes pending MGMT_OP_DISCONNECT commands, it doesn't matter if that connection was notified or not. * mgmt_device_disconnected() sends the mgmt event only if the link type is ACL_LINK or LE_LINK. For those connection type, the flag is always set. So this patch removes the HCI_CONN_MGMT_CONNECTED check. Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx> --- net/bluetooth/hci_event.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 142aa61..560296d 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1792,16 +1792,14 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) if (ev->status == 0) conn->state = BT_CLOSED; - if (test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags)) { - if (ev->status) { - mgmt_disconnect_failed(hdev, &conn->dst, conn->type, - conn->dst_type, ev->status); - } else { - u8 reason = hci_to_mgmt_reason(ev->reason); + if (ev->status) { + mgmt_disconnect_failed(hdev, &conn->dst, conn->type, + conn->dst_type, ev->status); + } else { + u8 reason = hci_to_mgmt_reason(ev->reason); - mgmt_device_disconnected(hdev, &conn->dst, conn->type, - conn->dst_type, reason); - } + mgmt_device_disconnected(hdev, &conn->dst, conn->type, + conn->dst_type, reason); } if (ev->status == 0) {It looks to me like this would cause an invalid mgmt_device_disconnected event to be sent of the ACL goes down before we notify over mgmt that it's connected. This could e.g. happen if the ACL disconnection happens before we complete the implicit name resolving (which is the main reason why this flag exists to begin with). Am I missing something here?
I guess you are right. I'll fix this patch by checking the flag before calling mgmt_device_disconnected().
Thanks for your review. Andre -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
